mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
6db6de031aaa2c30c690ab9749aee449d653e236
5055 Commits
-
build: fix Bazel lzma-sys wiring (#16634)
This seems to be required to fix bazel builds on an applied devbox ## Summary - add the Bazel `xz` module - wire `lzma-sys` directly to `@xz//:lzma` and disable its build script - refresh `MODULE.bazel.lock` ## Validation - `just bazel-lock-update` - `just bazel-lock-check` - `bazel run //codex-rs/cli:codex --run_under="cd $PWD &&" -- --version` - `just bazel-codex --version` Co-authored-by: Codex <noreply@openai.com>
starr-openai ·
2026-04-02 17:33:42 -07:00 -
test: use cmd.exe for ProviderAuthScript on Windows (#16629)
## Why The Windows `ProviderAuthScript` test helpers do not need PowerShell. Running them through `cmd.exe` is enough to emit the next fixture token and rotate `tokens.txt`, and it avoids a PowerShell-specific dependency in these tests. ## What changed - Replaced the Windows `print-token.ps1` fixtures with `print-token.cmd` in `codex-rs/core/src/models_manager/manager_tests.rs` and `codex-rs/login/src/auth/auth_tests.rs`. - Switched the failing external-auth helper in `codex-rs/login/src/auth/auth_tests.rs` from `powershell.exe -Command 'exit 1'` to `cmd.exe /d /s /c 'exit /b 1'`. - Updated Windows timeout comments so they no longer call out PowerShell specifically. ## Verification - `cargo test -p codex-login` - `cargo test -p codex-core` (fails in unrelated `core/src/config/config_tests.rs` assertions in this checkout)
Michael Bolin ·
2026-04-02 17:33:07 -07:00 -
app-server: make thread/shellCommand tests shell-aware (#16635)
## Why `thread/shellCommand` executes the raw command string through the current user shell, which is PowerShell on Windows. The two v2 app-server tests in `app-server/tests/suite/v2/thread_shell_command.rs` used POSIX `printf`, so Bazel CI on Windows failed with `printf` not being recognized as a PowerShell command. For reference, the user-shell task wraps commands with the active shell before execution: [`core/src/tasks/user_shell.rs`](https://github.com/openai/codex/blob/7a3eec6fdb356bd71f80582119eb829179ff0da1/codex-rs/core/src/tasks/user_shell.rs#L120-L126). ## What Changed Added a test-local helper that builds a shell-appropriate output command and expected newline sequence from `default_user_shell()`: - PowerShell: `Write-Output '...'` with `\r\n` - Cmd: `echo ...` with `\r\n` - POSIX shells: `printf '%s\n' ...` with `\n` Both `thread_shell_command_runs_as_standalone_turn_and_persists_history` and `thread_shell_command_uses_existing_active_turn` now use that helper. ## Verification - `cargo test -p codex-app-server thread_shell_command`
Michael Bolin ·
2026-04-02 17:28:47 -07:00 -
fix: address unused variable on windows (#16633)
This slipped in during https://github.com/openai/codex/pull/16578. I am still working on getting Windows working properly with Bazel on PRs.
Michael Bolin ·
2026-04-02 17:05:45 -07:00 -
Auto-trust cwd on thread start (#16492)
- Persist trusted cwd state during thread/start when the resolved sandbox is elevated. - Add app-server coverage for trusted root resolution and confirm turn/start does not mutate trust.
Ahmed Ibrahim ·
2026-04-03 00:02:56 +00:00 -
core: cut codex-core compile time 48% with native async SessionTask (#16631)
## Why This continues the compile-time cleanup from #16630. `SessionTask` implementations are monomorphized, but `Session` stores the task behind a `dyn` boundary so it can drive and abort heterogenous turn tasks uniformly. That means we can move the `#[async_trait]` expansion off the implementation trait, keep a small boxed adapter only at the storage boundary, and preserve the existing task lifecycle semantics while reducing the amount of generated async-trait glue in `codex-core`. One measurement caveat showed up while exploring this: a warm incremental benchmark based on `touch core/src/tasks/mod.rs && cargo check -p codex-core --lib` was basically flat, but that was the wrong benchmark for this change. Using package-clean `codex-core` rebuilds, like #16630, shows the real win. Relevant pre-change code: - [`SessionTask` with `#[async_trait]`](https://github.com/openai/codex/blob/3c7f013f9735e67796c70d95f75f436b7f97e3ec/codex-rs/core/src/tasks/mod.rs#L129-L182) - [`RunningTask` storing `Arc<dyn SessionTask>`](https://github.com/openai/codex/blob/3c7f013f9735e67796c70d95f75f436b7f97e3ec/codex-rs/core/src/state/turn.rs#L69-L77) ## What changed - Switched `SessionTask::{run, abort}` to native RPITIT futures with explicit `Send` bounds. - Added a private `AnySessionTask` adapter that boxes those futures only at the `Arc<dyn ...>` storage boundary. - Updated `RunningTask` to store `Arc<dyn AnySessionTask>` and removed `#[async_trait]` from the concrete task impls plus test-only `SessionTask` impls. ## Timing Benchmarked package-clean `codex-core` rebuilds with dependencies left warm: ```shell cargo check -p codex-core --lib >/dev/null cargo clean -p codex-core >/dev/null /usr/bin/time -p cargo +nightly rustc -p codex-core --lib -- \ -Z time-passes \ -Z time-passes-format=json >/dev/null ``` | revision | rustc `total` | process `real` | `generate_crate_metadata` | `MIR_borrow_checking` | `monomorphization_collector_graph_walk` | | --- | ---: | ---: | ---: | ---: | ---: | | parent `3c7f013f9735` | 67.21s | 67.71s | 24.61s | 23.43s | 22.43s | | this PR `2cafd783ac22` | 35.08s | 35.60s | 8.01s | 7.25s | 7.15s | | delta | -47.8% | -47.4% | -67.5% | -69.1% | -68.1% | For completeness, the warm touched-file benchmark stayed flat (`1.96s` parent vs `1.97s` this PR), which is why that benchmark should not be used to evaluate this refactor. ## Verification - Ran `cargo test -p codex-core`; this change compiled and task-related tests passed before hitting the same unrelated 5 `config::tests::*guardian*` failures already present on the parent stack.
Michael Bolin ·
2026-04-02 23:39:56 +00:00 -
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`
Michael Bolin ·
2026-04-02 16:03:52 -07:00 -
fix(tui): handle zellij redraw and composer rendering (#16578)
## TL;DR Fixes the issues when using Codex CLI with Zellij multiplexer. Before this PR there would be no scrollback when using it inside a zellij terminal. ## Problem Addresses #2558 Zellij does not support ANSI scroll-region manipulation (`DECSTBM` / Reverse Index) or the alternate screen buffer in the way traditional terminals do. When codex's TUI runs inside Zellij, two things break: (1) inline history insertion corrupts the display because the scroll-region escape sequences are silently dropped or mishandled, and (2) the composer textarea renders with inherited background/foreground styles that produce unreadable text against Zellij's pane chrome. ## Mental model The fix introduces a **Zellij mode** — a runtime boolean detected once at startup via `codex_terminal_detection::terminal_info().is_zellij()` — that gates two subsystems onto Zellij-safe terminal strategies: - **History insertion** (`insert_history.rs`): Instead of using `DECSTBM` scroll regions and Reverse Index (`ESC M`) to slide content above the viewport, Zellij mode scrolls the screen by emitting `\n` at the bottom row and then writes history lines at absolute positions. This avoids every escape sequence Zellij mishandles. - **Viewport expansion** (`tui.rs`): When the viewport grows taller than available space, the standard path uses `scroll_region_up` on the backend. Zellij mode instead emits newlines at the screen bottom to push content up, then invalidates the ratatui diff buffer so the next draw is a full repaint. - **Composer rendering** (`chat_composer.rs`, `textarea.rs`): All text rendering in the input area uses an explicit `base_style` with `Color::Reset` foreground, preventing Zellij's pane styling from bleeding into the textarea. The prompt chevron (`›`) and placeholder text use explicit color constants instead of relying on `.bold()` / `.dim()` modifiers that render inconsistently under Zellij. ## Non-goals - This change does not fix or improve Zellij's terminal emulation itself. - It does not rearchitect the inline viewport model; it adds a parallel code path gated on detection. - It does not touch the alternate-screen disable logic (that already existed and continues to use `is_zellij` via the same detection). ## Tradeoffs - **Code duplication in `insert_history.rs`**: The Zellij and Standard branches share the line-rendering loop (color setup, span merging, `write_spans`) but differ in the scrolling preamble. The duplication is intentional — merging them would force a complex conditional state machine that's harder to reason about than two flat sequences. - **`invalidate_viewport` after every Zellij history flush or viewport expansion**: This forces a full repaint on every draw cycle in Zellij, which is more expensive than ratatui's normal diff-based rendering. This is necessary because Zellij's lack of scroll-region support means the diff buffer's assumptions about what's on screen are invalid after we manually move content. - **Explicit colors vs semantic modifiers**: Replacing `.bold()` / `.dim()` with `Color::Cyan` / `Color::DarkGray` / `Color::White` in the Zellij branch sacrifices theme-awareness for correctness. If the project ever adopts a theming system, Zellij styling will need to participate. ## Architecture The Zellij detection flag flows through three layers: 1. **`codex_terminal_detection`** — `TerminalInfo::is_zellij()` (new convenience method) reads the already-detected `Multiplexer` variant. 2. **`Tui` struct** — caches `is_zellij` at construction; passes it into `update_inline_viewport`, `flush_pending_history_lines`, and `insert_history_lines_with_mode`. 3. **`ChatComposer` struct** — independently caches `is_zellij` at construction; uses it in `render_textarea` for style decisions. The two caches (`Tui.is_zellij` and `ChatComposer.is_zellij`) are read from the same global `OnceLock<TerminalInfo>`, so they always agree. ## Observability No new logging, metrics, or tracing is introduced. Diagnosis depends on: - Whether `ZELLIJ` or `ZELLIJ_SESSION_NAME` env vars are set (the detection heuristic). - Visual inspection of the rendered TUI inside Zellij vs a standard terminal. - The insta snapshot `zellij_empty_composer` captures the Zellij-mode render path. ## Tests - `terminal_info_reports_is_zellij` — unit test in `terminal-detection` confirming the convenience method. - `zellij_empty_composer_snapshot` — insta snapshot in `chat_composer` validating the Zellij render path for an empty composer. - `vt100_zellij_mode_inserts_history_and_updates_viewport` — integration test in `insert_history` verifying that Zellij-mode history insertion writes content and shifts the viewport. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fcoury-oai ·
2026-04-02 18:07:05 -03:00 -
Fix fork source display in /status (expose forked_from_id in app server) (#16596)
Addresses #16560 Problem: `/status` stopped showing the source thread id in forked TUI sessions after the app-server migration. Solution: Carry fork source ids through app-server v2 thread data and the TUI session adapter, and update TUI fixtures so `/status` matches the old TUI behavior.
Eric Traut ·
2026-04-02 14:05:29 -07:00 -
fix: add shell fallback paths for pwsh/powershell that work on GitHub Actions Windows runners (#16617)
Recently, I merged a number of PRs to increase startup timeouts for scripts that ran under PowerShell, but in the failure for `suite::codex_tool::test_shell_command_approval_triggers_elicitation`, I found this in the error logs when running on Bazel with BuildBuddy: ``` [mcp stderr] 2026-04-02T19:54:10.758951Z ERROR codex_core::tools::router: error=Exit code: 1 [mcp stderr] Wall time: 0.2 seconds [mcp stderr] Output: [mcp stderr] 'New-Item' is not recognized as an internal or external command, [mcp stderr] operable program or batch file. [mcp stderr] ``` This error implies that the command was run under `cmd.exe` instead of `pwsh.exe`. Under GitHub Actions, I suspect that the `%PATH%` that is passed to our Bazel builder is scrubbed such that our tests cannot find PowerShell where GitHub installs it. Having these explicit fallback paths should help. While we could enable these only for tests, I don't see any harm in keeping them in production, as well.
Michael Bolin ·
2026-04-02 13:47:10 -07:00 -
Fix stale turn steering during TUI review follow-ups (#16588)
Addresses #16389 Problem: `/review` follow-ups can crash when app-server TUI steers with a stale active turn id; #14717 introduced the client-side race, and #15714 only handled the “no active turn” half. Solution: Treat turn-id mismatch as stale cached state too, sync to the server’s current turn id, retry once, and let review turns fall into the existing queue path.
Eric Traut ·
2026-04-02 14:41:30 -06:00 -
Eric Traut ·
2026-04-02 14:34:23 -06:00 -
Fix resume picker stale thread names (#16601)
Addresses #16562 Problem: Resume picker could keep a stale backend-provided thread title instead of the latest name from session_index.jsonl. Solution: Always backfill/override picker row names from local session_index.jsonl and cover stale-name replacement with a regression test.
Eric Traut ·
2026-04-02 14:22:57 -06:00 -
Michael Bolin ·
2026-04-02 13:16:16 -07:00 -
Fix resume picker initial loading state (#16591)
Addresses #16514 Problem: Resume picker could show “No sessions yet” before the initial session fetch finished. Solution: Render a loading message while the first page is pending, and keep the empty state for truly empty results.
Eric Traut ·
2026-04-02 14:02:52 -06:00 -
fix: increase timeout to account for slow PowerShell startup (#16608)
Similar to https://github.com/openai/codex/pull/16604, I am seeing failures on Windows Bazel that could be due to PowerShell startup timeouts, so try increasing.
Michael Bolin ·
2026-04-02 12:40:19 -07:00 -
fix: add more detail to test assertion (#16606)
In https://github.com/openai/codex/pull/16528, I am trying to get tests running under Bazel on Windows, but currently I see: ``` thread 'suite::user_shell_cmd::user_shell_command_does_not_set_network_sandbox_env_var' (10220) panicked at core/tests\suite\user_shell_cmd.rs:358:5: assertion failed: `(left == right)` Diff < left / right > : <1 >0 ``` This PR updates the `assert_eq!()` to provide more information to help diagnose the failure. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16606). * #16608 * __->__ #16606
Michael Bolin ·
2026-04-02 12:34:42 -07:00 -
test: deflake external bearer auth token tests on Windows (#16604)
## Why `external_bearer_only_auth_manager_uses_cached_provider_token` can fail on Windows when cold `powershell.exe` startup exceeds the provider-auth helper's 1s timeout. When that happens, `AuthManager::resolve_external_api_key_auth()` [logs the resolver error and returns `None`](https://github.com/openai/codex/blob/024b08b411fe/codex-rs/login/src/auth/manager.rs#L1449-L1455), which is exactly the assertion failure from the flake. ## What - Invoke `powershell.exe` explicitly in the Windows provider-auth test helpers in `login/src/auth/auth_tests.rs`. - Increase the helper timeout to `10_000` ms and document why that slack exists. ## Verification - `cargo test -p codex-login`
Michael Bolin ·
2026-04-02 12:12:18 -07:00 -
Fix non-determinism in rules_rs/crate_git_repository.bzl (#16590)
Running multiple builds with no changes causes some differences, we see that https://app.buildbuddy.io/compare/a9719629-1660-4735-a477-d66357f234fb...df85310b-eb5c-4c10-8b79-4d0449ba6cdd#file shows the file-differences between two Bazel builds. These differences are caused by a non-deterministic `.git` entry in the rules_rs crates that are created with `crate_git_repository`. As a way to make these deterministic, we can remove this entry after we download the git source, so that the input to the compile action is deterministic. ### CLA I have read the CLA Document and I hereby sign the CLA
Tyler French ·
2026-04-02 11:21:11 -07:00 -
chore: move codex-exec unit tests into sibling files (#16581)
## Why `codex-rs/exec/src/lib.rs` already keeps unit tests in a sibling `lib_tests.rs` module so the implementation stays top-heavy and easier to read. This applies that same layout to the rest of `codex-rs/exec/src` so each production file keeps its entry points and helpers ahead of test code. ## What - Move inline unit tests out of `cli.rs`, `main.rs`, `event_processor_with_human_output.rs`, and `event_processor_with_jsonl_output.rs` into sibling `*_tests.rs` files. - Keep test modules wired through `#[cfg(test)]` plus `#[path = "..."] mod tests;`, matching the `lib.rs` pattern. - Preserve the existing test coverage and assertions while making this a source-layout-only refactor. ## Verification - `cargo test -p codex-exec`
Michael Bolin ·
2026-04-02 10:01:40 -07:00 -
ci: upload compact Bazel execution logs for bazel.yml (#16577)
## Why The main Bazel CI lanes need compact execution logs to investigate cache misses and unexpected rebuilds, but local users of the shared wrapper should not pay that log-generation cost by default. ## What Changed - [`.github/scripts/run-bazel-ci.sh`](https://github.com/openai/codex/blob/a6ec239a24fd21507bb75a28f17277653f52e3a2/.github/scripts/run-bazel-ci.sh#L149-L153) now appends `--execution_log_compact_file=...` only when `CODEX_BAZEL_EXECUTION_LOG_COMPACT_DIR` is set; the caller owns creating that directory. - [`.github/workflows/bazel.yml`](https://github.com/openai/codex/blob/a6ec239a24fd21507bb75a28f17277653f52e3a2/.github/workflows/bazel.yml#L66-L174) enables that env var only for the main `test` and `clippy` jobs, creates the temp log directory in each job, and uploads the resulting `*.zst` files from `runner.temp`. ## Verification - `bash -n .github/scripts/run-bazel-ci.sh` - Parsed `.github/workflows/bazel.yml` as YAML. - Ran a local opt-in wrapper smoke test and confirmed it writes `execution-log-cquery-local-*.zst` when the caller pre-creates `CODEX_BAZEL_EXECUTION_LOG_COMPACT_DIR`.
Michael Bolin ·
2026-04-02 08:41:04 -07:00 -
jif-oai ·
2026-04-02 16:51:17 +02:00 -
jif-oai ·
2026-04-02 16:18:53 +02:00 -
jif-oai ·
2026-04-02 16:15:28 +02:00 -
jif-oai ·
2026-04-02 15:55:55 +02:00 -
jif-oai ·
2026-04-02 15:41:18 +02:00 -
jif-oai ·
2026-04-02 15:31:30 +02:00 -
jif-oai ·
2026-04-02 14:48:43 +02:00 -
[codex] Remove codex-core config type shim (#16529)
## Why This finishes the config-type move out of `codex-core` by removing the temporary compatibility shim in `codex_core::config::types`. Callers now depend on `codex-config` directly, which keeps these config model types owned by the config crate instead of re-expanding `codex-core` as a transitive API surface. ## What Changed - Removed the `codex-rs/core/src/config/types.rs` re-export shim and the `core::config::ApprovalsReviewer` re-export. - Updated `codex-core`, `codex-cli`, `codex-tui`, `codex-app-server`, `codex-mcp-server`, and `codex-linux-sandbox` call sites to import `codex_config::types` directly. - Added explicit `codex-config` dependencies to downstream crates that previously relied on the `codex-core` re-export. - Regenerated `codex-rs/core/config.schema.json` after updating the config docs path reference.
Michael Bolin ·
2026-04-02 01:19:44 -07:00 -
fix: move some test utilities out of codex-rs/core/src/tools/spec.rs (#16524)
The `#[cfg(test)]` in `codex-rs/core/src/tools/spec.rs` smelled funny to me and it turns out these members were straightforward to move.
Michael Bolin ·
2026-04-02 00:49:37 -07:00 -
[codex] Move config types into codex-config (#16523)
## Why `codex-rs/core/src/config/types.rs` is a plain config-type module with no dependency on `codex-core`. Moving it into `codex-config` shrinks the core crate and gives config-only consumers a more natural dependency boundary. ## What Changed - Added `codex_config::types` with the moved structs, enums, constants, and unit tests. - Kept `codex_core::config::types` as a compatibility re-export to avoid a broad call-site migration in this PR. - Switched notice-table writes in `core/src/config/edit.rs` to a local `NOTICE_TABLE_KEY` constant. - Added the `wildmatch` runtime dependency and `tempfile` test dependency to `codex-config`.
Michael Bolin ·
2026-04-02 00:39:20 -07:00 -
Move tool registry plan tests into codex-tools (#16521)
## Why #16513 moved pure tool-registry planning into `codex-tools`, but much of the corresponding spec/feature-gating coverage still lived in `codex-core`. That leaves the tests for planner behavior in the crate that no longer owns that logic and makes the next extraction steps harder to review. ## What Move the planner-only `spec_tests.rs` coverage into `codex-rs/tools/src/tool_registry_plan_tests.rs` and wire it up from `codex-rs/tools/src/tool_registry_plan.rs` using the crate-local `#[path = "tool_registry_plan_tests.rs"] mod tests;` pattern. The `codex-core` test file now keeps the core-side integration checks: router-visible model tool lists, namespaced handler alias registration, shell adapter behavior, and MCP schema edge cases that still exercise the `core` binding layer. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-core tools::spec::tests`
Michael Bolin ·
2026-04-02 00:26:51 -07:00 -
Extract tool registry planning into codex-tools (#16513)
## Why This is a larger step in the `codex-core` -> `codex-tools` migration called out in `AGENTS.md`. `codex-rs/core/src/tools/spec.rs` had become mostly pure tool-spec assembly plus handler registration. That made it hard to move more of the tool-definition layer into `codex-tools`, because the runtime binding and the crate-independent planning logic were still interleaved in one function. Splitting those concerns gives `codex-tools` ownership of the declarative registry plan while keeping `codex-core` responsible for instantiating concrete handlers. ## What Changed - Add a `codex-tools` registry-plan layer in `codex-rs/tools/src/tool_registry_plan.rs` and `codex-rs/tools/src/tool_registry_plan_types.rs`. - Move feature-gated tool-spec assembly, MCP/dynamic tool conversion, tool-search aliases, and code-mode nested-plan expansion into `codex-tools`. - Keep `codex-rs/core/src/tools/spec.rs` as the core-side adapter that maps each planned handler kind to concrete runtime handler instances. - Update `spec_tests.rs` to import the moved `codex_tools` symbols directly instead of relying on top-level `spec.rs` re-exports. This is intended to be a straight refactor with no behavior change and no new test surface. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-core tools::spec::tests` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16513). * #16521 * __->__ #16513
Michael Bolin ·
2026-04-02 00:18:18 -07:00 -
fix: add update to Cargo.lock that was missed in #16512 (#16516)
This PR updates `Cargo.lock` to remove `codex-core` from `mcp_test_support`, which corresponds to `codex-rs/mcp-server/tests/common/Cargo.toml`. As noted in #16512, it updated that crate to drop its `codex-core` dependency.
Michael Bolin ·
2026-04-01 23:33:41 -07:00 -
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`.
Michael Bolin ·
2026-04-01 23:06:24 -07:00 -
Extract code-mode nested tool collection into codex-tools (#16509)
## Why This is another small step in the `codex-core` -> `codex-tools` migration described in `AGENTS.md`. `core/src/tools/spec.rs` and `core/src/tools/code_mode/mod.rs` were both hand-rolling the same pure transformation: convert visible `ToolSpec`s into code-mode nested tool definitions, then sort and deduplicate by tool name. That logic does not depend on core runtime state or handlers, so keeping it in `codex-core` makes `spec.rs` harder to peel out later than it needs to be. ## What Changed - Add `collect_code_mode_tool_definitions()` to `codex-rs/tools/src/code_mode.rs`. - Reuse that helper from `codex-rs/core/src/tools/spec.rs` when assembling the `exec` tool description. - Reuse the same helper from `codex-rs/core/src/tools/code_mode/mod.rs` when exposing nested tool metadata to the code-mode runtime. This is intended to be a straight refactor with no behavior change and no new test surface. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-core tools::spec::tests` - `cargo test -p codex-core code_mode_only_`
Michael Bolin ·
2026-04-01 22:17:55 -07:00 -
core: use codex-mcp APIs directly (#16510)
## Why `codex-mcp` already owns the shared MCP API surface, including `auth`, `McpConfig`, `CODEX_APPS_MCP_SERVER_NAME`, and tool-name helpers in [`codex-rs/codex-mcp/src/mcp/mod.rs`](https://github.com/openai/codex/blob/f61e85dbfb5373cde6827d232ac8ea447c237e81/codex-rs/codex-mcp/src/mcp/mod.rs#L1-L35). Re-exporting that surface from `codex_core::mcp` gives downstream crates two import paths for the same API and hides the real crate dependency. This PR keeps `codex_core::mcp` focused on the local `McpManager` wrapper in [`codex-rs/core/src/mcp.rs`](https://github.com/openai/codex/blob/f61e85dbfb5373cde6827d232ac8ea447c237e81/codex-rs/core/src/mcp.rs#L13-L40) and makes consumers import shared MCP APIs from `codex_mcp` directly. ## What - Remove the `codex_mcp::mcp` re-export surface from `core/src/mcp.rs`. - Update `codex-core` internals plus `codex-app-server`, `codex-cli`, and `codex-tui` test code to import MCP APIs from `codex_mcp::mcp` directly. - Add explicit `codex-mcp` dependencies where those crates now use that API surface, and refresh `Cargo.lock`. ## Verification - `just bazel-lock-check` - `cargo test -p codex-core -p codex-cli -p codex-tui` - `codex-cli` passed. - `codex-core` still fails five unrelated config tests in `core/src/config/config_tests.rs` (`approvals_reviewer_*` and `smart_approvals_alias_*`). - A broader `cargo test -p codex-core -p codex-app-server -p codex-cli -p codex-tui` run previously hung in `codex-app-server` test `in_process_start_uses_requested_session_source_for_thread_start`.
Michael Bolin ·
2026-04-01 21:55:22 -07:00 -
Extract request_user_input normalization into codex-tools (#16503)
## Why This is another incremental step in the `codex-core` -> `codex-tools` migration called out in `AGENTS.md`: keep pure tool-definition and wire-shaping logic out of `codex-core` so the core crate can stay focused on runtime orchestration. `request_user_input` already had its spec and mode-availability helpers in `codex-tools` after #16471. The remaining argument validation and normalization still lived in the core runtime handler, which left that tool split across the two crates. ## What Changed - Export `REQUEST_USER_INPUT_TOOL_NAME` and `normalize_request_user_input_args()` from `codex-rs/tools/src/request_user_input_tool.rs`. - Use that `codex-tools` surface from `codex-rs/core/src/tools/spec.rs` and `codex-rs/core/src/tools/handlers/request_user_input.rs`. - Keep the core handler responsible for payload parsing, session dispatch, cancellation handling, and response serialization. This is intended to be a straight refactor with no behavior change. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-core request_user_input`
Michael Bolin ·
2026-04-01 21:18:45 -07:00 -
core: use codex-tools config types directly (#16504)
## Why `codex-rs/tools/src/lib.rs` already defines the [canonical `codex_tools` export surface](https://github.com/openai/codex/blob/bf081b9e286e620d25c1d61cf0f38f74a0f982ce/codex-rs/tools/src/lib.rs#L83-L88) for `ToolsConfig`, `ToolsConfigParams`, and the shell backend config types. Re-exporting those same types from `core/src/tools/spec.rs` gives `codex-core` two import paths for one API and blurs which crate owns those config definitions. This PR removes that duplicate path so `codex-core` callsites depend on `codex_tools` directly. ## What - Remove the five `codex_tools` re-exports from `core/src/tools/spec.rs`. - Update `codex-core` production and test callsites to import `ShellCommandBackendConfig`, `ToolsConfig`, `ToolsConfigParams`, `UnifiedExecShellMode`, and `ZshForkConfig` from `codex_tools`. ## Verification - Ran `cargo test -p codex-core`. - The package run is currently red in five unrelated config tests in `core/src/config/config_tests.rs` (`approvals_reviewer_*` and `smart_approvals_alias_*`), while the tool/spec and shell tests touched by this import cleanup passed.
Michael Bolin ·
2026-04-01 21:16:44 -07:00 -
Fix paste-driven bottom pane completion teardown (#16202)
Fix paste-driven bottom-pane completion teardown (#16192) `BottomPane::handle_paste()` could leave a completed modal flow mounted while re-enabling the composer, putting the TUI in an inconsistent state where stale views could still affect rendering and input routing. Align the paste path with the existing key-driven completion logic by tearing down the active modal flow before restoring composer input, and add a regression test covering the stacked-view case that exposed the bug. Big thanks to @iqdoctor for identifying the root cause for this issue.
Eric Traut ·
2026-04-01 22:03:13 -06:00 -
Fix TUI app-server permission profile conversions (#16284)
Addresses #16283 Problem: TUI app-server permission approvals could drop filesystem grants because request and response payloads were round-tripped through mismatched camelCase and snake_case JSON shapes. Solution: Replace the lossy JSON round-trips with typed app-server/core permission conversions so requested and granted permission profiles, including filesystem paths and scope, are preserved end to end.
Eric Traut ·
2026-04-01 22:00:27 -06:00 -
Extract tool-suggest wire helpers into codex-tools (#16499)
## Why This is another straight-refactor step in the `codex-tools` migration. `core/src/tools/handlers/tool_suggest.rs` still owned request/response payload structs, elicitation metadata shaping, and connector-completion predicates that do not depend on `codex-core` session/runtime internals. Per the `AGENTS.md` guidance to keep shrinking `codex-core`, this moves that pure wire-format logic into `codex-rs/tools` so the core handler keeps only session orchestration, plugin/config refresh, and MCP cache updates. ## What changed - Added `codex-rs/tools/src/tool_suggest.rs` and exported its API from `codex-rs/tools/src/lib.rs`. - Moved `ToolSuggestArgs`, `ToolSuggestResult`, `ToolSuggestMeta`, `build_tool_suggestion_elicitation_request()`, `all_suggested_connectors_picked_up()`, and `verified_connector_suggestion_completed()` into `codex-tools`. - Rewired `core/src/tools/handlers/tool_suggest.rs` to consume those exports directly. - Ported the existing pure helper tests from `core/src/tools/handlers/tool_suggest_tests.rs` to `tools/src/tool_suggest_tests.rs` without adding new behavior coverage. ## Validation ```shell cargo test -p codex-tools cargo test -p codex-core tools::handlers::tool_suggest::tests just argument-comment-lint ```
Michael Bolin ·
2026-04-01 20:49:15 -07:00 -
fix: guard guardian_command_source_tool_name with cfg(unix) (#16498)
This currently contributing to `rust-ci-full.yml` being red on `main` for windows lint builds due to the cargo/bazel coverage gap that I'm working on. Hopefully this gets us back on track.
Michael Bolin ·
2026-04-01 20:16:44 -07:00 -
Extract tool-search output helpers into codex-tools (#16497)
## Why This is the next straight-refactor step in the `codex-tools` migration that follows #16493. `codex-rs/core` still owned a chunk of pure tool-discovery metadata and response shaping even though the corresponding `tool_search` / `tool_suggest` specs already live in `codex-rs/tools`. Per the guidance in `AGENTS.md`, this moves that crate-agnostic logic out of `codex-core` so the handler crate keeps only the BM25 ranking/orchestration and runtime glue. ## What changed - Moved the canonical `tool_search` / `tool_suggest` tool names and the `tool_search` default limit into `codex-rs/tools/src/tool_discovery.rs`. - Added `ToolSearchResultSource` and `collect_tool_search_output_tools()` in `codex-tools` so namespace grouping and deferred Responses API tool serialization happen outside `codex-core`. - Rewired `ToolSearchHandler`, `ToolSuggestHandler`, and `core/src/tools/spec.rs` to consume those exports directly from `codex-tools`. - Ported the existing `tool_search` serializer tests from `core/src/tools/handlers/tool_search_tests.rs` to `tools/src/tool_discovery_tests.rs` without adding new behavior coverage. ## Validation ```shell cargo test -p codex-tools cargo test -p codex-core tools::spec::tests just argument-comment-lint ```
Michael Bolin ·
2026-04-01 20:16:21 -07:00 -
Fix regression: "not available in TUI" error message (#16273)
Addresses a recent TUI regression Problem: Pressing Ctrl+C during early TUI startup could route an interrupt with no active turn into the generic unsupported-op fallback, showing “Not available in app-server TUI yet for thread …” repeatedly. Solution: Treat interrupt requests as handled when no active turn exists yet, preventing fallback error spam during startup, and add a regression test covering interrupt-without-active-turn behavior.
Eric Traut ·
2026-04-01 21:01:36 -06:00 -
Extract built-in tool spec constructors into codex-tools (#16493)
## Why `core/src/tools/spec.rs` still had a few built-in tool specs assembled inline even though those definitions are pure metadata and already live conceptually in `codex-tools`. Keeping that construction in `codex-core` makes `spec.rs` do more than registry orchestration and slows the migration toward a right-sized `codex-tools` crate. This continues the extraction stack from #16379, #16471, #16477, #16481, and #16482. ## What Changed - added `create_local_shell_tool()`, `create_web_search_tool(...)`, and `create_image_generation_tool(...)` to `codex-rs/tools/src/tool_spec.rs` - exported those helpers from `codex-rs/tools/src/lib.rs` - switched `codex-rs/core/src/tools/spec.rs` to call those helpers instead of constructing `ToolSpec::LocalShell`, `ToolSpec::WebSearch`, and `ToolSpec::ImageGeneration` inline - removed the remaining core-local web-search content-type constant and made the affected spec test assert the literal expected values directly This is intended to be a straight refactor: tool behavior and wire shape should not change. ## Testing - `cargo test -p codex-tools` - `cargo test -p codex-core tools::spec::tests`
Michael Bolin ·
2026-04-01 19:31:24 -07:00 -
fix: remove unused import (#16495)
This lint violation slipped through because our Bazel CI setup currently doesn't cover `--tests` when doing `cargo clippy`. I am working on fixing this via: - https://github.com/openai/codex/pull/16450 - https://github.com/openai/codex/pull/16460
Michael Bolin ·
2026-04-01 19:27:26 -07:00 -
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`
Michael Bolin ·
2026-04-01 19:15:15 -07:00 -
Extract MCP into codex-mcp crate (#15919)
- Split MCP runtime/server code out of `codex-core` into the new `codex-mcp` crate. New/moved public structs/types include `McpConfig`, `McpConnectionManager`, `ToolInfo`, `ToolPluginProvenance`, `CodexAppsToolsCacheKey`, and the `McpManager` API (`codex_mcp::mcp::McpManager` plus the `codex_core::mcp::McpManager` wrapper/shim). New/moved functions include `with_codex_apps_mcp`, `configured_mcp_servers`, `effective_mcp_servers`, `collect_mcp_snapshot`, `collect_mcp_snapshot_from_manager`, `qualified_mcp_tool_name_prefix`, and the MCP auth/skill-dependency helpers. Why: this creates a focused MCP crate boundary and shrinks `codex-core` without forcing every consumer to migrate in the same PR. - Move MCP server config schema and persistence into `codex-config`. New/moved structs/enums include `AppToolApproval`, `McpServerToolConfig`, `McpServerConfig`, `RawMcpServerConfig`, `McpServerTransportConfig`, `McpServerDisabledReason`, and `codex_config::ConfigEditsBuilder`. New/moved functions include `load_global_mcp_servers` and `ConfigEditsBuilder::replace_mcp_servers`/`apply`. Why: MCP TOML parsing/editing is config ownership, and this keeps config validation/round-tripping (including per-tool approval overrides and inline bearer-token rejection) in the config crate instead of `codex-core`. - Rewire `codex-core`, app-server, and plugin call sites onto the new crates. Updated `Config::to_mcp_config(&self, plugins_manager)`, `codex-rs/core/src/mcp.rs`, `codex-rs/core/src/connectors.rs`, `codex-rs/core/src/codex.rs`, `CodexMessageProcessor::list_mcp_server_status_task`, and `utils/plugins/src/mcp_connector.rs` to build/pass the new MCP config/runtime types. Why: plugin-provided MCP servers still merge with user-configured servers, and runtime auth (`CodexAuth`) is threaded into `with_codex_apps_mcp` / `collect_mcp_snapshot` explicitly so `McpConfig` stays config-only.
Ahmed Ibrahim ·
2026-04-01 19:03:26 -07:00 -
Extract update_plan tool spec into codex-tools (#16481)
## Why `codex-rs/core/src/tools/handlers/plan.rs` still owned both the `update_plan` runtime handler and the static tool definition. The tool definition is pure metadata, so keeping it in `codex-core` works against the ongoing effort to move tool-spec code into `codex-tools` and keep `codex-core` focused on orchestration and execution paths. This continues the extraction work from #16379, #16471, and #16477. ## What Changed - added `codex-rs/tools/src/plan_tool.rs` with `create_update_plan_tool()` - re-exported that constructor from `codex-rs/tools/src/lib.rs` - updated `codex-rs/core/src/tools/spec.rs` and `codex-rs/core/src/tools/spec_tests.rs` to use the `codex-tools` export instead of a core-local static - removed the old `PLAN_TOOL` definition from `codex-rs/core/src/tools/handlers/plan.rs`; the `PlanHandler` runtime logic still stays in `codex-core` - tightened two `codex-core` aliases to `#[cfg(test)]` now that production code no longer needs them ## Testing - `cargo test -p codex-tools` - `cargo test -p codex-core tools::spec::tests` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16481). * #16482 * __->__ #16481
Michael Bolin ·
2026-04-01 15:51:52 -07:00