Commit Graph

39 Commits

  • Wire the PatchUpdated events through app_server (#18289)
    Wires patch_updated events through app_server. These events are parsed
    and streamed while apply_patch is being written by the model. Also adds 500ms of buffering to the patch_updated events in the diff_consumer.
    
    The eventual goal is to use this to display better progress indicators in
    the codex app.
  • Move codex module under session (#18249)
    ## Summary
    - rename the core codex module root to session/mod.rs without using
    #[path]
    - move the codex module directory and tests under core/src/session
    - remove session/mod.rs reexports so call sites use explicit child
    module paths
    
    ## Testing
    - cargo test -p codex-core --lib
    - cargo check -p codex-core --tests
    - just fmt
    - just fix -p codex-core
    - git diff --check
  • Stream apply_patch changes (#17862)
    Adds new events for streaming apply_patch changes from responses api.
    This is to enable clients to show progress during file writes.
    
    Caveat: This does not work with apply_patch in function call mode, since
    that required adding streaming json parsing.
  • Spread AbsolutePathBuf (#17792)
    Mechanical change to promote absolute paths through code.
  • chore: refactor name and namespace to single type (#17402)
    avoid passing them both around, unify on a type. this now also keys
    `ToolRegistry`.
    
    tests pass
  • core: cut codex-core compile time 63% with native async ToolHandler (#16630)
    ## Why
    
    `ToolHandler` was still paying a large compile-time tax from
    `#[async_trait]` on every concrete handler impl, even though the only
    object-safe boundary the registry actually stores is the internal
    `AnyToolHandler` adapter.
    
    This PR removes that macro-generated async wrapper layer from concrete
    `ToolHandler` impls while keeping the existing object-safe shim in
    `AnyToolHandler`. In practice, that gets essentially the same
    compile-time win as the larger type-erasure refactor in #16627, but with
    a much smaller diff and without changing the public shape of
    `ToolHandler<Output = T>`.
    
    That tradeoff matters here because this is a broad `codex-core` hotspot
    and reviewers should be able to judge the compile-time impact from hard
    numbers, not vibes.
    
    ## Headline result
    
    On a clean `codex-core` package rebuild (`cargo clean -p codex-core`
    before each command), rustc `total` dropped from **187.15s to 68.98s**
    versus the shared `0bd31dc382bd` baseline: **-63.1%**.
    
    The biggest hot passes dropped by roughly **71-72%**:
    
    | Metric | Baseline `0bd31dc382bd` | This PR `41f7ac0adeac` | Delta |
    |---|---:|---:|---:|
    | `total` | 187.15s | 68.98s | **-63.1%** |
    | `generate_crate_metadata` | 84.53s | 24.49s | **-71.0%** |
    | `MIR_borrow_checking` | 84.13s | 24.58s | **-70.8%** |
    | `monomorphization_collector_graph_walk` | 79.74s | 22.19s | **-72.2%**
    |
    | `evaluate_obligation` self-time | 180.62s | 46.91s | **-74.0%** |
    
    Important caveat: `-Z time-passes` timings are nested, so
    `generate_crate_metadata` and `monomorphization_collector_graph_walk`
    are mostly overlapping, not additive.
    
    ## Why this PR over #16627
    
    #16627 already proved that the `ToolHandler` stack was the right
    hotspot, but it got there by making `ToolHandler` object-safe and
    changing every handler to return `BoxFuture<Result<AnyToolResult, _>>`
    directly.
    
    This PR keeps the lower-churn shape:
    
    - `ToolHandler` remains generic over `type Output`.
    - Concrete handlers use native RPITIT futures with explicit `Send`
    bounds.
    - `AnyToolHandler` remains the only object-safe adapter and still does
    the boxing at the registry boundary, as before.
    - The implementation diff is only **33 files, +28/-77**.
    
    The measurements are at least comparable, and in this run this PR is
    slightly faster than #16627 on the pass-level total:
    
    | Metric | #16627 | This PR | Delta |
    |---|---:|---:|---:|
    | `total` | 79.90s | 68.98s | **-13.7%** |
    | `generate_crate_metadata` | 25.88s | 24.49s | **-5.4%** |
    | `monomorphization_collector_graph_walk` | 23.54s | 22.19s | **-5.7%**
    |
    | `evaluate_obligation` self-time | 43.29s | 46.91s | +8.4% |
    
    ## Profile data
    
    ### Crate-level timings
    
    `cargo +nightly build -p codex-core --lib -Z unstable-options
    --timings=json` after `cargo clean -p codex-core`.
    
    Baseline data below is reused from the shared parent `0bd31dc382bd`
    profile because this PR and #16627 are both one commit on top of that
    same parent.
    
    | Crate | Baseline `duration` | This PR `duration` | Delta | Baseline
    `rmeta_time` | This PR `rmeta_time` | Delta |
    |---|---:|---:|---:|---:|---:|---:|
    | `codex_core` | 187.380776583s | 69.171113833s | **-63.1%** |
    174.474507208s | 55.873015583s | **-68.0%** |
    | `starlark` | 17.90s | 16.773824125s | -6.3% | n/a | 8.8999965s | n/a |
    
    ### Pass-level timings
    
    `cargo +nightly rustc -p codex-core --lib -- -Z time-passes -Z
    time-passes-format=json` after `cargo clean -p codex-core`.
    
    | Pass | Baseline | This PR | Delta |
    |---|---:|---:|---:|
    | `total` | 187.150662083s | 68.978770375s | **-63.1%** |
    | `generate_crate_metadata` | 84.531864625s | 24.487462958s | **-71.0%**
    |
    | `MIR_borrow_checking` | 84.131389375s | 24.575553875s | **-70.8%** |
    | `monomorphization_collector_graph_walk` | 79.737515042s |
    22.190207417s | **-72.2%** |
    | `codegen_crate` | 12.362532292s | 12.695237625s | +2.7% |
    | `type_check_crate` | 4.4765405s | 5.442019542s | +21.6% |
    | `coherence_checking` | 3.311121208s | 4.239935292s | +28.0% |
    | process `real` / `user` / `sys` | 187.70s / 201.87s / 4.99s | 69.52s /
    85.90s / 2.92s | n/a |
    
    ### Self-profile query summary
    
    `cargo +nightly rustc -p codex-core --lib -- -Z self-profile=... -Z
    self-profile-events=default,query-keys,args,llvm,artifact-sizes` after
    `cargo clean -p codex-core`, summarized with `measureme summarize -p
    0.5`.
    
    | Query / phase | Baseline self time | This PR self time | Delta |
    Baseline total time | This PR total time | Baseline item count | This PR
    item count | Baseline cache hits | This PR cache hits |
    |---|---:|---:|---:|---:|---:|---:|---:|---:|---:|
    | `evaluate_obligation` | 180.62s | 46.91s | **-74.0%** | 182.08s |
    48.37s | 572,234 | 388,659 | 1,130,998 | 1,058,553 |
    | `mir_borrowck` | 1.42s | 1.49s | +4.9% | 93.77s | 29.59s | n/a | 6,184
    | n/a | 15,298 |
    | `typeck` | 1.84s | 1.87s | +1.6% | 2.38s | 2.44s | n/a | 9,367 | n/a |
    79,247 |
    | `LLVM_module_codegen_emit_obj` | n/a | 17.12s | n/a | 17.01s | 17.12s
    | n/a | 256 | n/a | 0 |
    | `LLVM_passes` | n/a | 13.07s | n/a | 12.95s | 13.07s | n/a | 1 | n/a |
    0 |
    | `codegen_module` | n/a | 12.33s | n/a | 12.22s | 13.64s | n/a | 256 |
    n/a | 0 |
    | `items_of_instance` | n/a | 676.00ms | n/a | n/a | 24.96s | n/a |
    99,990 | n/a | 0 |
    | `type_op_prove_predicate` | n/a | 660.79ms | n/a | n/a | 24.78s | n/a
    | 78,762 | n/a | 235,877 |
    
    | Summary | Baseline | This PR |
    |---|---:|---:|
    | `evaluate_obligation` % of total CPU | 70.821% | 38.880% |
    | self-profile total CPU time | 255.042999997s | 120.661175956s |
    | process `real` / `user` / `sys` | 220.96s / 235.02s / 7.09s | 86.35s /
    103.66s / 3.54s |
    
    ### Artifact sizes
    
    From the same `measureme summarize` output:
    
    | Artifact | Baseline | This PR | Delta |
    |---|---:|---:|---:|
    | `crate_metadata` | 26,534,471 bytes | 26,545,248 bytes | +10,777 |
    | `dep_graph` | 253,181,425 bytes | 239,240,806 bytes | -13,940,619 |
    | `linked_artifact` | 565,366,624 bytes | 562,673,176 bytes | -2,693,448
    |
    | `object_file` | 513,127,264 bytes | 510,464,096 bytes | -2,663,168 |
    | `query_cache` | 137,440,945 bytes | 136,982,566 bytes | -458,379 |
    | `cgu_instructions` | 3,586,307 bytes | 3,575,121 bytes | -11,186 |
    | `codegen_unit_size_estimate` | 2,084,846 bytes | 2,078,773 bytes |
    -6,073 |
    | `work_product_index` | 19,565 bytes | 19,565 bytes | 0 |
    
    ### Baseline hotspots before this change
    
    These are the top normalized obligation buckets from the shared baseline
    profile:
    
    | Obligation bucket | Samples | Duration |
    |---|---:|---:|
    | `outlives:tasks::review::ReviewTask` | 1,067 | 6.33s |
    | `outlives:tools::handlers::unified_exec::UnifiedExecHandler` | 896 |
    5.63s |
    | `trait:T as tools::registry::ToolHandler` | 876 | 5.45s |
    | `outlives:tools::handlers::shell::ShellHandler` | 888 | 5.37s |
    | `outlives:tools::handlers::shell::ShellCommandHandler` | 870 | 5.29s |
    |
    `outlives:tools::runtimes::shell::unix_escalation::CoreShellActionProvider`
    | 637 | 3.73s |
    | `outlives:tools::handlers::mcp::McpHandler` | 695 | 3.61s |
    | `outlives:tasks::regular::RegularTask` | 726 | 3.57s |
    
    Top `items_of_instance` entries before this change were mostly concrete
    async handler/task impls:
    
    | Instance | Duration |
    |---|---:|
    | `tasks::regular::{impl#2}::run` | 3.79s |
    | `tools::handlers::mcp::{impl#0}::handle` | 3.27s |
    | `tools::runtimes::shell::unix_escalation::{impl#2}::determine_action`
    | 3.09s |
    | `tools::handlers::agent_jobs::{impl#11}::handle` | 3.07s |
    | `tools::handlers::multi_agents::spawn::{impl#1}::handle` | 2.84s |
    | `tasks::review::{impl#4}::run` | 2.82s |
    | `tools::handlers::multi_agents_v2::spawn::{impl#2}::handle` | 2.80s |
    | `tools::handlers::multi_agents::resume_agent::{impl#1}::handle` |
    2.73s |
    | `tools::handlers::unified_exec::{impl#2}::handle` | 2.54s |
    | `tasks::compact::{impl#4}::run` | 2.45s |
    
    ## What changed
    
    Relevant pre-change registry shape:
    [`codex-rs/core/src/tools/registry.rs`](https://github.com/openai/codex/blob/0bd31dc382bd1c33dc2bb6b97069c76aa10ba14b/codex-rs/core/src/tools/registry.rs#L38-L219)
    
    Current registry shape in this PR:
    [`codex-rs/core/src/tools/registry.rs`](https://github.com/openai/codex/blob/41f7ac0adeac81d667541853d6546267d6083613/codex-rs/core/src/tools/registry.rs#L38-L203)
    
    - `ToolHandler::{is_mutating, handle}` now return native `impl Future +
    Send` futures instead of using `#[async_trait]`.
    - `AnyToolHandler` remains the object-safe adapter and boxes those
    futures at the registry boundary with explicit lifetimes.
    - Concrete handlers and the registry test handler drop `#[async_trait]`
    but otherwise keep their async method bodies intact.
    - Representative examples:
    [`codex-rs/core/src/tools/handlers/shell.rs`](https://github.com/openai/codex/blob/41f7ac0adeac81d667541853d6546267d6083613/codex-rs/core/src/tools/handlers/shell.rs#L223-L379),
    [`codex-rs/core/src/tools/handlers/unified_exec.rs`](https://github.com/openai/codex/blob/41f7ac0adeac81d667541853d6546267d6083613/codex-rs/core/src/tools/handlers/unified_exec.rs),
    [`codex-rs/core/src/tools/registry_tests.rs`](https://github.com/openai/codex/blob/41f7ac0adeac81d667541853d6546267d6083613/codex-rs/core/src/tools/registry_tests.rs)
    
    ## Tradeoff
    
    This is intentionally less invasive than #16627: it does **not** move
    result boxing into every concrete handler and does **not** change
    `ToolHandler` into an object-safe trait.
    
    Instead, it keeps the existing registry-level type-erasure boundary and
    only removes the macro-generated async wrapper layer from concrete
    impls. So the runtime boxing story stays basically the same as before,
    while the compile-time savings are still large.
    
    ## Verification
    
    Existing verification for this branch still applies:
    
    - Ran `cargo test -p codex-core`; this change compiled and the suite
    reached the known unrelated `config::tests::*guardian*` failures, with
    no local diff under `codex-rs/core/src/config/`.
    
    Profiling commands used for the tables above:
    
    - `cargo clean -p codex-core`
    - `cargo +nightly build -p codex-core --lib -Z unstable-options
    --timings=json`
    - `cargo +nightly rustc -p codex-core --lib -- -Z time-passes -Z
    time-passes-format=json`
    - `cargo +nightly rustc -p codex-core --lib -- -Z self-profile=... -Z
    self-profile-events=default,query-keys,args,llvm,artifact-sizes`
    - `measureme summarize -p 0.5`
  • core: remove cross-crate re-exports from lib.rs (#16512)
    ## Why
    
    `codex-core` was re-exporting APIs owned by sibling `codex-*` crates,
    which made downstream crates depend on `codex-core` as a proxy module
    instead of the actual owner crate.
    
    Removing those forwards makes crate boundaries explicit and lets leaf
    crates drop unnecessary `codex-core` dependencies. In this PR, this
    reduces the dependency on `codex-core` to `codex-login` in the following
    files:
    
    ```
    codex-rs/backend-client/Cargo.toml
    codex-rs/mcp-server/tests/common/Cargo.toml
    ```
    
    ## What
    
    - Remove `codex-rs/core/src/lib.rs` re-exports for symbols owned by
    `codex-login`, `codex-mcp`, `codex-rollout`, `codex-analytics`,
    `codex-protocol`, `codex-shell-command`, `codex-sandboxing`,
    `codex-tools`, and `codex-utils-path`.
    - Delete the `default_client` forwarding shim in `codex-rs/core`.
    - Update in-crate and downstream callsites to import directly from the
    owning `codex-*` crate.
    - Add direct Cargo dependencies where callsites now target the owner
    crate, and remove `codex-core` from `codex-rs/backend-client`.
  • Remove client_common tool re-exports (#16482)
    ## Why
    
    `codex-rs/core/src/client_common.rs` still had a `tools` re-export
    module that forwarded `codex_tools` types back into `codex-core`. After
    the earlier extraction work in #16379, #16471, #16477, and #16481, that
    extra layer no longer adds value.
    
    Removing it keeps dependencies explicit: the `codex-core` modules that
    actually use `ToolSpec` and related types now depend on `codex_tools`
    directly instead of reaching through `client_common`.
    
    ## What Changed
    
    - removed the `client_common::tools` re-export module from
    `core/src/client_common.rs`
    - updated the remaining `codex-core` consumers to import `codex_tools`
    directly
    - adjusted the affected test code to reference
    `codex_tools::ResponsesApiTool` directly as well
    
    This is a mechanical cleanup only. It does not change tool behavior or
    runtime logic.
    
    ## Testing
    
    - `cargo test -p codex-core client_common::tests`
    - `cargo test -p codex-core tools::router::tests`
    - `cargo test -p codex-core tools::context::tests`
    - `cargo test -p codex-core tools::spec::tests`
  • codex-tools: extract configured tool specs (#16129)
    ## Why
    
    This continues the `codex-tools` migration by moving another passive
    tool-spec layer out of `codex-core`.
    
    After `ToolSpec` moved into `codex-tools`, `codex-core` still owned
    `ConfiguredToolSpec` and `create_tools_json_for_responses_api()`. Both
    are data-model and serialization helpers rather than runtime
    orchestration, so keeping them in `core/src/tools/registry.rs` and
    `core/src/tools/spec.rs` left passive tool-definition code coupled to
    `codex-core` longer than necessary.
    
    ## What changed
    
    - moved `ConfiguredToolSpec` into `codex-rs/tools/src/tool_spec.rs`
    - moved `create_tools_json_for_responses_api()` into
    `codex-rs/tools/src/tool_spec.rs`
    - re-exported the new surface from `codex-rs/tools/src/lib.rs`, which
    remains exports-only
    - updated `core/src/client.rs`, `core/src/tools/registry.rs`, and
    `core/src/tools/router.rs` to consume the extracted types and serializer
    from `codex-tools`
    - moved the tool-list serialization test into
    `codex-rs/tools/src/tool_spec_tests.rs`
    - added focused unit coverage for `ConfiguredToolSpec::name()`
    - simplified `core/src/tools/spec_tests.rs` to use the extracted
    `ConfiguredToolSpec::name()` directly and removed the now-redundant
    local `tool_name()` helper
    - updated `codex-rs/tools/README.md` so the crate boundary reflects the
    newly extracted tool-spec wrapper and serialization helper
    
    ## Test plan
    
    - `cargo test -p codex-tools`
    - `CARGO_TARGET_DIR=/tmp/codex-core-configured-spec cargo test -p
    codex-core --lib tools::spec::`
    - `CARGO_TARGET_DIR=/tmp/codex-core-configured-spec cargo test -p
    codex-core --lib client::`
    - `just fix -p codex-tools -p codex-core`
    - `just argument-comment-lint`
    
    ## References
    
    - #15923
    - #15928
    - #15944
    - #15953
    - #16031
    - #16047
  • [hooks] add non-streaming (non-stdin style) shell-only PostToolUse support (#15531)
    CHAINED PR - note that base is eternal/hooks-pretooluse-bash, not main
    -- so the following PR should be first
    
    Matching post-tool hook to the pre-tool functionality here:
    https://github.com/openai/codex/pull/15211
    
    So, PreToolUse calls for plain shell calls, allows blocking. This
    PostToolUse call runs after the command executed
    
    example run:
    
    ```
    › as a test, run in parallel the following commands:
      - echo 'one'
      - echo '[block-pre-tool-use]'
      - echo '[block-post-tool-use]'
    
    
    ⚠ MCP startup incomplete (failed: notion, linear)
    
    • Cruising through those three commands in parallel now, and I’ll share the exact outputs right after
      they land.
    
    • Running PreToolUse hook: checking the observatory runes
    
    • Running PreToolUse hook: checking the observatory runes
    
    • Running PreToolUse hook: checking the observatory runes
    
    PreToolUse hook (blocked)
      warning: wizard-tower PreToolUse demo blocked a Bash command on purpose.
      feedback: PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue.
    
    PreToolUse hook (completed)
      warning: wizard-tower PreToolUse demo inspected Bash: echo 'one'
    
    PreToolUse hook (completed)
      warning: wizard-tower PreToolUse demo inspected Bash: echo '[block-post-tool-use]'
    
    • Ran echo '[block-post-tool-use]'
      └ [block-post-tool-use]
    
    • Ran echo 'one'
      └ one
    
    • Running PostToolUse hook: reading the observatory aftermath
    
    • Running PostToolUse hook: reading the observatory aftermath
    
    PostToolUse hook (blocked)
      warning: wizard-tower PostToolUse demo blocked the Bash follow-through on purpose.
      feedback: PostToolUse demo blocked the result after execution. Remove [block-post-tool-use] to
    continue.
    
    PostToolUse hook (completed)
      warning: wizard-tower PostToolUse demo inspected Bash: echo 'one'
      hook context: PostToolUse demo saw the Bash result. Response preview: one
    
    ──────────────────────────────────────────────────────────────────────────────────────────────────────
    
    • Ran them in parallel. Little wave report:
    
      - echo 'one' completed and returned one.
      - echo '[block-pre-tool-use]' was blocked before execution by the PreToolUse hook.
      - echo '[block-post-tool-use]' ran, but its result was blocked afterward by the PostToolUse hook.
    
      The hook messages were:
    
      - PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue.
      - PostToolUse demo blocked the result after execution. Remove [block-post-tool-use] to continue.
    
    
    › for the blocked messages (correctly blocked in this test) did the tool output land back into
      context?
    
    
    • Yeah, from this run:
    
      - PreToolUse block: the command itself did not execute, so no command output landed in context. Only
        the block message did.
      - PostToolUse block: the command did execute, but the actual stdout did not come back into context.
        Only the post-tool block message did.
    
      So the blocked notifications landed in context, but the blocked command payloads themselves did not.
      Nice clean guardrail, aloha.
    ```
  • Use AbsolutePathBuf for cwd state (#15710)
    Migrate `cwd` and related session/config state to `AbsolutePathBuf` so
    downstream consumers consistently see absolute working directories.
    
    Add test-only `.abs()` helpers for `Path`, `PathBuf`, and `TempDir`, and
    update branch-local tests to use them instead of
    `AbsolutePathBuf::try_from(...)`.
    
    For the remaining TUI/app-server snapshot coverage that renders absolute
    cwd values, keep the snapshots unchanged and skip the Windows-only cases
    where the platform-specific absolute path layout differs.
  • [hooks] add non-streaming (non-stdin style) shell-only PreToolUse support (#15211)
    - add `PreToolUse` hook for bash-like tool execution only at first
    - block shell execution before dispatch with deny-only hook behavior
    - introduces common.rs matcher framework for matching when hooks are run
    
    example run:
    
    ```
    › run three parallel echo commands, and the second one should echo "[block-pre-tool-use]" as a test
    
    
    • Running the three echo commands in parallel now and I’ll report the output directly.
    
    • Running PreToolUse hook: name for demo pre tool use hook
    
    • Running PreToolUse hook: name for demo pre tool use hook
    
    • Running PreToolUse hook: name for demo pre tool use hook
    
    PreToolUse hook (completed)
      warning: wizard-tower PreToolUse demo inspected Bash: echo "first parallel echo"
      
    PreToolUse hook (blocked)
      warning: wizard-tower PreToolUse demo blocked a Bash command on purpose.
      feedback: PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue.
    
    PreToolUse hook (completed)
      warning: wizard-tower PreToolUse demo inspected Bash: echo "third parallel echo"
    
    • Ran echo "first parallel echo"
      └ first parallel echo
    
    • Ran echo "third parallel echo"
      └ third parallel echo
    
    • Three little waves went out in parallel.
    
      1. printed first parallel echo
      2. was blocked before execution because it contained the exact test string [block-pre-tool-use]
      3. printed third parallel echo
    
      There was also an unrelated macOS defaults warning around the successful commands, but the echoes
      themselves worked fine. If you want, I can rerun the second one with a slightly modified string so
      it passes cleanly.
    ```
  • 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: move inline codex-rs/core unit tests into sibling files (#14444)
    ## Why
    PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This
    applies the same extraction pattern across the rest of `codex-rs/core`
    so the production modules stay focused on runtime code instead of large
    inline test blocks.
    
    Keeping the tests in sibling files also makes follow-up edits easier to
    review because product changes no longer have to share a file with
    hundreds or thousands of lines of test scaffolding.
    
    ## What changed
    - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**`
    with a path-based module declaration
    - moved each extracted unit test module into a sibling `*_tests.rs`
    file, using `mod_tests.rs` for `mod.rs` modules
    - preserved the existing `cfg(...)` guards and module-local structure so
    the refactor remains structural rather than behavioral
    
    ## Testing
    - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`)
    - `just fix -p codex-core`
    - `cargo fmt --check`
    - `cargo shear`
  • fix: follow up on linux sandbox review nits (#14440)
    ## Summary
    - address the follow-up review nits from #13996 in a separate PR
    - make the approvals test command a raw string and keep the
    managed-network path using env proxy routing
    - inline `--apply-seccomp-then-exec` in the Linux sandbox inner command
    builder
    - remove the bubblewrap-specific sandbox metric tag path and drop the
    `use_legacy_landlock` shim from `sandbox_tag`/`TurnMetadataState::new`
    - restore the `Feature` import that `origin/main` currently still needs
    in `connectors.rs`
    
    ## Testing
    - `cargo test -p codex-linux-sandbox`
    - focused `codex-core` tests were rerun/started, but the final
    verification pass was interrupted when I pushed at request
  • refactor: make bubblewrap the default Linux sandbox (#13996)
    ## Summary
    - make bubblewrap the default Linux sandbox and keep
    `use_legacy_landlock` as the only override
    - remove `use_linux_sandbox_bwrap` from feature, config, schema, and
    docs surfaces
    - update Linux sandbox selection, CLI/config plumbing, and related
    tests/docs to match the new default
    - fold in the follow-up CI fixes for request-permissions responses and
    Linux read-only sandbox error text
  • feat: search_tool migrate to bring you own tool of Responses API (#14274)
    ## Why
    
    to support a new bring your own search tool in Responses
    API(https://developers.openai.com/api/docs/guides/tools-tool-search#client-executed-tool-search)
    we migrating our bm25 search tool to use official way to execute search
    on client and communicate additional tools to the model.
    
    ## What
    - replace the legacy `search_tool_bm25` flow with client-executed
    `tool_search`
    - add protocol, SSE, history, and normalization support for
    `tool_search_call` and `tool_search_output`
    - return namespaced Codex Apps search results and wire namespaced
    follow-up tool calls back into MCP dispatch
  • Expose strongly-typed result for exec_command (#14183)
    Summary
    - document output types for the various tool handlers and registry so
    the API exposes richer descriptions
    - update unified execution helpers and client tests to align with the
    new output metadata
    - clean up unused helpers across tool dispatch paths
    
    Testing
    - Not run (not requested)
  • Enforce single tool output type in codex handlers (#14157)
    We'll need to associate output schema with each tool. Each tool can only
    have on output type.
  • Refactor tool output into trait implementations (#14152)
    First state to making tool outputs strongly typed (and `renderable`).
  • chore(otel): rename OtelManager to SessionTelemetry (#13808)
    ## Summary
    This is a purely mechanical refactor of `OtelManager` ->
    `SessionTelemetry` to better convey what the struct is doing. No
    behavior change.
    
    ## Why
    
    `OtelManager` ended up sounding much broader than what this type
    actually does. It doesn't manage OTEL globally; it's the session-scoped
    telemetry surface for emitting log/trace events and recording metrics
    with consistent session metadata (`app_version`, `model`, `slug`,
    `originator`, etc.).
    
    `SessionTelemetry` is a more accurate name, and updating the call sites
    makes that boundary a lot easier to follow.
    
    ## Validation
    
    - `just fmt`
    - `cargo test -p codex-otel`
    - `cargo test -p codex-core`
  • notify: include client in legacy hook payload (#12968)
    ## Why
    
    The `notify` hook payload did not identify which Codex client started
    the turn. That meant downstream notification hooks could not distinguish
    between completions coming from the TUI and completions coming from
    app-server clients such as VS Code or Xcode. Now that the Codex App
    provides its own desktop notifications, it would be nice to be able to
    filter those out.
    
    This change adds that context without changing the existing payload
    shape for callers that do not know the client name, and keeps the new
    end-to-end test cross-platform.
    
    ## What changed
    
    - added an optional top-level `client` field to the legacy `notify` JSON
    payload
    - threaded that value through `core` and `hooks`; the internal session
    and turn state now carries it as `app_server_client_name`
    - set the field to `codex-tui` for TUI turns
    - captured `initialize.clientInfo.name` in the app server and applied it
    to subsequent turns before dispatching hooks
    - replaced the notify integration test hook with a `python3` script so
    the test does not rely on Unix shell permissions or `bash`
    - documented the new field in `docs/config.md`
    
    ## Testing
    
    - `cargo test -p codex-hooks`
    - `cargo test -p codex-tui`
    - `cargo test -p codex-app-server
    suite::v2::initialize::turn_start_notify_payload_includes_initialize_client_name
    -- --exact --nocapture`
    - `cargo test -p codex-core` (`src/lib.rs` passed; `core/tests/all.rs`
    still has unrelated existing failures in this environment)
    
    ## Docs
    
    The public config reference on `developers.openai.com/codex` should
    mention that the legacy `notify` payload may include a top-level
    `client` field. The TUI reports `codex-tui`, and the app server reports
    `initialize.clientInfo.name` when it is available.
  • Add MCP server context to otel tool_result logs (#12267)
    Summary
    - capture the origin for each configured MCP server and expose it via
    the connection manager
    - plumb MCP server name/origin into tool logging and emit
    codex.tool_result events with those fields
    - add unit coverage for origin parsing and extend OTEL tests to assert
    empty MCP fields for non-MCP tools
    - currently not logging full urls or url paths to prevent logging
    potentially sensitive data
    
    Testing
    - Not run (not requested)
  • Allow hooks to error (#11615)
    Allow hooks to return errors. 
    
    We should do this before introducing more hook types, or we'll have to
    migrate them all.
  • fix(core): add linux bubblewrap sandbox tag (#11767)
    ## Summary
    - add a distinct `linux_bubblewrap` sandbox tag when the Linux
    bubblewrap pipeline feature is enabled
    - thread the bubblewrap feature flag into sandbox tag generation for:
      - turn metadata header emission
      - tool telemetry metric tags and after-tool-use hooks
    - add focused unit tests for `sandbox_tag` precedence and Linux
    bubblewrap behavior
    
    ## Validation
    - `just fmt`
    - `cargo clippy -p codex-core --all-targets`
    - `cargo test -p codex-core sandbox_tags::tests`
    - started `cargo test -p codex-core` and stopped it per request
    
    Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
  • feat: make sandbox read access configurable with ReadOnlyAccess (#11387)
    `SandboxPolicy::ReadOnly` previously implied broad read access and could
    not express a narrower read surface.
    This change introduces an explicit read-access model so we can support
    user-configurable read restrictions in follow-up work, while preserving
    current behavior today.
    
    It also ensures unsupported backends fail closed for restricted-read
    policies instead of silently granting broader access than intended.
    
    ## What
    
    - Added `ReadOnlyAccess` in protocol with:
      - `Restricted { include_platform_defaults, readable_roots }`
      - `FullAccess`
    - Updated `SandboxPolicy` to carry read-access configuration:
      - `ReadOnly { access: ReadOnlyAccess }`
      - `WorkspaceWrite { ..., read_only_access: ReadOnlyAccess }`
    - Preserved existing behavior by defaulting current construction paths
    to `ReadOnlyAccess::FullAccess`.
    - Threaded the new fields through sandbox policy consumers and call
    sites across `core`, `tui`, `linux-sandbox`, `windows-sandbox`, and
    related tests.
    - Updated Seatbelt policy generation to honor restricted read roots by
    emitting scoped read rules when full read access is not granted.
    - Added fail-closed behavior on Linux and Windows backends when
    restricted read access is requested but not yet implemented there
    (`UnsupportedOperation`).
    - Regenerated app-server protocol schema and TypeScript artifacts,
    including `ReadOnlyAccess`.
    
    ## Compatibility / rollout
    
    - Runtime behavior remains unchanged by default (`FullAccess`).
    - API/schema changes are in place so future config wiring can enable
    restricted read access without another policy-shape migration.
  • Add AfterToolUse hook (#11335)
    Not wired up to config yet. (So we can change the name if we want)
    
    An example payload:
    
    ```
    {
      "session_id": "019c48b7-7098-7b61-bc48-32e82585d451",
      "cwd": "/Users/gt/code/codex/codex-rs",
      "triggered_at": "2026-02-10T18:02:31Z",
      "hook_event": {
        "event_type": "after_tool_use",
        "turn_id": "4",
        "call_id": "call_iuo4DqWgjE7OxQywnL2UzJUE",
        "tool_name": "apply_patch",
        "tool_kind": "custom",
        "tool_input": {
          "input_type": "custom",
          "input": "*** Begin Patch\n*** Update File: README.md\n@@\n-# Codex CLI hello (Rust Implementation)\n+# Codex CLI (Rust Implementation)\n*** End Patch\n"
        },
        "executed": true,
        "success": true,
        "duration_ms": 37,
        "mutating": true,
        "sandbox": "none",
        "sandbox_policy": "danger-full-access",
        "output_preview": "{\"output\":\"Success. Updated the following files:\\nM README.md\\n\",\"metadata\":{\"exit_code\":0,\"duration_seconds\":0.0}}"
      }
    }
    ```
  • include sandbox (seatbelt, elevated, etc.) as in turn metadata header (#10946)
    This will help us understand retention/usage for folks who use the
    Windows (or any other) sandboxes
  • add sandbox policy and sandbox name to codex.tool.call metrics (#10711)
    This will give visibility into the comparative success rate of the
    Windows sandbox implementations compared to other platforms.
  • Stop client from being state carrier (#10595)
    I'd like to make client session wide. This requires shedding all random
    state it has to carry.
  • nit: add docstring (#9099)
    Add docstring on `ToolHandler` trait
  • chore: align unified_exec (#5442)
    Align `unified_exec` with b implementation
  • feat: parallel tool calls (#4663)
    Add parallel tool calls. This is configurable at model level and tool
    level
  • chore: refactor tool handling (#4510)
    # Tool System Refactor
    
    - Centralizes tool definitions and execution in `core/src/tools/*`:
    specs (`spec.rs`), handlers (`handlers/*`), router (`router.rs`),
    registry/dispatch (`registry.rs`), and shared context (`context.rs`).
    One registry now builds the model-visible tool list and binds handlers.
    - Router converts model responses to tool calls; Registry dispatches
    with consistent telemetry via `codex-rs/otel` and unified error
    handling. Function, Local Shell, MCP, and experimental `unified_exec`
    all flow through this path; legacy shell aliases still work.
    - Rationale: reduce per‑tool boilerplate, keep spec/handler in sync, and
    make adding tools predictable and testable.
    
    Example: `read_file`
    - Spec: `core/src/tools/spec.rs` (see `create_read_file_tool`,
    registered by `build_specs`).
    - Handler: `core/src/tools/handlers/read_file.rs` (absolute `file_path`,
    1‑indexed `offset`, `limit`, `L#: ` prefixes, safe truncation).
    - E2E test: `core/tests/suite/read_file.rs` validates the tool returns
    the requested lines.
    
    ## Next steps:
    - Decompose `handle_container_exec_with_params` 
    - Add parallel tool calls