mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-26 17:12:27 +03:00
test(chatcore): guard the response tool-name alias recovery (#11575)
Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
This commit is contained in:
@@ -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))
|
||||
@@ -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<string, string> | 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;
|
||||
|
||||
|
||||
@@ -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<string, string> | null,
|
||||
requestToolIdentityMap: ReadonlyMap<string, unknown> | null
|
||||
): Map<string, string> | null {
|
||||
if (translatedToolNameMap instanceof Map && translatedToolNameMap.size > 0) {
|
||||
return translatedToolNameMap as Map<string, string>;
|
||||
}
|
||||
return nativeClaudeToolNameMap ?? toToolNameAliasMap(requestToolIdentityMap);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the #7936 request-tool identity map from the translated body and
|
||||
* strip both side channels before dispatch.
|
||||
|
||||
108
tests/unit/chatcore-response-toolname-map-recovery.test.ts
Normal file
108
tests/unit/chatcore-response-toolname-map-recovery.test.ts
Normal file
@@ -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<string, unknown>;
|
||||
}
|
||||
|
||||
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<string, unknown>([["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<string, unknown>([["lowercase", "Lowercase"]]);
|
||||
|
||||
assert.equal(resolveResponseToolNameMap(undefined, nativeClaude, identities), nativeClaude);
|
||||
});
|
||||
|
||||
test("namespace identities are not reinterpreted as response aliases", () => {
|
||||
const identities = new Map<string, unknown>([
|
||||
["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<string, unknown>([["wire", "Original"]]);
|
||||
|
||||
assert.deepEqual(
|
||||
resolveResponseToolNameMap(new Map(), null, recovered),
|
||||
new Map([["wire", "Original"]])
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user