From 7366bb6c3adef2b7e56b8def20bd7fd41e81b9a5 Mon Sep 17 00:00:00 2001 From: backryun Date: Thu, 13 Aug 2026 19:52:07 +0900 Subject: [PATCH] fix(types): tighten chatCore helper contracts (#10175) --- open-sse/handlers/chatCore.ts | 14 ++++---- .../chatCore/postCallGuardrailContext.ts | 36 +++++++++++++------ .../handlers/chatCore/requestToolIdentity.ts | 23 +++++++++++- ...hatcore-postcall-guardrail-context.test.ts | 17 +++++++++ ...re-request-tool-identity-contracts.test.ts | 30 ++++++++++++++++ 5 files changed, 100 insertions(+), 20 deletions(-) create mode 100644 tests/unit/chatcore-request-tool-identity-contracts.test.ts diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index d643961e9e..cf75785f0a 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -1,4 +1,7 @@ -import { extractRequestToolIdentityMap } from "./chatCore/requestToolIdentity.ts"; +import { + extractRequestToolIdentityMap, + toToolNameAliasMap, +} from "./chatCore/requestToolIdentity.ts"; import { injectMemoryAndSkills } from "./chatCore/memorySkillsInjection.ts"; import { resolveChatCoreRequestSetup } from "./chatCore/requestSetup.ts"; import { normalizeOpenAICompatibleTools } from "./chatCore/openAICompatibleTools.ts"; @@ -2317,13 +2320,8 @@ export async function handleChatCore({ // 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 && requestToolIdentityMap instanceof Map && requestToolIdentityMap.size > 0) { - const hasStringValues = [...requestToolIdentityMap.values()].every( - (v: unknown) => typeof v === "string" - ); - if (hasStringValues) { - toolNameMap = requestToolIdentityMap; - } + if (!toolNameMap) { + toolNameMap = toToolNameAliasMap(requestToolIdentityMap); } delete translatedBody._toolNameMap; delete translatedBody._disableToolPrefix; diff --git a/open-sse/handlers/chatCore/postCallGuardrailContext.ts b/open-sse/handlers/chatCore/postCallGuardrailContext.ts index 001c9c2f80..eab05b5191 100644 --- a/open-sse/handlers/chatCore/postCallGuardrailContext.ts +++ b/open-sse/handlers/chatCore/postCallGuardrailContext.ts @@ -5,14 +5,27 @@ * Extracted from handleChatCore's non-streaming success path: assemble the context object passed to * `guardrailRegistry.runPostCallHooks`. Pure value builder — no side effects, no early-returns. The * `disabledGuardrails` field is resolved via `resolveDisabledGuardrails` (injectable for tests). - * Behaviour is byte-identical to the previous inline literal, including the `method: "POST"` / - * `stream: false` constants and the headers/endpoint null-coalescing. + * Preserves the previous field mapping and constants while narrowing values from + * the untyped request boundary to the public guardrail contract. */ -import { resolveDisabledGuardrails as defaultResolveDisabled } from "@/lib/guardrails"; +import { + resolveDisabledGuardrails as defaultResolveDisabled, + type GuardrailContext, +} from "@/lib/guardrails"; -type LoggerLike = unknown; +type LoggerLike = GuardrailContext["log"]; type HeadersLike = Headers | Record | null; +function optionalRecord(value: unknown): Record | null { + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : null; +} + +function optionalString(value: unknown): string | null { + return typeof value === "string" && value.length > 0 ? value : null; +} + export function buildPostCallGuardrailContext( args: { apiKeyInfo: unknown; @@ -25,23 +38,24 @@ export function buildPostCallGuardrailContext( clientResponseFormat: unknown; }, resolveDisabledGuardrails: typeof defaultResolveDisabled = defaultResolveDisabled -) { +): GuardrailContext { const headers = (args.clientRawRequest?.headers as HeadersLike) ?? null; + const apiKeyInfo = optionalRecord(args.apiKeyInfo); return { - apiKeyInfo: args.apiKeyInfo, + apiKeyInfo, disabledGuardrails: resolveDisabledGuardrails({ - apiKeyInfo: (args.apiKeyInfo as Record | null) ?? null, + apiKeyInfo, body: args.body, headers, }), - endpoint: args.clientRawRequest?.endpoint || null, + endpoint: optionalString(args.clientRawRequest?.endpoint), headers, log: args.log, method: "POST", model: args.model, provider: args.provider, - sourceFormat: args.responsePayloadFormat, + sourceFormat: optionalString(args.responsePayloadFormat), stream: false, - targetFormat: args.clientResponseFormat, - } as const; + targetFormat: optionalString(args.clientResponseFormat), + }; } diff --git a/open-sse/handlers/chatCore/requestToolIdentity.ts b/open-sse/handlers/chatCore/requestToolIdentity.ts index 5b77866925..09e7602887 100644 --- a/open-sse/handlers/chatCore/requestToolIdentity.ts +++ b/open-sse/handlers/chatCore/requestToolIdentity.ts @@ -1,4 +1,25 @@ -type NamespaceIdentity = { namespace: string; name: string }; +export type NamespaceIdentity = { namespace: string; name: string }; + +/** + * Return a string-valued copy only when the complete map is an alias ledger. + * + * The legacy `_toolNameMap` side channel can carry either response aliases or + * namespace identities. Checking every value before copying keeps those two + * contracts separate and gives callers a real `Map` instead of + * asserting an identity map into the alias shape. + */ +export function toToolNameAliasMap( + map: ReadonlyMap | null +): Map | null { + if (!map || map.size === 0) return null; + + const aliases = new Map(); + for (const [wireName, originalName] of map) { + if (typeof originalName !== "string") return null; + aliases.set(wireName, originalName); + } + return aliases; +} /** * Extract the #7936 request-tool identity map from the translated body and diff --git a/tests/unit/chatcore-postcall-guardrail-context.test.ts b/tests/unit/chatcore-postcall-guardrail-context.test.ts index c539e7e1ea..74d39a4f4d 100644 --- a/tests/unit/chatcore-postcall-guardrail-context.test.ts +++ b/tests/unit/chatcore-postcall-guardrail-context.test.ts @@ -70,3 +70,20 @@ test("null apiKeyInfo coalesces to null for resolveDisabledGuardrails", () => { }); assert.equal(received.apiKeyInfo, null); }); + +test("normalizes untyped context fields before returning the guardrail contract", () => { + const ctx = buildPostCallGuardrailContext( + baseArgs({ + apiKeyInfo: ["not", "a", "record"], + clientRawRequest: { endpoint: 42 }, + responsePayloadFormat: { format: "openai" }, + clientResponseFormat: false, + }), + () => [] + ); + + assert.equal(ctx.apiKeyInfo, null); + assert.equal(ctx.endpoint, null); + assert.equal(ctx.sourceFormat, null); + assert.equal(ctx.targetFormat, null); +}); diff --git a/tests/unit/chatcore-request-tool-identity-contracts.test.ts b/tests/unit/chatcore-request-tool-identity-contracts.test.ts new file mode 100644 index 0000000000..91b7a078f3 --- /dev/null +++ b/tests/unit/chatcore-request-tool-identity-contracts.test.ts @@ -0,0 +1,30 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { toToolNameAliasMap } from "../../open-sse/handlers/chatCore/requestToolIdentity.ts"; + +test("copies a string-valued request tool alias ledger", () => { + const source = new Map([["lowercase_tool", "OriginalTool"]]); + + const aliases = toToolNameAliasMap(source); + + assert.deepEqual(aliases, new Map([["lowercase_tool", "OriginalTool"]])); + assert.notEqual(aliases, source); +}); + +test("does not reinterpret namespace identities as response aliases", () => { + const identities = new Map([ + ["mcp__files__read", { namespace: "mcp__files", name: "read" }], + ]); + + assert.equal(toToolNameAliasMap(identities), null); +}); + +test("rejects mixed alias and identity ledgers", () => { + const mixed = new Map([ + ["plain", "PlainTool"], + ["mcp__files__read", { namespace: "mcp__files", name: "read" }], + ]); + + assert.equal(toToolNameAliasMap(mixed), null); +});