Conversation
bolinfest
added a commit
that referenced
this pull request
Apr 2, 2026
## 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
Collaborator
Author
|
@codex review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
#16513 moved pure tool-registry planning into
codex-tools, but much of the corresponding spec/feature-gating coverage still lived incodex-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.rscoverage intocodex-rs/tools/src/tool_registry_plan_tests.rsand wire it up fromcodex-rs/tools/src/tool_registry_plan.rsusing the crate-local#[path = "tool_registry_plan_tests.rs"] mod tests;pattern.The
codex-coretest 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 thecorebinding layer.Verification
cargo test -p codex-toolscargo test -p codex-core tools::spec::tests