diff --git a/src/session.ts b/src/session.ts index b018398531..556c411439 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1716,6 +1716,18 @@ export function cacheBlockContent(session: Session, blockId: string, content: Bl session.blockContents.set(blockId, content); } +// #5984: canonical names of the two compress-failure accounting keys that +// stream.ts writes — the #2146 consecutive-failure breaker (COMPRESS_LOOP_KEY) +// and the #847 exact-spec repeat guard (FAIL_STREAK_KEY). Both live on metadata +// and are cleared on a successful compress; a native-compaction rebase must +// clear them too, because the ref space they describe is gone (refs restart at +// m00001). Defined here — not in stream.ts — because resetSessionCompression +// is the single choke point every rebase funnels through, and session.ts cannot +// import stream.ts (stream already imports session: a cycle). Exported so both +// modules share one source of truth for the key names. +export const COMPRESS_LOOP_KEY = "compressFailStreak"; +export const FAIL_STREAK_KEY = "compressFailKeys"; + export function resetSessionCompression(session: Session): void { session.state = createInitialState(); session.blockContents.clear(); @@ -1773,6 +1785,14 @@ export function resetSessionCompression(session: Session): void { delete session.metadata.postRebuildAnchor; session.stats.contextTokens = 0; delete session.stats.contextTokensSource; + // #5984: the rebase restarts the ref space, so the compress-failure + // accounting written against the pre-rebase generation is void — drop both + // keys here (they were the two failure-accounting fields this reset + // historically missed; only a successful compress cleared them, letting a + // pre-rebase streak carry across the boundary and arm mid-loop on the new + // refs). Covered by the trailing markDirty below. + delete session.metadata[COMPRESS_LOOP_KEY]; + delete session.metadata[FAIL_STREAK_KEY]; session.metadata.nativeCompactionAt = Date.now(); markDirty(session); } diff --git a/src/stream.ts b/src/stream.ts index 4f24c3d639..236322965a 100644 --- a/src/stream.ts +++ b/src/stream.ts @@ -3,7 +3,7 @@ import { collectBlockContent, countMessageTokens, defaultCountTokens, formatRang import { loadConfigFile, resolveMessageTimestamps } from "./config.js"; import { excludeDeadRefRanges, handleAcpStatus, liveCompressionRanges } from "./acp-status.js"; import { handleAcpCache, prefixTokensBeforeRef, recordCacheFoldsFromBlocks } from "./cache-ledger.js"; -import { type Session, cacheBlockContent, markDirty, residentPressure, scrubTurnSeparatorIds, statusInputBaseline, truncatedUpstreamOf, withoutTurnSeparators } from "./session.js"; +import { COMPRESS_LOOP_KEY, FAIL_STREAK_KEY, type Session, cacheBlockContent, markDirty, residentPressure, scrubTurnSeparatorIds, statusInputBaseline, truncatedUpstreamOf, withoutTurnSeparators } from "./session.js"; import { COMPRESS_TOOL_NAME, parseCompressInput, ABSORB_TOOL_NAME, type ParsedRange } from "./compress-tool.js"; import { effectiveAbsorbConfig, executeAbsorb, isProxyToolFor } from "./absorb.js"; import { recordCompressCommit } from "./context-analysis.js"; @@ -178,8 +178,9 @@ function reversedRanges(ranges: ParsedRange[]): ParsedRange[] { // call 7x over ~13 min while usage climbed 76%→89%). Track recent failed // specs per session and escalate on repeat instead of echoing the plain gate // error again. Stored on metadata (persisted) so the streak survives LRU -// eviction/reload; stale keys after a ref reset are harmless (no match). -const FAIL_STREAK_KEY = "compressFailKeys"; +// eviction/reload; stale keys after a ref reset are harmless (no match). The +// key NAME is defined in session.ts (#5984) so the rebase reset can clear it +// too — imported above; only the cap is stream-local. const FAIL_STREAK_CAP = 5; function normalizedSpecKey(ranges: ParsedRange[]): string { return ranges @@ -227,11 +228,30 @@ function compressFailOccurrencesOf(session: Session, key: string): number { // design (config-surface discipline, cf. DEGENERATE_FOLD_COVERAGE). Disarm is // success or 10 min of quiet — refs keep advancing DURING the loop (each // failed round appends messages), so a frontier-based decay would disarm -// mid-loop and be defeated the same way. -const COMPRESS_LOOP_KEY = "compressFailStreak"; +// mid-loop and be defeated the same way. The key NAME is defined in session.ts +// (#5984) so the rebase reset can clear it too — imported above; threshold and +// decay are stream-local. const COMPRESS_LOOP_THRESHOLD = 3; const COMPRESS_LOOP_DECAY_MS = 10 * 60 * 1000; +// #5984: how long after a native-compaction rebase a stale-ref compress failure +// is attributed to the proxy (its own rebase killed every pre-rebase ref the +// model still carries in its persisted history) rather than to the model. While +// the session sits inside this window, a stale-ref failure does NOT advance the +// model-failure breaker — otherwise the inevitable re-anchor round-trips arm a +// hard-stop that then scrubs the very acp_status recovery that would let the +// model out. Hardcoded by design like COMPRESS_LOOP_* (config-surface +// discipline); generous enough for a slow model working through stale history, +// bounded so a genuine post-rebase loop still arms once the grace elapses. +const REBASE_STALE_GRACE_MS = 30 * 60 * 1000; +function withinRebaseGrace(session: Session): boolean { + // nativeCompactionAt is set ONLY by resetSessionCompression (which runs in + // reconcileNativeCompactionBoundary), so a number here is a reliable + // "recently rebased" marker — no other path writes it. + const at = session.metadata.nativeCompactionAt; + return typeof at === "number" && Date.now() - at <= REBASE_STALE_GRACE_MS; +} + type CompressLoopStreak = { n: number; lastAt: number; cause?: string }; function readCompressLoopStreak(session: Session): CompressLoopStreak | undefined { @@ -303,20 +323,38 @@ const RETURN_TO_TASK_SUFFIX = " This failure only concerns the compress call. Re * it: successful calls always go through, and one success clears the streak * (clearCompressLoopStreak). The pause paragraph branches on the failure * cause (#2432): substrate-destruction gets the single-recovery-step - * wording, everything else keeps the verbatim loop-noise paragraph. */ -function noteCompressLoopFailure(ctx: RewriteCtx, specLabel: string, errs?: string, forcedCause?: string): string { + * wording, everything else keeps the verbatim loop-noise paragraph. + * #5984: a stale-ref failure inside the post-rebase grace window is + * proxy-caused (bili itself rebased, killing the model's retained refs) and is + * NOT counted against the model breaker — it returns "" so the non-armed + * recovery hints (rebaseNote + currentRefsSnapshot + acp_status guidance) stay + * visible until one success clears everything. Exported for direct testing. */ +export function noteCompressLoopFailure(ctx: RewriteCtx, specLabel: string, errs?: string, forcedCause?: string): string { const now = Date.now(); + // #2432: derive the cause FIRST so the #5984 rebase-grace exemption below + // can branch on it. A cause only ever moves forward: an unknown-cause + // failure keeps the last known verdict rather than erasing it. #3126: + // host-side gates that know their own verdict before the kernel runs pass + // it as forcedCause — deriving it from kernel error text would misattribute + // a gate refusal to content. + const freshCause = forcedCause ?? (errs !== undefined ? compressFailureCause(errs, ctx.session) : ""); + // #5984: a native-compaction rebase invalidates every pre-rebase ref the + // model still carries in its own persisted history, so the staleness is the + // proxy's own doing, not the model's. While the session sits in the + // post-rebase grace window, do NOT advance the model-failure breaker for it: + // returning "" leaves the non-armed hint cluster (rebaseNote + + // currentRefsSnapshot + the kernel's acp_status guidance) visible until one + // success clears everything. acp_status independently surfaces the rebase + // via nativeCompactionBoundary, so it does not depend on this streak to know + // the refs changed. Non-stale causes (parse / covered / content-changed) are + // unaffected and still count even right after a rebase. + if (freshCause.startsWith("stale-ref") && withinRebaseGrace(ctx.session)) { + ctx.log(`[info: compress-loop] stale-ref failure within ${REBASE_STALE_GRACE_MS / 60000} min of a native-compaction rebase — counted as proxy-caused, model breaker not advanced`); + return ""; + } let streak = readCompressLoopStreak(ctx.session); if (streak && now - streak.lastAt > COMPRESS_LOOP_DECAY_MS) streak = undefined; const n = (streak?.n ?? 0) + 1; - // #2432: persist the failure cause on the streak so surfaces OUTSIDE the - // receipt (acp_status) can react to a substrate/stale-ref verdict at n=1 - // instead of waiting for the third failure to arm the breaker. A cause - // only ever moves forward: an unknown-cause failure keeps the last known - // verdict rather than erasing it. #3126: host-side gates that know their - // own verdict before the kernel runs pass it as forcedCause — deriving it - // from kernel error text would misattribute a gate refusal to content. - const freshCause = forcedCause ?? (errs !== undefined ? compressFailureCause(errs, ctx.session) : ""); const cause = freshCause !== "" ? freshCause : streak?.cause; writeCompressLoopStreak(ctx.session, { n, lastAt: now, ...(cause !== undefined ? { cause } : {}) }); if (n < COMPRESS_LOOP_THRESHOLD) return ""; diff --git a/tests/issue5984-rebase-breaker.test.ts b/tests/issue5984-rebase-breaker.test.ts new file mode 100644 index 0000000000..d7331cbbc3 --- /dev/null +++ b/tests/issue5984-rebase-breaker.test.ts @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import { randomUUID } from "node:crypto"; +import { test } from "node:test"; +import type { Session } from "../src/session.ts"; +import { SessionStore, _setStoreForTest } from "../src/persist.ts"; +import { reconcileNativeCompactionBoundary } from "../src/session.js"; +import { noteCompressLoopFailure, type RewriteCtx } from "../src/stream.js"; + +_setStoreForTest(new SessionStore({ enabled: false })); + +// #5984: after a host-native compaction rebase the ref space restarts, but the +// compress-failure accounting written against the pre-rebase generation +// survived — so (a) a pre-rebase streak carried across the boundary could arm +// mid-loop on the new refs, and (b) the stale-ref failures that are INEVITABLE +// while the model still carries its old refs in its own persisted history +// armed a hard-stop that then scrubbed the very acp_status recovery the model +// needed to get out. Two fixes under test here: +// A — a rebase clears the #2146 consecutive-failure streak AND the #847 +// exact-spec repeat guard (the ref space they describe is gone). +// B — a stale-ref failure inside the post-rebase grace window is counted as +// proxy-caused (bili itself rebased) and does NOT advance the model +// breaker, so the non-armed recovery hints stay visible until a success. + +function makeSession(extraMetadata: Record = {}): Session { + return { + id: `issue5984-${randomUUID()}`, + meta: {}, + stats: {}, + metadata: { ...extraMetadata }, + state: { messageRefs: { byRaw: {}, byRef: {} }, blocks: [] }, + createdAt: Date.now(), + lastSeen: Date.now(), + blockContents: new Map(), + inFlight: 0, + persisted: false, + pendingRetrievals: [], + } as unknown as Session; +} + +function fakeCtx(session: Session): RewriteCtx { + return { session, log: () => {} } as unknown as RewriteCtx; +} + +const STALE_ERRS = "m00045–m00089: every ref is unknown to this session"; + +test("#5984 A: a native-compaction rebase clears the #2146 streak and the #847 guard", () => { + const session = makeSession({ + nativeCompactionBoundary: { at: 1, pendingRebase: true }, + compressFailStreak: { n: 2, lastAt: Date.now() }, + compressFailKeys: ["m00001..m00002"], + }); + // a live pre-rebase ref so the floor pin has something to capture + session.state.messageRefs.byRaw["rawA"] = "m00010"; + session.state.messageRefs.byRef["m00010"] = "rawA"; + + const rebased = reconcileNativeCompactionBoundary(session); + assert.equal(rebased, true); + assert.equal(session.metadata["compressFailStreak"], undefined, "#2146 streak must not carry across the generation break"); + assert.equal(session.metadata["compressFailKeys"], undefined, "#847 exact-spec guard must not carry across the generation break"); +}); + +test("#5984 A (guard): no pending boundary → no rebase → accounting untouched", () => { + const session = makeSession({ compressFailStreak: { n: 2, lastAt: Date.now() } }); + const rebased = reconcileNativeCompactionBoundary(session); + assert.equal(rebased, false, "no pending boundary means no rebase happened"); + assert.notEqual(session.metadata["compressFailStreak"], undefined, "no rebase → the streak must be left alone"); +}); + +test("#5984 B: stale-ref failures within the post-rebase grace never arm the breaker", () => { + const session = makeSession({ nativeCompactionAt: Date.now() }); + const ctx = fakeCtx(session); + for (let i = 1; i <= 5; i++) { + const guard = noteCompressLoopFailure(ctx, "m00045..m00089", STALE_ERRS); + assert.equal(guard, "", `failure ${i}: no pause paragraph — the non-armed recovery hints must stay visible`); + } + assert.equal(session.metadata["compressFailStreak"], undefined, "proxy-caused staleness must not advance the model-failure streak"); +}); + +test("#5984 B (guard): the same stale-ref storm WITHOUT a recent rebase still arms at 3", () => { + const session = makeSession({}); // never rebased + const ctx = fakeCtx(session); + const f1 = noteCompressLoopFailure(ctx, "a", STALE_ERRS); + const f2 = noteCompressLoopFailure(ctx, "b", STALE_ERRS); + const f3 = noteCompressLoopFailure(ctx, "c", STALE_ERRS); + assert.equal(f1, ""); + assert.equal(f2, ""); + assert.match(f3, /CIRCUIT BREAKER/, "no rebase → normal model-failure accounting, arms at 3"); +}); + +test("#5984 B (guard): a rebase older than the grace window stops the exemption", () => { + const session = makeSession({ nativeCompactionAt: Date.now() - 31 * 60 * 1000 }); + const ctx = fakeCtx(session); + noteCompressLoopFailure(ctx, "a", STALE_ERRS); + noteCompressLoopFailure(ctx, "b", STALE_ERRS); + const f3 = noteCompressLoopFailure(ctx, "c", STALE_ERRS); + assert.match(f3, /CIRCUIT BREAKER/, "grace elapsed → stale-ref counts again, arms at 3"); +}); + +test("#5984 B (guard): non-stale-ref failures are NOT exempted post-rebase", () => { + const session = makeSession({ nativeCompactionAt: Date.now() }); + const ctx = fakeCtx(session); + // parse-failure shape: errs === undefined → freshCause "" → not exempted + noteCompressLoopFailure(ctx, "unparseable"); + noteCompressLoopFailure(ctx, "unparseable"); + const f3 = noteCompressLoopFailure(ctx, "unparseable"); + assert.match(f3, /CIRCUIT BREAKER/, "genuine model-format errors still arm even right after a rebase"); +});