From b4c839b8f1f8ee0ea77897b945a3a745be3306de Mon Sep 17 00:00:00 2001 From: kosta Date: Sat, 19 Sep 2026 20:06:21 -0400 Subject: [PATCH] fix(responses): normalize wrapped MCP tool names --- src/types/tools.ts | 10 +++- .../decisions/ADR-0097-responses-http-sse.md | 12 +++++ structure/transports/responses.md | 8 ++++ ...s-default-namespace-emit-normalize.test.ts | 46 +++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 structure/decisions/ADR-0097-responses-http-sse.md diff --git a/src/types/tools.ts b/src/types/tools.ts index 83decd85b14..75f1b18f1d3 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -110,6 +110,8 @@ export const NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES: ReadonlySet = new Set * * Rewrites invented `default.` prefixes back to a declared bare tool when that bare tool * is declared and neither `default.` nor `default__` was explicitly declared (#4176). + * The same wrapper may surround an already-flattened namespace identity; accept that exact + * declared suffix without treating its child name as a bare declaration. * Also normalizes legacy helper names (`exec_command`, `shell_command`, `apply_patch`, `view_image`) to * `exec` when code-mode `exec` is declared in the request catalog. * @@ -132,7 +134,13 @@ export function normalizeDeclaredToolName( const bareDeclared = declaredBare ?? declared; if ( bare.length > 0 - && bareDeclared.has(bare) + && ( + bareDeclared.has(bare) + // Muse can wrap the complete `namespace__tool` identity in `default.`. Requiring the + // exact flattened identity to be declared preserves the #4176 provenance boundary: + // `default.tool` still cannot borrow a namespaced tool's manufactured bare alias. + || (bare.includes("__") && declared.has(bare)) + ) && !declared.has("default." + bare) && !declared.has("default__" + bare) ) { diff --git a/structure/decisions/ADR-0097-responses-http-sse.md b/structure/decisions/ADR-0097-responses-http-sse.md new file mode 100644 index 00000000000..ac9688b76ab --- /dev/null +++ b/structure/decisions/ADR-0097-responses-http-sse.md @@ -0,0 +1,12 @@ +# ADR-0097 — decision recorded under "Responses HTTP/SSE" + +- Contract owner: [transports/responses.md](../transports/responses.md#responses-httpsse) + +## Decision record + +- 목적과 의도: Restore a Muse callback that wraps a request-declared flattened namespace identity in an invented `default.` prefix without weakening the undeclared-tool boundary. +- 기존 구현 및 제약 조건: Default-namespace normalization accepted genuine bare declarations and bounded code-mode helpers, but intentionally rejected a namespaced tool's child name; the missing case carried the complete canonical `namespace__tool` identity after the prefix. +- 검토한 주요 대안: Strip every `default.` prefix; authorize any unique bare alias; special-case Codex App or Muse model names; require the complete suffix to be a declared flattened identity. +- 선택한 방식: Strip the wrapper only when the suffix contains `__`, is present verbatim in the current declared-name set, and no explicit default-namespace identity owns the emitted spelling. +- 다른 대안 대신 이 방식을 선택한 이유: Exact current-turn membership repairs the provider formatting error while preserving rejection for namespace-dropping guesses, unknown names, pruned tools, and explicitly declared default identities. +- 장점, 단점 및 영향: Streaming and buffered Responses paths emit the canonical client identity and continue the turn; providers inventing a different wrapper syntax still fail closed until measured and reviewed. diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 031f1d5417f..3f4f0c21dce 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -596,6 +596,14 @@ declared bare tool and to rewrite code-mode helper names into the declared `exec input unchanged when the set is absent, so the set reaches the bridge on every wire and enforcement is expressed by a separate flag rather than by withholding it. +Muse may also wrap an already-flattened namespace identity, for example +`default.mcp__server__tool`. That form resolves only when the complete suffix is an exact declared +name containing the flattened `__` delimiter and neither explicit `default.` nor `default__` +identity exists. It does not let `default.tool` borrow a namespaced tool's manufactured bare alias, +and an unknown suffix still reaches the undeclared-tool failure. + +> Decision record: [ADR-0097](../decisions/ADR-0097-responses-http-sse.md) + The passthrough guard resolves an emitted name through that same `normalizeDeclaredToolName`, so whatever it admits it must also EMIT under the resolved name. The two halves disagreed once: `normalizeDefaultNamespaceInItem` implemented only the bare-tool case (#4176), so a diff --git a/tests/responses/responses-default-namespace-emit-normalize.test.ts b/tests/responses/responses-default-namespace-emit-normalize.test.ts index 5b7952468b9..cdf036badaa 100644 --- a/tests/responses/responses-default-namespace-emit-normalize.test.ts +++ b/tests/responses/responses-default-namespace-emit-normalize.test.ts @@ -40,6 +40,15 @@ const CLASSIC_BODY = { ], } as const; +/** Codex App MCP tool shape from the Muse callback failure: namespace plus child function. */ +const CODEX_APP_BODY = { + tools: [{ + type: "namespace", + name: "mcp__codex_app", + tools: [{ type: "function", name: "send_message_to_thread", parameters: { type: "object" } }], + }], +} as const; + function declarationsOf(body: unknown): { declared: ReadonlySet; declaredBare: ReadonlySet; @@ -110,6 +119,28 @@ describe("default-namespaced helper names under a code-mode catalog", () => { }); }); +describe("default wrapper around a declared flattened namespace identity", () => { + const canonical = "mcp__codex_app__send_message_to_thread"; + const wrapped = `default.${canonical}`; + + test("the exact Muse callback name normalizes to the declared canonical identity", () => { + const item = { type: "function_call", call_id: "c1", name: wrapped, arguments: "{}" }; + expect(normalizedNames(CODEX_APP_BODY, item)).toEqual([canonical]); + expect(guardVerdict(CODEX_APP_BODY, item)).toBeUndefined(); + }); + + test("a namespace-dropping guess and an unknown suffix stay rejected", () => { + for (const name of [ + "default.send_message_to_thread", + "default.mcp__codex_app__delete_everything", + ]) { + const item = { type: "function_call", call_id: "c1", name, arguments: "{}" }; + expect(normalizedNames(CODEX_APP_BODY, item)).toEqual([name]); + expect(guardVerdict(CODEX_APP_BODY, item)).toBe(name); + } + }); +}); + describe("names the emit boundary must not touch", () => { test("a canonical declared name passes through byte-identical", () => { const item = { type: "function_call", call_id: "c1", name: "view_image", arguments: "{}" }; @@ -199,6 +230,21 @@ describe("the streaming boundary the report actually crossed", () => { expect(emitted[0]).not.toContain("default.view_image"); }); + test("the streamed Muse callback keeps its declared namespace identity", () => { + const { declared, declaredBare } = declarationsOf(CODEX_APP_BODY); + const rewrite = createUndeclaredToolCallGuardBlockRewrite(declared, undefined, undefined, declaredBare); + const emitted = blocks(rewrite, [{ + type: "function_call", + id: "fc_1", + call_id: "c1", + name: "default.mcp__codex_app__send_message_to_thread", + arguments: "{}", + }]); + expect(emitted).toHaveLength(1); + expect(emitted[0]).toContain('"name":"mcp__codex_app__send_message_to_thread"'); + expect(emitted[0]).not.toContain("default.mcp__codex_app"); + }); + test("an unresolvable dotted name ends the turn instead of reaching the client", () => { const { declared, declaredBare } = declarationsOf(CODE_MODE_BODY); const rewrite = createUndeclaredToolCallGuardBlockRewrite(declared, undefined, undefined, declaredBare);