Commit Graph

11 Commits

  • [codex] Move tool specs into core handlers (#21416)
    ## Why
    
    This is the first mechanical slice of moving tool spec ownership toward
    the handlers. `codex-tools` should keep shared primitives and conversion
    helpers, while builtin tool specs and registration planning live in
    `codex-core` with the handlers that own those tools.
    
    Keeping this PR to relocation and import updates isolates the copy/move
    review from the later logic change that wires specs through registered
    handlers.
    
    ## What changed
    
    - Moved builtin tool spec constructors from `codex-rs/tools/src` into
    `codex-rs/core/src/tools/handlers/*_spec.rs` or nearby core tool
    modules.
    - Moved the registry planning code into
    `codex-rs/core/src/tools/spec_plan.rs` and its associated types/tests
    into core.
    - Kept shared primitives in `codex-tools`, including `ToolSpec`,
    schema/types, discovery/config primitives, dynamic/MCP conversion
    helpers, and code-mode collection helpers.
    - Updated handlers that referenced moved argument types or tool-name
    constants to use the core spec modules.
    - Moved spec tests next to the moved spec modules.
    
    ## Verification
    
    - `cargo check -p codex-tools`
    - `cargo check -p codex-core`
    - `cargo test -p codex-tools`
    - `cargo test -p codex-core _spec::tests`
    - `cargo test -p codex-core tools::spec_plan::tests`
    - `just fix -p codex-tools`
    - `just fix -p codex-core`
    
    Note: I also tried the broader `cargo test -p codex-core tools::`; it
    reached the moved spec-plan/spec tests successfully, then aborted with a
    stack overflow in
    `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`,
    which is outside this spec relocation.
  • [tool_suggest] More prompt polishes. (#20566)
    Tool suggest still misfires when model needs tool_search, updating the
    prompts to further disambiguate it:
    
    - [x] rename it from `tool_suggest` to `request_plugin_install`
    - [x] rephrase "suggestion" to "install" in the tool descriptions.
    - [x] disambiguate "the tool" vs "the plugin/connector". 
    
    Tested with the Codex App and verified it still works.
  • [tool_suggest] Improve tool_suggest triggering conditions. (#20091)
    ## Summary
    - Tighten `tool_suggest` guidance so it prefers explicit plugin install
    requests, while still allowing a connector install when the relevant
    plugin is already installed and a needed connector from that plugin is
    missing.
    - Tell the model not to call `tool_suggest` in parallel with other
    tools.
    
    ## Testing
    - `cargo test -p codex-tools tool_suggest`
    - `cargo test -p codex-core tool_suggest`
  • enable tool search over dynamic tools (#18263)
    ## Summary
    
    - Normalize deferred MCP and dynamic tools into `ToolSearchEntry` values
    before constructing `ToolSearchHandler`.
    - Move the tool-search entry adapter out of `tools/handlers` and into
    `tools/tool_search_entry.rs` so the handlers directory stays focused on
    handlers.
    - Keep `ToolSearchHandler` operating over one generic entry list for
    BM25 search, namespace grouping, and per-bucket default limits.
    
    ## Why
    
    Follow-up cleanup for #17849. The dynamic tool-search support made the
    handler juggle source-specific MCP and dynamic tool lists, index
    arithmetic, output conversion, and namespace emission. This keeps source
    adaptation outside the handler so the search loop itself is smaller and
    source-agnostic.
    
    ## Validation
    
    - `just fmt`
    - `cargo test -p codex-core tools::handlers::tool_search::tests`
    - `git diff --check`
    - `cargo test -p codex-core` currently fails in unrelated
    `plugins::manager::tests::list_marketplaces_ignores_installed_roots_missing_from_config`;
    rerunning that single test fails the same way at
    `core/src/plugins/manager_tests.rs:1692`.
    
    ---------
    
    Co-authored-by: pash <pash@openai.com>
  • Fix empty tool descriptions (#17946)
    ## Summary
    - Ensure direct namespaced MCP tool groups are emitted with a non-empty
    namespace description even when namespace metadata is missing or blank.
    - Add regression coverage for missing MCP namespace descriptions.
    
    ## Cause
    Latest `main` can serialize a direct namespaced MCP tool group with an
    empty top-level `description`. The namespace description path used
    `unwrap_or_default()` when `tool_namespaces` did not include metadata
    for that namespace, so the outbound Responses API payload could contain
    a tool like `{"type":"namespace","description":""}`. The Responses API
    rejects that because namespace tool descriptions must be a non-empty
    string.
    
    ## Fix
    - Add a fallback namespace description: `Tools in the <namespace>
    namespace.`
    - Preserve provided namespace descriptions after trimming, but treat
    blank descriptions as missing.
    
    ### Issue I am seeing
    This is what I am seeing on the local build.
    <img width="1593" height="488" alt="Screenshot 2026-04-15 at 10 55 55
    AM"
    src="https://github.com/user-attachments/assets/bab668ba-bf17-4c71-be4e-b102202fce57"
    />
    
    ---------
    
    Co-authored-by: Sayan Sisodiya <sayan@openai.com>
  • preserve search results order in tool_search_output (#17263)
    we used to alpha-sort tool search results because we were using
    `BTreeMap`, which threw away the actual search result ordering.
    
    Now we use a vec to preserve it.
    
    ### Tests
    Updated tests
  • [mcp] Expand tool search to custom MCPs. (#16944)
    - [x] Expand tool search to custom MCPs.
    - [x] Rename several variables/fields to be more generic.
    
    Updated tool & server name lifecycles:
    
    **Raw Identity**
    
    ToolInfo.server_name is raw MCP server name.
    ToolInfo.tool.name is raw MCP tool name.
    MCP calls route back to raw via parse_tool_name() returning
    (tool.server_name, tool.tool.name).
    mcpServerStatus/list now groups by raw server and keys tools by
    Tool.name: mod.rs:599
    App-server just forwards that grouped raw snapshot:
    codex_message_processor.rs:5245
    
    **Callable Names**
    
    On list-tools, we create provisional callable_namespace / callable_name:
    mcp_connection_manager.rs:1556
    For non-app MCP, provisional callable name starts as raw tool name.
    For codex-apps, provisional callable name is sanitized and strips
    connector name/id prefix; namespace includes connector name.
    Then qualify_tools() sanitizes callable namespace + name to ASCII alnum
    / _ only: mcp_tool_names.rs:128
    Note: this is stricter than Responses API. Hyphen is currently replaced
    with _ for code-mode compatibility.
    
    **Collision Handling**
    
    We do initially collapse example-server and example_server to the same
    base.
    Then qualify_tools() detects distinct raw namespace identities behind
    the same sanitized namespace and appends a hash to the callable
    namespace: mcp_tool_names.rs:137
    Same idea for tool-name collisions: hash suffix goes on callable tool
    name.
    Final list_all_tools() map key is callable_namespace + callable_name:
    mcp_connection_manager.rs:769
    
    **Direct Model Tools**
    
    Direct MCP tool declarations use the full qualified sanitized key as the
    Responses function name.
    The raw rmcp Tool is converted but renamed for model exposure.
    
    **Tool Search / Deferred**
    
    Tool search result namespace = final ToolInfo.callable_namespace:
    tool_search.rs:85
    Tool search result nested name = final ToolInfo.callable_name:
    tool_search.rs:86
    Deferred tool handler is registered as "{namespace}:{name}":
    tool_registry_plan.rs:248
    When a function call comes back, core recombines namespace + name, looks
    up the full qualified key, and gets the raw server/tool for MCP
    execution: codex.rs:4353
    
    **Separate Legacy Snapshot**
    
    collect_mcp_snapshot_from_manager_with_detail() still returns a map
    keyed by qualified callable name.
    mcpServerStatus/list no longer uses that; it uses
    McpServerStatusSnapshot, which is raw-inventory shaped.
  • Support anyOf and enum in JsonSchema (#16875)
    This brings us into better alignment with the JSON schema subset that is
    supported in
    <https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas>,
    and also allows us to render richer function signatures in code mode
    (e.g., anyOf{null, OtherObjectType})
  • 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
    ```
  • codex-tools: extract discoverable tool models (#16254)
    ## Why
    
    `#16193` moved the pure `tool_search` and `tool_suggest` spec builders
    into `codex-tools`, but `codex-core` still owned the shared
    discoverable-tool model that those builders and the `tool_suggest`
    runtime both depend on. This change continues the migration by moving
    that reusable model boundary out of `codex-core` as well, so the
    discovery/suggestion stack uses one shared set of types and
    `core/src/tools` no longer needs its own `discoverable.rs` module.
    
    ## What changed
    
    - Moved `DiscoverableTool`, `DiscoverablePluginInfo`, and
    `filter_tool_suggest_discoverable_tools_for_client()` into
    `codex-rs/tools/src/tool_discovery.rs` alongside the extracted
    discovery/suggestion spec builders.
    - Added `codex-app-server-protocol` as a `codex-tools` dependency so the
    shared discoverable-tool model can own the connector-side `AppInfo`
    variant directly.
    - Updated `core/src/tools/handlers/tool_suggest.rs`,
    `core/src/tools/spec.rs`, `core/src/tools/router.rs`,
    `core/src/connectors.rs`, and `core/src/codex.rs` to consume the shared
    `codex-tools` model instead of the old core-local declarations.
    - Changed `core/src/plugins/discoverable.rs` to return
    `DiscoverablePluginInfo` directly, moved the pure client-filter coverage
    into `tool_discovery_tests.rs`, and deleted the old
    `core/src/tools/discoverable.rs` module.
    - Updated `codex-rs/tools/README.md` so the crate boundary documents
    that `codex-tools` now owns the discoverable-tool models in addition to
    the discovery/suggestion spec builders.
    
    ## Test plan
    
    - `cargo test -p codex-tools`
    - `CARGO_TARGET_DIR=/tmp/codex-core-discoverable-model cargo test -p
    codex-core --lib tools::handlers::tool_suggest::`
    - `CARGO_TARGET_DIR=/tmp/codex-core-discoverable-model cargo test -p
    codex-core --lib tools::spec::`
    - `CARGO_TARGET_DIR=/tmp/codex-core-discoverable-model cargo test -p
    codex-core --lib plugins::discoverable::`
    - `just bazel-lock-check`
    - `just argument-comment-lint`
    
    ## References
    
    - #16193
    - #16154
    - #15923
    - #15928
    - #15944
    - #15953
    - #16031
    - #16047
    - #16129
    - #16132
    - #16138
    - #16141
  • codex-tools: extract discovery tool specs (#16193)
    ## Why
    
    `core/src/tools/spec.rs` still owned the pure `tool_search` and
    `tool_suggest` spec builders even though that logic no longer needed
    `codex-core` runtime state. This change continues the `codex-tools`
    migration by moving the reusable discovery and suggestion spec
    construction out of `codex-core` so `spec.rs` is left with the
    core-owned policy decisions about when these tools are exposed and what
    metadata is available.
    
    ## What changed
    
    - Added `codex-rs/tools/src/tool_discovery.rs` with the shared
    `tool_search` and `tool_suggest` spec builders, plus focused unit tests
    in `tool_discovery_tests.rs`.
    - Moved the shared `DiscoverableToolAction` and `DiscoverableToolType`
    declarations into `codex-tools` so the `tool_suggest` handler and the
    extracted spec builders use the same wire-model enums.
    - Updated `core/src/tools/spec.rs` to translate `ToolInfo` and
    `DiscoverableTool` values into neutral `codex-tools` inputs and delegate
    the actual spec building there.
    - Removed the old template-based description rendering helpers from
    `core/src/tools/spec.rs` and deleted the now-dead helper methods in
    `core/src/tools/discoverable.rs`.
    - Updated `codex-rs/tools/README.md` to document that discovery and
    suggestion models/spec builders now live in `codex-tools`.
    
    ## Test plan
    
    - `cargo test -p codex-tools`
    - `CARGO_TARGET_DIR=/tmp/codex-core-discovery-specs cargo test -p
    codex-core --lib tools::spec::`
    - `CARGO_TARGET_DIR=/tmp/codex-core-discovery-specs cargo test -p
    codex-core --lib tools::handlers::tool_suggest::`
    - `just argument-comment-lint`
    
    ## References
    
    - #16154
    - #15923
    - #15928
    - #15944
    - #15953
    - #16031
    - #16047
    - #16129
    - #16132
    - #16138
    - #16141