Skip to content

fix: preserve zsh-fork escalation fds in unified-exec PTYs#13644

Open
bolinfest wants to merge 1 commit intomainfrom
pr13644
Open

fix: preserve zsh-fork escalation fds in unified-exec PTYs#13644
bolinfest wants to merge 1 commit intomainfrom
pr13644

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Mar 6, 2026

Why

zsh-fork sessions routed through unified-exec need fd-safe process launch semantics so escalation works reliably for long-lived PTY sessions.

This commit fixes fd handling at each handoff boundary, including a subtle spawn-error path where aggressive fd pruning could hide exec failures.

What Changed

  • Fixed wrapper fd transfer in shell-escalation (shell-escalation/src/unix/escalate_client.rs):
    • duplicate stdin/stdout/stderr before sending fds to the server
    • send destination stdio numbers (0/1/2) in SuperExecMessage while transferring the duplicated fds
  • Added inherited-fd plumbing for unified-exec process startup:
    • SpawnLifecycle::inherited_fds() in core/src/unified_exec/process.rs
    • process manager passes inherited fds into PTY spawn (core/src/unified_exec/process_manager.rs)
    • zsh-fork backend exposes the escalation socket fd via the lifecycle (core/src/tools/runtimes/shell/zsh_fork_backend.rs)
  • Added PTY spawn support for explicitly preserved inherited fds in utils/pty/src/pty.rs via spawn_process_with_inherited_fds(...).
  • Fixed Unix pre-exec fd pruning in utils/pty/src/pty.rs to not close FD_CLOEXEC descriptors, preserving Rust's internal exec-error pipe so missing executables still fail at spawn().
  • Added PTY regressions in utils/pty/src/tests.rs for:
    • preserved-fd spawn behavior
    • preserved-fd Python REPL continuity
    • missing executable propagation when inherited-fd spawn path is used

Verification

  • cargo clippy -p codex-utils-pty --tests -- -D warnings
  • cargo test -p codex-utils-pty
  • cargo test -p codex-shell-escalation

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b012624ce

ℹ️ 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".

Copy link
Collaborator

@jif-oai jif-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will require some heavy testing in the end but I guess it's in the follow up

Approving to unblock but please make sure the point raised (mainly for non-pty) are not real issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants