diff --git a/devlog/_plan/260916_release_2570_stabilization/041_preview_fence_test_is_structural.md b/devlog/_plan/260916_release_2570_stabilization/041_preview_fence_test_is_structural.md new file mode 100644 index 00000000000..27f84becf0f --- /dev/null +++ b/devlog/_plan/260916_release_2570_stabilization/041_preview_fence_test_is_structural.md @@ -0,0 +1,46 @@ +# Follow-up: the preview read-fence test asserts shape, not behaviour + +Raised by the third regression audit on the 2.57.0 candidate, deferred past the +release on purpose. + +## What the test does today + +`tests/responses/responses-preview-main-read-fence.test.ts` reads +`src/server/responses/request-prepare.ts` and `src/codex/auth-context.ts` as +text and asserts with regexes that both native-main read fences carry the +request-owned ownership term, that both preview sites validate ownership the way +`resolveCodexAuthContext` does, that no main exclusion is guarded by drain state +alone, and that `nativeMainSelectionOnly` stays derived from the drain. + +It was written that way deliberately, for the reason recorded in its own header: +driving the divergence end to end needs a `thread_spawn` whose caller bearer is +forwardable, an account-gated candidate model, and a denial cache whose only +entry is main. The sibling contract in +`tests/routing/subagent-fallback-preview-sites.test.ts` made the same call for +the same subsystem. + +## Why that is not sufficient + +A structural assertion catches the regression that has actually recurred twice -- +a fence reconstructed inline from drain state, losing the ownership half -- and +nothing else. It cannot see a fence that is present but wired to the wrong +headers, an ownership term computed against a stale route, or a consumer that +stops reading `nativeMainReadsForbidden`. Any of those is a semantic routing +regression that would keep this file green, which means the file reports more +confidence than it holds. + +## What the replacement needs + +A behavioural case that drives `prepareResponsesRequest` with a forwardable +caller bearer on a `thread_spawn` and observes that the preview performs no +credential-validating read of the physical main token and scores main the same +way final authentication does. The expensive part is the fixture, not the +assertion: an account-gated model, a populated denial cache, and an injected +entitlement resolver that records whether main was consulted. The existing pool +harness in `tests/routing/subagent-fallback-handle-responses.test.ts` already +carries most of it, but that file is at its size cap, so the work is a new file +in `tests/routing/` plus its two layout registrations. + +Keep the structural file when the behavioural one lands. They fail on different +things, and the cheap one is what catches the inline-reconstruction regression +before review. diff --git a/src/server/responses-undeclared-tool-guard.ts b/src/server/responses-undeclared-tool-guard.ts index 58b2ce727cd..4cefb22784a 100644 --- a/src/server/responses-undeclared-tool-guard.ts +++ b/src/server/responses-undeclared-tool-guard.ts @@ -1,7 +1,7 @@ import { collectAmbiguousDottedAliases, dottedAliasIsUnambiguous, wireToolInnerName } from "../responses/tool-name-aliases"; import { - CODE_MODE_EXEC_TOOL_NAME, dottedToolName, + NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES, namespacedToolName, normalizeDeclaredToolName, } from "../types"; @@ -104,10 +104,14 @@ function addWireToolName( if (dottedAliasIsUnambiguous(namespace, name) && !ambiguousDottedAliases?.has(dotted)) { names.add(dotted); } - // `exec` is the one name that also switches on nested-helper normalization, so a bare alias - // for a namespaced MCP tool would silently authorize `exec_command`/`shell_command`/ - // `apply_patch`/`view_image` the request never declared. Every other inner name keeps the bare alias. - if (name !== CODE_MODE_EXEC_TOOL_NAME) names.add(name); + // The code-mode helper spellings do not get a bare alias for a namespaced tool. Bare `exec` + // switches nested-helper normalization on for a catalog that never declared the shell; bare + // `exec_command`/`shell_command` switch it off for one that did; bare `write_stdin`/ + // `apply_patch`/`view_image` are simply accepted as declared under a name the caller only ever + // authorized inside a namespace. This guard named only `exec` and let the other five through, + // which is the same drift the bridge-side copy had; both now read one list + // (src/types/tools.ts). Every other inner name keeps the bare alias. + if (!NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES.has(name)) names.add(name); } /** diff --git a/src/server/responses/collaboration.ts b/src/server/responses/collaboration.ts index 0fb059faa8c..b40d3d01ec8 100644 --- a/src/server/responses/collaboration.ts +++ b/src/server/responses/collaboration.ts @@ -30,7 +30,7 @@ import { } from "../../combos"; import { isInjectionDebugEnabled } from "../../lib/debug-settings"; import { injectionDebugLog } from "../../lib/injection-debug-log"; -import { dottedToolName, modelInList, namespacedToolName, toolChoiceToolPredicate } from "../../types"; +import { dottedToolName, modelInList, namespacedToolName, NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES, toolChoiceToolPredicate } from "../../types"; import type { AdapterEvent, OcxConfig, OcxParsedRequest, OcxProviderConfig, OcxProviderContinuationState, OcxUsage } from "../../types"; import { forceRefreshOAuthAccessSnapshot, @@ -156,12 +156,9 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato // bare spelling is only a safe alias while it names ONE tool and cannot be read as // another identity's canonical or dotted spelling. // Code-mode helper spellings never gain a bare alias (#4679 review), whatever namespace - // declares them: admitting bare `exec` into the declared set would authorize the unrelated - // helper normalization that the CODE_MODE_EXEC exception exists to contain. The namespace is - // not the safety property here — the bare spelling is — so this is a property of the NAME. - const BARE_ECHO_EXCLUDED_NAMES = new Set([ - "exec", "exec_command", "shell_command", "write_stdin", "apply_patch", "view_image", - ]); + // declares them and whatever put the alias there. The list is owned by `src/types/tools.ts`, + // beside the names it protects, because the copy that used to live here drifted to a single + // namespace and had to be widened twice. const bareAliasOwners = new Map(); for (const t of authorizedTools) { // Bare (no-namespace) declarations participate as owners too: a namespaced tool whose @@ -223,7 +220,7 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato // is withdrawn, and only for these six spellings. if ( bareAliasOwners.get(t.name) === JSON.stringify([t.namespace, t.name]) - && !BARE_ECHO_EXCLUDED_NAMES.has(t.name) + && !NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES.has(t.name) ) { budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" }); declaredToolNames.add(t.name); @@ -257,6 +254,20 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato // Some routed providers echo a bare tool_choice selector instead of the flattened catalog // name. Accept only selectors the client actually sent and only when the full request catalog // contains one tool with that logical name. + // + // A helper spelling selected this way is split rather than refused (#4819). The two things a + // bare alias does are separable, and passthrough already relies on that: identity RESTORATION + // runs before authorization there, rewriting the echoed bare name to the namespaced identity + // the caller declared, and the guard then authorizes `ns__name`. DECLARATION is the part that + // is unsafe, because a declared-name set carrying bare `exec` is what makes + // `normalizeDeclaredToolName` rewrite an undeclared `apply_patch`, `exec_command` or + // `write_stdin` onto the selected tool (src/types/tools.ts). + // + // So a helper spelling gets the `toolNsMap` entry and not the `declaredToolNames` entry. The + // caller nominated exactly one tool by name, `bareNameCounts` proves nothing else answers to + // it, and restoring it authorizes nothing the request did not already declare. The echo path + // above withholds both, because a bare echo is a guess rather than a nomination and #4679 + // pinned that shape (`tests/responses/bare-echo-alias.test.ts`). const choice = parsed.options.toolChoice; const bareChoiceNames = new Set( choice && typeof choice === "object" @@ -269,8 +280,11 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato } for (const t of authorizedTools) { if (!t.namespace || !bareChoiceNames.has(t.name) || bareNameCounts.get(t.name) !== 1 || declaredToolNames.has(t.name)) continue; - budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" }); - declaredToolNames.add(t.name); + // Restore the identity; declare the name only when it is not a helper spelling. + if (!NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES.has(t.name)) { + budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" }); + declaredToolNames.add(t.name); + } budget?.chargeRetained(new TextEncoder().encode(JSON.stringify([t.name, t.namespace, t.name])).byteLength, { kind: "retained_collectors" }); toolNsMap.set(t.name, { namespace: t.namespace, name: t.name, ...(t.freeform ? { freeform: true } : {}) }); if (t.parameters && typeof t.parameters === "object") { diff --git a/src/types.ts b/src/types.ts index c2104f9d41a..9ef3936da97 100644 --- a/src/types.ts +++ b/src/types.ts @@ -16,6 +16,7 @@ export { isAllowedToolChoice, toolChoiceToolPredicate, declaresCodeModeExec, + NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES, } from "./types/tools"; export type { UpstreamHttpVersion, ReasoningSummaryDelivery, CodexAccountMode } from "./types/wire"; diff --git a/src/types/tools.ts b/src/types/tools.ts index fd80a4b100e..06fe8589e1e 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -67,6 +67,30 @@ const CODE_MODE_HELPER_TOOL_NAMES = [ */ export const CODE_MODE_EXEC_TOOL_NAME = "exec"; +/** + * Spellings that may never be MANUFACTURED as a bare alias for a namespaced tool. + * + * A bare alias is an ordinary compatibility affordance -- providers echo a namespaced tool + * without its prefix, and restoring the identity needs the bare spelling registered. For these + * six it is also an authorization decision, because a declared-name set is what + * `normalizeDeclaredToolName` and `declaresCodeModeExec` read: bare `exec` turns nested-helper + * normalization on for a catalog that never declared the shell, bare `exec_command` or + * `shell_command` turns it off for one that did, and the rest are accepted as declared calls the + * caller only ever authorized under a namespace. + * + * This is a property of the SPELLING, not of the namespace that declared it and not of the reason + * the alias was being added. It lives here, beside the names it protects, because every site that + * builds a declared-name set has to apply the same list -- the two that kept their own copies each + * drifted, once to a single namespace and once to a single name. + * + * A genuine namespace-free declaration is NOT covered: that is the caller declaring the tool, not + * a namespace being discarded to synthesize a bare name. + */ +export const NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES: ReadonlySet = new Set([ + CODE_MODE_EXEC_TOOL_NAME, + ...CODE_MODE_HELPER_TOOL_NAMES, +]); + /** * Normalizes provider-emitted tool names against declared tool catalogs. * diff --git a/structure/transports/responses.md b/structure/transports/responses.md index bee5e62c777..94dc919bbed 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -76,6 +76,28 @@ the tool surface so request-local aliases remain available for response restorat item records which tool actually ran, so re-pointing it at a same-named namespace child would rewrite that record on a coincidence rather than translate it. +A namespaced tool is registered under every coordinate a provider might echo — `ns__name`, the +dotted `ns.name`, and the bare `name` — but six spellings never reach a DECLARED-NAME set under +the bare one: `exec`, `exec_command`, `shell_command`, `write_stdin`, `apply_patch`, +`view_image` (`NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES`). A declared-name set is what decides +nested-helper normalization, so bare `exec` from a namespace turns it on for a catalog that never +declared the shell, and `normalizeDeclaredToolName` then rewrites an undeclared `apply_patch` +onto it. The fence is a property of the SPELLING, not of the declaring namespace and not of why +the alias was being added — both copies drifted once, one to `collaboration` only and one to +`exec` only, and each drift was a live authorization widening. Every site that builds a +declared-name set reads the one list: `buildToolBridgeMaps` for the echo and `tool_choice` +selector paths, and `collectDeclaredWireToolNames` for the passthrough catalog. + +Declaration and restoration are separate, and only declaration is fenced. Passthrough rewrites an +echoed bare name to its namespaced identity before authorizing anything +(`authorizedBareNamespaceToolAliases`, built from `toolNsMap`), and the guard then authorizes +`ns__name`, so a `tool_choice` that nominates one helper tool by its bare name keeps the +`toolNsMap` entry and loses only the declaration. The echo path withholds both, because a bare +echo is a guess rather than a nomination. The bridges check the declared set before consulting +`toolNsMap`, so there a bare helper echo is refused either way. A genuine namespace-free +declaration is untouched throughout: that is the caller declaring the tool, not a namespace being +discarded to manufacture a bare name. + Codex-private tool fields are removed at the same boundary from one table (`CANONICAL_ONLY_TOOL_FIELDS`) rather than one bespoke pass each: `external_web_access` on either web-search variant, and `defer_loading` on any declaration, which `activateDeferredTool` clears only diff --git a/tests/responses/responses-bare-echo-helper-fence.test.ts b/tests/responses/responses-bare-echo-helper-fence.test.ts index a21a19ae33a..4c4f1503d0a 100644 --- a/tests/responses/responses-bare-echo-helper-fence.test.ts +++ b/tests/responses/responses-bare-echo-helper-fence.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { parseRequest } from "../../src/responses/parser"; import { buildToolBridgeMaps } from "../../src/server/responses"; +import { collectDeclaredWireToolNames } from "../../src/server/responses-undeclared-tool-guard"; import { normalizeDeclaredToolName, declaresCodeModeExec } from "../../src/types/tools"; /** @@ -12,16 +13,25 @@ import { normalizeDeclaredToolName, declaresCodeModeExec } from "../../src/types * * The exclusion that prevents that was once scoped to the `collaboration` namespace, which made * the boundary a property of the declaring namespace rather than of the spelling, and any other - * namespace could then donate the bare name. These cases pin the exclusion to the NAME, and pin - * the half that has to keep working beside it: the namespaced tool stays reachable under the - * spellings that carry their namespace, and non-helper names keep their #4679 echo fallback. + * namespace could then donate the bare name. Fencing the echo path then left the SELECTOR path + * open one level down: a bare `tool_choice` for a namespaced helper name added the same bare + * spelling to the same set from a different loop. + * + * These cases pin the exclusion to the NAME across both paths, and pin the halves that have to + * keep working beside it. The line the fence runs along is DECLARATION, not restoration: no + * declared-name set ever gains a manufactured bare helper spelling, while the `toolNsMap` + * identity entry an explicit selector creates survives, because passthrough restores an echoed + * bare name to its namespaced identity before authorizing it and would otherwise refuse a call + * the caller had both declared and selected. The namespaced tool also stays reachable under the + * spellings that carry its namespace, selection by bare shorthand still resolves, and non-helper + * names keep both their #4679 echo fallback and their bare selector alias. * * Kept out of `bare-echo-alias.test.ts` so the namespace-independence contract has a file of its * own rather than growing the file that pins the original collaboration-only behaviour. */ -function namespacedToolRequest(namespace: string, name: string) { - return parseRequest({ +function namespacedToolRequest(namespace: string, name: string, choiceNames?: string[]) { + const parsed = parseRequest({ model: "claude-opus-5", input: "run it", tools: [{ @@ -30,11 +40,16 @@ function namespacedToolRequest(namespace: string, name: string) { tools: [{ type: "function", name, parameters: { type: "object" } }], }], }); + // Assigned rather than parsed from `tool_choice`, the way the sibling selector cases in + // `responses-parser.test.ts` do it. The loop under test reads `options.toolChoice` and nothing + // else, so going through selector validation would only add a second thing that can fail. + if (choiceNames) parsed.options.toolChoice = { allowedTools: choiceNames, mode: "required" }; + return parsed; } const HELPER_SPELLINGS = ["exec", "exec_command", "shell_command", "write_stdin", "apply_patch", "view_image"]; -describe("helper spellings are fenced from the bare echo alias in every namespace", () => { +describe("helper spellings are fenced from every declared-name set, in every namespace", () => { test("a foreign namespace donates no helper spelling", () => { const donated = HELPER_SPELLINGS.filter(name => { const maps = buildToolBridgeMaps(namespacedToolRequest("mcp__remote", name)); @@ -73,6 +88,68 @@ describe("helper spellings are fenced from the bare echo alias in every namespac expect(maps.toolNsMap.get("list_issues")).toMatchObject({ namespace: "mcp__remote", name: "list_issues" }); }); + test("an explicit bare tool_choice selector declares no helper spelling", () => { + // The bypass one level down: the echo path is fenced, so the selector loop was the remaining + // way to put bare `exec` in the DECLARED set, which is the set that switches nested-helper + // normalization on. + const declared = HELPER_SPELLINGS.filter( + name => buildToolBridgeMaps(namespacedToolRequest("mcp__remote", name, [name])).declaredToolNames.has(name), + ); + + expect(declared).toEqual([]); + }); + + test("but it does keep the identity alias, which is what restores the call", () => { + // The half that must survive. Passthrough restores an echoed bare name to the namespaced + // identity BEFORE authorizing it (`authorizedBareNamespaceToolAliases` in + // passthrough-dispatch.ts reads exactly this map), and the guard then authorizes + // `ns__name`. Withholding the map entry too refused a call the caller had declared and + // explicitly selected. + for (const name of HELPER_SPELLINGS) { + const maps = buildToolBridgeMaps(namespacedToolRequest("mcp__remote", name, [name])); + expect([name, maps.toolNsMap.get(name)]).toEqual([name, { namespace: "mcp__remote", name }]); + } + }); + + test("both tool_choice forms behave the same way", () => { + // `{name}` and `{allowedTools}` reach the selector loop through the same `bareChoiceNames` + // set, so both forms are pinned rather than only the one a fixture happened to build. + const parsed = namespacedToolRequest("mcp__remote", "exec", ["exec"]); + parsed.options.toolChoice = { name: "exec" }; + const maps = buildToolBridgeMaps(parsed); + + expect(maps.declaredToolNames.has("exec")).toBe(false); + expect(declaresCodeModeExec(maps.declaredToolNames)).toBe(false); + expect(maps.toolNsMap.get("exec")).toEqual({ namespace: "mcp__remote", name: "exec" }); + }); + + test("a bare selector still SELECTS the helper tool and declares its own spellings", () => { + // `toolAllowedByChoice` resolves the bare shorthand against the request catalog rather than + // against this map, so the tool stays authorized and stays forced. + const maps = buildToolBridgeMaps(namespacedToolRequest("mcp__remote", "exec", ["exec"])); + + expect(maps.declaredToolNames.has("mcp__remote__exec")).toBe(true); + expect(maps.declaredToolNames.has("mcp__remote.exec")).toBe(true); + expect(maps.toolNsMap.get("mcp__remote.exec")).toMatchObject({ namespace: "mcp__remote", name: "exec" }); + }); + + test("canonical and dotted selectors are unaffected for a helper name", () => { + for (const selector of ["mcp__remote__exec", "mcp__remote.exec"]) { + const maps = buildToolBridgeMaps(namespacedToolRequest("mcp__remote", "exec", [selector])); + expect([selector, maps.declaredToolNames.has(selector)]).toEqual([selector, true]); + expect([selector, maps.declaredToolNames.has("exec")]).toEqual([selector, false]); + } + }); + + test("a non-helper name still gains its bare alias through the selector path", () => { + // Same narrowness check as the echo path: fencing the selector loop must not take the bare + // selector alias away from every other namespaced tool. + const maps = buildToolBridgeMaps(namespacedToolRequest("mcp__remote", "list_issues", ["list_issues"])); + + expect(maps.declaredToolNames.has("list_issues")).toBe(true); + expect(maps.toolNsMap.get("list_issues")).toMatchObject({ namespace: "mcp__remote", name: "list_issues" }); + }); + test("the withheld name is exactly what would have turned helper normalization on", () => { // The consequence, asserted against the consumer rather than restated: a declared set that // carries bare `exec` rewrites undeclared helper calls onto it. This is the set the previous @@ -88,3 +165,48 @@ describe("helper spellings are fenced from the bare echo alias in every namespac .toEqual(["apply_patch", "exec_command", "write_stdin"]); }); }); + +/** + * The passthrough guard builds its own declared-name catalog from the outbound body, and it feeds + * the same consumers: `undeclaredNameInItem` passes it to `normalizeDeclaredToolName`, and + * custom-tool restoration passes it on to `resolveCodeModeHelperName` and + * `declaresCodeModeExec`. It had the same fence written as a single name -- `exec` -- so the + * other five spellings still got a bare alias for an arbitrary namespace. Both sites now read one + * list, so these cases are the other half of the same invariant. + */ +describe("the passthrough declared-name catalog applies the same fence", () => { + test("a namespaced helper gets canonical and dotted spellings but no bare alias", () => { + const withheld = HELPER_SPELLINGS.filter(name => collectDeclaredWireToolNames({ + tools: [{ type: "namespace", name: "mcp", tools: [{ type: "function", name }] }], + }).has(name)); + + expect(withheld).toEqual([]); + }); + + test("the namespaced spellings themselves are still admitted", () => { + const names = collectDeclaredWireToolNames({ + tools: [{ type: "namespace", name: "mcp", tools: [{ type: "function", name: "apply_patch" }] }], + }); + + expect([...names]).toEqual(["mcp__apply_patch", "mcp.apply_patch"]); + }); + + test("a genuine top-level helper declaration keeps its bare name", () => { + // The line the fence must not cross. Here the caller really did declare `apply_patch` as a + // bare tool; no namespace is being discarded to synthesize the spelling, so withholding it + // would refuse a call the request plainly authorized. + const names = collectDeclaredWireToolNames({ + tools: [{ type: "custom", name: "apply_patch" }, { type: "function", name: "exec" }], + }); + + expect([...names].sort()).toEqual(["apply_patch", "exec"]); + }); + + test("a non-helper namespaced tool keeps all three spellings", () => { + const names = collectDeclaredWireToolNames({ + tools: [{ type: "namespace", name: "linear", tools: [{ type: "function", name: "create_issue" }] }], + }); + + expect([...names].sort()).toEqual(["create_issue", "linear.create_issue", "linear__create_issue"]); + }); +}); diff --git a/tests/responses/responses-parser.test.ts b/tests/responses/responses-parser.test.ts index b42e286c8d8..4b6605c3169 100644 --- a/tests/responses/responses-parser.test.ts +++ b/tests/responses/responses-parser.test.ts @@ -227,26 +227,26 @@ describe("Responses parser", () => { tools: [{ type: "namespace", name: "mcp__functions", - tools: [{ type: "custom", name: "exec", description: "Run a command" }], + tools: [{ type: "custom", name: "run_command", description: "Run a command" }], }], tool_choice: { type: "allowed_tools", mode: "required", - tools: [{ type: "custom", name: "exec" }], + tools: [{ type: "custom", name: "run_command" }], }, }); let maps = buildToolBridgeMaps(parsed); expect([...maps.toolNsMap]).toEqual([ - ["mcp__functions__exec", { namespace: "mcp__functions", name: "exec", freeform: true }], - ["mcp__functions.exec", { namespace: "mcp__functions", name: "exec", freeform: true }], - ["exec", { namespace: "mcp__functions", name: "exec", freeform: true }], + ["mcp__functions__run_command", { namespace: "mcp__functions", name: "run_command", freeform: true }], + ["mcp__functions.run_command", { namespace: "mcp__functions", name: "run_command", freeform: true }], + ["run_command", { namespace: "mcp__functions", name: "run_command", freeform: true }], ]); - expect([...maps.declaredToolNames]).toEqual(["mcp__functions__exec", "mcp__functions.exec", "exec"]); - expect([...maps.freeformToolNames]).toEqual(["exec"]); + expect([...maps.declaredToolNames]).toEqual(["mcp__functions__run_command", "mcp__functions.run_command", "run_command"]); + expect([...maps.freeformToolNames]).toEqual(["run_command"]); const bridged = buildResponseJSON([ - { type: "tool_call_start", id: "call_exec", name: "exec" }, + { type: "tool_call_start", id: "call_exec", name: "run_command" }, { type: "tool_call_delta", arguments: '{"input":"pwd"}' }, { type: "tool_call_end" }, { type: "done" }, @@ -255,14 +255,37 @@ describe("Responses parser", () => { expect((bridged.output as Record[])[0]).toMatchObject({ type: "custom_tool_call", call_id: "call_exec", - name: "exec", + name: "run_command", input: "pwd", status: "completed", }); - parsed.options.toolChoice = { name: "exec" }; + parsed.options.toolChoice = { name: "run_command" }; maps = buildToolBridgeMaps(parsed); - expect([...maps.toolNsMap.keys()]).toEqual(["mcp__functions__exec", "mcp__functions.exec", "exec"]); + expect([...maps.toolNsMap.keys()]).toEqual(["mcp__functions__run_command", "mcp__functions.run_command", "run_command"]); + + // A code-mode helper spelling is the exception, and it is the spelling that decides -- not the + // namespace and not the fact that the caller selected it. Bare `exec` in the DECLARED set is + // what turns nested-helper normalization on, so the selector grants the identity alias that + // restores the call without granting the declaration that would rewrite helper names onto it. + const helperSelector = parseRequest({ + model: "claude-opus-5", + input: "run it", + tools: [{ + type: "namespace", + name: "mcp__functions", + tools: [{ type: "custom", name: "exec", description: "Run a command" }], + }], + tool_choice: { + type: "allowed_tools", + mode: "required", + tools: [{ type: "custom", name: "exec" }], + }, + }); + const helperMaps = buildToolBridgeMaps(helperSelector); + expect([...helperMaps.declaredToolNames]).toEqual(["mcp__functions__exec", "mcp__functions.exec"]); + expect(helperMaps.toolNsMap.get("exec")) + .toEqual({ namespace: "mcp__functions", name: "exec", freeform: true }); expect(() => parseRequest({ model: "claude-opus-5",