## 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]`](3c7f013f97/codex-rs/core/src/tasks/mod.rs (L129-L182))
- [`RunningTask` storing `Arc<dyn
SessionTask>`](3c7f013f97/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.
## 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>
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.
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.
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.
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.
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.
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
## 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`
## 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`
## 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.
## 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`.
## 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`
## 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
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.
## 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`.
## 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_`
## 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`](f61e85dbfb/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`](f61e85dbfb/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`.
## 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`
## Why
`codex-rs/tools/src/lib.rs` already defines the [canonical `codex_tools`
export
surface](bf081b9e28/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.
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.
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.
## 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
```
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.
## 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
```
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.
## 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`
## 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`
- 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.
## 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
## Description
Previously the `action` field on `EventMsg::GuardianAssessment`, which
describes what Guardian is reviewing, was typed as an arbitrary JSON
blob. This PR cleans it up and defines a sum type representing all the
various actions that Guardian can review.
This is a breaking change (on purpose), which is fine because:
- the Codex app / VSCE does not actually use `action` at the moment
- the TUI code that consumes `action` is updated in this PR as well
- rollout files that serialized old `EventMsg::GuardianAssessment` will
just silently drop these guardian events
- the contract is defined as unstable, so other clients have a fair
warning :)
This will make things much easier for followup Guardian work.
## Why
The old guardian review payloads worked, but they pushed too much shape
knowledge into downstream consumers. The TUI had custom JSON parsing
logic for commands, patches, network requests, and MCP calls, and the
app-server protocol was effectively just passing through an opaque blob.
Typing this at the protocol boundary makes the contract clearer.
## Why
Follow-up to #16288: the new dynamic provider auth token flow currently
defaults `refresh_interval_ms` to a non-zero value and rejects `0`
entirely.
For command-backed bearer auth, `0` should mean "never auto-refresh".
That lets callers keep using the cached token until the backend actually
returns `401 Unauthorized`, at which point Codex can rerun the auth
command as part of the existing retry path.
## What changed
- changed `ModelProviderAuthInfo.refresh_interval_ms` to accept `0` and
documented that value as disabling proactive refresh
- updated the external bearer token refresher to treat
`refresh_interval_ms = 0` as an indefinitely reusable cached token,
while still rerunning the auth command during unauthorized recovery
- regenerated `core/config.schema.json` so the schema minimum is `0` and
the new behavior is described in the field docs
- added coverage for both config deserialization and the no-auto-refresh
plus `401` recovery behavior
## How tested
- `cargo test -p codex-protocol`
- `cargo test -p codex-login`
- `cargo test -p codex-core test_deserialize_provider_auth_config_`
## Why
Follow-up to #16379 and #16471.
`codex-rs/core/src/tools/spec.rs` still owned the pure discovery-shaping
helpers that turn app metadata and discoverable tool metadata into the
inputs used by `tool_search` and `tool_suggest`. Those helpers do not
need `codex-core` runtime state, so keeping them in `codex-core`
continued to blur the crate boundary this migration is trying to
tighten.
This change keeps pushing spec-only logic behind the `codex-tools` API
so `codex-core` can focus on wiring runtime handlers to the resulting
tool definitions.
## What Changed
- Added `collect_tool_search_app_infos` and
`collect_tool_suggest_entries` to
`codex-rs/tools/src/tool_discovery.rs`.
- Added a small `ToolSearchAppSource` adapter type in `codex-tools` so
`codex-core` can pass app metadata into that shared helper logic without
exposing `ToolInfo` across the crate boundary.
- Re-exported the new discovery helpers from
`codex-rs/tools/src/lib.rs`, which remains exports-only.
- Updated `codex-rs/core/src/tools/spec.rs` to use those `codex-tools`
helpers instead of maintaining local `tool_search_app_infos` and
`tool_suggest_entries` functions.
- Removed the now-redundant helper implementations from `codex-core`.
## Testing
- `cargo test -p codex-tools`
- `cargo test -p codex-core tools::spec::tests`
## Why
Now that workspace crate features have been removed and
`.github/scripts/verify_cargo_workspace_manifests.py` hard-bans new
ones, Rust CI should stop building and testing with `--all-features`.
Keeping `--all-features` in CI no longer buys us meaningful coverage for
`codex-rs`, but it still makes the workflow look like we rely on Cargo
feature permutations that we are explicitly trying to eliminate. It also
leaves stale examples in the repo that suggest `--all-features` is a
normal or recommended way to run the workspace.
## What changed
- removed `--all-features` from the Rust CI `cargo chef cook`, `cargo
clippy`, and `cargo nextest` invocations in
`.github/workflows/rust-ci-full.yml`
- updated the `just test` guidance in `justfile` to reflect that
workspace crate features are banned and there should be no need to add
`--all-features`
- updated the multiline command example and snapshot in
`codex-rs/tui/src/history_cell.rs` to stop rendering `cargo test
--all-features --quiet`
- tightened the verifier docstring in
`.github/scripts/verify_cargo_workspace_manifests.py` so it no longer
talks about temporary remaining exceptions
## How tested
- `python3 .github/scripts/verify_cargo_workspace_manifests.py`
- `cargo test -p codex-tui`
## Why
Follow-up to #16379.
`codex-rs/core/src/tools/spec.rs` and the corresponding handlers still
owned several pure tool-definition helpers even though they do not need
`codex-core` runtime state. Keeping that spec-only logic in `codex-core`
keeps the crate boundary blurry and works against the guidance in
`AGENTS.md` to keep shared tooling out of `codex-core` when possible.
This change takes another step toward a dedicated `codex-tools` crate by
moving more metadata and schema-building code behind the `codex-tools`
API while leaving the actual tool execution paths in `codex-core`.
## What Changed
- Added `codex-rs/tools/src/apply_patch_tool.rs` to own
`ApplyPatchToolArgs`, the freeform/json `apply_patch` tool specs, and
the moved `tool_apply_patch.lark` grammar.
- Updated `codex-rs/tools/BUILD.bazel` so Bazel exposes the moved
grammar file to `codex-tools`.
- Moved the `request_user_input` availability and description helpers
into `codex-rs/tools/src/request_user_input_tool.rs`, with the related
unit tests moved alongside that business logic.
- Moved `request_permissions_tool_description()` into
`codex-rs/tools/src/local_tool.rs`.
- Rewired `codex-rs/core/src/tools/spec.rs`,
`codex-rs/core/src/tools/handlers/apply_patch.rs`, and
`codex-rs/core/src/tools/handlers/request_user_input.rs` to consume the
new `codex-tools` exports instead of local helper code.
- Removed the now-redundant helper implementations and tests from
`codex-core`, plus a couple of stale `client_common` re-exports that
became unused after the move.
## Testing
- `cargo test -p codex-tools`
- `cargo test -p codex-core tools::spec::tests`
- `cargo test -p codex-core tools::handlers::apply_patch::tests`
## Why
`codex-otel` still carried `disable-default-metrics-exporter`, which was
the last remaining workspace crate feature.
We are removing workspace crate features because they do not fit our
current build model well:
- our Bazel setup does not honor crate features today, which can let
feature-gated issues go unnoticed
- they create extra crate build permutations that we want to avoid
For this case, the feature was only being used to keep the built-in
Statsig metrics exporter off in test and debug-oriented contexts. This
repo already treats `debug_assertions` as the practical proxy for that
class of behavior, so OTEL should follow the same convention instead of
keeping a dedicated crate feature alive.
## What changed
- removed `disable-default-metrics-exporter` from
`codex-rs/otel/Cargo.toml`
- removed the `codex-otel` dev-dependency feature activation from
`codex-rs/core/Cargo.toml`
- changed `codex-rs/otel/src/config.rs` so the built-in
`OtelExporter::Statsig` default resolves to `None` when
`debug_assertions` is enabled, with a focused unit test covering that
behavior
- removed the final feature exceptions from
`.github/scripts/verify_cargo_workspace_manifests.py`, so workspace
crate features are now hard-banned instead of temporarily allowlisted
- expanded the verifier error message to explain the Bazel mismatch and
build-permutation cost behind that policy
## How tested
- `python3 .github/scripts/verify_cargo_workspace_manifests.py`
- `cargo test -p codex-otel`
- `cargo test -p codex-core
metrics_exporter_defaults_to_statsig_when_missing`
- `cargo test -p codex-app-server app_server_default_analytics_`
- `just bazel-lock-check`
## Why
`codex-core` already owns too much of the tool stack, and `AGENTS.md`
explicitly pushes us to move shared code out of `codex-core` instead of
letting it keep growing. This PR takes the next incremental step in
moving `core/src/tools` toward `codex-rs/tools` by extracting
low-coupling tool configuration and image-detail gating logic into
`codex-tools`.
That gives later extraction work a cleaner boundary to build on without
trying to move the entire tools subtree in one shot.
## What changed
- moved `ToolsConfig`, `ToolsConfigParams`, shell backend config, and
unified-exec session selection from `core/src/tools/spec.rs` into
`codex-tools`
- moved original image-detail gating and normalization into
`codex-tools`
- updated `codex-core` to consume the new `codex-tools` exports and pass
a rendered agent-type description instead of raw role config
- kept `codex-rs/tools/src/lib.rs` exports-only, with extracted unit
tests living in sibling `*_tests.rs` modules
## Testing
- `cargo test -p codex-tools`
- `cargo test -p codex-core --lib tools::spec::`
