Commit Graph

26 Commits

  • Use released DotSlash package for argument-comment lint (#15199)
    ## Why
    The argument-comment lint now has a packaged DotSlash artifact from
    [#15198](https://github.com/openai/codex/pull/15198), so the normal repo
    lint path should use that released payload instead of rebuilding the
    lint from source every time.
    
    That keeps `just clippy` and CI aligned with the shipped artifact while
    preserving a separate source-build path for people actively hacking on
    the lint crate.
    
    The current alpha package also exposed two integration wrinkles that the
    repo-side prebuilt wrapper needs to smooth over:
    - the bundled Dylint library filename includes the host triple, for
    example `@nightly-2025-09-18-aarch64-apple-darwin`, and Dylint derives
    `RUSTUP_TOOLCHAIN` from that filename
    - on Windows, Dylint's driver path also expects `RUSTUP_HOME` to be
    present in the environment
    
    Without those adjustments, the prebuilt CI jobs fail during `cargo
    metadata` or driver setup. This change makes the checked-in prebuilt
    wrapper normalize the packaged library name to the plain
    `nightly-2025-09-18` channel before invoking `cargo-dylint`, and it
    teaches both the wrapper and the packaged runner source to infer
    `RUSTUP_HOME` from `rustup show home` when the environment does not
    already provide it.
    
    After the prebuilt Windows lint job started running successfully, it
    also surfaced a handful of existing anonymous literal callsites in
    `windows-sandbox-rs`. This PR now annotates those callsites so the new
    cross-platform lint job is green on the current tree.
    
    ## What Changed
    - checked in the current
    `tools/argument-comment-lint/argument-comment-lint` DotSlash manifest
    - kept `tools/argument-comment-lint/run.sh` as the source-build wrapper
    for lint development
    - added `tools/argument-comment-lint/run-prebuilt-linter.sh` as the
    normal enforcement path, using the checked-in DotSlash package and
    bundled `cargo-dylint`
    - updated `just clippy` and `just argument-comment-lint` to use the
    prebuilt wrapper
    - split `.github/workflows/rust-ci.yml` so source-package checks live in
    a dedicated `argument_comment_lint_package` job, while the released lint
    runs in an `argument_comment_lint_prebuilt` matrix on Linux, macOS, and
    Windows
    - kept the pinned `nightly-2025-09-18` toolchain install in the prebuilt
    CI matrix, since the prebuilt package still relies on rustup-provided
    toolchain components
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` to
    normalize host-qualified nightly library filenames, keep the `rustup`
    shim directory ahead of direct toolchain `cargo` binaries, and export
    `RUSTUP_HOME` when needed for Windows Dylint driver setup
    - updated `tools/argument-comment-lint/src/bin/argument-comment-lint.rs`
    so future published DotSlash artifacts apply the same nightly-filename
    normalization and `RUSTUP_HOME` inference internally
    - fixed the remaining Windows lint violations in
    `codex-rs/windows-sandbox-rs` by adding the required `/*param*/`
    comments at the reported callsites
    - documented the checked-in DotSlash file, wrapper split, archive
    layout, nightly prerequisite, and Windows `RUSTUP_HOME` requirement in
    `tools/argument-comment-lint/README.md`
  • 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
  • windows-sandbox: add runner IPC foundation for future unified_exec (#14139)
    # Summary
    
    This PR introduces the Windows sandbox runner IPC foundation that later
    unified_exec work will build on.
    
    The key point is that this is intentionally infrastructure-only. The new
    IPC transport, runner plumbing, and ConPTY helpers are added here, but
    the active elevated Windows sandbox path still uses the existing
    request-file bootstrap. In other words, this change prepares the
    transport and module layout we need for unified_exec without switching
    production behavior over yet.
    
    Part of this PR is also a source-layout cleanup: some Windows sandbox
    files are moved into more explicit `elevated/`, `conpty/`, and shared
    locations so it is clearer which code is for the elevated sandbox flow,
    which code is legacy/direct-spawn behavior, and which helpers are shared
    between them. That reorganization is intentional in this first PR so
    later behavioral changes do not also have to carry a large amount of
    file-move churn.
    
    # Why This Is Needed For unified_exec
    
    Windows elevated sandboxed unified_exec needs a long-lived,
    bidirectional control channel between the CLI and a helper process
    running under the sandbox user. That channel has to support:
    
    - starting a process and reporting structured spawn success/failure
    - streaming stdout/stderr back incrementally
    - forwarding stdin over time
    - terminating or polling a long-lived process
    - supporting both pipe-backed and PTY-backed sessions
    
    The existing elevated one-shot path is built around a request-file
    bootstrap and does not provide those primitives cleanly. Before we can
    turn on Windows sandbox unified_exec, we need the underlying runner
    protocol and transport layer that can carry those lifecycle events and
    streams.
    
    # Why Windows Needs More Machinery Than Linux Or macOS
    
    Linux and macOS can generally build unified_exec on top of the existing
    sandbox/process model: the parent can spawn the child directly, retain
    normal ownership of stdio or PTY handles, and manage the lifetime of the
    sandboxed process without introducing a second control process.
    
    Windows elevated sandboxing is different. To run inside the sandbox
    boundary, we cross into a different user/security context and then need
    to manage a long-lived process from outside that boundary. That means we
    need an explicit helper process plus an IPC transport to carry spawn,
    stdin, output, and exit events back and forth. The extra code here is
    mostly that missing Windows sandbox infrastructure, not a conceptual
    difference in unified_exec itself.
    
    # What This PR Adds
    
    - the framed IPC message types and transport helpers for parent <->
    runner communication
    - the renamed Windows command runner with both the existing request-file
    bootstrap and the dormant IPC bootstrap
    - named-pipe helpers for the elevated runner path
    - ConPTY helpers and process-thread attribute plumbing needed for
    PTY-backed sessions
    - shared sandbox/process helpers that later PRs will reuse when
    switching live execution paths over
    - early file/module moves so later PRs can focus on behavior rather than
    layout churn
    
    # What This PR Does Not Yet Do
    
    - it does not switch the active elevated one-shot path over to IPC yet
    - it does not enable Windows sandbox unified_exec yet
    - it does not remove the existing request-file bootstrap yet
    
    So while this code compiles and the new path has basic validation, it is
    not yet the exercised production path. That is intentional for this
    first PR: the goal here is to land the transport and runner foundation
    cleanly before later PRs start routing real command execution through
    it.
    
    # Follow-Ups
    
    Planned follow-up PRs will:
    
    1. switch elevated one-shot Windows sandbox execution to the new runner
    IPC path
    2. layer Windows sandbox unified_exec sessions on top of the same
    transport
    3. remove the legacy request-file path once the IPC-based path is live
    
    # Validation
    
    - `cargo build -p codex-windows-sandbox`
  • 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
  • Stabilize pipe process stdin round-trip test (#14013)
    ## What changed
    - keep the explicit stdin-close behavior after writing so the child
    still receives EOF deterministically
    - on Windows, stop using `python -c` for the round-trip assertion and
    instead run a native `cmd.exe` pipeline that reads one line from stdin
    with `set /p` and echoes it back
    - send `
    ` on Windows so the stdin payload matches the platform-native line
    ending the shell reader expects
    
    ## Why this fixes flakiness
    The failing branch-local flake was not in `spawn_pipe_process` itself.
    The child exited cleanly, but the Windows ARM runner sometimes produced
    an empty stdout string when the test used Python as the stdin consumer.
    That makes the test sensitive to Python startup and stdin-close timing
    rather than the pipe primitive we actually want to validate. Switching
    the Windows path to a native `cmd.exe` reader keeps the assertion
    focused on our pipe behavior: bytes written to stdin should come back on
    stdout before EOF closes the process. The explicit `
    ` write removes line-ending ambiguity on Windows.
    
    ## Scope
    - test-only
    - no production logic change
  • Stabilize split PTY output on Windows (#14003)
    ## Summary
    - run the split stdout/stderr PTY test through the normal shell helper
    on every platform
    - use a Windows-native command string instead of depending on Python to
    emit split streams
    - assert CRLF line endings on Windows explicitly
    
    ## Why this fixes the flake
    The earlier PTY split-output test used a Python one-liner on Windows
    while the rest of the file exercised shell-command behavior. That made
    the test depend on runner-local Python availability and masked the real
    Windows shell output shape. Using a native cmd-compatible command and
    asserting the actual CRLF output makes the split stdout/stderr coverage
    deterministic on Windows runners.
  • start of hooks engine (#13276)
    (Experimental)
    
    This PR adds a first MVP for hooks, with SessionStart and Stop
    
    The core design is:
    
    - hooks live in a dedicated engine under codex-rs/hooks
    - each hook type has its own event-specific file
    - hook execution is synchronous and blocks normal turn progression while
    running
    - matching hooks run in parallel, then their results are aggregated into
    a normalized HookRunSummary
    
    On the AppServer side, hooks are exposed as operational metadata rather
    than transcript-native items:
    
    - new live notifications: hook/started, hook/completed
    - persisted/replayed hook results live on Turn.hookRuns
    - we intentionally did not add hook-specific ThreadItem variants
    
    Hooks messages are not persisted, they remain ephemeral. The context
    changes they add are (they get appended to the user's prompt)
  • Stabilize PTY Python REPL test (#13883)
    ## What changed
    - The PTY Python REPL test now starts Python with a startup marker
    already embedded in argv.
    - The test waits for that marker in PTY output before making assertions.
    
    ## Why this fixes the flake
    - The old version tried to probe the live REPL almost immediately after
    spawn.
    - That races PTY initialization, Python startup, and prompt buffering,
    all of which vary across platforms and CI load.
    - By having the child process emit a known marker as part of its own
    startup path, the test gets a deterministic synchronization point that
    comes from the process under test rather than from guessed timing.
    
    ## Scope
    - Test-only change.
  • Fix inverted Windows PTY TerminateProcess handling (#13989)
    Addresses #13945
    
    The vendored WezTerm ConPTY backend in
    `codex-rs/utils/pty/src/win/mod.rs` treated `TerminateProcess` return
    values backwards: nonzero success was handled as failure, and `0`
    failure was handled as success.
    
    This is likely causing a number of bugs reported against Codex running
    on Windows native where processes are not cleaned up.
  • 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
  • feat: use process group to kill the PTY (#12688)
    Use the process group kill logic to kill the PTY
  • Fix flaky windows CI test (#10993)
    Hardens PTY Python REPL test and make MCP test startup deterministic
    
    **Summary**
    - `utils/pty/src/tests.rs`
    - Added a REPL readiness handshake (`wait_for_python_repl_ready`) that
    repeatedly sends a marker and waits for it in PTY output before sending
    test commands.
      - Updated `pty_python_repl_emits_output_and_exits` to:
        - wait for readiness first,
        - preserve startup output,
        - append output collected through process exit.
    - Reduces Windows/ConPTY flakiness from early stdin writes racing REPL
    startup.
    
    - `mcp-server/tests/suite/codex_tool.rs`
    - Avoid remote model refresh during MCP test startup, reducing
    timeout-prone nondeterminism.
  • Process-group cleanup for stdio MCP servers to prevent orphan process storms (#10710)
    This PR changes stdio MCP child processes to run in their own process
    group
    * Add guarded teardown in codex-rmcp-client: send SIGTERM to the group
    first, then SIGKILL after a short grace period.
    * Add terminate_process_group helper in process_group.rs.
    * Add Unix regression test in process_group_cleanup.rs to verify wrapper
    + grandchild are reaped on client drop.
    
    Addresses reported MCP process/thread storm: #10581
  • 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
  • feat: add support for building with Bazel (#8875)
    This PR configures Codex CLI so it can be built with
    [Bazel](https://bazel.build) in addition to Cargo. The `.bazelrc`
    includes configuration so that remote builds can be done using
    [BuildBuddy](https://www.buildbuddy.io).
    
    If you are familiar with Bazel, things should work as you expect, e.g.,
    run `bazel test //... --keep-going` to run all the tests in the repo,
    but we have also added some new aliases in the `justfile` for
    convenience:
    
    - `just bazel-test` to run tests locally
    - `just bazel-remote-test` to run tests remotely (currently, the remote
    build is for x86_64 Linux regardless of your host platform). Note we are
    currently seeing the following test failures in the remote build, so we
    still need to figure out what is happening here:
    
    ```
    failures:
        suite::compact::manual_compact_twice_preserves_latest_user_messages
        suite::compact_resume_fork::compact_resume_after_second_compaction_preserves_history
        suite::compact_resume_fork::compact_resume_and_fork_preserve_model_history_view
    ```
    
    - `just build-for-release` to build release binaries for all
    platforms/architectures remotely
    
    To setup remote execution:
    - [Create a buildbuddy account](https://app.buildbuddy.io/) (OpenAI
    employees should also request org access at
    https://openai.buildbuddy.io/join/ with their `@openai.com` email
    address.)
    - [Copy your API key](https://app.buildbuddy.io/docs/setup/) to
    `~/.bazelrc` (add the line `build
    --remote_header=x-buildbuddy-api-key=YOUR_KEY`)
    - Use `--config=remote` in your `bazel` invocations (or add `common
    --config=remote` to your `~/.bazelrc`, or use the `just` commands)
    
    ## CI
    
    In terms of CI, this PR introduces `.github/workflows/bazel.yml`, which
    uses Bazel to run the tests _locally_ on Mac and Linux GitHub runners
    (we are working on supporting Windows, but that is not ready yet). Note
    that the failures we are seeing in `just bazel-remote-test` do not occur
    on these GitHub CI jobs, so everything in `.github/workflows/bazel.yml`
    is green right now.
    
    The `bazel.yml` uses extra config in `.github/workflows/ci.bazelrc` so
    that macOS CI jobs build _remotely_ on Linux hosts (using the
    `docker://docker.io/mbolin491/codex-bazel` Docker image declared in the
    root `BUILD.bazel`) using cross-compilation to build the macOS
    artifacts. Then these artifacts are downloaded locally to GitHub's macOS
    runner so the tests can be executed natively. This is the relevant
    config that enables this:
    
    ```
    common:macos --config=remote
    common:macos --strategy=remote
    common:macos --strategy=TestRunner=darwin-sandbox,local
    ```
    
    Because of the remote caching benefits we get from BuildBuddy, these new
    CI jobs can be extremely fast! For example, consider these two jobs that
    ran all the tests on Linux x86_64:
    
    - Bazel 1m37s
    https://github.com/openai/codex/actions/runs/20861063212/job/59940545209?pr=8875
    - Cargo 9m20s
    https://github.com/openai/codex/actions/runs/20861063192/job/59940559592?pr=8875
    
    For now, we will continue to run both the Bazel and Cargo jobs for PRs,
    but once we add support for Windows and running Clippy, we should be
    able to cutover to using Bazel exclusively for PRs, which should still
    speed things up considerably. We will probably continue to run the Cargo
    jobs post-merge for commits that land on `main` as a sanity check.
    
    Release builds will also continue to be done by Cargo for now.
    
    Earlier attempt at this PR: https://github.com/openai/codex/pull/8832
    Earlier attempt to add support for Buck2, now abandoned:
    https://github.com/openai/codex/pull/8504
    
    ---------
    
    Co-authored-by: David Zbarsky <dzbarsky@gmail.com>
    Co-authored-by: Michael Bolin <mbolin@openai.com>
  • fix: race on rx subscription (#7921)
    Fix race where the PTY was sending first chunk before the subscription
    to the broadcast
  • Vendor ConPtySystem (#7656)
    The repo we were depending on is very large and we need very small part
    of it.
    
    ---------
    
    Co-authored-by: Pavel <pavel@krymets.com>
  • Fix unified_exec on windows (#7620)
    Fix unified_exec on windows
    
    Requires removal of PSUEDOCONSOLE_INHERIT_CURSOR flag so child processed
    don't attempt to wait for cursor position response (and timeout).
    
    
    https://github.com/wezterm/wezterm/compare/main...pakrym:wezterm:PSUEDOCONSOLE_INHERIT_CURSOR?expand=1
    
    ---------
    
    Co-authored-by: pakrym-oai <pakrym@openai.com>
  • chore: add cargo-deny configuration (#7119)
    - add GitHub workflow running cargo-deny on push/PR
    - document cargo-deny allowlist with workspace-dep notes and advisory
    ignores
    - align workspace crates to inherit version/edition/license for
    consistent checks
  • Use codex-linux-sandbox in unified exec (#6480)
    Unified exec isn't working on Linux because we don't provide the correct
    arg0.
    
    The library we use for pty management doesn't allow setting arg0
    separately from executable. Use the same aliasing strategy we use for
    `apply_patch` for `codex-linux-sandbox`.
    
    Use `#[ctor]` hack to dispatch codex-linux-sandbox calls.
    
    
    Addresses https://github.com/openai/codex/issues/6450
  • chore: rework tools execution workflow (#5278)
    Re-work the tool execution flow. Read `orchestrator.rs` to understand
    the structure