-
Notifications
You must be signed in to change notification settings - Fork 718
docs(devlog): routed tool-discovery profile plan with landing verification #1606
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
base: dev
Are you sure you want to change the base?
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 |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| # 000 - Master plan: routed tool discovery profiles | ||
|
|
||
| Status: PLAN / PATCH-DRAFT / PROTOTYPE-VERIFIED / LANDED-AND-RE-VERIFIED | ||
| Created: 2026-08-13 | ||
| Target repository: `lidge-jun/opencodex` | ||
| Target branch: `dev` | ||
| Packaging-time `dev` head: `2cdbf66a23f9fd8f2f38dcc702ccd3f2e60ac535` | ||
| Tool-discovery semantic base: `5703473041a9f4f415743652de5d86d51fd66db5` (PR #1596 parent of the packaging-time head) | ||
| Base change: PR #1596, `fix(codex): restore deferred tool discovery for non-Cursor routed rows` | ||
|
|
||
| > **Read `094_landing_verification_pass.md` before implementing anything.** This | ||
| > bundle was authored without a mounted checkout; every claim below was | ||
| > re-verified on 2026-08-13 against a real worktree, the upstream `codex-rs` | ||
| > source and live GitHub state. Eight corrections were recorded, the most | ||
| > consequential being that an **eligible** MCP tool stays callable in **both** | ||
| > discovery modes under code mode — so for those tools `direct` is a | ||
| > comprehension/compatibility lever with a payload cost, not a reachability fix. | ||
| > "Eligible" excludes `direct_only_tool_namespaces`, `excluded_tool_namespaces`, | ||
| > and anything removed by MCP/App policy filtering; see `094` for the exclusion | ||
| > table and the differential test that must prove the claim. | ||
|
|
||
| ## 1. Objective | ||
|
|
||
| The immediate objective is to preserve the good part of PR #1596—small turn-1 payloads and Code Mode access through `exec`/`tools`/`ALL_TOOLS`—while adding a narrow, evidence-driven escape hatch for exact client/provider/model combinations where deferred discovery is proven unusable. | ||
|
|
||
| This plan does **not** revert non-Cursor routed rows to blanket `supports_search_tool: false`. That would recreate the measured full-catalog payload tax. It also does **not** claim that one catalog boolean can represent every tool lifecycle. The work is split into four layers: | ||
|
|
||
| 1. **Catalog policy**: resolve whether a routed row advertises deferred discovery or direct discovery. | ||
| 2. **Protocol conformance**: prove that `additional_tools`, custom tools, namespaces and tool-search history survive each adapter. | ||
| 3. **Fallback architecture**: add bounded meta-tools for routes that cannot preserve native discovery. | ||
| 4. **Live validation**: test exact Codex App + Browser plugin + external-model combinations. | ||
|
|
||
| ## 2. Verified current state | ||
|
|
||
| At the verified `dev` head: | ||
|
|
||
| - `normalizeRoutedCatalogEntry()` lives in `src/codex/catalog/parsing.ts`. | ||
| - Every routed row receives `tool_mode = "code_mode_only"` through `applyRoutedCodexToolMode()`. | ||
| - Cursor rows receive `supports_search_tool = false` and no hosted web-search metadata. | ||
| - Every other routed row receives `supports_search_tool = true` and `web_search_tool_type = "text_and_image"`. | ||
| - The template-less fallback in `src/codex/catalog/sync.ts` reproduces the same Cursor/non-Cursor split. | ||
| - `tests/catalog-cursor-search.test.ts` pins the template and template-less paths. | ||
| - PR #1596 reports a measured request-size change of 96,699 → 258,929 characters when deferred discovery is disabled under Code Mode. | ||
| - PR #1596 also records a live canary where routed `kimi/k3` called `tools.mcp__node_repl__js` successfully. | ||
| - The exact #1522 pairing—Codex App + DeepSeek-compatible routed model + Browser plugin—remains the material evidence gap. | ||
|
|
||
| ## 3. Recommended PR stack | ||
|
|
||
| ### PR A — profile resolver and explicit escape hatch | ||
|
|
||
| Default behavior remains byte-for-byte equivalent to #1596: | ||
|
|
||
| - non-Cursor: deferred | ||
| - Cursor: direct | ||
|
|
||
| New provider-level and model-level settings allow a proven-bad route to opt into direct discovery without changing unrelated routes. | ||
|
|
||
| Suggested fields: | ||
|
|
||
| ```ts | ||
| export type OcxRoutedToolDiscoveryMode = "auto" | "deferred" | "direct"; | ||
|
|
||
| interface OcxProviderConfig { | ||
| routedToolDiscovery?: OcxRoutedToolDiscoveryMode; | ||
| modelRoutedToolDiscovery?: Record<string, OcxRoutedToolDiscoveryMode>; | ||
| } | ||
| ``` | ||
|
|
||
| Resolution precedence: | ||
|
|
||
| ```text | ||
| Cursor hard fence | ||
| > exact model override | ||
| > provider override | ||
| > auto default | ||
| ``` | ||
|
|
||
| `auto` resolves to `deferred` for non-Cursor and `direct` for Cursor. | ||
|
|
||
| ### PR B — Responses tool conformance | ||
|
|
||
| Build fixture-driven tests for: | ||
|
|
||
| - top-level `tools` | ||
| - `input[].type == "additional_tools"` | ||
| - function/custom/namespace conversion | ||
| - tool-search call/output history | ||
| - streaming and non-streaming output | ||
| - continuation, compaction and resume | ||
|
|
||
| No route may claim native discovery unless this matrix passes for its active adapter. | ||
|
|
||
| ### PR C — bounded meta-tool fallback | ||
|
|
||
| Introduce a sidecar/multiplexer surface: | ||
|
|
||
| ```text | ||
| ocx_tool_search | ||
| ocx_tool_describe | ||
| ocx_tool_call | ||
| ``` | ||
|
|
||
| This is the long-term fallback for incompatible routes. It avoids both silent tool loss and full eager schemas. | ||
|
|
||
| ### PR D — exact live E2E and rollout telemetry | ||
|
|
||
| Automate or manually certify the #1522 pairing and representative variants. Record payload size, discovery path, actual tool call, adapter, client surface and model. | ||
|
|
||
| ## 4. Files in this unit | ||
|
|
||
| ### Research and decisions | ||
|
|
||
| - `001_verified_dev_baseline.md` | ||
| - `002_incident_history_1522_1529_1596.md` | ||
| - `003_current_code_map.md` | ||
| - `004_upstream_codex_code_mode.md` | ||
| - `005_comparator_findings.md` | ||
| - `006_architecture_invariants.md` | ||
| - `007_scenario_matrix.md` | ||
| - `008_risk_register.md` | ||
| - `009_open_questions_and_evidence_gaps.md` | ||
| - `094_landing_verification_pass.md` — worktree/upstream/GitHub re-verification | ||
|
|
||
| ### Implementation roadmap | ||
|
|
||
| - `010_phase1_profile_resolver.md` | ||
| - `011_phase1_types_and_config.md` | ||
| - `012_phase1_catalog_patch.md` | ||
| - `013_phase1_sync_and_fingerprint.md` | ||
| - `014_phase1_diagnostics.md` | ||
| - `020_phase2_unit_tests.md` | ||
| - `021_catalog_test_cases.md` | ||
| - `022_config_and_precedence_tests.md` | ||
| - `023_backward_compatibility_tests.md` | ||
| - `030_phase3_protocol_conformance.md` | ||
| - `031_responses_lite_additional_tools.md` | ||
| - `032_custom_namespace_roundtrip.md` | ||
| - `033_tool_search_history_and_compaction.md` | ||
| - `040_phase4_live_e2e.md` | ||
| - `041_code_mode_all_tools_canary.md` | ||
| - `042_codex_app_deepseek_browser.md` | ||
| - `043_cursor_and_direct_bounded.md` | ||
| - `044_weak_model_meta_tool_fallback.md` | ||
| - `050_phase5_payload_cache_benchmarks.md` | ||
| - `051_benchmark_methodology.md` | ||
| - `052_acceptance_thresholds.md` | ||
| - `060_phase6_meta_tool_design.md` | ||
| - `061_meta_tool_contract.md` | ||
| - `062_meta_tool_security.md` | ||
| - `070_rollout_plan.md` | ||
| - `071_observability.md` | ||
| - `072_canary_matrix.md` | ||
| - `080_rollback_plan.md` | ||
| - `081_failure_triage_runbook.md` | ||
| - `090_final_recommendation.md` | ||
| - `091_pr_stack_and_commits.md` | ||
| - `092_definition_of_done.md` | ||
|
|
||
| ### Executable supplements | ||
|
|
||
| - `prototype/mvp-resolver.mjs` | ||
| - `prototype/profile-resolver.mjs` | ||
| - `prototype/tool-discovery-profile.test.mjs` | ||
| - `prototype/payload-benchmark.mjs` | ||
| - `patches/0001-routed-tool-discovery-profile.patch` | ||
| - `patches/0002-focused-tests.patch` | ||
| - `scripts/run_repo_validation.sh` | ||
| - `results/prototype-test-output.txt` | ||
| - `results/payload-benchmark.json` | ||
|
Comment on lines
+159
to
+169
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Do not present archival patches as executable supplements. The PR objective says the draft patches are archival and their execution instructions are withdrawn. This section still labels them executable and lists names that do not match the supplied bundle, such as 🤖 Prompt for AI Agents |
||
|
|
||
| ## 5. Decision summary | ||
|
|
||
| The first code change should be a **small resolver and override**, not the full meta-tool system. It gives operators a safe emergency lever while preserving the current default. The full fallback belongs in a separate PR because it adds a new execution surface, security boundary and search-quality problem. | ||
|
|
||
| The minimum acceptable outcome is: | ||
|
|
||
| - zero default behavior change from PR #1596; | ||
| - explicit direct override only for a named provider/model; | ||
| - exact unit coverage for precedence and fallback catalog paths; | ||
| - payload warning for direct mode; | ||
| - no claim of universal native tool-search support until adapter conformance passes. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| # 001 - Verified `dev` baseline | ||
|
|
||
| Status: VERIFIED FROM GITHUB | ||
| Branch: `dev` | ||
| Packaging-time head: `2cdbf66a23f9fd8f2f38dcc702ccd3f2e60ac535` | ||
| Packaging-time commit: PR #1600, CI timeout/watchdog scaling | ||
| Tool-discovery semantic base: `5703473041a9f4f415743652de5d86d51fd66db5`, PR #1596 | ||
|
|
||
| The packaging-time head has PR #1596 as a direct parent. Relevant source files were fetched again from `dev` after the CI-only head advance; the catalog policy seams documented below remained present. | ||
|
|
||
| ## Source locations | ||
|
|
||
| | Concern | Current location | Verified behavior | | ||
| |---|---|---| | ||
| | Routed normalization | `src/codex/catalog/parsing.ts` | `tool_mode=code_mode_only`; Cursor false, non-Cursor true for `supports_search_tool` | | ||
| | Template-less fallback | `src/codex/catalog/sync.ts` | Repeats Cursor/non-Cursor discovery split | | ||
| | Provider metadata → catalog | `src/codex/catalog/provider-fetch.ts` | `applyProviderConfigHints()` already carries per-provider/model capability hints such as context, modalities, reasoning and parallel tools | | ||
| | Provider config type | `src/types.ts` | `OcxProviderConfig` contains provider-level and model-level capability overrides; no routed discovery override yet | | ||
| | Config schema | `src/config.ts` | `providerConfigSchema` validates known provider fields and passes unknown fields through | | ||
| | Facade exports | `src/codex/catalog.ts` | Re-exports catalog parsing/provider-fetch/sync surfaces | | ||
| | Focused tests | `tests/catalog-cursor-search.test.ts` | Pins template and fallback search advertising | | ||
| | Broad catalog tests | `tests/codex-catalog.test.ts` | Pins normalized and combo rows | | ||
| | Native-parity smoke | `tests/e2e-style/phase100-native-parity.test.ts` | Expects routed DeepSeek row to advertise search | | ||
|
|
||
| ## Current normalization seam | ||
|
|
||
| The load-bearing current code is conceptually: | ||
|
|
||
| ```ts | ||
| const isCursorEntry = typeof entry.slug === "string" | ||
| && entry.slug.startsWith("cursor/"); | ||
|
|
||
| if (isCursorEntry) { | ||
| delete entry.web_search_tool_type; | ||
| } else { | ||
| entry.web_search_tool_type = "text_and_image"; | ||
| } | ||
| entry.supports_search_tool = !isCursorEntry; | ||
| ``` | ||
|
|
||
| This is not yet a capability resolver. It is a surface-name special case. | ||
|
|
||
| ## Current provider hint seam | ||
|
|
||
| `applyProviderConfigHints(name, prov, model, providerCap)` is the correct place to resolve a provider/model override because it already applies: | ||
|
|
||
| - `modelContextWindows` | ||
| - `modelInputModalities` | ||
| - `modelMaxInputTokens` | ||
| - reasoning ladders and defaults | ||
| - reasoning-summary support | ||
| - parallel-tool-call support | ||
|
|
||
| A resolved `toolDiscoveryMode` carried on `CatalogModel` follows the existing architecture instead of making `normalizeRoutedCatalogEntry()` reach back into global config. | ||
|
|
||
| ## Current gather identity seam | ||
|
|
||
| `providerCatalogFingerprint()` includes fields that influence catalog output. A discovery override must be added there so two different configs do not share a stale gather key. The newer `providerGraphIdentity` hashes the whole admitted provider row as a second fence, but the explicit fingerprint should still record the field because it is part of the catalog contract and testable identity. | ||
|
|
||
| ## Baseline test commands from the repository | ||
|
|
||
| ```bash | ||
| bun run typecheck | ||
| bun test tests/catalog-cursor-search.test.ts \ | ||
| tests/codex-catalog.test.ts \ | ||
| tests/e2e-style/phase100-native-parity.test.ts | ||
| bun run test | ||
| ``` | ||
|
|
||
| ## Baseline constraint | ||
|
|
||
| This bundle was built without a mounted full repository checkout. The executable tests included here validate the resolver and synthetic payload model independently. The repository commands above are the required integration gate when applied in the actual OpenCodex worktree. | ||
|
|
||
| **Superseded on 2026-08-13:** that gate has now been run. See | ||
| `094_landing_verification_pass.md` for the re-verification of every claim above | ||
| against a real worktree, the upstream `codex-rs` source, and live GitHub state. | ||
| Eight corrections were recorded there; the ones touching this document are: | ||
| `CatalogModel` lives in `src/codex/catalog/parsing.ts`, not `src/types.ts`, and | ||
| `supports_search_tool` is gated upstream by a second conjunct | ||
| (`namespace_tools_enabled`), so it is not the sole switch for deferred discovery. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # 002 - Incident history: #1522 → #1529 → #1596 | ||
|
|
||
| ## Timeline | ||
|
|
||
| ### Issue #1522 | ||
|
|
||
| Reported behavior: | ||
|
|
||
| - Codex App | ||
| - OpenAI-compatible third-party provider | ||
| - DeepSeek-compatible routed model | ||
| - Browser plugin enabled | ||
| - `supports_search_tool=true` | ||
| - Browser deferred tools such as `mcp__node_repl__js` did not appear to the model | ||
|
|
||
| The reporter changed only the catalog flag to false and observed direct MCP tools become visible and callable. | ||
|
|
||
| This proved a real user-visible failure, but it did **not** prove that all non-Cursor routed models require direct discovery. It proved one exact client/provider/model/plugin combination was broken. | ||
|
|
||
| ### PR #1529 | ||
|
|
||
| Response: | ||
|
|
||
| - set `supports_search_tool=false` for every routed row; | ||
| - kept hosted web-search metadata separate for non-Cursor rows; | ||
| - updated template and template-less catalog paths; | ||
| - added regression expectations for direct discovery. | ||
|
|
||
| Benefit: | ||
|
|
||
| - restored the reporter's direct MCP surface. | ||
|
|
||
| Cost discovered later: | ||
|
|
||
| - under `code_mode_only`, disabling deferred discovery causes Codex to include every MCP declaration in `exec.description`; | ||
| - request size was measured at approximately 96,699 versus 258,929 characters; | ||
| - the fix applied the cost globally to solve one unverified scope. | ||
|
|
||
| ### PR #1596 | ||
|
|
||
| Response: | ||
|
|
||
| - restored `supports_search_tool=true` for non-Cursor routed rows; | ||
| - kept Cursor false; | ||
| - pinned `code_mode_only + supports_search_tool=true` as a pair; | ||
| - measured payload size; | ||
| - ran a live Kimi Code Mode canary through `tools.mcp__node_repl__js`; | ||
| - added template and template-less regression tests. | ||
|
|
||
| Residual risk explicitly recorded: | ||
|
|
||
| - the exact #1522 DeepSeek-compatible Browser pairing was not reproduced on the development machine; | ||
| - the canary proved the Code Mode mechanism, not every model's compliance or every Codex App plugin lifecycle. | ||
|
|
||
| ## Root lesson | ||
|
|
||
| The mistake was not “true” versus “false.” The mistake was treating a route-dependent capability as a global Boolean default with no escape hatch and no evidence record. | ||
|
|
||
| ## Required correction | ||
|
|
||
| Keep #1596 as the default. Add: | ||
|
|
||
| 1. a provider/model override for exact proven failures; | ||
| 2. a diagnostic explaining the resolved mode and reason; | ||
| 3. payload warnings when direct mode is selected; | ||
| 4. an E2E evidence matrix that can later drive automatic profile selection. | ||
|
|
||
| ## Source links | ||
|
|
||
| - Issue #1522: https://github.com/lidge-jun/opencodex/issues/1522 | ||
| - PR #1529: https://github.com/lidge-jun/opencodex/pull/1529 | ||
| - PR #1596: https://github.com/lidge-jun/opencodex/pull/1596 |
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.
Anyone following the master plan cannot locate any of these five artifacts: a repo-wide filename search finds only these references. The committed files are instead
0001-add-tool-discovery-module.patch,0002-focused-test-plan.patch,run-repo-validation.sh,prototype-test.txt, and the two size-specificpayload-benchmark-{250,1000}.jsonfiles, so update this inventory to point to the actual executable evidence.Useful? React with 👍 / 👎.