[do not merge] Run shell commands in remote environments#28722
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e0b7d2d33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
0e0b7d2 to
45bf315
Compare
45bf315 to
3c3dd87
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c3dd87856
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ctx.session | ||
| .services | ||
| .unified_exec_manager | ||
| .execute_remote_shell( | ||
| exec_env, | ||
| req.turn_environment.environment.as_ref(), | ||
| &context, | ||
| ) |
There was a problem hiding this comment.
For remote environments, this sends the already-transformed exec_env to the exec server, but attempt.env_for(...) above can wrap the argv with the client host's sandbox helper before this branch runs. In a cross-OS remote setup (for example a Linux client controlling a Windows/Wine exec environment) with the default/read-only sandbox selected, the remote side will try to launch a host-specific wrapper such as the Linux sandbox instead of a target-native command and the shell call fails before the user command runs; remote execution needs target-environment sandboxing or to avoid host wrappers.
AGENTS.md reference: AGENTS.md:L306-L308
Useful? React with 👍 / 👎.
jif-oai
changed the title
## Why Command approval cache keys included the command and working directory, but not the execution environment. An approval for `/workspace` locally could therefore be reused for the same command and path on an executor. ## What changed - Include the selected environment ID in shell and unified-exec approval cache keys. - Carry that ID through the normal command approval request so clients can show which environment is being approved. - Expose the environment through app-server as a required nullable `environmentId` and show it in the inline TUI approval prompt. - Keep older recorded approval events compatible when the environment is absent. For example, `echo ok` in local `/workspace` and `echo ok` in executor `/workspace` now produce different approval keys and separate prompts. ## Scope This PR does not change network approvals, Guardian review actions, MCP elicitation, full-screen TUI rendering, or environment-ID validation. Remote `shell_command` execution itself remains in #28722; this PR only makes its approval key environment-aware.
## Why Command approval cache keys included the command and working directory, but not the execution environment. An approval for `/workspace` locally could therefore be reused for the same command and path on an executor. ## What changed - Include the selected environment ID in shell and unified-exec approval cache keys. - Carry that ID through the normal command approval request so clients can show which environment is being approved. - Expose the environment through app-server as a required nullable `environmentId` and show it in the inline TUI approval prompt. - Keep older recorded approval events compatible when the environment is absent. For example, `echo ok` in local `/workspace` and `echo ok` in executor `/workspace` now produce different approval keys and separate prompts. ## Scope This PR does not change network approvals, Guardian review actions, MCP elicitation, full-screen TUI rendering, or environment-ID validation. Remote `shell_command` execution itself remains in openai#28722; this PR only makes its approval key environment-aware.
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
## Why Command approval cache keys included the command and working directory, but not the execution environment. An approval for `/workspace` locally could therefore be reused for the same command and path on an executor. ## What changed - Include the selected environment ID in shell and unified-exec approval cache keys. - Carry that ID through the normal command approval request so clients can show which environment is being approved. - Expose the environment through app-server as a required nullable `environmentId` and show it in the inline TUI approval prompt. - Keep older recorded approval events compatible when the environment is absent. For example, `echo ok` in local `/workspace` and `echo ok` in executor `/workspace` now produce different approval keys and separate prompts. ## Scope This PR does not change network approvals, Guardian review actions, MCP elicitation, full-screen TUI rendering, or environment-ID validation. Remote `shell_command` execution itself remains in openai#28722; this PR only makes its approval key environment-aware.
1 participant
Why
shell_commandstill assumes that the Codex orchestrator owns the selected working directory and can spawn the command locally. In a split environment, the primary working directory and shell belong to the remote executor, so the command can run against the wrong filesystem or fail before reaching the executor.Unified exec already routes commands through the selected environment. This change gives
shell_commandthe same split execution boundary without changing its local execution path.What this achieves
shell_commandapproval and sandbox transformation flow.shell_commandexecution unchanged.Known limitations
Remote
shell_commandintentionally supports no more than unified exec currently supports on the split: