-
Notifications
You must be signed in to change notification settings - Fork 788
fix(codex): restore deferred tool discovery for non-Cursor routed rows #1596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -394,17 +394,21 @@ export function normalizeRoutedCatalogEntry(entry: RawEntry, parallelToolCalls = | |
| delete entry.supports_reasoning_summaries; | ||
| const isCursorEntry = typeof entry.slug === "string" && entry.slug.startsWith("cursor/"); | ||
| // `supports_search_tool` selects Codex's deferred tool-discovery surface; it is not the hosted | ||
| // web-search capability. OpenCodex can round-trip tool_search when a client sends it, but routed | ||
| // providers have no provider/model proof that Codex App plugins work through that deferred | ||
| // surface. Advertising it unconditionally hides the App's compatible direct MCP tools (#1522), | ||
| // so routed rows fail closed to direct discovery. The sidecar-backed hosted web-search metadata | ||
| // remains advertised independently for non-Cursor routes. | ||
| // web-search capability. Routed rows also carry tool_mode=code_mode_only (below), and under code | ||
| // mode DEFERRED MCP tools remain callable through exec's `tools` global / ALL_TOOLS without any | ||
| // tool_search round-trip (upstream codex-rs code_mode suite; live canary 2026-08-13: routed | ||
| // kimi/k3 called tools.mcp__node_repl__js → isError:false). Stamping false here instead forces | ||
| // every MCP declaration into exec.description — a measured 2.7x turn-1 payload regression | ||
| // (96,699 → 258,929 chars; devlog/_plan/260813_tool_catalog_deferral/010). So non-Cursor routed | ||
| // rows advertise deferred discovery; the #1522 reachability concern is covered by the code-mode | ||
| // path, not by paying the full-catalog tax. Cursor stays false: its runTurn transport bypasses | ||
| // the web-search sidecar and has no proven deferred path. | ||
| if (isCursorEntry) { | ||
| delete entry.web_search_tool_type; | ||
| } else { | ||
| entry.web_search_tool_type = "text_and_image"; | ||
| } | ||
| entry.supports_search_tool = false; | ||
| entry.supports_search_tool = !isCursorEntry; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a valid combo uses an alias such as AGENTS.md reference: src/AGENTS.md:L15-L19 Useful? React with 👍 / 👎. |
||
| // Cursor's transport already serializes overlapping tool calls into atomic Responses tool events. | ||
| // Advertising parallel calls lets Codex send the same native capability bit it sends for OpenAI. | ||
| // Opt-in providers (OcxProviderConfig.parallelToolCalls, e.g. xAI) advertise it too: the | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -149,21 +149,24 @@ the flag and thread count decide what the native runtime allows. | |||||||||||
|
|
||||||||||||
| ## Routed tool discovery and hosted search | ||||||||||||
|
|
||||||||||||
| Routed catalog rows advertise `supports_search_tool: false` by default. That field selects Codex's | ||||||||||||
| deferred tool-discovery surface; it does not describe the hosted web-search sidecar. OpenCodex still | ||||||||||||
| round-trips an explicit `tool_search` request, but it does not claim that every routed provider/model | ||||||||||||
| can discover Codex App plugins through that surface. The conservative catalog value keeps direct MCP | ||||||||||||
| tools visible in Codex App. Non-Cursor routed rows independently keep | ||||||||||||
| `web_search_tool_type: "text_and_image"` for the OpenCodex search sidecar; Cursor advertises neither | ||||||||||||
| because its transport bypasses that sidecar. | ||||||||||||
| Non-Cursor routed catalog rows advertise `supports_search_tool: true` together with | ||||||||||||
| `tool_mode: "code_mode_only"` — the pair is load-bearing. The field selects Codex's deferred | ||||||||||||
| tool-discovery surface; it does not describe the hosted web-search sidecar. Under code mode, | ||||||||||||
| deferred MCP tools remain callable through exec's `tools` global / `ALL_TOOLS` without a | ||||||||||||
| `tool_search` round-trip (upstream codex-rs code_mode suite; live canary 2026-08-13: routed | ||||||||||||
| kimi/k3 executed `tools.mcp__node_repl__js`, devlog `260813_tool_catalog_deferral/010+020`). | ||||||||||||
| Stamping `false` instead forces every MCP declaration into `exec.description` — a measured 2.7x | ||||||||||||
| turn-1 payload regression (96,699 → 258,929 chars). Non-Cursor routed rows independently keep | ||||||||||||
| `web_search_tool_type: "text_and_image"` for the OpenCodex search sidecar; Cursor advertises | ||||||||||||
| neither flag because its runTurn transport bypasses that sidecar and has no proven deferred path. | ||||||||||||
|
Comment on lines
+160
to
+161
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win State the Cursor capability value explicitly. The implementation emits Proposed wording-`web_search_tool_type: "text_and_image"` for the OpenCodex search sidecar; Cursor advertises
-neither flag because its runTurn transport bypasses that sidecar and has no proven deferred path.
+`web_search_tool_type: "text_and_image"` for the OpenCodex search sidecar; Cursor sets
+`supports_search_tool: false` and omits `web_search_tool_type` because its runTurn transport
+bypasses that sidecar and has no proven deferred path.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
|
|
||||||||||||
| [Decision Log] | ||||||||||||
| - 목적과 의도: routed models must not hide direct Codex App plugin tools behind an unverified deferred discovery capability. | ||||||||||||
| - 기존 구현 및 제약 조건: every non-Cursor row advertised `supports_search_tool: true`; the parser and bridge can still relay explicit `tool_search` calls. | ||||||||||||
| - 검토한 주요 대안: keep the blanket flag, disable both deferred discovery and hosted search, or add a future evidence-backed provider/model opt-in. | ||||||||||||
| - 선택한 방식: default routed deferred discovery to false while preserving the independent non-Cursor hosted-search metadata. | ||||||||||||
| - 다른 대안 대신 이 방식을 선택한 이유: it fixes plugin availability without removing the existing web-search sidecar or deleting runtime protocol support. | ||||||||||||
| - 장점, 단점 및 영향: direct MCP tools remain available; a routed model cannot use Codex's deferred discovery solely from generated catalog metadata until a verified opt-in exists. | ||||||||||||
| - 목적과 의도: keep routed plugin/MCP tools reachable without paying the full-catalog turn-1 payload tax. | ||||||||||||
| - 기존 구현 및 제약 조건: #1529 stamped `supports_search_tool: false` on all routed rows to fix #1522-era plugin invisibility; routed rows already carry `tool_mode: code_mode_only` (f60dd981d), and codex-rs keeps Deferred-exposure tools callable inside the exec isolate. | ||||||||||||
| - 검토한 주요 대안: keep the blanket false (2.7x payload regression), per-provider opt-in flags, or hybrid `direct_only_tool_namespaces` allowlists. | ||||||||||||
| - 선택한 방식: non-Cursor routed rows advertise deferred discovery again, paired with code-mode-only; Cursor stays opted out; a dual-seam regression test pins the pair on both the template and the template-less fallback paths. | ||||||||||||
| - 다른 대안 대신 이 방식을 선택한 이유: WP2 measurement (devlog `260813_tool_catalog_deferral/010`) showed the search=true code-mode profile is the cheapest shape (~97K vs ~259K chars turn-1), and the live canary showed reachability rides the code-mode isolate, not the tool_search round-trip — so the fail-closed flag paid the tax without buying the safety. | ||||||||||||
| - 장점, 단점 및 영향: turn-1 payload stays at the measured minimum and deferred tools stay reachable; residual risk is model compliance (a weak routed model may not use `exec` well) — the mechanism itself is client-side and model-independent. #1522's exact DeepSeek-compatible pairing remains unverified on this machine and is documented in the PR. | ||||||||||||
|
|
||||||||||||
| ## Ultra reasoning level | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new behavior is justified by
devlog/_plan/260813_tool_catalog_deferral/010, and the commit message additionally names010_wp2_measurement.mdand020_wp3_scoped_revert.md, but a fullgit ls-treesearch of this commit contains none of those paths. Consequently reviewers and future maintainers cannot inspect or reproduce the measurement that supports reverting the prior plugin-visibility fix; add the referenced investigation artifacts—under_finif this shipped change is the terminal outcome—and update these references accordingly.AGENTS.md reference: AGENTS.md:L33-L42
Useful? React with 👍 / 👎.