Commit Graph

10 Commits

  • [codex] Move config loading into codex-config (#19487)
    ## Why
    
    Config loading had become split across crates: `codex-config` owned the
    config types and merge logic, while `codex-core` still owned the loader
    that assembled the layer stack. This change consolidates that
    responsibility in `codex-config`, so the crate that defines config
    behavior also owns how configs are discovered and loaded.
    
    To make that move possible without reintroducing the old dependency
    cycle, the shell-environment policy types and helpers that
    `codex-exec-server` needs now live in `codex-protocol` instead of
    flowing through `codex-config`.
    
    This also makes the migrated loader tests more deterministic on machines
    that already have managed or system Codex config installed by letting
    tests override the system config and requirements paths instead of
    reading the host's `/etc/codex`.
    
    ## What Changed
    
    - moved the config loader implementation from `codex-core` into
    `codex-config::loader` and deleted the old `core::config_loader` module
    instead of leaving a compatibility shim
    - moved shell-environment policy types and helpers into
    `codex-protocol`, then updated `codex-exec-server` and other downstream
    crates to import them from their new home
    - updated downstream callers to use loader/config APIs from
    `codex-config`
    - added test-only loader overrides for system config and requirements
    paths so loader-focused tests do not depend on host-managed config state
    - cleaned up now-unused dependency entries and platform-specific cfgs
    that were surfaced by post-push CI
    
    ## Testing
    
    - `cargo test -p codex-config`
    - `cargo test -p codex-core config_loader_tests::`
    - `cargo test -p codex-protocol -p codex-exec-server -p
    codex-cloud-requirements -p codex-rmcp-client --lib`
    - `cargo test --lib -p codex-app-server-client -p codex-exec`
    - `cargo test --no-run --lib -p codex-app-server`
    - `cargo test -p codex-linux-sandbox --lib`
    - `cargo shear`
    - `just bazel-lock-check`
    
    ## Notes
    
    - I did not chase unrelated full-suite failures outside the migrated
    loader surface.
    - `cargo test -p codex-core --lib` still hits unrelated proxy-sensitive
    failures on this machine, and Windows CI still shows unrelated
    long-running/timeouting test noise outside the loader migration itself.
  • fix(exec-server): retain output until streams close (#18946)
    ## Why
    
    A Mac Bazel run hit a flake in
    `server::handler::tests::output_and_exit_are_retained_after_notification_receiver_closes`
    where the read path observed process exit but lost the expected buffered
    stdout (`first\nsecond\n`). See the [GitHub Actions
    job](https://github.com/openai/codex/actions/runs/24758468552/job/72436716505)
    and [BuildBuddy
    invocation](https://app.buildbuddy.io/invocation/37475a12-4ef2-45fb-ab8a-e49a2aba1d59).
    
    The underlying race is that process exit is not the same thing as
    stdout/stderr closure. If a child or grandchild inherits the pipe write
    end, or a process duplicates it with `dup2`, the watched process can
    exit while the stream is still open and more output can still arrive.
    The exec-server was starting exited-process retention cleanup from the
    exit event, so the process entry could be removed before the output
    streams had actually closed.
    
    While stress-testing the exec-server unit suite,
    `server::handler::tests::long_poll_read_fails_after_session_resume`
    exposed a separate test race: it started a short-lived command that
    could exit and wake the pending long-poll read before the session-resume
    assertion observed the resumed-session error. That test is intended to
    cover resume eviction, not process-exit delivery, so this change keeps
    the process alive and quiet while the second connection resumes the
    session.
    
    ## What changed
    
    - Keep exec-server process entries retained until stdout/stderr streams
    close, then start the post-exit retention timer from the closed event.
    - Wake long-poll readers when the closed event is emitted.
    - Add focused `local_process` unit coverage that proves late output is
    still retained after the short test retention interval has elapsed, and
    that closed process entries are eventually evicted.
    - Add a local and remote regression test where a parent exits while a
    child keeps inherited stdout open. The child waits on an explicit
    release file, so the test deterministically observes exit first,
    releases the child, then requires a nonzero-wait read from the exit
    sequence to receive the late output.
    - In `codex-rs/exec-server/src/server/handler/tests.rs`, make
    `long_poll_read_fails_after_session_resume` run a long-lived silent
    command instead of a short command that prints and exits. This isolates
    the test to session-resume behavior and prevents a normal process exit
    from satisfying the pending long-poll read first.
    
    ## Testing
    
    - `cargo test -p codex-exec-server
    exec_process_retains_output_after_exit_until_streams_close`
    - `cargo test -p codex-exec-server local_process::tests`
    - `cargo test -p codex-exec-server`
    - `just fix -p codex-exec-server`
    - `bazel test //codex-rs/exec-server:exec-server-unit-tests
    //codex-rs/exec-server:exec-server-exec_process-test
    //codex-rs/exec-server:exec-server-file_system-test
    //codex-rs/exec-server:exec-server-http_client-test
    //codex-rs/exec-server:exec-server-initialize-test
    //codex-rs/exec-server:exec-server-process-test
    //codex-rs/exec-server:exec-server-websocket-test`
    - `bazel test --runs_per_test=25
    //codex-rs/exec-server:exec-server-unit-tests`
    
    ## Documentation
    
    No docs update needed; this is an internal exec-server correctness fix.
  • exec-server: wait for close after observed exit (#19130)
    ## Why
    
    Windows CI can flake in
    `server::handler::tests::output_and_exit_are_retained_after_notification_receiver_closes`
    after a process has exited but before both output streams have closed.
    `exec/read` returned immediately whenever `exited` was true, so callers
    that had already observed the exit event could spin instead of
    long-polling for the later `closed` state.
    
    ## What Changed
    
    - Keep returning immediately when a terminal exit event is newly
    observable.
    - Allow later reads, after the caller has advanced past that event, to
    wait for `closed` or new output until `wait_ms` expires.
    
    ## Verification
    
    - CI pending.
  • [3/6] Add pushed exec process events (#18020)
    ## Summary
    - Add a pushed `ExecProcessEvent` stream alongside retained
    `process/read` output.
    - Publish local and remote output, exit, close, and failure events.
    - Cover the event stream with shared local/remote exec process tests.
    
    ## Testing
    - `cargo check -p codex-exec-server`
    - `cargo check -p codex-rmcp-client`
    - Not run: `cargo test` per repo instruction; CI will cover.
    
    ## Stack
    ```text
    o  #18027 [6/6] Fail exec client operations after disconnect
    │
    o  #18212 [5/6] Wire executor-backed MCP stdio
    │
    o  #18087 [4/6] Abstract MCP stdio server launching
    │
    @  #18020 [3/6] Add pushed exec process events
    │
    o  #18086 [2/6] Support piped stdin in exec process API
    │
    o  #18085 [1/6] Add MCP server environment config
    │
    o  main
    ```
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • [2/8] Support piped stdin in exec process API (#18086)
    ## Summary
    - Add an explicit stdin mode to process/start.
    - Keep normal non-interactive exec stdin closed while allowing
    pipe-backed processes.
    
    ## Stack
    ```text
    o  #18027 [8/8] Fail exec client operations after disconnect
    │
    o  #18025 [7/8] Cover MCP stdio tests with executor placement
    │
    o  #18089 [6/8] Wire remote MCP stdio through executor
    │
    o  #18088 [5/8] Add executor process transport for MCP stdio
    │
    o  #18087 [4/8] Abstract MCP stdio server launching
    │
    o  #18020 [3/8] Add pushed exec process events
    │
    @  #18086 [2/8] Support piped stdin in exec process API
    │
    o  #18085 [1/8] Add MCP server environment config
    │
    o  main
    ```
    
    Co-authored-by: Codex <noreply@openai.com>
  • Build remote exec env from exec-server policy (#17216)
    ## Summary
    - add an exec-server `envPolicy` field; when present, the server starts
    from its own process env and applies the shell environment policy there
    - keep `env` as the exact environment for local/embedded starts, but
    make it an overlay for remote unified-exec starts
    - move the shell-environment-policy builder into `codex-config` so Core
    and exec-server share the inherit/filter/set/include behavior
    - overlay only runtime/sandbox/network deltas from Core onto the
    exec-server-derived env
    
    ## Why
    Remote unified exec was materializing the shell env inside Core and
    forwarding the whole map to exec-server, so remote processes could
    inherit the orchestrator machine's `HOME`, `PATH`, etc. This keeps the
    base env on the executor while preserving Core-owned runtime additions
    like `CODEX_THREAD_ID`, unified-exec defaults, network proxy env, and
    sandbox marker env.
    
    ## Validation
    - `just fmt`
    - `git diff --check`
    - `cargo test -p codex-exec-server --lib`
    - `cargo test -p codex-core --lib unified_exec::process_manager::tests`
    - `cargo test -p codex-core --lib exec_env::tests`
    - `cargo test -p codex-core --lib exec_env_tests` (compile-only; filter
    matched 0 tests)
    - `cargo test -p codex-config --lib shell_environment` (compile-only;
    filter matched 0 tests)
    - `just bazel-lock-update`
    
    ## Known local validation issue
    - `just bazel-lock-check` is not runnable in this checkout: it invokes
    `./scripts/check-module-bazel-lock.sh`, which is missing.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
    Co-authored-by: pakrym-oai <pakrym@openai.com>
  • feat: move exec-server ownership (#16344)
    This introduces session-scoped ownership for exec-server so ws
    disconnects no longer immediately kill running remote exec processes,
    and it prepares the protocol for reconnect-based resume.
    - add session_id / resume_session_id to the exec-server initialize
    handshake
      - move process ownership under a shared session registry
    - detach sessions on websocket disconnect and expire them after a TTL
    instead of killing processes immediately (we will resume based on this)
    - allow a new connection to resume an existing session and take over
    notifications/ownership
    - I use UUID to make them not predictable as we don't have auth for now
    - make detached-session expiry authoritative at resume time so teardown
    wins at the TTL boundary
    - reject long-poll process/read calls that get resumed out from under an
    older attachment
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • feat: use ProcessId in exec-server (#15866)
    Use a full struct for the ProcessId to increase readability and make it
    easier in the future to make it evolve if needed
  • feat: exec-server prep for unified exec (#15691)
    This PR partially rebase `unified_exec` on the `exec-server` and adapt
    the `exec-server` accordingly.
    
    ## What changed in `exec-server`
    
    1. Replaced the old "broadcast-driven; process-global" event model with
    process-scoped session events. The goal is to be able to have dedicated
    handler for each process.
    2. Add to protocol contract to support explicit lifecycle status and
    stream ordering:
    - `WriteResponse` now returns `WriteStatus` (Accepted, UnknownProcess,
    StdinClosed, Starting) instead of a bool.
      - Added seq fields to output/exited notifications.
      - Added terminal process/closed notification.
    3. Demultiplexed remote notifications into per-process channels. Same as
    for the event sys
    4. Local and remote backends now both implement ExecBackend.
    5. Local backend wraps internal process ID/operations into per-process
    ExecProcess objects.
    6. Remote backend registers a session channel before launch and
    unregisters on failed launch.
    
    ## What changed in `unified_exec`
    
    1. Added unified process-state model and backend-neutral process
    wrapper. This will probably disappear in the future, but it makes it
    easier to keep the work flowing on both side.
    - `UnifiedExecProcess` now handles both local PTY sessions and remote
    exec-server processes through a shared `ProcessHandle`.
    - Added `ProcessState` to track has_exited, exit_code, and terminal
    failure message consistently across backends.
    2. Routed write and lifecycle handling through process-level methods.
    
    ## Some rationals
    
    1. The change centralizes execution transport in exec-server while
    preserving policy and orchestration ownership in core, avoiding
    duplicated launch approval logic. This comes from internal discussion.
    2. Session-scoped events remove coupling/cross-talk between processes
    and make stream ordering and terminal state explicit (seq, closed,
    failed).
    3. The failure-path surfacing (remote launch failures, write failures,
    transport disconnects) makes command tool output and cleanup behavior
    deterministic
    
    ## Follow-ups:
    * Unify the concept of thread ID behind an obfuscated struct
    * FD handling
    * Full zsh-fork compatibility
    * Full network sandboxing compatibility
    * Handle ws disconnection
  • Split exec process into local and remote implementations (#15233)
    ## Summary
    - match the exec-process structure to filesystem PR #15232
    - expose `ExecProcess` on `Environment`
    - make `LocalProcess` the real implementation and `RemoteProcess` a thin
    network proxy over `ExecServerClient`
    - make `ProcessHandler` a thin RPC adapter delegating to `LocalProcess`
    - add a shared local/remote process test
    
    ## Validation
    - `just fmt`
    - `CARGO_TARGET_DIR=~/.cache/cargo-target/codex cargo test -p
    codex-exec-server`
    - `just fix -p codex-exec-server`
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>