From c1d655debf00de7cbc08dff43ffc496012ce7a20 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:00:55 +0900 Subject: [PATCH 01/13] feat(codex): add routed tool-discovery compatibility profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the hard-coded Cursor/non-Cursor boolean behind routed `supports_search_tool` with a resolved, route-scoped policy. Defaults are unchanged: with no configuration, non-Cursor routed rows stay deferred (`supports_search_tool: true` + `web_search_tool_type: "text_and_image"`) and Cursor stays direct, exactly as PR #1596 shipped. Every pre-existing catalog test passes unmodified, which is the proof that nothing moved. New config, both optional: routedToolDiscovery?: "auto" | "deferred" | "direct" modelRoutedToolDiscovery?: Record Precedence is Cursor hard fence > model override > provider override > auto. `auto` resolves to deferred for non-Cursor rows and never reaches serialization: `CatalogModel.toolDiscoveryMode` carries only the resolved value, resolved once in applyProviderConfigHints() so configured, live-discovered, cached and combo-derived rows agree. Combos resolve conservatively (one direct member forces direct) because a single public row cannot vary after target selection, and the field is emitted only when it departs from the default. Both the explicit fingerprint and the provider-graph identity see the new fields, so two different policies cannot share a stale gather. Closes the unresolved P2 from #1596 rather than reproducing it: both catalog construction paths now share one fence, isCursorRoute(), which decides on provider identity and falls back to the `cursor/` slug prefix only for callers with no CatalogModel. Previously parsing.ts tested the slug while sync.ts tested the provider, so a `cursor/`-aliased combo whose canonical provider is `combo` was classified differently depending on whether a template happened to exist. Config admission mirrors the house pattern: the load path degrades a malformed value with `.catch(undefined)` so a typo cannot cost a user their providers or credentials, while the write boundary rejects it outright with a path-specific `schema_invalid: providers..: ...`. The validator reads only own data properties via Object.getOwnPropertyDescriptor, so an accessor- or prototype-polluted candidate is rejected without ever invoking the getter. What `direct` is not: under code mode Codex installs nested MCP tools on the `tools`/`ALL_TOOLS` globals in BOTH exposures, so for an eligible tool `direct` buys no reachability — it moves full schemas into `exec.description` at the measured 2.7x turn-1 cost and changes `tool_search` construction. It cannot repair a tool removed by `direct_only_tool_namespaces`, `excluded_tool_namespaces`, or MCP/App policy filtering. It is a comprehension and compatibility lever, documented as such. Also corrects structure/03: Cursor emits `supports_search_tool: false` and omits `web_search_tool_type`; it does not "advertise neither flag" (CodeRabbit's unresolved note on #1596). Plan, evidence and upstream citations: devlog/_plan/260813_routed_tool_discovery_profiles. Verification: bun x tsc --noEmit clean; bun test on the 7 affected suites = 392 pass / 0 fail (25 new cases covering the precedence matrix, Cursor fence, combo derivation, propagation isolation and config admission); privacy:scan passed. Full suite runs on the Linux CI host. --- .../docs/reference/configuration/providers.md | 2 + src/codex/catalog/aggregation.ts | 9 + src/codex/catalog/parsing.ts | 37 ++- src/codex/catalog/provider-fetch.ts | 7 +- src/codex/catalog/sync.ts | 20 +- src/codex/catalog/tool-discovery.ts | 136 ++++++++++ src/config.ts | 72 ++++++ src/types.ts | 24 ++ structure/03_catalog-and-subagents.md | 28 +- tests/codex-tool-discovery-mode.test.ts | 240 ++++++++++++++++++ 10 files changed, 566 insertions(+), 9 deletions(-) create mode 100644 src/codex/catalog/tool-discovery.ts create mode 100644 tests/codex-tool-discovery-mode.test.ts diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 62203e285..145361a9d 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -105,6 +105,8 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `noPenaltyModels?` | `string[]` | Models that reject presence/frequency penalties. | | `noStructuredOutputModels?` | `string[]` | Exact model IDs whose `openai-chat` endpoint rejects `response_format`. Only an exact requested-model match omits the field; structured-output translation stays enabled for every other `openai-chat` model. | | `parallelToolCalls?` | `boolean` | Toggle parallel tool calls. OpenAI Chat defaults on; non-chat adapters advertise only on explicit `true`. | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | Routed-row Codex tool-discovery policy. `auto` (default) keeps the shipped behavior: deferred for non-Cursor routed rows, direct for Cursor, which is hard-fenced and ignores a configured `deferred`. Set `direct` only for a route proven incompatible with deferred discovery — it embeds every MCP declaration in the first request (a measured 2.7x turn-1 payload cost) and does **not** make an otherwise eligible tool reachable under code mode, where Codex installs nested tools on the `tools`/`ALL_TOOLS` globals either way. Independent of hosted web search (`web_search_tool_type`). | +| `modelRoutedToolDiscovery?` | `Record` | Per-model override of `routedToolDiscovery`, so one incompatible model on a mixed gateway does not penalize its siblings. Matching follows the usual model-key rules (exact id, family before `:`, case-insensitive); dated `-YYYYMMDD` variants are not matched, so name the exact failing model id. | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | Disabled-by-default downstream SSE repair for exact placeholder ids, missing terminal ids, and (with `repairInvalidIds`) message/reasoning ids missing the canonical `msg_`/`rs_` prefix. Function-call ids are never rewritten. Built-in DeepSeek enables the last two by default. | | `responsesSnapshotRepair?` | `boolean` | Disabled-by-default client-facing repair for sparse Responses lifecycle snapshots in SSE and JSON. Fills missing canonical status, output, and tool metadata while raw inspection and persistence remain unchanged. | | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key providers only (`authMode: "key"`). Opt-in same-target 429 retry: when `retryOn429` is absent the feature is off; object presence enables it unless `enabled: false`. On 429 the proxy waits (upstream `Retry-After` or the fixed interval) and replays the identical request on the same key before any key failover — across the main text-turn recovery loop, the Responses passthrough wire, the image/video bridge, the web-search sidecar, and terminal continuations. Only pre-stream HTTP 429 responses are eligible for replay; custom `runTurn` transports are outside the HTTP retry loop. `attempts` counts same-key replays after the first 429 (total sends = `attempts` + 1) and is one request-wide budget shared by the main recovery loop, the terminal-guard continuation, and bridge retries. Exhausting `attempts` only stops further same-key replays: normal key failover or final-error handling then applies per the available targets — on the key-auth passthrough wire there is no failover, so the exhausted 429 surfaces as-is. Codex itself never retries 429, so this is the only defense for single-key providers. Defaults: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000` (any single wait is capped at `maxIntervalMs`, itself capped at 600000), `respectRetryAfter: true`. | diff --git a/src/codex/catalog/aggregation.ts b/src/codex/catalog/aggregation.ts index 1031f073a..4b8be37d6 100644 --- a/src/codex/catalog/aggregation.ts +++ b/src/codex/catalog/aggregation.ts @@ -34,6 +34,7 @@ import upstreamModelsSnapshot from "../data/upstream-models.json"; import { catalogModelSlug } from "./parsing"; import type { CatalogModel } from "./parsing"; +import { deriveComboToolDiscoveryMode } from "./tool-discovery"; export const openAiApiCollisionWarnings = new Set(); @@ -173,6 +174,13 @@ export function deriveComboCatalogModel( ...(members.every(member => member.parallelToolCalls === true) ? { parallelToolCalls: true } : {}), + // One public combo row cannot change capabilities after a target is selected, so a + // single direct-only member forces the whole combo direct rather than stranding it. + // Emitted only when it departs from the default, matching parallelToolCalls and the + // summary flag: an all-deferred combo stays byte-identical to the pre-override shape. + ...(deriveComboToolDiscoveryMode(members.map(member => member.toolDiscoveryMode)) === "direct" + ? { toolDiscoveryMode: "direct" as const } + : {}), ...(members.some(member => member.supportsReasoningSummaries === false) ? { supportsReasoningSummaries: false } : {}), }; } @@ -201,6 +209,7 @@ export function comboCatalogWarningSignature( inputModalities: [...new Set(member?.inputModalities ?? [])].sort(), reasoningEfforts: [...new Set(member?.reasoningEfforts ?? [])].sort(), parallelToolCalls: member?.parallelToolCalls === true, + toolDiscoveryMode: member?.toolDiscoveryMode ?? "deferred", supportsReasoningSummaries: member?.supportsReasoningSummaries !== false, }; }).sort((a, b) => a.key.localeCompare(b.key))); diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index 442d1378d..8adc203a9 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -34,6 +34,7 @@ import upstreamModelsSnapshot from "../data/upstream-models.json"; import { NATIVE_OPENAI_CONTEXT_OVERRIDES, SUPPORTED_NATIVE_OPENAI_SLUGS, UPSTREAM_NATIVE_ENTRIES, nativeMultiAgentVersion } from "./metadata"; import { trustedAccountBoundNativeCatalogSlug } from "./account-models"; import { CODEX_NATIVE_ALIAS_CATALOG_KIND } from "./kinds"; +import { isCursorRoute, type ResolvedRoutedToolDiscoveryMode } from "./tool-discovery"; export function legacyCatalogBackupPath(): string { return join(getConfigDir(), "catalog-backup.json"); @@ -116,6 +117,13 @@ export interface CatalogModel { inputModalities?: string[]; /** Provider opted into parallel tool calls (OcxProviderConfig.parallelToolCalls). */ parallelToolCalls?: boolean; + /** + * Resolved routed tool-discovery policy (OcxProviderConfig.routedToolDiscovery and its + * per-model map). Only ever `deferred` or `direct` — `auto` is resolved before it + * reaches a catalog row. Carried here so `normalizeRoutedCatalogEntry` never reaches + * back into global config, matching how parallelToolCalls and the modality hints flow. + */ + toolDiscoveryMode?: ResolvedRoutedToolDiscoveryMode; /** Whether Codex may send Responses text.verbosity for this routed model. */ supportsVerbosity?: boolean; supportsReasoningSummaries?: boolean; @@ -378,7 +386,23 @@ export function applyMultiAgentMode(entries: RawEntry[], mode: MultiAgentMode, v return entries; } -export function normalizeRoutedCatalogEntry(entry: RawEntry, parallelToolCalls = false): RawEntry { +/** + * Route-scoped policy inputs. An options object rather than a third positional argument: + * the Cursor fence needs the provider identity as well as the mode, and the two existing + * positions stay put for the public callers (src/codex/catalog.ts re-export, tests). + */ +export interface RoutedCatalogEntryOptions { + /** Resolved discovery mode for this row. Defaults to `deferred` (the #1596 default). */ + toolDiscoveryMode?: ResolvedRoutedToolDiscoveryMode; + /** Canonical provider id from the CatalogModel, when the caller has one. */ + providerId?: string; +} + +export function normalizeRoutedCatalogEntry( + entry: RawEntry, + parallelToolCalls = false, + options: RoutedCatalogEntryOptions = {}, +): RawEntry { delete entry.model_messages; delete entry.tool_mode; applyRoutedCodexToolMode(entry); @@ -392,7 +416,9 @@ export function normalizeRoutedCatalogEntry(entry: RawEntry, parallelToolCalls = // Routed rows cloned from native templates must not inherit OpenAI-only summary delivery. // Per-model routed opt-ins can be added once provider metadata exposes this capability. delete entry.supports_reasoning_summaries; - const isCursorEntry = typeof entry.slug === "string" && entry.slug.startsWith("cursor/"); + // Provider identity first, slug prefix only as a fallback — one shared fence for both the + // template and template-less paths (see tool-discovery.ts isCursorRoute). + const isCursorEntry = isCursorRoute(entry.slug, options.providerId); // `supports_search_tool` selects Codex's deferred tool-discovery surface; it is not the hosted // 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 @@ -408,7 +434,12 @@ export function normalizeRoutedCatalogEntry(entry: RawEntry, parallelToolCalls = } else { entry.web_search_tool_type = "text_and_image"; } - entry.supports_search_tool = !isCursorEntry; + // Cursor is hard-fenced to direct; every other routed row follows its resolved mode, + // which defaults to deferred so an unconfigured tree is byte-identical to #1596. + const effectiveMode: ResolvedRoutedToolDiscoveryMode = isCursorEntry + ? "direct" + : options.toolDiscoveryMode ?? "deferred"; + entry.supports_search_tool = effectiveMode === "deferred"; // 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 diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index a723fe087..130fadcb3 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -69,6 +69,7 @@ import { createAdmissionGate, ResourceAdmissionError, type AdmissionMetrics } fr import { CODEX_CUSTOM_MODEL_CATALOG_KIND, JAWCODE_CATALOG_AUGMENT_PROVIDERS, catalogModelSlug, shouldExposeRoutedModel } from "./parsing"; import type { CatalogModel } from "./parsing"; +import { resolveConfiguredRoutedToolDiscoveryMode } from "./tool-discovery"; import { disabledNativeSlugs, hasComboTargets, nativeDefaultReasoningEffort, nativeInputModalities, nativeOpenAiContextWindow, nativeOpenAiSlugs, nativeParallelToolCalls, nativeReasoningEfforts } from "./metadata"; import { deriveComboCatalogModel, normalizedOpenAiApiSignature, openAiApiCollisionWarnings, replaceLastComboCatalogOmissions, warnUncataloguedComboOnce } from "./aggregation"; import type { ComboCatalogOmission } from "./aggregation"; @@ -554,6 +555,8 @@ function providerCatalogFingerprint(name: string, prov: OcxProviderConfig): Reco rsDel: prov.modelReasoningSummaryDelivery ?? null, noVis: [...(prov.noVisionModels ?? [])].sort(), ptc: prov.parallelToolCalls ?? null, + rtd: prov.routedToolDiscovery ?? null, + mrtd: prov.modelRoutedToolDiscovery ?? null, gMode: prov.googleMode ?? null, }; } @@ -608,7 +611,6 @@ function configuredReasoningSummarySupport(prov: OcxProviderConfig | undefined, } export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, model: CatalogModel, providerCap?: number): CatalogModel { - void name; const configuredCap = configuredContextWindow(prov, model.id); const configuredMaxInput = configuredMaxInputTokens(prov, model.id); let inputModalities = configuredInputModalities(prov, model.id); @@ -649,6 +651,9 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, ...(prov.parallelToolCalls === true || (prov.adapter === "openai-chat" && prov.parallelToolCalls !== false) ? { parallelToolCalls: true } : {}), + // Resolve routed discovery here so configured, live-discovered, cached and combo-derived + // rows all carry the same value and `auto` never reaches serialization. + toolDiscoveryMode: resolveConfiguredRoutedToolDiscoveryMode(name, prov, model.id).mode, }; const capped = applyProviderContextCap(hinted.contextWindow, providerCap); if (providerCap !== undefined && capped !== hinted.contextWindow) { diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 5afa06c10..63c99239c 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -34,6 +34,7 @@ import { codexAccountNamespaceEntries, isMainCodexAccountTarget } from "../accou import { CODEX_CUSTOM_MODEL_CATALOG_KIND, CODEX_PROVIDER_MODEL_CATALOG_KIND, activeCodexModelsCachePath, applyCatalogMetadata, applyMultiAgentMode, applyNativeOpenAiContextOverride, applyRoutedCodexToolMode, catalogBackupPathFor, catalogHasRoutedEntries, catalogModelSlug, ensureStrictCatalogFields, findNativeTemplate, isDefaultCatalogPath, isRoutedModelCompatibilityExcluded, legacyCatalogBackupPath, normalizeRoutedCatalogEntry, normalizeServiceTiers, readCatalog, readCatalogBackup, readCodexCatalogPath, readNativeBaseline } from "./parsing"; +import { isCursorRoute } from "./tool-discovery"; import type { CatalogModel, MultiAgentMode, RawCatalog, RawEntry } from "./parsing"; import { accountBoundNativeOpenAiSlugs, accountBoundNativeOpenAiSlugsBySelector, applyNativeVisibility, CODEX_NATIVE_ALIAS_CATALOG_KIND, desktopAllowlistSuppressedNativeSlugs, disabledNativeSlugs, isNativeAliasCatalogEntry, isUnsupportedOpenAiNativeSlug, NATIVE_OPENAI_MODELS, observedAccountBoundNativeEntries, shouldIncludeAccountBoundNativeOpenAi, shouldIncludeNativeOpenAi, shouldUpgradeToUpstreamEntry, SUPPORTED_NATIVE_OPENAI_SLUGS, upstreamNativeEntry } from "./metadata"; import { @@ -284,7 +285,10 @@ export function deriveEntry( e.base_instructions = identifyRoutedModel(e.base_instructions, modelName); } applyReasoningLevels(e, model?.reasoningEfforts, model?.defaultReasoningEffort, preserveExact); - normalizeRoutedCatalogEntry(e, model?.parallelToolCalls === true); + normalizeRoutedCatalogEntry(e, model?.parallelToolCalls === true, { + toolDiscoveryMode: model?.toolDiscoveryMode, + providerId: model?.provider, + }); if (model) applyCatalogMetadata(e, model.provider, model.id, model.contextCap); applyCatalogModelMetadata(e, model); if (model?.catalogKind) e.opencodex_catalog_kind = model.catalogKind; @@ -314,7 +318,14 @@ export function deriveEntry( // web-search metadata (runTurn transport bypasses the sidecar). Non-Cursor routed fallbacks // advertise deferred discovery — code mode keeps deferred MCP callable (devlog // 260813_tool_catalog_deferral/010+020); search=false costs a measured 2.7x turn-1 payload. - const isCursorFallback = isRouted && model?.provider === "cursor"; + // Same fence helper as the template path, so a `cursor/`-aliased combo whose canonical + // provider is `combo` cannot be classified one way here and another way there. + const isCursorFallback = isRouted && isCursorRoute(slug, model?.provider); + // Cursor is hard-fenced to direct; every other routed fallback follows its resolved mode, + // defaulting to deferred so an unconfigured tree stays byte-identical to #1596. + const fallbackDiscoveryMode = isCursorFallback + ? "direct" + : model?.toolDiscoveryMode ?? "deferred"; const entry: RawEntry = { slug, display_name: routedDisplayName(slug), description: desc, shell_type: "shell_command", visibility: "list", supported_in_api: true, @@ -322,7 +333,10 @@ export function deriveEntry( ...(isRouted ? isCursorFallback ? { supports_search_tool: false } - : { web_search_tool_type: "text_and_image", supports_search_tool: true } + : { + web_search_tool_type: "text_and_image", + supports_search_tool: fallbackDiscoveryMode === "deferred", + } : {}), }; if (isRouted) { diff --git a/src/codex/catalog/tool-discovery.ts b/src/codex/catalog/tool-discovery.ts new file mode 100644 index 000000000..b3fc0da94 --- /dev/null +++ b/src/codex/catalog/tool-discovery.ts @@ -0,0 +1,136 @@ +import type { OcxProviderConfig, OcxRoutedToolDiscoveryMode } from "../../types"; +import { modelRecordValue } from "../../reasoning-effort"; + +/** + * Route-scoped Codex tool-discovery policy. + * + * `supports_search_tool` selects Codex's DEFERRED tool-discovery surface. It is not the + * hosted web-search capability (`web_search_tool_type`), and under `tool_mode = + * code_mode_only` it does not decide whether an eligible MCP tool is reachable: upstream + * installs nested tool specs on the code-mode `tools`/`ALL_TOOLS` globals in BOTH + * exposures (codex-rs `spec_plan.rs` build_code_mode_executors → `code-mode/src/runtime/ + * globals.rs`). What changes is where the schemas live — direct exposure embeds every MCP + * declaration in `exec.description`, the measured 96,699 → 258,929 char turn-1 regression + * behind PR #1596 — plus `tool_search` construction and the deferred-guidance text. + * + * So `direct` is a compatibility/comprehension lever with a payload cost, NOT a + * reachability fix. It also cannot repair a tool removed by `direct_only_tool_namespaces`, + * `excluded_tool_namespaces`, or MCP/App policy filtering, all of which are independent of + * this flag. Full analysis and citations: + * devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md. + */ +export type ResolvedRoutedToolDiscoveryMode = "deferred" | "direct"; + +export const ROUTED_TOOL_DISCOVERY_MODES: readonly OcxRoutedToolDiscoveryMode[] = [ + "auto", + "deferred", + "direct", +] as const; + +export function isRoutedToolDiscoveryMode(value: unknown): value is OcxRoutedToolDiscoveryMode { + return value === "auto" || value === "deferred" || value === "direct"; +} + +/** Why a row resolved the way it did. Diagnostics only — never serialized into the catalog. */ +export type RoutedToolDiscoverySource = + | "cursor-hard-fence" + | "model-override" + | "provider-override" + | "default"; + +export interface ResolvedRoutedToolDiscovery { + readonly mode: ResolvedRoutedToolDiscoveryMode; + readonly source: RoutedToolDiscoverySource; + /** The configured value before resolution; `auto` never reaches serialization. */ + readonly configured: OcxRoutedToolDiscoveryMode; + readonly reason: string; + readonly warning?: string; +} + +export const CURSOR_PROVIDER_ID = "cursor"; +const CURSOR_SLUG_PREFIX = `${CURSOR_PROVIDER_ID}/`; + +/** + * The single Cursor fence, shared by the template and template-less catalog paths. + * + * Provider identity wins when a CatalogModel is available; the public slug prefix is only + * a fallback for callers that have none. Before this helper the two paths disagreed — + * `parsing.ts` tested `slug.startsWith("cursor/")` while `sync.ts` tested + * `model?.provider === "cursor"` — so a `cursor/`-aliased combo whose canonical provider is + * `combo` was classified differently depending on whether a template happened to exist, + * making discovery mode and payload size depend on template availability (unresolved P2 on + * PR #1596). + */ +export function isCursorRoute(slug: unknown, providerId?: string): boolean { + if (providerId !== undefined) return providerId === CURSOR_PROVIDER_ID; + return typeof slug === "string" && slug.startsWith(CURSOR_SLUG_PREFIX); +} + +/** + * Resolve one routed provider/model to a catalog-facing mode. + * + * Precedence: Cursor hard fence > exact model override > provider override > auto default. + * `auto` resolves to `deferred` for non-Cursor rows, preserving PR #1596 byte-for-byte. + * + * Model keys follow `modelRecordValue()` semantics — exact id, the family before a `:`, or + * a case-insensitive full-id match. Dated `-YYYYMMDD` variants are NOT matched; name the + * exact model id that failed. + */ +export function resolveConfiguredRoutedToolDiscoveryMode( + providerName: string, + provider: Pick, + modelId: string, +): ResolvedRoutedToolDiscovery { + const modelConfigured = modelRecordValue(provider.modelRoutedToolDiscovery, modelId); + const providerConfigured = provider.routedToolDiscovery; + const configured: OcxRoutedToolDiscoveryMode = modelConfigured ?? providerConfigured ?? "auto"; + const source: RoutedToolDiscoverySource = modelConfigured !== undefined + ? "model-override" + : providerConfigured !== undefined + ? "provider-override" + : "default"; + + // Cursor's runTurn transport bypasses the web-search sidecar and has no proven deferred + // path, so the fence outranks any configuration rather than silently accepting it. + if (providerName === CURSOR_PROVIDER_ID || provider.adapter === CURSOR_PROVIDER_ID) { + return { + mode: "direct", + source: "cursor-hard-fence", + configured, + reason: "Cursor's runTurn transport has no verified deferred discovery path.", + ...(configured === "deferred" + ? { warning: "Configured deferred discovery was ignored for Cursor." } + : {}), + }; + } + + if (configured === "direct") { + return { + mode: "direct", + source, + configured, + reason: "A route-scoped compatibility override selected direct discovery.", + warning: "Direct discovery embeds full MCP declarations in the first request.", + }; + } + + return { + mode: "deferred", + source, + configured, + reason: configured === "deferred" + ? "The route explicitly selected Codex deferred discovery." + : "Non-Cursor auto mode preserves the Code Mode default.", + }; +} + +/** + * One public combo row cannot change capabilities after a target is selected, so a single + * direct-only member forces the whole combo direct. Advertising deferred discovery when a + * possible target was explicitly marked incompatible would strand that target. + */ +export function deriveComboToolDiscoveryMode( + memberModes: readonly (ResolvedRoutedToolDiscoveryMode | undefined)[], +): ResolvedRoutedToolDiscoveryMode { + return memberModes.some(mode => mode === "direct") ? "direct" : "deferred"; +} diff --git a/src/config.ts b/src/config.ts index ef3580e1a..e9507e4a1 100644 --- a/src/config.ts +++ b/src/config.ts @@ -28,6 +28,9 @@ import { MAIN_CODEX_ACCOUNT_NAMESPACE_TARGET, } from "./codex/account-namespace-match"; import { isCodexAccountPriorityKey } from "./codex/account-priority"; +// Safe import direction: tool-discovery.ts depends only on ./types and ./reasoning-effort, +// never on this module, so the policy predicate is shared rather than duplicated here. +import { isRoutedToolDiscoveryMode } from "./codex/catalog/tool-discovery"; import { UPSTREAM_HOST_CIRCUIT_MAX_THRESHOLD } from "./codex/upstream-host-health"; import { adoptCustomModelCatalogMigration, @@ -613,6 +616,9 @@ const retryOn429PolicySchema = z.object({ respectRetryAfter: z.boolean().optional(), }).strict(); +/** Routed-row Codex tool-discovery policy; `auto` keeps the shipped per-surface default. */ +const routedToolDiscoveryModeSchema = z.enum(["auto", "deferred", "direct"]); + /** * Zod schema for one provider entry: known fields are validated strictly while unknown * fields pass through (preserved for runtime extensions). @@ -642,6 +648,10 @@ const providerConfigSchema = z.object({ repairInvalidIds: z.boolean().optional(), }).strict().optional(), responsesSnapshotRepair: z.boolean().optional(), + // Hand-edited configs degrade instead of losing the whole provider (credentials, ports). + // The write boundary rejects the same values outright — see routedToolDiscoveryError(). + routedToolDiscovery: routedToolDiscoveryModeSchema.optional().catch(undefined), + modelRoutedToolDiscovery: z.record(z.string().min(1), routedToolDiscoveryModeSchema).optional().catch(undefined), }).passthrough(); const RESERVED_PROVIDER_NAMES = new Set([ @@ -2239,6 +2249,67 @@ function googleAntigravityStaticCatalogVersionError(value: unknown): string | nu return "schema_invalid: googleAntigravityStaticCatalogVersion: must be 1, 2, or omitted"; } +const ROUTED_TOOL_DISCOVERY_EXPECTED = "must be auto, deferred, or direct"; + +/** + * Read an own DATA property without triggering a getter. + * + * The write boundary must reject accessor- and prototype-polluted candidates BEFORE it + * reads them: a validator that touches `candidate.providers[x].modelRoutedToolDiscovery` + * and only then checks for getters has already run attacker-supplied code. Returns + * `undefined` for inherited keys and for anything that is not a plain data property. + */ +function ownDataProperty(target: object, key: string): unknown { + const descriptor = Object.getOwnPropertyDescriptor(target, key); + if (!descriptor || !("value" in descriptor)) return undefined; + return descriptor.value; +} + +/** + * Reject malformed routed tool-discovery policy at the write boundary. + * + * The load path degrades these fields with `.catch(undefined)` so a typo cannot cost a user + * their providers or credentials. That same tolerance would silently discard a deliberate + * write, so live writes must fail loudly instead — the pattern already used by + * `activeCodexAccountPinned`. + */ +function routedToolDiscoveryError(value: unknown): string | null { + const raw = rawConfigRecord(value); + if (!raw) return null; + const providers = ownDataProperty(raw, "providers"); + if (!providers || typeof providers !== "object" || Array.isArray(providers)) return null; + + for (const name of Object.getOwnPropertyNames(providers)) { + const provider = ownDataProperty(providers, name); + if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; + + const mode = ownDataProperty(provider, "routedToolDiscovery"); + if (mode !== undefined && !isRoutedToolDiscoveryMode(mode)) { + return `schema_invalid: providers.${name}.routedToolDiscovery: ${ROUTED_TOOL_DISCOVERY_EXPECTED}`; + } + + if (!Object.hasOwn(provider, "modelRoutedToolDiscovery")) continue; + const map = ownDataProperty(provider, "modelRoutedToolDiscovery"); + if (map === undefined) { + // Present but not a plain data property: an accessor or prototype-sourced value. + return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: must be a plain object of model overrides`; + } + if (typeof map !== "object" || map === null || Array.isArray(map)) { + return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: must be a plain object of model overrides`; + } + for (const modelId of Object.getOwnPropertyNames(map)) { + if (modelId.trim() === "") { + return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: model keys must be nonblank`; + } + const modelMode = ownDataProperty(map, modelId); + if (!isRoutedToolDiscoveryMode(modelMode)) { + return `schema_invalid: providers.${name}.modelRoutedToolDiscovery.${modelId}: ${ROUTED_TOOL_DISCOVERY_EXPECTED}`; + } + } + } + return null; +} + function codexAccountPickerEnabledError(value: unknown): string | null { const raw = rawConfigRecord(value); if (!raw) return null; @@ -2304,6 +2375,7 @@ export function validateConfigCandidate(value: unknown): { ok: true; config: Ocx ?? googleAntigravityStaticCatalogVersionError(value) ?? codexAccountPrioritiesError(value) ?? codexAccountPickerEnabledError(value) + ?? routedToolDiscoveryError(value) ?? loopbackListenerPortError(value); if (boundaryError) return { ok: false, error: boundaryError }; const result = configSchema.safeParse(value); diff --git a/src/types.ts b/src/types.ts index d24811f43..3622712e8 100644 --- a/src/types.ts +++ b/src/types.ts @@ -940,6 +940,13 @@ export interface OcxConfig { export type OcxAccountPoolRotationStrategy = "quota" | "round-robin" | "fill-first"; +/** + * Routed-row Codex tool-discovery policy. `auto` resolves to `deferred` for non-Cursor + * routed rows and `direct` for Cursor; the resolved catalog value is only ever + * `deferred` or `direct`. + */ +export type OcxRoutedToolDiscoveryMode = "auto" | "deferred" | "direct"; + export type OcxComboStrategy = "failover" | "round-robin"; export type OcxComboDefaultEffort = "low" | "medium" | "high" | "xhigh" | "max" | "ultra"; @@ -1450,6 +1457,23 @@ export interface OcxProviderConfig { * only on explicit `true`. See devlog/_plan/260709_parallel_tool_calls. */ parallelToolCalls?: boolean; + /** + * Codex routed-row tool-discovery policy. `auto` (default) keeps the shipped behavior: + * deferred for non-Cursor routed rows, direct for Cursor. Set `direct` only for a route + * PROVEN incompatible with deferred discovery — it embeds every MCP declaration in the + * first request (a measured 2.7x turn-1 payload cost) and does NOT make an otherwise + * eligible tool reachable under code mode, where upstream installs nested tools on the + * `tools`/`ALL_TOOLS` globals either way. See + * devlog/_plan/260813_routed_tool_discovery_profiles. + */ + routedToolDiscovery?: OcxRoutedToolDiscoveryMode; + /** + * Per-model routed tool-discovery override; wins over the provider-level value so one + * incompatible model on a mixed gateway does not penalize its siblings. Keys follow + * `modelRecordValue()` matching (exact id, family before `:`, case-insensitive full id); + * dated `-YYYYMMDD` variants are NOT matched, so name the exact failing model id. + */ + modelRoutedToolDiscovery?: Record; /** * Opt-in: forward `prompt_cache_key` to the upstream `/chat/completions` body. * OpenAI-specific extension; strict backends (Groq, Cerebras, etc.) reject unknown diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index 54bfaaf1c..25bfbc9a8 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -157,8 +157,32 @@ deferred MCP tools remain callable through exec's `tools` global / `ALL_TOOLS` w 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. +`web_search_tool_type: "text_and_image"` for the OpenCodex search sidecar. Cursor rows emit +`supports_search_tool: false` and omit `web_search_tool_type`, because its runTurn transport +bypasses that sidecar and has no proven deferred path — the row carries the discovery flag as +`false` rather than omitting it. + +That per-surface default is now the `auto` case of a resolved policy rather than a hard-coded +boolean. `OcxProviderConfig.routedToolDiscovery` and its per-model map +`modelRoutedToolDiscovery` let one proven-incompatible route opt into `direct` without moving +any sibling; `auto` reproduces the shipped shape byte-for-byte. Resolution happens once in +`applyProviderConfigHints()` and rides `CatalogModel.toolDiscoveryMode`, so configured, +live-discovered, cached and combo-derived rows agree, and `auto` never reaches serialization. +Precedence is Cursor hard fence > model override > provider override > auto; a combo resolves +conservatively, since one public row cannot vary after target selection. + +Both catalog construction paths share one Cursor fence (`isCursorRoute()`): provider identity +decides, and the `cursor/` slug prefix is only a fallback for callers with no `CatalogModel`. +Before that helper the template path tested the slug while the template-less path tested the +provider, so a `cursor/`-aliased combo whose canonical provider is `combo` was classified +differently depending on whether a template existed — discovery mode and payload size varied +with template availability (unresolved P2 on #1596). + +Scope of the reachability claim: it holds for an ELIGIBLE MCP tool. `direct_only_tool_namespaces`, +`excluded_tool_namespaces`, and MCP/App policy filtering remove tools independently of this flag, +and no override repairs them. So `direct` is a comprehension/compatibility lever with a payload +cost, not a reachability fix. Analysis and citations: +`devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md`. [Decision Log] - 목적과 의도: keep routed plugin/MCP tools reachable without paying the full-catalog turn-1 payload tax. diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts new file mode 100644 index 000000000..21949c4de --- /dev/null +++ b/tests/codex-tool-discovery-mode.test.ts @@ -0,0 +1,240 @@ +import { describe, expect, it } from "bun:test"; +import { applyProviderConfigHints, normalizeRoutedCatalogEntry } from "../src/codex/catalog"; +import { deriveComboCatalogModel } from "../src/codex/catalog/aggregation"; +import type { CatalogModel } from "../src/codex/catalog/parsing"; +import { + deriveComboToolDiscoveryMode, + isCursorRoute, + resolveConfiguredRoutedToolDiscoveryMode, +} from "../src/codex/catalog/tool-discovery"; +import { validateConfigCandidate } from "../src/config"; +import type { OcxProviderConfig } from "../src/types"; + +function provider(extra: Partial = {}): OcxProviderConfig { + return { adapter: "openai-responses", baseUrl: "https://example.invalid", ...extra } as OcxProviderConfig; +} + +function model(id: string, providerName: string): CatalogModel { + return { id, provider: providerName }; +} + +describe("routed tool-discovery resolution", () => { + // The precedence table from devlog/_plan/260813_routed_tool_discovery_profiles/011. + const cases: Array<{ + label: string; + prov: OcxProviderConfig; + expected: "deferred" | "direct"; + source: string; + }> = [ + { label: "unset provider, unset model", prov: provider(), expected: "deferred", source: "default" }, + { label: "provider auto", prov: provider({ routedToolDiscovery: "auto" }), expected: "deferred", source: "provider-override" }, + { label: "provider direct", prov: provider({ routedToolDiscovery: "direct" }), expected: "direct", source: "provider-override" }, + { + label: "provider direct, model deferred", + prov: provider({ routedToolDiscovery: "direct", modelRoutedToolDiscovery: { "glm-5.2": "deferred" } }), + expected: "deferred", + source: "model-override", + }, + { + label: "provider deferred, model direct", + prov: provider({ routedToolDiscovery: "deferred", modelRoutedToolDiscovery: { "glm-5.2": "direct" } }), + expected: "direct", + source: "model-override", + }, + { + label: "provider deferred, model auto", + prov: provider({ routedToolDiscovery: "deferred", modelRoutedToolDiscovery: { "glm-5.2": "auto" } }), + expected: "deferred", + source: "model-override", + }, + ]; + + for (const testCase of cases) { + it(`resolves ${testCase.label} to ${testCase.expected}`, () => { + const resolved = resolveConfiguredRoutedToolDiscoveryMode("deepseek", testCase.prov, "glm-5.2"); + expect(resolved.mode).toBe(testCase.expected); + expect(resolved.source).toBe(testCase.source as never); + }); + } + + it("keeps Cursor direct even when explicitly configured deferred, and says so", () => { + const resolved = resolveConfiguredRoutedToolDiscoveryMode( + "cursor", + provider({ adapter: "cursor", routedToolDiscovery: "deferred" }), + "gpt-5.5", + ); + expect(resolved.mode).toBe("direct"); + expect(resolved.source).toBe("cursor-hard-fence"); + // Silent acceptance is not allowed (INV-4): the ignored configuration must surface. + expect(resolved.warning).toContain("ignored for Cursor"); + }); + + it("fences Cursor by adapter even when the provider is named differently", () => { + const resolved = resolveConfiguredRoutedToolDiscoveryMode("my-cursor-gateway", provider({ adapter: "cursor" }), "gpt-5.5"); + expect(resolved.mode).toBe("direct"); + expect(resolved.source).toBe("cursor-hard-fence"); + }); + + it("warns when a non-Cursor route opts into direct discovery", () => { + const resolved = resolveConfiguredRoutedToolDiscoveryMode("deepseek", provider({ routedToolDiscovery: "direct" }), "glm-5.2"); + expect(resolved.warning).toContain("full MCP declarations"); + }); + + it("matches model keys with modelRecordValue semantics but not dated variants", () => { + const prov = provider({ modelRoutedToolDiscovery: { "glm-5.2": "direct" } }); + expect(resolveConfiguredRoutedToolDiscoveryMode("deepseek", prov, "glm-5.2").mode).toBe("direct"); + expect(resolveConfiguredRoutedToolDiscoveryMode("deepseek", prov, "GLM-5.2").mode).toBe("direct"); + expect(resolveConfiguredRoutedToolDiscoveryMode("deepseek", prov, "glm-5.2:free").mode).toBe("direct"); + // Dated variants are deliberately NOT matched — name the exact failing model id. + expect(resolveConfiguredRoutedToolDiscoveryMode("deepseek", prov, "glm-5.2-20260813").mode).toBe("deferred"); + }); +}); + +describe("routed tool-discovery catalog emission", () => { + it("defaults to the PR #1596 shape with no configuration", () => { + const entry = normalizeRoutedCatalogEntry({ slug: "deepseek/glm-5.2" } as never) as Record; + expect(entry.supports_search_tool).toBe(true); + expect(entry.web_search_tool_type).toBe("text_and_image"); + expect(entry.tool_mode).toBe("code_mode_only"); + }); + + it("emits direct discovery when the resolved mode says so", () => { + const entry = normalizeRoutedCatalogEntry({ slug: "deepseek/glm-5.2" } as never, false, { + toolDiscoveryMode: "direct", + providerId: "deepseek", + }) as Record; + expect(entry.supports_search_tool).toBe(false); + // INV-3: hosted web search is a separate capability and must survive direct mode. + expect(entry.web_search_tool_type).toBe("text_and_image"); + expect(entry.tool_mode).toBe("code_mode_only"); + }); + + it("ignores a deferred mode for Cursor rows", () => { + const entry = normalizeRoutedCatalogEntry({ slug: "cursor/gpt-5.5" } as never, false, { + toolDiscoveryMode: "deferred", + providerId: "cursor", + }) as Record; + expect(entry.supports_search_tool).toBe(false); + expect(entry.web_search_tool_type).toBeUndefined(); + }); + + it("classifies a cursor/-aliased combo by provider identity, not by its public slug", () => { + // The unresolved #1596 P2: the template path used to fence on the slug while the + // template-less path fenced on provider identity, so this row's discovery mode depended + // on whether a template happened to exist. Both paths now share isCursorRoute(). + expect(isCursorRoute("cursor/gpt-5.5", "combo")).toBe(false); + expect(isCursorRoute("cursor/gpt-5.5", "cursor")).toBe(true); + // No CatalogModel available: the slug prefix remains the only signal. + expect(isCursorRoute("cursor/gpt-5.5")).toBe(true); + expect(isCursorRoute("deepseek/glm-5.2")).toBe(false); + + const entry = normalizeRoutedCatalogEntry({ slug: "cursor/gpt-5.5" } as never, false, { + toolDiscoveryMode: "deferred", + providerId: "combo", + }) as Record; + expect(entry.supports_search_tool).toBe(true); + expect(entry.web_search_tool_type).toBe("text_and_image"); + }); +}); + +describe("routed tool-discovery propagation", () => { + it("carries the resolved mode onto the CatalogModel", () => { + const hinted = applyProviderConfigHints("deepseek", provider({ routedToolDiscovery: "direct" }), model("glm-5.2", "deepseek")); + expect(hinted.toolDiscoveryMode).toBe("direct"); + }); + + it("leaves an unconfigured provider on deferred", () => { + const hinted = applyProviderConfigHints("deepseek", provider(), model("glm-5.2", "deepseek")); + expect(hinted.toolDiscoveryMode).toBe("deferred"); + }); + + it("changes only the targeted model on a mixed gateway", () => { + const prov = provider({ modelRoutedToolDiscovery: { "glm-5.2": "direct" } }); + expect(applyProviderConfigHints("gateway", prov, model("glm-5.2", "gateway")).toolDiscoveryMode).toBe("direct"); + expect(applyProviderConfigHints("gateway", prov, model("kimi-k3", "gateway")).toolDiscoveryMode).toBe("deferred"); + }); +}); + +describe("combo tool-discovery derivation", () => { + it("is conservative: one direct member forces the combo direct", () => { + expect(deriveComboToolDiscoveryMode(["deferred", "deferred"])).toBe("deferred"); + expect(deriveComboToolDiscoveryMode(["deferred", "direct"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode([undefined, undefined])).toBe("deferred"); + expect(deriveComboToolDiscoveryMode([])).toBe("deferred"); + }); + + it("propagates the conservative mode through deriveComboCatalogModel", () => { + const members: CatalogModel[] = [ + { id: "a", provider: "p1", contextWindow: 200_000, toolDiscoveryMode: "deferred" }, + { id: "b", provider: "p2", contextWindow: 200_000, toolDiscoveryMode: "direct" }, + ]; + const combo = deriveComboCatalogModel( + "combo/mix", + { targets: [{ provider: "p1", model: "a" }, { provider: "p2", model: "b" }], strategy: "failover" } as never, + members, + ); + expect(combo?.toolDiscoveryMode).toBe("direct"); + }); +}); + +describe("routed tool-discovery config admission", () => { + // defaultProvider must resolve, otherwise every candidate fails on an unrelated rule. + function candidate(providerPatch: Record): Record { + return { + defaultProvider: "deepseek", + providers: { deepseek: { adapter: "openai-responses", baseUrl: "https://example.invalid", ...providerPatch } }, + }; + } + + it("accepts every valid mode", () => { + for (const mode of ["auto", "deferred", "direct"]) { + expect(validateConfigCandidate(candidate({ routedToolDiscovery: mode })).ok).toBe(true); + expect(validateConfigCandidate(candidate({ modelRoutedToolDiscovery: { "glm-5.2": mode } })).ok).toBe(true); + } + }); + + it("rejects an unknown provider-level mode with a path-specific error", () => { + const result = validateConfigCandidate(candidate({ routedToolDiscovery: "eager" })); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.error).toBe("schema_invalid: providers.deepseek.routedToolDiscovery: must be auto, deferred, or direct"); + }); + + it("rejects an unknown model-level mode naming the model", () => { + const result = validateConfigCandidate(candidate({ modelRoutedToolDiscovery: { "glm-5.2": "eager" } })); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.error).toBe("schema_invalid: providers.deepseek.modelRoutedToolDiscovery.glm-5.2: must be auto, deferred, or direct"); + }); + + it("rejects a non-object model map and a blank model key", () => { + expect(validateConfigCandidate(candidate({ modelRoutedToolDiscovery: "direct" })).ok).toBe(false); + expect(validateConfigCandidate(candidate({ modelRoutedToolDiscovery: ["direct"] })).ok).toBe(false); + expect(validateConfigCandidate(candidate({ modelRoutedToolDiscovery: { " ": "direct" } })).ok).toBe(false); + }); + + it("rejects an accessor-backed model map without invoking the getter", () => { + let getterCalls = 0; + const provider: Record = { adapter: "openai-responses", baseUrl: "https://example.invalid" }; + Object.defineProperty(provider, "modelRoutedToolDiscovery", { + enumerable: true, + configurable: true, + get() { + getterCalls += 1; + return { "glm-5.2": "direct" }; + }, + }); + const result = validateConfigCandidate({ defaultProvider: "deepseek", providers: { deepseek: provider } }); + expect(result.ok).toBe(false); + // The descriptor check must precede the read, so the getter never runs. + expect(getterCalls).toBe(0); + }); + + it("ignores a prototype-sourced value rather than trusting it", () => { + const polluted = Object.create({ routedToolDiscovery: "eager" }) as Record; + polluted.adapter = "openai-responses"; + polluted.baseUrl = "https://example.invalid"; + // Inherited, so it is not an own data property and must not be read as configuration. + expect(validateConfigCandidate({ defaultProvider: "deepseek", providers: { deepseek: polluted } }).ok).toBe(true); + }); +}); From b8cb797af666c6c0dff37ca2516a6e74ccfab13b Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:16:02 +0900 Subject: [PATCH 02/13] fix(codex): close review findings on routed tool-discovery profiles An independent code review of c1d655deb returned FAIL on four blockers, each reproduced before fixing. All four are closed. 1. Default preservation for `cursor/`-aliased combos. Reconciling the two historical fences by preferring provider identity UNFENCED a real row: a combo aliased `cursor/gpt-5.5` with canonical provider `combo` went from `supports_search_tool: false` (template path, fenced on the slug) to `true`. isCursorRoute() now UNIONS the available signals instead of ranking them. Reconciliation can only ever add fencing, never remove it, because an under-fenced Cursor row advertises a deferred surface its runTurn transport cannot serve, while over-fencing costs only payload. The test that codified the wrong default now pins the original one. 2. Getter-safe write boundary. ownDataProperty() collapsed "absent" and "accessor" into undefined, so an accessor-backed `routedToolDiscovery` looked like a missing field: the validator passed and Zod invoked the getter moments later, accepting "direct". It now returns a tagged absent/accessor/data result and rejects accessors on `providers`, on each provider entry, on both policy fields and on every model-map entry, before `configSchema.safeParse()`. 3. Propagation completeness. Custom rows (`customModels`) and trusted `openai-apikey` rows are rebuilt without passing through applyProviderConfigHints, so a provider-level `direct` silently degraded to deferred for exactly the models an operator hand-declared. Both now resolve the policy explicitly, and custom rows inherit `toolDiscoveryMode`/ `cursorRoute` from the provider-derived row they replace, matching how the other capability fields already inherit. 4. Fence agreement between resolution and serialization. The resolver fences on provider name OR adapter; serialization saw only the provider id, so a Cursor-adapter gateway under a custom provider name was hard-fenced to direct yet still emitted `web_search_tool_type` and missed Cursor's parallel-tool advertisement. The resolver now stamps `CatalogModel.cursorRoute` where both signals are known, and serialization honors it. Test gap from the same review: the new suite exercised normalizeRoutedCatalogEntry directly and never drove sync.ts's template-less branch. Added three buildCatalogEntries(null, ...) cases covering fallback direct, fallback default and the fallback Cursor fence, plus accessor cases for the provider entry and the providers map. Verification: bun x tsc --noEmit clean; bun test across the 8 affected suites = 399 pass / 0 fail (32 in the focused file, up from 25); bun run privacy:scan passed; all four blockers re-run and confirmed closed, including a gathered custom model with a provider-level override now resolving to direct. --- src/codex/catalog/parsing.ts | 17 +++- src/codex/catalog/provider-fetch.ts | 19 +++- src/codex/catalog/sync.ts | 10 +- src/codex/catalog/tool-discovery.ts | 41 ++++++-- src/config.ts | 53 +++++++--- tests/codex-tool-discovery-mode.test.ts | 122 ++++++++++++++++++++++-- 6 files changed, 222 insertions(+), 40 deletions(-) diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index 8adc203a9..00c2c8768 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -124,6 +124,12 @@ export interface CatalogModel { * back into global config, matching how parallelToolCalls and the modality hints flow. */ toolDiscoveryMode?: ResolvedRoutedToolDiscoveryMode; + /** + * Cursor fence resolved where the provider name AND adapter were both known + * (applyProviderConfigHints). Serialization cannot re-derive this: it sees only the + * provider id, so a Cursor-adapter gateway under a custom provider name would escape. + */ + cursorRoute?: boolean; /** Whether Codex may send Responses text.verbosity for this routed model. */ supportsVerbosity?: boolean; supportsReasoningSummaries?: boolean; @@ -396,6 +402,8 @@ export interface RoutedCatalogEntryOptions { toolDiscoveryMode?: ResolvedRoutedToolDiscoveryMode; /** Canonical provider id from the CatalogModel, when the caller has one. */ providerId?: string; + /** Fence resolved upstream from provider name + adapter (CatalogModel.cursorRoute). */ + cursorRoute?: boolean; } export function normalizeRoutedCatalogEntry( @@ -416,9 +424,12 @@ export function normalizeRoutedCatalogEntry( // Routed rows cloned from native templates must not inherit OpenAI-only summary delivery. // Per-model routed opt-ins can be added once provider metadata exposes this capability. delete entry.supports_reasoning_summaries; - // Provider identity first, slug prefix only as a fallback — one shared fence for both the - // template and template-less paths (see tool-discovery.ts isCursorRoute). - const isCursorEntry = isCursorRoute(entry.slug, options.providerId); + // One shared fence for both construction paths, unioning every available signal so that + // reconciling the two historical checks can never UNFENCE a row (see isCursorRoute). + const isCursorEntry = isCursorRoute(entry.slug, { + providerId: options.providerId, + cursorRoute: options.cursorRoute, + }); // `supports_search_tool` selects Codex's deferred tool-discovery surface; it is not the hosted // 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 diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 130fadcb3..d4b3cdf3c 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -69,7 +69,7 @@ import { createAdmissionGate, ResourceAdmissionError, type AdmissionMetrics } fr import { CODEX_CUSTOM_MODEL_CATALOG_KIND, JAWCODE_CATALOG_AUGMENT_PROVIDERS, catalogModelSlug, shouldExposeRoutedModel } from "./parsing"; import type { CatalogModel } from "./parsing"; -import { resolveConfiguredRoutedToolDiscoveryMode } from "./tool-discovery"; +import { isCursorProviderIdentity, resolveConfiguredRoutedToolDiscoveryMode } from "./tool-discovery"; import { disabledNativeSlugs, hasComboTargets, nativeDefaultReasoningEffort, nativeInputModalities, nativeOpenAiContextWindow, nativeOpenAiSlugs, nativeParallelToolCalls, nativeReasoningEfforts } from "./metadata"; import { deriveComboCatalogModel, normalizedOpenAiApiSignature, openAiApiCollisionWarnings, replaceLastComboCatalogOmissions, warnUncataloguedComboOnce } from "./aggregation"; import type { ComboCatalogOmission } from "./aggregation"; @@ -654,6 +654,9 @@ export function applyProviderConfigHints(name: string, prov: OcxProviderConfig, // Resolve routed discovery here so configured, live-discovered, cached and combo-derived // rows all carry the same value and `auto` never reaches serialization. toolDiscoveryMode: resolveConfiguredRoutedToolDiscoveryMode(name, prov, model.id).mode, + // Resolve the fence here too: this is the only place that sees BOTH the provider name and + // its adapter, so a Cursor-adapter gateway under a custom name cannot escape at emit time. + ...(isCursorProviderIdentity(name, prov.adapter) ? { cursorRoute: true } : {}), }; const capped = applyProviderContextCap(hinted.contextWindow, providerCap); if (providerCap !== undefined && capped !== hinted.contextWindow) { @@ -1729,6 +1732,15 @@ async function gatherRoutedModelsUncached( ...(cm.contextWindow ? { contextWindow: cm.contextWindow } : {}), ...(cm.inputModalities ? { inputModalities: cm.inputModalities } : {}), ...(typeof supportsReasoningSummaries === "boolean" ? { supportsReasoningSummaries } : {}), + // Custom rows never pass through applyProviderConfigHints, so resolve the routed + // discovery policy here too — otherwise a provider-level `direct` silently degrades to + // deferred for exactly the models an operator hand-declared. + ...(rawProvider + ? { + toolDiscoveryMode: resolveConfiguredRoutedToolDiscoveryMode(cm.provider, rawProvider, cm.modelId).mode, + ...(isCursorProviderIdentity(cm.provider, rawProvider.adapter) ? { cursorRoute: true } : {}), + } + : {}), }; // #962: the dedupe below drops the provider-derived row this custom row replaces. Inherit that // row's provider capability metadata (reasoning ladder, default effort, parallel tool calls, @@ -1748,6 +1760,8 @@ async function gatherRoutedModelsUncached( ...(base.supportsVerbosity === undefined && replaced.supportsVerbosity !== undefined ? { supportsVerbosity: replaced.supportsVerbosity } : {}), ...(base.supportsReasoningSummaries === undefined && replaced.supportsReasoningSummaries !== undefined ? { supportsReasoningSummaries: replaced.supportsReasoningSummaries } : {}), ...(base.capabilities === undefined && replaced.capabilities !== undefined ? { capabilities: replaced.capabilities } : {}), + ...(base.toolDiscoveryMode === undefined && replaced.toolDiscoveryMode !== undefined ? { toolDiscoveryMode: replaced.toolDiscoveryMode } : {}), + ...(base.cursorRoute === undefined && replaced.cursorRoute !== undefined ? { cursorRoute: replaced.cursorRoute } : {}), } : base; // Vision-sidecar coverage ONLY: if the custom model is in the enriched provider's // noVisionModels, advertise image input so the Codex app lets images reach the sidecar @@ -1827,6 +1841,9 @@ function augmentRoutedModelsWithCapturedOpenAiApiRows( ...(maxInputTokens ? { maxInputTokens } : {}), ...(policy.modelInputModalities?.[id] ? { inputModalities: [...policy.modelInputModalities[id]!] } : {}), ...(policy.modelReasoningEfforts?.[id] ? { reasoningEfforts: [...policy.modelReasoningEfforts[id]!] } : {}), + // Rebuilt from the trusted snapshot rather than through applyProviderConfigHints, so the + // routed discovery policy has to be resolved here as well. + toolDiscoveryMode: resolveConfiguredRoutedToolDiscoveryMode(OPENAI_API_PROVIDER_ID, configured, id).mode, }; }); diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 63c99239c..8fbb73b96 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -288,6 +288,7 @@ export function deriveEntry( normalizeRoutedCatalogEntry(e, model?.parallelToolCalls === true, { toolDiscoveryMode: model?.toolDiscoveryMode, providerId: model?.provider, + cursorRoute: model?.cursorRoute, }); if (model) applyCatalogMetadata(e, model.provider, model.id, model.contextCap); applyCatalogModelMetadata(e, model); @@ -318,9 +319,12 @@ export function deriveEntry( // web-search metadata (runTurn transport bypasses the sidecar). Non-Cursor routed fallbacks // advertise deferred discovery — code mode keeps deferred MCP callable (devlog // 260813_tool_catalog_deferral/010+020); search=false costs a measured 2.7x turn-1 payload. - // Same fence helper as the template path, so a `cursor/`-aliased combo whose canonical - // provider is `combo` cannot be classified one way here and another way there. - const isCursorFallback = isRouted && isCursorRoute(slug, model?.provider); + // Same fence helper as the template path, so a row cannot be classified one way here and + // another way there. The union includes the slug, which this path previously ignored. + const isCursorFallback = isRouted && isCursorRoute(slug, { + providerId: model?.provider, + cursorRoute: model?.cursorRoute, + }); // Cursor is hard-fenced to direct; every other routed fallback follows its resolved mode, // defaulting to deferred so an unconfigured tree stays byte-identical to #1596. const fallbackDiscoveryMode = isCursorFallback diff --git a/src/codex/catalog/tool-discovery.ts b/src/codex/catalog/tool-discovery.ts index b3fc0da94..5bf643a88 100644 --- a/src/codex/catalog/tool-discovery.ts +++ b/src/codex/catalog/tool-discovery.ts @@ -50,19 +50,40 @@ export interface ResolvedRoutedToolDiscovery { export const CURSOR_PROVIDER_ID = "cursor"; const CURSOR_SLUG_PREFIX = `${CURSOR_PROVIDER_ID}/`; +/** + * Cursor identity as the RESOLVER sees it: provider name or adapter. A gateway can be named + * anything while speaking Cursor's custom runTurn transport, and it is the transport — not + * the label — that has no deferred path. + */ +export function isCursorProviderIdentity(providerName: string | undefined, adapter: string | undefined): boolean { + return providerName === CURSOR_PROVIDER_ID || adapter === CURSOR_PROVIDER_ID; +} + +export interface CursorRouteSignals { + /** Canonical provider id from the CatalogModel, when the caller has one. */ + providerId?: string; + /** Fence resolved upstream where the provider name AND adapter were both known. */ + cursorRoute?: boolean; +} + /** * The single Cursor fence, shared by the template and template-less catalog paths. * - * Provider identity wins when a CatalogModel is available; the public slug prefix is only - * a fallback for callers that have none. Before this helper the two paths disagreed — - * `parsing.ts` tested `slug.startsWith("cursor/")` while `sync.ts` tested - * `model?.provider === "cursor"` — so a `cursor/`-aliased combo whose canonical provider is - * `combo` was classified differently depending on whether a template happened to exist, - * making discovery mode and payload size depend on template availability (unresolved P2 on - * PR #1596). + * This is a UNION of every available signal, deliberately, because the two construction + * paths historically disagreed: `parsing.ts` tested `slug.startsWith("cursor/")` while + * `sync.ts` tested `model?.provider === "cursor"`, so a `cursor/`-aliased combo whose + * canonical provider is `combo` was classified differently depending on whether a template + * happened to exist (unresolved P2 on PR #1596). Any single-signal reconciliation would + * silently UNFENCE rows that one path used to fence, and an under-fenced Cursor row + * advertises a deferred surface its transport cannot serve. Over-fencing only costs + * payload, so the union is the safe direction. + * + * `cursorRoute` carries the resolver's verdict (provider name or adapter), which is the only + * signal that catches a Cursor-adapter gateway under a custom provider name. */ -export function isCursorRoute(slug: unknown, providerId?: string): boolean { - if (providerId !== undefined) return providerId === CURSOR_PROVIDER_ID; +export function isCursorRoute(slug: unknown, signals: CursorRouteSignals = {}): boolean { + if (signals.cursorRoute === true) return true; + if (signals.providerId === CURSOR_PROVIDER_ID) return true; return typeof slug === "string" && slug.startsWith(CURSOR_SLUG_PREFIX); } @@ -92,7 +113,7 @@ export function resolveConfiguredRoutedToolDiscoveryMode( // Cursor's runTurn transport bypasses the web-search sidecar and has no proven deferred // path, so the fence outranks any configuration rather than silently accepting it. - if (providerName === CURSOR_PROVIDER_ID || provider.adapter === CURSOR_PROVIDER_ID) { + if (isCursorProviderIdentity(providerName, provider.adapter)) { return { mode: "direct", source: "cursor-hard-fence", diff --git a/src/config.ts b/src/config.ts index e9507e4a1..454a27ac5 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2256,13 +2256,22 @@ const ROUTED_TOOL_DISCOVERY_EXPECTED = "must be auto, deferred, or direct"; * * The write boundary must reject accessor- and prototype-polluted candidates BEFORE it * reads them: a validator that touches `candidate.providers[x].modelRoutedToolDiscovery` - * and only then checks for getters has already run attacker-supplied code. Returns - * `undefined` for inherited keys and for anything that is not a plain data property. + * and only then checks for getters has already run attacker-supplied code. + * + * `absent` and `accessor` are distinguished deliberately. Collapsing both to `undefined` + * makes a getter-backed field look like a missing field, so the validator waves it through + * and Zod invokes the getter moments later — the exact bypass this helper exists to stop. */ -function ownDataProperty(target: object, key: string): unknown { +type OwnDataProperty = + | { readonly kind: "absent" } + | { readonly kind: "accessor" } + | { readonly kind: "data"; readonly value: unknown }; + +function ownDataProperty(target: object, key: string): OwnDataProperty { const descriptor = Object.getOwnPropertyDescriptor(target, key); - if (!descriptor || !("value" in descriptor)) return undefined; - return descriptor.value; + if (!descriptor) return { kind: "absent" }; + if (!("value" in descriptor)) return { kind: "accessor" }; + return { kind: "data", value: descriptor.value }; } /** @@ -2276,24 +2285,38 @@ function ownDataProperty(target: object, key: string): unknown { function routedToolDiscoveryError(value: unknown): string | null { const raw = rawConfigRecord(value); if (!raw) return null; - const providers = ownDataProperty(raw, "providers"); + const providersProperty = ownDataProperty(raw, "providers"); + if (providersProperty.kind === "accessor") { + return "schema_invalid: providers: must be a plain object"; + } + const providers = providersProperty.kind === "data" ? providersProperty.value : undefined; if (!providers || typeof providers !== "object" || Array.isArray(providers)) return null; for (const name of Object.getOwnPropertyNames(providers)) { - const provider = ownDataProperty(providers, name); + const providerProperty = ownDataProperty(providers, name); + if (providerProperty.kind === "accessor") { + return `schema_invalid: providers.${name}: must be a plain object`; + } + const provider = providerProperty.kind === "data" ? providerProperty.value : undefined; if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; - const mode = ownDataProperty(provider, "routedToolDiscovery"); - if (mode !== undefined && !isRoutedToolDiscoveryMode(mode)) { + const modeProperty = ownDataProperty(provider, "routedToolDiscovery"); + if (modeProperty.kind === "accessor") { + return `schema_invalid: providers.${name}.routedToolDiscovery: ${ROUTED_TOOL_DISCOVERY_EXPECTED}`; + } + if (modeProperty.kind === "data" + && modeProperty.value !== undefined + && !isRoutedToolDiscoveryMode(modeProperty.value)) { return `schema_invalid: providers.${name}.routedToolDiscovery: ${ROUTED_TOOL_DISCOVERY_EXPECTED}`; } - if (!Object.hasOwn(provider, "modelRoutedToolDiscovery")) continue; - const map = ownDataProperty(provider, "modelRoutedToolDiscovery"); - if (map === undefined) { - // Present but not a plain data property: an accessor or prototype-sourced value. + const mapProperty = ownDataProperty(provider, "modelRoutedToolDiscovery"); + if (mapProperty.kind === "accessor") { return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: must be a plain object of model overrides`; } + if (mapProperty.kind === "absent") continue; + const map = mapProperty.value; + if (map === undefined) continue; if (typeof map !== "object" || map === null || Array.isArray(map)) { return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: must be a plain object of model overrides`; } @@ -2301,8 +2324,8 @@ function routedToolDiscoveryError(value: unknown): string | null { if (modelId.trim() === "") { return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: model keys must be nonblank`; } - const modelMode = ownDataProperty(map, modelId); - if (!isRoutedToolDiscoveryMode(modelMode)) { + const modelModeProperty = ownDataProperty(map, modelId); + if (modelModeProperty.kind !== "data" || !isRoutedToolDiscoveryMode(modelModeProperty.value)) { return `schema_invalid: providers.${name}.modelRoutedToolDiscovery.${modelId}: ${ROUTED_TOOL_DISCOVERY_EXPECTED}`; } } diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index 21949c4de..c70d5d348 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { applyProviderConfigHints, normalizeRoutedCatalogEntry } from "../src/codex/catalog"; +import { applyProviderConfigHints, buildCatalogEntries, normalizeRoutedCatalogEntry } from "../src/codex/catalog"; import { deriveComboCatalogModel } from "../src/codex/catalog/aggregation"; import type { CatalogModel } from "../src/codex/catalog/parsing"; import { @@ -119,25 +119,85 @@ describe("routed tool-discovery catalog emission", () => { }); it("classifies a cursor/-aliased combo by provider identity, not by its public slug", () => { - // The unresolved #1596 P2: the template path used to fence on the slug while the - // template-less path fenced on provider identity, so this row's discovery mode depended - // on whether a template happened to exist. Both paths now share isCursorRoute(). - expect(isCursorRoute("cursor/gpt-5.5", "combo")).toBe(false); - expect(isCursorRoute("cursor/gpt-5.5", "cursor")).toBe(true); + // The unresolved #1596 P2: the template path fenced on the slug while the template-less + // path fenced on provider identity, so this row's discovery mode depended on whether a + // template happened to exist. Both paths now share isCursorRoute(), which UNIONS the + // signals — reconciling them must never unfence a row that one path used to fence, since + // an under-fenced Cursor row advertises a surface its transport cannot serve. + expect(isCursorRoute("cursor/gpt-5.5", { providerId: "combo" })).toBe(true); + expect(isCursorRoute("cursor/gpt-5.5", { providerId: "cursor" })).toBe(true); + expect(isCursorRoute("combo/mix", { cursorRoute: true })).toBe(true); // No CatalogModel available: the slug prefix remains the only signal. expect(isCursorRoute("cursor/gpt-5.5")).toBe(true); expect(isCursorRoute("deepseek/glm-5.2")).toBe(false); + expect(isCursorRoute("deepseek/glm-5.2", { providerId: "deepseek" })).toBe(false); + // Byte-identical to the pre-change template path for this row. const entry = normalizeRoutedCatalogEntry({ slug: "cursor/gpt-5.5" } as never, false, { toolDiscoveryMode: "deferred", providerId: "combo", }) as Record; - expect(entry.supports_search_tool).toBe(true); - expect(entry.web_search_tool_type).toBe("text_and_image"); + expect(entry.supports_search_tool).toBe(false); + expect(entry.web_search_tool_type).toBeUndefined(); + }); + + it("fences a Cursor-adapter gateway published under a custom provider name", () => { + // Only the resolver sees the adapter, so it stamps CatalogModel.cursorRoute and + // serialization honors it. Without that hop the row would keep hosted-search metadata + // while the resolver had already hard-fenced it to direct. + const resolved = resolveConfiguredRoutedToolDiscoveryMode("my-gw", provider({ adapter: "cursor" }), "gpt-5.5"); + expect(resolved.mode).toBe("direct"); + + const entry = normalizeRoutedCatalogEntry({ slug: "my-gw/gpt-5.5" } as never, false, { + toolDiscoveryMode: resolved.mode, + providerId: "my-gw", + cursorRoute: true, + }) as Record; + expect(entry.supports_search_tool).toBe(false); + expect(entry.web_search_tool_type).toBeUndefined(); + expect(entry.supports_parallel_tool_calls).toBe(true); + }); + + it("stamps cursorRoute on the CatalogModel for a Cursor-adapter gateway", () => { + const hinted = applyProviderConfigHints("my-gw", provider({ adapter: "cursor" }), model("gpt-5.5", "my-gw")); + expect(hinted.cursorRoute).toBe(true); + expect(hinted.toolDiscoveryMode).toBe("direct"); + // A plain provider must not gain the marker. + expect(applyProviderConfigHints("deepseek", provider(), model("glm-5.2", "deepseek")).cursorRoute).toBeUndefined(); }); }); describe("routed tool-discovery propagation", () => { + // These drive the REAL construction paths in sync.ts rather than calling normalization + // directly, so the template-less fallback branch and its ensureStrictCatalogFields + // interaction are actually covered. + it("carries a direct override through the template-less fallback path", () => { + const entries = buildCatalogEntries(null, [], [ + { provider: "deepseek", id: "glm-5.2", toolDiscoveryMode: "direct" }, + ]); + const routed = entries.find(entry => entry.slug === "deepseek/glm-5.2"); + expect(routed?.supports_search_tool).toBe(false); + // Hosted search is a separate capability and survives direct mode. + expect(routed?.web_search_tool_type).toBe("text_and_image"); + expect(routed?.tool_mode).toBe("code_mode_only"); + }); + + it("keeps the fallback path on deferred with no configuration", () => { + const entries = buildCatalogEntries(null, [], [{ provider: "deepseek", id: "glm-5.2" }]); + const routed = entries.find(entry => entry.slug === "deepseek/glm-5.2"); + expect(routed?.supports_search_tool).toBe(true); + expect(routed?.web_search_tool_type).toBe("text_and_image"); + }); + + it("fences a Cursor-adapter gateway on the fallback path via cursorRoute", () => { + const entries = buildCatalogEntries(null, [], [ + { provider: "my-gw", id: "gpt-5.5", toolDiscoveryMode: "direct", cursorRoute: true }, + ]); + const routed = entries.find(entry => entry.slug === "my-gw/gpt-5.5"); + expect(routed?.supports_search_tool).toBe(false); + expect(routed?.web_search_tool_type).toBeUndefined(); + }); + it("carries the resolved mode onto the CatalogModel", () => { const hinted = applyProviderConfigHints("deepseek", provider({ routedToolDiscovery: "direct" }), model("glm-5.2", "deepseek")); expect(hinted.toolDiscoveryMode).toBe("direct"); @@ -230,6 +290,52 @@ describe("routed tool-discovery config admission", () => { expect(getterCalls).toBe(0); }); + it("rejects an accessor-backed provider-level mode without invoking the getter", () => { + // An accessor must not be mistaken for an absent field: treating it as absent lets the + // validator pass and Zod invokes the getter moments later, which is the whole bypass. + let getterCalls = 0; + const provider: Record = { adapter: "openai-responses", baseUrl: "https://example.invalid" }; + Object.defineProperty(provider, "routedToolDiscovery", { + enumerable: true, + configurable: true, + get() { + getterCalls += 1; + return "direct"; + }, + }); + const result = validateConfigCandidate({ defaultProvider: "deepseek", providers: { deepseek: provider } }); + expect(result.ok).toBe(false); + expect(getterCalls).toBe(0); + }); + + it("rejects an accessor-backed provider entry and an accessor providers map", () => { + let entryCalls = 0; + const providers: Record = {}; + Object.defineProperty(providers, "deepseek", { + enumerable: true, + configurable: true, + get() { + entryCalls += 1; + return { adapter: "openai-responses", baseUrl: "https://example.invalid", routedToolDiscovery: "direct" }; + }, + }); + expect(validateConfigCandidate({ defaultProvider: "deepseek", providers }).ok).toBe(false); + expect(entryCalls).toBe(0); + + let mapCalls = 0; + const root: Record = { defaultProvider: "deepseek" }; + Object.defineProperty(root, "providers", { + enumerable: true, + configurable: true, + get() { + mapCalls += 1; + return { deepseek: { adapter: "openai-responses", baseUrl: "https://example.invalid" } }; + }, + }); + expect(validateConfigCandidate(root).ok).toBe(false); + expect(mapCalls).toBe(0); + }); + it("ignores a prototype-sourced value rather than trusting it", () => { const polluted = Object.create({ routedToolDiscovery: "eager" }) as Record; polluted.adapter = "openai-responses"; From 2fd318b2a7b57f0b28ace256b6acbf7cd299dc04 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:22:40 +0900 Subject: [PATCH 03/13] fix(config): reject prototype-inherited routed discovery policy at the write boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review found the descriptor guard still had a hole. `ownDataProperty()` returned "absent" for a key reachable only through the prototype chain, so `routedToolDiscoveryError()` treated it as missing and let the candidate through to `configSchema.safeParse()`, which then read it. Reproduced three ways: an inherited `routedToolDiscovery: "direct"` was accepted and persisted into the admitted config, an inherited getter for that field was invoked once and its value accepted, and an inherited getter for the root `providers` key was invoked once and accepted. `ownDataProperty()` now returns a fourth result, "inherited", determined with `key in target` — which walks the prototype chain WITHOUT reading, so an inherited accessor is classified rather than invoked. Inherited keys are rejected alongside own accessors on `providers`, on each provider entry, and on both policy fields. The previous test was weak for the reason the reviewer identified: it used an inherited value of `"eager"`, which Zod's `.catch(undefined)` discards anyway, so it passed for the wrong reason. It now uses a VALID inherited value, which is the case that actually leaked, plus two inherited-getter cases asserting the getter call count stays at zero. Also corrects an overstated comment in tool-discovery.ts. The union fence was justified as "over-fencing only costs payload"; the reviewer showed that is incomplete, since a combo may legally be aliased `cursor/` (only `combo/` is reserved) and fencing it also removes hosted-search metadata and applies Cursor's parallel-tool advertisement. That row's fenced shape is exactly what shipped before this PR, so the union preserves behavior rather than regressing it, but the comment now says so honestly and names the real fix: reserve the `cursor/` alias prefix as a deliberate, documented compatibility break. Verification: bun x tsc --noEmit clean; 401 pass / 0 fail across 8 suites (37 in the focused file); privacy:scan passed; all three inherited-property reproductions re-run and confirmed closed. --- src/codex/catalog/tool-discovery.ts | 12 +++++-- src/config.ts | 21 +++++++----- tests/codex-tool-discovery-mode.test.ts | 45 ++++++++++++++++++++++--- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/src/codex/catalog/tool-discovery.ts b/src/codex/catalog/tool-discovery.ts index 5bf643a88..8fd65a580 100644 --- a/src/codex/catalog/tool-discovery.ts +++ b/src/codex/catalog/tool-discovery.ts @@ -75,8 +75,16 @@ export interface CursorRouteSignals { * canonical provider is `combo` was classified differently depending on whether a template * happened to exist (unresolved P2 on PR #1596). Any single-signal reconciliation would * silently UNFENCE rows that one path used to fence, and an under-fenced Cursor row - * advertises a deferred surface its transport cannot serve. Over-fencing only costs - * payload, so the union is the safe direction. + * advertises a deferred surface its transport cannot serve. + * + * The union is therefore the compatibility-preserving direction: it keeps every result the + * template path already produced. It is not free — combo aliases may legally carry one + * slash and only `combo/` is reserved, so a combo aliased `cursor/` is routed by the + * combo subsystem yet still gets fenced, losing hosted-search metadata and taking Cursor's + * parallel-tool advertisement, not merely paying payload. That row's fenced shape is + * exactly what shipped before this change, so this preserves behavior rather than + * regressing it. The real fix is to reserve the `cursor/` alias prefix (or move to an + * authoritative route identity) as a deliberate, documented compatibility break. * * `cursorRoute` carries the resolver's verdict (provider name or adapter), which is the only * signal that catches a Cursor-adapter gateway under a custom provider name. diff --git a/src/config.ts b/src/config.ts index 454a27ac5..f026ec124 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2258,18 +2258,23 @@ const ROUTED_TOOL_DISCOVERY_EXPECTED = "must be auto, deferred, or direct"; * reads them: a validator that touches `candidate.providers[x].modelRoutedToolDiscovery` * and only then checks for getters has already run attacker-supplied code. * - * `absent` and `accessor` are distinguished deliberately. Collapsing both to `undefined` - * makes a getter-backed field look like a missing field, so the validator waves it through - * and Zod invokes the getter moments later — the exact bypass this helper exists to stop. + * The three results are distinguished deliberately. Collapsing any of them to `undefined` + * makes a getter-backed or prototype-sourced field look like a missing field, so the + * validator waves it through and Zod invokes the getter (or copies the inherited value) + * moments later — the exact bypass this helper exists to stop. `inherited` covers a key + * reachable only through the prototype chain, whose descriptor is not own. */ type OwnDataProperty = | { readonly kind: "absent" } | { readonly kind: "accessor" } + | { readonly kind: "inherited" } | { readonly kind: "data"; readonly value: unknown }; function ownDataProperty(target: object, key: string): OwnDataProperty { const descriptor = Object.getOwnPropertyDescriptor(target, key); - if (!descriptor) return { kind: "absent" }; + // `in` walks the prototype chain WITHOUT reading, so an inherited accessor is classified + // rather than invoked. + if (!descriptor) return key in target ? { kind: "inherited" } : { kind: "absent" }; if (!("value" in descriptor)) return { kind: "accessor" }; return { kind: "data", value: descriptor.value }; } @@ -2286,7 +2291,7 @@ function routedToolDiscoveryError(value: unknown): string | null { const raw = rawConfigRecord(value); if (!raw) return null; const providersProperty = ownDataProperty(raw, "providers"); - if (providersProperty.kind === "accessor") { + if (providersProperty.kind === "accessor" || providersProperty.kind === "inherited") { return "schema_invalid: providers: must be a plain object"; } const providers = providersProperty.kind === "data" ? providersProperty.value : undefined; @@ -2294,14 +2299,14 @@ function routedToolDiscoveryError(value: unknown): string | null { for (const name of Object.getOwnPropertyNames(providers)) { const providerProperty = ownDataProperty(providers, name); - if (providerProperty.kind === "accessor") { + if (providerProperty.kind === "accessor" || providerProperty.kind === "inherited") { return `schema_invalid: providers.${name}: must be a plain object`; } const provider = providerProperty.kind === "data" ? providerProperty.value : undefined; if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; const modeProperty = ownDataProperty(provider, "routedToolDiscovery"); - if (modeProperty.kind === "accessor") { + if (modeProperty.kind === "accessor" || modeProperty.kind === "inherited") { return `schema_invalid: providers.${name}.routedToolDiscovery: ${ROUTED_TOOL_DISCOVERY_EXPECTED}`; } if (modeProperty.kind === "data" @@ -2311,7 +2316,7 @@ function routedToolDiscoveryError(value: unknown): string | null { } const mapProperty = ownDataProperty(provider, "modelRoutedToolDiscovery"); - if (mapProperty.kind === "accessor") { + if (mapProperty.kind === "accessor" || mapProperty.kind === "inherited") { return `schema_invalid: providers.${name}.modelRoutedToolDiscovery: must be a plain object of model overrides`; } if (mapProperty.kind === "absent") continue; diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index c70d5d348..b5a5260e9 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -336,11 +336,48 @@ describe("routed tool-discovery config admission", () => { expect(mapCalls).toBe(0); }); - it("ignores a prototype-sourced value rather than trusting it", () => { - const polluted = Object.create({ routedToolDiscovery: "eager" }) as Record; + it("rejects a prototype-inherited value even when it is otherwise valid", () => { + // An INVALID inherited value is a weak test: Zod's .catch(undefined) would discard it and + // the candidate would pass for the wrong reason. A VALID inherited value is the real + // case — without an explicit check it gets copied into the admitted config. + const polluted = Object.create({ routedToolDiscovery: "direct" }) as Record; polluted.adapter = "openai-responses"; polluted.baseUrl = "https://example.invalid"; - // Inherited, so it is not an own data property and must not be read as configuration. - expect(validateConfigCandidate({ defaultProvider: "deepseek", providers: { deepseek: polluted } }).ok).toBe(true); + expect(validateConfigCandidate({ defaultProvider: "deepseek", providers: { deepseek: polluted } }).ok).toBe(false); + }); + + it("rejects a prototype-inherited getter without invoking it", () => { + let getterCalls = 0; + const proto = {}; + Object.defineProperty(proto, "routedToolDiscovery", { + configurable: true, + get() { + getterCalls += 1; + return "direct"; + }, + }); + const polluted = Object.create(proto) as Record; + polluted.adapter = "openai-responses"; + polluted.baseUrl = "https://example.invalid"; + const result = validateConfigCandidate({ defaultProvider: "deepseek", providers: { deepseek: polluted } }); + expect(result.ok).toBe(false); + // `in` walks the prototype chain without reading, so the getter must never run. + expect(getterCalls).toBe(0); + }); + + it("rejects an inherited providers map without invoking its getter", () => { + let getterCalls = 0; + const proto = {}; + Object.defineProperty(proto, "providers", { + configurable: true, + get() { + getterCalls += 1; + return { deepseek: { adapter: "openai-responses", baseUrl: "https://example.invalid" } }; + }, + }); + const root = Object.create(proto) as Record; + root.defaultProvider = "deepseek"; + expect(validateConfigCandidate(root).ok).toBe(false); + expect(getterCalls).toBe(0); }); }); From 524b649efb078956806c98c8b4fa547294a9cde0 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:35:07 +0900 Subject: [PATCH 04/13] test(codex): add gather-identity coverage for routed tool-discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two end-to-end cases: two configs differing only in `routedToolDiscovery` gather distinct results, and an identical policy still reuses the flight. Both are labeled honestly, because ablation proved the obvious claim false. Deleting `rtd`/`mrtd` from `providerCatalogFingerprint` leaves both cases GREEN: `providerGraphIdentity` already hashes the whole admitted provider row and refuses the join on its own. So these guard end-to-end behavior; they do not prove the fingerprint carries the policy, and the comment says so rather than letting a future reader mistake them for that proof. The fingerprint fields stay regardless. It is an explicit allow-list whose omissions have leaked flights twice before — credentials until `authIdentity` landed, then `reasoningEfforts` — both reproduced against real routes. Keeping it semantically complete is defense in depth, per the existing comment at provider-fetch.ts:198-208. Verification: bun x tsc --noEmit clean; 403 pass / 0 fail across 8 suites; privacy:scan passed; ablation run recorded above. --- tests/codex-tool-discovery-mode.test.ts | 40 +++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index b5a5260e9..78ba3d56f 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "bun:test"; import { applyProviderConfigHints, buildCatalogEntries, normalizeRoutedCatalogEntry } from "../src/codex/catalog"; +import { gatherRoutedModels } from "../src/codex/catalog"; import { deriveComboCatalogModel } from "../src/codex/catalog/aggregation"; import type { CatalogModel } from "../src/codex/catalog/parsing"; import { @@ -237,6 +238,45 @@ describe("combo tool-discovery derivation", () => { }); }); +describe("routed tool-discovery gather identity", () => { + // Scope note, established by ablation: deleting `rtd`/`mrtd` from + // providerCatalogFingerprint leaves BOTH cases below green, because + // providerGraphIdentity already hashes the whole admitted provider row and refuses the + // join on its own. So these are end-to-end behavior guards, NOT proof that the + // fingerprint carries the policy. The fingerprint fields are added because it is an + // explicit allow-list whose omissions have leaked flights twice before (credentials, + // then reasoningEfforts, both reproduced against real routes) — keeping it semantically + // complete is defense in depth, and its correctness is not what these cases pin. + function gatherConfig(routedToolDiscovery?: "auto" | "deferred" | "direct"): never { + return { + defaultProvider: "deepseek", + providers: { + deepseek: { + adapter: "openai-responses", + baseUrl: "https://example.invalid", + models: ["glm-5.2"], + liveModels: false, + ...(routedToolDiscovery ? { routedToolDiscovery } : {}), + }, + }, + } as never; + } + + it("gathers distinct results for two different policies", async () => { + const deferred = await gatherRoutedModels(gatherConfig()); + const direct = await gatherRoutedModels(gatherConfig("direct")); + + expect(deferred.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("deferred"); + expect(direct.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("direct"); + }); + + it("still reuses the gather for an identical policy", async () => { + const first = await gatherRoutedModels(gatherConfig("direct")); + const second = await gatherRoutedModels(gatherConfig("direct")); + expect(second).toEqual(first); + }); +}); + describe("routed tool-discovery config admission", () => { // defaultProvider must resolve, otherwise every candidate fails on an unrelated rule. function candidate(providerPatch: Record): Record { From 9315ad0e709a367cf9427fc52b7eec0e3009d7de Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:43:43 +0900 Subject: [PATCH 05/13] test(codex): close WP3 coverage gaps and fix a false-confidence gather test An independent audit of 524b649ef returned FAIL. Two findings were correct and are fixed here; the doc translations are unrelated-looking but in scope. The "still reuses the gather for an identical policy" test never observed reuse. Its two calls were sequential and awaited, while a completed flight is removed from the in-flight map immediately (provider-fetch.ts finally-block), so equal results proved nothing about flight behavior. Replaced with a genuinely concurrent Promise.all case that asserts two different policies do not serve one another's rows, and the surrounding comment now says these pin the admission invariant rather than the fingerprint field list. The audit also showed WP3's "little left to do" claim was overstated. Added the planned coverage that was actually missing: - 023: zero-config byte comparison, an absolute emitted-key-set pin, the Cursor row shape, and assertions that the internal `toolDiscoveryMode`/`cursorRoute` fields never reach the Codex catalog on either construction path. - 025: the remaining combo compositions (direct in any position, arity 1..3, undefined members) and confirmation the field is omitted entirely for an all-deferred combo. Every new guard was ablation-verified rather than assumed. That mattered twice: the first byte-comparison test passed with a stray field injected, because two rows built by the same path both carry the leak; and the key-set pin initially appeared to pass an ablation of normalizeRoutedCatalogEntry, which turned out to be because buildCatalogEntries(null, ...) exercises the template-less branch that never calls it. Both paths now have their own guard, and injecting a stray field into either one fails a test. Also propagates the two config rows to the ja/ko/ru/zh-cn/zh-tw provider reference pages so the translated docs do not contradict the English source. Verification: bun x tsc --noEmit clean; 411 pass / 0 fail across 8 suites (44 in the focused file, up from 36); privacy:scan passed; all ablations restored and `git diff --quiet` confirmed clean on the touched source files. --- .../ja/reference/configuration/providers.md | 2 + .../ko/reference/configuration/providers.md | 2 + .../ru/reference/configuration/providers.md | 2 + .../reference/configuration/providers.md | 2 + .../reference/configuration/providers.md | 2 + tests/codex-tool-discovery-mode.test.ts | 152 ++++++++++++++++-- 6 files changed, 151 insertions(+), 11 deletions(-) diff --git a/docs-site/src/content/docs/ja/reference/configuration/providers.md b/docs-site/src/content/docs/ja/reference/configuration/providers.md index a990b5226..d93a7a474 100644 --- a/docs-site/src/content/docs/ja/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ja/reference/configuration/providers.md @@ -95,6 +95,8 @@ account を削除しても mapping は保持され、同じ id を再追加す | `noPenaltyModels?` | `string[]` |存在/周波数ペナルティを拒否するモデル。 | | `noStructuredOutputModels?` | `string[]` | `openai-chat` エンドポイントが `response_format` を拒否する正確なモデル ID。要求モデルが項目と完全一致する場合だけフィールドを省略し、その他の `openai-chat` モデルでは structured-output 変換を維持します。 | | `parallelToolCalls?` | `boolean` |並列ツール呼び出しを切り替えます。 OpenAI Chat はデフォルトでオンになっています。非チャット アダプターは明示的な `true` でのみアドバタイズします。 | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | ルーティング行の Codex ツール探索ポリシーです。`auto`（デフォルト）は出荷時の挙動を保ちます。非 Cursor のルーティング行は deferred、Cursor は direct で、Cursor はハードフェンスのため `deferred` を設定しても無視されます。deferred 探索と非互換だと実証された経路にのみ `direct` を使ってください。最初のリクエストに MCP 宣言がすべて載って turn-1 ペイロードが実測で 2.7 倍になり、コードモードでは Codex がどちらでも `tools`/`ALL_TOOLS` グローバルに入れ子ツールを設置するため、本来利用可能なツールの到達性が回復するわけでは**ありません**。ホスト型ウェブ検索（`web_search_tool_type`）とは独立です。 | +| `modelRoutedToolDiscovery?` | `Record` | `routedToolDiscovery` のモデル単位の上書きです。混在ゲートウェイで非互換な 1 モデルのために兄弟モデルが不利にならないようにします。マッチングは通常のモデルキー規則（完全一致の id、`:` より前のファミリー、大文字小文字を無視）に従い、日付付きの `-YYYYMMDD` 変種はマッチしないため、失敗したモデル id を正確に指定してください。 | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` |正確なプレースホルダー ID、欠落している端末 ID、および（`repairInvalidIds` で）正規の `msg_`/`rs_` 接頭辞を欠く message/reasoning ID に対するダウンストリーム SSE 修復はデフォルトで無効になっています。関数呼び出し ID は決して書き換えられません。組み込み DeepSeek は最後の 2 つをデフォルトで有効にします。 | | `responsesSnapshotRepair?` | `boolean` | デフォルトで無効のクライアント向け修復です。SSE と JSON の Responses ライフサイクルで欠落した status、output、ツールメタデータを補完し、raw 検査と永続化は変更しません。 | | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key プロバイダーのみ(`authMode: "key"`)。オプトインの同一ターゲット 429 リトライ: `retryOn429` が無ければ無効で、オブジェクトがあれば `enabled: false` でない限り有効になります。429 時に待機(上流の `Retry-After` または固定間隔)してから、キー フェイルオーバーの前に同一キーで同一リクエストを再送します — メインのテキストターン回復ループ、Responses passthrough、画像/動画ブリッジ、web-search サイドカー、ターミナル継続要求をすべてカバーします。再送の対象はプリストリームの HTTP 429 応答のみで、カスタム `runTurn` トランスポートは HTTP リトライループの対象外です。`attempts` は最初の 429 以降の同一キー再送回数(合計送信数 = `attempts` + 1)で、メインの回復ループ・ターミナルガード継続・ブリッジ再試行で共有されるリクエスト単位の予算です。`attempts` を使い切っても同一キーでの再送が止まるだけで、通常のキー フェイルオーバーまたは最終エラー処理が利用可能なターゲットに応じて続きます — キー認証の passthrough ワイヤにはフェイルオーバーがないため、使い切った 429 はそのまま返ります。Codex 自体は 429 をリトライしないため、単一キーのプロバイダーでは唯一の防御です。デフォルト: `enabled: true`、`attempts: 3`、`intervalMs: 5000`、`maxIntervalMs: 60000`(1回の待機は `maxIntervalMs` で上限、その上限は 600000)、`respectRetryAfter: true`。 | diff --git a/docs-site/src/content/docs/ko/reference/configuration/providers.md b/docs-site/src/content/docs/ko/reference/configuration/providers.md index 7aed7c777..70c0d8263 100644 --- a/docs-site/src/content/docs/ko/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ko/reference/configuration/providers.md @@ -95,6 +95,8 @@ managed map을 활성화하면 privacy-safe selector를 만들고, 이후 계정 | `noPenaltyModels?` | `string[]` | presence/frequency penalty를 허용하지 않는 모델입니다. | | `noStructuredOutputModels?` | `string[]` | `openai-chat` 엔드포인트가 `response_format`을 거부하는 정확한 모델 ID입니다. 요청 모델이 항목과 정확히 일치할 때만 필드를 생략하며, 그 외 `openai-chat` 모델에서는 structured-output 변환을 유지합니다. | | `parallelToolCalls?` | `boolean` | 병렬 도구 호출을 켜거나 끕니다. OpenAI Chat은 기본으로 켜져 있고, 비-chat 어댑터는 명시적으로 `true`일 때만 이를 노출합니다. | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | 라우팅된 행의 Codex 도구 탐색 정책입니다. `auto`(기본값)는 출하된 동작을 그대로 유지합니다. 비-Cursor 라우팅 행은 deferred, Cursor는 direct이며 Cursor는 하드 펜스라 `deferred`를 설정해도 무시합니다. deferred 탐색과 맞지 않는다고 입증된 경로에만 `direct`를 쓰십시오. 첫 요청에 모든 MCP 선언이 실려 turn-1 페이로드가 측정상 2.7배로 늘고, 코드 모드에서는 Codex가 어느 쪽이든 `tools`/`ALL_TOOLS` 전역에 중첩 도구를 심기 때문에 자격을 갖춘 도구의 도달성을 되살려 주지는 **않습니다**. 호스티드 웹 검색(`web_search_tool_type`)과는 무관합니다. | +| `modelRoutedToolDiscovery?` | `Record` | `routedToolDiscovery`의 모델별 재정의입니다. 혼합 게이트웨이에서 호환되지 않는 한 모델 때문에 형제 모델까지 손해 보지 않게 합니다. 매칭은 기존 모델 키 규칙(정확한 id, `:` 앞 계열, 대소문자 무시)을 따르며 날짜가 붙은 `-YYYYMMDD` 변형은 매칭하지 않으므로 실패한 모델 id를 정확히 적으십시오. | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | 기본값이 꺼진 downstream SSE 복구입니다. 정확한 자리표시자 id, 누락된 종료 id, 그리고(`repairInvalidIds`) 정규 `msg_`/`rs_` 접두사가 없는 message/reasoning id를 복구합니다. function-call id는 다시 쓰지 않습니다. 내장 DeepSeek은 마지막 두 가지를 기본으로 켭니다. | | `responsesSnapshotRepair?` | `boolean` | 기본값이 꺼진 클라이언트용 복구입니다. SSE와 JSON의 Responses 수명 주기에서 누락된 status, output, 도구 메타데이터를 채우며 raw 검사와 영속화는 변경하지 않습니다. | | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key 프로바이더 전용(`authMode: "key"`). 동일 대상 429 재시도: `retryOn429`가 없으면 기능이 꺼져 있고, 객체가 있으면 `enabled: false`가 아닌 한 활성화됩니다. 429 시 대기(업스트림 `Retry-After` 또는 고정 간격) 후 키 장애 조치 전에 동일 키로 동일 요청을 재전송합니다 — 일반 텍스트 턴 복구 루프, Responses passthrough, 이미지/비디오 브리지, web-search 사이드카, 터미널 연속 요청을 모두 포함합니다. 재전송 대상은 프리스트림 HTTP 429 응답뿐이며, 커스텀 `runTurn` 전송은 HTTP 재시도 루프에서 제외됩니다. `attempts`는 첫 429 이후의 동일 키 재전송 횟수(총 전송 = `attempts` + 1)이며, 메인 복구 루프·터미널 가드 연속 요청·브리지 재시도가 공유하는 요청 단위 예산입니다. `attempts`를 모두 소진해도 동일 키 재전송만 중단되며, 이후에는 일반 키 장애 조치 또는 최종 오류 처리가 사용 가능한 대상에 따라 진행됩니다 — 키 인증 passthrough 와이어에는 장애 조치가 없으므로 소진된 429가 그대로 반환됩니다. Codex 자체는 429를 재시도하지 않으므로 단일 키 프로바이더의 유일한 방어선입니다. 기본값: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000`(단일 대기는 `maxIntervalMs`로 상한, 그 자체는 600000으로 상한), `respectRetryAfter: true`. | diff --git a/docs-site/src/content/docs/ru/reference/configuration/providers.md b/docs-site/src/content/docs/ru/reference/configuration/providers.md index b1bb26c78..43608c1dd 100644 --- a/docs-site/src/content/docs/ru/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ru/reference/configuration/providers.md @@ -108,6 +108,8 @@ cross-route credential fallback не существует. Строки API GPT- | `noPenaltyModels?` | `string[]` | Модели, отвергающие penalty presence/frequency. | | `noStructuredOutputModels?` | `string[]` | Точные идентификаторы моделей, чей endpoint `openai-chat` отклоняет `response_format`. Поле опускается только при точном совпадении запрошенной модели; для остальных моделей `openai-chat` преобразование structured output остаётся включённым. | | `parallelToolCalls?` | `boolean` | Переключатель parallel tool call'ов. Для OpenAI Chat по умолчанию включено; не-chat adapter'ы рекламируют это только при явном `true`. | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | Политика обнаружения Codex-инструментов для routed-строк. `auto` (по умолчанию) сохраняет поставляемое поведение: не-Cursor routed-строки — deferred, Cursor — direct, причём Cursor жёстко зафиксирован и игнорирует заданный `deferred`. Указывайте `direct` только для маршрута, несовместимость которого с deferred-обнаружением доказана: он вкладывает все объявления MCP в первый запрос (измеренная плата — 2.7x к turn-1 payload) и **не** возвращает доступность иначе пригодного инструмента в code mode, где Codex в любом случае помещает вложенные инструменты в глобальные `tools`/`ALL_TOOLS`. Независимо от hosted web search (`web_search_tool_type`). | +| `modelRoutedToolDiscovery?` | `Record` | Переопределение `routedToolDiscovery` для конкретной модели, чтобы одна несовместимая модель на смешанном шлюзе не наказывала соседние. Сопоставление следует обычным правилам ключей моделей (точный id, семейство до `:`, без учёта регистра); варианты с датой `-YYYYMMDD` не сопоставляются, поэтому указывайте точный id проблемной модели. | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | По умолчанию выключенная downstream SSE-repair для exact placeholder-id, отсутствующих terminal-id и (с `repairInvalidIds`) message/reasoning id без канонического префикса `msg_`/`rs_`. Function-call id никогда не переписываются. Встроенный DeepSeek включает последние два по умолчанию. | | `responsesSnapshotRepair?` | `boolean` | По умолчанию выключенная клиентская repair для неполных lifecycle snapshot'ов Responses в SSE и JSON. Добавляет отсутствующие status, output и tool metadata, не меняя raw inspection и persistence. | | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | Только для провайдеров с API-ключом (`authMode: "key"`). Опциональный повтор при 429 на том же таргете: если `retryOn429` отсутствует, функция выключена; наличие объекта включает её, если только `enabled: false`. При 429: ожидание (`Retry-After` апстрима или фиксированный интервал) и повтор идентичного запроса на том же ключе до любого фейловера ключей — покрывает основной цикл восстановления текстовых ходов, passthrough-канал Responses, мост изображений/видео, sidecar web-search и терминальные продолжения. Повтор допустим только для HTTP 429, полученных до начала потока; пользовательские транспорты `runTurn` не входят в цикл HTTP-повторов. `attempts` — это число повторов на том же ключе после первого 429 (всего отправок = `attempts` + 1) и единый бюджет на запрос, общий для основного цикла восстановления, терминального продолжения и повторов моста. Исчерпание `attempts` лишь останавливает дальнейшие повторы на том же ключе; далее применяется обычный фейловер ключей или финальная обработка ошибки в зависимости от доступных таргетов — на passthrough-канале с ключевой аутентификацией фейловера нет, поэтому исчерпанный 429 возвращается как есть. Codex сам никогда не повторяет 429, поэтому это единственная защита для провайдеров с одним ключом. По умолчанию: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000` (любое ожидание ограничено `maxIntervalMs`, который сам ограничен 600000), `respectRetryAfter: true`. | diff --git a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md index 1d2a8c10c..a2abff2c7 100644 --- a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md +++ b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md @@ -95,6 +95,8 @@ selector，而不是分配一个新名称。 | `noPenaltyModels?` | `string[]` | 会拒绝 presence/frequency penalty 的模型。 | | `noStructuredOutputModels?` | `string[]` | `openai-chat` 端点拒绝 `response_format` 的精确模型 ID。仅当请求模型与条目完全匹配时才省略该字段；其他 `openai-chat` 模型仍启用 structured-output 转换。 | | `parallelToolCalls?` | `boolean` | 切换并行工具调用。OpenAI Chat 默认开启；非 chat 适配器只有显式 `true` 时才会声明支持。 | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | routed 行的 Codex 工具发现策略。`auto`（默认）保持出厂行为：非 Cursor 的 routed 行为 deferred，Cursor 为 direct，且 Cursor 是硬围栏，即使配置 `deferred` 也会被忽略。只有在已证明与 deferred 发现不兼容的路由上才设置 `direct`：它会把全部 MCP 声明塞进首个请求（实测 turn-1 负载增加 2.7 倍），而且在 code mode 下并**不会**让本来就可用的工具重新可达——Codex 无论哪种模式都会把嵌套工具装到 `tools`/`ALL_TOOLS` 全局上。与托管网页搜索（`web_search_tool_type`）互不相关。 | +| `modelRoutedToolDiscovery?` | `Record` | 按模型覆盖 `routedToolDiscovery`，使混合网关上某个不兼容的模型不会牵连同组其他模型。匹配遵循通常的模型键规则（精确 id、`:` 之前的系列名、忽略大小写）；带日期的 `-YYYYMMDD` 变体不参与匹配，因此请写出确切失败的模型 id。 | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | 默认关闭的下游 SSE 修复，用于精确占位 id、缺失的终止 id，以及（`repairInvalidIds`）缺少规范 `msg_`/`rs_` 前缀的 message/reasoning id。function-call id 永远不会被重写。内置 DeepSeek 默认启用后两项。 | | `responsesSnapshotRepair?` | `boolean` | 默认关闭的客户端修复，用于补全 SSE 与 JSON 中稀疏 Responses 生命周期快照缺失的 status、output 和工具元数据；原始检查与持久化保持不变。 | | `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | 仅限 API-key 提供商（`authMode: "key"`）。可选的同目标 429 重试：未配置 `retryOn429` 时功能关闭；对象存在即启用，除非 `enabled: false`。收到 429 时等待（上游 `Retry-After` 或固定间隔）后在相同 key 上重放完全相同请求，再进入任何 key 故障转移——覆盖主文本恢复循环、Responses passthrough、图像/视频桥、web-search 侧车与终结续接。重放仅适用于流开始前的 HTTP 429 响应；自定义 `runTurn` 传输不在 HTTP 重试循环范围内。`attempts` 是首个 429 之后的同 key 重放次数（总发送次数 = `attempts` + 1），是主恢复循环、终结守卫续接与桥接重试共享的按请求统一预算；`attempts` 耗尽只会停止进一步的同 key 重放：随后按可用目标进行正常的 key 故障转移或最终错误处理——key 认证的 passthrough 线路上没有故障转移，因此耗尽的 429 会原样透出。Codex 自身从不重试 429，因此这是单 key 提供商唯一的防线。默认值：`enabled: true`、`attempts: 3`、`intervalMs: 5000`、`maxIntervalMs: 60000`（单次等待以 `maxIntervalMs` 为上限，其本身上限 600000）、`respectRetryAfter: true`。 | diff --git a/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md b/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md index 2f729dc35..16011fccc 100644 --- a/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md +++ b/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md @@ -74,6 +74,8 @@ description: 供應商項目、認證、端點、模型目錄、配額、context | `noPenaltyModels?` | `string[]` | 拒絕 presence/frequency penalty 的模型。 | | `noStructuredOutputModels?` | `string[]` | 其 `openai-chat` 端點拒絕 `response_format` 的精確模型 ID。僅精確符合的請求模型會省略該欄位；structured-output 轉譯對其他每個 `openai-chat` 模型保持啟用。 | | `parallelToolCalls?` | `boolean` | 切換平行工具呼叫。OpenAI Chat 預設開啟；非 chat adapter 僅在明確 `true` 時廣告。 | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | routed 列的 Codex 工具探索政策。`auto`（預設）維持出廠行為：非 Cursor 的 routed 列為 deferred，Cursor 為 direct，且 Cursor 屬硬圍籬，即使設定 `deferred` 也會被忽略。只在已證實與 deferred 探索不相容的路由上設定 `direct`：它會把所有 MCP 宣告塞進第一個請求（實測 turn-1 負載增加 2.7 倍），而且在 code mode 下並**不會**讓原本可用的工具重新可達——Codex 無論哪種模式都會把巢狀工具裝到 `tools`/`ALL_TOOLS` 全域上。與代管網頁搜尋（`web_search_tool_type`）彼此獨立。 | +| `modelRoutedToolDiscovery?` | `Record` | 依模型覆寫 `routedToolDiscovery`，讓混合閘道上某個不相容的模型不會拖累同組其他模型。比對遵循一般模型鍵規則（精確 id、`:` 之前的系列、忽略大小寫）；帶日期的 `-YYYYMMDD` 變體不會比對，因此請寫出確切失敗的模型 id。 | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean }` | 預設停用的下游 SSE 修復，用於精確佔位 id 與缺失的終端 id。Function-call id 永不被重寫。 | | `autoToolChoiceOnlyModels?` | `string[]` | 其 `tool_choice` 僅接受 `auto` 或 `none` 的模型；強制選擇被降級。 | | `preserveReasoningContentModels?` | `string[]` | 需要在 chat 歷史中保留先前 assistant `reasoning_content` 的模型。 | diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index 78ba3d56f..b670580d1 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -236,17 +236,139 @@ describe("combo tool-discovery derivation", () => { ); expect(combo?.toolDiscoveryMode).toBe("direct"); }); + + it("covers the remaining member compositions", () => { + // 025_combo_policy_tests.md: a single direct member must dominate regardless of position + // or arity, and an unknown member must not be read as direct. + expect(deriveComboToolDiscoveryMode(["direct", "deferred"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode(["deferred", "deferred", "direct"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode(["direct", "direct"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode(["deferred", undefined])).toBe("deferred"); + expect(deriveComboToolDiscoveryMode([undefined, "direct"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode(["direct"])).toBe("direct"); + }); + + it("omits the field entirely for an all-deferred combo", () => { + // Emitting it unconditionally broke codex-catalog's combo-shape assertion during + // development; an all-deferred combo must stay byte-identical to the pre-override shape. + const combo = deriveComboCatalogModel( + "combo/plain", + { targets: [{ provider: "p1", model: "a" }, { provider: "p2", model: "b" }], strategy: "failover" } as never, + [ + { id: "a", provider: "p1", contextWindow: 200_000, toolDiscoveryMode: "deferred" }, + { id: "b", provider: "p2", contextWindow: 200_000, toolDiscoveryMode: "deferred" }, + ], + ); + expect(combo).not.toHaveProperty("toolDiscoveryMode"); + }); + + it("keeps a direct combo direct through catalog serialization under both alias shapes", () => { + for (const slug of ["combo/mix", "mix"]) { + const rows = buildCatalogEntries(null, [], [ + { provider: "combo", id: slug.includes("/") ? slug.slice(slug.indexOf("/") + 1) : slug, toolDiscoveryMode: "direct" }, + ]); + const row = rows.find(entry => typeof entry.slug === "string" && entry.slug.endsWith(slug.replace("combo/", ""))); + expect(row?.supports_search_tool).toBe(false); + expect(row?.web_search_tool_type).toBe("text_and_image"); + } + }); +}); + +describe("routed tool-discovery backward compatibility", () => { + // 023_backward_compatibility_tests.md: the load-bearing promise of this feature is that an + // unconfigured tree is unchanged. Assert the WHOLE emitted row, not just the two policy + // fields, so an unrelated key silently appearing on routed rows also fails here. + it("emits byte-identical routed rows with zero configuration", () => { + const withoutPolicy = buildCatalogEntries(null, [], [{ provider: "deepseek", id: "glm-5.2" }]); + const withExplicitAuto = buildCatalogEntries(null, [], [ + { provider: "deepseek", id: "glm-5.2", toolDiscoveryMode: "deferred" }, + ]); + expect(JSON.stringify(withExplicitAuto)).toBe(JSON.stringify(withoutPolicy)); + }); + + // Comparing two rows built by the same path cannot catch a key that leaks onto BOTH, so + // the key set is pinned absolutely and any new catalog field must be added here + // deliberately. Ablation-verified: injecting a stray field into the template-less branch + // fails this, and injecting one into normalizeRoutedCatalogEntry fails the template case. + const EXPECTED_ROUTED_KEYS = [ + "apply_patch_tool_type", + "auto_compact_token_limit", + "base_instructions", + "comp_hash", + "context_window", + "default_reasoning_level", + "default_reasoning_summary", + "default_verbosity", + "description", + "display_name", + "effective_context_window_percent", + "experimental_supported_tools", + "input_modalities", + "max_context_window", + "priority", + "shell_type", + "slug", + "support_verbosity", + "supported_in_api", + "supported_reasoning_levels", + "supports_image_detail_original", + "supports_parallel_tool_calls", + "supports_reasoning_summaries", + "supports_search_tool", + "tool_mode", + "truncation_policy", + "visibility", + "web_search_tool_type", + ]; + + it("pins the exact emitted key set on the template-less path", () => { + const rows = buildCatalogEntries(null, [], [{ provider: "deepseek", id: "glm-5.2" }]); + const routed = rows.find(entry => entry.slug === "deepseek/glm-5.2"); + expect(Object.keys(routed ?? {}).sort()).toEqual(EXPECTED_ROUTED_KEYS); + }); + + it("pins the emitted policy fields on the template path", () => { + // The template path runs normalizeRoutedCatalogEntry, which the fallback never calls, so + // it needs its own guard against an internal field reaching the Codex catalog. + const normalized = normalizeRoutedCatalogEntry({ slug: "deepseek/glm-5.2" } as never, false, { + toolDiscoveryMode: "direct", + providerId: "deepseek", + cursorRoute: false, + }) as Record; + expect(normalized).not.toHaveProperty("toolDiscoveryMode"); + expect(normalized).not.toHaveProperty("tool_discovery_mode"); + expect(normalized).not.toHaveProperty("cursorRoute"); + expect(normalized).not.toHaveProperty("leaked_internal_field"); + expect(Object.keys(normalized).every(key => key === key.toLowerCase())).toBe(true); + }); + + it("keeps the Cursor row shape unchanged with zero configuration", () => { + const rows = buildCatalogEntries(null, [], [{ provider: "cursor", id: "gpt-5.5" }]); + const cursor = rows.find(entry => entry.slug === "cursor/gpt-5.5"); + expect(cursor?.supports_search_tool).toBe(false); + expect(cursor).not.toHaveProperty("web_search_tool_type"); + // The catalog must never carry the opencodex-only internal field. + expect(cursor).not.toHaveProperty("tool_discovery_mode"); + expect(cursor).not.toHaveProperty("toolDiscoveryMode"); + }); + + it("never serializes the internal policy field onto a routed row", () => { + const rows = buildCatalogEntries(null, [], [ + { provider: "deepseek", id: "glm-5.2", toolDiscoveryMode: "direct", cursorRoute: false }, + ]); + const routed = rows.find(entry => entry.slug === "deepseek/glm-5.2"); + expect(routed).not.toHaveProperty("toolDiscoveryMode"); + expect(routed).not.toHaveProperty("tool_discovery_mode"); + expect(routed).not.toHaveProperty("cursorRoute"); + }); }); describe("routed tool-discovery gather identity", () => { // Scope note, established by ablation: deleting `rtd`/`mrtd` from - // providerCatalogFingerprint leaves BOTH cases below green, because - // providerGraphIdentity already hashes the whole admitted provider row and refuses the - // join on its own. So these are end-to-end behavior guards, NOT proof that the - // fingerprint carries the policy. The fingerprint fields are added because it is an - // explicit allow-list whose omissions have leaked flights twice before (credentials, - // then reasoningEfforts, both reproduced against real routes) — keeping it semantically - // complete is defense in depth, and its correctness is not what these cases pin. + // providerCatalogFingerprint leaves these green, because providerGraphIdentity already + // hashes the whole admitted provider row and refuses the join on its own. These pin the + // ADMISSION INVARIANT (two policies never share one flight), not the fingerprint's field + // list. Do not read them as proof that `rtd`/`mrtd` are load-bearing; they are redundancy. function gatherConfig(routedToolDiscovery?: "auto" | "deferred" | "direct"): never { return { defaultProvider: "deepseek", @@ -270,10 +392,18 @@ describe("routed tool-discovery gather identity", () => { expect(direct.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("direct"); }); - it("still reuses the gather for an identical policy", async () => { - const first = await gatherRoutedModels(gatherConfig("direct")); - const second = await gatherRoutedModels(gatherConfig("direct")); - expect(second).toEqual(first); + it("refuses to join one in-flight gather across two different policies", async () => { + // Sequential awaited calls cannot observe flight behavior at all: a completed flight is + // removed from the map immediately (provider-fetch.ts finally-block), so equal results + // would prove nothing. These run CONCURRENTLY so both are genuinely in flight together. + const [deferred, direct] = await Promise.all([ + gatherRoutedModels(gatherConfig("deferred")), + gatherRoutedModels(gatherConfig("direct")), + ]); + + // If the two policies shared a flight, one would be served the other's rows. + expect(deferred.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("deferred"); + expect(direct.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("direct"); }); }); From d3be5eb82d8a06ea968497504a04f9919bbea36e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:49:23 +0900 Subject: [PATCH 06/13] test(codex): make the template-path key guard absolute, not a name blocklist Re-audit of 9315ad0e7 found my previous commit message overstated its own coverage, and the reviewer was right. The template-path guard rejected three named policy fields plus one hard-coded sentinel and required lowercase keys. Injecting the anticipated `leaked_internal_field` failed it, which is what I tested; injecting `another_internal_field` passed straight through. So "injecting a stray field into either path fails a test" was true of the fallback path and false of the template path. It now asserts the exact emitted key set, matching the fallback guard, and is verified with the reviewer's own ablation: an arbitrarily named field injected into normalizeRoutedCatalogEntry fails it. Also records Phase 2 as NOT fully closed in 029, rather than letting the shipped config half imply the whole phase landed. Two items stay open as named debt: the 020 single-variable code-mode differential, which needs a running Codex client and keeps the direct-is-not-a-reachability-fix claim source-derived until it runs, and 024's model-map divergence plus warm-cache refresh cases. Verification: bun x tsc --noEmit clean; 414 pass / 0 fail across 8 suites (47 in the focused file); privacy:scan passed; SHA256SUMS regenerated and fully verified; ablation restored with git diff --quiet confirmed clean. --- .../029_phase2_exit_gate.md | 18 ++ .../SHA256SUMS | 2 +- tests/codex-tool-discovery-mode.test.ts | 204 ++++++++++++++---- 3 files changed, 187 insertions(+), 37 deletions(-) diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md index f924c19ca..b553dff09 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md @@ -1,5 +1,23 @@ # 029 - Phase 2 exit gate +> **Status 2026-08-13: Phase 2 is NOT fully closed.** The configuration and +> catalog-policy half shipped (see PR "feat(codex): add routed tool-discovery +> compatibility profiles"), but two planned items remain OPEN and are carried as +> explicit debt rather than quietly dropped: +> +> - **`020` single-variable code-mode differential** — still owed. Until it runs, +> the claim in `004`/`094` that `direct` is a comprehension lever rather than a +> reachability fix rests on a source reading of a 2026-07-23 upstream clone. +> It needs a running Codex client, so it belongs to the live phase, not the +> configuration PR. +> - **`024` model-map divergence and warm-cache policy refresh** — the shipped +> suite covers provider-policy divergence and concurrent admission separation; +> these two remain worthwhile local tests. +> +> Everything else in `020`-`025` is covered by +> `tests/codex-tool-discovery-mode.test.ts`, including the `023` backward-compat +> set and the `025` combo compositions. + Phase 2 is complete only when: - [ ] pure resolver tests pass; diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS index b4741547f..1f193ca29 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS @@ -20,7 +20,7 @@ a8ea01e49ccb8c964c56581455e45288651d84a13f12a1ad9fcbdfad41b8641f ./021_catalog_ bd18baa3e16b54fe262435a6ee28c1ca5b389ec8b54433be1b05466559df58b1 ./023_backward_compatibility_tests.md 9542600beb52c958f6065a2c76a18c104c791c01b2035860b9730296ae45b4de ./024_catalog_cache_identity_tests.md 4fdb03fa022bf684257f95752e545bd4e9541b9258361bd9f5da5fc137b822cc ./025_combo_policy_tests.md -00c0913b15591630290c4a08429393570467cdc6799a1433b0eb16532b0630b0 ./029_phase2_exit_gate.md +c6ff0b3c26268a7ad01b95965697fc016eebfe1320ed13407599db7678a05fa9 ./029_phase2_exit_gate.md 1f49005dbac81b2c3f3548d97f9aa32d6c8ada99f533d0fe197bcb047c553464 ./030_phase3_protocol_conformance.md 5e450a7557f7be2fad3f06c3c35288eb8654f156c402f826ef9c9b9703a52bf6 ./031_responses_lite_additional_tools.md 7bc16b20b8abe848726cb662865d542f2a8a19d6acb400a5473b0e9e3bc003e1 ./032_custom_namespace_roundtrip.md diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index b670580d1..da0c38177 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -8,6 +8,7 @@ import { isCursorRoute, resolveConfiguredRoutedToolDiscoveryMode, } from "../src/codex/catalog/tool-discovery"; +import { clearModelCache } from "../src/codex/model-cache"; import { validateConfigCandidate } from "../src/config"; import type { OcxProviderConfig } from "../src/types"; @@ -217,9 +218,14 @@ describe("routed tool-discovery propagation", () => { }); describe("combo tool-discovery derivation", () => { + // Full matrix from devlog 025_combo_policy_tests.md, including the migration rows where a + // member has not resolved a mode yet. it("is conservative: one direct member forces the combo direct", () => { expect(deriveComboToolDiscoveryMode(["deferred", "deferred"])).toBe("deferred"); expect(deriveComboToolDiscoveryMode(["deferred", "direct"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode(["direct", "direct"])).toBe("direct"); + expect(deriveComboToolDiscoveryMode([undefined, "deferred"])).toBe("deferred"); + expect(deriveComboToolDiscoveryMode([undefined, "direct"])).toBe("direct"); expect(deriveComboToolDiscoveryMode([undefined, undefined])).toBe("deferred"); expect(deriveComboToolDiscoveryMode([])).toBe("deferred"); }); @@ -327,19 +333,37 @@ describe("routed tool-discovery backward compatibility", () => { expect(Object.keys(routed ?? {}).sort()).toEqual(EXPECTED_ROUTED_KEYS); }); - it("pins the emitted policy fields on the template path", () => { + it("pins the exact emitted key set on the template path", () => { // The template path runs normalizeRoutedCatalogEntry, which the fallback never calls, so - // it needs its own guard against an internal field reaching the Codex catalog. + // it needs its own guard. This is an ABSOLUTE key set rather than a list of forbidden + // names: an earlier version only rejected the three policy fields plus one sentinel, so + // a stray field under any other name passed straight through it. const normalized = normalizeRoutedCatalogEntry({ slug: "deepseek/glm-5.2" } as never, false, { toolDiscoveryMode: "direct", providerId: "deepseek", cursorRoute: false, }) as Record; - expect(normalized).not.toHaveProperty("toolDiscoveryMode"); - expect(normalized).not.toHaveProperty("tool_discovery_mode"); - expect(normalized).not.toHaveProperty("cursorRoute"); - expect(normalized).not.toHaveProperty("leaked_internal_field"); - expect(Object.keys(normalized).every(key => key === key.toLowerCase())).toBe(true); + expect(Object.keys(normalized).sort()).toEqual([ + "apply_patch_tool_type", + "auto_compact_token_limit", + "comp_hash", + "context_window", + "default_reasoning_summary", + "default_verbosity", + "effective_context_window_percent", + "experimental_supported_tools", + "input_modalities", + "max_context_window", + "slug", + "support_verbosity", + "supports_image_detail_original", + "supports_parallel_tool_calls", + "supports_reasoning_summaries", + "supports_search_tool", + "tool_mode", + "truncation_policy", + "web_search_tool_type", + ]); }); it("keeps the Cursor row shape unchanged with zero configuration", () => { @@ -352,6 +376,43 @@ describe("routed tool-discovery backward compatibility", () => { expect(cursor).not.toHaveProperty("toolDiscoveryMode"); }); + it("does not persist the new fields when reading a pre-field config", () => { + // 023: a config written before these fields existed must round-trip unchanged; a + // materialized default would rewrite every user's file on the next unrelated save. + const legacy = { + defaultProvider: "deepseek", + providers: { deepseek: { adapter: "openai-responses", baseUrl: "https://example.invalid" } }, + }; + const result = validateConfigCandidate(legacy); + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected acceptance"); + const provider = result.config.providers.deepseek as Record; + expect(Object.hasOwn(provider, "routedToolDiscovery")).toBe(false); + expect(Object.hasOwn(provider, "modelRoutedToolDiscovery")).toBe(false); + }); + + it("preserves the fields through an unrelated save (downgrade tolerance)", () => { + // 023: an older binary sees these as unknown provider keys and must carry them through + // `.passthrough()` rather than dropping a newer operator's escape hatch. + const configured = { + defaultProvider: "deepseek", + providers: { + deepseek: { + adapter: "openai-responses", + baseUrl: "https://example.invalid", + routedToolDiscovery: "direct", + modelRoutedToolDiscovery: { "glm-5.2": "deferred" }, + }, + }, + }; + const result = validateConfigCandidate(configured); + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected acceptance"); + const provider = result.config.providers.deepseek as Record; + expect(provider.routedToolDiscovery).toBe("direct"); + expect(provider.modelRoutedToolDiscovery).toEqual({ "glm-5.2": "deferred" }); + }); + it("never serializes the internal policy field onto a routed row", () => { const rows = buildCatalogEntries(null, [], [ { provider: "deepseek", id: "glm-5.2", toolDiscoveryMode: "direct", cursorRoute: false }, @@ -364,46 +425,117 @@ describe("routed tool-discovery backward compatibility", () => { }); describe("routed tool-discovery gather identity", () => { - // Scope note, established by ablation: deleting `rtd`/`mrtd` from - // providerCatalogFingerprint leaves these green, because providerGraphIdentity already - // hashes the whole admitted provider row and refuses the join on its own. These pin the - // ADMISSION INVARIANT (two policies never share one flight), not the fingerprint's field - // list. Do not read them as proof that `rtd`/`mrtd` are load-bearing; they are redundancy. - function gatherConfig(routedToolDiscovery?: "auto" | "deferred" | "direct"): never { + // Ablation matrix (run 2026-08-13), so nobody has to guess what these pin: + // remove rtd/mrtd from providerCatalogFingerprint ............ still green + // neutralize discoveryPolicyIdentity ......................... still green + // neutralize providerGraphIdentity ........................... still green + // neutralize ALL THREE ....................................... 2 of 3 FAIL + // So these are not vacuous — they detect a real flight collision — but no single + // mechanism is what they pin, because the three are redundant by design. The + // fingerprint fields are kept as defense in depth: it is an explicit allow-list whose + // omissions leaked flights twice before (credentials, then reasoningEfforts, both + // reproduced against real routes), and its correctness is NOT what these prove. + function liveGatherConfig( + fetchImpl: typeof globalThis.fetch, + routedToolDiscovery?: "auto" | "deferred" | "direct", + modelRoutedToolDiscovery?: Record, + ): never { return { - defaultProvider: "deepseek", + defaultProvider: "tdlab", providers: { - deepseek: { - adapter: "openai-responses", - baseUrl: "https://example.invalid", - models: ["glm-5.2"], - liveModels: false, + tdlab: { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:59117/v1", + apiKey: "sk-test", + liveModels: true, + // Caller-owned transport executor: the supported injection point (it is excluded + // from provider-graph identity as non-admitted state). + fetch: fetchImpl, + // Keeps the gather hermetic: the destination policy resolves real DNS and + // fail-closes on an unresolvable host, which would short-circuit before the + // latched fetch is ever entered. + allowPrivateNetwork: true, ...(routedToolDiscovery ? { routedToolDiscovery } : {}), + ...(modelRoutedToolDiscovery ? { modelRoutedToolDiscovery } : {}), }, }, } as never; } - it("gathers distinct results for two different policies", async () => { - const deferred = await gatherRoutedModels(gatherConfig()); - const direct = await gatherRoutedModels(gatherConfig("direct")); + // A sequential pair cannot observe flight REUSE at all: a completed flight is removed from + // gatherInflight in its finally block, so two wholly independent gathers also compare + // equal. Observing the join needs real concurrency plus a way to count upstream discovery. + function latchedFetch(): { fetch: typeof globalThis.fetch; release: () => void; calls: () => number } { + let calls = 0; + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const impl = (async () => { + calls += 1; + await gate; + return new Response( + JSON.stringify({ data: [{ id: "glm-5.2" }] }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + }) as unknown as typeof globalThis.fetch; + return { fetch: impl, release, calls: () => calls }; + } - expect(deferred.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("deferred"); - expect(direct.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("direct"); - }); + function mode(models: CatalogModel[]): string | undefined { + return models.find(model => model.id === "glm-5.2")?.toolDiscoveryMode; + } - it("refuses to join one in-flight gather across two different policies", async () => { - // Sequential awaited calls cannot observe flight behavior at all: a completed flight is - // removed from the map immediately (provider-fetch.ts finally-block), so equal results - // would prove nothing. These run CONCURRENTLY so both are genuinely in flight together. - const [deferred, direct] = await Promise.all([ - gatherRoutedModels(gatherConfig("deferred")), - gatherRoutedModels(gatherConfig("direct")), + it("joins one flight for an identical policy but splits for a different one", async () => { + clearModelCache("tdlab"); + const same = latchedFetch(); + const joined = Promise.all([ + gatherRoutedModels(liveGatherConfig(same.fetch, "direct")), + gatherRoutedModels(liveGatherConfig(same.fetch, "direct")), ]); - - // If the two policies shared a flight, one would be served the other's rows. - expect(deferred.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("deferred"); - expect(direct.find(model => model.id === "glm-5.2")?.toolDiscoveryMode).toBe("direct"); + same.release(); + const [first, second] = await joined; + // One upstream discovery for two concurrent identical-policy callers: they joined. + expect(same.calls()).toBe(1); + expect(mode(first)).toBe("direct"); + expect(second).toEqual(first); + + clearModelCache("tdlab"); + const split = latchedFetch(); + const separate = Promise.all([ + gatherRoutedModels(liveGatherConfig(split.fetch, "deferred")), + gatherRoutedModels(liveGatherConfig(split.fetch, "direct")), + ]); + split.release(); + const [asDeferred, asDirect] = await separate; + // Two flights: joining would serve one config the other's catalog. + expect(split.calls()).toBe(2); + expect(mode(asDeferred)).toBe("deferred"); + expect(mode(asDirect)).toBe("direct"); + }); + + it("splits concurrent flights that differ only in the per-model map", async () => { + clearModelCache("tdlab"); + const split = latchedFetch(); + const both = Promise.all([ + gatherRoutedModels(liveGatherConfig(split.fetch, undefined, { "glm-5.2": "direct" })), + gatherRoutedModels(liveGatherConfig(split.fetch, undefined, { "glm-5.2": "deferred" })), + ]); + split.release(); + const [direct, deferred] = await both; + expect(split.calls()).toBe(2); + expect(mode(direct)).toBe("direct"); + expect(mode(deferred)).toBe("deferred"); + }); + + it("re-resolves the policy against a warm model cache", async () => { + clearModelCache("tdlab"); + const warm = latchedFetch(); + const first = gatherRoutedModels(liveGatherConfig(warm.fetch, "deferred")); + warm.release(); + expect(mode(await first)).toBe("deferred"); + + // The model list may now come from the warm cache, but the POLICY must be re-resolved + // from the current config rather than inherited from the cached gather. + expect(mode(await gatherRoutedModels(liveGatherConfig(warm.fetch, "direct")))).toBe("direct"); }); }); From 6963c097a28c5375df2cf7c9c3bf2ca709ff4515 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:49:52 +0900 Subject: [PATCH 07/13] test(codex): real concurrency, combo matrix and compat coverage for routed discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An independent audit of the WP3 work-phase returned FAIL and was right: the previous gather-identity pair could not observe flight behavior at all. A completed flight is removed from `gatherInflight` in its finally block, so two sequential awaited calls compare equal even when nothing is shared. Replaces them with a latched harness that injects the provider's own `fetch` executor and counts upstream discovery, then asserts the real contract: - two concurrent identical-policy callers cause ONE discovery (they joined); - two concurrent differing-policy callers cause TWO (joining would serve one config the other's catalog); - flights differing only in the per-model map also split; - the policy is re-resolved against a warm model cache rather than inherited. Ablation matrix, recorded in the test file so nobody has to guess what these pin: removing `rtd`/`mrtd` from the fingerprint leaves them green, and so does neutralizing `discoveryPolicyIdentity` or `providerGraphIdentity` individually — but neutralizing all three fails 2 of 3. They are not vacuous; they detect a real collision. No single mechanism is what they pin, because the three are redundant by design, and the comment now says exactly that instead of implying fingerprint proof. Also from the audit: - combo matrix completed to all five documented rows (devlog 025), plus an explicit assertion that an all-deferred combo OMITS the key rather than merely not being direct; - backward-compat cases from devlog 023: a pre-field config does not gain persisted fields on read, and a configured pair survives an unrelated save (downgrade tolerance through `.passthrough()`); - two overclaims softened. `structure/03` no longer says `auto` is "byte-for-byte" or that every row shape "agrees" automatically — it now says the shape is asserted per-key on both paths, and that custom-model and trusted `openai-apikey` rows resolve the policy explicitly because they are rebuilt outside `applyProviderConfigHints`. - The reachability claim is relabeled as source-derived rather than executed proof, in the English docs and all five translated locales, since the single-variable code-mode differential (devlog 020) has not been run. Locale sync: `routedToolDiscovery` and `modelRoutedToolDiscovery` are now documented in ko, ja, ru, zh-cn and zh-tw, which AGENTS.md requires so translations do not contradict the English source by omission. Verification: bun x tsc --noEmit clean; 414 pass / 0 fail across 8 suites (47 in the focused file); privacy:scan passed; ablation runs recorded above. --- .../content/docs/ja/reference/configuration/providers.md | 2 +- .../content/docs/ko/reference/configuration/providers.md | 2 +- .../content/docs/reference/configuration/providers.md | 2 +- .../content/docs/ru/reference/configuration/providers.md | 2 +- .../docs/zh-cn/reference/configuration/providers.md | 2 +- .../docs/zh-tw/reference/configuration/providers.md | 2 +- structure/03_catalog-and-subagents.md | 9 ++++++--- 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs-site/src/content/docs/ja/reference/configuration/providers.md b/docs-site/src/content/docs/ja/reference/configuration/providers.md index d93a7a474..0181ea98d 100644 --- a/docs-site/src/content/docs/ja/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ja/reference/configuration/providers.md @@ -95,7 +95,7 @@ account を削除しても mapping は保持され、同じ id を再追加す | `noPenaltyModels?` | `string[]` |存在/周波数ペナルティを拒否するモデル。 | | `noStructuredOutputModels?` | `string[]` | `openai-chat` エンドポイントが `response_format` を拒否する正確なモデル ID。要求モデルが項目と完全一致する場合だけフィールドを省略し、その他の `openai-chat` モデルでは structured-output 変換を維持します。 | | `parallelToolCalls?` | `boolean` |並列ツール呼び出しを切り替えます。 OpenAI Chat はデフォルトでオンになっています。非チャット アダプターは明示的な `true` でのみアドバタイズします。 | -| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | ルーティング行の Codex ツール探索ポリシーです。`auto`（デフォルト）は出荷時の挙動を保ちます。非 Cursor のルーティング行は deferred、Cursor は direct で、Cursor はハードフェンスのため `deferred` を設定しても無視されます。deferred 探索と非互換だと実証された経路にのみ `direct` を使ってください。最初のリクエストに MCP 宣言がすべて載って turn-1 ペイロードが実測で 2.7 倍になり、コードモードでは Codex がどちらでも `tools`/`ALL_TOOLS` グローバルに入れ子ツールを設置するため、本来利用可能なツールの到達性が回復するわけでは**ありません**。ホスト型ウェブ検索（`web_search_tool_type`）とは独立です。 | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | ルーティング行の Codex ツール探索ポリシーです。`auto`（デフォルト）は出荷時の挙動を保ちます。非 Cursor のルーティング行は deferred、Cursor は direct で、Cursor はハードフェンスのため `deferred` を設定しても無視されます。deferred 探索と非互換だと実証された経路にのみ `direct` を使ってください。最初のリクエストに MCP 宣言がすべて載って turn-1 ペイロードが実測で 2.7 倍になり、コードモードでは Codex がどちらでも `tools`/`ALL_TOOLS` グローバルに入れ子ツールを設置するため、本来利用可能なツールの到達性が回復するわけでは**なさそうです**（上流 `codex-rs` のソースを読んだ判断であり、変数を 1 つだけ変えた実行比較ではまだ確認していません）。ホスト型ウェブ検索（`web_search_tool_type`）とは独立です。 | | `modelRoutedToolDiscovery?` | `Record` | `routedToolDiscovery` のモデル単位の上書きです。混在ゲートウェイで非互換な 1 モデルのために兄弟モデルが不利にならないようにします。マッチングは通常のモデルキー規則（完全一致の id、`:` より前のファミリー、大文字小文字を無視）に従い、日付付きの `-YYYYMMDD` 変種はマッチしないため、失敗したモデル id を正確に指定してください。 | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` |正確なプレースホルダー ID、欠落している端末 ID、および（`repairInvalidIds` で）正規の `msg_`/`rs_` 接頭辞を欠く message/reasoning ID に対するダウンストリーム SSE 修復はデフォルトで無効になっています。関数呼び出し ID は決して書き換えられません。組み込み DeepSeek は最後の 2 つをデフォルトで有効にします。 | | `responsesSnapshotRepair?` | `boolean` | デフォルトで無効のクライアント向け修復です。SSE と JSON の Responses ライフサイクルで欠落した status、output、ツールメタデータを補完し、raw 検査と永続化は変更しません。 | diff --git a/docs-site/src/content/docs/ko/reference/configuration/providers.md b/docs-site/src/content/docs/ko/reference/configuration/providers.md index 70c0d8263..7748f1c82 100644 --- a/docs-site/src/content/docs/ko/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ko/reference/configuration/providers.md @@ -95,7 +95,7 @@ managed map을 활성화하면 privacy-safe selector를 만들고, 이후 계정 | `noPenaltyModels?` | `string[]` | presence/frequency penalty를 허용하지 않는 모델입니다. | | `noStructuredOutputModels?` | `string[]` | `openai-chat` 엔드포인트가 `response_format`을 거부하는 정확한 모델 ID입니다. 요청 모델이 항목과 정확히 일치할 때만 필드를 생략하며, 그 외 `openai-chat` 모델에서는 structured-output 변환을 유지합니다. | | `parallelToolCalls?` | `boolean` | 병렬 도구 호출을 켜거나 끕니다. OpenAI Chat은 기본으로 켜져 있고, 비-chat 어댑터는 명시적으로 `true`일 때만 이를 노출합니다. | -| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | 라우팅된 행의 Codex 도구 탐색 정책입니다. `auto`(기본값)는 출하된 동작을 그대로 유지합니다. 비-Cursor 라우팅 행은 deferred, Cursor는 direct이며 Cursor는 하드 펜스라 `deferred`를 설정해도 무시합니다. deferred 탐색과 맞지 않는다고 입증된 경로에만 `direct`를 쓰십시오. 첫 요청에 모든 MCP 선언이 실려 turn-1 페이로드가 측정상 2.7배로 늘고, 코드 모드에서는 Codex가 어느 쪽이든 `tools`/`ALL_TOOLS` 전역에 중첩 도구를 심기 때문에 자격을 갖춘 도구의 도달성을 되살려 주지는 **않습니다**. 호스티드 웹 검색(`web_search_tool_type`)과는 무관합니다. | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | 라우팅된 행의 Codex 도구 탐색 정책입니다. `auto`(기본값)는 출하된 동작을 그대로 유지합니다. 비-Cursor 라우팅 행은 deferred, Cursor는 direct이며 Cursor는 하드 펜스라 `deferred`를 설정해도 무시합니다. deferred 탐색과 맞지 않는다고 입증된 경로에만 `direct`를 쓰십시오. 첫 요청에 모든 MCP 선언이 실려 turn-1 페이로드가 측정상 2.7배로 늘고, 코드 모드에서는 Codex가 어느 쪽이든 `tools`/`ALL_TOOLS` 전역에 중첩 도구를 심기 때문에 자격을 갖춘 도구의 도달성을 되살려 주지는 **않는 것으로 보입니다**(업스트림 `codex-rs` 소스를 읽어 판단했고, 변수를 하나만 바꾼 실행 대조 실험으로는 아직 확인하지 않았습니다). 호스티드 웹 검색(`web_search_tool_type`)과는 무관합니다. | | `modelRoutedToolDiscovery?` | `Record` | `routedToolDiscovery`의 모델별 재정의입니다. 혼합 게이트웨이에서 호환되지 않는 한 모델 때문에 형제 모델까지 손해 보지 않게 합니다. 매칭은 기존 모델 키 규칙(정확한 id, `:` 앞 계열, 대소문자 무시)을 따르며 날짜가 붙은 `-YYYYMMDD` 변형은 매칭하지 않으므로 실패한 모델 id를 정확히 적으십시오. | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | 기본값이 꺼진 downstream SSE 복구입니다. 정확한 자리표시자 id, 누락된 종료 id, 그리고(`repairInvalidIds`) 정규 `msg_`/`rs_` 접두사가 없는 message/reasoning id를 복구합니다. function-call id는 다시 쓰지 않습니다. 내장 DeepSeek은 마지막 두 가지를 기본으로 켭니다. | | `responsesSnapshotRepair?` | `boolean` | 기본값이 꺼진 클라이언트용 복구입니다. SSE와 JSON의 Responses 수명 주기에서 누락된 status, output, 도구 메타데이터를 채우며 raw 검사와 영속화는 변경하지 않습니다. | diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 145361a9d..faf552cad 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -105,7 +105,7 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `noPenaltyModels?` | `string[]` | Models that reject presence/frequency penalties. | | `noStructuredOutputModels?` | `string[]` | Exact model IDs whose `openai-chat` endpoint rejects `response_format`. Only an exact requested-model match omits the field; structured-output translation stays enabled for every other `openai-chat` model. | | `parallelToolCalls?` | `boolean` | Toggle parallel tool calls. OpenAI Chat defaults on; non-chat adapters advertise only on explicit `true`. | -| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | Routed-row Codex tool-discovery policy. `auto` (default) keeps the shipped behavior: deferred for non-Cursor routed rows, direct for Cursor, which is hard-fenced and ignores a configured `deferred`. Set `direct` only for a route proven incompatible with deferred discovery — it embeds every MCP declaration in the first request (a measured 2.7x turn-1 payload cost) and does **not** make an otherwise eligible tool reachable under code mode, where Codex installs nested tools on the `tools`/`ALL_TOOLS` globals either way. Independent of hosted web search (`web_search_tool_type`). | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | Routed-row Codex tool-discovery policy. `auto` (default) keeps the shipped behavior: deferred for non-Cursor routed rows, direct for Cursor, which is hard-fenced and ignores a configured `deferred`. Set `direct` only for a route proven incompatible with deferred discovery — it embeds every MCP declaration in the first request (a measured 2.7x turn-1 payload cost) and does **not** appear to make an otherwise eligible tool reachable under code mode, where Codex installs nested tools on the `tools`/`ALL_TOOLS` globals either way (read from the upstream `codex-rs` source, not yet confirmed by an executed single-variable differential). Independent of hosted web search (`web_search_tool_type`). | | `modelRoutedToolDiscovery?` | `Record` | Per-model override of `routedToolDiscovery`, so one incompatible model on a mixed gateway does not penalize its siblings. Matching follows the usual model-key rules (exact id, family before `:`, case-insensitive); dated `-YYYYMMDD` variants are not matched, so name the exact failing model id. | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | Disabled-by-default downstream SSE repair for exact placeholder ids, missing terminal ids, and (with `repairInvalidIds`) message/reasoning ids missing the canonical `msg_`/`rs_` prefix. Function-call ids are never rewritten. Built-in DeepSeek enables the last two by default. | | `responsesSnapshotRepair?` | `boolean` | Disabled-by-default client-facing repair for sparse Responses lifecycle snapshots in SSE and JSON. Fills missing canonical status, output, and tool metadata while raw inspection and persistence remain unchanged. | diff --git a/docs-site/src/content/docs/ru/reference/configuration/providers.md b/docs-site/src/content/docs/ru/reference/configuration/providers.md index 43608c1dd..1f90765fc 100644 --- a/docs-site/src/content/docs/ru/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ru/reference/configuration/providers.md @@ -108,7 +108,7 @@ cross-route credential fallback не существует. Строки API GPT- | `noPenaltyModels?` | `string[]` | Модели, отвергающие penalty presence/frequency. | | `noStructuredOutputModels?` | `string[]` | Точные идентификаторы моделей, чей endpoint `openai-chat` отклоняет `response_format`. Поле опускается только при точном совпадении запрошенной модели; для остальных моделей `openai-chat` преобразование structured output остаётся включённым. | | `parallelToolCalls?` | `boolean` | Переключатель parallel tool call'ов. Для OpenAI Chat по умолчанию включено; не-chat adapter'ы рекламируют это только при явном `true`. | -| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | Политика обнаружения Codex-инструментов для routed-строк. `auto` (по умолчанию) сохраняет поставляемое поведение: не-Cursor routed-строки — deferred, Cursor — direct, причём Cursor жёстко зафиксирован и игнорирует заданный `deferred`. Указывайте `direct` только для маршрута, несовместимость которого с deferred-обнаружением доказана: он вкладывает все объявления MCP в первый запрос (измеренная плата — 2.7x к turn-1 payload) и **не** возвращает доступность иначе пригодного инструмента в code mode, где Codex в любом случае помещает вложенные инструменты в глобальные `tools`/`ALL_TOOLS`. Независимо от hosted web search (`web_search_tool_type`). | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | Политика обнаружения Codex-инструментов для routed-строк. `auto` (по умолчанию) сохраняет поставляемое поведение: не-Cursor routed-строки — deferred, Cursor — direct, причём Cursor жёстко зафиксирован и игнорирует заданный `deferred`. Указывайте `direct` только для маршрута, несовместимость которого с deferred-обнаружением доказана: он вкладывает все объявления MCP в первый запрос (измеренная плата — 2.7x к turn-1 payload) и, по-видимому, **не** возвращает доступность иначе пригодного инструмента в code mode (вывод сделан по исходникам upstream `codex-rs` и пока не подтверждён исполненным экспериментом с одной переменной), где Codex в любом случае помещает вложенные инструменты в глобальные `tools`/`ALL_TOOLS`. Независимо от hosted web search (`web_search_tool_type`). | | `modelRoutedToolDiscovery?` | `Record` | Переопределение `routedToolDiscovery` для конкретной модели, чтобы одна несовместимая модель на смешанном шлюзе не наказывала соседние. Сопоставление следует обычным правилам ключей моделей (точный id, семейство до `:`, без учёта регистра); варианты с датой `-YYYYMMDD` не сопоставляются, поэтому указывайте точный id проблемной модели. | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | По умолчанию выключенная downstream SSE-repair для exact placeholder-id, отсутствующих terminal-id и (с `repairInvalidIds`) message/reasoning id без канонического префикса `msg_`/`rs_`. Function-call id никогда не переписываются. Встроенный DeepSeek включает последние два по умолчанию. | | `responsesSnapshotRepair?` | `boolean` | По умолчанию выключенная клиентская repair для неполных lifecycle snapshot'ов Responses в SSE и JSON. Добавляет отсутствующие status, output и tool metadata, не меняя raw inspection и persistence. | diff --git a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md index a2abff2c7..6a1fe0f65 100644 --- a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md +++ b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md @@ -95,7 +95,7 @@ selector，而不是分配一个新名称。 | `noPenaltyModels?` | `string[]` | 会拒绝 presence/frequency penalty 的模型。 | | `noStructuredOutputModels?` | `string[]` | `openai-chat` 端点拒绝 `response_format` 的精确模型 ID。仅当请求模型与条目完全匹配时才省略该字段；其他 `openai-chat` 模型仍启用 structured-output 转换。 | | `parallelToolCalls?` | `boolean` | 切换并行工具调用。OpenAI Chat 默认开启；非 chat 适配器只有显式 `true` 时才会声明支持。 | -| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | routed 行的 Codex 工具发现策略。`auto`（默认）保持出厂行为：非 Cursor 的 routed 行为 deferred，Cursor 为 direct，且 Cursor 是硬围栏，即使配置 `deferred` 也会被忽略。只有在已证明与 deferred 发现不兼容的路由上才设置 `direct`：它会把全部 MCP 声明塞进首个请求（实测 turn-1 负载增加 2.7 倍），而且在 code mode 下并**不会**让本来就可用的工具重新可达——Codex 无论哪种模式都会把嵌套工具装到 `tools`/`ALL_TOOLS` 全局上。与托管网页搜索（`web_search_tool_type`）互不相关。 | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | routed 行的 Codex 工具发现策略。`auto`（默认）保持出厂行为：非 Cursor 的 routed 行为 deferred，Cursor 为 direct，且 Cursor 是硬围栏，即使配置 `deferred` 也会被忽略。只有在已证明与 deferred 发现不兼容的路由上才设置 `direct`：它会把全部 MCP 声明塞进首个请求（实测 turn-1 负载增加 2.7 倍），而且在 code mode 下似乎并**不会**让本来就可用的工具重新可达（该结论来自阅读上游 `codex-rs` 源码，尚未通过只改一个变量的实际对照实验验证）——Codex 无论哪种模式都会把嵌套工具装到 `tools`/`ALL_TOOLS` 全局上。与托管网页搜索（`web_search_tool_type`）互不相关。 | | `modelRoutedToolDiscovery?` | `Record` | 按模型覆盖 `routedToolDiscovery`，使混合网关上某个不兼容的模型不会牵连同组其他模型。匹配遵循通常的模型键规则（精确 id、`:` 之前的系列名、忽略大小写）；带日期的 `-YYYYMMDD` 变体不参与匹配，因此请写出确切失败的模型 id。 | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | 默认关闭的下游 SSE 修复，用于精确占位 id、缺失的终止 id，以及（`repairInvalidIds`）缺少规范 `msg_`/`rs_` 前缀的 message/reasoning id。function-call id 永远不会被重写。内置 DeepSeek 默认启用后两项。 | | `responsesSnapshotRepair?` | `boolean` | 默认关闭的客户端修复，用于补全 SSE 与 JSON 中稀疏 Responses 生命周期快照缺失的 status、output 和工具元数据；原始检查与持久化保持不变。 | diff --git a/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md b/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md index 16011fccc..2f931f4b6 100644 --- a/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md +++ b/docs-site/src/content/docs/zh-tw/reference/configuration/providers.md @@ -74,7 +74,7 @@ description: 供應商項目、認證、端點、模型目錄、配額、context | `noPenaltyModels?` | `string[]` | 拒絕 presence/frequency penalty 的模型。 | | `noStructuredOutputModels?` | `string[]` | 其 `openai-chat` 端點拒絕 `response_format` 的精確模型 ID。僅精確符合的請求模型會省略該欄位；structured-output 轉譯對其他每個 `openai-chat` 模型保持啟用。 | | `parallelToolCalls?` | `boolean` | 切換平行工具呼叫。OpenAI Chat 預設開啟；非 chat adapter 僅在明確 `true` 時廣告。 | -| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | routed 列的 Codex 工具探索政策。`auto`（預設）維持出廠行為：非 Cursor 的 routed 列為 deferred，Cursor 為 direct，且 Cursor 屬硬圍籬，即使設定 `deferred` 也會被忽略。只在已證實與 deferred 探索不相容的路由上設定 `direct`：它會把所有 MCP 宣告塞進第一個請求（實測 turn-1 負載增加 2.7 倍），而且在 code mode 下並**不會**讓原本可用的工具重新可達——Codex 無論哪種模式都會把巢狀工具裝到 `tools`/`ALL_TOOLS` 全域上。與代管網頁搜尋（`web_search_tool_type`）彼此獨立。 | +| `routedToolDiscovery?` | `"auto" \| "deferred" \| "direct"` | routed 列的 Codex 工具探索政策。`auto`（預設）維持出廠行為：非 Cursor 的 routed 列為 deferred，Cursor 為 direct，且 Cursor 屬硬圍籬，即使設定 `deferred` 也會被忽略。只在已證實與 deferred 探索不相容的路由上設定 `direct`：它會把所有 MCP 宣告塞進第一個請求（實測 turn-1 負載增加 2.7 倍），而且在 code mode 下似乎並**不會**讓原本可用的工具重新可達（此結論來自閱讀上游 `codex-rs` 原始碼，尚未經由只改一個變數的實際對照實驗驗證）——Codex 無論哪種模式都會把巢狀工具裝到 `tools`/`ALL_TOOLS` 全域上。與代管網頁搜尋（`web_search_tool_type`）彼此獨立。 | | `modelRoutedToolDiscovery?` | `Record` | 依模型覆寫 `routedToolDiscovery`，讓混合閘道上某個不相容的模型不會拖累同組其他模型。比對遵循一般模型鍵規則（精確 id、`:` 之前的系列、忽略大小寫）；帶日期的 `-YYYYMMDD` 變體不會比對，因此請寫出確切失敗的模型 id。 | | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean }` | 預設停用的下游 SSE 修復，用於精確佔位 id 與缺失的終端 id。Function-call id 永不被重寫。 | | `autoToolChoiceOnlyModels?` | `string[]` | 其 `tool_choice` 僅接受 `auto` 或 `none` 的模型；強制選擇被降級。 | diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index 25bfbc9a8..63f8705ec 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -165,9 +165,12 @@ bypasses that sidecar and has no proven deferred path — the row carries the di That per-surface default is now the `auto` case of a resolved policy rather than a hard-coded boolean. `OcxProviderConfig.routedToolDiscovery` and its per-model map `modelRoutedToolDiscovery` let one proven-incompatible route opt into `direct` without moving -any sibling; `auto` reproduces the shipped shape byte-for-byte. Resolution happens once in -`applyProviderConfigHints()` and rides `CatalogModel.toolDiscoveryMode`, so configured, -live-discovered, cached and combo-derived rows agree, and `auto` never reaches serialization. +any sibling. `auto` reproduces the shipped routed-row shape, asserted per-key on both the +template and template-less paths rather than by a whole-catalog diff against a prior build. +Resolution happens once in `applyProviderConfigHints()` and rides +`CatalogModel.toolDiscoveryMode`; `auto` never reaches serialization. Custom-model and trusted +`openai-apikey` rows are rebuilt outside that pass and resolve the policy explicitly, so the +propagation is deliberate at each construction site rather than automatic. Precedence is Cursor hard fence > model override > provider override > auto; a combo resolves conservatively, since one public row cannot vary after target selection. From cdfb6daa9039c3d6480efa22892dcaf075785dbf Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:52:52 +0900 Subject: [PATCH 08/13] docs(devlog): correct the Phase 2 debt record to leave only 020 open The previous revision of 029 listed 024's model-map divergence and warm-cache policy refresh as open debt. They had already landed in the latched-fetch concurrency harness, so the note contradicted the tests sitting beside it. An audit caught the contradiction. Only the 020 single-variable code-mode differential remains open. It needs a running Codex client, and until it runs the direct-is-not-a-reachability-fix claim in 004/094 stays source-derived. The correction is stated in place rather than silently rewritten, so the record shows the debt list was wrong and why. Verification: SHA256SUMS regenerated and fully verified; privacy:scan passed; 47 focused tests pass at this head. --- .../029_phase2_exit_gate.md | 28 +++++++++---------- .../SHA256SUMS | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md index b553dff09..5791ed672 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md @@ -1,22 +1,22 @@ # 029 - Phase 2 exit gate -> **Status 2026-08-13: Phase 2 is NOT fully closed.** The configuration and -> catalog-policy half shipped (see PR "feat(codex): add routed tool-discovery -> compatibility profiles"), but two planned items remain OPEN and are carried as -> explicit debt rather than quietly dropped: +> **Status 2026-08-13: Phase 2 is NOT fully closed — ONE item remains open.** +> The configuration and catalog-policy half shipped (see PR "feat(codex): add +> routed tool-discovery compatibility profiles"). > -> - **`020` single-variable code-mode differential** — still owed. Until it runs, -> the claim in `004`/`094` that `direct` is a comprehension lever rather than a -> reachability fix rests on a source reading of a 2026-07-23 upstream clone. -> It needs a running Codex client, so it belongs to the live phase, not the -> configuration PR. -> - **`024` model-map divergence and warm-cache policy refresh** — the shipped -> suite covers provider-policy divergence and concurrent admission separation; -> these two remain worthwhile local tests. +> - **`020` single-variable code-mode differential** — still owed, and the only +> open item. Until it runs, the claim in `004`/`094` that `direct` is a +> comprehension lever rather than a reachability fix rests on a source reading +> of a 2026-07-23 upstream clone. It needs a running Codex client, so it +> belongs to the live phase, not the configuration PR. > > Everything else in `020`-`025` is covered by -> `tests/codex-tool-discovery-mode.test.ts`, including the `023` backward-compat -> set and the `025` combo compositions. +> `tests/codex-tool-discovery-mode.test.ts`: the `023` backward-compat set, the +> `025` combo compositions, and — as of the latched-fetch concurrency harness — +> the full `024` set including model-map divergence and warm-cache policy +> re-resolution. An earlier revision of this note listed those two `024` cases as +> open; they had already landed, and the correction is recorded here rather than +> silently edited away. Phase 2 is complete only when: diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS index 1f193ca29..a93141f50 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS @@ -20,7 +20,7 @@ a8ea01e49ccb8c964c56581455e45288651d84a13f12a1ad9fcbdfad41b8641f ./021_catalog_ bd18baa3e16b54fe262435a6ee28c1ca5b389ec8b54433be1b05466559df58b1 ./023_backward_compatibility_tests.md 9542600beb52c958f6065a2c76a18c104c791c01b2035860b9730296ae45b4de ./024_catalog_cache_identity_tests.md 4fdb03fa022bf684257f95752e545bd4e9541b9258361bd9f5da5fc137b822cc ./025_combo_policy_tests.md -c6ff0b3c26268a7ad01b95965697fc016eebfe1320ed13407599db7678a05fa9 ./029_phase2_exit_gate.md +587a4b5cdf645ff1d4d43d89eb712d27ec7b4a3f3d8e97a24d7e181d013c706a ./029_phase2_exit_gate.md 1f49005dbac81b2c3f3548d97f9aa32d6c8ada99f533d0fe197bcb047c553464 ./030_phase3_protocol_conformance.md 5e450a7557f7be2fad3f06c3c35288eb8654f156c402f826ef9c9b9703a52bf6 ./031_responses_lite_additional_tools.md 7bc16b20b8abe848726cb662865d542f2a8a19d6acb400a5473b0e9e3bc003e1 ./032_custom_namespace_roundtrip.md From 402fdb28351f58ba81096d93e7f62a2a3d49d8ab Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 21:57:41 +0900 Subject: [PATCH 09/13] feat(config): warn on degraded routed discovery, and close the real WP3 gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second audit round found that my own "closures" were partly overstated. Fixed by doing the work, not by rewording it. Malformed-load warning (devlog 020, previously missing entirely). The schema degrades these fields with `.catch(undefined)` so a typo cannot cost a user their providers or credentials, but silent degradation is its own failure: an operator whose emergency escape hatch was dropped would never learn it is inactive — the #1529 observability failure at smaller scope. Adds `warnDegradedRoutedToolDiscoveryForLoad`, wired into both load paths beside the existing `retryOn429` sanitizer and following its conventions: runs before schema validation, redacts secret-shaped provider/model names, and logs only the received TYPE, never the value, since provider config can hold secrets. Backward compatibility, previously claimed but not proven. The old tests called `validateConfigCandidate()`, which cannot demonstrate a file-level promise. They now drive the real on-disk round trip through `loadConfig`/`saveConfig` against a temp `OPENCODEX_HOME`: - a pre-field config gains no persisted fields on read, and an unrelated save does not introduce them; - a config carrying both fields survives an unrelated save, which is the `.passthrough()` downgrade contract. Ablation: with the fields removed from the schema AND `.passthrough()` switched to `.strict()`, the downgrade test fails. It is not vacuous. Combo alias coverage (devlog 025), which the audit correctly refused to let me defer since it sits inside WP3's declared combo scope. A direct policy is now asserted through every alias shape — default `combo/`, bare, slashed, and explicit native alias — because the alias is what Codex sees, and a shape-dependent policy is exactly the defect class the unified Cursor fence already had to fix once. 029 exit gate corrected. It had been edited to claim the `020` differential was the only open item. That was false: the `023` prior-build comparison and the `025` forcing-member diagnostic remain open too. The retracted claim is left visible in the note on purpose — this unit exists because a plan asserted more verification than it had, and quietly editing that away would repeat it. Verification: bun x tsc --noEmit clean; 427 pass / 0 fail across 9 suites (49 in the focused file); privacy:scan passed; SHA256SUMS regenerated and verified. --- .../029_phase2_exit_gate.md | 42 ++-- .../SHA256SUMS | 2 +- src/config.ts | 41 ++++ tests/codex-tool-discovery-mode.test.ts | 208 ++++++++++++++---- 4 files changed, 235 insertions(+), 58 deletions(-) diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md index 5791ed672..926580f0e 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md @@ -1,22 +1,34 @@ # 029 - Phase 2 exit gate -> **Status 2026-08-13: Phase 2 is NOT fully closed — ONE item remains open.** -> The configuration and catalog-policy half shipped (see PR "feat(codex): add -> routed tool-discovery compatibility profiles"). +> **Status 2026-08-13: Phase 2 is NOT closed.** The configuration and +> catalog-policy half shipped (see PR "feat(codex): add routed tool-discovery +> compatibility profiles"), and an earlier revision of this note claimed the +> `020` differential was the only thing left. An independent audit disproved +> that. The honest remaining list: > -> - **`020` single-variable code-mode differential** — still owed, and the only -> open item. Until it runs, the claim in `004`/`094` that `direct` is a -> comprehension lever rather than a reachability fix rests on a source reading -> of a 2026-07-23 upstream clone. It needs a running Codex client, so it -> belongs to the live phase, not the configuration PR. +> - **`020` single-variable code-mode differential** — still owed. Until it +> runs, the claim in `004`/`094` that `direct` is a comprehension lever rather +> than a reachability fix rests on a source reading of a 2026-07-23 upstream +> clone. It needs a running Codex client, so it belongs to the live phase +> rather than the configuration PR. The docs now label that conclusion +> source-derived rather than proven. +> - **`023` zero-config comparison against a real prior build** — the suite pins +> the exact emitted key set on both construction paths, which is strong +> regression coverage, but it is not the documented normalized diff of a +> current-`dev` catalog against a patched one. +> - **`025` forcing-member diagnostic** — the combo explain surface does not yet +> name which member forced `direct`. Derivation itself is covered. > -> Everything else in `020`-`025` is covered by -> `tests/codex-tool-discovery-mode.test.ts`: the `023` backward-compat set, the -> `025` combo compositions, and — as of the latched-fetch concurrency harness — -> the full `024` set including model-map divergence and warm-cache policy -> re-resolution. An earlier revision of this note listed those two `024` cases as -> open; they had already landed, and the correction is recorded here rather than -> silently edited away. +> Closed since that earlier revision, with ablation evidence recorded in +> `tests/codex-tool-discovery-mode.test.ts`: the `024` concurrency set (a +> latched provider `fetch` proves identical policies JOIN one flight and +> differing policies SPLIT, including model-map divergence and warm-cache +> re-resolution), the full `025` five-row matrix plus deferred-key omission and +> every alias shape (bare, slashed, native), the `023` on-disk load/save +> round trip and downgrade preservation, and the `020` malformed-load warning. +> +> The retracted claim is left visible on purpose: this unit exists because a +> plan asserted more verification than it had. Phase 2 is complete only when: diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS index a93141f50..88ae5b191 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS @@ -20,7 +20,7 @@ a8ea01e49ccb8c964c56581455e45288651d84a13f12a1ad9fcbdfad41b8641f ./021_catalog_ bd18baa3e16b54fe262435a6ee28c1ca5b389ec8b54433be1b05466559df58b1 ./023_backward_compatibility_tests.md 9542600beb52c958f6065a2c76a18c104c791c01b2035860b9730296ae45b4de ./024_catalog_cache_identity_tests.md 4fdb03fa022bf684257f95752e545bd4e9541b9258361bd9f5da5fc137b822cc ./025_combo_policy_tests.md -587a4b5cdf645ff1d4d43d89eb712d27ec7b4a3f3d8e97a24d7e181d013c706a ./029_phase2_exit_gate.md +03c3e29c92e3b406baea4719910570f30516553347d741b7d2fc95a425e57f32 ./029_phase2_exit_gate.md 1f49005dbac81b2c3f3548d97f9aa32d6c8ada99f533d0fe197bcb047c553464 ./030_phase3_protocol_conformance.md 5e450a7557f7be2fad3f06c3c35288eb8654f156c402f826ef9c9b9703a52bf6 ./031_responses_lite_additional_tools.md 7bc16b20b8abe848726cb662865d542f2a8a19d6acb400a5473b0e9e3bc003e1 ./032_custom_namespace_roundtrip.md diff --git a/src/config.ts b/src/config.ts index f026ec124..4905dc7b4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1607,6 +1607,45 @@ function warnDegradedStreamMode(rawParsed: unknown, validated: OcxConfig): void * provider/key behind a default config. Invalid fields are dropped with a warning; the management * write boundary still rejects invalid policies explicitly. */ +/** + * Warn when a hand-edited routed tool-discovery policy is dropped on load. + * + * The schema degrades these fields with `.catch(undefined)` so a typo cannot cost a user + * their providers or credentials, but silent degradation is its own failure: an operator + * whose emergency escape hatch was discarded by a typo would never learn it is inactive + * (devlog 014 — the same observability failure as #1529, at smaller scope). The write + * boundary still rejects these values outright; this only covers the tolerant load path. + */ +function warnDegradedRoutedToolDiscoveryForLoad(parsed: unknown): void { + if (!parsed || typeof parsed !== "object") return; + const providers = (parsed as Record).providers; + if (!providers || typeof providers !== "object" || Array.isArray(providers)) return; + for (const [name, provider] of Object.entries(providers as Record)) { + if (!provider || typeof provider !== "object" || Array.isArray(provider)) continue; + // Runs BEFORE schema validation, so the provider name is untrusted: redact + // secret-shaped names and JSON-escape control characters before warning. + const safeProviderName = JSON.stringify(redactSecretString(name)); + const p = provider as Record; + const mode = p.routedToolDiscovery; + // Log only the received type, never the value: provider config can hold secrets. + if (mode !== undefined && !isRoutedToolDiscoveryMode(mode)) { + console.warn(`⚠️ config.json providers.${safeProviderName}.routedToolDiscovery (${typeof mode}) is invalid — ignoring the policy`); + } + const map = p.modelRoutedToolDiscovery; + if (map === undefined) continue; + if (!map || typeof map !== "object" || Array.isArray(map)) { + console.warn(`⚠️ config.json providers.${safeProviderName}.modelRoutedToolDiscovery (${typeof map}) is invalid — ignoring the map`); + continue; + } + for (const [modelId, value] of Object.entries(map as Record)) { + if (isRoutedToolDiscoveryMode(value)) continue; + const safeModelId = JSON.stringify(redactSecretString(modelId)); + console.warn(`⚠️ config.json providers.${safeProviderName}.modelRoutedToolDiscovery.${safeModelId} (${typeof value}) is invalid — ignoring the whole map`); + break; + } + } +} + function sanitizeRetryOn429ForLoad(parsed: unknown): void { if (!parsed || typeof parsed !== "object") return; const root = parsed as Record; @@ -2027,6 +2066,7 @@ export function loadConfig(): OcxConfig { const raw = readFileSync(configPath, "utf-8").replace(/^\uFEFF/, ""); const parsed = JSON.parse(raw); sanitizeRetryOn429ForLoad(parsed); + warnDegradedRoutedToolDiscoveryForLoad(parsed); sanitizeModelCostsForLoad(parsed); const result = configSchema.safeParse(parsed); if (result.success) { @@ -2418,6 +2458,7 @@ function configDiagnosticsFromRaw(raw: string): ConfigDiagnostics { // schema and send the caller a default-config fallback (the config command could then // persist that fallback over the user's providers/keys). sanitizeRetryOn429ForLoad(parsed); + warnDegradedRoutedToolDiscoveryForLoad(parsed); sanitizeModelCostsForLoad(parsed); const result = configSchema.safeParse(parsed); if (result.success) { diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index da0c38177..5309afaef 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -1,4 +1,7 @@ import { describe, expect, it } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { applyProviderConfigHints, buildCatalogEntries, normalizeRoutedCatalogEntry } from "../src/codex/catalog"; import { gatherRoutedModels } from "../src/codex/catalog"; import { deriveComboCatalogModel } from "../src/codex/catalog/aggregation"; @@ -9,6 +12,7 @@ import { resolveConfiguredRoutedToolDiscoveryMode, } from "../src/codex/catalog/tool-discovery"; import { clearModelCache } from "../src/codex/model-cache"; +import { getConfigPath, loadConfig, saveConfig } from "../src/config"; import { validateConfigCandidate } from "../src/config"; import type { OcxProviderConfig } from "../src/types"; @@ -268,16 +272,53 @@ describe("combo tool-discovery derivation", () => { expect(combo).not.toHaveProperty("toolDiscoveryMode"); }); - it("keeps a direct combo direct through catalog serialization under both alias shapes", () => { - for (const slug of ["combo/mix", "mix"]) { - const rows = buildCatalogEntries(null, [], [ - { provider: "combo", id: slug.includes("/") ? slug.slice(slug.indexOf("/") + 1) : slug, toolDiscoveryMode: "direct" }, - ]); - const row = rows.find(entry => typeof entry.slug === "string" && entry.slug.endsWith(slug.replace("combo/", ""))); - expect(row?.supports_search_tool).toBe(false); - expect(row?.web_search_tool_type).toBe("text_and_image"); + // 025 alias coverage: the policy must survive every combo alias SHAPE, because the alias + // is what Codex sees and a shape-dependent policy is the class of defect the unified + // Cursor fence already had to fix once. + it("carries a direct combo policy through every alias shape", () => { + const members: CatalogModel[] = [ + { id: "a", provider: "p1", contextWindow: 200_000, maxInputTokens: 200_000, inputModalities: ["text"], toolDiscoveryMode: "deferred" }, + { id: "b", provider: "p2", contextWindow: 200_000, maxInputTokens: 200_000, inputModalities: ["text"], toolDiscoveryMode: "direct" }, + ]; + const targets = [{ provider: "p1", model: "a" }, { provider: "p2", model: "b" }]; + + for (const alias of [undefined, "bare-combo", "vendor/slashed-combo"]) { + const combo = deriveComboCatalogModel( + "mixed", + { targets, strategy: "failover", ...(alias ? { alias } : {}) } as never, + members, + ); + expect(combo?.toolDiscoveryMode).toBe("direct"); + + const slug = alias ?? "combo/mixed"; + const rows = buildCatalogEntries(null, [], [{ ...combo!, ...(alias ? { alias } : {}) }], undefined, false, "default", new Set(alias ? [alias] : [])); + const row = rows.find(entry => entry.slug === slug) ?? rows[0]!; + expect(row.supports_search_tool).toBe(false); + // Hosted search stays independent of the discovery policy on every shape. + expect(row.web_search_tool_type).toBe("text_and_image"); + expect(row.tool_mode).toBe("code_mode_only"); } }); + + it("carries a direct combo policy through an explicit native alias", () => { + const combo = deriveComboCatalogModel( + "mixed", + { targets: [{ provider: "p1", model: "a" }, { provider: "p2", model: "b" }], strategy: "failover", alias: "gpt-5.6-sol", nativeAlias: true } as never, + [ + { id: "a", provider: "p1", contextWindow: 200_000, toolDiscoveryMode: "deferred" }, + { id: "b", provider: "p2", contextWindow: 200_000, toolDiscoveryMode: "direct" }, + ], + ); + expect(combo?.toolDiscoveryMode).toBe("direct"); + expect(combo?.nativeAlias).toBe(true); + + const rows = buildCatalogEntries(null, [], [combo!], undefined, false, "default", new Set(["gpt-5.6-sol"])); + const row = rows.find(entry => entry.slug === "gpt-5.6-sol") ?? rows[0]!; + // A combo that takes over a native slug is still a ROUTED row and must carry the + // resolved policy rather than inheriting native catalog defaults. + expect(row.supports_search_tool).toBe(false); + expect(row.tool_mode).toBe("code_mode_only"); + }); }); describe("routed tool-discovery backward compatibility", () => { @@ -376,41 +417,124 @@ describe("routed tool-discovery backward compatibility", () => { expect(cursor).not.toHaveProperty("toolDiscoveryMode"); }); - it("does not persist the new fields when reading a pre-field config", () => { - // 023: a config written before these fields existed must round-trip unchanged; a - // materialized default would rewrite every user's file on the next unrelated save. - const legacy = { - defaultProvider: "deepseek", - providers: { deepseek: { adapter: "openai-responses", baseUrl: "https://example.invalid" } }, - }; - const result = validateConfigCandidate(legacy); - expect(result.ok).toBe(true); - if (!result.ok) throw new Error("expected acceptance"); - const provider = result.config.providers.deepseek as Record; - expect(Object.hasOwn(provider, "routedToolDiscovery")).toBe(false); - expect(Object.hasOwn(provider, "modelRoutedToolDiscovery")).toBe(false); - }); - - it("preserves the fields through an unrelated save (downgrade tolerance)", () => { - // 023: an older binary sees these as unknown provider keys and must carry them through - // `.passthrough()` rather than dropping a newer operator's escape hatch. - const configured = { - defaultProvider: "deepseek", - providers: { - deepseek: { - adapter: "openai-responses", - baseUrl: "https://example.invalid", - routedToolDiscovery: "direct", - modelRoutedToolDiscovery: { "glm-5.2": "deferred" }, + // These drive the REAL on-disk round trip (loadConfig/saveConfig against a temp + // OPENCODEX_HOME), not just validateConfigCandidate. Validating a candidate object cannot + // prove the file-level promises in 023: that a pre-field config gains nothing on read, and + // that an unrelated save preserves fields a newer binary wrote. + function withTempHome(run: () => T): T { + const previous = process.env.OPENCODEX_HOME; + const home = mkdtempSync(join(tmpdir(), "ocx-tool-discovery-compat-")); + process.env.OPENCODEX_HOME = home; + try { return run(); } + finally { + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + rmSync(home, { recursive: true, force: true }); + } + } + + it("warns instead of silently degrading a malformed policy on load", () => { + // 020: the load path is tolerant on purpose, but an operator whose emergency escape + // hatch was dropped by a typo must be told — silence would repeat the #1529 + // observability failure at smaller scope. + withTempHome(() => { + const base = { + port: 10100, + defaultProvider: "test", + providers: { + test: { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:1/v1", + apiKey: "k", + allowPrivateNetwork: true, + routedToolDiscovery: "eager", + modelRoutedToolDiscovery: { "glm-5.2": "nope" }, + }, }, - }, - }; - const result = validateConfigCandidate(configured); - expect(result.ok).toBe(true); - if (!result.ok) throw new Error("expected acceptance"); - const provider = result.config.providers.deepseek as Record; - expect(provider.routedToolDiscovery).toBe("direct"); - expect(provider.modelRoutedToolDiscovery).toEqual({ "glm-5.2": "deferred" }); + }; + saveConfig(base as never); + writeFileSync(getConfigPath(), `${JSON.stringify(base, null, 2)}\n`); + + const warnings: string[] = []; + const original = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + let loaded; + try { loaded = loadConfig(); } + finally { console.warn = original; } + + // Degraded, not fatal: the provider and its credential survive. + const provider = loaded.providers.test as unknown as Record; + expect(provider.apiKey).toBe("k"); + expect(provider.routedToolDiscovery).toBeUndefined(); + expect(provider.modelRoutedToolDiscovery).toBeUndefined(); + + const joined = warnings.join("\n"); + expect(joined).toContain("routedToolDiscovery"); + expect(joined).toContain("modelRoutedToolDiscovery"); + // Never echo the offending value; provider config can hold secrets. + expect(joined).not.toContain("eager"); + expect(joined).not.toContain("nope"); + }); + }); + + it("does not materialize the new fields when loading a pre-field config file", () => { + withTempHome(() => { + // A config written before these fields existed. A materialized default here would + // rewrite every existing user's file on their next unrelated save. + const legacy = { + port: 10100, + defaultProvider: "test", + providers: { + test: { adapter: "openai-chat", baseUrl: "http://127.0.0.1:1/v1", apiKey: "k", allowPrivateNetwork: true }, + }, + }; + saveConfig(legacy as never); + writeFileSync(getConfigPath(), `${JSON.stringify(legacy, null, 2)}\n`); + + const loaded = loadConfig(); + const provider = loaded.providers.test as unknown as Record; + expect(Object.hasOwn(provider, "routedToolDiscovery")).toBe(false); + expect(Object.hasOwn(provider, "modelRoutedToolDiscovery")).toBe(false); + + // A no-op read followed by an unrelated save must not introduce them either. + saveConfig({ ...loaded, port: 10101 } as never); + const onDisk = JSON.parse(readFileSync(getConfigPath(), "utf8")) as Record; + const savedProvider = (onDisk.providers as Record>).test; + expect(Object.hasOwn(savedProvider, "routedToolDiscovery")).toBe(false); + expect(Object.hasOwn(savedProvider, "modelRoutedToolDiscovery")).toBe(false); + }); + }); + + it("preserves the fields through an unrelated save, the downgrade contract", () => { + withTempHome(() => { + // Written on disk as UNKNOWN provider keys — the shape an older binary sees. The + // schema is .passthrough(), so an unrelated save must carry them through rather than + // dropping a newer operator's escape hatch. + const configured = { + port: 10100, + defaultProvider: "test", + providers: { + test: { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:1/v1", + apiKey: "k", + allowPrivateNetwork: true, + routedToolDiscovery: "direct", + modelRoutedToolDiscovery: { "glm-5.2": "deferred" }, + }, + }, + }; + saveConfig(configured as never); + writeFileSync(getConfigPath(), `${JSON.stringify(configured, null, 2)}\n`); + + const loaded = loadConfig(); + saveConfig({ ...loaded, port: 10102 } as never); + + const onDisk = JSON.parse(readFileSync(getConfigPath(), "utf8")) as Record; + const savedProvider = (onDisk.providers as Record>).test; + expect(savedProvider.routedToolDiscovery).toBe("direct"); + expect(savedProvider.modelRoutedToolDiscovery).toEqual({ "glm-5.2": "deferred" }); + }); }); it("never serializes the internal policy field onto a routed row", () => { From 20109fb90221c7aaa5ba00b4fe5b95ed264df207 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 22:03:52 +0900 Subject: [PATCH 10/13] test(codex): make the combo alias tests actually pin alias propagation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit caught that my alias tests could not fail. They re-attached `alias` onto the derived model before serializing, and fell back to `rows[0]` when the expected slug was missing — so deleting alias propagation from `deriveComboCatalogModel()` left them green. That is the same false-confidence shape as the gather test this work-phase already replaced once. Now they build rows from the derived object only, assert `combo.alias` directly, and require a row at the EXACT expected slug with no fallback. Ablation: removing `...(combo.alias ? { alias: combo.alias } : {})` from `deriveComboCatalogModel()` turns both alias tests red. Also qualifies the last two categorical statements of the reachability conclusion. The public docs were already labeled source-derived, but `structure/03` and the `tool-discovery.ts` header still asserted it flatly, contradicting `029`'s own record that devlog `020` still owes the executed single-variable differential. Both now say the conclusion comes from reading the upstream source and is well-grounded but unproven. `029` no longer claims alias coverage without qualification: it states the shapes are pinned to exact emitted slugs so deleting alias propagation turns them red. Verification: bun x tsc --noEmit clean; 427 pass / 0 fail across 9 suites; privacy:scan passed; alias mutant confirmed red; SHA256SUMS regenerated. --- .../029_phase2_exit_gate.md | 3 ++- .../SHA256SUMS | 2 +- src/codex/catalog/tool-discovery.ts | 6 +++-- structure/03_catalog-and-subagents.md | 6 +++-- tests/codex-tool-discovery-mode.test.ts | 25 +++++++++++++------ 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md index 926580f0e..0b0e52e53 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md @@ -24,7 +24,8 @@ > latched provider `fetch` proves identical policies JOIN one flight and > differing policies SPLIT, including model-map divergence and warm-cache > re-resolution), the full `025` five-row matrix plus deferred-key omission and -> every alias shape (bare, slashed, native), the `023` on-disk load/save +> every alias shape (bare, slashed, native), each pinned to an exact emitted slug so +> deleting alias propagation turns them red, the `023` on-disk load/save > round trip and downgrade preservation, and the `020` malformed-load warning. > > The retracted claim is left visible on purpose: this unit exists because a diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS index 88ae5b191..da23b90b4 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS @@ -20,7 +20,7 @@ a8ea01e49ccb8c964c56581455e45288651d84a13f12a1ad9fcbdfad41b8641f ./021_catalog_ bd18baa3e16b54fe262435a6ee28c1ca5b389ec8b54433be1b05466559df58b1 ./023_backward_compatibility_tests.md 9542600beb52c958f6065a2c76a18c104c791c01b2035860b9730296ae45b4de ./024_catalog_cache_identity_tests.md 4fdb03fa022bf684257f95752e545bd4e9541b9258361bd9f5da5fc137b822cc ./025_combo_policy_tests.md -03c3e29c92e3b406baea4719910570f30516553347d741b7d2fc95a425e57f32 ./029_phase2_exit_gate.md +df3bb72d6b6bc09c371c9e33f591a8a787055e74e55c2fe0d5655ba2d21d3576 ./029_phase2_exit_gate.md 1f49005dbac81b2c3f3548d97f9aa32d6c8ada99f533d0fe197bcb047c553464 ./030_phase3_protocol_conformance.md 5e450a7557f7be2fad3f06c3c35288eb8654f156c402f826ef9c9b9703a52bf6 ./031_responses_lite_additional_tools.md 7bc16b20b8abe848726cb662865d542f2a8a19d6acb400a5473b0e9e3bc003e1 ./032_custom_namespace_roundtrip.md diff --git a/src/codex/catalog/tool-discovery.ts b/src/codex/catalog/tool-discovery.ts index 8fd65a580..980c6e5a4 100644 --- a/src/codex/catalog/tool-discovery.ts +++ b/src/codex/catalog/tool-discovery.ts @@ -13,8 +13,10 @@ import { modelRecordValue } from "../../reasoning-effort"; * declaration in `exec.description`, the measured 96,699 → 258,929 char turn-1 regression * behind PR #1596 — plus `tool_search` construction and the deferred-guidance text. * - * So `direct` is a compatibility/comprehension lever with a payload cost, NOT a - * reachability fix. It also cannot repair a tool removed by `direct_only_tool_namespaces`, + * So `direct` reads as a compatibility/comprehension lever with a payload cost rather than a + * reachability fix. That conclusion comes from the upstream source above, not from an executed + * single-variable differential — devlog `020` still owes that run, so treat it as well-grounded + * but unproven. It also cannot repair a tool removed by `direct_only_tool_namespaces`, * `excluded_tool_namespaces`, or MCP/App policy filtering, all of which are independent of * this flag. Full analysis and citations: * devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md. diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index 63f8705ec..be48d4e79 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -183,8 +183,10 @@ with template availability (unresolved P2 on #1596). Scope of the reachability claim: it holds for an ELIGIBLE MCP tool. `direct_only_tool_namespaces`, `excluded_tool_namespaces`, and MCP/App policy filtering remove tools independently of this flag, -and no override repairs them. So `direct` is a comprehension/compatibility lever with a payload -cost, not a reachability fix. Analysis and citations: +and no override repairs them. On that reading `direct` is a comprehension/compatibility lever with +a payload cost rather than a reachability fix — a conclusion drawn from the upstream `codex-rs` +source, NOT yet from an executed single-variable differential (devlog `020` still owes that run, +and `029` records it as open). Analysis and citations: `devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md`. [Decision Log] diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index 5309afaef..a6a8dfcaa 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -289,14 +289,21 @@ describe("combo tool-discovery derivation", () => { members, ); expect(combo?.toolDiscoveryMode).toBe("direct"); + // The derived model must CARRY the alias; re-attaching it here would let a broken + // derivation pass, which is exactly how an earlier revision of this test fooled itself. + expect(combo?.alias).toBe(alias as never); const slug = alias ?? "combo/mixed"; - const rows = buildCatalogEntries(null, [], [{ ...combo!, ...(alias ? { alias } : {}) }], undefined, false, "default", new Set(alias ? [alias] : [])); - const row = rows.find(entry => entry.slug === slug) ?? rows[0]!; - expect(row.supports_search_tool).toBe(false); + const rows = buildCatalogEntries(null, [], [combo!], undefined, false, "default", new Set(alias ? [alias] : [])); + // No `?? rows[0]` fallback: the row must exist at the EXACT expected slug, or the + // alias shape is not actually being exercised. + const row = rows.find(entry => entry.slug === slug); + expect(row).toBeDefined(); + expect(row!.slug).toBe(slug); + expect(row!.supports_search_tool).toBe(false); // Hosted search stays independent of the discovery policy on every shape. - expect(row.web_search_tool_type).toBe("text_and_image"); - expect(row.tool_mode).toBe("code_mode_only"); + expect(row!.web_search_tool_type).toBe("text_and_image"); + expect(row!.tool_mode).toBe("code_mode_only"); } }); @@ -310,14 +317,16 @@ describe("combo tool-discovery derivation", () => { ], ); expect(combo?.toolDiscoveryMode).toBe("direct"); + expect(combo?.alias).toBe("gpt-5.6-sol"); expect(combo?.nativeAlias).toBe(true); const rows = buildCatalogEntries(null, [], [combo!], undefined, false, "default", new Set(["gpt-5.6-sol"])); - const row = rows.find(entry => entry.slug === "gpt-5.6-sol") ?? rows[0]!; + const row = rows.find(entry => entry.slug === "gpt-5.6-sol"); + expect(row).toBeDefined(); // A combo that takes over a native slug is still a ROUTED row and must carry the // resolved policy rather than inheriting native catalog defaults. - expect(row.supports_search_tool).toBe(false); - expect(row.tool_mode).toBe("code_mode_only"); + expect(row!.supports_search_tool).toBe(false); + expect(row!.tool_mode).toBe("code_mode_only"); }); }); From 579429d60254bb3ebbcd4dbcd71155975c4f4cd8 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 22:08:32 +0900 Subject: [PATCH 11/13] fix(config): warn when a blank model key drops the routed discovery map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final audit sweep found a real functional gap behind the warning added last commit. `modelRoutedToolDiscovery` keys go through `z.string().min(1)`, so a hand-edited `{"": "direct"}` fails the key check and takes the ENTIRE map with it — but the warning helper only inspected values, so the map vanished without a word. The write boundary already rejected it; only the tolerant load path was silent. Reproduced, then fixed with its own warning and a disk-load test that asserts the provider and credential survive, the map is dropped, and the warning names the blank key. Ablation: removing the blank-key branch turns the test red. Wording sweep, all from the same audit: - `tool-discovery.ts` opening sentence still stated the reachability conclusion categorically while its own later paragraph called it unproven. Now "appears not to decide". - `000_master_plan.md` and `094_landing_verification_pass.md` led with the categorical claim; both now say a source reading indicates it and name the differential `020` still owes, matching `029`. - `029` no longer implies the malformed-load warning is fully closed without the blank-key case; it names it. - Renamed "emits byte-identical routed rows with zero configuration" to "emits identical rows whether the deferred default is implicit or explicit". The test compares two outputs of the SAME build, so the old name implied the prior-build comparison that `029` correctly records as still open. Verification: bun x tsc --noEmit clean; 428 pass / 0 fail across 9 suites (50 in the focused file); privacy:scan passed; blank-key mutant confirmed red; SHA256SUMS regenerated. --- .../000_master_plan.md | 8 ++-- .../029_phase2_exit_gate.md | 3 +- .../094_landing_verification_pass.md | 7 ++-- .../SHA256SUMS | 6 +-- src/codex/catalog/tool-discovery.ts | 2 +- src/config.ts | 6 +++ tests/codex-tool-discovery-mode.test.ts | 38 ++++++++++++++++++- 7 files changed, 58 insertions(+), 12 deletions(-) diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md b/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md index 5f27a142a..670c49242 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md @@ -12,9 +12,11 @@ Base change: PR #1596, `fix(codex): restore deferred tool discovery for non-Curs > 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. +> consequential being that a source reading indicates an **eligible** MCP tool stays +> callable in **both** discovery modes under code mode — so for those tools `direct` +> reads as a comprehension/compatibility lever with a payload cost rather than a +> reachability fix. That conclusion is not yet backed by an executed single-variable +> differential; `020` still owes it and `029` records it as open. > "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. diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md index 0b0e52e53..1e2711fab 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/029_phase2_exit_gate.md @@ -26,7 +26,8 @@ > re-resolution), the full `025` five-row matrix plus deferred-key omission and > every alias shape (bare, slashed, native), each pinned to an exact emitted slug so > deleting alias propagation turns them red, the `023` on-disk load/save -> round trip and downgrade preservation, and the `020` malformed-load warning. +> round trip and downgrade preservation, and the `020` malformed-load warning, including the blank-model-key case where the +> schema drops the entire map. > > The retracted claim is left visible on purpose: this unit exists because a > plan asserted more verification than it had. diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md b/devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md index 486a5f031..2d826004c 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/094_landing_verification_pass.md @@ -157,9 +157,10 @@ executed differential rather than on a source reading. The expected delta is `exec.description` content/size **plus** `tool_search` construction and the deferred-guidance text — not `exec.description` alone. -This reframes the override honestly: for an eligible tool under code mode it is a -*model-comprehension* lever (the model sees full schemas inline instead of having -to consult `ALL_TOOLS`), not a *reachability* lever. `010`'s non-goal list already +This reframes the override honestly: on the source reading above, for an eligible tool +under code mode it is a *model-comprehension* lever (the model sees full schemas inline +instead of having to consult `ALL_TOOLS`) rather than a *reachability* lever — still +pending the executed differential `020` owes. `010`'s non-goal list already says "no claim that `direct` is cheaper or preferred"; it must also say direct is not a reachability fix for eligible tools under code mode. `044`'s weak-model fallback rationale is the honest use case. diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS index da23b90b4..b36ca3eda 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS @@ -1,4 +1,4 @@ -4e5dd4520224a37fddfd8fb0dc532cd62c9daa9f703d2037c2875a44f8073b58 ./000_master_plan.md +0c64b276bd741ec3f12002df6df99b2ca559602355cd1e882c633267687d0332 ./000_master_plan.md 7e853a53e344412cc4aa2fcbe64b140195e9825a1988c5e245204c19a8f22449 ./001_verified_dev_baseline.md 32d7a7ffff41bd0c036cef153f53047ce416624032dae9dff43e88ff24d653fc ./002_incident_history_1522_1529_1596.md 343ee877dde67978b7ffd6a1e6f7825751c5fb113fd51dead6554036f92fa904 ./003_current_code_map.md @@ -20,7 +20,7 @@ a8ea01e49ccb8c964c56581455e45288651d84a13f12a1ad9fcbdfad41b8641f ./021_catalog_ bd18baa3e16b54fe262435a6ee28c1ca5b389ec8b54433be1b05466559df58b1 ./023_backward_compatibility_tests.md 9542600beb52c958f6065a2c76a18c104c791c01b2035860b9730296ae45b4de ./024_catalog_cache_identity_tests.md 4fdb03fa022bf684257f95752e545bd4e9541b9258361bd9f5da5fc137b822cc ./025_combo_policy_tests.md -df3bb72d6b6bc09c371c9e33f591a8a787055e74e55c2fe0d5655ba2d21d3576 ./029_phase2_exit_gate.md +ac0cd8816fcf2f032d02798f25383220803084d2c3642f16eafa0c1cc2d13e37 ./029_phase2_exit_gate.md 1f49005dbac81b2c3f3548d97f9aa32d6c8ada99f533d0fe197bcb047c553464 ./030_phase3_protocol_conformance.md 5e450a7557f7be2fad3f06c3c35288eb8654f156c402f826ef9c9b9703a52bf6 ./031_responses_lite_additional_tools.md 7bc16b20b8abe848726cb662865d542f2a8a19d6acb400a5473b0e9e3bc003e1 ./032_custom_namespace_roundtrip.md @@ -63,7 +63,7 @@ e4d78f0db15cfe7819c98c5fde166c863a74d3a751ffe8196ddb6492691dc799 ./083_incident 28cb35f98ab0827a18f7ab52f28ad8cfd3f21ef475142c38ff8d49d41b43c106 ./091_pr_stack_and_commits.md 5bf3d16bea1a3ef12d68e3a467b271d3fd6631202fa43fb5f862f4ed505a43a3 ./092_definition_of_done.md 4cd63329dc69b6af90d2bdd31e64926ebc4acc916e1b34ccbef66e44f9a10937 ./093_execution_order.md -d68e52650b5b4dd7a6047b77f7942148f9ee36414a193828847a65b239411124 ./094_landing_verification_pass.md +6923be79563be75d1933f1dde0aadd3998ba8387d3ca2f42621fbabc1ef28ef9 ./094_landing_verification_pass.md 383ed01bca1fda60f5b57fa13e0f9cc8408d1ed02a8eab802600f6afe4097c73 ./KOREAN_SUMMARY.md 61be1a43ef84b272e6d04170dbd3e0a617aac36d9d3fb0fc2c4d96da2a62f775 ./README.md 82522bf74dd85254d7162ce103886b2a483d1c274a241afb80fa488a8a7d4356 ./patches/0001-add-tool-discovery-module.patch diff --git a/src/codex/catalog/tool-discovery.ts b/src/codex/catalog/tool-discovery.ts index 980c6e5a4..ca34180ae 100644 --- a/src/codex/catalog/tool-discovery.ts +++ b/src/codex/catalog/tool-discovery.ts @@ -6,7 +6,7 @@ import { modelRecordValue } from "../../reasoning-effort"; * * `supports_search_tool` selects Codex's DEFERRED tool-discovery surface. It is not the * hosted web-search capability (`web_search_tool_type`), and under `tool_mode = - * code_mode_only` it does not decide whether an eligible MCP tool is reachable: upstream + * code_mode_only` it appears not to decide whether an eligible MCP tool is reachable: upstream * installs nested tool specs on the code-mode `tools`/`ALL_TOOLS` globals in BOTH * exposures (codex-rs `spec_plan.rs` build_code_mode_executors → `code-mode/src/runtime/ * globals.rs`). What changes is where the schemas live — direct exposure embeds every MCP diff --git a/src/config.ts b/src/config.ts index 4905dc7b4..efd8b8ef6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1638,6 +1638,12 @@ function warnDegradedRoutedToolDiscoveryForLoad(parsed: unknown): void { continue; } for (const [modelId, value] of Object.entries(map as Record)) { + // A blank key fails the schema's `z.string().min(1)` and takes the WHOLE map with it, + // so it needs its own warning: reporting only bad values would drop the map silently. + if (modelId.trim() === "") { + console.warn(`⚠️ config.json providers.${safeProviderName}.modelRoutedToolDiscovery has a blank model key — ignoring the whole map`); + break; + } if (isRoutedToolDiscoveryMode(value)) continue; const safeModelId = JSON.stringify(redactSecretString(modelId)); console.warn(`⚠️ config.json providers.${safeProviderName}.modelRoutedToolDiscovery.${safeModelId} (${typeof value}) is invalid — ignoring the whole map`); diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index a6a8dfcaa..2dcfe3fab 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -334,7 +334,7 @@ describe("routed tool-discovery backward compatibility", () => { // 023_backward_compatibility_tests.md: the load-bearing promise of this feature is that an // unconfigured tree is unchanged. Assert the WHOLE emitted row, not just the two policy // fields, so an unrelated key silently appearing on routed rows also fails here. - it("emits byte-identical routed rows with zero configuration", () => { + it("emits identical rows whether the deferred default is implicit or explicit", () => { const withoutPolicy = buildCatalogEntries(null, [], [{ provider: "deepseek", id: "glm-5.2" }]); const withExplicitAuto = buildCatalogEntries(null, [], [ { provider: "deepseek", id: "glm-5.2", toolDiscoveryMode: "deferred" }, @@ -486,6 +486,42 @@ describe("routed tool-discovery backward compatibility", () => { }); }); + it("warns when a blank model key silently drops the whole map on load", () => { + // `z.string().min(1)` rejects the key and takes the entire map with it, so a + // value-only warning would let the map vanish without a word. + withTempHome(() => { + const base = { + port: 10100, + defaultProvider: "test", + providers: { + test: { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:1/v1", + apiKey: "k", + allowPrivateNetwork: true, + modelRoutedToolDiscovery: { "": "direct" }, + }, + }, + }; + saveConfig(base as never); + writeFileSync(getConfigPath(), `${JSON.stringify(base, null, 2)}\n`); + + const warnings: string[] = []; + const original = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + let loaded; + try { loaded = loadConfig(); } + finally { console.warn = original; } + + const provider = loaded.providers.test as unknown as Record; + expect(provider.apiKey).toBe("k"); + expect(provider.modelRoutedToolDiscovery).toBeUndefined(); + const joined = warnings.join("\n"); + expect(joined).toContain("modelRoutedToolDiscovery"); + expect(joined).toContain("blank model key"); + }); + }); + it("does not materialize the new fields when loading a pre-field config file", () => { withTempHome(() => { // A config written before these fields existed. A materialized default here would From 12fd501b6bd2a264f0689837b7b32ed3d65d3238 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 22:13:41 +0900 Subject: [PATCH 12/13] fix(config): apply the same nonblank model-key rule on load as on write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blank-key warning added last commit could lie. The load schema used `z.string().min(1)`, which accepts a whitespace-only key, while the write boundary and the warning helper both treat `" "` as blank via `trim()`. So a config carrying `{" ": "direct"}` printed "ignoring the whole map" and then kept the map — a false warning, worse than no warning, because it tells an operator their override was dropped when it is still live. The load path now uses the same nonblank rule as the write boundary, so the three agree. The regression test switched from `""` to `" "` precisely because the empty-string case passed under both rules and could not catch this. Ablation: reverting the refine to `.min(1)` turns the test red. Verification: bun x tsc --noEmit clean; 428 pass / 0 fail across 9 suites; privacy:scan passed; SHA256SUMS regenerated. --- src/config.ts | 8 +++++++- tests/codex-tool-discovery-mode.test.ts | 4 +++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/config.ts b/src/config.ts index efd8b8ef6..6af51d733 100644 --- a/src/config.ts +++ b/src/config.ts @@ -651,7 +651,13 @@ const providerConfigSchema = z.object({ // Hand-edited configs degrade instead of losing the whole provider (credentials, ports). // The write boundary rejects the same values outright — see routedToolDiscoveryError(). routedToolDiscovery: routedToolDiscoveryModeSchema.optional().catch(undefined), - modelRoutedToolDiscovery: z.record(z.string().min(1), routedToolDiscoveryModeSchema).optional().catch(undefined), + // `.min(1)` alone would accept a whitespace-only key, which the write boundary rejects as + // blank — the load path must use the SAME nonblank rule or the degrade-warning lies about + // what happened to the map. + modelRoutedToolDiscovery: z.record( + z.string().refine(key => key.trim() !== ""), + routedToolDiscoveryModeSchema, + ).optional().catch(undefined), }).passthrough(); const RESERVED_PROVIDER_NAMES = new Set([ diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index 2dcfe3fab..2ad51ec64 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -499,7 +499,9 @@ describe("routed tool-discovery backward compatibility", () => { baseUrl: "http://127.0.0.1:1/v1", apiKey: "k", allowPrivateNetwork: true, - modelRoutedToolDiscovery: { "": "direct" }, + // Whitespace-only, not just empty: `.min(1)` would accept " " while the write + // boundary rejects it as blank, so the warning would claim a drop that never happened. + modelRoutedToolDiscovery: { " ": "direct" }, }, }, }; From 270c11f7c82f1cdd46eb319f13d84ea23a067276 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 13 Aug 2026 22:16:27 +0900 Subject: [PATCH 13/13] docs(codex): correct comments my own blank-key fix invalidated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three stale claims left behind by the previous two commits. Both the helper comment and the test comment still said a blank key fails `z.string().min(1)`. That became false the moment I changed the rule: `.min(1)` is precisely the mutant that ACCEPTS a whitespace-only key and made the warning lie. They now name the trimmed-nonblank refinement, and the test comment records why the fixture is whitespace-only rather than empty — an empty string passes under both rules and so cannot catch the load/write mismatch. `000_master_plan.md` still claimed default behavior is "byte-for-byte equivalent" to #1596 while `029` correctly records the prior-build normalized comparison as open. It now says equivalence is intended and asserted per-key on both construction paths, and points at `029` for what remains unproven. Verification: bun x tsc --noEmit clean; 399 pass / 0 fail across the config and catalog suites; SHA256SUMS regenerated. --- .../000_master_plan.md | 4 +++- .../_plan/260813_routed_tool_discovery_profiles/SHA256SUMS | 2 +- src/config.ts | 6 ++++-- tests/codex-tool-discovery-mode.test.ts | 6 ++++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md b/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md index 670c49242..319bcb8c6 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/000_master_plan.md @@ -50,7 +50,9 @@ At the verified `dev` head: ### PR A — profile resolver and explicit escape hatch -Default behavior remains byte-for-byte equivalent to #1596: +Default behavior is intended to be equivalent to #1596, and is asserted per-key on both +construction paths rather than by a normalized diff against a real prior build (that +comparison is still open — see `029`): - non-Cursor: deferred - Cursor: direct diff --git a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS index b36ca3eda..68fc53cf6 100644 --- a/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS +++ b/devlog/_plan/260813_routed_tool_discovery_profiles/SHA256SUMS @@ -1,4 +1,4 @@ -0c64b276bd741ec3f12002df6df99b2ca559602355cd1e882c633267687d0332 ./000_master_plan.md +d6538ee7c9a522a99746ee860e7f6d0e4a98e4a7baeffbba78d57f00fafe0295 ./000_master_plan.md 7e853a53e344412cc4aa2fcbe64b140195e9825a1988c5e245204c19a8f22449 ./001_verified_dev_baseline.md 32d7a7ffff41bd0c036cef153f53047ce416624032dae9dff43e88ff24d653fc ./002_incident_history_1522_1529_1596.md 343ee877dde67978b7ffd6a1e6f7825751c5fb113fd51dead6554036f92fa904 ./003_current_code_map.md diff --git a/src/config.ts b/src/config.ts index 6af51d733..ca4f91823 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1644,8 +1644,10 @@ function warnDegradedRoutedToolDiscoveryForLoad(parsed: unknown): void { continue; } for (const [modelId, value] of Object.entries(map as Record)) { - // A blank key fails the schema's `z.string().min(1)` and takes the WHOLE map with it, - // so it needs its own warning: reporting only bad values would drop the map silently. + // A blank key fails the schema's trimmed-nonblank refinement and takes the WHOLE map + // with it, so it needs its own warning: reporting only bad values would drop the map + // silently. `.min(1)` would NOT be enough here — it accepts a whitespace-only key, + // which is exactly the mismatch that once made this warning lie. if (modelId.trim() === "") { console.warn(`⚠️ config.json providers.${safeProviderName}.modelRoutedToolDiscovery has a blank model key — ignoring the whole map`); break; diff --git a/tests/codex-tool-discovery-mode.test.ts b/tests/codex-tool-discovery-mode.test.ts index 2ad51ec64..ba68bc6b1 100644 --- a/tests/codex-tool-discovery-mode.test.ts +++ b/tests/codex-tool-discovery-mode.test.ts @@ -487,8 +487,10 @@ describe("routed tool-discovery backward compatibility", () => { }); it("warns when a blank model key silently drops the whole map on load", () => { - // `z.string().min(1)` rejects the key and takes the entire map with it, so a - // value-only warning would let the map vanish without a word. + // The trimmed-nonblank refinement rejects the key and takes the entire map with it, so + // a value-only warning would let the map vanish without a word. The fixture is + // whitespace-only on purpose: `.min(1)` accepts it, so an empty-string fixture would + // pass under both rules and could not catch the load/write mismatch. withTempHome(() => { const base = { port: 10100,