fix(thinking-block-sanitize): protect continuations by shape, not by tail distance

proxy/extensions/thinking-block-sanitize.mjs

@@ -92,13 +92,19 @@ 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 ITSELF 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 for v1. (planSanitize additionally scopes
+// v2's use of this predicate to the latest turn only — that's a decision
+// planSanitize makes on top of this predicate, not a change to the predicate.)
 export function isActiveToolContinuation(messages, idx) {
   const msg = messages[idx];
   if (!msg || !Array.isArray(msg.content) || msg.content.length === 0) return false;
@@ -110,13 +116,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 +141,42 @@ 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 — for v1. v1's
+// omitted-thinking drop is empirical: nothing forces us to touch a
+// byte-identical replayed block once it ages out of the tail, and doing so
+// buys nothing (the 400 this extension prevents is about the LATEST turn,
+// #63147) while costing a full re-write.
+//
+// v2 does NOT get the same by-shape protection. v2 fires only on a
+// tools-hash mismatch, which already busts the cache prefix from that point
+// on — so protecting a deep-history continuation's signed thinking there
+// buys zero cache benefit while reintroducing the stale-signature risk v2
+// exists to remove (docs/directives/proxy-thinking-block-sanitize-v2.md:58-67,
+// 153-159; PR #279 round-1 review, 2026-07-31). v2's protection therefore
+// stays tail-scoped: only the LATEST assistant turn, when it is itself an
+// active continuation, is left intact. Any earlier active continuation loses
+// its signed/redacted thinking under v2StripSigned exactly as main did
+// before this patch.
 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 latestAsst = -1;
+  for (let i = 0; i < messages.length; i++) {
+    if (messages[i] && messages[i].role === "assistant") latestAsst = i;
+  }
 
   let dropped = 0;
   let droppedV2 = 0;
@@ -157,22 +188,35 @@ export function planSanitize(messages, { v2StripSigned = false } = {}) {
       out.push(msg);
       continue;
     }
-    if (i === latestAsst && protectLatest) {
-      // Active continuation — leave thinking intact (both v1 and v2 respect
-      // this; the API needs the signed thinking for the pending tool call).
+    const activeCont = isActiveToolContinuation(messages, i);
+    // v1: protect by shape, any position — the fix this PR makes.
+    const protectV1 = activeCont;
+    // v2: protect ONLY the latest active continuation — position-scoped,
+    // matching the v2 directive's "strip all prior signed thinking" rule.
+    const protectV2 = activeCont && i === latestAsst;
+
+    if (protectV1 && (!v2StripSigned || protectV2)) {
+      // Either v2 isn't stripping this request, or this message is the one
+      // case v2 also protects (the latest active continuation) — leave the
+      // whole message intact, both gates agree.
       out.push(msg);
       continue;
     }
+
     const kept = msg.content.filter((b) => {
-      // v1 always-active drop predicate.
+      // v1's shape-based protection still applies here even though we fell
+      // through the whole-message shortcut above — that fallthrough can
+      // only happen when protectV1 is true and v2StripSigned is stripping a
+      // deep-history continuation, which must not touch v1's target blocks.
       if (isOmittedThinking(b)) {
+        if (protectV1) return true;
         dropped++;
         return false;
       }
       // v2-only drop predicate. Predicates are mutually exclusive on a single
       // block: omitted thinking matches v1's predicate but not v2's, and
       // signed/redacted thinking matches v2's predicate but not v1's.
-      if (v2StripSigned && isSignedThinkingForV2(b)) {
+      if (v2StripSigned && isSignedThinkingForV2(b) && !protectV2) {
         droppedV2++;
         return false;
       }

test/proxy-thinking-block-sanitize.test.mjs

@@ -853,3 +853,98 @@ 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) ---
+//
+// v1's 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 old `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.
+//
+// This is a v1-only claim — the content is OMITTED thinking (v1's target;
+// v2 never touches omitted blocks) and v2StripSigned is left at its default
+// false. Round-1 review (PR #279, 2026-07-31) established that v2 does NOT
+// get this same by-shape protection — see the paired test below.
+test("planSanitize: v1 — an answered tool-continuation stays protected after it ages out of the tail", () => {
+  const continuation = { role: "assistant", content: [omitted(), 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).messages[1];
+  const midOut = planSanitize(asMidHistory).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 under v1 — 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",
+  );
+});
+
+// v2's contract is the opposite once a continuation ages out of the tail:
+// strip its signed thinking exactly as main did before PR #279, because a
+// v2StripSigned request has already busted the cache prefix on the tools
+// change — protecting deep history there buys nothing and reintroduces the
+// stale-signature risk v2 exists to remove
+// (docs/directives/proxy-thinking-block-sanitize-v2.md:58-67,153-159).
+// Reviewer's own repro (PR #279 round-1 review, 2026-07-31): this must
+// match main's droppedV2:1 behavior, not the PR head's (buggy) droppedV2:0.
+test("planSanitize: v2 — a historical answered continuation loses its signed thinking once it ages out of the tail (not shape-protected)", () => {
+  const continuation = { role: "assistant", content: [realSigned(), toolUse("t9")] };
+  const messages = [
+    { role: "user", content: [text("q")] },
+    continuation, // answered → active continuation, but NOT the latest assistant turn
+    { role: "user", content: [toolResult("t9")] },
+    { role: "assistant", content: [text("done")] }, // later assistant turn
+  ];
+  const r = planSanitize(messages, { v2StripSigned: true });
+  assert.deepEqual(
+    r.messages[1].content,
+    [toolUse("t9")],
+    "signed thinking stripped from the now-historical continuation — matches main",
+  );
+  assert.equal(r.droppedV2, 1);
+});
+
+// The latest active continuation keeps its v2 protection — same case as the
+// existing "v2 onRequest: active-tool-continuation latest turn protected
+// even on mismatch" integration test above, pinned here at the pure-planner
+// level too so the two layers can't drift apart.
+test("planSanitize: v2 — the LATEST active continuation stays protected even with v2StripSigned", () => {
+  const continuation = { role: "assistant", content: [realSigned(), toolUse("t9")] };
+  const messages = [
+    { role: "user", content: [text("q")] },
+    { role: "assistant", content: [realSigned(), text("prior")] },
+    { role: "user", content: [text("q2")] },
+    continuation, // latest assistant turn, answered
+    { role: "user", content: [toolResult("t9")] },
+  ];
+  const r = planSanitize(messages, { v2StripSigned: true });
+  assert.deepEqual(r.messages[1].content, [text("prior")], "prior non-continuation turn stripped");
+  assert.deepEqual(
+    r.messages[3],
+    continuation,
+    "latest active continuation left byte-identical",
+  );
+  assert.equal(r.droppedV2, 1);
+});