From 152d95108c9c3d557562311ffed63240a511eb31 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 11 Sep 2026 22:29:29 -0300 Subject: [PATCH] fix(cache): include tool_choice/tools/response_format in semantic cache signature (#12734) (#13267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged as part of the owner batch of 2026-09-11. This PR had a live worktree in another session, so it sat outside the main 39. Merged on your explicit call, validated first rather than taken on trust: boarded with the other 10 worktree-held PRs into a consolidated worktree off `release/v3.8.51`. - ESLint over every changed file: no errors - `typecheck:core` clean; `check:dashboard-typecheck` OK; `check:changelog-integrity` OK - complexity 2821 / baseline 3218 and cognitive-complexity 1272 / baseline 1437 - 203 of 208 assertions green. The 5 remaining (`guide-settings-route` ×4, `hard-session-lease-bypass-inventory` ×1) reproduce on the pure tip with nothing from this batch applied. - `imageGeneration.ts` rebaselined 3259 → 3293 for #12945's image-only-model guard, landed separately in #13392 so nothing was pushed onto a live branch. ⚠️ base-red inherited: #12732 — provider count 356 vs 358 and `open-sse/utils/stream.ts` 3115 > frozen 3098, both reproducing on the pure tip. --- .../fixes/12734-semantic-cache-tool-choice.md | 1 + open-sse/handlers/chatCore/semanticCache.ts | 11 ++- .../handlers/chatCore/semanticCacheStore.ts | 10 ++- .../chatCore/streamingSemanticCacheStore.ts | 10 ++- src/lib/semanticCache.ts | 45 ++++++++++- .../chatcore-semantic-cache-store.test.ts | 35 ++++++++ tests/unit/chatcore-semantic-cache.test.ts | 81 ++++++++++++++++++- .../chatcore-streaming-cache-store.test.ts | 35 ++++++++ tests/unit/semantic-cache.test.ts | 75 +++++++++++++++++ 9 files changed, 295 insertions(+), 8 deletions(-) create mode 100644 changelog.d/fixes/12734-semantic-cache-tool-choice.md diff --git a/changelog.d/fixes/12734-semantic-cache-tool-choice.md b/changelog.d/fixes/12734-semantic-cache-tool-choice.md new file mode 100644 index 0000000000..e3dc7045f9 --- /dev/null +++ b/changelog.d/fixes/12734-semantic-cache-tool-choice.md @@ -0,0 +1 @@ +- fix(cache): fold tool_choice/tools/response_format into the semantic cache signature so a cached tool_calls response can no longer be replayed for a request whose tool policy forbids it (#12734) diff --git a/open-sse/handlers/chatCore/semanticCache.ts b/open-sse/handlers/chatCore/semanticCache.ts index fbcf53fedb..dcfd792600 100644 --- a/open-sse/handlers/chatCore/semanticCache.ts +++ b/open-sse/handlers/chatCore/semanticCache.ts @@ -29,7 +29,13 @@ export async function checkSemanticCache({ semanticCacheEnabled: boolean; // Only the fields this read path actually touches are named; everything else // on the request body stays `unknown` via the index signature. - body: Record & { temperature?: number; top_p?: number }; + body: Record & { + temperature?: number; + top_p?: number; + tool_choice?: unknown; + tools?: unknown; + response_format?: unknown; + }; clientRawRequest: { headers?: unknown } | null; model: string; provider: string; @@ -51,7 +57,8 @@ export async function checkSemanticCache({ body.messages ?? body.input, body.temperature, body.top_p, - apiKeyId ?? undefined + apiKeyId ?? undefined, + { toolChoice: body.tool_choice, tools: body.tools, responseFormat: body.response_format } ); const cached = getCachedResponse(signature); if (cached) { diff --git a/open-sse/handlers/chatCore/semanticCacheStore.ts b/open-sse/handlers/chatCore/semanticCacheStore.ts index ff4e7d590c..07ee1175d0 100644 --- a/open-sse/handlers/chatCore/semanticCacheStore.ts +++ b/open-sse/handlers/chatCore/semanticCacheStore.ts @@ -22,6 +22,9 @@ type CacheBody = { input?: unknown; temperature?: number; top_p?: number; + tool_choice?: unknown; + tools?: unknown; + response_format?: unknown; }; type UsageLike = { prompt_tokens?: number; completion_tokens?: number } | null | undefined; @@ -65,7 +68,12 @@ export function storeSemanticCacheResponse( args.body.messages ?? args.body.input, args.body.temperature, args.body.top_p, - args.apiKeyId ?? undefined + args.apiKeyId ?? undefined, + { + toolChoice: args.body.tool_choice, + tools: args.body.tools, + responseFormat: args.body.response_format, + } ); const tokensSaved = args.usage?.prompt_tokens + args.usage?.completion_tokens || 0; deps.setCachedResponse(signature, args.model, args.translatedResponse, tokensSaved); diff --git a/open-sse/handlers/chatCore/streamingSemanticCacheStore.ts b/open-sse/handlers/chatCore/streamingSemanticCacheStore.ts index 48bd144a3c..2a8434f4a3 100644 --- a/open-sse/handlers/chatCore/streamingSemanticCacheStore.ts +++ b/open-sse/handlers/chatCore/streamingSemanticCacheStore.ts @@ -23,6 +23,9 @@ type CacheBody = { input?: unknown; temperature?: number; top_p?: number; + tool_choice?: unknown; + tools?: unknown; + response_format?: unknown; }; export interface StreamingSemanticCacheStoreDeps { @@ -69,7 +72,12 @@ function writeStreamingCacheEntry( args.body.messages ?? args.body.input, args.body.temperature, args.body.top_p, - args.apiKeyId ?? undefined + args.apiKeyId ?? undefined, + { + toolChoice: args.body.tool_choice, + tools: args.body.tools, + responseFormat: args.body.response_format, + } ); const tokensSaved = streamTokensSaved(args.streamUsage); deps.setCachedResponse(sig, args.model, cleanBody, tokensSaved); diff --git a/src/lib/semanticCache.ts b/src/lib/semanticCache.ts index d2a1758ce2..c6704108f3 100644 --- a/src/lib/semanticCache.ts +++ b/src/lib/semanticCache.ts @@ -137,6 +137,43 @@ export function clearMemoryCache(): void { // ─── Signature Generation ───────────────── +/** + * Behavior-changing generation constraints that MUST be folded into the cache signature + * (#12734). Without these, a cached response produced under one `tool_choice`/`tools`/ + * `response_format` could be replayed for a later request that forbids or changes that + * behavior (e.g. a cached `tool_calls` response served to a `tool_choice: "none"` request). + */ +export interface SignatureConstraints { + toolChoice?: unknown; + tools?: unknown; + responseFormat?: unknown; +} + +/** Normalize a single tool definition, keeping only the fields that define its policy. */ +function normalizeTool(tool: unknown): unknown { + const record = asRecord(tool); + const fn = asRecord(record.function); + if (Object.keys(fn).length === 0 && Object.keys(record).length === 0) return tool; + return { + type: typeof record.type === "string" ? record.type : "function", + function: { + name: fn.name, + description: fn.description, + parameters: fn.parameters, + }, + }; +} + +/** + * Normalize `tools` for consistent hashing (mirrors `normalizeConversation` for messages): + * strips volatile/irrelevant fields while keeping name/description/parameters, which are + * what actually define the tool policy a cached response was generated under. + */ +function normalizeTools(tools: unknown): unknown { + if (!Array.isArray(tools) || tools.length === 0) return undefined; + return tools.map(normalizeTool); +} + /** * Generate deterministic cache signature from request params. * @param {string} model @@ -144,6 +181,8 @@ export function clearMemoryCache(): void { * @param {number} temperature * @param {number} topP * @param {string} [apiKeyId] - API key ID for per-key isolation (prevents cross-user cache hits) + * @param {SignatureConstraints} [constraints] - tool_choice/tools/response_format (#12734): + * these change model behavior and must not collide with a signature computed without them. * @returns {string} hex signature */ export function generateSignature( @@ -151,13 +190,17 @@ export function generateSignature( conversation, temperature = 0, topP = 1, - apiKeyId?: string + apiKeyId?: string, + constraints?: SignatureConstraints ) { const payload = JSON.stringify({ model, messages: normalizeConversation(conversation), temperature, top_p: topP, + tool_choice: constraints?.toolChoice, + tools: normalizeTools(constraints?.tools), + response_format: constraints?.responseFormat, }); const digest = crypto.createHash("sha256").update(payload).digest("hex"); // Per-key cache isolation (#3740) namespaces the signature with the apiKeyId as a diff --git a/tests/unit/chatcore-semantic-cache-store.test.ts b/tests/unit/chatcore-semantic-cache-store.test.ts index a39d039400..501733fe4a 100644 --- a/tests/unit/chatcore-semantic-cache-store.test.ts +++ b/tests/unit/chatcore-semantic-cache-store.test.ts @@ -135,3 +135,38 @@ test("missing usage → tokensSaved coerces to 0 (NaN || 0)", () => { storeSemanticCacheResponse(baseArgs({ usage: undefined }), deps); assert.equal(stored[0].tokens, 0); }); + +// #12734: tool_choice/tools/response_format must reach generateSignature so a cached +// tool_calls response cannot be replayed under a stricter tool policy. +test("signature is called with tool_choice/tools/response_format from body (#12734)", () => { + let captured: unknown[] = []; + const { deps } = makeDeps({ + generateSignature: (...a: unknown[]) => { + captured = a; + return "sig"; + }, + }); + const tools = [{ type: "function", function: { name: "get_weather" } }]; + storeSemanticCacheResponse( + baseArgs({ + body: { + messages: [{ role: "user", content: "hi" }], + temperature: 0, + top_p: 1, + tool_choice: "none", + tools, + response_format: { type: "json_object" }, + }, + }), + deps + ); + // args: (model, messages ?? input, temperature, top_p, apiKeyId, constraints) + const constraints = captured[5] as { + toolChoice: unknown; + tools: unknown; + responseFormat: unknown; + }; + assert.equal(constraints.toolChoice, "none"); + assert.deepEqual(constraints.tools, tools); + assert.deepEqual(constraints.responseFormat, { type: "json_object" }); +}); diff --git a/tests/unit/chatcore-semantic-cache.test.ts b/tests/unit/chatcore-semantic-cache.test.ts index 61b86cc4a1..e78adf25a1 100644 --- a/tests/unit/chatcore-semantic-cache.test.ts +++ b/tests/unit/chatcore-semantic-cache.test.ts @@ -180,12 +180,14 @@ function makeHitArgs(overrides: Record = {}) { // Seed the cache under the EXACT signature checkSemanticCache rebuilds for `args`. function seedHit(args: ReturnType["args"], response: unknown) { + const body = args.body as Record; const signature = generateSignature( args.model, - args.body.messages ?? (args.body as Record).input, + body.messages ?? body.input, args.body.temperature, - (args.body as Record).top_p, - args.apiKeyId ?? undefined + body.top_p, + args.apiKeyId ?? undefined, + { toolChoice: body.tool_choice, tools: body.tools, responseFormat: body.response_format } ); setCachedResponse(signature, args.model, response); return signature; @@ -492,3 +494,76 @@ test("checkSemanticCache HIT includes X-OmniRoute-Cache-Latency: synthetic heade "HIT response carries X-OmniRoute-Cache-Latency: synthetic marker" ); }); + +// ─── tool_choice / tools / response_format must be part of the signature (#12734) ──────────── + +test("#12734: cached tool_calls response must NOT be replayed for tool_choice: 'none'", async () => { + clearCache(); + const messages = [{ role: "user", content: "what is 2+2?" }]; + const toolCallResponse = { + id: "chatcmpl-tool-calls", + choices: [ + { + index: 0, + finish_reason: "tool_calls", + message: { + role: "assistant", + content: null, + tool_calls: [ + { id: "call_1", type: "function", function: { name: "memory_search", arguments: "{}" } }, + ], + }, + }, + ], + usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 }, + }; + // Stored under a body with NO tool_choice (mirrors the real pipeline: the cache check + // runs before memory/skill tool injection, so the signature it stores under never saw + // tool_choice at all). + const { args: storeArgs } = makeHitArgs({ body: { model: "gpt-4o", messages, temperature: 0 } }); + seedHit(storeArgs, toolCallResponse); + + const { args: forbidArgs } = makeHitArgs({ + body: { model: "gpt-4o", messages, temperature: 0, tool_choice: "none" }, + }); + const result = await checkSemanticCache(forbidArgs as Parameters[0]); + + assert.equal( + result, + null, + "a tool_choice:'none' request must be a cache MISS against a tool_calls response cached without tool_choice" + ); +}); + +test("#12734: identical tool_choice/tools/response_format across requests still HITs", async () => { + clearCache(); + const messages = [{ role: "user", content: "what is the weather?" }]; + const tools = [ + { + type: "function", + function: { name: "get_weather", description: "Get the weather", parameters: { type: "object" } }, + }, + ]; + const cached = { + id: "chatcmpl-tool-config-hit", + choices: [ + { index: 0, message: { role: "assistant", content: "sunny" }, finish_reason: "stop" }, + ], + usage: { prompt_tokens: 8, completion_tokens: 2, total_tokens: 10 }, + }; + const body = { + model: "gpt-4o", + messages, + temperature: 0, + tool_choice: "auto", + tools, + response_format: { type: "json_object" }, + }; + const { args: storeArgs } = makeHitArgs({ body }); + seedHit(storeArgs, cached); + + const { args: readArgs } = makeHitArgs({ body: { ...body } }); + const result = await checkSemanticCache(readArgs as Parameters[0]); + + assert.ok(result, "identical tool_choice/tools/response_format must still HIT"); +}); diff --git a/tests/unit/chatcore-streaming-cache-store.test.ts b/tests/unit/chatcore-streaming-cache-store.test.ts index d66986f361..9489ca0ed8 100644 --- a/tests/unit/chatcore-streaming-cache-store.test.ts +++ b/tests/unit/chatcore-streaming-cache-store.test.ts @@ -127,3 +127,38 @@ test("a throwing dep is swallowed (fail-open, non-critical)", () => { }); assert.doesNotThrow(() => storeStreamingSemanticCacheResponse(baseArgs(), deps)); }); + +// #12734: tool_choice/tools/response_format must reach generateSignature so a cached +// tool_calls streaming response cannot be replayed under a stricter tool policy. +test("signature is called with tool_choice/tools/response_format from body (#12734)", () => { + let captured: unknown[] = []; + const { deps } = makeDeps({ + generateSignature: (...a: unknown[]) => { + captured = a; + return "sig"; + }, + }); + const tools = [{ type: "function", function: { name: "get_weather" } }]; + storeStreamingSemanticCacheResponse( + baseArgs({ + body: { + messages: [{ role: "user", content: "hi" }], + temperature: 0, + top_p: 1, + tool_choice: "none", + tools, + response_format: { type: "json_object" }, + }, + }), + deps + ); + // args: (model, messages ?? input, temperature, top_p, apiKeyId, constraints) + const constraints = captured[5] as { + toolChoice: unknown; + tools: unknown; + responseFormat: unknown; + }; + assert.equal(constraints.toolChoice, "none"); + assert.deepEqual(constraints.tools, tools); + assert.deepEqual(constraints.responseFormat, { type: "json_object" }); +}); diff --git a/tests/unit/semantic-cache.test.ts b/tests/unit/semantic-cache.test.ts index a83cf2a799..e7f0ab44f7 100644 --- a/tests/unit/semantic-cache.test.ts +++ b/tests/unit/semantic-cache.test.ts @@ -107,6 +107,81 @@ describe("Semantic Cache", () => { const sigKeyless = generateSignature("gpt-4o", messages, 0, 1, undefined); assert.notEqual(sigKeyed, sigKeyless); }); + + // #12734: tool_choice/tools/response_format change model behavior and must not be + // ignored by the signature — otherwise a cached tool_calls response can be replayed + // for a request whose tool policy forbids it. + describe("tool_choice / tools / response_format (#12734)", () => { + const messages = [{ role: "user", content: "what is 2+2?" }]; + + it("generates different signatures for different tool_choice ('auto' vs 'none')", () => { + const sig1 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + toolChoice: "auto", + }); + const sig2 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + toolChoice: "none", + }); + assert.notEqual(sig1, sig2); + }); + + it("generates different signatures for a forced-function tool_choice", () => { + const sig1 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + toolChoice: "auto", + }); + const sig2 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + toolChoice: { type: "function", function: { name: "get_weather" } }, + }); + assert.notEqual(sig1, sig2); + }); + + it("generates different signatures for no tool_choice vs an explicit one (the #12734 collision)", () => { + const sigNoToolChoice = generateSignature("gpt-4o", messages, 0, 1); + const sigWithToolChoice = generateSignature("gpt-4o", messages, 0, 1, undefined, { + toolChoice: "none", + }); + assert.notEqual(sigNoToolChoice, sigWithToolChoice); + }); + + it("generates different signatures for different tools arrays", () => { + const tools1 = [{ type: "function", function: { name: "get_weather", parameters: {} } }]; + const tools2 = [{ type: "function", function: { name: "get_stock_price", parameters: {} } }]; + const sig1 = generateSignature("gpt-4o", messages, 0, 1, undefined, { tools: tools1 }); + const sig2 = generateSignature("gpt-4o", messages, 0, 1, undefined, { tools: tools2 }); + assert.notEqual(sig1, sig2); + }); + + it("generates different signatures for different response_format", () => { + const sig1 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + responseFormat: { type: "text" }, + }); + const sig2 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + responseFormat: { type: "json_object" }, + }); + assert.notEqual(sig1, sig2); + }); + + it("generates identical signatures when constraints are identical (no hit-rate regression)", () => { + const tools = [{ type: "function", function: { name: "get_weather", parameters: {} } }]; + const constraints = { + toolChoice: "auto", + tools, + responseFormat: { type: "json_object" }, + }; + const sig1 = generateSignature("gpt-4o", messages, 0, 1, undefined, constraints); + const sig2 = generateSignature("gpt-4o", messages, 0, 1, undefined, { + toolChoice: "auto", + tools: [{ type: "function", function: { name: "get_weather", parameters: {} } }], + responseFormat: { type: "json_object" }, + }); + assert.equal(sig1, sig2); + }); + + it("generates identical signatures for omitted constraints vs an explicitly empty constraints object", () => { + const sig1 = generateSignature("gpt-4o", messages, 0, 1, undefined); + const sig2 = generateSignature("gpt-4o", messages, 0, 1, undefined, {}); + assert.equal(sig1, sig2); + }); + }); }); describe("isCacheableForRead", () => {