diff --git a/docs-site/src/content/docs/reference/configuration/catalog.md b/docs-site/src/content/docs/reference/configuration/catalog.md index 360cf70e9cf..bc6bed900fd 100644 --- a/docs-site/src/content/docs/reference/configuration/catalog.md +++ b/docs-site/src/content/docs/reference/configuration/catalog.md @@ -3,6 +3,41 @@ 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. + +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 with the count and the threshold, then forwards the request unchanged. The +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 +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, 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 +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 +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-responses.json b/scripts/test-layout/layout-responses.json index e28aac8d851..e043f50d889 100644 --- a/scripts/test-layout/layout-responses.json +++ b/scripts/test-layout/layout-responses.json @@ -11,6 +11,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", "responses-workflow-send-ceiling.test.ts": "responses", diff --git a/src/responses/state.ts b/src/responses/state.ts index 602d19a3724..7faf782bda1 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -609,6 +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(); +const clientCarriedReplays = new WeakSet(); // overlap skip: the client resent the window itself const replayFailures = new WeakMap(); let loaded = false; let persistTimer: ReturnType | null = null; @@ -1150,6 +1151,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 +1174,11 @@ export function previousResponseReplayPrefixLength(body: unknown): number { return replayedInputPrefixLengths.get(body) ?? 0; } +/** Leading items the PROXY prepended; 0 when the client resent the stored window itself (overlap skip). */ +export function previousResponseRestoredPrefixLength(body: unknown): number { + 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. */ export function copyPreviousResponseReplayProvenance(source: unknown, target: unknown): void { if (!source || typeof source !== "object" || Array.isArray(source)) return; @@ -1181,6 +1188,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/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 new file mode 100644 index 00000000000..9be1c68fcda --- /dev/null +++ b/src/server/responses/codex-tool-catalog.ts @@ -0,0 +1,156 @@ +import { currentTurnWireToolCatalogBody } from "../responses-undeclared-tool-guard"; +import { sessionLaneIdFromRequest } from "../request-log-conversation"; +import { codeModeRenderedToolNames } from "./code-mode-collab-surface"; + +/** + * 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, 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; + // 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)) { + if (namespace === undefined) 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; + // `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. + if ((namespace === undefined || namespace === "functions") && spec.type === "custom") { + for (const nested of codeModeRenderedToolNames(tool.name, tool.description)) { + if (nested.startsWith("mcp__")) names.add(nested); + } + } + } +} + +/** + * 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 clientSentAdditionalTools(input: unknown[]): unknown[][] { + const groups: unknown[][] = []; + for (const item of input) { + 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. `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", + 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, 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 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 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, + 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; + if (session !== undefined) { + 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. " + + "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/compact.ts b/src/server/responses/compact.ts index 8867cdc3a78..6c0665e3297 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -238,6 +238,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 { warnEagerCodexToolCatalog } from "./codex-tool-catalog"; export const COMPACT_RESPONSE_MAX_BYTES = 32 * 1024 * 1024; @@ -696,6 +697,8 @@ export async function handleResponsesCompact( } const clientModelLabel = options.clientModelLabel ?? clientFacingModelLabel(options.compactionRoutingOverride?.sourceModel ?? (body as { model?: unknown }).model); options = { ...options, clientModelLabel }; + // 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..339632cea3c 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, + previousResponseRestoredPrefixLength, 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: previousResponseRestoredPrefixLength(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 7efbc2d0aff..d9de8558e87 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -69,6 +69,7 @@ import { import { copyPreviousResponseReplayProvenance, expandPreviousResponseInput, + previousResponseRestoredPrefixLength, previousResponseReplayFailure, markBodyNonPersistable, previousResponseProviderState, @@ -183,6 +184,7 @@ import { } from "./account-change-state"; import { redactHostedImageDisplayPaths } from "../responses-hosted-image-display"; +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 { @@ -456,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: previousResponseRestoredPrefixLength(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 1e96bd7999e..9abcd7773e6 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. +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 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 8c3a4984acd..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 @@ -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). 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 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, 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/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 43b1cbcbd5f..36d6a11d3df 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -109,6 +109,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/helpers/responses-core-source.ts b/tests/helpers/responses-core-source.ts index 541517462ce..ad2a30065c9 100644 --- a/tests/helpers/responses-core-source.ts +++ b/tests/helpers/responses-core-source.ts @@ -44,6 +44,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 new file mode 100644 index 00000000000..ae1c9ec98fc --- /dev/null +++ b/tests/responses/codex-eager-tool-catalog.test.ts @@ -0,0 +1,450 @@ +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, + 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"; + +/** + * 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, length = EAGER_CODEX_MCP_TOOL_WARNING_THRESHOLD) { + return [{ + type: "namespace", name: "mcp__fixture", + tools: Array.from({ length }, (_value, index) => ({ + type: "function", name: `tool_${index}`, description: "Fixture tool", + parameters: { type: "object", properties: {} }, + ...(deferred ? { defer_loading: true } : {}), + })), + }]; +} + +/** 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]")); +} + +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; + warn = spyOn(console, "warn"); + upstream = Bun.serve({ + port: 0, + 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" }], + usage: { prompt_tokens: 2, completion_tokens: 1, total_tokens: 3 }, + }); + }, + }); + 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 }); +}); + +afterEach(async () => { + warn.mockRestore(); + await server?.stop(true); + upstream?.stop(true); + clearResponseStateForTests(); + home.remove(); +}); + +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", ...headers }, + body: JSON.stringify({ model: "fixture/model", input: "hello", stream: false, tools, ...extra }), + }); + const text = await response.text(); + return { status: response.status, text }; +} + +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("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 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.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.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 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(1); + }); + + 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("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 }, + { 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(), { + 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(catalogWarnings()).toHaveLength(1); + }); + + 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(result.status, result.text).toBe(200); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(0); + }); + + 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("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("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.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 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); + }); + + 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: liteFirstTurn(mcpTools()) })); + expect(await terminal).toBe("response.completed"); + expect(upstreamCalls).toBe(1); + expect(catalogWarnings()).toHaveLength(1); + } finally { + socket.close(); + } + }); +}); + +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" }, + ]], + ["an assistant message", [message("assistant", "Earlier response")]], + ["reasoning", [{ type: "reasoning", summary: [] }]], + ["a compaction", [{ type: "compaction", encrypted_content: "opaque" }]], + ])("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("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" }); + 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", () => { + 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); + }); +});