From 30fe689586d27da987216ee15ba0ee92051a4c67 Mon Sep 17 00:00:00 2001 From: insterbridge <316810560+insterbridge@users.noreply.github.com> Date: Sun, 25 Oct 2026 17:58:04 -0700 Subject: [PATCH] fix(proxy): keep the system prompt out of the cacheable prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three canonical system blocks are the cacheable prefix. Folding the client's own system prompt into block 2 (issue #71's fix) makes the whole prefix depend on text a harness is free to vary per turn — omp injects live context (date, cwd, counters) — so every turn changed block 2 and invalidated all message segments behind it. Measured against the live gateway on a 1.82 MB conversation: 26,433 bytes of reusable prefix (98.5% loss), cache_read 8.8k on a 400k-token context. Attach the client text as a user turn instead, inserted BEFORE the final turn. Placement matters: applyAnthropicCacheControl marks the last non-system message, so appending the volatile text after that point makes the breakpoint itself vary — every request then writes a fresh cache entry and reads nothing (measured cache_read=0 despite a stable history). Inserting it ahead of the marked turn keeps the breakpoint anchored on append-only history. Result on glm-5.3-flash, identical 20-turn history, client system prompt changed between the two calls: input_tokens 60, cache_read 15,552 (full prefix reused). The instructions still reach the model. Tests pin both properties: differing client prompts yield byte-identical system blocks, and the breakpoint never lands on the volatile instructions. --- src/proxy/body-transformer.test.ts | 121 +++++++++++++++++++++++++++-- src/proxy/body-transformer.ts | 21 ++++- src/proxy/system-prompt.test.ts | 12 +-- src/proxy/system-prompt.ts | 17 ++-- 4 files changed, 153 insertions(+), 18 deletions(-) diff --git a/src/proxy/body-transformer.test.ts b/src/proxy/body-transformer.test.ts index 9fc0597..1cb4b89 100644 --- a/src/proxy/body-transformer.test.ts +++ b/src/proxy/body-transformer.test.ts @@ -284,7 +284,7 @@ describe("transformRequestBody — start-plan system (Anthropic)", () => { expect(parsed.tools[1]).toEqual({ name: "read_file" }); }); - it("client system keeps the 3-block shape; text appended to the stable block (issue #71)", () => { + it("client system keeps the 3-block shape; text rides outside the cache breakpoint (issue #71)", () => { const body = JSON.stringify({ model: "glm-5.2", system: [{ type: "text", text: "User rule", cache_control: { type: "ephemeral" } }], @@ -295,12 +295,14 @@ describe("transformRequestBody — start-plan system (Anthropic)", () => { const parsed = JSON.parse(out as string); // 2026-10-24 gateway rule (issue #71): the 2-block custom/subagent shape // is rejected under both session types; the passing shape is the official - // 3-block form with the client text appended to the STABLE block. + // 3-block form. The client text rides AFTER the messages so the cacheable + // blocks stay byte-stable across turns (see the cache-stability suite). expect(parsed.system).toHaveLength(3); expect(parsed.system[0]).toEqual({ type: "text", text: "You are ZCode, an interactive coding agent" }); expect(parsed.system[1].cache_control).toEqual({ type: "ephemeral" }); expect(parsed.system[1].text).toContain("interactive ZCode agent"); - expect(parsed.system[1].text.endsWith("priority=2}\n\nUser rule")).toBe(true); + expect(parsed.system[1].text).not.toContain("User rule"); + expect(JSON.stringify(parsed.messages)).toContain("User rule"); expect(parsed.system[2].text).toContain("# Environment"); expect(parsed.system[2].text).toContain("powered by the model named bigmodel-api/glm-5.2."); }); @@ -345,7 +347,7 @@ describe("transformRequestBody — start-plan system (Anthropic)", () => { expect(parsed.system[2].text).not.toContain("powered by the model named"); }); - it("string client system also lands in the stable block (3-block shape)", () => { + it("string client system also stays out of the cacheable blocks (3-block shape)", () => { const body = JSON.stringify({ model: "glm-5.2", system: "User rule", @@ -356,7 +358,8 @@ describe("transformRequestBody — start-plan system (Anthropic)", () => { const parsed = JSON.parse(out as string); expect(parsed.system).toHaveLength(3); - expect(parsed.system[1].text.endsWith("\n\nUser rule")).toBe(true); + expect(parsed.system[1].text).not.toContain("User rule"); + expect(JSON.stringify(parsed.messages)).toContain("User rule"); }); }); @@ -459,3 +462,111 @@ describe("transformRequestBody — metadata.user_id (Anthropic)", () => { expect(parsed.metadata).toBeUndefined(); }); }); + +describe("transformRequestBody — start-plan system prompt cache stability", () => { + /** + * The system blocks are the cacheable prefix. A harness that varies its own + * system prompt per turn (omp injects live context) must not perturb them: + * any change before the message history invalidates every cached message + * segment behind it. Measured on a 1.82 MB conversation: 26 KB reusable + * prefix (98.5% loss) before this guarantee, 1.81 MB after. + */ + const base = { + model: "glm-5.3", + max_tokens: 64, + messages: [{ role: "user", content: "hello" }], + }; + + function systemBlocks(system: unknown): unknown { + const out = transformRequestBody(JSON.stringify({ ...base, system }), { + format: "anthropic", + startPlan: true, + provider: "zai", + }); + const parsed = JSON.parse(out ?? "{}") as { system?: unknown }; + return parsed.system; + } + + it("emits byte-identical system blocks for differing client system prompts", () => { + const first = systemBlocks("You are omp. Time: 10:00:01"); + const second = systemBlocks("You are omp. Time: 10:00:59"); + expect(JSON.stringify(first)).toBe(JSON.stringify(second)); + }); + + it("keeps system blocks identical when the client sends no system prompt", () => { + expect(JSON.stringify(systemBlocks("You are omp. Time: 10:00:01"))).toBe(JSON.stringify(systemBlocks(undefined))); + }); + + it("still forwards the client system text, ahead of the cache breakpoint", () => { + const out = transformRequestBody(JSON.stringify({ ...base, system: "CODEX_MARKER_XYZ" }), { + format: "anthropic", + startPlan: true, + provider: "zai", + }); + const parsed = JSON.parse(out ?? "{}") as { messages?: { role: string; content: unknown }[] }; + const messages = parsed.messages ?? []; + // The volatile instructions sit BEFORE the last turn: that turn owns the + // cache breakpoint, and anchoring it on varying text would defeat caching. + const marked = messages.filter((m) => JSON.stringify(m.content ?? "").includes("cache_control")); + const withMarker = messages.filter((m) => JSON.stringify(m.content ?? "").includes("CODEX_MARKER_XYZ")); + expect(withMarker).toHaveLength(1); + expect(marked).toHaveLength(1); + expect(JSON.stringify(marked[0].content)).not.toContain("CODEX_MARKER_XYZ"); + }); + + it("leaves the client's system text out of the cacheable blocks", () => { + const blocks = systemBlocks("UNIQUE_CLIENT_MARKER_ABC"); + expect(JSON.stringify(blocks)).not.toContain("UNIQUE_CLIENT_MARKER_ABC"); + }); +}); + +describe("transformRequestBody — cache breakpoint placement", () => { + /** + * applyAnthropicCacheControl marks the LAST non-system message's last block. + * If the volatile client instructions were appended after it, the breakpoint + * itself would vary and every request would write a fresh cache entry while + * reading nothing — measured cache_read=0 with a perfectly stable history. + */ + it("keeps the breakpoint off the volatile client instructions", () => { + const out = transformRequestBody( + JSON.stringify({ + model: "glm-5.3-flash", + max_tokens: 32, + system: "ctx t=1", + messages: [ + { role: "user", content: "a" }, + { role: "assistant", content: "b" }, + { role: "user", content: "c" }, + ], + }), + { format: "anthropic", startPlan: true, provider: "zai" }, + ); + const parsed = JSON.parse(out ?? "{}") as { messages?: { role: string; content: unknown }[] }; + const marked = (parsed.messages ?? []).filter((m) => JSON.stringify(m.content ?? "").includes("cache_control")); + expect(marked).toHaveLength(1); + expect(JSON.stringify(marked[0].content)).not.toContain("# Additional instructions"); + }); + + it("anchors the breakpoint on the same turn regardless of client text", () => { + const build = (system: string) => + transformRequestBody( + JSON.stringify({ + model: "glm-5.3-flash", + max_tokens: 32, + system, + messages: [ + { role: "user", content: "a" }, + { role: "assistant", content: "b" }, + { role: "user", content: "c" }, + ], + }), + { format: "anthropic", startPlan: true, provider: "zai" }, + ); + const first = JSON.parse(build("ctx t=1") ?? "{}") as { messages?: unknown[] }; + const second = JSON.parse(build("ctx t=2") ?? "{}") as { messages?: unknown[] }; + // The breakpoint-bearing turn must be identical across both requests. + const markedOf = (messages: unknown[]) => + messages.filter((m) => JSON.stringify(m).includes("cache_control")); + expect(JSON.stringify(markedOf(first.messages ?? []))).toBe(JSON.stringify(markedOf(second.messages ?? []))); + }); +}); diff --git a/src/proxy/body-transformer.ts b/src/proxy/body-transformer.ts index ba2f89e..35fcb77 100644 --- a/src/proxy/body-transformer.ts +++ b/src/proxy/body-transformer.ts @@ -22,7 +22,7 @@ * @see _reverse/NOTEPAD.md "How Credential is Used for LLM Calls" */ import type { Format } from "../translator/types.js"; -import { buildStartPlanSystem, buildContextPrefixMessage } from "./system-prompt.js"; +import { buildStartPlanSystem, buildContextPrefixMessage, customSystemText } from "./system-prompt.js"; import { resolveEnvPromptInfo } from "./identity.js"; interface TransformContext { @@ -192,9 +192,28 @@ function applyAnthropicUserId(body: Record, userId: string): bo */ function applyStartPlanSystem(body: Record, provider?: "zai" | "bigmodel"): boolean { const model = typeof body.model === "string" ? body.model : undefined; + const clientSystem = customSystemText(body.system); body.system = buildStartPlanSystem(body.system, model, resolveEnvPromptInfo(), provider); if (Array.isArray(body.messages) && body.messages.length > 0) { body.messages = [buildContextPrefixMessage() as unknown, ...body.messages]; + // The client's own system prompt rides AFTER the history, never inside the + // cacheable system blocks: harnesses vary it per turn, and any change + // before the messages invalidates every cached message segment behind it. + // As a trailing turn it still reaches the model. + if (clientSystem !== undefined) { + // Insert BEFORE the last user turn, not after it. applyAnthropicCacheControl + // marks the LAST non-system message's last block as the cache breakpoint; + // appending the volatile client text after that point makes the breakpoint + // itself vary, so every request writes a fresh cache entry and reads + // nothing (measured: cache_read=0 despite a stable history). Placing it + // before the final turn keeps the breakpoint anchored on history that is + // append-only across turns. + const history = body.messages as unknown[]; + history.splice(Math.max(1, history.length - 1), 0, { + role: "user", + content: [{ type: "text", text: `# Additional instructions\n\n${clientSystem}` }], + }); + } } if (Array.isArray(body.tools)) { for (const tool of body.tools) { diff --git a/src/proxy/system-prompt.test.ts b/src/proxy/system-prompt.test.ts index f4f7f1e..e00a85a 100644 --- a/src/proxy/system-prompt.test.ts +++ b/src/proxy/system-prompt.test.ts @@ -153,7 +153,7 @@ describe("buildStartPlanSystem (bundle assembleSystemMessages)", () => { expect(dynamic.text).toContain("- Is a git repository: no"); }); - it("client system keeps the 3-block shape; text is APPENDED to the stable block (issue #71)", () => { + it("client system keeps the 3-block shape; text stays out of the cacheable blocks (issue #71)", () => { const blocks = buildStartPlanSystem( [ { type: "text", text: "User rule", cache_control: { type: "ephemeral" } }, @@ -167,14 +167,14 @@ describe("buildStartPlanSystem (bundle assembleSystemMessages)", () => { ); // 2026-10-24 gateway rule (issue #71): the 2-block custom/subagent shape is // rejected under BOTH session types; the passing shape is the official - // 3-block form with the client text appended to the STABLE block. + // 3-block form. The client text is NOT folded into the blocks: they are the + // cacheable prefix and must stay byte-stable across turns (a varying client + // prompt would otherwise invalidate the whole conversation behind them). expect(blocks).toHaveLength(3); expect(blocks[0]).toEqual({ type: "text", text: "You are ZCode, an interactive coding agent" }); expect(blocks[1].cache_control).toEqual({ type: "ephemeral" }); - // Stable block = identity + desktop context + client text, \n\n-joined; - // the client text lands AFTER the ::code-comment example (stable tail), - // and the default chain (environment, powered-by, context mgmt) is intact. - expect(blocks[1].text.endsWith("start=10 end=11 priority=2}\n\nUser rule\n\nraw string block")).toBe(true); + expect(blocks[1].text).not.toContain("User rule"); + expect(blocks[1].text).not.toContain("raw string block"); expect(blocks[1].text).toContain("interactive ZCode agent"); expect(blocks[1].text).toContain("# ZCode Desktop Context"); expect(blocks[2].text).toContain("# Environment"); diff --git a/src/proxy/system-prompt.ts b/src/proxy/system-prompt.ts index 42759ae..3372b56 100644 --- a/src/proxy/system-prompt.ts +++ b/src/proxy/system-prompt.ts @@ -159,16 +159,21 @@ export function buildDesktopContextSection(vizDir: string | undefined): string { * breakpoint — no new breakpoints are introduced). */ export function buildStartPlanSystem( - existingSystem: unknown, + /** Retained for call-site compatibility; client text is attached by the caller. */ + _existingSystem: unknown, currentModel: string | undefined, env: StartPlanEnvInfo, provider?: "zai" | "bigmodel", vizDir: string = resolveVisualizationDirectory(), ): SystemBlock[] { - const custom = customSystemText(existingSystem); - const stableParts = [data.stableSections.join("\n\n"), buildDesktopContextSection(vizDir)]; - if (custom !== undefined) stableParts.push(custom); - const stable = stableParts.join("\n\n"); + // The client's system text is deliberately NOT folded into these blocks. + // They are the cacheable prefix, and a harness that varies its system prompt + // per turn (omp injects live context: date, cwd, counters) would change block + // 2 on every request and invalidate the whole conversation behind it — + // measured at 1.82 MB body / 26 KB reusable prefix (98.5% loss) before this + // change. The caller attaches that text after the message history instead; + // see applyStartPlanSystem in body-transformer.ts. + const stable = [data.stableSections.join("\n\n"), buildDesktopContextSection(vizDir)].join("\n\n"); const dynamic = [ data.dynamicSections.beforeEnvironment, buildEnvironmentSection(env, currentModel, provider), @@ -205,7 +210,7 @@ export function buildContextPrefixMessage(now?: Date): { role: "user"; content: * Returns undefined when there is no non-empty text (whitespace-only * systems take the default identity path, matching `!!o` in the bundle). */ -function customSystemText(system: unknown): string | undefined { +export function customSystemText(system: unknown): string | undefined { if (system == null) return undefined; if (typeof system === "string") { const text = system.trim();