diff --git a/changelog.d/maintenance/11575-toolname-map-recovery-guard.md b/changelog.d/maintenance/11575-toolname-map-recovery-guard.md new file mode 100644 index 0000000000..7f821195e4 --- /dev/null +++ b/changelog.d/maintenance/11575-toolname-map-recovery-guard.md @@ -0,0 +1 @@ +- **test(chatcore):** pin the response tool-name alias resolution against the ordering hazard that broke every Gemini/Antigravity MCP tool call in v3.8.49. `extractRequestToolIdentityMap` deletes `translatedBody._toolNameMap`, so the later read is always undefined and the ledger survives only through the `requestToolIdentityMap` fallback — removing that fallback previously left the entire tool-name suite green. The resolution moves into `resolveResponseToolNameMap()` next to the map it depends on, with a regression guard that fails without the recovery. No behaviour change. ([#11575](https://github.com/diegosouzapw/OmniRoute/pull/11575)) diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index d6b43b05b5..4d8abc4f51 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -1,6 +1,6 @@ import { extractRequestToolIdentityMap, - toToolNameAliasMap, + resolveResponseToolNameMap, } from "./chatCore/requestToolIdentity.ts"; import { injectMemoryAndSkills } from "./chatCore/memorySkillsInjection.ts"; import { resolveChatCoreRequestSetup } from "./chatCore/requestSetup.ts"; @@ -2596,19 +2596,14 @@ export async function handleChatCore({ const nativeClaudeToolNameMap = isClaudePassthrough ? buildClaudePassthroughToolNameMap(body) : null; - let toolNameMap: Map | null = - translatedToolNameMap instanceof Map && translatedToolNameMap.size > 0 - ? translatedToolNameMap - : nativeClaudeToolNameMap; - - // For providers whose _toolNameMap was extracted as requestToolIdentityMap - // before the Kiro merge block (Gemini/Antigravity), merge it into the - // response toolNameMap so the response translator can restore tool names - // from their lowercased form (#9568). Only merge string-valued entries - // (tool name aliases), not object-valued namespace identities (#7936). - if (!toolNameMap) { - toolNameMap = toToolNameAliasMap(requestToolIdentityMap); - } + // Resolution order matters: `_toolNameMap` was already deleted by + // `extractRequestToolIdentityMap`, so Gemini/Antigravity depend on the + // `requestToolIdentityMap` fallback inside this helper (#9568 / #7936). + const toolNameMap = resolveResponseToolNameMap( + translatedToolNameMap, + nativeClaudeToolNameMap, + requestToolIdentityMap + ); delete translatedBody._toolNameMap; delete translatedBody._disableToolPrefix; diff --git a/open-sse/handlers/chatCore/requestToolIdentity.ts b/open-sse/handlers/chatCore/requestToolIdentity.ts index 09e7602887..2befc971d7 100644 --- a/open-sse/handlers/chatCore/requestToolIdentity.ts +++ b/open-sse/handlers/chatCore/requestToolIdentity.ts @@ -21,6 +21,34 @@ export function toToolNameAliasMap( return aliases; } +/** + * Decide which alias ledger the response translator gets. + * + * The ordering here is load-bearing. `extractRequestToolIdentityMap` runs + * earlier in the request and DELETES `translatedBody._toolNameMap`, so by the + * time the response map is resolved that property is already gone and + * `translatedToolNameMap` is undefined for every Gemini/Antigravity request. + * Without the `requestToolIdentityMap` fallback the ledger is silently dropped, + * the response translator has nothing to reverse the sanitized wire name with + * (`mcp__chrome-devtools__list_pages` goes out as + * `mcp_chrome_devtools_list_pages`), and clients reject every MCP tool call + * with "No such tool available" (#9568 / #7936). + * + * Only string-valued ledgers are recovered — `toToolNameAliasMap` returns null + * for object-valued namespace identities so those are not reinterpreted as + * response aliases. + */ +export function resolveResponseToolNameMap( + translatedToolNameMap: unknown, + nativeClaudeToolNameMap: Map | null, + requestToolIdentityMap: ReadonlyMap | null +): Map | null { + if (translatedToolNameMap instanceof Map && translatedToolNameMap.size > 0) { + return translatedToolNameMap as Map; + } + return nativeClaudeToolNameMap ?? toToolNameAliasMap(requestToolIdentityMap); +} + /** * Extract the #7936 request-tool identity map from the translated body and * strip both side channels before dispatch. diff --git a/tests/unit/chatcore-response-toolname-map-recovery.test.ts b/tests/unit/chatcore-response-toolname-map-recovery.test.ts new file mode 100644 index 0000000000..5ce809d9e2 --- /dev/null +++ b/tests/unit/chatcore-response-toolname-map-recovery.test.ts @@ -0,0 +1,108 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + extractRequestToolIdentityMap, + resolveResponseToolNameMap, +} from "../../open-sse/handlers/chatCore/requestToolIdentity.ts"; +import { openaiToGeminiRequest } from "../../open-sse/translator/request/openai-to-gemini.ts"; +import { caseInsensitiveToolNameLookup } from "../../open-sse/translator/helpers/toolCallHelper.ts"; + +// Regression guard for the ordering hazard between `extractRequestToolIdentityMap` +// and the response-side alias resolution. +// +// `extractRequestToolIdentityMap` DELETES `translatedBody._toolNameMap`, and the +// response map is resolved ~40 lines later. Read naively, the property is gone +// by then, so `translatedToolNameMap` is undefined for every Gemini/Antigravity +// request and the alias ledger is dropped: the response translator has nothing +// to reverse the sanitized wire name with, and clients reject each MCP tool call +// with "No such tool available". +// +// `toToolNameAliasMap` was already covered in isolation +// (chatcore-request-tool-identity-contracts), but nothing pinned the ORDERING — +// deleting the recovery fallback left the whole tool-name suite green. + +const MCP_TOOL_NAME = "mcp__chrome-devtools__list_pages"; +const GEMINI_WIRE_NAME = "mcp_chrome_devtools_list_pages"; + +function geminiRequestWithMcpTool() { + return openaiToGeminiRequest( + "gemini-3.5-flash", + { + messages: [{ role: "user", content: "list the browser pages" }], + tools: [ + { + type: "function", + function: { + name: MCP_TOOL_NAME, + description: "List browser pages", + parameters: { type: "object", properties: {} }, + }, + }, + ], + }, + false + ) as Record; +} + +test("gemini MCP tool alias survives extractRequestToolIdentityMap consuming the ledger", () => { + const translatedBody = geminiRequestWithMcpTool(); + + const declared = (translatedBody.tools as { functionDeclarations?: { name?: string }[] }[])?.[0] + ?.functionDeclarations?.[0]?.name; + assert.equal(declared, GEMINI_WIRE_NAME, "precondition: the wire name is sanitized"); + + const requestToolIdentityMap = extractRequestToolIdentityMap(translatedBody); + assert.equal( + translatedBody._toolNameMap, + undefined, + "precondition: extract consumes the ledger, so the later read sees nothing" + ); + + const toolNameMap = resolveResponseToolNameMap( + translatedBody._toolNameMap, + null, + requestToolIdentityMap + ); + + assert.ok( + toolNameMap instanceof Map, + "the consumed ledger must be recovered, or every Gemini MCP tool call breaks" + ); + assert.equal( + caseInsensitiveToolNameLookup(GEMINI_WIRE_NAME, toolNameMap), + MCP_TOOL_NAME, + "the client must get back the tool name it registered" + ); +}); + +test("an intact translated ledger still wins over the recovered one", () => { + const intact = new Map([["wire_name", "Intact"]]); + const recovered = new Map([["wire_name", "Recovered"]]); + + assert.equal(resolveResponseToolNameMap(intact, null, recovered), intact); +}); + +test("native Claude passthrough takes precedence over the identity fallback", () => { + const nativeClaude = new Map([["proxy_read", "Read"]]); + const identities = new Map([["lowercase", "Lowercase"]]); + + assert.equal(resolveResponseToolNameMap(undefined, nativeClaude, identities), nativeClaude); +}); + +test("namespace identities are not reinterpreted as response aliases", () => { + const identities = new Map([ + ["mcp__files__read", { namespace: "mcp__files", name: "read" }], + ]); + + assert.equal(resolveResponseToolNameMap(undefined, null, identities), null); +}); + +test("an empty translated ledger falls through to recovery", () => { + const recovered = new Map([["wire", "Original"]]); + + assert.deepEqual( + resolveResponseToolNameMap(new Map(), null, recovered), + new Map([["wire", "Original"]]) + ); +});