Conversation
This was referenced May 28, 2026
bolinfest
force-pushed
the
pr24980
branch
2 times, most recently
from
9bc6a27 to
b56c3ae
Compare
jif-oai
approved these changes
Jun 1, 2026
| } | ||
| } | ||
|
|
||
| fn unified_exec_should_include_shell_parameter(turn_context: &TurnContext) -> bool { |
Collaborator
There was a problem hiding this comment.
I think this still leaves shell as a silent no-op in mixed-env sessions: once any remote env exists we expose it, but local execs still ignore it under zsh-for k
Should we reject shell for local targets instead of silently dropping it?
Collaborator
Author
There was a problem hiding this comment.
[codex] Agreed. This is now changed on #24980: local zsh-fork unified exec rejects an explicit shell instead of silently ignoring it, with an error telling the model to omit shell or target a remote environment. Remote targets still map to direct unified exec via shell_mode_for_environment(), so their shell parameter remains supported.
bolinfest
force-pushed
the
pr24980
branch
2 times, most recently
from
9bc8a51 to
79a708f
Compare
bolinfest
added a commit
that referenced
this pull request
Jun 1, 2026
## Why `shell_zsh_fork` and unified exec need to remain independently controllable for enterprise rollouts, but we also need a third mode that composes them. That composed mode is intended to preserve unified exec command lifecycle support while letting the zsh fork provide more accurate `execv(2)` interception. Enabling `unified_exec_zsh_fork` by itself is intentionally not sufficient. It is a composition gate, not a dependency-enabling shortcut: - `unified_exec` selects the PTY-backed unified exec tool. - `shell_zsh_fork` opts into the zsh fork backend. - `unified_exec_zsh_fork` only allows those two already-enabled modes to be composed so local zsh unified exec commands can launch through the zsh fork. This separation is deliberate. Enterprises and staged rollouts must be able to enable or disable unified exec and zsh-fork independently. If `unified_exec_zsh_fork` implied either dependency, then enabling one under-development composition flag would silently activate a shell backend that the configured feature set left disabled. This PR introduces only the configuration and planning gate for that composition. Existing `shell_zsh_fork` behavior continues to use the standalone shell tool unless the new composition feature is explicitly enabled alongside both dependencies. ## What Changed - Added the under-development feature flag `unified_exec_zsh_fork`. - Added `UnifiedExecFeatureMode` so the three input feature flags collapse into `Disabled`, `Direct`, or `ZshFork` mode before tool planning. - Updated tool selection so zsh-fork composition requires `unified_exec`, `shell_zsh_fork`, and `unified_exec_zsh_fork`. - Kept the existing standalone zsh-fork shell tool behavior when only `shell_zsh_fork` is enabled. - Updated config schema output for the new feature flag. ## Verification - Added feature and tool-config coverage for the new gate. - Added planner coverage proving `shell_zsh_fork` remains standalone until composition is explicitly enabled. - Ran focused tests for `codex-features`, `codex-tools`, and the affected `codex-core` planner case. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/24979). * #24982 * #24981 * #24980 * __->__ #24979
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
When unified exec is configured to launch through the zsh fork, local commands should not let the model override the shell binary with the
shellparameter. The configured zsh fork is the mechanism that makesexecv(2)interception reliable, so exposingshellfor local zsh-fork execution would create a confusing API surface and undermine the composition.Remote environments are different: zsh-fork interception is local-only, so remote unified-exec calls must keep direct unified-exec behavior and still expose
shellwhen a remote environment can be selected.What Changed
exec_commandschema builder to omit theshellparameter when requested.shellfrom the unified-exec tool schema only when zsh-fork unified exec applies to all selectable environments.shellvisible when any remote environment can be targeted, because those calls run through direct unified exec.Verification
exec_commandcan hideshell.shellfor local-only execution while direct unified exec still exposes it.shellremains visible when a remote environment is available.codex-coreshell-parameter and zsh-fork tests.Stack created with Sapling. Best reviewed with ReviewStack.