From 9b7ef5a4714997c32504dd72ed210919ea7562f2 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 22:08:26 -0700 Subject: [PATCH 1/2] fix(compaction): notify and stop when compaction fails A verify abort or summarizer keep-prior, including an AbortError with no lifecycle signal, keeps the prior turns and reports the failure instead of throwing or starting another inference after /handoff. --- src/agent/compaction.test.ts | 15 ++ src/agent/compaction.ts | 9 + src/agent/director-compaction.test.ts | 30 +++ src/agent/director.ts | 10 + src/exec/runner.ts | 3 + src/session/runtime-assembly.test.ts | 271 +++++++++++++++++--------- src/session/runtime-assembly.ts | 63 +++++- src/session/summarizer.ts | 8 +- src/tui/runner/session.ts | 9 +- 9 files changed, 309 insertions(+), 109 deletions(-) diff --git a/src/agent/compaction.test.ts b/src/agent/compaction.test.ts index 9d40fb9cb..92d27d3b8 100644 --- a/src/agent/compaction.test.ts +++ b/src/agent/compaction.test.ts @@ -1202,6 +1202,21 @@ describe("handoff arming (/handoff)", () => { ).toBeNull(); }); + test("a failed handoff compact does not schedule the post-compact infer", () => { + const governor = createCompactionGovernor(undefined); + governor.syncFromTurns(tenTurns); + expect(governor.requestHandoff("now do the UI audit")).toBe("armed"); + const actions = governor.interceptIdleContinuation( + pivot("now do the UI audit"), + capabilities, + ); + expect(actions?.some((a) => a.type === "compact")).toBe(true); + expect(governor.hasOutstandingContinuation()).toBe(true); + governor.noteKeptPrior(); + expect(governor.hasOutstandingContinuation()).toBe(false); + expect(governor.resumeAfterCompact(emptyMessage())).toBeNull(); + }); + test("instructions stay sticky for the summary after the fold fires", () => { const governor = createCompactionGovernor(undefined); governor.syncFromTurns(tenTurns); diff --git a/src/agent/compaction.ts b/src/agent/compaction.ts index 8f6cbd470..14867d73e 100644 --- a/src/agent/compaction.ts +++ b/src/agent/compaction.ts @@ -414,6 +414,14 @@ export function createCompactionGovernor( ]; } + // A keep-prior compact (verify abort, summarizer failure) must not resume + // into inference: the continuation is already queued, and inferring on the + // full transcript is what 401/429s the handoff. + function noteKeptPrior(): void { + postCompactInfer = false; + postCompactMeter = false; + } + // After compact, a content-less continuation re-enters decide. "infer" // resumes the interrupted loop; "meter" adopts the shrunk turns for the // Ctx display and stays idle (idle empty compact has nothing to answer). @@ -559,6 +567,7 @@ export function createCompactionGovernor( interceptIdleContinuation, interceptOverflow, resumeAfterCompact, + noteKeptPrior, hasOutstandingContinuation, }; } diff --git a/src/agent/director-compaction.test.ts b/src/agent/director-compaction.test.ts index 4a1434196..872578f2d 100644 --- a/src/agent/director-compaction.test.ts +++ b/src/agent/director-compaction.test.ts @@ -211,6 +211,36 @@ test("compaction is self-regulating: a cycle back under threshold does not re-co expect(hasContinuationEmit(arr)).toBe(false); }); +test("a failed handoff compact does not schedule the post-compact infer", async () => { + const director = makeChatDirector(); + director.restoreCacheWrite({ + at: 0, + source, + turns: manyTurnsState.turns, + }); + expect(director.requestHandoff("now do the UI audit")).toBe("armed"); + const armed = await director.decide( + { + type: "message.received", + message: { content: "now do the UI audit" }, + } as ReactorInboundEvent, + manyTurnsState, + makeCapabilities(), + ); + const armedArr = Array.isArray(armed) ? armed : [armed]; + expect(armedArr.some((a) => a.type === "compact")).toBe(true); + expect(hasContinuationEmit(armedArr)).toBe(true); + director.noteCompactionKeptPrior(); + const resume = await director.decide( + emptyMessageReceived(), + manyTurnsState, + makeCapabilities(), + ); + const resumeArr = Array.isArray(resume) ? resume : [resume]; + expect(resumeArr.some((a) => a.type === "infer")).toBe(false); + expect(resumeArr.some((a) => a.type === "compact")).toBe(false); +}); + test("a fresh director answers an unsolicited empty continuation without inferring", async () => { const director = makeChatDirector(); diff --git a/src/agent/director.ts b/src/agent/director.ts index 6943c4172..42db026bd 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -762,6 +762,11 @@ class ChatDirectorImpl extends DefaultDirector { return this.compaction.requestHandoff(instructions); } + /** A compact that kept prior context: do not infer on the queued continuation. */ + noteCompactionKeptPrior(): void { + this.compaction.noteKeptPrior(); + } + // Drop a handoff arming whose pivot never delivered, so the next operator // message does not fold unexpectedly. cancelManualCompact(): void { @@ -888,6 +893,9 @@ class ChatDirectorImpl extends DefaultDirector { capabilities: ReactorCapabilities, ): Promise { const afterCompact = this.compaction.resumeAfterCompact(event); + // noteKeptPrior clears the resume flags during apply, before this + // continuation is decided. A keep-prior handoff must wait, not infer + // on the unchanged transcript. if (afterCompact !== null) { // Compacted history is the live occupancy until the next // inference.done; paint from the estimate meanwhile. @@ -1358,4 +1366,6 @@ export interface ChatDirector extends ReactorDirector { requestHandoff(instructions: string): HandoffArming; /** Drop a handoff arming whose pivot never delivered. */ cancelManualCompact(): void; + /** Compact kept the prior transcript; the queued continuation must not infer. */ + noteCompactionKeptPrior(): void; } diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 0e41744c7..2bcee171a 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -944,6 +944,9 @@ export async function runExec(config: Config): Promise { onFailure: (text) => { stderr.write(`${text}\n`); }, + onKeptPrior: () => { + directorHolder.instance?.noteCompactionKeptPrior(); + }, onFolded: () => { // Fold restarts the cached prefix: drop idle execute-promoted // schemas and persist, so a crash resume cannot re-flush them. diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index 11f456b81..90d690823 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -667,112 +667,189 @@ describe("createSessionPruningCompactor stub fallback", () => { expect(notices[0]).toContain("model unreachable"); }); - test("verify abort after a failed summary keeps prior context and fires no stub notice", async () => { + test.each([ + ["model unreachable", "verify"], + ["401 Unauthorized", "auth"], + ["429 quota exceeded", "quota"], + ])( + "verify abort after summarizer failure (%s) keeps prior context and notifies", + async (errorMessage, reason) => { + const notices: string[] = []; + const kept: number[] = []; + const folds: { stub: boolean }[] = []; + const summarize = createModelSummarizer({ + getSource: () => + ({ + id: "test", + provider: "openai", + model: "test-model", + baseURL: "http://localhost:1", + credentialId: "test", + }) as never, + complete: async () => { + throw new Error(errorMessage); + }, + }); + const now = Date.now(); + const turns = [ + { + role: "user" as const, + content: [ + { + type: "text" as const, + text: "Migrate the auth module to opaque tokens", + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { + type: "tool_call" as const, + id: "c1", + name: "run_shell", + arguments: { command: "bun test auth" }, + }, + ], + timestamp: now, + }, + { + role: "user" as const, + content: [ + { + type: "tool_result" as const, + callId: "c1", + isError: true, + content: [ + { type: "text" as const, text: "token refresh assertion failed" }, + ], + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { type: "text" as const, text: "Working through the failure" }, + ], + timestamp: now, + }, + { + role: "user" as const, + content: [ + { + type: "text" as const, + text: "Confirm there are no errors remaining in auth", + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { type: "text" as const, text: "Continuing the auth work now" }, + ], + timestamp: now, + }, + { + role: "user" as const, + content: [ + { + type: "text" as const, + text: "There are no errors remaining in the suite", + }, + ], + timestamp: now, + }, + { + role: "assistant" as const, + content: [ + { type: "text" as const, text: "I will keep going from here" }, + ], + timestamp: now, + }, + ]; + const result = await createSessionPruningCompactor({ + summarize, + onFolded: (info) => folds.push(info), + onFailure: (text) => notices.push(text), + onKeptPrior: () => { + kept.push(1); + }, + compactionShape: { tailBudgetTokens: 1 }, + }).apply(turns as never, { state: {} as never, trigger: "test" }); + expect(result.output).toBe(turns); + expect(result.record.reason).toBe("verify failed — keeping prior context"); + expect(result.record.decisions.summarizedTurnCount).toBeUndefined(); + expect(result.record.decisions.verifyAborted).toBe(1); + expect(folds).toEqual([]); + expect(kept).toEqual([1]); + expect(notices).toHaveLength(1); + expect(notices[0]?.toLowerCase()).toContain("fail"); + expect(notices[0]?.toLowerCase()).toContain("verify"); + expect(notices[0]?.toLowerCase()).toContain(reason); + expect(notices[0]).not.toContain("statistics-only stub"); + expect(notices[0]).not.toContain("Compaction interrupted"); + }, + ); + + test("AbortError without a lifecycle signal keeps prior context and notifies", async () => { const notices: string[] = []; - const folds: { stub: boolean }[] = []; - const summarize = createModelSummarizer({ - getSource: () => - ({ - id: "test", - provider: "openai", - model: "test-model", - baseURL: "http://localhost:1", - credentialId: "test", - }) as never, - complete: async () => { - throw new Error("model unreachable"); - }, - }); + const kept: number[] = []; const now = Date.now(); - const turns = [ - { - role: "user" as const, - content: [ - { - type: "text" as const, - text: "Migrate the auth module to opaque tokens", - }, - ], - timestamp: now, - }, - { - role: "assistant" as const, - content: [ - { - type: "tool_call" as const, - id: "c1", - name: "run_shell", - arguments: { command: "bun test auth" }, - }, - ], - timestamp: now, - }, - { - role: "user" as const, - content: [ - { - type: "tool_result" as const, - callId: "c1", - isError: true, - content: [ - { type: "text" as const, text: "token refresh assertion failed" }, - ], - }, - ], - timestamp: now, - }, - { - role: "assistant" as const, - content: [ - { type: "text" as const, text: "Working through the failure" }, - ], - timestamp: now, - }, - { - role: "user" as const, - content: [ - { - type: "text" as const, - text: "Confirm there are no errors remaining in auth", - }, - ], - timestamp: now, - }, - { - role: "assistant" as const, - content: [ - { type: "text" as const, text: "Continuing the auth work now" }, - ], - timestamp: now, - }, - { - role: "user" as const, - content: [ - { - type: "text" as const, - text: "There are no errors remaining in the suite", - }, - ], - timestamp: now, - }, - { - role: "assistant" as const, - content: [ - { type: "text" as const, text: "I will keep going from here" }, - ], - timestamp: now, - }, - ]; + const turns = Array.from({ length: 8 }, (_, i) => ({ + role: (i % 2 === 0 ? "user" : "assistant") as "user" | "assistant", + content: [{ type: "text" as const, text: `t${i}` }], + timestamp: now, + })); + const summarize = async () => { + throw new DOMException("The operation was aborted", "AbortError"); + }; const result = await createSessionPruningCompactor({ summarize, - onFolded: (info) => folds.push(info), onFailure: (text) => notices.push(text), + onKeptPrior: () => { + kept.push(1); + }, compactionShape: { tailBudgetTokens: 1 }, }).apply(turns as never, { state: {} as never, trigger: "test" }); expect(result.output).toBe(turns); - expect(result.record.reason).toBe("verify failed — keeping prior context"); + expect(result.record.reason).toBe("summarize failed"); expect(result.record.decisions.summarizedTurnCount).toBeUndefined(); - expect(folds).toEqual([]); + expect(kept).toEqual([1]); + expect(notices).toEqual([ + "Compaction failed — keeping prior context (failed).", + ]); + expect(notices[0]).not.toContain("Compaction interrupted"); + expect(notices[0]).not.toContain("statistics-only stub"); + }); + + test("a signaled lifecycle abort does not emit the keep-prior failure notice", async () => { + const notices: string[] = []; + const kept: number[] = []; + const now = Date.now(); + const turns = Array.from({ length: 8 }, (_, i) => ({ + role: (i % 2 === 0 ? "user" : "assistant") as "user" | "assistant", + content: [{ type: "text" as const, text: `t${i}` }], + timestamp: now, + })); + const controller = new AbortController(); + controller.abort(); + const summarize = async () => { + throw new DOMException("The operation was aborted", "AbortError"); + }; + const result = await createSessionPruningCompactor({ + summarize, + onFailure: (text) => notices.push(text), + onKeptPrior: () => { + kept.push(1); + }, + getSignal: () => controller.signal, + compactionShape: { tailBudgetTokens: 1 }, + }).apply(turns as never, { state: {} as never, trigger: "test" }); + expect(result.output).toBe(turns); + expect(kept).toEqual([]); expect(notices).toEqual([]); }); diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index fceab617b..1486a57ad 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -59,6 +59,7 @@ import { import { classifySummarizerFailure, summarizerStubFallbackNotice, + type SummarizerFailureClass, type SummaryContext, } from "./summarizer.js"; import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; @@ -443,10 +444,16 @@ export interface SessionPruningCompactorArgs { stub: boolean; }) => void; /** - * Operator-visible notice for a statistics-only stub that actually replaced - * turns. Verify abort and completeness-gate discard do not fire this. + * Operator-visible notice. A committed stub says the fold used a stub. + * Verify abort says compaction failed and kept prior context. */ onFailure?: (text: string) => void; + /** + * Keep-prior compact (verify abort, or summarize failed with no fold). + * Clears the governor's post-compact infer so /handoff does not re-infer + * on the full transcript. + */ + onKeptPrior?: () => void; /** * True when the lifecycle has discarded (or will discard) the in-flight * compact — e.g. bound to the lifecycle's signal. @@ -473,11 +480,20 @@ export interface SessionPruningCompactorArgs { compactionShape?: Partial; } -function stubFallbackNoticeFromError(error: unknown): string | undefined { - const err = error instanceof Error ? error : new Error(String(error)); - const kind = classifySummarizerFailure(err); - if (kind === "aborted") return undefined; - return summarizerStubFallbackNotice(kind, err); +function compactionFailedKeepNotice( + kind: SummarizerFailureClass | undefined, +): string { + const reason = + kind === "auth" || kind === "quota" ? `${kind}, verify` : "verify"; + return `Compaction failed — keeping prior context (${reason}).`; +} + +/** Keep-prior after summarize itself failed (not a verify abort). */ +function summarizerKeepPriorFailureNotice( + kind: SummarizerFailureClass | undefined, +): string { + const reason = kind === "auth" || kind === "quota" ? kind : "failed"; + return `Compaction failed — keeping prior context (${reason}).`; } function stubFallbackNoticeFromRecord( @@ -494,6 +510,7 @@ export function createSessionPruningCompactor( args: SessionPruningCompactorArgs, ): Compactor { let pendingStubNotice: string | undefined; + let pendingFailureKind: SummarizerFailureClass | undefined; const innerSummarize = args.summarize; const summarize = innerSummarize === undefined @@ -502,7 +519,14 @@ export function createSessionPruningCompactor( try { return await innerSummarize(turns, ctx); } catch (error) { - pendingStubNotice = stubFallbackNoticeFromError(error); + const err = + error instanceof Error ? error : new Error(String(error)); + const kind = classifySummarizerFailure(err); + pendingFailureKind = kind; + pendingStubNotice = + kind === "aborted" + ? undefined + : summarizerStubFallbackNotice(kind, err); throw error; } }; @@ -528,6 +552,7 @@ export function createSessionPruningCompactor( ...compactor, async apply(turns, ctx) { pendingStubNotice = undefined; + pendingFailureKind = undefined; const turnsBefore = turns.length; const startedAt = Date.now(); const abortSignal = args.getSignal?.(); @@ -541,6 +566,28 @@ export function createSessionPruningCompactor( if (isAborted() || result.record.reason === COMPACTION_ABORTED_REASON) { return result; } + // Verify abort keeps the prior turns and does not set + // summarizedTurnCount, so it must notify here. A statistics stub is + // not committed. Lifecycle abort is handled above and stays on the + // interrupted notice. + if (result.record.decisions.verifyAborted === 1) { + args.onKeptPrior?.(); + args.onFailure?.(compactionFailedKeepNotice(pendingFailureKind)); + return result; + } + if ( + result.record.decisions.summarizeFailed === 1 && + result.record.decisions.summarizedTurnCount === undefined + ) { + // Reached when classifySummarizerFailure is "aborted" with no + // lifecycle signal (exec never passes getSignal). Still keep prior + // context, but the operator must see the failure — not silence. + // A signaled lifecycle abort returned above and stays on the + // interrupted notice only. + args.onKeptPrior?.(); + args.onFailure?.(summarizerKeepPriorFailureNotice(pendingFailureKind)); + return result; + } // summarizedTurnCount is set only when turns were folded; the no-op // branch must not report compaction (it would skew duration and // turn-count averages). diff --git a/src/session/summarizer.ts b/src/session/summarizer.ts index 635a23fbf..f50dae450 100644 --- a/src/session/summarizer.ts +++ b/src/session/summarizer.ts @@ -349,6 +349,7 @@ function defaultComplete(deps: Dependencies, timeoutMs: number): CompletionFn { // exists to stop. export type SummarizerFailureClass = | "auth" + | "quota" | "provider" | "timeout" | "aborted" @@ -382,6 +383,8 @@ export function classifySummarizerFailure( cause.statusCode === 403 ) return "auth"; + if (cause.category === "quota_exhausted" || cause.statusCode === 429) + return "quota"; if ( (cause.statusCode !== undefined && cause.statusCode >= 500 && @@ -396,6 +399,7 @@ export function classifySummarizerFailure( if (error instanceof Error && error.name === "AbortError") return "aborted"; if (/\b(?:timeout|timed out)\b/i.test(message)) return "timeout"; if (/\b(?:401|403)\b|\bunauthorized\b/i.test(message)) return "auth"; + if (/\b429\b|\bquota\b/i.test(message)) return "quota"; if (/\b5\d\d\b/.test(message) || PROVIDER_INTERNAL_ERROR.test(message)) return "provider"; return "failed"; @@ -403,8 +407,8 @@ export function classifySummarizerFailure( // One-line operator notice for a stub fold that actually committed: the // provider's first error line when short enough to be useful, else the -// failure class. Callers must not fire this until the fold lands — verify -// abort keeps prior context, so claiming a stub was used would lie. +// failure class. Callers must not fire this on verify abort — that path +// keeps prior context, so claiming a stub was used would lie. export function summarizerStubFallbackNotice( failureClass: SummarizerFailureClass, error: Error, diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index bfc012768..18c3f8fc8 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -726,9 +726,14 @@ export async function assembleTUISession( // that never landed. Capture the signal at apply start: onBuilt // reset() replaces the live controller. getSignal: () => compactionLifecycle.getSignal(), - // Stub notice waits until the fold commits (verify abort and - // completeness-gate discard stay silent). + // A committed stub notices after the fold lands. Verify abort + // notices that compaction failed and kept prior context. + // Completeness-gate discard stays silent. Lifecycle abort uses + // the interrupted notice, not this callback. onFailure: (text) => state.systemNotice?.(text), + onKeptPrior: () => { + directorHolder.instance?.noteCompactionKeptPrior(); + }, // Main-session folds only — exec runner and subagents stay silent. onFolded: (info) => { // Fold restarts the cached prefix — drop idle execute-promoted From 8c0f8d66aa4ce5202b0c50714993908f666c97db Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 22:55:44 -0700 Subject: [PATCH 2/2] style(session): format runtime assembly tests --- src/session/runtime-assembly.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index 90d690823..2f5584fdc 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -722,7 +722,10 @@ describe("createSessionPruningCompactor stub fallback", () => { callId: "c1", isError: true, content: [ - { type: "text" as const, text: "token refresh assertion failed" }, + { + type: "text" as const, + text: "token refresh assertion failed", + }, ], }, ], @@ -780,7 +783,9 @@ describe("createSessionPruningCompactor stub fallback", () => { compactionShape: { tailBudgetTokens: 1 }, }).apply(turns as never, { state: {} as never, trigger: "test" }); expect(result.output).toBe(turns); - expect(result.record.reason).toBe("verify failed — keeping prior context"); + expect(result.record.reason).toBe( + "verify failed — keeping prior context", + ); expect(result.record.decisions.summarizedTurnCount).toBeUndefined(); expect(result.record.decisions.verifyAborted).toBe(1); expect(folds).toEqual([]);