From 423c14cf870132e09fe290866d2c4b891d662b60 Mon Sep 17 00:00:00 2001 From: ci-check Date: Thu, 29 Oct 2026 09:33:20 +0800 Subject: [PATCH 1/2] fix: run decompress range endpoints through the superseded-ref ladder (#5049 item 4) A folded message cited by its PRE-restamp number hit a generic '[decompress FAILED]' on the range path, while compress translated the same citation to the live ref of the same message (supersededRefRejection). Asymmetric UX for one mechanism, and a shifted-window hazard if the old numbers happened to fall inside the post-restamp coverage. - extract supersededRefRejection (+ interface) verbatim into src/superseded-refs.ts so both lanes share ONE resolution ladder without a stream<->decompress import cycle; stream.ts re-exports it for the existing import sites - resolveDecompressRange now translates startId/endId through the ladder before comparing against block coverage; only genuinely dead cites refuse, with the compress-side actionable wording (re-read acp_status, re-issue) - live citations and sessions without a superseded map are byte-identical to the old behavior (pinned) Part of #5049 --- src/decompress-shared.ts | 19 ++- src/stream.ts | 80 +-------- src/superseded-refs.ts | 79 +++++++++ tests/issue5049-decompress-superseded.test.ts | 153 ++++++++++++++++++ 4 files changed, 254 insertions(+), 77 deletions(-) create mode 100644 src/superseded-refs.ts create mode 100644 tests/issue5049-decompress-superseded.test.ts diff --git a/src/decompress-shared.ts b/src/decompress-shared.ts index 74a8896108..9709fd028c 100644 --- a/src/decompress-shared.ts +++ b/src/decompress-shared.ts @@ -28,6 +28,7 @@ import { safePrefix } from "./text-safe.js"; import { decompressTmpCap as knobDecompressTmpCap } from "./knobs.js"; import { toolFail, toolOk, type ProxyToolResult } from "./proxy-tool-result.js"; import { describeRestorable } from "./image-restore.js"; +import { supersededRefRejection } from "./superseded-refs.js"; /** Bounded retention for large-decompress temp files. Each decompress with * body > 10000 writes one file under tmpdir(); the reaper unlinks oldest past @@ -397,11 +398,25 @@ function resolveDecompressRange(args: Record, ctx: ProxyToolCtx if (!sb || sb.kind !== "message" || !eb || eb.kind !== "message") { return toolFail(`[decompress FAILED: startId/endId must be mNNNNN message refs (got "${startRaw}", "${endRaw}") — block ids (bN) are not valid here]`); } - if (sb.numericId > eb.numericId) return toolFail(`[decompress FAILED: startId ${startRaw} is after endId ${endRaw} — swap them]`); + // #5049 item 4: run the endpoints through the SAME resolution ladder as + // compress (live-first, then superseded translation, refuse only dead + // cites) so a pre-restamp citation restores the same messages instead of + // falling into a generic FAILED or silently picking a shifted window. + const sup = supersededRefRejection( + [{ startRef: `m${String(sb.numericId).padStart(5, "0")}`, endRef: `m${String(eb.numericId).padStart(5, "0")}` }], + ctx.session, + ); + if (sup.rejection) { + ctx.log(`[acp-decompress-range] ${block.blockId} ${startRaw}–${endRaw}: superseded refusal — ${sup.rejection}`); + return toolFail(`[decompress FAILED: ${sup.rejection}]`); + } + const sNum = refNum(sup.translatedRanges[0].startRef) ?? sb.numericId; + const eNum = refNum(sup.translatedRanges[0].endRef) ?? eb.numericId; + if (sNum > eNum) return toolFail(`[decompress FAILED: startId ${startRaw} is after endId ${endRaw} — swap them]`); const state = ctx.session.state; const cov = coveredMessages(state, block); if (!cov) return toolFail(`[decompress FAILED: ${block.blockId} has no per-message coverage recorded (older block) — use plain decompress {blockId} for the whole block]`); - const pickedSet = new Set(cov.raws.filter(({ num }) => num >= sb.numericId && num <= eb.numericId).map(({ raw }) => raw)); + const pickedSet = new Set(cov.raws.filter(({ num }) => num >= sNum && num <= eNum).map(({ raw }) => raw)); if (pickedSet.size === 0) return toolFail(`[decompress FAILED: ${block.blockId} covers no messages in ${startRaw}–${endRaw} (its coverage is ${cov.text})]`); const parts: string[] = []; let restoredFromStore = 0; diff --git a/src/stream.ts b/src/stream.ts index 502acda0af..305e6bd4e3 100644 --- a/src/stream.ts +++ b/src/stream.ts @@ -22,6 +22,7 @@ import { safePrefix, safeSuffix, scrubLoneSurrogates } from "./text-safe.js"; import { applyConfiguredCompression } from "./external-summary-compress.js"; import { NUDGE_CREDIT_ROUNDS, unpromptedGateOf } from "./unprompted-gate.js"; import { describeUncoveredRuns, shortTokenCount, uncoveredHistoryRuns } from "./uncovered.js"; +import { supersededRefRejection } from "./superseded-refs.js"; export type RewriteCtx = { core: CompressionCore; @@ -889,81 +890,10 @@ export function findVisibleStubResubmissions(ctx: RewriteCtx, ranges: ParsedRang return hits; } -/** #4316 layer 3 PR3/3, #4797 rework: resolve a compress call's refs - * against the CURRENT live map first, the superseded map second, and refuse - * only genuinely-dead cites. - * - * The original span-scan refused any range with a superseded KEY anywhere - * between its endpoints — but the restamp's vacated numbers can be handed to - * other messages as seq+1 targets in the same sweep, so in a dense space a - * superseded key is routinely ALSO the live ref of a different message - * (exactly the numbers acp_status advertises). That made the guard reject - * every advertised range and dead-loop the session: "call acp_status" → - * status lists the same live numbers → refused again — the proxy's own - * preflight rescue included (#4797). - * - * Resolution ladder, per ENDPOINT (span folding is endpoint-defined; the - * interior number scan only manufactured false hits): - * 1. live in the current map → keep as-is (collision-band artifact, the - * common case for numbers straight out of acp_status / the nudge); - * 2. superseded key (genuinely vacated) → identity-preserving translation - * oldRef → rawId → the same message's current ref — a model citing a - * number it saw BEFORE the restamp folds the SAME messages, never the - * new owner of that number; - * 3. neither → leave to the kernel's boundary-not-found handling. - * Refusal only when an endpoint is a superseded key whose translation - * dead-ends (the message lost its mapping entirely — an honest stale cite). - * - * Transitional honesty: sessions whose restamp predates kernel #5196's - * slot-retirement may still carry an ambiguity — a number restamped over a - * live occupant, so case 1 can in principle swallow a resent pre-restart - * call's intent. #4807 (#4797) traded that seconds-long replay window - * against a permanent deadlock; live-first keeps that trade. Kernels from - * #5196 onward retire every seat a mover vacates into its own seq+1, so no - * freshly stamped number ever names two messages and case 1 can never - * mis-engage on such data. */ -interface SupersededResolution { - rejection: string | null; - translatedRanges: T[]; - translated: number; -} - -export function supersededRefRejection( - ranges: T[], - session: Session, -): SupersededResolution { - const superseded = session.restampSuperseded; - const out: SupersededResolution = { rejection: null, translatedRanges: [], translated: 0 }; - if (!superseded || superseded.size === 0) { - out.translatedRanges = ranges.map((r) => ({ ...r })); - return out; - } - const byRef = session.state.messageRefs?.byRef ?? {}; - const byRaw = session.state.messageRefs?.byRaw ?? {}; - const dead: string[] = []; - const resolveEndpoint = (ref: string): string => { - if (byRef[ref] !== undefined) return ref; - const rawId = superseded.get(ref); - if (rawId === undefined) return ref; - const next = byRaw[rawId]; - if (typeof next === "string" && next !== ref && byRef[next] === rawId) { - out.translated++; - return next; - } - dead.push(ref); - return ref; - }; - out.translatedRanges = ranges.map((range) => ({ - ...range, - startRef: resolveEndpoint(range.startRef), - endRef: resolveEndpoint(range.endRef), - })); - if (dead.length === 0) return out; - const listed = dead.slice(0, 5).join(", "); - const more = dead.length > 5 ? ` (+${dead.length - 5} more)` : ""; - out.rejection = `ref(s) ${listed}${more} were re-numbered by the ledger alignment and no longer resolve to a live message. Re-read the current labels with acp_status and re-issue.`; - return out; -} +// #5049 item 4: supersededRefRejection (imported above) lives in +// ./superseded-refs.ts so the decompress range path can share it without an +// import cycle; re-exported for the existing import sites. +export { supersededRefRejection }; // #5683: out-of-bounds gate helpers. The recommendation set a compress call may // cover is judged by COVERAGE over message-ref indices: every index of a diff --git a/src/superseded-refs.ts b/src/superseded-refs.ts new file mode 100644 index 0000000000..76197ef97e --- /dev/null +++ b/src/superseded-refs.ts @@ -0,0 +1,79 @@ +import { type Session } from "./session.js"; + +/** #4316 layer 3 PR3/3, #4797 rework: resolve a tool call's refs + * against the CURRENT live map first, the superseded map second, and refuse + * only genuinely-dead cites. Shared by compress (ranges) and decompress + * (range endpoints, #5049 item 4) so both lanes translate pre-restamp + * citations identically. + * + * The original span-scan refused any range with a superseded KEY anywhere + * between its endpoints — but the restamp's vacated numbers can be handed to + * other messages as seq+1 targets in the same sweep, so in a dense space a + * superseded key is routinely ALSO the live ref of a different message + * (exactly the numbers acp_status advertises). That made the guard reject + * every advertised range and dead-loop the session: "call acp_status" → + * status lists the same live numbers → refused again — the proxy's own + * preflight rescue included (#4797). + * + * Resolution ladder, per ENDPOINT (span folding is endpoint-defined; the + * interior number scan only manufactured false hits): + * 1. live in the current map → keep as-is (collision-band artifact, the + * common case for numbers straight out of acp_status / the nudge); + * 2. superseded key (genuinely vacated) → identity-preserving translation + * oldRef → rawId → the same message's current ref — a model citing a + * number it saw BEFORE the restamp folds the SAME messages, never the + * new owner of that number; + * 3. neither → leave to the kernel's boundary-not-found handling. + * Refusal only when an endpoint is a superseded key whose translation + * dead-ends (the message lost its mapping entirely — an honest stale cite). + * + * Transitional honesty: sessions whose restamp predates kernel #5196's + * slot-retirement may still carry an ambiguity — a number restamped over a + * live occupant, so case 1 can in principle swallow a resent pre-restart + * call's intent. #4807 (#4797) traded that seconds-long replay window + * against a permanent deadlock; live-first keeps that trade. Kernels from + * #5196 onward retire every seat a mover vacates into its own seq+1, so no + * freshly stamped number ever names two messages and case 1 can never + * mis-engage on such data. */ +export interface SupersededResolution { + rejection: string | null; + translatedRanges: T[]; + translated: number; +} + +export function supersededRefRejection( + ranges: T[], + session: Session, +): SupersededResolution { + const superseded = session.restampSuperseded; + const out: SupersededResolution = { rejection: null, translatedRanges: [], translated: 0 }; + if (!superseded || superseded.size === 0) { + out.translatedRanges = ranges.map((r) => ({ ...r })); + return out; + } + const byRef = session.state.messageRefs?.byRef ?? {}; + const byRaw = session.state.messageRefs?.byRaw ?? {}; + const dead: string[] = []; + const resolveEndpoint = (ref: string): string => { + if (byRef[ref] !== undefined) return ref; + const rawId = superseded.get(ref); + if (rawId === undefined) return ref; + const next = byRaw[rawId]; + if (typeof next === "string" && next !== ref && byRef[next] === rawId) { + out.translated++; + return next; + } + dead.push(ref); + return ref; + }; + out.translatedRanges = ranges.map((range) => ({ + ...range, + startRef: resolveEndpoint(range.startRef), + endRef: resolveEndpoint(range.endRef), + })); + if (dead.length === 0) return out; + const listed = dead.slice(0, 5).join(", "); + const more = dead.length > 5 ? ` (+${dead.length - 5} more)` : ""; + out.rejection = `ref(s) ${listed}${more} were re-numbered by the ledger alignment and no longer resolve to a live message. Re-read the current labels with acp_status and re-issue.`; + return out; +} diff --git a/tests/issue5049-decompress-superseded.test.ts b/tests/issue5049-decompress-superseded.test.ts new file mode 100644 index 0000000000..8d048ada79 --- /dev/null +++ b/tests/issue5049-decompress-superseded.test.ts @@ -0,0 +1,153 @@ +// #5049 item 4: decompress range endpoints go through the SAME resolution +// ladder as compress (supersededRefRejection) — a pre-restamp citation +// translates to the live refs of the SAME messages instead of falling into a +// generic "[decompress FAILED]" or silently picking a shifted window; only a +// genuinely dead cite refuses, with the compress-side actionable wording. +import test from "node:test"; +import assert from "node:assert/strict"; +import { + createCore, + defaultConfig, + type Config, + type CoreMessage, +} from "acp-kernel"; +import { applyCompressSettings } from "../src/compress-settings.ts"; +import { storeEffectiveCcr, drainPendingRetrievals } from "../src/store.ts"; +import { getSession } from "../src/session.ts"; +import { applyRanges } from "../src/stream.ts"; +import { parseCompressInput } from "../src/compress-tool.ts"; +import { resolveDecompress } from "../src/decompress-shared.ts"; + +process.env.BILI_PERSIST = "0"; + +const pad = (n: number): string => String(n).padStart(5, "0"); + +// Same corpus discipline as tests/ccr-v2.test.ts: 20 messages so the +// m00001–m00007 fold range sits outside the protected recent zone. +function makeMsgs(): CoreMessage[] { + const msgs: CoreMessage[] = []; + for (let i = 0; i < 20; i++) { + msgs.push({ + id: `h_${i}`, + role: i % 2 === 0 ? "user" : "assistant", + contentType: "text", + text: `\x3cacp tokens="2K" type="text"\x3em${pad(i + 1)}\x3c/acp\x3e\nHistorical detail ${i}. ${"x".repeat(2000)}`, + }); + } + return msgs; +} + +type FoldResult = { + core: ReturnType; + config: Config; + session: ReturnType; + msgs: CoreMessage[]; + blockId: string; +}; + +function fold(): FoldResult { + const core = createCore(); + const config = applyCompressSettings( + defaultConfig(200_000), + 200_000, + { absorb: { enabled: true, minToolTokens: 50 }, ccr: { enabled: true, minToolTokens: 50 } }, + ) as Config; + const session = getSession(`issue5049-${Math.random().toString(36).slice(2)}`); + storeEffectiveCcr(session, { enabled: true, minToolTokens: 50 }); + const raw = makeMsgs(); + const turn = core.processTurn({ messages: raw, state: session.state, config, tokenCount: 9999, renderTags: "text-only" }); + session.state = turn.state; + const ctx = { core, config, messages: turn.messages, session, log: () => {} }; + applyRanges(parseCompressInput({ content: [{ startId: "m00001", endId: "m00007", summary: "Early history: messages 1-7 covered initial setup.", topic: "Early history" }] }), ctx); + const block = [...session.state.blocks].find((b) => b.active); + assert.ok(block, "a block was created"); + return { core, config, session, msgs: turn.messages, blockId: block.blockId }; +} + +function ctxOf(f: FoldResult) { + return { core: f.core, config: f.config, messages: f.msgs, session: f.session, log: () => {} }; +} + +/** Simulate a ledger restamp: every live ref shifts by `delta`, the vacated + * numbers land in session.restampSuperseded (oldRef → rawId), exactly the + * shape persist.ts produces on restart reconciliation. */ +function restampShift(f: FoldResult, delta: number): void { + const st = f.session.state; + const superseded = new Map(); + const byRef: Record = {}; + const byRaw: Record = {}; + // byRaw is keyed by raw id (raw → ref); the vacated KEYS of byRef are the old refs. + for (const [raw, ref] of Object.entries(st.messageRefs.byRaw)) { + const num = Number.parseInt(ref.slice(1), 10); + const next = `m${String(num + delta).padStart(5, "0")}`; + superseded.set(ref, raw); + byRef[next] = raw; + byRaw[raw] = next; + } + st.messageRefs.byRef = byRef; + st.messageRefs.byRaw = byRaw; + f.session.restampSuperseded = superseded; +} + +test("pre-restamp citation translates to the same messages instead of failing (#5049 item 4)", () => { + const f = fold(); + restampShift(f, 100); // m00001–m00007 → m00101–m00107 + // The model cites the numbers it saw BEFORE the restamp. Pre-fix this hit + // "covers no messages in m00002–m00004 (its coverage is m00101–m00107)". + const ack = resolveDecompress({ blockId: f.blockId, startId: "m00002", endId: "m00004" }, ctxOf(f)).text; + assert.match(ack, /\[decompress .* m00002\u2013m00004: restored 3 item\(s\)/, `expected translated restore, got: ${ack.slice(0, 200)}`); + const injs = drainPendingRetrievals(f.session); + assert.equal(injs.length, 1, "one ephemeral injection queued"); + const text = injs[0]!.text!; + // Same MESSAGES as the pre-restamp citation means: details 1–3 (h_1..h_3). + assert.match(text, /Historical detail 1\./); + assert.match(text, /Historical detail 2\./); + assert.match(text, /Historical detail 3\./); + assert.doesNotMatch(text, /Historical detail 0\./); + assert.doesNotMatch(text, /Historical detail 4\./); +}); + +test("live citations pass through untouched when a superseded map exists", () => { + const f = fold(); + restampShift(f, 100); + // Numbers straight out of the POST-restamp acp_status surface are live — + // the ladder must keep them as-is (case 1), not translate or refuse. + const ack = resolveDecompress({ blockId: f.blockId, startId: "m00102", endId: "m00103" }, ctxOf(f)).text; + assert.match(ack, /restored 2 item\(s\)/, `got: ${ack.slice(0, 200)}`); + const text = drainPendingRetrievals(f.session)[0]!.text!; + assert.match(text, /Historical detail 1\./); + assert.match(text, /Historical detail 2\./); + assert.doesNotMatch(text, /Historical detail 3\./); +}); + +test("a dead superseded cite refuses with the compress-side actionable wording", () => { + const f = fold(); + restampShift(f, 100); + // Kill the mapping of h_5 entirely (the message left the ledger map) while + // keeping its superseded key: old m00006 → h_5 has nowhere to translate to. + const st = f.session.state; + delete st.messageRefs.byRaw["h_5"]; + delete st.messageRefs.byRef["m00106"]; + const out = resolveDecompress({ blockId: f.blockId, startId: "m00006", endId: "m00006" }, ctxOf(f)); + assert.equal(out.outcome, "failure"); + assert.match(out.text, /\[decompress FAILED:/); + assert.match(out.text, /re-numbered by the ledger alignment and no longer resolve to a live message/); + assert.match(out.text, /acp_status/); + assert.equal(drainPendingRetrievals(f.session).length, 0, "a refused range queues nothing"); + assert.equal(f.session.stats.rangeRestores, 0); +}); + +test("order check runs on the TRANSLATED endpoints (inverted old cites still swap-refused)", () => { + const f = fold(); + restampShift(f, 100); + const out = resolveDecompress({ blockId: f.blockId, startId: "m00004", endId: "m00002" }, ctxOf(f)); + assert.match(out.text, /swap them/); + assert.equal(drainPendingRetrievals(f.session).length, 0); +}); + +test("without a superseded map the old behavior is byte-identical (out-of-coverage still 'covers no messages')", () => { + const f = fold(); + const out = resolveDecompress({ blockId: f.blockId, startId: "m00015", endId: "m00016" }, ctxOf(f)); + assert.match(out.text, /covers no messages in m00015\u2013m00016/); + assert.equal(drainPendingRetrievals(f.session).length, 0); +}); From e1d42b633ea1ee8089cd0a4d9ad970d14eb21d3f Mon Sep 17 00:00:00 2001 From: ci-check Date: Thu, 29 Oct 2026 11:25:04 +0800 Subject: [PATCH 2/2] fix: keep SupersededResolution module-local (CI unused-exports gate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The extraction made the interface exported, but every reference stays inside src/superseded-refs.ts (callers consume supersededRefRejection's result structurally) — the gate flags it INTERNAL_ONLY. Restore the original stream.ts shape (non-exported interface, exported function): behavior-neutral, one keyword. --- src/superseded-refs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/superseded-refs.ts b/src/superseded-refs.ts index 76197ef97e..9ec1d1aad3 100644 --- a/src/superseded-refs.ts +++ b/src/superseded-refs.ts @@ -35,7 +35,7 @@ import { type Session } from "./session.js"; * #5196 onward retire every seat a mover vacates into its own seq+1, so no * freshly stamped number ever names two messages and case 1 can never * mis-engage on such data. */ -export interface SupersededResolution { +interface SupersededResolution { rejection: string | null; translatedRanges: T[]; translated: number;