Commit Graph

9 Commits

  • Add Windows sandbox unified exec runtime support (#15578)
    ## Summary
    
    This is the runtime/foundation half of the Windows sandbox unified-exec
    work.
    
    - add Windows sandbox `unified_exec` session support in
    `windows-sandbox-rs` for both:
      - the legacy restricted-token backend
      - the elevated runner backend
    - extend the PTY/process runtime so driver-backed sessions can support:
      - stdin streaming
      - stdout/stderr separation
      - exit propagation
      - PTY resize hooks
    - add Windows sandbox runtime coverage in `codex-windows-sandbox` /
    `codex-utils-pty`
    
    This PR does **not** enable Windows sandbox `UnifiedExec` for product
    callers yet because hooking this up to app-server comes in the next PR.
    
    Windows sandbox advertising is intentionally kept aligned with `main`,
    so sandboxed Windows callers still fall back to `ShellCommand`.
    
    This PR isolates the runtime/session layer so it can be reviewed
    independently from product-surface enablement.
    
    ---------
    
    Co-authored-by: jif-oai <jif@openai.com>
    Co-authored-by: Codex <noreply@openai.com>
  • refactor: narrow async lock guard lifetimes (#18211)
    Follow-up to https://github.com/openai/codex/pull/18178, where we called
    out enabling the await-holding lint as a follow-up.
    
    The long-term goal is to enable Clippy coverage for async guards held
    across awaits. This PR is intentionally only the first, low-risk cleanup
    pass: it narrows obvious lock guard lifetimes and leaves
    `codex-rs/Cargo.toml` unchanged so the lint is not enabled until the
    remaining cases are fixed or explicitly justified. It intentionally
    leaves the active-turn/turn-state locking pattern alone because those
    checks and mutations need to stay atomic.
    
    ## Common fixes used here
    
    These are the main patterns reviewers should expect in this PR, and they
    are also the patterns to reach for when fixing future `await_holding_*`
    findings:
    
    - **Scope the guard to the synchronous work.** If the code only needs
    data from a locked value, move the lock into a small block, clone or
    compute the needed values, and do the later `.await` after the block.
    - **Use direct one-line mutations when there is no later await.** Cases
    like `map.lock().await.remove(&id)` are acceptable when the guard is
    only needed for that single mutation and the statement ends before any
    async work.
    - **Drain or clone work out of the lock before notifying or awaiting.**
    For example, the JS REPL drains pending exec senders into a local vector
    and the websocket writer clones buffered envelopes before it serializes
    or sends them.
    - **Use a `Semaphore` only when serialization is intentional across
    async work.** The test serialization guards intentionally span awaited
    setup or execution, so using a semaphore communicates "one at a time"
    without holding a mutex guard.
    - **Remove the mutex when there is only one owner.** The PTY stdin
    writer task owns `stdin` directly; the old `Arc<Mutex<_>>` did not
    protect shared access because nothing else had access to the writer.
    - **Do not split locks that protect an atomic invariant.** This PR
    deliberately leaves active-turn/turn-state paths alone because those
    checks and mutations need to stay atomic. Those cases should be fixed
    separately with a design change or documented with `#[expect]`.
    
    ## What changed
    
    - Narrow scoped async mutex guards in app-server, JS REPL, network
    approval, remote-control websocket, and the RMCP test server.
    - Replace test-only async mutex serialization guards with semaphores
    where the guard intentionally lives across async work.
    - Let the PTY pipe writer task own stdin directly instead of wrapping it
    in an async mutex.
    
    ## Verification
    
    - `just fix -p codex-core -p codex-app-server -p codex-rmcp-client -p
    codex-shell-escalation -p codex-utils-pty -p codex-utils-readiness`
    - `just clippy -p codex-core`
    - `cargo test -p codex-core -p codex-app-server -p codex-rmcp-client -p
    codex-shell-escalation -p codex-utils-pty -p codex-utils-readiness` was
    run; the app-server suite passed, and `codex-core` failed in the local
    sandbox on six otel approval tests plus
    `suite::user_shell_cmd::user_shell_command_does_not_set_network_sandbox_env_var`,
    which appear to depend on local command approval/default rules and
    `CODEX_SANDBOX_NETWORK_DISABLED=1` in this environment.
  • chore: move pty and windows sandbox to Rust 2024 (#15954)
    ## Why
    
    `codex-utils-pty` and `codex-windows-sandbox` were the remaining crates
    in `codex-rs` that still overrode the workspace's Rust 2024 edition.
    Moving them forward in a separate PR keeps the baseline edition update
    isolated from the follow-on Bazel clippy workflow in #15955, while
    making linting and formatting behavior consistent with the rest of the
    workspace.
    
    This PR also needs Cargo and Bazel to agree on the edition for
    `codex-windows-sandbox`. Without the Bazel-side sync, the experimental
    Bazel app-server builds fail once they compile `windows-sandbox-rs`.
    
    ## What changed
    
    - switch `codex-rs/utils/pty` and `codex-rs/windows-sandbox-rs` to
    `edition = "2024"`
    - update `codex-utils-pty` callsites and tests to use the collapsed `if
    let` form that Clippy expects under the new edition
    - fix the Rust 2024 fallout in `windows-sandbox-rs`, including the
    reserved `gen` identifier, `unsafe extern` requirements, and new Clippy
    findings that surfaced under the edition bump
    - keep the edition bump separate from a larger unsafe cleanup by
    temporarily allowing `unsafe_op_in_unsafe_fn` in the Windows entrypoint
    modules that now report it under Rust 2024
    - update `codex-rs/windows-sandbox-rs/BUILD.bazel` to `crate_edition =
    "2024"` so Bazel compiles the crate with the same edition as Cargo
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/15954).
    * #15976
    * #15955
    * __->__ #15954
  • Apply argument comment lint across codex-rs (#14652)
    ## Why
    
    Once the repo-local lint exists, `codex-rs` needs to follow the
    checked-in convention and CI needs to keep it from drifting. This commit
    applies the fallback `/*param*/` style consistently across existing
    positional literal call sites without changing those APIs.
    
    The longer-term preference is still to avoid APIs that require comments
    by choosing clearer parameter types and call shapes. This PR is
    intentionally the mechanical follow-through for the places where the
    existing signatures stay in place.
    
    After rebasing onto newer `main`, the rollout also had to cover newly
    introduced `tui_app_server` call sites. That made it clear the first cut
    of the CI job was too expensive for the common path: it was spending
    almost as much time installing `cargo-dylint` and re-testing the lint
    crate as a representative test job spends running product tests. The CI
    update keeps the full workspace enforcement but trims that extra
    overhead from ordinary `codex-rs` PRs.
    
    ## What changed
    
    - keep a dedicated `argument_comment_lint` job in `rust-ci`
    - mechanically annotate remaining opaque positional literals across
    `codex-rs` with exact `/*param*/` comments, including the rebased
    `tui_app_server` call sites that now fall under the lint
    - keep the checked-in style aligned with the lint policy by using
    `/*param*/` and leaving string and char literals uncommented
    - cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
    registry/git metadata in the lint job
    - split changed-path detection so the lint crate's own `cargo test` step
    runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
    - continue to run the repo wrapper over the `codex-rs` workspace, so
    product-code enforcement is unchanged
    
    Most of the code changes in this commit are intentionally mechanical
    comment rewrites or insertions driven by the lint itself.
    
    ## Verification
    
    - `./tools/argument-comment-lint/run.sh --workspace`
    - `cargo test -p codex-tui-app-server -p codex-tui`
    - parsed `.github/workflows/rust-ci.yml` locally with PyYAML
    
    ---
    
    * -> #14652
    * #14651
  • fix: preserve zsh-fork escalation fds across unified-exec spawn paths (#13644)
    ## Why
    
    `zsh-fork` sessions launched through unified-exec need the escalation
    socket to survive the wrapper -> server -> child handoff so later
    intercepted `exec()` calls can still reach the escalation server.
    
    The inherited-fd spawn path also needs to avoid closing Rust's internal
    exec-error pipe, and the shell-escalation handoff needs to tolerate the
    receive-side case where a transferred fd is installed into the same
    stdio slot it will be mapped onto.
    
    ## What Changed
    
    - Added `SpawnLifecycle::inherited_fds()` in
    `codex-rs/core/src/unified_exec/process.rs` and threaded inherited fds
    through `codex-rs/core/src/unified_exec/process_manager.rs` so
    unified-exec can preserve required descriptors across both PTY and
    no-stdin pipe spawn paths.
    - Updated `codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs`
    to expose the escalation socket fd through the spawn lifecycle.
    - Added inherited-fd-aware spawn helpers in
    `codex-rs/utils/pty/src/pty.rs` and `codex-rs/utils/pty/src/pipe.rs`,
    including Unix pre-exec fd pruning that preserves requested inherited
    fds while leaving `FD_CLOEXEC` descriptors alone. The pruning helper is
    now named `close_inherited_fds_except()` to better describe that
    behavior.
    - Updated `codex-rs/shell-escalation/src/unix/escalate_client.rs` to
    duplicate local stdio before transfer and send destination stdio numbers
    in `SuperExecMessage`, so the wrapper keeps using its own
    `stdin`/`stdout`/`stderr` until the escalated child takes over.
    - Updated `codex-rs/shell-escalation/src/unix/escalate_server.rs` so the
    server accepts the overlap case where a received fd reuses the same
    stdio descriptor number that the child setup will target with `dup2`.
    - Added comments around the PTY stdio wiring and the overlap regression
    helper to make the fd handoff and controlling-terminal setup easier to
    follow.
    
    ## Verification
    
    - `cargo test -p codex-utils-pty`
    - covers preserved-fd PTY spawn behavior, PTY resize, Python REPL
    continuity, exec-failure reporting, and the no-stdin pipe path
    - `cargo test -p codex-shell-escalation`
    - covers duplicated-fd transfer on the client side and verifies the
    overlap case by passing a pipe-backed stdin payload through the
    server-side `dup2` path
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13644).
    * #14624
    * __->__ #13644
  • utils/pty: add streaming spawn and terminal sizing primitives (#13695)
    Enhance pty utils:
    * Support closing stdin
    * Separate stderr and stdout streams to allow consumers differentiate them
    * Provide compatibility helper to merge both streams back into combined one
    * Support specifying terminal size for pty, including on-demand resizes while process is already running
    * Support terminating the process while still consuming its outputs
  • chore: close pipe on non-pty processes (#9369)
    Closing the STDIN of piped process when starting them to avoid commands
    like `rg` to wait for content on STDIN and hangs for ever