From bc229433ad11be977b91308cfbc85bc3377eddeb Mon Sep 17 00:00:00 2001 From: Jonathan Li <47408717+jonathanli12@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:21:16 -0700 Subject: [PATCH 1/3] fix(adapters): teach code-mode nested helpers in the shared catalog nudge Routed providers were told the valid names were exactly the flat top-level catalog. In Codex code mode, deferred helpers such as tools.codex_app__list_threads stay callable inside exec even when they are omitted from the listed names and from exec description. Discover them from the isolate global ALL_TOOLS, not tools.ALL_TOOLS. --- src/adapters/cursor/tool-definitions.ts | 2 +- src/adapters/tool-catalog-nudge.ts | 21 ++++++++++++++++----- tests/cursor-tool-definitions.test.ts | 3 +++ tests/tool-catalog-nudge.test.ts | 25 ++++++++++++++++++++++++- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index 057399dacf..5fd733dbe4 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -615,7 +615,7 @@ export function buildCursorToolGuidanceSystemNote( // Code mode: shell/edit/MCP live inside freeform `exec` as nested helpers. Without this the // model probes for a top-level shell tool that is not there. codeMode - ? `\`${CODEX_UNIFIED_EXEC_TOOL}\` is Codex code mode: its body is JavaScript evaluated in a V8 isolate, not a shell command and not Node. Shell, file edits, and MCP are nested helpers called INSIDE that body as \`await tools.(...)\`, for example \`await tools.exec_command({cmd: \"ls\"})\`. Read the tool description for the exact nested helpers this turn provides. Those nested helpers are not themselves top-level tools, so do not call \`exec_command\`, \`shell_command\`, or \`apply_patch\` at the top level here${codeModeOtherTopLevelNames.length > 0 ? `; every other tool this turn lists, including ${quotedNames(codeModeOtherTopLevelNames)}, remains callable at the top level as usual` : ""}.` + ? `\`${CODEX_UNIFIED_EXEC_TOOL}\` is Codex code mode: its body is JavaScript evaluated in a V8 isolate, not a shell command and not Node. Shell, file edits, and MCP are nested helpers called INSIDE that body as \`await tools.(...)\`, for example \`await tools.exec_command({cmd: \"ls\"})\`. Read the tool description and the isolate global \`ALL_TOOLS\` (not \`tools.ALL_TOOLS\`) for helpers this turn provides; absence from the top-level catalog or from \`exec\`'s description is not absence. Those nested helpers are not themselves top-level tools, so do not call \`exec_command\`, \`shell_command\`, or \`apply_patch\` at the top level here${codeModeOtherTopLevelNames.length > 0 ? `; every other tool this turn lists, including ${quotedNames(codeModeOtherTopLevelNames)}, remains callable at the top level as usual` : ""}.` : undefined, codeMode ? "In code mode the isolate returns nothing on its own: call `text(...)` (or `notify(...)`) on any value you need to see, or the call completes with empty output. There is no `require`, no `module`, and no filesystem or network globals; reach the host only through the nested helpers." diff --git a/src/adapters/tool-catalog-nudge.ts b/src/adapters/tool-catalog-nudge.ts index 905f3e0835..561f91886c 100644 --- a/src/adapters/tool-catalog-nudge.ts +++ b/src/adapters/tool-catalog-nudge.ts @@ -16,9 +16,10 @@ import { // `python3` heredoc edits. The sibling list in `./cursor/tool-definitions.ts` never // included it either. const NEIGHBOR_AGENT_TOOL_NAMES = ["Read", "Grep", "Glob", "Bash", "LS"] as const; +const CODEX_CODE_MODE_EXEC_TOOL = "exec"; function quoteNames(names: readonly string[]): string { - return names.map(name => `\`${name}\``).join(", "); + return names.map(name => "`" + name + "`").join(", "); } function uniqueNames(names: readonly string[]): string[] { @@ -40,6 +41,13 @@ export function shouldInjectNonOpenAIToolCatalogNudge(provider: Pick, + toWireName: (name: string) => string, +): boolean { + return advertised.has(CODEX_CODE_MODE_EXEC_TOOL) || advertised.has(toWireName(CODEX_CODE_MODE_EXEC_TOOL)); +} + export function buildNonOpenAIToolCatalogNudgeFromNames( wireNames: readonly string[] | undefined, toWireName: (name: string) => string = name => name, @@ -50,21 +58,24 @@ export function buildNonOpenAIToolCatalogNudgeFromNames( const advertised = new Set(names); // Compare in the catalog's own coordinate system. `advertised` holds WIRE names, so a // provider that rewrites them (Claude OAuth `custom_`, Anthropic compat `cx_`) would never - // match a bare neighbor name and would forbid tools the turn actually advertises — the + // match a bare neighbor name and would forbid tools the turn actually advertises -- the // catalog would list `custom_apply_patch` while the same sentence banned `apply_patch`. const unavailableNeighborNames = NEIGHBOR_AGENT_TOOL_NAMES.filter( name => !advertised.has(name) && !advertised.has(toWireName(name)), ); + const codeMode = catalogListsCodeModeExec(advertised, toWireName); return [ "Tool contract: use the current tool catalog as ground truth.", - `Valid tool names for this turn are exactly ${quoteNames(names)}.`, + "Valid tool names for this turn are exactly " + quoteNames(names) + ".", "These listed names are the complete top-level tool-call surface for this turn.", "Call only listed names with their listed argument keys; do not invent, translate, or rename tools.", "Names mentioned only in instructions, tool descriptions, argument descriptions, or nested helper APIs are not additional top-level tools.", - "If a listed tool exposes nested helpers such as a tools.* API, call the listed parent tool and use those helpers only inside that tool's input.", + codeMode + ? "If `exec` is listed, it is Codex code mode: its body is JavaScript evaluated in a V8 isolate. Nested helpers are called INSIDE that body as `await tools.(...)`, for example `await tools.exec_command({cmd: \"ls\"})` or `await tools.codex_app__list_threads({})`. Absence from the top-level catalog or from `exec`'s description is not absence: deferred helpers stay callable on `tools.`. Discover them from the isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`. Do not skip an available nested helper because it is omitted from the listed top-level names." + : "If a listed tool exposes nested helpers such as a tools.* API, call the listed parent tool and use those helpers only inside that tool's input.", unavailableNeighborNames.length > 0 - ? `Do not use neighboring-agent tool names ${quoteNames(unavailableNeighborNames)} unless this turn's catalog lists those exact names.` + ? "Do not use neighboring-agent tool names " + quoteNames(unavailableNeighborNames) + " unless this turn's catalog lists those exact names." : undefined, "If you need shell, file search, file read, edit, or discovery behavior, choose the listed tool that provides that capability.", "Count a tool call only after its tool result returns; batch independent read-only calls when the runtime supports it.", diff --git a/tests/cursor-tool-definitions.test.ts b/tests/cursor-tool-definitions.test.ts index 45c674219b..862f95756a 100644 --- a/tests/cursor-tool-definitions.test.ts +++ b/tests/cursor-tool-definitions.test.ts @@ -447,6 +447,9 @@ describe("Cursor code mode tool guidance", () => { expect(note).toContain("await tools.exec_command({cmd: " + "\"" + "ls" + "\"" + "})"); expect(note).toContain("text(...)"); expect(note).toContain("There is no `require`"); + expect(note).toContain("isolate global `ALL_TOOLS`"); + expect(note).toContain("not `tools.ALL_TOOLS`"); + expect(note).toContain("absence from the top-level catalog"); // The flat-catalog shell-bridge guidance must NOT appear: naming a top-level // `exec_command` in code mode sends the model after a tool that does not exist. diff --git a/tests/tool-catalog-nudge.test.ts b/tests/tool-catalog-nudge.test.ts index 09a4b61a1c..1c6e6b7ca3 100644 --- a/tests/tool-catalog-nudge.test.ts +++ b/tests/tool-catalog-nudge.test.ts @@ -55,10 +55,33 @@ describe("non-OpenAI tool catalog nudge", () => { expect(note).toContain("Valid tool names for this turn are exactly `exec`, `wait`, `request_user_input`"); expect(note).toContain("complete top-level tool-call surface"); expect(note).toContain("nested helper APIs are not additional top-level tools"); - expect(note).toContain("call the listed parent tool"); + expect(note).toContain("If `exec` is listed, it is Codex code mode"); + expect(note).toContain("await tools.(...)"); + expect(note).toContain("await tools.codex_app__list_threads({})"); + expect(note).toContain("isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`"); + expect(note).toContain("Do not skip an available nested helper"); + expect(note).not.toContain("call the listed parent tool and use those helpers only inside that tool's input"); expect(note).not.toContain("apply_patch"); }); + test("keeps the generic nested-helper parent-tool rule when exec is not listed", () => { + const note = buildNonOpenAIToolCatalogNudgeFromNames(["exec_command", "mcp__fs__read_file"]); + + expect(note).toContain("call the listed parent tool and use those helpers only inside that tool's input"); + expect(note).not.toContain("If `exec` is listed, it is Codex code mode"); + expect(note).not.toContain("tools.ALL_TOOLS"); + }); + + test("detects a wire-renamed exec as code mode", () => { + const note = buildNonOpenAIToolCatalogNudgeFromNames( + ["cx_exec", "cx_wait"], + name => `cx_${name}`, + ); + + expect(note).toContain("If `exec` is listed, it is Codex code mode"); + expect(note).toContain("isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`"); + }); + // `advertised` holds WIRE names. A provider that rewrites them (Claude OAuth `custom_`, // Anthropic compat `cx_`) must not have every neighbor name declared unavailable while the // catalog plainly lists the prefixed form. From 8a4040384dcf4eddb5932048bde78e1053c4470e Mon Sep 17 00:00:00 2001 From: Jonathan Li <47408717+jonathanli12@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:33:48 -0700 Subject: [PATCH 2/3] fix(adapters): name the advertised exec tool and keep listed apply_patch callable CodeRabbit on #1895: use the transformed exec wire name in the shared catalog nudge, and do not forbid apply_patch at the top level when that tool is separately advertised in code mode. --- src/adapters/cursor/tool-definitions.ts | 2 +- src/adapters/tool-catalog-nudge.ts | 15 +++++++++------ tests/cursor-tool-definitions.test.ts | 15 +++++++++++++++ tests/tool-catalog-nudge.test.ts | 4 +++- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/adapters/cursor/tool-definitions.ts b/src/adapters/cursor/tool-definitions.ts index 5fd733dbe4..9ffdd3b527 100644 --- a/src/adapters/cursor/tool-definitions.ts +++ b/src/adapters/cursor/tool-definitions.ts @@ -615,7 +615,7 @@ export function buildCursorToolGuidanceSystemNote( // Code mode: shell/edit/MCP live inside freeform `exec` as nested helpers. Without this the // model probes for a top-level shell tool that is not there. codeMode - ? `\`${CODEX_UNIFIED_EXEC_TOOL}\` is Codex code mode: its body is JavaScript evaluated in a V8 isolate, not a shell command and not Node. Shell, file edits, and MCP are nested helpers called INSIDE that body as \`await tools.(...)\`, for example \`await tools.exec_command({cmd: \"ls\"})\`. Read the tool description and the isolate global \`ALL_TOOLS\` (not \`tools.ALL_TOOLS\`) for helpers this turn provides; absence from the top-level catalog or from \`exec\`'s description is not absence. Those nested helpers are not themselves top-level tools, so do not call \`exec_command\`, \`shell_command\`, or \`apply_patch\` at the top level here${codeModeOtherTopLevelNames.length > 0 ? `; every other tool this turn lists, including ${quotedNames(codeModeOtherTopLevelNames)}, remains callable at the top level as usual` : ""}.` + ? `\`${CODEX_UNIFIED_EXEC_TOOL}\` is Codex code mode: its body is JavaScript evaluated in a V8 isolate, not a shell command and not Node. Shell, file edits, and MCP are nested helpers called INSIDE that body as \`await tools.(...)\`, for example \`await tools.exec_command({cmd: \"ls\"})\`. Read the tool description and the isolate global \`ALL_TOOLS\` (not \`tools.ALL_TOOLS\`) for helpers this turn provides; absence from the top-level catalog or from \`exec\`'s description is not absence. Those nested helpers are not themselves top-level tools, so do not call \`exec_command\` or \`shell_command\` at the top level here${codeModeOtherTopLevelNames.length > 0 ? `; every other tool this turn lists, including ${quotedNames(codeModeOtherTopLevelNames)}, remains callable at the top level as usual` : ""}.` : undefined, codeMode ? "In code mode the isolate returns nothing on its own: call `text(...)` (or `notify(...)`) on any value you need to see, or the call completes with empty output. There is no `require`, no `module`, and no filesystem or network globals; reach the host only through the nested helpers." diff --git a/src/adapters/tool-catalog-nudge.ts b/src/adapters/tool-catalog-nudge.ts index 561f91886c..6d47774a9f 100644 --- a/src/adapters/tool-catalog-nudge.ts +++ b/src/adapters/tool-catalog-nudge.ts @@ -41,11 +41,14 @@ export function shouldInjectNonOpenAIToolCatalogNudge(provider: Pick, toWireName: (name: string) => string, -): boolean { - return advertised.has(CODEX_CODE_MODE_EXEC_TOOL) || advertised.has(toWireName(CODEX_CODE_MODE_EXEC_TOOL)); +): string | undefined { + const wireName = toWireName(CODEX_CODE_MODE_EXEC_TOOL); + if (advertised.has(wireName)) return wireName; + if (advertised.has(CODEX_CODE_MODE_EXEC_TOOL)) return CODEX_CODE_MODE_EXEC_TOOL; + return undefined; } export function buildNonOpenAIToolCatalogNudgeFromNames( @@ -63,7 +66,7 @@ export function buildNonOpenAIToolCatalogNudgeFromNames( const unavailableNeighborNames = NEIGHBOR_AGENT_TOOL_NAMES.filter( name => !advertised.has(name) && !advertised.has(toWireName(name)), ); - const codeMode = catalogListsCodeModeExec(advertised, toWireName); + const codeModeExecName = advertisedCodeModeExecName(advertised, toWireName); return [ "Tool contract: use the current tool catalog as ground truth.", @@ -71,8 +74,8 @@ export function buildNonOpenAIToolCatalogNudgeFromNames( "These listed names are the complete top-level tool-call surface for this turn.", "Call only listed names with their listed argument keys; do not invent, translate, or rename tools.", "Names mentioned only in instructions, tool descriptions, argument descriptions, or nested helper APIs are not additional top-level tools.", - codeMode - ? "If `exec` is listed, it is Codex code mode: its body is JavaScript evaluated in a V8 isolate. Nested helpers are called INSIDE that body as `await tools.(...)`, for example `await tools.exec_command({cmd: \"ls\"})` or `await tools.codex_app__list_threads({})`. Absence from the top-level catalog or from `exec`'s description is not absence: deferred helpers stay callable on `tools.`. Discover them from the isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`. Do not skip an available nested helper because it is omitted from the listed top-level names." + codeModeExecName + ? "If `" + codeModeExecName + "` is listed, it is Codex code mode: its body is JavaScript evaluated in a V8 isolate. Nested helpers are called INSIDE that body as `await tools.(...)`, for example `await tools.exec_command({cmd: \"ls\"})` or `await tools.codex_app__list_threads({})`. Absence from the top-level catalog or from `" + codeModeExecName + "`'s description is not absence: deferred helpers stay callable on `tools.`. Discover them from the isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`. Do not skip an available nested helper because it is omitted from the listed top-level names." : "If a listed tool exposes nested helpers such as a tools.* API, call the listed parent tool and use those helpers only inside that tool's input.", unavailableNeighborNames.length > 0 ? "Do not use neighboring-agent tool names " + quoteNames(unavailableNeighborNames) + " unless this turn's catalog lists those exact names." diff --git a/tests/cursor-tool-definitions.test.ts b/tests/cursor-tool-definitions.test.ts index 862f95756a..0afd6a0b16 100644 --- a/tests/cursor-tool-definitions.test.ts +++ b/tests/cursor-tool-definitions.test.ts @@ -458,6 +458,21 @@ describe("Cursor code mode tool guidance", () => { expect(note).not.toContain("For file read/search/listing, use"); }); + test("does not forbid a separately listed apply_patch in code mode", () => { + const note = buildCursorToolGuidanceSystemNote([ + codeModeExec(), + { name: "apply_patch", description: "Apply a patch", parameters: {}, freeform: true }, + ]); + expect(note).toBeDefined(); + if (!note) throw new Error("Expected Cursor tool guidance note"); + + expect(note).toContain("is Codex code mode"); + expect(note).toContain("remains callable at the top level as usual"); + expect(note).toContain("`apply_patch`"); + expect(note).not.toContain("do not call `exec_command`, `shell_command`, or `apply_patch` at the top level here"); + expect(note).toContain("do not call `exec_command` or `shell_command` at the top level here"); + }); + test("keeps other visible top-level tools callable in code mode", () => { // Code mode is about how `exec` works, not a claim that the rest of the catalog is nested. // A turn can advertise freeform `exec` alongside ordinary top-level tools, and describing diff --git a/tests/tool-catalog-nudge.test.ts b/tests/tool-catalog-nudge.test.ts index 1c6e6b7ca3..89189ff287 100644 --- a/tests/tool-catalog-nudge.test.ts +++ b/tests/tool-catalog-nudge.test.ts @@ -78,8 +78,10 @@ describe("non-OpenAI tool catalog nudge", () => { name => `cx_${name}`, ); - expect(note).toContain("If `exec` is listed, it is Codex code mode"); + expect(note).toContain("If `cx_exec` is listed, it is Codex code mode"); + expect(note).toContain("from `cx_exec`'s description is not absence"); expect(note).toContain("isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`"); + expect(note).not.toContain("If `exec` is listed, it is Codex code mode"); }); // `advertised` holds WIRE names. A provider that rewrites them (Claude OAuth `custom_`, From e2720f854d0c45c09be67aa024b2552db82b1656 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 18 Aug 2026 10:18:01 +0900 Subject: [PATCH 3/3] fix(adapters): decide code mode from tool semantics, not from the name exec The shared catalog nudge classified any advertised tool named exec as Codex code mode. That is too broad in two directions the review named. A provider can advertise an ordinary structured exec that takes a shell string, and a catalog can list exec alongside exec_command or shell_command, which is the flat-bridge shape rather than code mode. Both of those turns were being told that exec is JavaScript evaluated in a V8 isolate and that shell is reachable only as a nested tools.* helper. A model that believes it sends the wrong arguments, or avoids a legitimate top-level execution tool because it thinks the tool is something else. The repository already had the right predicate - a freeform exec with no visible bare shell bridge - but it lived in the Cursor tool definitions behind a provider namespace check, and this nudge is shared by Anthropic, Google, Kiro, OpenAI-chat and command-code. So the same two halves are defined here without the provider gate, and the decision happens in the tool-object entry point while freeform still exists. Reducing to wire names first throws away the only field that distinguishes the two tools. The name-only entry point can no longer guess. It accepts a verified wire name from a caller that had the objects, and falls back to the generic parent-tool sentence otherwise - which is the honest answer when the metadata needed to decide was never passed in. Regressions cover the three cases from the review: a structured exec gets generic guidance, freeform exec beside either shell bridge is not classified as nested-only, and a transformed freeform exec such as custom_exec still gets the code-mode guidance. --- src/adapters/tool-catalog-nudge.ts | 68 ++++++++++++++++++++++++------ tests/tool-catalog-nudge.test.ts | 65 ++++++++++++++++++++++++---- 2 files changed, 111 insertions(+), 22 deletions(-) diff --git a/src/adapters/tool-catalog-nudge.ts b/src/adapters/tool-catalog-nudge.ts index 6d47774a9f..913ce12584 100644 --- a/src/adapters/tool-catalog-nudge.ts +++ b/src/adapters/tool-catalog-nudge.ts @@ -16,7 +16,25 @@ import { // `python3` heredoc edits. The sibling list in `./cursor/tool-definitions.ts` never // included it either. const NEIGHBOR_AGENT_TOOL_NAMES = ["Read", "Grep", "Glob", "Bash", "LS"] as const; -const CODEX_CODE_MODE_EXEC_TOOL = "exec"; + +/** + * The two halves of the code-mode shape, kept provider-neutral here. + * + * `./cursor/tool-definitions.ts` owns the Cursor-scoped versions of these + * (`isCursorCodeModeExecTool` / `isBareCodexShellBridgeTool`), but those additionally require + * the Cursor Responses namespace. This nudge is shared by Anthropic, Google, Kiro, + * OpenAI-chat and command-code, so it needs the same semantics without that provider gate. + */ +const CODEX_UNIFIED_EXEC_TOOL_NAME = "exec"; +const CODEX_SHELL_BRIDGE_TOOL_NAMES = ["exec_command", "shell_command"] as const; + +function isCodexCodeModeExecTool(tool: Pick): boolean { + return tool.name === CODEX_UNIFIED_EXEC_TOOL_NAME && tool.freeform === true; +} + +function isBareShellBridgeTool(tool: Pick): boolean { + return (CODEX_SHELL_BRIDGE_TOOL_NAMES as readonly string[]).includes(tool.name); +} function quoteNames(names: readonly string[]): string { return names.map(name => "`" + name + "`").join(", "); @@ -41,19 +59,33 @@ export function shouldInjectNonOpenAIToolCatalogNudge(provider: Pick, - toWireName: (name: string) => string, + verifiedName: string | undefined, ): string | undefined { - const wireName = toWireName(CODEX_CODE_MODE_EXEC_TOOL); - if (advertised.has(wireName)) return wireName; - if (advertised.has(CODEX_CODE_MODE_EXEC_TOOL)) return CODEX_CODE_MODE_EXEC_TOOL; - return undefined; + if (!verifiedName) return undefined; + return advertised.has(verifiedName) ? verifiedName : undefined; } export function buildNonOpenAIToolCatalogNudgeFromNames( wireNames: readonly string[] | undefined, toWireName: (name: string) => string = name => name, + codeModeExecName?: string, ): string | undefined { const names = uniqueNames(wireNames ?? []); if (names.length === 0) return undefined; @@ -66,7 +98,7 @@ export function buildNonOpenAIToolCatalogNudgeFromNames( const unavailableNeighborNames = NEIGHBOR_AGENT_TOOL_NAMES.filter( name => !advertised.has(name) && !advertised.has(toWireName(name)), ); - const codeModeExecName = advertisedCodeModeExecName(advertised, toWireName); + const verifiedCodeModeExecName = codeModeExecWireName(advertised, codeModeExecName); return [ "Tool contract: use the current tool catalog as ground truth.", @@ -74,8 +106,8 @@ export function buildNonOpenAIToolCatalogNudgeFromNames( "These listed names are the complete top-level tool-call surface for this turn.", "Call only listed names with their listed argument keys; do not invent, translate, or rename tools.", "Names mentioned only in instructions, tool descriptions, argument descriptions, or nested helper APIs are not additional top-level tools.", - codeModeExecName - ? "If `" + codeModeExecName + "` is listed, it is Codex code mode: its body is JavaScript evaluated in a V8 isolate. Nested helpers are called INSIDE that body as `await tools.(...)`, for example `await tools.exec_command({cmd: \"ls\"})` or `await tools.codex_app__list_threads({})`. Absence from the top-level catalog or from `" + codeModeExecName + "`'s description is not absence: deferred helpers stay callable on `tools.`. Discover them from the isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`. Do not skip an available nested helper because it is omitted from the listed top-level names." + verifiedCodeModeExecName + ? "`" + verifiedCodeModeExecName + "` is Codex code mode: its body is JavaScript evaluated in a V8 isolate. Nested helpers are called INSIDE that body as `await tools.(...)`, for example `await tools.exec_command({cmd: \"ls\"})` or `await tools.codex_app__list_threads({})`. Absence from the top-level catalog or from `" + verifiedCodeModeExecName + "`'s description is not absence: deferred helpers stay callable on `tools.`. Discover them from the isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`. Do not skip an available nested helper because it is omitted from the listed top-level names." : "If a listed tool exposes nested helpers such as a tools.* API, call the listed parent tool and use those helpers only inside that tool's input.", unavailableNeighborNames.length > 0 ? "Do not use neighboring-agent tool names " + quoteNames(unavailableNeighborNames) + " unless this turn's catalog lists those exact names." @@ -86,16 +118,24 @@ export function buildNonOpenAIToolCatalogNudgeFromNames( } export function buildNonOpenAIToolCatalogNudgeForTools( - tools: readonly Pick[] | undefined, + tools: readonly Pick[] | undefined, toolChoice?: OcxRequestOptions["toolChoice"], toWireName: (tool: Pick) => string = tool => namespacedToolName(tool.namespace, tool.name), ): string | undefined { - const visibleNames = tools - ?.filter(toolChoiceToolPredicate(toolChoice)) - .map(toWireName); + const visible = tools?.filter(toolChoiceToolPredicate(toolChoice)); + const visibleNames = visible?.map(toWireName); + // Decide code mode from the tool OBJECTS, while the `freeform` flag still exists — reducing + // to wire names first throws away the only thing that distinguishes Codex's JavaScript + // `exec` from an ordinary structured tool that happens to share the name. + const codeModeExecTool = visible?.find(isCodexCodeModeExecTool); + const codeModeExecName = codeModeExecTool + && !visible?.some(isBareShellBridgeTool) + ? toWireName(codeModeExecTool) + : undefined; // Neighbor names are bare and un-namespaced, so probe the same transform with a bare tool. return buildNonOpenAIToolCatalogNudgeFromNames( visibleNames, name => toWireName({ name }), + codeModeExecName, ); } diff --git a/tests/tool-catalog-nudge.test.ts b/tests/tool-catalog-nudge.test.ts index 89189ff287..91d4ec7b54 100644 --- a/tests/tool-catalog-nudge.test.ts +++ b/tests/tool-catalog-nudge.test.ts @@ -49,13 +49,24 @@ describe("non-OpenAI tool catalog nudge", () => { expect(buildNonOpenAIToolCatalogNudgeForTools(tools)).not.toContain("apply_patch"); }); + const codeModeExec = (): OcxTool => ({ + name: "exec", + freeform: true, + description: "Run JavaScript in a V8 isolate.", + parameters: {}, + } as OcxTool); + test("defines nested helper names as non-callable unless separately listed", () => { - const note = buildNonOpenAIToolCatalogNudgeFromNames(["exec", "wait", "request_user_input"]); + const note = buildNonOpenAIToolCatalogNudgeForTools([ + codeModeExec(), + { name: "wait", parameters: {} } as OcxTool, + { name: "request_user_input", parameters: {} } as OcxTool, + ]); expect(note).toContain("Valid tool names for this turn are exactly `exec`, `wait`, `request_user_input`"); expect(note).toContain("complete top-level tool-call surface"); expect(note).toContain("nested helper APIs are not additional top-level tools"); - expect(note).toContain("If `exec` is listed, it is Codex code mode"); + expect(note).toContain("`exec` is Codex code mode"); expect(note).toContain("await tools.(...)"); expect(note).toContain("await tools.codex_app__list_threads({})"); expect(note).toContain("isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`"); @@ -68,20 +79,58 @@ describe("non-OpenAI tool catalog nudge", () => { const note = buildNonOpenAIToolCatalogNudgeFromNames(["exec_command", "mcp__fs__read_file"]); expect(note).toContain("call the listed parent tool and use those helpers only inside that tool's input"); - expect(note).not.toContain("If `exec` is listed, it is Codex code mode"); + expect(note).not.toContain("is Codex code mode"); expect(note).not.toContain("tools.ALL_TOOLS"); }); test("detects a wire-renamed exec as code mode", () => { - const note = buildNonOpenAIToolCatalogNudgeFromNames( - ["cx_exec", "cx_wait"], - name => `cx_${name}`, + const note = buildNonOpenAIToolCatalogNudgeForTools( + [codeModeExec(), { name: "wait", parameters: {} } as OcxTool], + undefined, + tool => `cx_${tool.name}`, ); - expect(note).toContain("If `cx_exec` is listed, it is Codex code mode"); + expect(note).toContain("`cx_exec` is Codex code mode"); expect(note).toContain("from `cx_exec`'s description is not absence"); expect(note).toContain("isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`"); - expect(note).not.toContain("If `exec` is listed, it is Codex code mode"); + }); + + // The three cases the #1895 review named. Code mode is a semantic shape, not the name `exec`: + // a structured `exec` runs a shell string, and `exec` beside a visible shell bridge is the + // flat-catalog shape. Telling either of those turns that `exec` takes JavaScript and that + // shell is nested-only is actively wrong — the model then sends the wrong arguments or + // avoids a legitimate top-level execution tool. + test("a structured tool named exec is NOT code mode", () => { + const note = buildNonOpenAIToolCatalogNudgeForTools([ + { name: "exec", freeform: false, parameters: {} } as OcxTool, + { name: "mcp__fs__read_file", parameters: {} } as OcxTool, + ]); + + expect(note).not.toContain("is Codex code mode"); + expect(note).not.toContain("tools.ALL_TOOLS"); + expect(note).toContain("call the listed parent tool and use those helpers only inside that tool's input"); + }); + + test("freeform exec beside a visible shell bridge is NOT code mode", () => { + for (const bridge of ["exec_command", "shell_command"]) { + const note = buildNonOpenAIToolCatalogNudgeForTools([ + codeModeExec(), + { name: bridge, parameters: {} } as OcxTool, + ]); + + expect(note).not.toContain("is Codex code mode"); + expect(note).toContain("call the listed parent tool and use those helpers only inside that tool's input"); + } + }); + + test("a transformed freeform exec still receives code-mode guidance", () => { + const note = buildNonOpenAIToolCatalogNudgeForTools( + [codeModeExec()], + undefined, + tool => `custom_${tool.name}`, + ); + + expect(note).toContain("`custom_exec` is Codex code mode"); }); // `advertised` holds WIRE names. A provider that rewrites them (Claude OAuth `custom_`,