diff --git a/src/lib/memory/__tests__/injection.test.ts b/src/lib/memory/__tests__/injection.test.ts index 716f40ddd9..8e9bbce030 100644 --- a/src/lib/memory/__tests__/injection.test.ts +++ b/src/lib/memory/__tests__/injection.test.ts @@ -188,6 +188,32 @@ describe("injectMemory — edge cases", () => { }); }); +describe("injectMemory — Claude-family cache-safe splice gate (#11290)", () => { + test("does not splice mid-array on anthropic when the last turn before the splice point is plain assistant text", () => { + const request = makeRequest({ + messages: [ + { role: "system", content: "SYSTEM PROMPT" }, + { role: "user", content: "turn 1 question" }, + { role: "assistant", content: "turn 1 answer" }, + { role: "user", content: "turn 2 question" }, + ], + }); + const memories = [makeMemory("dark mode")]; + + const result = injectMemory(request, memories, "anthropic", { cacheSafe: true }); + + // The plain-text assistant turn must stay immediately followed by the final user + // turn — no system message spliced between them (that shape is what Opus 5 rejects + // with HTTP 400, #11290). Memory is merged into the leading system message instead. + expect(result.messages).toHaveLength(4); + expect(result.messages[0].role).toBe("system"); + expect(result.messages[0].content).toContain("Memory context: dark mode"); + expect(result.messages[0].content).toContain("SYSTEM PROMPT"); + expect(result.messages[2]).toEqual({ role: "assistant", content: "turn 1 answer" }); + expect(result.messages[3]).toEqual({ role: "user", content: "turn 2 question" }); + }); +}); + describe("shouldInjectMemory", () => { test("returns true when messages are present and enabled not set", () => { const request = makeRequest(); diff --git a/src/lib/memory/injection.ts b/src/lib/memory/injection.ts index d4d8ead7f7..c509583529 100644 --- a/src/lib/memory/injection.ts +++ b/src/lib/memory/injection.ts @@ -12,6 +12,10 @@ import { Memory } from "./types"; import { logger } from "../../../open-sse/utils/logger.ts"; +import { + isAnthropicCompatibleProvider, + isClaudeCodeCompatibleProvider, +} from "../../shared/constants/providers"; const log = logger("MEMORY_INJECTION"); @@ -170,6 +174,43 @@ function injectSystemFirst( return { ...request, messages: [memorySystemMessage, ...messages] }; } +/** + * #11290: providers in the Claude family (direct Anthropic, and any + * anthropic-compatible / Claude-Code-compatible passthrough connection) — the + * ones affected by the stricter Opus 5 message-ordering validation described + * below. Deliberately narrower than `systemMessageMustBeFirst()`'s strict-set: + * this only gates the cache-safe mid-array splice, not the leading-system-message + * requirement, so non-Claude providers keep the #3890 cache-hit optimization + * unconditionally. + */ +function isClaudeFamilyProvider(provider: string | null | undefined): boolean { + if (!provider) return false; + const normalized = provider.toLowerCase().trim(); + return ( + normalized === "claude" || + normalized === "anthropic" || + isClaudeCodeCompatibleProvider(provider) || + isAnthropicCompatibleProvider(provider) + ); +} + +/** + * True when an assistant message's content ends in a server-side tool result + * block (e.g. `web_search_tool_result`, `code_execution_tool_result`, + * `mcp_tool_result` — any Anthropic content block whose type ends in + * `_tool_result`, produced by a server-executed tool rather than a + * client-executed one). `content` is typed as `string` on `ChatMessage` for + * the common case, but the Claude-native wire shape carries an array of + * content blocks — this only recognizes that richer shape. + */ +function endsWithServerToolResult(message: ChatMessage | undefined): boolean { + if (!message || message.role !== "assistant") return false; + const content = message.content as unknown; + if (!Array.isArray(content) || content.length === 0) return false; + const lastBlock = content[content.length - 1] as { type?: unknown } | null | undefined; + return typeof lastBlock?.type === "string" && lastBlock.type.endsWith("_tool_result"); +} + /** * Place a memory message at the #3890 cache-safe anchor (just before the last * user turn) when one exists, else prepend it. Shared by the system and user @@ -222,6 +263,24 @@ export function injectMemory( return injectSystemFirst(request, messages, memoryText, memories.length); } + // #11290: Claude Opus 5 tightened server-side validation of the cache-safe + // mid-array splice — a system message spliced right after a plain-text assistant + // turn is rejected with HTTP 400 (the immediately preceding message must end in a + // server-side tool result for a following system message to be accepted). Rather + // than adding "claude"/"anthropic" outright to `systemMessageMustBeFirst()` (which + // would revert the #3890 cache-hit optimization for every Claude request, including + // the ones that work fine today), only fall back to the leading-system-message + // placement for the specific requests where the turn right before the splice point + // isn't a server tool result. + if ( + supportsSystem && + cacheSafeIndex >= 0 && + isClaudeFamilyProvider(provider) && + !endsWithServerToolResult(messages[cacheSafeIndex - 1]) + ) { + return injectSystemFirst(request, messages, memoryText, memories.length); + } + // Strategy 1 (system): prepend before existing system messages, preserving the // caller's own instructions. Strategy 2 (user, e.g. o1-mini): inject as a user // message. Both honor the #3890 cache-safe anchor via placeMessage. diff --git a/tests/unit/memory-cache-safe-injection.test.ts b/tests/unit/memory-cache-safe-injection.test.ts index c86e39df08..be97c54c5e 100644 --- a/tests/unit/memory-cache-safe-injection.test.ts +++ b/tests/unit/memory-cache-safe-injection.test.ts @@ -39,16 +39,20 @@ function multiTurn(): ChatRequest { describe("injectMemory cache-safe positioning (#3890)", () => { it("default (cacheSafe off) prepends memory at index 0 — unchanged legacy behavior", () => { - const out = injectMemory(multiTurn(), [mem("dark mode")], "anthropic"); + const out = injectMemory(multiTurn(), [mem("dark mode")], "openai"); assert.equal(out.messages[0].role, "system"); assert.ok(out.messages[0].content.includes("Memory context")); assert.equal(out.messages[1].content, "SYSTEM PROMPT"); }); + // Note: "openai" here stands in for any non-Claude-family provider that honors the + // cache-safe mid-array splice (e.g. DashScope/Xiaomi MiMo via OpenAI-format + // cache_control). Claude-family providers (anthropic/claude/CC-compatible) have their + // own, narrower gate covered in the "#11290" describe block below. it("cacheSafe inserts memory just before the last user message, preserving the prefix", () => { const req = multiTurn(); const prefixBefore = JSON.stringify(req.messages.slice(0, 3)); // sys, u1, a1 - const out = injectMemory(req, [mem("dark mode")], "anthropic", { cacheSafe: true }); + const out = injectMemory(req, [mem("dark mode")], "openai", { cacheSafe: true }); // The cacheable prefix (system + prior turns up to the last assistant) is byte-identical. assert.equal(JSON.stringify(out.messages.slice(0, 3)), prefixBefore); @@ -75,8 +79,8 @@ describe("injectMemory cache-safe positioning (#3890)", () => { }; const turn2 = multiTurn(); - const out1 = injectMemory(turn1, [mem("A")], "anthropic", { cacheSafe: true }); - const out2 = injectMemory(turn2, [mem("B")], "anthropic", { cacheSafe: true }); + const out1 = injectMemory(turn1, [mem("A")], "openai", { cacheSafe: true }); + const out2 = injectMemory(turn2, [mem("B")], "openai", { cacheSafe: true }); // The cache-breakpoint-bearing system message stays at the head, byte-identical, in // both turns (and is NOT displaced by the per-query memory) — so the prompt cache @@ -103,3 +107,76 @@ describe("injectMemory cache-safe positioning (#3890)", () => { assert.equal(out.messages[1].content, "SYS"); }); }); + +/** + * #11290: Claude Opus 5 tightened server-side validation and started rejecting the + * #3890 cache-safe mid-array splice with HTTP 400 whenever the assistant turn + * immediately before the splice point is a plain-text turn (not a server-side tool + * result). These tests pin the narrower, Claude-family-only gate added to + * `injectMemory()`: fall back to leading-system-message placement in that specific + * case, while still honoring the mid-array splice everywhere it is safe (non-Claude + * providers unconditionally, and Claude providers whose preceding turn IS a server + * tool result). + */ +describe("injectMemory cache-safe positioning — Claude-family server-tool-result gate (#11290)", () => { + it("falls back to leading system-message placement for anthropic when the preceding assistant turn is plain text", () => { + const out = injectMemory(multiTurn(), [mem("dark mode")], "anthropic", { cacheSafe: true }); + + // No splice: the memory is merged into the leading system message instead of being + // inserted right after the plain-text "turn 1 answer" assistant turn. + assert.equal(out.messages.length, 4); + assert.equal(out.messages[0].role, "system"); + assert.ok(out.messages[0].content.includes("Memory context: dark mode")); + assert.ok(out.messages[0].content.includes("SYSTEM PROMPT")); + assert.equal(out.messages[1].content, "turn 1 question"); + assert.equal(out.messages[2].content, "turn 1 answer"); + assert.equal(out.messages[3].content, "turn 2 question"); + }); + + it("still splices mid-array for anthropic when the preceding assistant turn ends in a server tool result", () => { + const req: ChatRequest = { + model: "anthropic/claude-opus-5", + messages: [ + { role: "system", content: "SYSTEM PROMPT" }, + { role: "user", content: "turn 1 question" }, + { + role: "assistant", + content: [ + { type: "server_tool_use", id: "srvtoolu_1", name: "web_search", input: {} }, + { type: "web_search_tool_result", tool_use_id: "srvtoolu_1", content: [] }, + ], + } as unknown as ChatRequest["messages"][number], + { role: "user", content: "turn 2 question" }, + ], + }; + + const out = injectMemory(req, [mem("dark mode")], "anthropic", { cacheSafe: true }); + + assert.equal(out.messages.length, 5); + assert.equal(out.messages[0].content, "SYSTEM PROMPT"); + assert.equal(out.messages[3].role, "system"); + assert.ok(out.messages[3].content.includes("Memory context")); + assert.equal(out.messages[4].content, "turn 2 question"); + }); + + it("applies the same fallback to a Claude-Code-compatible passthrough provider id", () => { + const out = injectMemory(multiTurn(), [mem("dark mode")], "anthropic-compatible-cc-github-copilot", { + cacheSafe: true, + }); + + assert.equal(out.messages.length, 4); + assert.equal(out.messages[0].role, "system"); + assert.ok(out.messages[0].content.includes("Memory context: dark mode")); + assert.ok(out.messages[0].content.includes("SYSTEM PROMPT")); + }); + + it("does not gate non-Claude providers even without a server tool result", () => { + const out = injectMemory(multiTurn(), [mem("dark mode")], "openai", { cacheSafe: true }); + + // Unaffected by #11290: the mid-array splice is preserved for non-Claude providers. + assert.equal(out.messages.length, 5); + assert.equal(out.messages[3].role, "system"); + assert.ok(out.messages[3].content.includes("Memory context")); + assert.equal(out.messages[4].content, "turn 2 question"); + }); +});