From 0f1920efb6d694cceebf03154c19612f0009c105 Mon Sep 17 00:00:00 2001 From: Gunther Schulz Date: Wed, 29 Jul 2026 12:27:06 +0200 Subject: [PATCH] fix(thinking-block-sanitize): protect continuations by shape, not by tail distance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit planSanitize protected a tool-continuation's thinking only while it was the LATEST assistant turn (i === latestAsst). The predicate that matters — "is this turn's terminal tool_use answered by a following tool_result" — is a function of the message and what follows it, not of its distance from the tail. The two agree while the continuation is at the tail; they diverge the moment another turn lands after it, and the gate then flipped a byte-identical message from protected to stripped — a mid-history mutation the proxy itself causes, on every request where a continuation ages out of the tail, which is exactly the cache re-write this extension exists to prevent. Measured before the fix by an offline cross-request stability check over live captures: 133 violations over 563 requests on one session, 76 over 169 on another, all attributed to this extension. Zero after. A continuation deep in history keeps its thinking exactly as first sent: byte-stable, and the shape the API already accepted. Dropping it later buys nothing — the 400 this extension prevents concerns the latest turn — and costs a full prefix re-write. Regression test pins the v2StripSigned arm (v1 only drops omitted thinking, so the signed path is where the old gate bit): against the pre-fix planSanitize it fails, with the fix all 47 pass. Co-Authored-By: Claude Fable 5 --- proxy/extensions/thinking-block-sanitize.mjs | 36 +++++++++++----- test/proxy-thinking-block-sanitize.test.mjs | 45 ++++++++++++++++++++ 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/proxy/extensions/thinking-block-sanitize.mjs b/proxy/extensions/thinking-block-sanitize.mjs index 7a97bdf2..fee00cd9 100644 --- a/proxy/extensions/thinking-block-sanitize.mjs +++ b/proxy/extensions/thinking-block-sanitize.mjs @@ -92,13 +92,17 @@ function answersToolUse(msg, toolUseId) { ); } -// The latest assistant message is an active tool-continuation when its terminal +// An assistant message is an active tool-continuation when its terminal // block is a `tool_use` that is *paired with* — i.e. answered by — a following // `tool_result` carrying the same `tool_use_id`. Only then does the API require // that turn's thinking intact, so only then must we leave it untouched. Matching // the id (not merely the presence of any later tool_result) keeps the guard as // narrow as the approved rule: an unanswered terminal tool_use, or a later // tool_result that answers a *different* call, is not the protected case. +// +// The predicate is a function of the message and what follows it, NOT of its +// distance from the tail — see planSanitize's stability note for why that +// distinction is load-bearing. export function isActiveToolContinuation(messages, idx) { const msg = messages[idx]; if (!msg || !Array.isArray(msg.content) || msg.content.length === 0) return false; @@ -110,13 +114,6 @@ export function isActiveToolContinuation(messages, idx) { return false; } -function latestAssistantIndex(messages) { - for (let i = messages.length - 1; i >= 0; i--) { - if (messages[i] && messages[i].role === "assistant") return i; - } - return -1; -} - // --- v2 predicate --- // v2 strips signed `thinking` blocks (non-empty text) AND `redacted_thinking` @@ -142,10 +139,26 @@ export function isSignedThinkingForV2(block) { // `v2StripSigned` is the externally-determined boolean: should v2's // signed-thinking drop fire this request? (Caller has already computed // hash mismatch + session-state checks.) +// CROSS-REQUEST BYTE STABILITY (2026-07-28). Protection is decided by the +// message's own shape — "is this turn's terminal tool_use answered by a +// following tool_result" — and never by whether it is the LAST assistant +// turn. The two agree while the turn is at the tail; they diverge the moment +// another turn lands after it, and the earlier `i === latestAsst` gate then +// flipped a byte-identical message from protected to stripped. That is a +// mid-history mutation the proxy itself causes, on every request where a +// tool-continuation turn ages out of the tail — and a mid-history mutation is +// exactly what re-writes the cache we exist to preserve. Measured before the +// fix: 133 violations over 563 requests (session 35d72503) and 76 over 169 +// (session 58c979ce), every one attributed to this extension by +// tools/replay.mjs's cross-request check. +// +// A continuation stays protected once it is deep history: its thinking is +// forwarded exactly as first sent, which is both byte-stable AND the shape +// the API accepted the first time. Dropping it later buys nothing (the 400 +// this extension prevents is about the LATEST turn, #63147) and costs a +// full re-write. export function planSanitize(messages, { v2StripSigned = false } = {}) { if (!Array.isArray(messages)) return { messages, dropped: 0, droppedV2: 0 }; - const latestAsst = latestAssistantIndex(messages); - const protectLatest = latestAsst >= 0 && isActiveToolContinuation(messages, latestAsst); let dropped = 0; let droppedV2 = 0; @@ -157,9 +170,10 @@ export function planSanitize(messages, { v2StripSigned = false } = {}) { out.push(msg); continue; } - if (i === latestAsst && protectLatest) { + if (isActiveToolContinuation(messages, i)) { // Active continuation — leave thinking intact (both v1 and v2 respect // this; the API needs the signed thinking for the pending tool call). + // Position-independent by design: see the stability note above. out.push(msg); continue; } diff --git a/test/proxy-thinking-block-sanitize.test.mjs b/test/proxy-thinking-block-sanitize.test.mjs index f98a0fa4..964ad6ac 100644 --- a/test/proxy-thinking-block-sanitize.test.mjs +++ b/test/proxy-thinking-block-sanitize.test.mjs @@ -853,3 +853,48 @@ test("v2 session-file merge: cache-telemetry spread writes v2 fields to sessions assert.equal(sessionFileContents.tools_hash_baseline, ctx.meta._thinkingSanitizeV2.tools_hash_baseline); }); }); + + +// --- planSanitize: position independence (cross-request byte stability) --- +// +// Protection must be a function of the message's own shape — "is this turn's +// terminal tool_use answered by a following tool_result" — never of its +// distance from the tail. The two agree while the continuation IS the latest +// assistant turn; they diverge the moment another turn lands after it, and +// the earlier `i === latestAsst` gate then flipped a byte-identical message +// from protected to stripped. That is a mid-history mutation the proxy +// itself causes on every request where a continuation ages out of the tail — +// measured before the fix: 133 cross-request violations over 563 requests on +// one session, 76 over 169 on another, all attributed to this extension. +// +// v2StripSigned: true is the arm that bites — v1 only drops OMITTED thinking, +// so a signed-thinking continuation is exactly the case the old tail gate +// stripped once it aged out. (This test fails against the pre-fix planSanitize.) +test("planSanitize: an answered tool-continuation stays protected after it ages out of the tail", () => { + const continuation = { role: "assistant", content: [realThinking(), toolUse("t9")] }; + const asTail = [ + { role: "user", content: [text("q")] }, + continuation, + { role: "user", content: [toolResult("t9")] }, + ]; + // Same messages, one later exchange appended — the continuation is now + // mid-history and a NEWER assistant turn exists behind it. + const asMidHistory = [ + ...asTail, + { role: "assistant", content: [text("done")] }, + { role: "user", content: [text("next")] }, + ]; + const tailOut = planSanitize(asTail, { v2StripSigned: true }).messages[1]; + const midOut = planSanitize(asMidHistory, { v2StripSigned: true }).messages[1]; + assert.deepEqual(tailOut, continuation, "protected while latest (both gates agree here)"); + assert.deepEqual( + midOut, + continuation, + "the SAME bytes must stay protected once a later turn exists — stripping here is a mid-history mutation that re-bills the whole prefix", + ); + assert.equal( + JSON.stringify(tailOut), + JSON.stringify(midOut), + "cross-request byte stability: request N and N+1 must serialize this message identically", + ); +});