[codex] exec-server honors remote environment cwd and shell#28122
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 846637c561
ℹ️ 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".
| shell_mode_for_environment(&turn.unified_exec_shell_mode, environment.as_ref()); | ||
| // Remote environments may use a different OS and must build commands with their native | ||
| // shell; fall back to the session shell when the environment did not report one. | ||
| let shell = turn_environment |
There was a problem hiding this comment.
Will we use shell snapshot for local because of this/
There was a problem hiding this comment.
That is (sadly) done in a different place.
There was a problem hiding this comment.
Looks like unified_exec only wraps local environment execs in snapshots. Seems like we might need to figure out whether we want shell snapshots for remote environments but that's a later problem I think
bf1245b to
4f6dd50
Compare
Why
Next slice needed to make progress on the
remote_env_windowstest is to support passing a Windows cwd for the remote environment and using that environment's native shell. This lets the test run a real Windows process instead of only recording an early path or shell mismatch.What
TurnEnvironmentSelection.cwdfromAbsolutePathBuftoPathUriC:\windowsand verify successful process completion