From 4999655f7841eb16706159fb7d2e0eefa08a76f1 Mon Sep 17 00:00:00 2001 From: atinseau <65445747+atinseau@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:03:23 +0100 Subject: [PATCH 1/9] fix(responses): reject eagerly expanded Codex MCP catalogs --- .../docs/reference/configuration/catalog.md | 17 +++ scripts/test-layout/layout.json | 2 +- src/server/responses/codex-tool-catalog.ts | 51 +++++++ src/server/responses/compact.ts | 3 + src/server/responses/request-prepare.ts | 5 + structure/catalog.md | 2 +- tests/fixtures/test-layout-expected.json | 1 + .../codex-eager-tool-catalog.test.ts | 131 ++++++++++++++++++ 8 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 src/server/responses/codex-tool-catalog.ts create mode 100644 tests/responses/codex-eager-tool-catalog.test.ts diff --git a/docs-site/src/content/docs/reference/configuration/catalog.md b/docs-site/src/content/docs/reference/configuration/catalog.md index 360cf70e9cf..58856258031 100644 --- a/docs-site/src/content/docs/reference/configuration/catalog.md +++ b/docs-site/src/content/docs/reference/configuration/catalog.md @@ -3,6 +3,23 @@ title: Catalog options description: Optional native model omission in the generated Codex catalog. --- +## Reloading model metadata + +Codex loads a local model catalog at startup. After OpenCodex changes that catalog, +run `ocx sync --restart-codex` to refresh existing clients. Publishing a new catalog +file does not update a client that is already running. + +If a Codex client sends 64 or more eager MCP tool definitions, OpenCodex refuses the +request before contacting a model provider with HTTP 400 and error code +`codex_catalog_reload_required`. This prevents missing or stale model metadata from +causing repeated large prompts and compactions. Run the restart command and retry. +The check also covers definitions embedded in Code Mode; it leaves deferred tools, +ordinary conversation history, and other API clients unchanged. + +Routed models advertise deferred discovery. In Code Mode, discover available tools +through `ALL_TOOLS` and call them through `tools`; a separate `tool_search` function +is not required. Models using the direct tool interface can expose `tool_search`. + ## Omit disabled native models `omitDisabledNativeModels` is an optional boolean in `config.json`, defaulting to `false`. diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 55bae0134d2..777d502f2e0 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -726,7 +726,7 @@ "codex-sync-api.test.ts": "codex-integration", "codex-sync-new-model-policy.test.ts": "codex-integration", "codex-sync-response.test.ts": "codex-integration", - "codex-tool-mode.test.ts": "codex-integration", + "codex-tool-mode.test.ts": "codex-integration", "codex-eager-tool-catalog.test.ts": "responses", "codex-transition-state-adoption.test.ts": "codex-integration", "codex-transition-state-first-use-regression.test.ts": "codex-integration", "codex-transition-state-race.test.ts": "codex-integration", diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts new file mode 100644 index 00000000000..3ce5260115f --- /dev/null +++ b/src/server/responses/codex-tool-catalog.ts @@ -0,0 +1,51 @@ +import { currentTurnWireToolCatalogBody } from "../responses-undeclared-tool-guard"; + +const MAX_EAGER_CODEX_MCP_TOOLS = 64; + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function eagerMcpToolCount(specs: unknown[], names: Set, namespace?: string): void { + for (const spec of specs) { + if (!isRecord(spec) || spec.defer_loading === true) continue; + if (spec.type === "namespace" && typeof spec.name === "string" && Array.isArray(spec.tools)) { + eagerMcpToolCount(spec.tools, names, spec.name); + continue; + } + const tool = isRecord(spec.function) ? spec.function : spec; + if (tool.defer_loading === true || typeof tool.name !== "string") continue; + const name = namespace ? `${namespace}__${tool.name}` : tool.name; + if (name.startsWith("mcp__")) names.add(name); + if (tool.name === "exec" && typeof tool.description === "string" + && tool.description.includes("ALL_TOOLS") && tool.description.includes("V8")) { + for (const match of tool.description.matchAll(/\bdeclare const tools:\s*\{\s*(mcp__[A-Za-z0-9_]+)\s*\(/g)) { + names.add(match[1]!); + } + } + } +} + +export function codexToolCatalogReloadResponse(body: unknown, headers: Headers): Response | null { + const identity = `${headers.get("originator") ?? ""} ${headers.get("user-agent") ?? ""}`; + if (!headers.has("x-codex-turn-metadata") && !/\bcodex(?:[ _/\-]|$)/i.test(identity)) return null; + const current = currentTurnWireToolCatalogBody(body, undefined); + if (!isRecord(current)) return null; + const names = new Set(); + if (Array.isArray(current.tools)) eagerMcpToolCount(current.tools, names); + if (Array.isArray(current.input)) { + for (const item of current.input) { + if (isRecord(item) && item.type === "additional_tools" && Array.isArray(item.tools)) { + eagerMcpToolCount(item.tools, names); + } + } + } + if (names.size < MAX_EAGER_CODEX_MCP_TOOLS) return null; + return Response.json({ + error: { + type: "invalid_request_error", + code: "codex_catalog_reload_required", + message: "Codex sent a large eager MCP tool catalog instead of deferred tool discovery. Its model metadata may be missing or stale. Run 'ocx sync --restart-codex', then retry. OpenCodex refused this request before contacting the model provider.", + }, + }, { status: 400 }); +} diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 66dc78297f4..b1400592f67 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -237,6 +237,7 @@ import { mapCodexAuthContextErrorToResponse, nativeMainRefreshFailureResponse } import { linkRequestSessionLane, sessionLaneIdFromRequest, reasoningReplayConversationIdFromResponsesRequest, sessionIdHeaderFromRequest } from "../request-log-conversation"; import { recallComboForLane } from "./combo-session-recall"; import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; +import { codexToolCatalogReloadResponse } from "./codex-tool-catalog"; export const COMPACT_RESPONSE_MAX_BYTES = 32 * 1024 * 1024; @@ -690,6 +691,8 @@ export async function handleResponsesCompact( } const clientModelLabel = options.clientModelLabel ?? clientFacingModelLabel(options.compactionRoutingOverride?.sourceModel ?? (body as { model?: unknown }).model); options = { ...options, clientModelLabel }; + const catalogReload = codexToolCatalogReloadResponse(body, req.headers); + if (catalogReload) return catalogReload; // Compaction forwards client history upstream too; keep local artifact paths out of it. redactHostedImageDisplayPaths(body); if (!options.compactionRoutingOverride) { diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 49c7d12ace6..dcf326e378d 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -181,6 +181,7 @@ import { } from "./account-change-state"; import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; +import { codexToolCatalogReloadResponse } from "./codex-tool-catalog"; /** Policy-routing evidence. The direct lane's settings-only body has no conversation to scan. */ function routeEvidence(options: Pick, parsed: OcxParsedRequest): PolicyRequestEvidence { @@ -248,6 +249,10 @@ export async function prepareResponsesRequest( } return decodeRequestErrorResponse(err, "responses"); } + if (inboundWire === "responses" && !concreteSelection) { + const catalogReload = codexToolCatalogReloadResponse(body, req.headers); + if (catalogReload) return catalogReload; + } // Remove display-only artifact paths before helpers, combo dispatch and upstream replay. const originalSelector = options.mainHardLockFallbackOverride?.sourceModel ?? options.compactionRoutingOverride?.sourceModel ?? (options.comboAttempt || options.policyFallbackCandidate || options.internalDecisionCall || options.memoryModelPhase diff --git a/structure/catalog.md b/structure/catalog.md index b918d2ba096..11f8a097745 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -474,7 +474,7 @@ turn-1 payload regression (96,699 → 258,929 chars). For Cursor this can also m `exec` exceed the 120,000-byte serialized `McpTools` ceiling; the budget then drops `exec` and its companion `wait` (#1830). Hosted search remains independent: non-Cursor routes keep `web_search_tool_type: "text_and_image"`, while Cursor omits it because runTurn bypasses the -search sidecar. +search sidecar. Under code mode, > Decision record: [ADR-0022](decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md) diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d6cc69371a2..1cbd58a780a 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -108,6 +108,7 @@ "messages-native-key-selection.test.ts": "claude-integration", "messages-native-empty-inline.test.ts": "claude-integration", "explicit-empty-tool-catalog.test.ts": "responses", + "codex-eager-tool-catalog.test.ts": "responses", "explicit-empty-tool-catalog-history.test.ts": "responses", "current-turn-tool-catalog-direct.test.ts": "responses", "current-turn-tool-catalog-namespace.test.ts": "responses", diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts new file mode 100644 index 00000000000..66a61d08364 --- /dev/null +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -0,0 +1,131 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { saveConfig } from "../../src/config"; +import { startServer } from "../../src/server"; +import type { OcxConfig } from "../../src/types"; +import { createTempHome } from "../helpers/temp-home"; +import { ownedServiceHomeInspection } from "../helpers/owned-service-home-inspection"; + +const inspectNativeCodexOwnership = ownedServiceHomeInspection("Codex tool catalog reload test"); +let home: ReturnType; +let upstream: ReturnType; +let server: ReturnType; +let upstreamCalls = 0; + +function mcpTools(deferred = false) { + return [{ + type: "namespace", name: "mcp__fixture", + tools: Array.from({ length: 64 }, (_value, index) => ({ + type: "function", name: `tool_${index}`, description: "Fixture tool", + parameters: { type: "object", properties: {} }, + ...(deferred ? { defer_loading: true } : {}), + })), + }]; +} + +beforeEach(() => { + home = createTempHome("ocx-codex-tool-catalog-"); + upstreamCalls = 0; + upstream = Bun.serve({ + port: 0, + async fetch() { + upstreamCalls += 1; + return Response.json({ + id: "chatcmpl_fixture", object: "chat.completion", model: "model", + choices: [{ index: 0, message: { role: "assistant", content: "ok" }, finish_reason: "stop" }], + usage: { prompt_tokens: 2, completion_tokens: 1, total_tokens: 3 }, + }); + }, + }); + saveConfig({ + port: 0, defaultProvider: "fixture", + providers: { fixture: { adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, apiKey: "fixture-key", liveModels: false, allowPrivateNetwork: true } }, + } as OcxConfig); + server = startServer(0, { inspectNativeCodexOwnership }); +}); + +afterEach(async () => { + await server?.stop(true); + upstream?.stop(true); + home.remove(); +}); + +function post(path: string, tools: unknown[], codex = true, extra: Record = {}) { + return fetch(new URL(path, server.url), { + method: "POST", + headers: { + "content-type": "application/json", authorization: "Bearer fixture-client", + ...(codex ? { originator: "codex_cli_rs", "user-agent": "codex_cli_rs/0.166.0" } : {}), + }, + body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools, ...extra }), + }); +} + +describe("Codex stale metadata cannot spend an eagerly expanded MCP catalog", () => { + test.each(["/v1/responses", "/backend-api/codex/responses", "/v1/responses/compact"])("%s refuses before contacting a provider", async path => { + const response = await post(path, mcpTools()); + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ error: { code: "codex_catalog_reload_required" } }); + expect(upstreamCalls).toBe(0); + }); + + test("flattened MCP declarations are refused with Codex turn metadata alone", async () => { + const tools = mcpTools()[0]!.tools.map(tool => ({ ...tool, name: `mcp__fixture__${tool.name}` })); + const response = await fetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json", authorization: "Bearer fixture-client", "x-codex-turn-metadata": "{}" }, + body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools }), + }); + expect(response.status).toBe(400); + expect(upstreamCalls).toBe(0); + }); + + test("Responses Lite additional_tools declarations are checked before dispatch", async () => { + const response = await post("/v1/responses", [], true, { + input: [{ type: "additional_tools", role: "developer", tools: mcpTools() }, { role: "user", content: "hello" }], + }); + expect(response.status).toBe(400); + expect(upstreamCalls).toBe(0); + }); + + test("a small explicitly loaded MCP subset is not rejected", async () => { + const declarations = mcpTools(); + declarations[0]!.tools.length = 3; + const response = await post("/v1/responses", declarations); + expect(response.status).toBe(200); + await response.text(); + expect(upstreamCalls).toBe(1); + }); + + test("deferred MCP definitions with tool search remain usable", async () => { + const response = await post("/v1/responses", [...mcpTools(true), { type: "tool_search", execution: "client" }]); + expect(response.status).toBe(200); + await response.text(); + expect(upstreamCalls).toBe(1); + }); + + test("ordinary clients can explicitly choose an eager MCP catalog", async () => { + const response = await post("/v1/responses", mcpTools(), false); + expect(response.status).toBe(200); + await response.text(); + expect(upstreamCalls).toBe(1); + }); + + test("code-mode discovery does not count examples or historical text as declarations", async () => { + const response = await post("/v1/responses", [{ + type: "custom", name: "exec", description: "JavaScript in a V8 isolate. Discover helpers from ALL_TOOLS; example mcp__fixture__read.", + }], true, { input: [{ role: "user", content: JSON.stringify(mcpTools()) }] }); + expect(response.status).toBe(200); + await response.text(); + expect(upstreamCalls).toBe(1); + }); + + test("an expanded code-mode description is refused too", async () => { + const description = "JavaScript in a V8 isolate. ALL_TOOLS\n" + Array.from({ length: 64 }, (_value, index) => + `declare const tools: { mcp__fixture__tool_${index}(args: {}): Promise; };`, + ).join("\n"); + const response = await post("/v1/responses", [{ type: "custom", name: "exec", description }]); + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ error: { code: "codex_catalog_reload_required" } }); + expect(upstreamCalls).toBe(0); + }); +}); From a28e9e68990fbd74c43474bd0fb6d8069d575f95 Mon Sep 17 00:00:00 2001 From: atinseau <65445747+atinseau@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:25:51 +0100 Subject: [PATCH 2/9] fix(responses): preserve compaction history and narrow Codex detection --- .../docs/reference/configuration/catalog.md | 10 ++++- src/server/responses/codex-tool-catalog.ts | 10 +++-- src/server/responses/compact.ts | 2 +- src/server/responses/request-prepare.ts | 11 ++--- structure/catalog.md | 24 +++++----- .../codex-eager-tool-catalog.test.ts | 45 +++++++++++++++++++ 6 files changed, 78 insertions(+), 24 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/catalog.md b/docs-site/src/content/docs/reference/configuration/catalog.md index 58856258031..fd576165e77 100644 --- a/docs-site/src/content/docs/reference/configuration/catalog.md +++ b/docs-site/src/content/docs/reference/configuration/catalog.md @@ -13,8 +13,14 @@ If a Codex client sends 64 or more eager MCP tool definitions, OpenCodex refuses request before contacting a model provider with HTTP 400 and error code `codex_catalog_reload_required`. This prevents missing or stale model metadata from causing repeated large prompts and compactions. Run the restart command and retry. -The check also covers definitions embedded in Code Mode; it leaves deferred tools, -ordinary conversation history, and other API clients unchanged. +The check also covers definitions embedded in Code Mode. It recognizes canonical +Codex client identities or Codex turn metadata; an incidental `codex` substring in +another client's name does not activate it. For compaction, including Responses +API compaction triggers, only the top-level tool catalog is checked: +`additional_tools` items in the input are history, not +current declarations. Responses Lite current `additional_tools` declarations +remain checked. Explicitly deferred tools, conversation text, and other API +clients retain their existing behavior. Routed models advertise deferred discovery. In Code Mode, discover available tools through `ALL_TOOLS` and call them through `tools`; a separate `tool_search` function diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index 3ce5260115f..05e9ced677c 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -1,6 +1,7 @@ import { currentTurnWireToolCatalogBody } from "../responses-undeclared-tool-guard"; const MAX_EAGER_CODEX_MCP_TOOLS = 64; +const CODEX_CLIENT_IDENTITY = /^(?:codex|codex_cli_rs|codex_app|codex_work_desktop|codexless_agent|Codex Desktop)(?:\/|$)/i; function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); @@ -26,14 +27,15 @@ function eagerMcpToolCount(specs: unknown[], names: Set, namespace?: str } } -export function codexToolCatalogReloadResponse(body: unknown, headers: Headers): Response | null { - const identity = `${headers.get("originator") ?? ""} ${headers.get("user-agent") ?? ""}`; - if (!headers.has("x-codex-turn-metadata") && !/\bcodex(?:[ _/\-]|$)/i.test(identity)) return null; +export function codexToolCatalogReloadResponse(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): Response | null { + if (!headers.has("x-codex-turn-metadata") + && !CODEX_CLIENT_IDENTITY.test(headers.get("originator") ?? "") + && !CODEX_CLIENT_IDENTITY.test(headers.get("user-agent") ?? "")) return null; const current = currentTurnWireToolCatalogBody(body, undefined); if (!isRecord(current)) return null; const names = new Set(); if (Array.isArray(current.tools)) eagerMcpToolCount(current.tools, names); - if (Array.isArray(current.input)) { + if (surface === "responses" && Array.isArray(current.input)) { for (const item of current.input) { if (isRecord(item) && item.type === "additional_tools" && Array.isArray(item.tools)) { eagerMcpToolCount(item.tools, names); diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index b1400592f67..0fb99c32d86 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -691,7 +691,7 @@ export async function handleResponsesCompact( } const clientModelLabel = options.clientModelLabel ?? clientFacingModelLabel(options.compactionRoutingOverride?.sourceModel ?? (body as { model?: unknown }).model); options = { ...options, clientModelLabel }; - const catalogReload = codexToolCatalogReloadResponse(body, req.headers); + const catalogReload = codexToolCatalogReloadResponse(body, req.headers, "compact"); if (catalogReload) return catalogReload; // Compaction forwards client history upstream too; keep local artifact paths out of it. redactHostedImageDisplayPaths(body); diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index dcf326e378d..b5f82a88b78 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -249,8 +249,13 @@ export async function prepareResponsesRequest( } return decodeRequestErrorResponse(err, "responses"); } + const isCompactionTrigger = body !== null && typeof body === "object" && !Array.isArray(body) + && Array.isArray((body as { input?: unknown }).input) + && ((body as { input: unknown[] }).input).some((item: unknown) => + typeof item === "object" && item !== null && (item as { type?: string }).type === "compaction_trigger"); if (inboundWire === "responses" && !concreteSelection) { - const catalogReload = codexToolCatalogReloadResponse(body, req.headers); + const surface = options.compactionRecoveryKind === "compaction-v1" || isCompactionTrigger ? "compact" : "responses"; + const catalogReload = codexToolCatalogReloadResponse(body, req.headers, surface); if (catalogReload) return catalogReload; } // Remove display-only artifact paths before helpers, combo dispatch and upstream replay. @@ -319,10 +324,6 @@ export async function prepareResponsesRequest( effort: comboEffortRow.effort, }; } - const isCompactionTrigger = body !== null && typeof body === "object" && !Array.isArray(body) - && Array.isArray((body as { input?: unknown }).input) - && ((body as { input: unknown[] }).input).some((item: unknown) => - typeof item === "object" && item !== null && (item as { type?: string }).type === "compaction_trigger"); // Compaction may send the last client-visible bare model after a combo switch. // Configured selectors take precedence; otherwise recall before combo dispatch (#3891). if (!concreteSelection && !options.compactionRoutingOverride && body && typeof body === "object" && !Array.isArray(body)) { diff --git a/structure/catalog.md b/structure/catalog.md index 11f8a097745..343f55a71f6 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -463,18 +463,18 @@ real turn depends on it (`src/codex/warmup.ts`). ## Routed tool discovery and hosted search -All routed catalog rows advertise `supports_search_tool: true` together with -`tool_mode: "code_mode_only"` — the pair is load-bearing. The field selects Codex's deferred -tool-discovery surface; it does not describe the hosted web-search sidecar. Under code mode, -deferred MCP tools remain callable through exec's `tools` global / `ALL_TOOLS` without a -`tool_search` round-trip (upstream codex-rs code_mode suite; live canary 2026-08-13: routed -kimi/k3 executed `tools.mcp__node_repl__js`, devlog `260813_tool_catalog_deferral/010+020`). -Stamping `false` instead forces every MCP declaration into `exec.description` — a measured 2.7x -turn-1 payload regression (96,699 → 258,929 chars). For Cursor this can also make the unified -`exec` exceed the 120,000-byte serialized `McpTools` ceiling; the budget then drops `exec` and -its companion `wait` (#1830). Hosted search remains independent: non-Cursor routes keep -`web_search_tool_type: "text_and_image"`, while Cursor omits it because runTurn bypasses the -search sidecar. Under code mode, +All routed rows pair `supports_search_tool: true` with `tool_mode: "code_mode_only"`; together they enable Codex deferred tool discovery, independently of hosted web search. In code mode, deferred MCP tools are callable through exec's +`tools` global / `ALL_TOOLS` without `tool_search` (upstream codex-rs suite; live canary 2026-08-13: routed kimi/k3 +executed `tools.mcp__node_repl__js`, devlog `260813_tool_catalog_deferral/010+020`). Stamping `false` forces MCP +declarations into `exec.description`, a measured 2.7x turn-1 payload increase (96,699 → 258,929 chars); for Cursor +this can exceed the 120,000-byte `McpTools` ceiling and drop `exec` and `wait` (#1830). Hosted search is separate: +non-Cursor routes keep `web_search_tool_type: "text_and_image"`; Cursor omits it because runTurn bypasses the sidecar. + +Canonical Codex identities or Codex turn metadata activate the guard. Responses checks 64+ eager MCP definitions +in top-level `tools`, current-turn Responses Lite `input` +`additional_tools`, and expanded Code Mode `exec` descriptions, returning HTTP 400 (`codex_catalog_reload_required`) +before provider dispatch. Deferred definitions are excluded. Compaction, including internal V1 handoff and V2 +`compaction_trigger` requests, checks top-level `tools` and ignores `input` history including `additional_tools`; normal clients may send eager catalogs. Helper: `src/server/responses/codex-tool-catalog.ts`; test: `tests/responses/codex-eager-tool-catalog.test.ts`. > Decision record: [ADR-0022](decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md) diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index 66a61d08364..8127495dd18 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -87,6 +87,51 @@ describe("Codex stale metadata cannot spend an eagerly expanded MCP catalog", () expect(upstreamCalls).toBe(0); }); + test.each(["/v1/responses/compact", "/v1/responses"])("%s compaction history does not trigger a reload refusal", async path => { + const response = await post(path, [], true, { + input: [ + { type: "additional_tools", role: "developer", tools: mcpTools() }, + { role: "user", content: "Earlier request" }, + { role: "assistant", content: "Earlier response" }, + ...(path === "/v1/responses" ? [{ type: "compaction_trigger" }] : []), + ], + }); + expect({ status: response.status, body: await response.clone().text() }).toMatchObject({ status: 200 }); + await response.text(); + expect(upstreamCalls).toBe(1); + }); + + test.each([ + ["originator", "my-codex-wrapper/1"], + ["user-agent", "my-codex-wrapper/1"], + ])("a generic %s containing codex remains allowed", async (header, value) => { + const response = await fetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { + "content-type": "application/json", authorization: "Bearer fixture-client", + [header]: value, + }, + body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools: mcpTools() }), + }); + expect(response.status).toBe(200); + await response.text(); + expect(upstreamCalls).toBe(1); + }); + + test("the canonical Codex user-agent alone still triggers the reload refusal", async () => { + const response = await fetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { + "content-type": "application/json", authorization: "Bearer fixture-client", + "user-agent": "codex_cli_rs/0.166.0", + }, + body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools: mcpTools() }), + }); + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ error: { code: "codex_catalog_reload_required" } }); + expect(upstreamCalls).toBe(0); + }); + test("a small explicitly loaded MCP subset is not rejected", async () => { const declarations = mcpTools(); declarations[0]!.tools.length = 3; From 37cfccdb91246a4e7f4949fba1dfc10e5d952734 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:40:44 +0900 Subject: [PATCH 3/9] fix(responses): diagnose eager Codex MCP catalogs without refusing them Owner decision on #8133: an eagerly expanded Codex MCP catalog is a valid request, so OpenCodex keeps forwarding it unchanged and logs one warning per session naming the definition count and EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD (64). No tool names, request content or session identifiers are logged. Count only the current request's declarations. A resumed thread resends earlier turns' Responses Lite additional_tools; those are history. The current turn is the additional_tools directly ahead of the last user input or after it. - Code Mode detection reuses the codex-rs rendering parser in code-mode-collab-surface.ts (codeModeRenderedToolNames) instead of matching "ALL_TOOLS"/"V8" literals. - Register the test in the layout-responses.json shard. - Record the decision in ADR-0022; update structure/catalog.md and the catalog reference page. Co-authored-by: atinseau <65445747+atinseau@users.noreply.github.com> --- .../docs/reference/configuration/catalog.md | 26 +- scripts/test-layout/layout-responses.json | 1 + scripts/test-layout/layout.json | 2 +- .../responses/code-mode-collab-surface.ts | 11 + src/server/responses/codex-tool-catalog.ts | 100 +++++-- src/server/responses/compact.ts | 5 +- src/server/responses/request-prepare.ts | 7 +- structure/catalog.md | 10 +- ...routed-tool-discovery-and-hosted-search.md | 9 + tests/helpers/responses-core-source.ts | 4 + .../codex-eager-tool-catalog.test.ts | 259 ++++++++++++------ 11 files changed, 295 insertions(+), 139 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/catalog.md b/docs-site/src/content/docs/reference/configuration/catalog.md index fd576165e77..70c3ce7ac14 100644 --- a/docs-site/src/content/docs/reference/configuration/catalog.md +++ b/docs-site/src/content/docs/reference/configuration/catalog.md @@ -9,18 +9,20 @@ Codex loads a local model catalog at startup. After OpenCodex changes that catal run `ocx sync --restart-codex` to refresh existing clients. Publishing a new catalog file does not update a client that is already running. -If a Codex client sends 64 or more eager MCP tool definitions, OpenCodex refuses the -request before contacting a model provider with HTTP 400 and error code -`codex_catalog_reload_required`. This prevents missing or stale model metadata from -causing repeated large prompts and compactions. Run the restart command and retry. -The check also covers definitions embedded in Code Mode. It recognizes canonical -Codex client identities or Codex turn metadata; an incidental `codex` substring in -another client's name does not activate it. For compaction, including Responses -API compaction triggers, only the top-level tool catalog is checked: -`additional_tools` items in the input are history, not -current declarations. Responses Lite current `additional_tools` declarations -remain checked. Explicitly deferred tools, conversation text, and other API -clients retain their existing behavior. +A Codex client with missing or stale model metadata can send every MCP tool definition +eagerly instead of using deferred tool discovery, which makes each prompt much larger. +When a Codex request declares 64 or more eager MCP tool definitions, OpenCodex logs a +warning once per session with the count and the threshold, then forwards the request +unchanged. The warning contains no tool names or request content. Run the restart +command to reload the catalog. + +The count covers top-level tools, the current turn's Responses Lite `additional_tools` +declarations, and MCP tools rendered into the Code Mode `exec` description. Declarations +from earlier turns of a resumed thread are history and are not counted; compaction, +including Responses API compaction triggers, counts only the top-level tool catalog. +The check recognizes canonical Codex client identities or Codex turn metadata; an +incidental `codex` substring in another client's name does not activate it. Explicitly +deferred tools, conversation text, and other API clients are not counted. Routed models advertise deferred discovery. In Code Mode, discover available tools through `ALL_TOOLS` and call them through `tools`; a separate `tool_search` function diff --git a/scripts/test-layout/layout-responses.json b/scripts/test-layout/layout-responses.json index 60f1e7075ea..451f72b2e27 100644 --- a/scripts/test-layout/layout-responses.json +++ b/scripts/test-layout/layout-responses.json @@ -7,6 +7,7 @@ "passthrough-override.test.ts": "responses", "passthrough-headers.test.ts": "responses", "passthrough-abort.test.ts": "responses", + "codex-eager-tool-catalog.test.ts": "responses", "current-turn-tool-catalog-direct.test.ts": "responses", "current-turn-tool-catalog-namespace.test.ts": "responses" } diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 777d502f2e0..55bae0134d2 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -726,7 +726,7 @@ "codex-sync-api.test.ts": "codex-integration", "codex-sync-new-model-policy.test.ts": "codex-integration", "codex-sync-response.test.ts": "codex-integration", - "codex-tool-mode.test.ts": "codex-integration", "codex-eager-tool-catalog.test.ts": "responses", + "codex-tool-mode.test.ts": "codex-integration", "codex-transition-state-adoption.test.ts": "codex-integration", "codex-transition-state-first-use-regression.test.ts": "codex-integration", "codex-transition-state-race.test.ts": "codex-integration", diff --git a/src/server/responses/code-mode-collab-surface.ts b/src/server/responses/code-mode-collab-surface.ts index 0ef6d4b8b42..d65d574eacf 100644 --- a/src/server/responses/code-mode-collab-surface.ts +++ b/src/server/responses/code-mode-collab-surface.ts @@ -258,6 +258,17 @@ function renderedNestedTools(description: string): RenderedTool[] { return rendered; } +/** + * Nested tools a codex-rs code-mode exec renders, read from its wire form: a top-level custom + * `exec` whose description opens with the template header. Each name needs a heading followed by + * its own declaration, so an example or quoted text in the description renders nothing. + */ +export function codeModeRenderedToolNames(name: unknown, description: unknown): string[] { + if (name !== CODE_MODE_EXEC_TOOL_NAME || typeof description !== "string" + || !description.startsWith(CODE_MODE_EXEC_TEMPLATE_HEADER)) return []; + return renderedNestedTools(description).map(tool => tool.name); +} + /** A nested name that claims a collaboration surface other than V1's. */ function isForeignCollabName(name: string): boolean { if (name.startsWith("mcp__")) return false; // MCP server tools are never native collaboration diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index 05e9ced677c..39528ddec58 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -1,53 +1,105 @@ import { currentTurnWireToolCatalogBody } from "../responses-undeclared-tool-guard"; +import { sessionLaneIdFromRequest } from "../request-log-conversation"; +import { codeModeRenderedToolNames } from "./code-mode-collab-surface"; -const MAX_EAGER_CODEX_MCP_TOOLS = 64; +/** + * Eager MCP definitions at which a Codex request is diagnosed as carrying an expanded catalog. + * Routed catalog rows advertise deferred discovery, so a Codex client with current model metadata + * keeps MCP tools deferred; one whose metadata is missing or stale expands every MCP tool into the + * request. The request is still forwarded unchanged: this is a diagnostic, never an admission + * rule, because an eager catalog stays a valid request (ADR-0022). + */ +export const EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD = 64; +/** Sessions already warned. Bounded so a long-lived proxy cannot grow it without limit. */ +const WARNED_SESSION_LIMIT = 1024; +const warnedSessions = new Set(); const CODEX_CLIENT_IDENTITY = /^(?:codex|codex_cli_rs|codex_app|codex_work_desktop|codexless_agent|Codex Desktop)(?:\/|$)/i; function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } -function eagerMcpToolCount(specs: unknown[], names: Set, namespace?: string): void { +function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: string): void { for (const spec of specs) { if (!isRecord(spec) || spec.defer_loading === true) continue; if (spec.type === "namespace" && typeof spec.name === "string" && Array.isArray(spec.tools)) { - eagerMcpToolCount(spec.tools, names, spec.name); + collectEagerMcpTools(spec.tools, names, spec.name); continue; } const tool = isRecord(spec.function) ? spec.function : spec; if (tool.defer_loading === true || typeof tool.name !== "string") continue; const name = namespace ? `${namespace}__${tool.name}` : tool.name; if (name.startsWith("mcp__")) names.add(name); - if (tool.name === "exec" && typeof tool.description === "string" - && tool.description.includes("ALL_TOOLS") && tool.description.includes("V8")) { - for (const match of tool.description.matchAll(/\bdeclare const tools:\s*\{\s*(mcp__[A-Za-z0-9_]+)\s*\(/g)) { - names.add(match[1]!); + // Code Mode renders each enabled, non-deferred nested tool into the exec description. + if (!namespace && spec.type === "custom") { + for (const nested of codeModeRenderedToolNames(tool.name, tool.description)) { + if (nested.startsWith("mcp__")) names.add(nested); } } } } -export function codexToolCatalogReloadResponse(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): Response | null { +/** A message that opens a Responses turn ahead of its user input, such as instructions. */ +function isTurnPreamble(item: unknown): boolean { + if (!isRecord(item)) return false; + if (item.type === "additional_tools") return true; + return (item.type === undefined || item.type === "message") && (item.role === "developer" || item.role === "system"); +} + +/** + * Responses Lite `additional_tools` declared by the current turn. A resumed thread resends its + * whole history, including earlier turns' declarations, so only the declarations directly ahead of + * the last user input, or after it, belong to this request. Without user input every item counts. + */ +function currentTurnAdditionalTools(input: unknown[]): unknown[][] { + let start = 0; + for (let index = input.length - 1; index >= 0; index -= 1) { + const item = input[index]; + if (!isRecord(item) || item.role !== "user") continue; + start = index; + while (start > 0 && isTurnPreamble(input[start - 1])) start -= 1; + break; + } + const groups: unknown[][] = []; + for (const item of input.slice(start)) { + if (isRecord(item) && item.type === "additional_tools" && Array.isArray(item.tools)) groups.push(item.tools); + } + return groups; +} + +/** + * Eager MCP definitions a canonical Codex client declared for this request, or 0 for any other + * client. Compaction reads only top-level `tools`: its `input` is history. + */ +export function eagerCodexMcpToolCount(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): number { if (!headers.has("x-codex-turn-metadata") && !CODEX_CLIENT_IDENTITY.test(headers.get("originator") ?? "") - && !CODEX_CLIENT_IDENTITY.test(headers.get("user-agent") ?? "")) return null; + && !CODEX_CLIENT_IDENTITY.test(headers.get("user-agent") ?? "")) return 0; const current = currentTurnWireToolCatalogBody(body, undefined); - if (!isRecord(current)) return null; + if (!isRecord(current)) return 0; const names = new Set(); - if (Array.isArray(current.tools)) eagerMcpToolCount(current.tools, names); + if (Array.isArray(current.tools)) collectEagerMcpTools(current.tools, names); if (surface === "responses" && Array.isArray(current.input)) { - for (const item of current.input) { - if (isRecord(item) && item.type === "additional_tools" && Array.isArray(item.tools)) { - eagerMcpToolCount(item.tools, names); - } - } + for (const tools of currentTurnAdditionalTools(current.input)) collectEagerMcpTools(tools, names); } - if (names.size < MAX_EAGER_CODEX_MCP_TOOLS) return null; - return Response.json({ - error: { - type: "invalid_request_error", - code: "codex_catalog_reload_required", - message: "Codex sent a large eager MCP tool catalog instead of deferred tool discovery. Its model metadata may be missing or stale. Run 'ocx sync --restart-codex', then retry. OpenCodex refused this request before contacting the model provider.", - }, - }, { status: 400 }); + return names.size; +} + +/** + * Warn once per session when a Codex request carries an eagerly expanded MCP catalog. Only the + * count and the threshold are logged; tool names, bodies and session identifiers are not. A + * request without a session identity shares one anonymous slot. + */ +export function warnEagerCodexToolCatalog(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): void { + const count = eagerCodexMcpToolCount(body, headers, surface); + if (count < EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD) return; + const session = sessionLaneIdFromRequest(headers) ?? ""; + if (warnedSessions.has(session)) return; + if (warnedSessions.size >= WARNED_SESSION_LIMIT) warnedSessions.delete(warnedSessions.values().next().value!); + warnedSessions.add(session); + console.warn( + `[codex-catalog] Codex sent ${count} eager MCP tool definitions (warning threshold ${EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD}) ` + + "instead of deferred tool discovery; its model metadata may be missing or stale. The request was forwarded unchanged. " + + "Run 'ocx sync --restart-codex' to reload the catalog. Shown once per session.", + ); } diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 0fb99c32d86..26bb03d371f 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -237,7 +237,7 @@ import { mapCodexAuthContextErrorToResponse, nativeMainRefreshFailureResponse } import { linkRequestSessionLane, sessionLaneIdFromRequest, reasoningReplayConversationIdFromResponsesRequest, sessionIdHeaderFromRequest } from "../request-log-conversation"; import { recallComboForLane } from "./combo-session-recall"; import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; -import { codexToolCatalogReloadResponse } from "./codex-tool-catalog"; +import { warnEagerCodexToolCatalog } from "./codex-tool-catalog"; export const COMPACT_RESPONSE_MAX_BYTES = 32 * 1024 * 1024; @@ -691,8 +691,7 @@ export async function handleResponsesCompact( } const clientModelLabel = options.clientModelLabel ?? clientFacingModelLabel(options.compactionRoutingOverride?.sourceModel ?? (body as { model?: unknown }).model); options = { ...options, clientModelLabel }; - const catalogReload = codexToolCatalogReloadResponse(body, req.headers, "compact"); - if (catalogReload) return catalogReload; + warnEagerCodexToolCatalog(body, req.headers, "compact"); // Compaction forwards client history upstream too; keep local artifact paths out of it. redactHostedImageDisplayPaths(body); if (!options.compactionRoutingOverride) { diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index b5f82a88b78..d7227e5f4a5 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -181,7 +181,7 @@ import { } from "./account-change-state"; import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; -import { codexToolCatalogReloadResponse } from "./codex-tool-catalog"; +import { warnEagerCodexToolCatalog } from "./codex-tool-catalog"; /** Policy-routing evidence. The direct lane's settings-only body has no conversation to scan. */ function routeEvidence(options: Pick, parsed: OcxParsedRequest): PolicyRequestEvidence { @@ -254,9 +254,8 @@ export async function prepareResponsesRequest( && ((body as { input: unknown[] }).input).some((item: unknown) => typeof item === "object" && item !== null && (item as { type?: string }).type === "compaction_trigger"); if (inboundWire === "responses" && !concreteSelection) { - const surface = options.compactionRecoveryKind === "compaction-v1" || isCompactionTrigger ? "compact" : "responses"; - const catalogReload = codexToolCatalogReloadResponse(body, req.headers, surface); - if (catalogReload) return catalogReload; + warnEagerCodexToolCatalog(body, req.headers, + options.compactionRecoveryKind === "compaction-v1" || isCompactionTrigger ? "compact" : "responses"); } // Remove display-only artifact paths before helpers, combo dispatch and upstream replay. const originalSelector = options.mainHardLockFallbackOverride?.sourceModel ?? options.compactionRoutingOverride?.sourceModel diff --git a/structure/catalog.md b/structure/catalog.md index 343f55a71f6..4e6b2ca787d 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -470,11 +470,11 @@ declarations into `exec.description`, a measured 2.7x turn-1 payload increase (9 this can exceed the 120,000-byte `McpTools` ceiling and drop `exec` and `wait` (#1830). Hosted search is separate: non-Cursor routes keep `web_search_tool_type: "text_and_image"`; Cursor omits it because runTurn bypasses the sidecar. -Canonical Codex identities or Codex turn metadata activate the guard. Responses checks 64+ eager MCP definitions -in top-level `tools`, current-turn Responses Lite `input` -`additional_tools`, and expanded Code Mode `exec` descriptions, returning HTTP 400 (`codex_catalog_reload_required`) -before provider dispatch. Deferred definitions are excluded. Compaction, including internal V1 handoff and V2 -`compaction_trigger` requests, checks top-level `tools` and ignores `input` history including `additional_tools`; normal clients may send eager catalogs. Helper: `src/server/responses/codex-tool-catalog.ts`; test: `tests/responses/codex-eager-tool-catalog.test.ts`. +Canonical Codex identities or Codex turn metadata activate a diagnostic, never a refusal: at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64) eager MCP +definitions in top-level `tools`, current-turn Responses Lite `additional_tools` (directly ahead of or after the last user input; a resumed +thread's earlier declarations are history) or rendered Code Mode `exec` sections, one warning per session logs only the count and threshold and the +request is forwarded unchanged. Deferred definitions are excluded; compaction (V1 handoff, V2 `compaction_trigger`) counts top-level `tools` only. +Helper: `src/server/responses/codex-tool-catalog.ts`; test: `tests/responses/codex-eager-tool-catalog.test.ts`. > Decision record: [ADR-0022](decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md) diff --git a/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md b/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md index 8c3a4984acd..a1e9c8a4032 100644 --- a/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md +++ b/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md @@ -10,3 +10,12 @@ - 선택한 방식: enable Codex deferred exposure for Cursor code-mode rows too, while continuing to omit Cursor's hosted `web_search_tool_type`. - 다른 대안 대신 이 방식을 선택한 이유: it removes the known exec-description inflation before Cursor budgeting without weakening the measured transport limit, inventing caller tools, or turning bridge absence into local-execution authority. - 장점, 단점 및 영향: Cursor keeps a compact Responses-owned `exec` path under rich tool catalogs and hosted-search behavior remains unchanged; the existing Cursor budget and native-local-exec fail-closed policy remain authoritative. + +## Decision record — eagerly expanded Codex MCP catalogs (#8133) + +- 목적과 의도: tell an operator when a Codex client expands every MCP tool into the request, which usually means its model metadata is missing or stale and every turn pays the full-catalog payload. +- 기존 구현 및 제약 조건: an eager catalog is a valid request, and no supported-version evidence shows that every client sending 64+ eager MCP tools has stale metadata (an older client or disabled tool search may send one legitimately). A resumed thread resends earlier turns' `additional_tools`, so a whole-input count misreads history as the current turn. +- 검토한 주요 대안: refuse with HTTP 400 `codex_catalog_reload_required`; refuse with a config opt-out; warn without refusing. +- 선택한 방식: warn once per session at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64), logging only the count and threshold, and forward the request unchanged; count only the current turn's declarations. +- 다른 대안 대신 이 방식을 선택한 이유: a refusal turns a recoverable cost problem into a hard outage for any client the heuristic misjudges, and its remedy cannot help a client whose catalog is eager by design. The diagnostic keeps every existing request working. +- 장점, 단점 및 영향: no request changes behavior; operators get one actionable line per session. A stale client keeps paying the larger payload until it is restarted. diff --git a/tests/helpers/responses-core-source.ts b/tests/helpers/responses-core-source.ts index b4a250a95fa..59699edbd06 100644 --- a/tests/helpers/responses-core-source.ts +++ b/tests/helpers/responses-core-source.ts @@ -42,6 +42,10 @@ export const RESPONSES_CORE_MODULES = [ // Reached from core-combo.ts: JEV effort choices and resolved native quota identity. "jev-combo-choice.ts", "request-prepare.ts", + // Reached from request-prepare.ts and compact.ts: the eager Codex MCP catalog diagnostic (#8133). + "codex-tool-catalog.ts", + // Reached from codex-tool-catalog.ts: the rendered Code Mode exec section parser. + "code-mode-collab-surface.ts", "turn-membership.ts", "policy-request-scope.ts", "skills-snapshot.ts", diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index 8127495dd18..5e753bcbe16 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -1,20 +1,30 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { saveConfig } from "../../src/config"; import { startServer } from "../../src/server"; +import { EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD } from "../../src/server/responses/codex-tool-catalog"; import type { OcxConfig } from "../../src/types"; import { createTempHome } from "../helpers/temp-home"; import { ownedServiceHomeInspection } from "../helpers/owned-service-home-inspection"; -const inspectNativeCodexOwnership = ownedServiceHomeInspection("Codex tool catalog reload test"); +/** + * An eagerly expanded Codex MCP catalog is a diagnostic, never a refusal (#8133, ADR-0022): every + * request below must still reach the provider. Only the current request's declarations count, and + * the warning names the count and threshold once per session without tool names. + */ +const inspectNativeCodexOwnership = ownedServiceHomeInspection("Codex tool catalog diagnostic test"); +const TICK = "`"; +const FENCE = TICK.repeat(3); let home: ReturnType; let upstream: ReturnType; let server: ReturnType; let upstreamCalls = 0; +let warn: ReturnType>; +let sessionCounter = 0; -function mcpTools(deferred = false) { +function mcpTools(deferred = false, length = EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD) { return [{ type: "namespace", name: "mcp__fixture", - tools: Array.from({ length: 64 }, (_value, index) => ({ + tools: Array.from({ length }, (_value, index) => ({ type: "function", name: `tool_${index}`, description: "Fixture tool", parameters: { type: "object", properties: {} }, ...(deferred ? { defer_loading: true } : {}), @@ -22,13 +32,37 @@ function mcpTools(deferred = false) { }]; } +/** codex-rs build_exec_tool_description: a heading, the description, then a ts declaration fence. */ +function renderedExec(names: string[], preface = ""): Record { + const sections = names.map(name => [ + "### " + TICK + name + TICK, "Fixture tool", "", "exec tool declaration:", FENCE + "ts", + "declare const tools: { " + name + "(args: {}): Promise; };", FENCE, + ].join("\n")); + return { type: "custom", name: "exec", + description: ["Run JavaScript code to orchestrate/compose tool calls", preface, ...sections].join("\n\n") }; +} + +function catalogWarnings(): string[] { + return warn.mock.calls.map(call => String(call[0])).filter(text => text.startsWith("[codex-catalog]")); +} + beforeEach(() => { home = createTempHome("ocx-codex-tool-catalog-"); upstreamCalls = 0; + warn = spyOn(console, "warn"); upstream = Bun.serve({ port: 0, - async fetch() { + async fetch(req) { upstreamCalls += 1; + const streamed = (await req.json() as { stream?: unknown }).stream === true; + if (streamed) { + const chunk = (delta: Record, finish: string | null) => `data: ${JSON.stringify({ + id: "chatcmpl_fixture", object: "chat.completion.chunk", model: "model", + choices: [{ index: 0, delta, finish_reason: finish }], + })}\n\n`; + return new Response(chunk({ role: "assistant", content: "ok" }, null) + chunk({}, "stop") + "data: [DONE]\n\n", + { headers: { "content-type": "text/event-stream" } }); + } return Response.json({ id: "chatcmpl_fixture", object: "chat.completion", model: "model", choices: [{ index: 0, message: { role: "assistant", content: "ok" }, finish_reason: "stop" }], @@ -37,140 +71,185 @@ beforeEach(() => { }, }); saveConfig({ - port: 0, defaultProvider: "fixture", + port: 0, hostname: "127.0.0.1", websockets: true, defaultProvider: "fixture", providers: { fixture: { adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, apiKey: "fixture-key", liveModels: false, allowPrivateNetwork: true } }, } as OcxConfig); server = startServer(0, { inspectNativeCodexOwnership }); }); afterEach(async () => { + warn.mockRestore(); await server?.stop(true); upstream?.stop(true); home.remove(); }); -function post(path: string, tools: unknown[], codex = true, extra: Record = {}) { - return fetch(new URL(path, server.url), { +function codexHeaders(session = `catalog-session-${++sessionCounter}`): Record { + return { originator: "codex_cli_rs", "user-agent": "codex_cli_rs/0.166.0", session_id: session }; +} + +async function send(path: string, tools: unknown[], headers: Record, extra: Record = {}) { + const response = await fetch(new URL(path, server.url), { method: "POST", - headers: { - "content-type": "application/json", authorization: "Bearer fixture-client", - ...(codex ? { originator: "codex_cli_rs", "user-agent": "codex_cli_rs/0.166.0" } : {}), - }, + headers: { "content-type": "application/json", authorization: "Bearer fixture-client", ...headers }, body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools, ...extra }), }); + const text = await response.text(); + return { status: response.status, text }; } -describe("Codex stale metadata cannot spend an eagerly expanded MCP catalog", () => { - test.each(["/v1/responses", "/backend-api/codex/responses", "/v1/responses/compact"])("%s refuses before contacting a provider", async path => { - const response = await post(path, mcpTools()); - expect(response.status).toBe(400); - expect(await response.json()).toMatchObject({ error: { code: "codex_catalog_reload_required" } }); - expect(upstreamCalls).toBe(0); +describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time diagnostic", () => { + test.each(["/v1/responses", "/backend-api/codex/responses", "/v1/responses/compact"])("%s forwards the request and warns", async path => { + const result = await send(path, mcpTools(), codexHeaders()); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + const warnings = catalogWarnings(); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(`Codex sent ${EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD} eager MCP tool definitions`); + expect(warnings[0]).toContain(`warning threshold ${EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD}`); + expect(warnings[0]).not.toContain("mcp__fixture"); + expect(warnings[0]).not.toContain("tool_0"); + expect(warnings[0]).not.toContain("catalog-session"); }); - test("flattened MCP declarations are refused with Codex turn metadata alone", async () => { + test("the warning is shown once per session", async () => { + const first = codexHeaders(); + for (const headers of [first, first, codexHeaders()]) { + const result = await send("/v1/responses", mcpTools(), headers); + expect(result.status, result.text).toBe(200); + } + expect(upstreamCalls).toBe(3); + expect(catalogWarnings()).toHaveLength(2); + }); + + test("flattened MCP declarations are diagnosed with Codex turn metadata alone", async () => { const tools = mcpTools()[0]!.tools.map(tool => ({ ...tool, name: `mcp__fixture__${tool.name}` })); - const response = await fetch(new URL("/v1/responses", server.url), { - method: "POST", - headers: { "content-type": "application/json", authorization: "Bearer fixture-client", "x-codex-turn-metadata": "{}" }, - body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools }), - }); - expect(response.status).toBe(400); - expect(upstreamCalls).toBe(0); + const result = await send("/v1/responses", tools, { "x-codex-turn-metadata": "{}", session_id: "catalog-metadata-only" }); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); + }); + + test.each([ + ["originator", "codex_cli_rs"], + ["user-agent", "codex_cli_rs/0.166.0"], + ])("the canonical Codex %s alone is enough for the diagnostic", async (header, value) => { + const result = await send("/v1/responses", mcpTools(), { [header]: value, session_id: `catalog-${header}-only` }); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); }); - test("Responses Lite additional_tools declarations are checked before dispatch", async () => { - const response = await post("/v1/responses", [], true, { + test.each([ + ["originator", "my-codex-wrapper/1"], + ["user-agent", "my-codex-wrapper/1"], + ])("a generic %s containing codex is not diagnosed", async (header, value) => { + const result = await send("/v1/responses", mcpTools(), { [header]: value, session_id: `catalog-generic-${header}` }); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); + }); + + test("current Responses Lite additional_tools declarations are diagnosed", async () => { + const result = await send("/v1/responses", [], codexHeaders(), { input: [{ type: "additional_tools", role: "developer", tools: mcpTools() }, { role: "user", content: "hello" }], }); - expect(response.status).toBe(400); - expect(upstreamCalls).toBe(0); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); }); - test.each(["/v1/responses/compact", "/v1/responses"])("%s compaction history does not trigger a reload refusal", async path => { - const response = await post(path, [], true, { + test.each([ + ["with a small current declaration", true], + ["without a current declaration", false], + ])("a resumed thread's historical eager additional_tools are not counted (%s)", async (_label, currentDeclaration) => { + const result = await send("/v1/responses", [], codexHeaders(), { input: [ { type: "additional_tools", role: "developer", tools: mcpTools() }, { role: "user", content: "Earlier request" }, { role: "assistant", content: "Earlier response" }, - ...(path === "/v1/responses" ? [{ type: "compaction_trigger" }] : []), + ...(currentDeclaration ? [{ type: "additional_tools", role: "developer", tools: mcpTools(false, 3) }] : []), + { role: "user", content: "Current request" }, ], }); - expect({ status: response.status, body: await response.clone().text() }).toMatchObject({ status: 200 }); - await response.text(); + expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); }); - test.each([ - ["originator", "my-codex-wrapper/1"], - ["user-agent", "my-codex-wrapper/1"], - ])("a generic %s containing codex remains allowed", async (header, value) => { - const response = await fetch(new URL("/v1/responses", server.url), { - method: "POST", - headers: { - "content-type": "application/json", authorization: "Bearer fixture-client", - [header]: value, - }, - body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools: mcpTools() }), + test.each(["/v1/responses/compact", "/v1/responses"])("%s compaction history is not diagnosed", async path => { + const result = await send(path, [], codexHeaders(), { + input: [ + { type: "additional_tools", role: "developer", tools: mcpTools() }, + { role: "user", content: "Earlier request" }, + { role: "assistant", content: "Earlier response" }, + ...(path === "/v1/responses" ? [{ type: "compaction_trigger" }] : []), + ], }); - expect(response.status).toBe(200); - await response.text(); + expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); }); - test("the canonical Codex user-agent alone still triggers the reload refusal", async () => { - const response = await fetch(new URL("/v1/responses", server.url), { - method: "POST", - headers: { - "content-type": "application/json", authorization: "Bearer fixture-client", - "user-agent": "codex_cli_rs/0.166.0", - }, - body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools: mcpTools() }), - }); - expect(response.status).toBe(400); - expect(await response.json()).toMatchObject({ error: { code: "codex_catalog_reload_required" } }); - expect(upstreamCalls).toBe(0); - }); - - test("a small explicitly loaded MCP subset is not rejected", async () => { - const declarations = mcpTools(); - declarations[0]!.tools.length = 3; - const response = await post("/v1/responses", declarations); - expect(response.status).toBe(200); - await response.text(); + test.each([ + ["a small explicitly loaded MCP subset", mcpTools(false, 3)], + ["deferred MCP definitions with tool search", [...mcpTools(true), { type: "tool_search", execution: "client" }]], + ])("%s is not diagnosed", async (_label, tools) => { + const result = await send("/v1/responses", tools, codexHeaders()); + expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); }); - test("deferred MCP definitions with tool search remain usable", async () => { - const response = await post("/v1/responses", [...mcpTools(true), { type: "tool_search", execution: "client" }]); - expect(response.status).toBe(200); - await response.text(); + test("ordinary clients choosing an eager MCP catalog are not diagnosed", async () => { + const result = await send("/v1/responses", mcpTools(), { session_id: "catalog-ordinary" }); + expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); }); - test("ordinary clients can explicitly choose an eager MCP catalog", async () => { - const response = await post("/v1/responses", mcpTools(), false); - expect(response.status).toBe(200); - await response.text(); + test("code-mode examples and historical text are not counted as declarations", async () => { + const example = "Example: declare const tools: { mcp__fixture__read(args: {}): Promise; };"; + const result = await send("/v1/responses", [renderedExec([], example)], codexHeaders(), + { input: [{ role: "user", content: JSON.stringify(mcpTools()) }] }); + expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); }); - test("code-mode discovery does not count examples or historical text as declarations", async () => { - const response = await post("/v1/responses", [{ - type: "custom", name: "exec", description: "JavaScript in a V8 isolate. Discover helpers from ALL_TOOLS; example mcp__fixture__read.", - }], true, { input: [{ role: "user", content: JSON.stringify(mcpTools()) }] }); - expect(response.status).toBe(200); - await response.text(); + test("an expanded code-mode exec description is diagnosed", async () => { + const names = Array.from({ length: EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD }, (_value, index) => `mcp__fixture__tool_${index}`); + const result = await send("/v1/responses", [renderedExec(names)], codexHeaders()); + expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); }); - test("an expanded code-mode description is refused too", async () => { - const description = "JavaScript in a V8 isolate. ALL_TOOLS\n" + Array.from({ length: 64 }, (_value, index) => - `declare const tools: { mcp__fixture__tool_${index}(args: {}): Promise; };`, - ).join("\n"); - const response = await post("/v1/responses", [{ type: "custom", name: "exec", description }]); - expect(response.status).toBe(400); - expect(await response.json()).toMatchObject({ error: { code: "codex_catalog_reload_required" } }); - expect(upstreamCalls).toBe(0); + test("a Codex WebSocket turn is diagnosed from its handshake identity and still served", async () => { + const url = new URL("/v1/responses", server.url); + url.protocol = "ws:"; + const socket = new WebSocket(url, { headers: { authorization: "Bearer fixture-client", ...codexHeaders() } } as unknown as string[]); + try { + const terminal = new Promise((resolve, reject) => { + socket.addEventListener("message", event => { + const frame = JSON.parse(String(event.data)) as { type?: string }; + if (frame.type === "response.completed" || frame.type === "response.failed" || frame.type === "error") resolve(frame.type); + }); + socket.addEventListener("close", () => reject(new Error("socket closed before a terminal frame")), { once: true }); + }); + await new Promise((resolve, reject) => { + socket.addEventListener("open", () => resolve(), { once: true }); + socket.addEventListener("error", () => reject(new Error("socket handshake failed")), { once: true }); + }); + socket.send(JSON.stringify({ type: "response.create", model: "fixture/model", input: [ + { type: "additional_tools", role: "developer", tools: mcpTools() }, + { type: "message", role: "user", content: "hello" }, + ] })); + expect(await terminal).toBe("response.completed"); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); + } finally { + socket.close(); + } }); }); From 0de3b6aa4e2858fd385875aa918b81a046949c26 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:20:37 +0900 Subject: [PATCH 4/9] fix(responses): count Codex catalogs in the real Responses Lite layout Review of 37cfccdb on the codex-rs request shapes: a first turn sends the additional_tools catalog, two developer messages and a user-role environment context ahead of the prompt, so walking back from the last user input stopped before the catalog and counted 0; and Responses Lite nests Code Mode exec inside the functions namespace, which was skipped. - The current turn now starts after the last model output item (assistant message, *_call, reasoning, compaction); a first turn has none. A resumed thread's earlier declarations stay history. - Code Mode exec is read bare or inside the functions namespace. - Tests use the codex-rs first-turn layout (HTTP and WebSocket), a resumed thread in that layout, and a namespaced exec; the three positive cases fail on 37cfccdb. Co-authored-by: atinseau <65445747+atinseau@users.noreply.github.com> --- src/server/responses/codex-tool-catalog.ts | 26 +++++----- structure/catalog.md | 4 +- .../codex-eager-tool-catalog.test.ts | 48 +++++++++++++++---- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index 39528ddec58..e0b519128de 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -30,8 +30,9 @@ function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: if (tool.defer_loading === true || typeof tool.name !== "string") continue; const name = namespace ? `${namespace}__${tool.name}` : tool.name; if (name.startsWith("mcp__")) names.add(name); - // Code Mode renders each enabled, non-deferred nested tool into the exec description. - if (!namespace && spec.type === "custom") { + // Code Mode renders each enabled, non-deferred nested tool into the exec description. Codex + // sends exec bare or, on Responses Lite, inside its `functions` namespace. + if ((namespace === undefined || namespace === "functions") && spec.type === "custom") { for (const nested of codeModeRenderedToolNames(tool.name, tool.description)) { if (nested.startsWith("mcp__")) names.add(nested); } @@ -39,26 +40,25 @@ function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: } } -/** A message that opens a Responses turn ahead of its user input, such as instructions. */ -function isTurnPreamble(item: unknown): boolean { +/** An item the model produced: assistant text, reasoning, a tool call or a compaction. */ +function isModelOutput(item: unknown): boolean { if (!isRecord(item)) return false; - if (item.type === "additional_tools") return true; - return (item.type === undefined || item.type === "message") && (item.role === "developer" || item.role === "system"); + if (item.role === "assistant") return true; + const type = typeof item.type === "string" ? item.type : ""; + return type.endsWith("_call") || type === "reasoning" + || type === "compaction" || type === "compaction_summary" || type === "context_compaction"; } /** * Responses Lite `additional_tools` declared by the current turn. A resumed thread resends its - * whole history, including earlier turns' declarations, so only the declarations directly ahead of - * the last user input, or after it, belong to this request. Without user input every item counts. + * whole history, including earlier turns' declarations, so only items after the last model output + * belong to this request. A first turn has none, so its catalog at the head of `input` counts even + * behind Codex's developer and environment-context messages. */ function currentTurnAdditionalTools(input: unknown[]): unknown[][] { let start = 0; for (let index = input.length - 1; index >= 0; index -= 1) { - const item = input[index]; - if (!isRecord(item) || item.role !== "user") continue; - start = index; - while (start > 0 && isTurnPreamble(input[start - 1])) start -= 1; - break; + if (isModelOutput(input[index])) { start = index + 1; break; } } const groups: unknown[][] = []; for (const item of input.slice(start)) { diff --git a/structure/catalog.md b/structure/catalog.md index 4e6b2ca787d..e6ba7076a03 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -471,8 +471,8 @@ this can exceed the 120,000-byte `McpTools` ceiling and drop `exec` and `wait` ( non-Cursor routes keep `web_search_tool_type: "text_and_image"`; Cursor omits it because runTurn bypasses the sidecar. Canonical Codex identities or Codex turn metadata activate a diagnostic, never a refusal: at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64) eager MCP -definitions in top-level `tools`, current-turn Responses Lite `additional_tools` (directly ahead of or after the last user input; a resumed -thread's earlier declarations are history) or rendered Code Mode `exec` sections, one warning per session logs only the count and threshold and the +definitions in top-level `tools`, current-turn Responses Lite `additional_tools` (after the last model output item; a resumed thread's earlier +declarations are history) or rendered Code Mode `exec` sections (bare or in `functions`), one warning per session logs only the count and threshold and the request is forwarded unchanged. Deferred definitions are excluded; compaction (V1 handoff, V2 `compaction_trigger`) counts top-level `tools` only. Helper: `src/server/responses/codex-tool-catalog.ts`; test: `tests/responses/codex-eager-tool-catalog.test.ts`. diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index 5e753bcbe16..ade357c8021 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -46,6 +46,24 @@ function catalogWarnings(): string[] { return warn.mock.calls.map(call => String(call[0])).filter(text => text.startsWith("[codex-catalog]")); } +function message(role: string, text: string): Record { + return { type: "message", role, content: [{ type: role === "assistant" ? "output_text" : "input_text", text }] }; +} + +/** + * A Responses Lite first turn as codex-rs sends it (incremental_tools / responses_lite): the tool + * catalog, two developer messages, a user-role environment context, then the user's prompt. + */ +function liteFirstTurn(tools: unknown[]): unknown[] { + return [ + { type: "additional_tools", role: "developer", tools }, + message("developer", "fixture"), + message("developer", "fixture"), + message("user", "/fixture"), + message("user", "hello"), + ]; +} + beforeEach(() => { home = createTempHome("ocx-codex-tool-catalog-"); upstreamCalls = 0; @@ -150,13 +168,23 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia expect(catalogWarnings()).toHaveLength(0); }); - test("current Responses Lite additional_tools declarations are diagnosed", async () => { + test.each([ + ["directly ahead of the prompt", [{ type: "additional_tools", role: "developer", tools: mcpTools() }, { role: "user", content: "hello" }]], + ["in the codex-rs first-turn layout", liteFirstTurn(mcpTools())], + ])("current Responses Lite additional_tools declarations are diagnosed (%s)", async (_label, input) => { + const result = await send("/v1/responses", [], codexHeaders(), { input }); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); + }); + + test("a resumed codex-rs thread's first-turn catalog is history", async () => { const result = await send("/v1/responses", [], codexHeaders(), { - input: [{ type: "additional_tools", role: "developer", tools: mcpTools() }, { role: "user", content: "hello" }], + input: [...liteFirstTurn(mcpTools()), message("assistant", "Earlier response"), message("user", "Current request")], }); expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); - expect(catalogWarnings()).toHaveLength(1); + expect(catalogWarnings()).toHaveLength(0); }); test.each([ @@ -217,9 +245,14 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia expect(catalogWarnings()).toHaveLength(0); }); - test("an expanded code-mode exec description is diagnosed", async () => { + test.each([ + ["bare", false], + ["inside the Responses Lite functions namespace", true], + ])("an expanded code-mode exec description is diagnosed (%s)", async (_label, namespaced) => { const names = Array.from({ length: EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD }, (_value, index) => `mcp__fixture__tool_${index}`); - const result = await send("/v1/responses", [renderedExec(names)], codexHeaders()); + const exec = renderedExec(names); + const tools = namespaced ? [{ type: "namespace", name: "functions", description: "", tools: [exec] }] : [exec]; + const result = await send("/v1/responses", tools, codexHeaders()); expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); expect(catalogWarnings()).toHaveLength(1); @@ -241,10 +274,7 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia socket.addEventListener("open", () => resolve(), { once: true }); socket.addEventListener("error", () => reject(new Error("socket handshake failed")), { once: true }); }); - socket.send(JSON.stringify({ type: "response.create", model: "fixture/model", input: [ - { type: "additional_tools", role: "developer", tools: mcpTools() }, - { type: "message", role: "user", content: "hello" }, - ] })); + socket.send(JSON.stringify({ type: "response.create", model: "fixture/model", input: liteFirstTurn(mcpTools()) })); expect(await terminal).toBe("response.completed"); expect(upstreamCalls).toBe(1); expect(catalogWarnings()).toHaveLength(1); From 019bc89a36ca82878fb63dbe79e7ec922d1c82e1 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:05:34 +0900 Subject: [PATCH 5/9] fix(responses): keep the eager catalog diagnostic bounded and non-fatal Review of 0de3b6aa: collectEagerMcpTools recursed into nested namespaces without a depth bound, so a Codex-identified body with ~20,000 nested namespaces overflowed the stack and the request failed with 500 where dev forwards it. Codex namespaces are one level deep, so only top-level namespaces are read, and warnEagerCodexToolCatalog ignores any counting failure so the diagnostic can never change the request. Tests: a 25,000-deep body neither throws nor counts (RangeError before the fix); a namespace nested in a namespace is not counted; history whose last model output is a tool call, reasoning or a compaction keeps the earlier catalog out of the count (pins the model-output boundary). Co-authored-by: atinseau <65445747+atinseau@users.noreply.github.com> --- src/server/responses/codex-tool-catalog.ts | 9 +++-- .../codex-eager-tool-catalog.test.ts | 36 ++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index e0b519128de..a1721e21fd7 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -22,8 +22,10 @@ function isRecord(value: unknown): value is Record { function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: string): void { for (const spec of specs) { if (!isRecord(spec) || spec.defer_loading === true) continue; + // Codex namespaces are one level deep. A namespace nested inside another is not a Codex + // declaration, and recursing into caller-shaped depth could exhaust the stack. if (spec.type === "namespace" && typeof spec.name === "string" && Array.isArray(spec.tools)) { - collectEagerMcpTools(spec.tools, names, spec.name); + if (namespace === undefined) collectEagerMcpTools(spec.tools, names, spec.name); continue; } const tool = isRecord(spec.function) ? spec.function : spec; @@ -91,7 +93,10 @@ export function eagerCodexMcpToolCount(body: unknown, headers: Headers, surface: * request without a session identity shares one anonymous slot. */ export function warnEagerCodexToolCatalog(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): void { - const count = eagerCodexMcpToolCount(body, headers, surface); + // A diagnostic must never change the request it observes: any failure while counting a + // malformed body skips the warning and lets the request continue exactly as before. + let count: number; + try { count = eagerCodexMcpToolCount(body, headers, surface); } catch { return; } if (count < EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD) return; const session = sessionLaneIdFromRequest(headers) ?? ""; if (warnedSessions.has(session)) return; diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index ade357c8021..9ac7e0158b1 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -1,7 +1,11 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { saveConfig } from "../../src/config"; import { startServer } from "../../src/server"; -import { EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD } from "../../src/server/responses/codex-tool-catalog"; +import { + EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD, + eagerCodexMcpToolCount, + warnEagerCodexToolCatalog, +} from "../../src/server/responses/codex-tool-catalog"; import type { OcxConfig } from "../../src/types"; import { createTempHome } from "../helpers/temp-home"; import { ownedServiceHomeInspection } from "../helpers/owned-service-home-inspection"; @@ -283,3 +287,33 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia } }); }); + +describe("the eager catalog count stays a bounded, side-effect-free diagnostic", () => { + const headers = new Headers({ originator: "codex_cli_rs" }); + + test.each([ + ["a tool call and its output", [ + { type: "function_call", call_id: "call_1", name: "mcp__fixture__tool_0", arguments: "{}" }, + { type: "function_call_output", call_id: "call_1", output: "ok" }, + ]], + ["reasoning", [{ type: "reasoning", summary: [] }]], + ["a compaction", [{ type: "compaction", encrypted_content: "opaque" }]], + ])("history whose last model output is %s keeps the earlier catalog out of the count", (_label, tail) => { + const input = [...liteFirstTurn(mcpTools()), ...tail]; + expect(eagerCodexMcpToolCount({ input }, headers)).toBe(0); + expect(eagerCodexMcpToolCount({ input: liteFirstTurn(mcpTools()) }, headers)).toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + }); + + test("a deeply nested namespace body neither throws nor counts", () => { + let tools: unknown[] = mcpTools(); + for (let depth = 0; depth < 25_000; depth += 1) tools = [{ type: "namespace", name: `mcp__nest_${depth % 7}`, tools }]; + const body = { model: "fixture/model", input: "hello", tools }; + expect(() => warnEagerCodexToolCatalog(body, headers)).not.toThrow(); + expect(eagerCodexMcpToolCount(body, headers)).toBe(0); + }); + + test("a namespace nested inside a namespace is not a Codex declaration", () => { + const body = { input: "hello", tools: [{ type: "namespace", name: "mcp__outer", tools: mcpTools() }] }; + expect(eagerCodexMcpToolCount(body, headers)).toBe(0); + }); +}); From 4679c29af311f0311e92f20e3ddb447ec3ba763e Mon Sep 17 00:00:00 2001 From: cpa Date: Tue, 10 Nov 2026 20:45:27 +0900 Subject: [PATCH 6/9] fix(responses): count the catalog each Codex request actually carries Count every client-sent additional_tools declaration instead of only those after the last model output: Codex resends a window's catalog at the head of each request, so the positional scan dropped a fresh prefix that preceded replayed tool calls and misread a resumed window as history. Run the diagnostic after expandPreviousResponseInput and pass the replay prefix, so only history the proxy itself restored is excluded. Count it for combo-routed turns at the ladder, which owns the logical request, because concrete children would never report it. Dedup now keeps a bounded LRU that refreshes recency on every request, so an active session is not the one evicted. A request with no session identity warns once per logical request through its log context instead of sharing one anonymous slot that suppressed every later anonymous client. structure/catalog.md keeps dev's mac-app-bundle and Qoder text within the 600-line budget; the docs page and ADR-0022 describe the final behaviour. --- .../docs/reference/configuration/catalog.md | 30 +++-- src/server/responses/codex-tool-catalog.ts | 112 +++++++++++----- src/server/responses/compact.ts | 3 +- src/server/responses/core-combo.ts | 12 ++ src/server/responses/request-prepare.ts | 16 ++- structure/catalog.md | 10 +- ...routed-tool-discovery-and-hosted-search.md | 6 +- .../codex-eager-tool-catalog.test.ts | 123 +++++++++++++++--- 8 files changed, 234 insertions(+), 78 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/catalog.md b/docs-site/src/content/docs/reference/configuration/catalog.md index 70c3ce7ac14..3726940382f 100644 --- a/docs-site/src/content/docs/reference/configuration/catalog.md +++ b/docs-site/src/content/docs/reference/configuration/catalog.md @@ -12,17 +12,25 @@ file does not update a client that is already running. A Codex client with missing or stale model metadata can send every MCP tool definition eagerly instead of using deferred tool discovery, which makes each prompt much larger. When a Codex request declares 64 or more eager MCP tool definitions, OpenCodex logs a -warning once per session with the count and the threshold, then forwards the request -unchanged. The warning contains no tool names or request content. Run the restart -command to reload the catalog. - -The count covers top-level tools, the current turn's Responses Lite `additional_tools` -declarations, and MCP tools rendered into the Code Mode `exec` description. Declarations -from earlier turns of a resumed thread are history and are not counted; compaction, -including Responses API compaction triggers, counts only the top-level tool catalog. -The check recognizes canonical Codex client identities or Codex turn metadata; an -incidental `codex` substring in another client's name does not activate it. Explicitly -deferred tools, conversation text, and other API clients are not counted. +warning with the count and the threshold, then forwards the request unchanged. The +warning contains no tool names or request content. Run the restart command to reload the +catalog. + +The warning is shown once per session. A request without a session identity has no +session to warn once per, so it is warned once per request instead: a retry or compaction +handoff of the same request stays silent, and the next anonymous request warns again. +The session list is bounded, so an install running more than 1024 concurrent sessions can +see one repeat warning from the session evicted after the longest idle time. + +The count covers top-level tools, the Responses Lite `additional_tools` declarations the +client sent with this request, and MCP tools rendered into the Code Mode `exec` +description. Codex resends the window's catalog at the head of every request, so a +resumed turn's client-sent declarations count too; only history the proxy itself restored +from `previous_response_id` state is excluded. Compaction, including Responses API +compaction triggers, counts only the top-level tool catalog. The check recognizes +canonical Codex client identities or Codex turn metadata; an incidental `codex` substring +in another client's name does not activate it. Explicitly deferred tools, conversation +text, and other API clients are not counted. Routed models advertise deferred discovery. In Code Mode, discover available tools through `ALL_TOOLS` and call them through `tools`; a separate `tool_search` function diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index a1721e21fd7..4a200317810 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -10,15 +10,35 @@ import { codeModeRenderedToolNames } from "./code-mode-collab-surface"; * rule, because an eager catalog stays a valid request (ADR-0022). */ export const EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD = 64; -/** Sessions already warned. Bounded so a long-lived proxy cannot grow it without limit. */ -const WARNED_SESSION_LIMIT = 1024; -const warnedSessions = new Set(); +/** + * Sessions already warned, least recently sent first. Bounded so a long-lived proxy cannot grow + * it without limit: the dedupe covers this many sessions and no more, so an operator running more + * sessions than that can see one repeat warning. Recency refreshes on every request a session + * sends, so the session being evicted is the one that has been idle longest. + */ +export const WARNED_SESSION_LIMIT = 1024; +const warnedSessions = new Map(); +/** Logical requests already warned without a session identity; weak so completed requests leave no trace. */ +const warnedAnonymousRequests = new WeakSet(); const CODEX_CLIENT_IDENTITY = /^(?:codex|codex_cli_rs|codex_app|codex_work_desktop|codexless_agent|Codex Desktop)(?:\/|$)/i; function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } +/** + * Which wire surface a request is being counted for. Compaction sends its history as `input` rather + * than as declarations, so only its top-level `tools` are this request's catalog. + */ +export function eagerCodexToolCatalogSurface( + body: unknown, + compactionRecoveryKind: string | undefined, +): "responses" | "compact" { + if (compactionRecoveryKind === "compaction-v1") return "compact"; + const input = isRecord(body) && Array.isArray(body.input) ? body.input : []; + return input.some(item => isRecord(item) && item.type === "compaction_trigger") ? "compact" : "responses"; +} + function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: string): void { for (const spec of specs) { if (!isRecord(spec) || spec.defer_loading === true) continue; @@ -42,28 +62,19 @@ function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: } } -/** An item the model produced: assistant text, reasoning, a tool call or a compaction. */ -function isModelOutput(item: unknown): boolean { - if (!isRecord(item)) return false; - if (item.role === "assistant") return true; - const type = typeof item.type === "string" ? item.type : ""; - return type.endsWith("_call") || type === "reasoning" - || type === "compaction" || type === "compaction_summary" || type === "context_compaction"; -} - /** - * Responses Lite `additional_tools` declared by the current turn. A resumed thread resends its - * whole history, including earlier turns' declarations, so only items after the last model output - * belong to this request. A first turn has none, so its catalog at the head of `input` counts even - * behind Codex's developer and environment-context messages. + * Responses Lite `additional_tools` the client sent for this request. + * + * Codex does not express "current turn" by position: a window resends its base catalog at the head + * of `input` on every request and appends an incremental `additional_tools` update wherever the + * change happened, including after a replayed tool call. Both are live declarations that this + * request pays for, so every client-sent item counts. Only what the proxy itself replayed from + * stored continuation state is history, and `currentTurnWireToolCatalogBody` removes it before + * this runs. */ -function currentTurnAdditionalTools(input: unknown[]): unknown[][] { - let start = 0; - for (let index = input.length - 1; index >= 0; index -= 1) { - if (isModelOutput(input[index])) { start = index + 1; break; } - } +function clientSentAdditionalTools(input: unknown[]): unknown[][] { const groups: unknown[][] = []; - for (const item of input.slice(start)) { + for (const item of input) { if (isRecord(item) && item.type === "additional_tools" && Array.isArray(item.tools)) groups.push(item.tools); } return groups; @@ -71,40 +82,71 @@ function currentTurnAdditionalTools(input: unknown[]): unknown[][] { /** * Eager MCP definitions a canonical Codex client declared for this request, or 0 for any other - * client. Compaction reads only top-level `tools`: its `input` is history. + * client. `replayPrefixLength` is the leading input the proxy restored from stored continuation + * state; those items are history and grant no declaration for this turn. Compaction reads only + * top-level `tools`: its `input` is history. */ -export function eagerCodexMcpToolCount(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): number { +export function eagerCodexMcpToolCount( + body: unknown, + headers: Headers, + surface: "responses" | "compact" = "responses", + replayPrefixLength?: number, +): number { if (!headers.has("x-codex-turn-metadata") && !CODEX_CLIENT_IDENTITY.test(headers.get("originator") ?? "") && !CODEX_CLIENT_IDENTITY.test(headers.get("user-agent") ?? "")) return 0; - const current = currentTurnWireToolCatalogBody(body, undefined); + const current = currentTurnWireToolCatalogBody(body, replayPrefixLength); if (!isRecord(current)) return 0; const names = new Set(); if (Array.isArray(current.tools)) collectEagerMcpTools(current.tools, names); if (surface === "responses" && Array.isArray(current.input)) { - for (const tools of currentTurnAdditionalTools(current.input)) collectEagerMcpTools(tools, names); + for (const tools of clientSentAdditionalTools(current.input)) collectEagerMcpTools(tools, names); } return names.size; } /** * Warn once per session when a Codex request carries an eagerly expanded MCP catalog. Only the - * count and the threshold are logged; tool names, bodies and session identifiers are not. A - * request without a session identity shares one anonymous slot. + * count and the threshold are logged; tool names, bodies and session identifiers are not. + * + * A request without a session identity has no session to warn once per, so the + * `logicalRequest` scope stands in for one: each logical request warns, and a retry, combo child + * or compaction replay that shares the scope does not repeat it. Callers that omit the scope fall + * back to the session only. */ -export function warnEagerCodexToolCatalog(body: unknown, headers: Headers, surface: "responses" | "compact" = "responses"): void { +export function warnEagerCodexToolCatalog( + body: unknown, + headers: Headers, + options: { surface?: "responses" | "compact"; replayPrefixLength?: number; logicalRequest?: object } = {}, +): void { + const surface = options.surface ?? "responses"; // A diagnostic must never change the request it observes: any failure while counting a // malformed body skips the warning and lets the request continue exactly as before. let count: number; - try { count = eagerCodexMcpToolCount(body, headers, surface); } catch { return; } + try { count = eagerCodexMcpToolCount(body, headers, surface, options.replayPrefixLength); } catch { return; } if (count < EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD) return; - const session = sessionLaneIdFromRequest(headers) ?? ""; - if (warnedSessions.has(session)) return; - if (warnedSessions.size >= WARNED_SESSION_LIMIT) warnedSessions.delete(warnedSessions.values().next().value!); - warnedSessions.add(session); + const session = sessionLaneIdFromRequest(headers); + if (session !== undefined) { + if (warnedSessions.has(session)) { + // Refresh recency: a session still sending traffic is never the one evicted next. + warnedSessions.delete(session); + warnedSessions.set(session, true); + return; + } + while (warnedSessions.size >= WARNED_SESSION_LIMIT) { + const oldest = warnedSessions.keys().next().value; + if (oldest === undefined) break; + warnedSessions.delete(oldest); + } + warnedSessions.set(session, true); + } else if (options.logicalRequest !== undefined) { + if (warnedAnonymousRequests.has(options.logicalRequest)) return; + warnedAnonymousRequests.add(options.logicalRequest); + } console.warn( `[codex-catalog] Codex sent ${count} eager MCP tool definitions (warning threshold ${EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD}) ` + "instead of deferred tool discovery; its model metadata may be missing or stale. The request was forwarded unchanged. " - + "Run 'ocx sync --restart-codex' to reload the catalog. Shown once per session.", + + "Run 'ocx sync --restart-codex' to reload the catalog. Shown once per session, or once per request " + + "when the client sends no session identity.", ); } diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 8990059a48e..39c687e7b4a 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -691,7 +691,8 @@ export async function handleResponsesCompact( } const clientModelLabel = options.clientModelLabel ?? clientFacingModelLabel(options.compactionRoutingOverride?.sourceModel ?? (body as { model?: unknown }).model); options = { ...options, clientModelLabel }; - warnEagerCodexToolCatalog(body, req.headers, "compact"); + // Compaction body `input` is forwarded history, so only the top-level catalog is counted. + warnEagerCodexToolCatalog(body, req.headers, { surface: "compact", logicalRequest: logCtx }); // Compaction forwards client history upstream too; keep local artifact paths out of it. redactHostedImageDisplayPaths(body); if (!options.compactionRoutingOverride) { diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index 337540ad8e4..2891ec7dd2c 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -62,10 +62,12 @@ import { formatErrorResponse } from "../../bridge"; import { SEND_BUDGET_EXHAUSTED_CODE } from "../../lib/errors"; import { expandPreviousResponseInput, + previousResponseReplayPrefixLength, previousResponseReplayFailure, previousResponseProviderState, } from "../../responses/state"; import { hasUnreadableEncryptedAgentTask } from "./encrypted-payload"; +import { eagerCodexToolCatalogSurface, warnEagerCodexToolCatalog } from "./codex-tool-catalog"; import { captureRouteStaticPolicy, routeConcreteModel, comboRouteDecisionTrace } from "../../router"; import { memoryModelRouteReason } from "./memory-models"; import { baseProviderLabel, poolAccountProviderLabel } from "../../providers/label"; @@ -469,6 +471,16 @@ export async function executeComboResponses( if (combo.imageInput === "disabled" && comboRequestHasImageInput(body)) { return formatErrorResponse(400, "invalid_request_error", "This combo does not accept image input"); } + // The combo ladder owns the whole logical request, so the catalog diagnostic is counted here for + // combo-routed turns; children run as concrete selections and would otherwise never report it. + // Counting after the replay expansion keeps restored history out of the tally. + if ((options.inboundWire ?? "responses") === "responses") { + warnEagerCodexToolCatalog(body, req.headers, { + surface: eagerCodexToolCatalogSurface(body, options.compactionRecoveryKind), + replayPrefixLength: previousResponseReplayPrefixLength(body), + logicalRequest: logCtx, + }); + } const comboReplaySnapshot = { sourceBody: body, previousResponseInputExpanded: body !== rawBody diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 98e7800f287..3aad5314181 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -69,6 +69,7 @@ import { import { copyPreviousResponseReplayProvenance, expandPreviousResponseInput, + previousResponseReplayPrefixLength, previousResponseReplayFailure, markBodyNonPersistable, previousResponseProviderState, @@ -183,7 +184,7 @@ import { } from "./account-change-state"; import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; -import { warnEagerCodexToolCatalog } from "./codex-tool-catalog"; +import { eagerCodexToolCatalogSurface, warnEagerCodexToolCatalog } from "./codex-tool-catalog"; /** Policy-routing evidence. The direct lane's settings-only body has no conversation to scan. */ function routeEvidence(options: Pick, parsed: OcxParsedRequest): PolicyRequestEvidence { @@ -324,10 +325,6 @@ export async function prepareResponsesRequest( && Array.isArray((body as { input?: unknown }).input) && ((body as { input: unknown[] }).input).some((item: unknown) => typeof item === "object" && item !== null && (item as { type?: string }).type === "compaction_trigger"); - if (inboundWire === "responses" && !concreteSelection) { - warnEagerCodexToolCatalog(body, req.headers, - options.compactionRecoveryKind === "compaction-v1" || isCompactionTrigger ? "compact" : "responses"); - } // Compaction may send the last client-visible bare model after a combo switch. // Configured selectors take precedence; otherwise recall before combo dispatch (#3891). if (!concreteSelection && !options.compactionRoutingOverride && body && typeof body === "object" && !Array.isArray(body)) { @@ -461,6 +458,15 @@ export async function prepareResponsesRequest( && typeof (body as { previous_response_id?: unknown }).previous_response_id === "string"); options.responsesInstructionImage ||= responsesHasInstructionImage(body); options.responsesFileImage ||= responsesHasFileImage(body); + // Counted after replay expansion so the catalog this request pays for is the one measured: + // input restored from stored continuation state is history, and the prefix removes it. + if (inboundWire === "responses" && !concreteSelection) { + warnEagerCodexToolCatalog(body, req.headers, { + surface: eagerCodexToolCatalogSurface(body, options.compactionRecoveryKind), + replayPrefixLength: previousResponseReplayPrefixLength(body), + logicalRequest: logCtx, + }); + } // Spawn-message compatibility (both directions): agent_message task payloads ride in // encrypted_content slots as plaintext. Rewrite them to input_text on the RAW body BEFORE diff --git a/structure/catalog.md b/structure/catalog.md index f9dd4c177a1..9abcd7773e6 100644 --- a/structure/catalog.md +++ b/structure/catalog.md @@ -470,11 +470,11 @@ declarations into `exec.description`, a measured 2.7x turn-1 payload increase (9 this can exceed the 120,000-byte `McpTools` ceiling and drop `exec` and `wait` (#1830). Hosted search is separate: non-Cursor routes keep `web_search_tool_type: "text_and_image"`; Cursor omits it because runTurn bypasses the sidecar. -Canonical Codex identities or Codex turn metadata activate a diagnostic, never a refusal: at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64) eager MCP -definitions in top-level `tools`, current-turn Responses Lite `additional_tools` (after the last model output item; a resumed thread's earlier -declarations are history) or rendered Code Mode `exec` sections (bare or in `functions`), one warning per session logs only the count and threshold and the -request is forwarded unchanged. Deferred definitions are excluded; compaction (V1 handoff, V2 `compaction_trigger`) counts top-level `tools` only. -Helper: `src/server/responses/codex-tool-catalog.ts`; test: `tests/responses/codex-eager-tool-catalog.test.ts`. +Canonical Codex identities or turn metadata activate a diagnostic, never a refusal: at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64) eager MCP definitions +in top-level `tools`, client-sent Responses Lite `additional_tools`, or rendered Code Mode `exec` sections, `src/server/responses/codex-tool-catalog.ts` logs +only the count and threshold and forwards the request unchanged. Codex resends a window's catalog at the head of each request, so position cannot mark +history; the count runs after `expandPreviousResponseInput` and excludes only the proxy-restored prefix. Deferred definitions and compaction `input` are +excluded. Dedup is one session, or one request without a session identity, in a bounded LRU. Test: `tests/responses/codex-eager-tool-catalog.test.ts`. > Decision record: [ADR-0022](decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md) diff --git a/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md b/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md index a1e9c8a4032..22a6f0ccdd8 100644 --- a/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md +++ b/structure/decisions/ADR-0022-routed-tool-discovery-and-hosted-search.md @@ -14,8 +14,8 @@ ## Decision record — eagerly expanded Codex MCP catalogs (#8133) - 목적과 의도: tell an operator when a Codex client expands every MCP tool into the request, which usually means its model metadata is missing or stale and every turn pays the full-catalog payload. -- 기존 구현 및 제약 조건: an eager catalog is a valid request, and no supported-version evidence shows that every client sending 64+ eager MCP tools has stale metadata (an older client or disabled tool search may send one legitimately). A resumed thread resends earlier turns' `additional_tools`, so a whole-input count misreads history as the current turn. +- 기존 구현 및 제약 조건: an eager catalog is a valid request, and no supported-version evidence shows that every client sending 64+ eager MCP tools has stale metadata (an older client or disabled tool search may send one legitimately). Codex resends the window's catalog at the head of every request, including after a resumed turn's replayed calls, so position alone cannot separate the current request's declarations from the proxy's own restored history: only `previous_response_id` replay provenance can, and that prefix exists only after expansion. - 검토한 주요 대안: refuse with HTTP 400 `codex_catalog_reload_required`; refuse with a config opt-out; warn without refusing. -- 선택한 방식: warn once per session at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64), logging only the count and threshold, and forward the request unchanged; count only the current turn's declarations. +- 선택한 방식: warn at `EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD` (64), logging only the count and threshold, and forward the request unchanged. Count top-level `tools`, every client-sent `additional_tools` item, and rendered Code Mode sections, after `expandPreviousResponseInput` and with its replay prefix excluded. Dedup is once per session; without a session identity it is once per logical request, and the session set is a bounded LRU so the evicted entry is the longest-idle session. - 다른 대안 대신 이 방식을 선택한 이유: a refusal turns a recoverable cost problem into a hard outage for any client the heuristic misjudges, and its remedy cannot help a client whose catalog is eager by design. The diagnostic keeps every existing request working. -- 장점, 단점 및 영향: no request changes behavior; operators get one actionable line per session. A stale client keeps paying the larger payload until it is restarted. +- 장점, 단점 및 영향: no request changes behavior; operators get one actionable line per session, or per anonymous request, counting what that request actually pays for. A stale client keeps paying the larger payload until it is restarted, and an install running more than the 1024-session bound can see one repeat warning from the longest-idle evicted session. diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index 9ac7e0158b1..bef72cf1c8c 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -3,10 +3,12 @@ import { saveConfig } from "../../src/config"; import { startServer } from "../../src/server"; import { EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD, + WARNED_SESSION_LIMIT, eagerCodexMcpToolCount, warnEagerCodexToolCatalog, } from "../../src/server/responses/codex-tool-catalog"; import type { OcxConfig } from "../../src/types"; +import { clearResponseStateForTests, rememberResponseState } from "../../src/responses/state"; import { createTempHome } from "../helpers/temp-home"; import { ownedServiceHomeInspection } from "../helpers/owned-service-home-inspection"; @@ -95,6 +97,7 @@ beforeEach(() => { saveConfig({ port: 0, hostname: "127.0.0.1", websockets: true, defaultProvider: "fixture", providers: { fixture: { adapter: "openai-chat", baseUrl: new URL("/v1", upstream.url).href, apiKey: "fixture-key", liveModels: false, allowPrivateNetwork: true } }, + combos: { catalog: { strategy: "failover", targets: [{ provider: "fixture", model: "model" }] } }, } as OcxConfig); server = startServer(0, { inspectNativeCodexOwnership }); }); @@ -103,6 +106,7 @@ afterEach(async () => { warn.mockRestore(); await server?.stop(true); upstream?.stop(true); + clearResponseStateForTests(); home.remove(); }); @@ -182,30 +186,40 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia expect(catalogWarnings()).toHaveLength(1); }); - test("a resumed codex-rs thread's first-turn catalog is history", async () => { + test("a resumed codex-rs window's client-sent catalog is counted, not treated as history", async () => { + // Codex resends the window's base catalog when its input diverges, and the request pays for + // every definition it carries. The client's own resend is this request's catalog. const result = await send("/v1/responses", [], codexHeaders(), { input: [...liteFirstTurn(mcpTools()), message("assistant", "Earlier response"), message("user", "Current request")], }); expect(result.status, result.text).toBe(200); expect(upstreamCalls).toBe(1); - expect(catalogWarnings()).toHaveLength(0); + expect(catalogWarnings()).toHaveLength(1); }); - test.each([ - ["with a small current declaration", true], - ["without a current declaration", false], - ])("a resumed thread's historical eager additional_tools are not counted (%s)", async (_label, currentDeclaration) => { + test("a combo-routed Codex turn is diagnosed once for the logical request", async () => { + const result = await send("/v1/responses", mcpTools(), codexHeaders(), { model: "combo/catalog" }); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); + }); + + test("a previous_response_id continuation that replays stored history is not re-counted", async () => { + // Codex carries its own history on every turn. This entry's request held the eager catalog, + // so the continuation's replayed prefix holds it too; only genuinely new declarations count. + const storedInput = liteFirstTurn(mcpTools()); + rememberResponseState( + { model: "fixture/model", input: storedInput }, + { id: "resp_catalog_replay", status: "completed", + output: [{ type: "message", id: "msg_catalog_replay", role: "assistant", content: "Earlier response" }] }, + ); const result = await send("/v1/responses", [], codexHeaders(), { - input: [ - { type: "additional_tools", role: "developer", tools: mcpTools() }, - { role: "user", content: "Earlier request" }, - { role: "assistant", content: "Earlier response" }, - ...(currentDeclaration ? [{ type: "additional_tools", role: "developer", tools: mcpTools(false, 3) }] : []), - { role: "user", content: "Current request" }, - ], + previous_response_id: "resp_catalog_replay", + input: [...storedInput, + { type: "message", id: "msg_catalog_replay", role: "assistant", content: "Earlier response" }, + message("user", "Current request")], }); expect(result.status, result.text).toBe(200); - expect(upstreamCalls).toBe(1); expect(catalogWarnings()).toHaveLength(0); }); @@ -296,12 +310,85 @@ describe("the eager catalog count stays a bounded, side-effect-free diagnostic", { type: "function_call", call_id: "call_1", name: "mcp__fixture__tool_0", arguments: "{}" }, { type: "function_call_output", call_id: "call_1", output: "ok" }, ]], + ["an assistant message", [message("assistant", "Earlier response")]], ["reasoning", [{ type: "reasoning", summary: [] }]], ["a compaction", [{ type: "compaction", encrypted_content: "opaque" }]], - ])("history whose last model output is %s keeps the earlier catalog out of the count", (_label, tail) => { - const input = [...liteFirstTurn(mcpTools()), ...tail]; - expect(eagerCodexMcpToolCount({ input }, headers)).toBe(0); - expect(eagerCodexMcpToolCount({ input: liteFirstTurn(mcpTools()) }, headers)).toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + ])("a trailing %s neither adds nor removes declarations", (_label, tail) => { + // The client resends this window's catalog on every request, so the definitions it carries are + // this request's cost no matter what the model produced afterwards. + expect(eagerCodexMcpToolCount({ input: [...liteFirstTurn(mcpTools()), ...tail] }, headers)) + .toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + }); + + test("a continuation's fresh additional_tools catalog ahead of replayed calls is counted", () => { + // A resumed Codex turn resends the window's base catalog, then its own tool calls. The catalog + // precedes those calls, so a scan that starts after the last model output reads only history + // and misses the definitions this request actually carries. + const input = [ + ...liteFirstTurn(mcpTools()), + message("assistant", "Earlier response"), + { type: "function_call", call_id: "call_1", name: "wait", arguments: "{}" }, + { type: "function_call_output", call_id: "call_1", output: "ok" }, + message("user", "Current request"), + ]; + expect(eagerCodexMcpToolCount({ input }, headers)).toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + }); + + test("an incremental additional_tools update after a replayed step is counted", () => { + const input = [ + ...liteFirstTurn([{ type: "tool_search", execution: "client" }]), + { type: "reasoning", summary: [] }, + { type: "additional_tools", role: "developer", tools: mcpTools() }, + message("user", "Current request"), + ]; + expect(eagerCodexMcpToolCount({ input }, headers)).toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + }); + + test("input restored from stored continuation state is history, not this request's catalog", () => { + const stored = liteFirstTurn(mcpTools()); + const clientTools = [{ type: "namespace", name: "mcp__current", tools: mcpTools(false, 3)[0]!.tools }]; + const client = [{ type: "additional_tools", role: "developer", tools: clientTools }]; + const body = { input: [...stored, ...client] }; + expect(eagerCodexMcpToolCount(body, headers, "responses", stored.length)).toBe(3); + expect(eagerCodexMcpToolCount(body, headers)).toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD + 3); + }); + + test("an active session is not evicted before an idle one", () => { + const capacity = WARNED_SESSION_LIMIT; + const session = (index: number) => new Headers({ originator: "codex_cli_rs", session_id: `catalog-lru-${index}` }); + const eager = { input: "hello", tools: mcpTools() }; + for (let index = 0; index < capacity; index += 1) warnEagerCodexToolCatalog(eager, session(index)); + expect(catalogWarnings()).toHaveLength(capacity); + // The first session sends again, which must refresh its recency... + warnEagerCodexToolCatalog(eager, session(0)); + expect(catalogWarnings()).toHaveLength(capacity); + // ...so filling the bound again evicts the next idlest session, not this one. + warnEagerCodexToolCatalog(eager, session(capacity)); + warnEagerCodexToolCatalog(eager, session(0)); + expect(catalogWarnings()).toHaveLength(capacity + 1); + }); + + test("each sessionless logical request is warned, and one request is warned once", () => { + const eager = { input: "hello", tools: mcpTools() }; + const anonymous = new Headers({ originator: "codex_cli_rs" }); + const first = {}; + const second = {}; + warnEagerCodexToolCatalog(eager, anonymous, { logicalRequest: first }); + // A retry or compaction handoff of the same request shares its scope, so it stays silent. + warnEagerCodexToolCatalog(eager, anonymous, { logicalRequest: first }); + expect(catalogWarnings()).toHaveLength(1); + warnEagerCodexToolCatalog(eager, anonymous, { logicalRequest: second }); + expect(catalogWarnings()).toHaveLength(2); + }); + + test("a sessionless caller does not suppress a later anonymous caller's warning", async () => { + const anonymous = { "x-codex-turn-metadata": "{}" }; + for (const _ of [0, 1]) { + const result = await send("/v1/responses", mcpTools(), anonymous); + expect(result.status, result.text).toBe(200); + } + expect(upstreamCalls).toBe(2); + expect(catalogWarnings()).toHaveLength(2); }); test("a deeply nested namespace body neither throws nor counts", () => { From 9e4d8a09247d866f4b8d049f4f4cccb893fddc92 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Thu, 12 Nov 2026 11:10:39 +0900 Subject: [PATCH 7/9] fix(responses): count a client-resent catalog under previous_response_id When a resumed Codex turn resends its whole stored window under previous_response_id, the proxy prepends nothing (overlap skip), but the catalog diagnostic still excluded that client-carried prefix as restored history and the warning was lost. state.ts now records client-carried replays and exposes previousResponseRestoredPrefixLength, which the diagnostic uses. The warning and catalog guide now point at 'ocx system codex-restart --yes', since 'ocx sync --restart-codex' cannot restart a healthy install on request. --- .../docs/reference/configuration/catalog.md | 10 ++++--- src/responses/state.ts | 16 +++++++++++ src/server/responses/codex-tool-catalog.ts | 3 ++- src/server/responses/core-combo.ts | 4 +-- src/server/responses/request-prepare.ts | 4 +-- .../codex-eager-tool-catalog.test.ts | 27 ++++++++++++++++--- 6 files changed, 51 insertions(+), 13 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/catalog.md b/docs-site/src/content/docs/reference/configuration/catalog.md index 3726940382f..bc6bed900fd 100644 --- a/docs-site/src/content/docs/reference/configuration/catalog.md +++ b/docs-site/src/content/docs/reference/configuration/catalog.md @@ -13,8 +13,9 @@ A Codex client with missing or stale model metadata can send every MCP tool defi eagerly instead of using deferred tool discovery, which makes each prompt much larger. When a Codex request declares 64 or more eager MCP tool definitions, OpenCodex logs a warning with the count and the threshold, then forwards the request unchanged. The -warning contains no tool names or request content. Run the restart command to reload the -catalog. +warning contains no tool names or request content. Restart Codex to reload the catalog: +`ocx system codex-restart --yes` restarts it on request (it interrupts running turns), while +`ocx sync --restart-codex` restarts only after it writes a changed catalog or configuration. The warning is shown once per session. A request without a session identity has no session to warn once per, so it is warned once per request instead: a retry or compaction @@ -25,8 +26,9 @@ see one repeat warning from the session evicted after the longest idle time. The count covers top-level tools, the Responses Lite `additional_tools` declarations the client sent with this request, and MCP tools rendered into the Code Mode `exec` description. Codex resends the window's catalog at the head of every request, so a -resumed turn's client-sent declarations count too; only history the proxy itself restored -from `previous_response_id` state is excluded. Compaction, including Responses API +resumed turn's client-sent declarations count too, including a turn that resends its whole +stored window under `previous_response_id`; only history the proxy itself restored from +`previous_response_id` state is excluded. Compaction, including Responses API compaction triggers, counts only the top-level tool catalog. The check recognizes canonical Codex client identities or Codex turn metadata; an incidental `codex` substring in another client's name does not activate it. Explicitly deferred tools, conversation diff --git a/src/responses/state.ts b/src/responses/state.ts index 602d19a3724..630fffd3542 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -609,6 +609,9 @@ bindSpillQueueStore({ // once. It records the boundary whether the proxy prepended the history or the client already // carried it — the boundary is the same either way, and only its provenance differs. const replayedInputPrefixLengths = new WeakMap(); +// Bodies whose replay boundary the CLIENT carried (the overlap-skip path): nothing was prepended, +// so a consumer that must exclude only proxy-restored history reads a zero-length restored prefix. +const clientCarriedReplays = new WeakSet(); const replayFailures = new WeakMap(); let loaded = false; let persistTimer: ReturnType | null = null; @@ -1150,6 +1153,7 @@ export function expandPreviousResponseInput(body: unknown, clientThreadId?: stri // not re-acknowledge historical compaction markers (parser.ts) and stays visible to // guidance de-duplication (collaboration.ts). replayedInputPrefixLengths.set(unchanged, carried); + clientCarriedReplays.add(unchanged); return unchanged; } } @@ -1172,6 +1176,17 @@ export function previousResponseReplayPrefixLength(body: unknown): number { return replayedInputPrefixLengths.get(body) ?? 0; } +/** + * Number of leading input items the PROXY prepended from previous_response_id state. Unlike + * `previousResponseReplayPrefixLength`, this is zero when the client already resent the stored + * window verbatim (overlap skip): those items arrived on the wire from the client this turn. + */ +export function previousResponseRestoredPrefixLength(body: unknown): number { + if (!body || typeof body !== "object" || Array.isArray(body)) return 0; + if (clientCarriedReplays.has(body)) return 0; + return replayedInputPrefixLengths.get(body) ?? 0; +} + /** Copy proxy-private replay provenance to an internal clone with the same materialized input. */ export function copyPreviousResponseReplayProvenance(source: unknown, target: unknown): void { if (!source || typeof source !== "object" || Array.isArray(source)) return; @@ -1181,6 +1196,7 @@ export function copyPreviousResponseReplayProvenance(source: unknown, target: un const input = (target as { input?: unknown }).input; if (!Array.isArray(input) || prefixLength > input.length) return; replayedInputPrefixLengths.set(target, prefixLength); + if (clientCarriedReplays.has(source)) clientCarriedReplays.add(target); } /** True when this exact request could not replay because its task scope did not match. */ diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index 4a200317810..e2f6ba752b4 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -146,7 +146,8 @@ export function warnEagerCodexToolCatalog( console.warn( `[codex-catalog] Codex sent ${count} eager MCP tool definitions (warning threshold ${EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD}) ` + "instead of deferred tool discovery; its model metadata may be missing or stale. The request was forwarded unchanged. " - + "Run 'ocx sync --restart-codex' to reload the catalog. Shown once per session, or once per request " + + "Restart Codex to reload the catalog ('ocx system codex-restart --yes'; it interrupts running turns). " + + "Shown once per session, or once per request " + "when the client sends no session identity.", ); } diff --git a/src/server/responses/core-combo.ts b/src/server/responses/core-combo.ts index 2891ec7dd2c..339632cea3c 100644 --- a/src/server/responses/core-combo.ts +++ b/src/server/responses/core-combo.ts @@ -62,7 +62,7 @@ import { formatErrorResponse } from "../../bridge"; import { SEND_BUDGET_EXHAUSTED_CODE } from "../../lib/errors"; import { expandPreviousResponseInput, - previousResponseReplayPrefixLength, + previousResponseRestoredPrefixLength, previousResponseReplayFailure, previousResponseProviderState, } from "../../responses/state"; @@ -477,7 +477,7 @@ export async function executeComboResponses( if ((options.inboundWire ?? "responses") === "responses") { warnEagerCodexToolCatalog(body, req.headers, { surface: eagerCodexToolCatalogSurface(body, options.compactionRecoveryKind), - replayPrefixLength: previousResponseReplayPrefixLength(body), + replayPrefixLength: previousResponseRestoredPrefixLength(body), logicalRequest: logCtx, }); } diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 3aad5314181..d9de8558e87 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -69,7 +69,7 @@ import { import { copyPreviousResponseReplayProvenance, expandPreviousResponseInput, - previousResponseReplayPrefixLength, + previousResponseRestoredPrefixLength, previousResponseReplayFailure, markBodyNonPersistable, previousResponseProviderState, @@ -463,7 +463,7 @@ export async function prepareResponsesRequest( if (inboundWire === "responses" && !concreteSelection) { warnEagerCodexToolCatalog(body, req.headers, { surface: eagerCodexToolCatalogSurface(body, options.compactionRecoveryKind), - replayPrefixLength: previousResponseReplayPrefixLength(body), + replayPrefixLength: previousResponseRestoredPrefixLength(body), logicalRequest: logCtx, }); } diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index bef72cf1c8c..4c4ae5412f5 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -204,9 +204,28 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia expect(catalogWarnings()).toHaveLength(1); }); - test("a previous_response_id continuation that replays stored history is not re-counted", async () => { - // Codex carries its own history on every turn. This entry's request held the eager catalog, - // so the continuation's replayed prefix holds it too; only genuinely new declarations count. + test("history the proxy restores from previous_response_id is not re-counted", async () => { + // The client sends only its new input; the proxy prepends the stored entry, whose request held + // the eager catalog. That restored prefix is history this request did not send. + const storedInput = liteFirstTurn(mcpTools()); + rememberResponseState( + { model: "fixture/model", input: storedInput }, + { id: "resp_catalog_restored", status: "completed", + output: [{ type: "message", id: "msg_catalog_restored", role: "assistant", content: "Earlier response" }] }, + ); + const result = await send("/v1/responses", [], codexHeaders(), { + previous_response_id: "resp_catalog_restored", + input: [message("user", "Current request")], + }); + expect(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); + }); + + test("a client that resends its stored window under previous_response_id is counted", async () => { + // Codex carries its own history on every turn. When it resends the stored window verbatim the + // proxy prepends nothing (overlap skip), so every additional_tools item on the wire came from + // the client this turn and the eager catalog it carries is this request's cost. const storedInput = liteFirstTurn(mcpTools()); rememberResponseState( { model: "fixture/model", input: storedInput }, @@ -220,7 +239,7 @@ describe("an eagerly expanded Codex MCP catalog is forwarded with a one-time dia message("user", "Current request")], }); expect(result.status, result.text).toBe(200); - expect(catalogWarnings()).toHaveLength(0); + expect(catalogWarnings()).toHaveLength(1); }); test.each(["/v1/responses/compact", "/v1/responses"])("%s compaction history is not diagnosed", async path => { From a7ab21c8a872c9bc22c49f674fab7e0dcc377dfc Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Thu, 12 Nov 2026 11:11:42 +0900 Subject: [PATCH 8/9] refactor(responses): keep state.ts within its file-size cap Compress the client-carried replay marker and previousResponseRestoredPrefixLength to the eight lines the ratchet baseline allows (1371). --- src/responses/state.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/responses/state.ts b/src/responses/state.ts index 630fffd3542..7faf782bda1 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -609,9 +609,7 @@ bindSpillQueueStore({ // once. It records the boundary whether the proxy prepended the history or the client already // carried it — the boundary is the same either way, and only its provenance differs. const replayedInputPrefixLengths = new WeakMap(); -// Bodies whose replay boundary the CLIENT carried (the overlap-skip path): nothing was prepended, -// so a consumer that must exclude only proxy-restored history reads a zero-length restored prefix. -const clientCarriedReplays = new WeakSet(); +const clientCarriedReplays = new WeakSet(); // overlap skip: the client resent the window itself const replayFailures = new WeakMap(); let loaded = false; let persistTimer: ReturnType | null = null; @@ -1176,15 +1174,9 @@ export function previousResponseReplayPrefixLength(body: unknown): number { return replayedInputPrefixLengths.get(body) ?? 0; } -/** - * Number of leading input items the PROXY prepended from previous_response_id state. Unlike - * `previousResponseReplayPrefixLength`, this is zero when the client already resent the stored - * window verbatim (overlap skip): those items arrived on the wire from the client this turn. - */ +/** Leading items the PROXY prepended; 0 when the client resent the stored window itself (overlap skip). */ export function previousResponseRestoredPrefixLength(body: unknown): number { - if (!body || typeof body !== "object" || Array.isArray(body)) return 0; - if (clientCarriedReplays.has(body)) return 0; - return replayedInputPrefixLengths.get(body) ?? 0; + return body && typeof body === "object" && clientCarriedReplays.has(body) ? 0 : previousResponseReplayPrefixLength(body); } /** Copy proxy-private replay provenance to an internal clone with the same materialized input. */ From 338f04bc5d560b8f51a2d9bbecf79d1481dc0666 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Thu, 12 Nov 2026 12:51:29 +0900 Subject: [PATCH 9/9] fix(responses): count MCP tools in the functions group and refresh any warned session MCP declarations inside the reserved functions group were prefixed functions__ and missed the mcp__ check, so such a catalog was never counted. A warned session that sent only below-threshold traffic never refreshed its recency and could be evicted and warned again, contradicting the documented longest-idle eviction. Any request from a warned session now refreshes it before counting. --- src/server/responses/codex-tool-catalog.ts | 19 ++++++++------ .../codex-eager-tool-catalog.test.ts | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/server/responses/codex-tool-catalog.ts b/src/server/responses/codex-tool-catalog.ts index e2f6ba752b4..9be1c68fcda 100644 --- a/src/server/responses/codex-tool-catalog.ts +++ b/src/server/responses/codex-tool-catalog.ts @@ -50,7 +50,9 @@ function collectEagerMcpTools(specs: unknown[], names: Set, namespace?: } const tool = isRecord(spec.function) ? spec.function : spec; if (tool.defer_loading === true || typeof tool.name !== "string") continue; - const name = namespace ? `${namespace}__${tool.name}` : tool.name; + // `functions` is the reserved unnamespaced group (as in the undeclared-tool guard and parser), + // so an MCP tool declared inside it keeps its own `mcp__` name. + const name = namespace && namespace !== "functions" ? `${namespace}__${tool.name}` : tool.name; if (name.startsWith("mcp__")) names.add(name); // Code Mode renders each enabled, non-deferred nested tool into the exec description. Codex // sends exec bare or, on Responses Lite, inside its `functions` namespace. @@ -120,19 +122,20 @@ export function warnEagerCodexToolCatalog( options: { surface?: "responses" | "compact"; replayPrefixLength?: number; logicalRequest?: object } = {}, ): void { const surface = options.surface ?? "responses"; + const session = sessionLaneIdFromRequest(headers); + if (session !== undefined && warnedSessions.has(session)) { + // Refresh recency on every request the session sends, eager or not, so an active session is + // never the one evicted next. An already-warned session needs no count at all. + warnedSessions.delete(session); + warnedSessions.set(session, true); + return; + } // A diagnostic must never change the request it observes: any failure while counting a // malformed body skips the warning and lets the request continue exactly as before. let count: number; try { count = eagerCodexMcpToolCount(body, headers, surface, options.replayPrefixLength); } catch { return; } if (count < EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD) return; - const session = sessionLaneIdFromRequest(headers); if (session !== undefined) { - if (warnedSessions.has(session)) { - // Refresh recency: a session still sending traffic is never the one evicted next. - warnedSessions.delete(session); - warnedSessions.set(session, true); - return; - } while (warnedSessions.size >= WARNED_SESSION_LIMIT) { const oldest = warnedSessions.keys().next().value; if (oldest === undefined) break; diff --git a/tests/responses/codex-eager-tool-catalog.test.ts b/tests/responses/codex-eager-tool-catalog.test.ts index 4c4ae5412f5..ae1c9ec98fc 100644 --- a/tests/responses/codex-eager-tool-catalog.test.ts +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -387,6 +387,31 @@ describe("the eager catalog count stays a bounded, side-effect-free diagnostic", expect(catalogWarnings()).toHaveLength(capacity + 1); }); + test("a warned session stays resident while it sends only below-threshold traffic", () => { + const capacity = WARNED_SESSION_LIMIT; + const session = (index: number) => new Headers({ originator: "codex_cli_rs", session_id: `catalog-lru-small-${index}` }); + const eager = { input: "hello", tools: mcpTools() }; + const small = { input: "hello", tools: mcpTools(false, EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD - 1) }; + for (let index = 0; index < capacity; index += 1) warnEagerCodexToolCatalog(eager, session(index)); + // Session 0 keeps working with a smaller catalog; that traffic still refreshes its recency... + warnEagerCodexToolCatalog(small, session(0)); + warnEagerCodexToolCatalog(eager, session(capacity)); + expect(catalogWarnings()).toHaveLength(capacity + 1); + // ...so the next eager request from it is still deduplicated. + warnEagerCodexToolCatalog(eager, session(0)); + expect(catalogWarnings()).toHaveLength(capacity + 1); + }); + + test("MCP tools declared inside the reserved functions group are counted under their own names", () => { + const tools = [{ type: "namespace", name: "functions", tools: Array.from({ length: EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD }, (_value, index) => ({ + type: "function", name: `mcp__fixture__tool_${index}`, description: "Fixture tool", parameters: { type: "object", properties: {} }, + })) }]; + expect(eagerCodexMcpToolCount({ input: "hello", tools }, headers)).toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + // The same tools carried both inside `functions` and as a top-level group count once. + expect(eagerCodexMcpToolCount({ input: "hello", tools: [...tools, ...mcpTools()] }, headers)) + .toBe(EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD); + }); + test("each sessionless logical request is warned, and one request is warned once", () => { const eager = { input: "hello", tools: mcpTools() }; const anonymous = new Headers({ originator: "codex_cli_rs" });