From 747a58db94a254416fbad63647b1ca83a71c5bd6 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 16 Sep 2026 12:13:49 +0900 Subject: [PATCH 1/4] test(compaction): pin recovery from a provider-killed summary stream A summary stream terminated by a provider or credential failure classifies as undefined today, so required compaction rethrows the marker-bearing error, the session cannot retry or fall back, and the next prompt repeats it forever. Final result() settlement also sits outside the watchdog, and nothing bounds one compaction's total wall clock independently of its input size. Refs #1741 --- ...rization-provider-failure-recovery.test.ts | 309 ++++++++++++++++++ .../summarization-total-budget.test.ts | 55 ++++ 2 files changed, 364 insertions(+) create mode 100644 packages/coding-agent/test/compaction/summarization-provider-failure-recovery.test.ts create mode 100644 packages/coding-agent/test/compaction/summarization-total-budget.test.ts diff --git a/packages/coding-agent/test/compaction/summarization-provider-failure-recovery.test.ts b/packages/coding-agent/test/compaction/summarization-provider-failure-recovery.test.ts new file mode 100644 index 000000000..4decbcc19 --- /dev/null +++ b/packages/coding-agent/test/compaction/summarization-provider-failure-recovery.test.ts @@ -0,0 +1,309 @@ +// Issue #1741: a summarization stream killed by a provider or credential failure +// must never leave the session uncontinuable. The reporter's session repeated +// `senpi:no-turn-retry:Codex error: …` on every prompt because the terminal +// provider error was not a recognized deterministic-fallback class, so required +// compaction rethrew it, the marker suppressed session retry and model fallback, +// and the context stayed above the threshold forever. +import { + type AssistantMessage, + type AssistantMessageEvent, + createAssistantMessageEventStream, + fauxAssistantMessage, + type Model, +} from "@earendil-works/pi-ai"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { completeSummarization } from "../../src/core/compaction/compaction.ts"; +import { prepareCompaction } from "../../src/core/compaction/index.ts"; +import { + consumeStreamWithIdleTimeout, + DEFAULT_SUMMARIZATION_MAX_DURATION_MS, + StreamDurationBudgetError, +} from "../../src/core/compaction/stream-watchdog.ts"; +import { CredentialFailoverError, TURN_RETRY_SUPPRESSION_PREFIX } from "../../src/core/credential-pool/failover.ts"; +import { classifyRequiredCompactionFallbackFailure } from "../../src/core/extensions/builtin/compaction/deterministic-fallback.ts"; +import { + SummaryGenerationError, + SummaryRequestError, +} from "../../src/core/extensions/builtin/compaction/speculative.ts"; +import type { ExtensionContext } from "../../src/core/extensions/index.ts"; +import { createBlockingContext, createCompactionHandlers } from "../helpers/blocking-compaction-harness.ts"; +import { OPENAI_NATIVE_LEGACY_MODEL } from "./openai-remote-test-models.ts"; + +/** Exactly what `runCredentialFailover` rethrows once any event past `start` reached the caller. */ +function markerBearingFailover(detail = "Codex error: stream ended with an error response"): CredentialFailoverError { + return new CredentialFailoverError({ kind: "fail_request" }, new Error(detail), { suppressTurnRetry: true }); +} + +function partialMessage(model: Model): AssistantMessage { + return { + role: "assistant", + content: [], + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }; +} + +function startEvent(model: Model): AssistantMessageEvent { + return { type: "start", partial: partialMessage(model) }; +} + +/** + * Replace the harness model runtime with one whose summarization stream commits + * output and then throws — the shape credential rotation produces, and the shape + * the faux provider cannot express (it converts throws into error stops). + */ +function installThrowingSummarizationRuntime(ctx: ExtensionContext, error: unknown): () => number { + let calls = 0; + const runtime = { + stream: (model: Model) => { + calls++; + const stream = createAssistantMessageEventStream(); + stream.push(startEvent(model)); + stream.push({ type: "text_delta", contentIndex: 0, delta: "partial summary", partial: partialMessage(model) }); + stream.fail(error); + return stream; + }, + }; + (ctx.modelRegistry as unknown as { modelRuntime: typeof runtime }).modelRuntime = runtime; + return () => calls; +} + +describe("summarization provider failure authorizes the deterministic fallback", () => { + it("classifies a marker-bearing credential failover error as a provider failure", () => { + expect(classifyRequiredCompactionFallbackFailure(markerBearingFailover())).toBe("summarization-provider-failure"); + // A pool that never committed output rethrows without the marker; it is just + // as terminal for this summary, so it authorizes the same recovery. + expect( + classifyRequiredCompactionFallbackFailure( + new CredentialFailoverError({ kind: "fail_request" }, new Error("all slots blocked"), { + suppressTurnRetry: false, + }), + ), + ).toBe("summarization-provider-failure"); + // Single-key providers have no rotation wrapper: the same outage arrives as a + // bare error carrying the marker minted by another failover lane. + expect( + classifyRequiredCompactionFallbackFailure(new Error(`${TURN_RETRY_SUPPRESSION_PREFIX}Codex error: boom`)), + ).toBe("summarization-provider-failure"); + }); + + it("classifies a non-transient summary request error as a provider failure", () => { + expect(classifyRequiredCompactionFallbackFailure(new SummaryRequestError("Codex error: boom", false))).toBe( + "summarization-provider-failure", + ); + }); + + it("keeps aborts, refusals and retryable failures out of the destructive fallback", () => { + // A refusal must stay loud: reducing context would not make the model comply. + expect( + classifyRequiredCompactionFallbackFailure(new SummaryRequestError("refused", false, undefined, true)), + ).toBeUndefined(); + // A transient provider failure is answered by another attempt, not by dropping context. + expect(classifyRequiredCompactionFallbackFailure(new SummaryRequestError("overloaded", true))).toBeUndefined(); + // Missing credentials are a configuration fault with an actionable message. + expect( + classifyRequiredCompactionFallbackFailure( + new SummaryGenerationError("auth", "summarization credentials unavailable: no API key configured"), + ), + ).toBeUndefined(); + // An ordinary bug must not authorize destructive context reduction. + expect( + classifyRequiredCompactionFallbackFailure(new TypeError("cannot read properties of undefined")), + ).toBeUndefined(); + }); + + it("applies a deterministic checkpoint when the blocking route's summary stream throws a marker error", async () => { + const handlers = createCompactionHandlers(); + const harness = createBlockingContext({ usageTokens: 9_900 }); + const summarizationCalls = installThrowingSummarizationRuntime(harness.ctx, markerBearingFailover()); + const branchEntries = harness.ctx.sessionManager.getBranch(); + const preparation = prepareCompaction(branchEntries, harness.ctx.getCompactionSettings(), true); + expect(preparation).toBeDefined(); + + const result = await handlers.sessionBeforeCompact( + { + type: "session_before_compact", + reason: "threshold", + willRetry: false, + requestId: "issue-1741-marker", + preparation: preparation!, + branchEntries, + signal: new AbortController().signal, + }, + harness.ctx, + ); + + if (!result) throw new Error("Expected a compaction handler result"); + // The wedge was `{ cancel: true }`: compaction never applied, so the context + // stayed over threshold and the next prompt repeated the identical failure. + expect(result).not.toHaveProperty("cancel"); + expect(result).toMatchObject({ + compaction: { + details: { + schema: "senpi.compaction.deterministic-fallback.v1", + origin: "required-compaction-recovery", + failureKind: "summarization-provider-failure", + }, + }, + }); + // The marker class is terminal: it must never be re-billed as a retry. + expect(summarizationCalls()).toBe(1); + + // The next turn proceeds: applying the checkpoint drops the bulk that kept the + // session above the threshold while the live request survives. + const compaction = result.compaction; + if (!compaction) throw new Error("Expected deterministic recovery compaction"); + harness.sessionManager.appendCompaction( + compaction.summary, + compaction.firstKeptEntryId, + compaction.tokensBefore, + compaction.details, + true, + ); + const retained = JSON.stringify(harness.sessionManager.buildSessionContext().messages); + expect(retained).toContain("Keep latest request"); + expect(retained).not.toContain("Old assistant context"); + }); + + it("recovers the blocking route from a non-transient provider error stop", async () => { + const handlers = createCompactionHandlers(); + const harness = createBlockingContext({ usageTokens: 9_900 }); + harness.registration.setResponses([ + fauxAssistantMessage("", { + stopReason: "error", + errorMessage: `${TURN_RETRY_SUPPRESSION_PREFIX}Codex error: stream ended with an error response`, + }), + ]); + const branchEntries = harness.ctx.sessionManager.getBranch(); + const preparation = prepareCompaction(branchEntries, harness.ctx.getCompactionSettings(), true); + + const result = await handlers.sessionBeforeCompact( + { + type: "session_before_compact", + reason: "threshold", + willRetry: false, + requestId: "issue-1741-error-stop", + preparation: preparation!, + branchEntries, + signal: new AbortController().signal, + }, + harness.ctx, + ); + + if (!result) throw new Error("Expected a compaction handler result"); + expect(result).not.toHaveProperty("cancel"); + expect(result).toMatchObject({ + compaction: { details: { failureKind: "summarization-provider-failure" } }, + }); + expect(harness.registration.getCallLog()).toHaveLength(1); + }); + + it("tells the user plainly what happened without the internal retry-suppression marker", async () => { + const handlers = createCompactionHandlers(); + const harness = createBlockingContext({ usageTokens: 9_900 }); + const notify = vi.fn(); + (harness.ctx as unknown as { ui: { notify: typeof notify } }).ui = { notify }; + const failure = markerBearingFailover(); + installThrowingSummarizationRuntime(harness.ctx, failure); + const branchEntries = harness.ctx.sessionManager.getBranch(); + const preparation = prepareCompaction(branchEntries, harness.ctx.getCompactionSettings(), true); + + await handlers.sessionBeforeCompact( + { + type: "session_before_compact", + reason: "threshold", + willRetry: false, + requestId: "issue-1741-message", + preparation: preparation!, + branchEntries, + signal: new AbortController().signal, + }, + harness.ctx, + ); + + // The marker stays on the internal error the session-level retry-suppression + // predicates read, and never reaches what the user is shown. + expect(failure.message.startsWith(TURN_RETRY_SUPPRESSION_PREFIX)).toBe(true); + expect(notify).toHaveBeenCalledTimes(1); + const [message] = notify.mock.calls[0] as [string, string?]; + expect(message).not.toContain(TURN_RETRY_SUPPRESSION_PREFIX); + expect(message).toContain("could not complete a provider summary"); + expect(message).toContain("deterministic checkpoint was applied"); + expect(message).toContain("safe to continue"); + for (const call of harness.endCompaction.mock.calls as Array<[{ errorMessage?: string }]>) { + expect(call[0]?.errorMessage ?? "").not.toContain(TURN_RETRY_SUPPRESSION_PREFIX); + } + }); +}); + +describe("summarization settlement stays inside the watched budget", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("bounds a stream whose iterator ends without a terminal result event", async () => { + let aborted = false; + const outcome = consumeStreamWithIdleTimeout<{ type: string }, string>( + { + async *[Symbol.asyncIterator]() { + yield { type: "start" }; + }, + }, + { + idleTimeoutMs: 10_000, + maxDurationMs: 500, + abort: () => { + aborted = true; + }, + // A provider that ends its iterator without pushing `done`/`error` + // leaves `result()` pending forever. + settle: () => new Promise(() => undefined), + }, + ).catch((caught: unknown) => caught); + + await vi.advanceTimersByTimeAsync(600); + + expect(await outcome).toBeInstanceOf(StreamDurationBudgetError); + expect(aborted).toBe(true); + }); + + it("bounds completeSummarization when the provider never settles its result", async () => { + let requestSignal: AbortSignal | undefined; + const outcome = completeSummarization( + OPENAI_NATIVE_LEGACY_MODEL, + { systemPrompt: "", messages: [] }, + { maxTokens: 32 }, + (_model, _context, options) => { + requestSignal = options?.signal; + const stream = createAssistantMessageEventStream(); + stream.push(startEvent(OPENAI_NATIVE_LEGACY_MODEL)); + // Iterator done, `result()` never resolves: today this parks compaction + // with no timer armed at all. + stream.end(); + return stream; + }, + ).catch((caught: unknown) => caught); + + await vi.advanceTimersByTimeAsync(DEFAULT_SUMMARIZATION_MAX_DURATION_MS + 1); + + const pending = Symbol("pending"); + const observed = await Promise.race([outcome, Promise.resolve(pending)]); + expect(observed).toBeInstanceOf(StreamDurationBudgetError); + expect(requestSignal?.aborted).toBe(true); + }); +}); diff --git a/packages/coding-agent/test/compaction/summarization-total-budget.test.ts b/packages/coding-agent/test/compaction/summarization-total-budget.test.ts new file mode 100644 index 000000000..5c4253b00 --- /dev/null +++ b/packages/coding-agent/test/compaction/summarization-total-budget.test.ts @@ -0,0 +1,55 @@ +// Issue #1741: the per-attempt summarization budget is size-scaled (2ms per +// estimated input token, 30-minute ceiling), so a large session legally licenses +// a 690s+ stream and every retry re-arms that budget from scratch. One compaction +// needs a wall-clock bound that does not grow with the input. +import { describe, expect, it } from "vitest"; +import { + createSummarizationDeadline, + SUMMARIZATION_MAX_DURATION_CAP_MS, + SUMMARIZATION_TOTAL_BUDGET_MS, + SummarizationTotalBudgetError, + summarizationMaxDurationMs, + summarizationTotalBudgetMs, +} from "../../src/core/compaction/stream-watchdog.ts"; +import { classifyRequiredCompactionFallbackFailure } from "../../src/core/extensions/builtin/compaction/deterministic-fallback.ts"; + +describe("one compaction is bounded across every attempt and retry", () => { + it("keeps the total budget independent of input size", () => { + expect(SUMMARIZATION_TOTAL_BUDGET_MS).toBe(900_000); + expect(summarizationTotalBudgetMs()).toBe(SUMMARIZATION_TOTAL_BUDGET_MS); + // A 345k-token input licenses a 690s attempt and a 900k-token input licenses + // the full 30-minute ceiling; neither raises the total a session may wait. + expect(summarizationMaxDurationMs(345_000)).toBe(690_000); + expect(summarizationMaxDurationMs(1_000_000)).toBe(SUMMARIZATION_MAX_DURATION_CAP_MS); + expect(summarizationTotalBudgetMs()).toBe(SUMMARIZATION_TOTAL_BUDGET_MS); + }); + + it("honors an explicit per-attempt override as the total, clamped to the ceiling", () => { + expect(summarizationTotalBudgetMs(60_000)).toBe(SUMMARIZATION_TOTAL_BUDGET_MS); + expect(summarizationTotalBudgetMs(1_200_000)).toBe(1_200_000); + expect(summarizationTotalBudgetMs(5_000_000)).toBe(SUMMARIZATION_MAX_DURATION_CAP_MS); + expect(summarizationTotalBudgetMs(0)).toBe(SUMMARIZATION_TOTAL_BUDGET_MS); + expect(summarizationTotalBudgetMs(Number.NaN)).toBe(SUMMARIZATION_TOTAL_BUDGET_MS); + }); + + it("clamps each attempt to what is left and refuses a new attempt past the deadline", () => { + let now = 1_000; + const deadline = createSummarizationDeadline(10_000, () => now); + expect(deadline.totalBudgetMs).toBe(10_000); + expect(deadline.attemptBudgetMs(30_000)).toBe(10_000); + now += 7_000; + expect(deadline.remainingMs()).toBe(3_000); + expect(deadline.attemptBudgetMs(30_000)).toBe(3_000); + // A shorter request is never widened by the remaining budget. + expect(deadline.attemptBudgetMs(500)).toBe(500); + now += 3_000; + expect(deadline.remainingMs()).toBe(0); + expect(() => deadline.attemptBudgetMs(30_000)).toThrow(SummarizationTotalBudgetError); + }); + + it("routes an exhausted total budget into the deterministic fallback", () => { + const error = new SummarizationTotalBudgetError(900_000); + expect(error.message).toContain("900000ms"); + expect(classifyRequiredCompactionFallbackFailure(error)).toBe("summarization-timeout"); + }); +}); From 6093edf1432531060f40f23421778efb19ff556c Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 16 Sep 2026 12:21:41 +0900 Subject: [PATCH 2/4] fix(compaction): recover a session from a provider-killed summary stream A summary stream terminated by a credential-rotation or provider error classified as undefined, so the deterministic fallback that exists exactly for "summarization did not complete" never ran: the required route rethrew, the `senpi:no-turn-retry:` marker disabled both session retry and model fallback, the context stayed above the threshold, and the next prompt repeated the same failure forever. Authorize the fallback by outcome instead of by a four-class enumeration: a credential failover error, a marker-bearing error, or a non-transient, non-refused summary-request error now maps to `summarization-provider-failure` on every required route, and the same classes are mirrored in the summarization retry predicate so the terminal class is never re-billed. Refusals, aborts and missing credentials stay loud. Bound one compaction at 15 minutes across every attempt and retry regardless of input size, keep the stream's final settlement inside the watchdog so a provider that ends its iterator without a terminal event cannot park compaction with no timer armed, and state the recovery to the user in plain words without the internal marker. Fixes #1741 --- packages/coding-agent/CHANGELOG.md | 2 + .../src/core/compaction/changes.md | 21 +++ .../src/core/compaction/compaction.ts | 6 +- .../src/core/compaction/stream-watchdog.ts | 145 ++++++++++++++++-- .../extensions/builtin/compaction/changes.md | 25 +++ .../compaction/deterministic-fallback.ts | 82 +++++++++- .../extensions/builtin/compaction/index.ts | 30 +++- .../builtin/compaction/speculative-summary.ts | 6 +- .../builtin/compaction/speculative.ts | 46 +++++- .../builtin/compaction/transient-failure.ts | 12 +- 10 files changed, 339 insertions(+), 36 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 18ee92017..01d22e1ff 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,8 @@ ### Fixed +- A session no longer dead-ends when the compaction summarizer is killed by a provider or credential failure. Such a stream used to end required compaction with the raw internal `senpi:no-turn-retry:` marker, which also disabled auto-retry and model fallback; because compaction never applied, the context stayed above the threshold and every following prompt failed identically. Those failures now apply the deterministic compaction checkpoint (retained-suffix safety checks unchanged) and the turn continues, with one plain warning saying a provider summary could not be completed, that a checkpoint was applied and older detail was dropped, and that it is safe to continue. Provider refusals, missing credentials and user aborts still surface loudly instead of reducing context. One compaction is also bounded at 15 minutes total across every attempt and retry regardless of input size (an explicit `compaction.summarizationMaxDurationMs` above that still wins), and the summary stream's final settlement now happens inside the watchdog, so a provider whose stream ends without a terminal event can no longer park compaction with no timer armed ([#1741](https://github.com/code-yeongyu/senpi/issues/1741)). + ### Removed ## [2026.9.16] - 2026-09-16 diff --git a/packages/coding-agent/src/core/compaction/changes.md b/packages/coding-agent/src/core/compaction/changes.md index a5b4c3785..a9ebc3e8d 100644 --- a/packages/coding-agent/src/core/compaction/changes.md +++ b/packages/coding-agent/src/core/compaction/changes.md @@ -1,3 +1,24 @@ +## 2026-09-16 - Bound one compaction and settle its stream inside the watchdog (#1741) + +### What changed + +- `packages/coding-agent/src/core/compaction/stream-watchdog.ts`: `consumeStreamWithIdleTimeout` gains an optional `settle()` callback and returns its value, awaiting the stream's final `result()` under the SAME idle and wall-clock timers as iteration (overloads keep the settle-less call sites at `Promise`). Adds the compaction-wide bound `SUMMARIZATION_TOTAL_BUDGET_MS` (900,000 ms), `summarizationTotalBudgetMs(attemptOverrideMs?)`, `SummarizationTotalBudgetError`, and `createSummarizationDeadline(totalBudgetMs, now?)` whose `attemptBudgetMs()` clamps one attempt to the compaction's remaining budget and throws once nothing is left. +- `packages/coding-agent/src/core/compaction/compaction.ts`: `completeSummarization` returns the value settled inside `consumeStreamWithIdleTimeout` instead of awaiting `responseStream.result()` after the watchdog's `finally` cleared its timers. + +### Why + +- Issue #1741: final `result()` settlement sat outside the watchdog, so a provider whose iterator ends without a terminal `done`/`error` event parked compaction forever with no timer armed at all. +- The per-attempt budget is size-scaled (2 ms per estimated input token, 30-minute ceiling) and every retry re-arms it, so a large session's total wait grew with the very thing that made it slow. One compaction now shares a single deadline that never scales with the input; only an explicit `compaction.summarizationMaxDurationMs` override raises it. + +### Why an extension could not handle it + +- The watchdog is core compaction mechanics shared by the core route and the builtin extension route; an extension cannot arm a timer around a stream core owns, nor bound an operation whose attempts core and the extension split between them. + +### Expected merge conflict zones + +- MEDIUM: `stream-watchdog.ts` `consumeStreamWithIdleTimeout` signature and its loop exits. +- LOW: `compaction.ts` `completeSummarization` stream settlement. + ## 2026-09-07 - Effective admission reserve (#7921 case 2) ### What changed diff --git a/packages/coding-agent/src/core/compaction/compaction.ts b/packages/coding-agent/src/core/compaction/compaction.ts index 083306a25..4772d2382 100644 --- a/packages/coding-agent/src/core/compaction/compaction.ts +++ b/packages/coding-agent/src/core/compaction/compaction.ts @@ -764,13 +764,15 @@ export async function completeSummarization( const responseStream = Promise.resolve( streamFn ? streamFn(model, context, requestOptions) : streamSimple(model, context, requestOptions), ); - await consumeStreamWithIdleTimeout(responseStream, { + // Settlement rides inside the watchdog: a provider whose iterator ends + // without a terminal event used to park here with every timer cleared. + return await consumeStreamWithIdleTimeout(responseStream, { idleTimeoutMs: DEFAULT_SUMMARIZATION_IDLE_TIMEOUT_MS, maxDurationMs, abort: () => requestController.abort(), signal: callerSignal, + settle: async () => await (await responseStream).result(), }); - return await (await responseStream).result(); } finally { if (callerSignal) callerSignal.removeEventListener("abort", onCallerAbort); } diff --git a/packages/coding-agent/src/core/compaction/stream-watchdog.ts b/packages/coding-agent/src/core/compaction/stream-watchdog.ts index fb45a0b24..37f1d50a8 100644 --- a/packages/coding-agent/src/core/compaction/stream-watchdog.ts +++ b/packages/coding-agent/src/core/compaction/stream-watchdog.ts @@ -64,6 +64,79 @@ export const SUMMARIZATION_MAX_DURATION_PER_TOKEN_MS = 2; */ export const SUMMARIZATION_MAX_DURATION_CAP_MS = 1_800_000; +/** + * Total wall clock ONE compaction may hold the session across every attempt, + * retry and overflow shrink. + * + * The per-attempt budget is deliberately proportional to the input, so a large + * session legally licenses a 690s attempt (345k tokens) or the full 30-minute + * ceiling (900k tokens), and each retry re-arms that budget from scratch: the + * user's wait grew with the very thing that made it slow, without bound (#1741). + * This cap is the session-health bound the per-attempt budget cannot be: it + * never scales with the input, and every attempt of one compaction shares it. + */ +export const SUMMARIZATION_TOTAL_BUDGET_MS = 900_000; + +/** + * Total budget for one compaction. Size never raises it; only an explicit + * `compaction.summarizationMaxDurationMs` override does, because an operator who + * deliberately allows a longer single attempt must not have that attempt cut + * short by the total. Clamped to {@link SUMMARIZATION_MAX_DURATION_CAP_MS}. + */ +export function summarizationTotalBudgetMs(attemptOverrideMs?: number): number { + const override = + attemptOverrideMs !== undefined && Number.isFinite(attemptOverrideMs) && attemptOverrideMs > 0 + ? Math.min(SUMMARIZATION_MAX_DURATION_CAP_MS, attemptOverrideMs) + : 0; + return Math.max(SUMMARIZATION_TOTAL_BUDGET_MS, override); +} + +/** + * One compaction outlived {@link SUMMARIZATION_TOTAL_BUDGET_MS}. Distinct from + * {@link StreamDurationBudgetError}, which bounds a single attempt: this one says + * no further attempt may start, so recovery must come from the deterministic + * fallback rather than another provider request. + */ +export class SummarizationTotalBudgetError extends Error { + readonly totalBudgetMs: number; + constructor(totalBudgetMs: number) { + super( + `Compaction exceeded its ${totalBudgetMs}ms total wall-clock budget across every summarization attempt and retry`, + ); + this.name = "SummarizationTotalBudgetError"; + this.totalBudgetMs = totalBudgetMs; + } +} + +export interface SummarizationDeadline { + readonly totalBudgetMs: number; + /** Time left before the whole compaction is out of budget; never negative. */ + remainingMs(): number; + /** + * Clamp one attempt's wall-clock budget to what the compaction has left, so a + * retry started near the deadline cannot re-arm a full attempt budget. Throws + * {@link SummarizationTotalBudgetError} once nothing is left. + */ + attemptBudgetMs(requestedMs: number): number; +} + +export function createSummarizationDeadline( + totalBudgetMs: number, + now: () => number = Date.now, +): SummarizationDeadline { + const startedMs = now(); + const remainingMs = (): number => Math.max(0, totalBudgetMs - (now() - startedMs)); + return { + totalBudgetMs, + remainingMs, + attemptBudgetMs: (requestedMs: number): number => { + const remaining = remainingMs(); + if (remaining <= 0) throw new SummarizationTotalBudgetError(totalBudgetMs); + return Math.min(requestedMs, remaining); + }, + }; +} + /** * Total time one summarization attempt may hold the session, sized to its input. * @@ -83,7 +156,7 @@ export function summarizationMaxDurationMs(estimatedInputTokens: number, overrid return Math.min(SUMMARIZATION_MAX_DURATION_CAP_MS, Math.max(DEFAULT_SUMMARIZATION_MAX_DURATION_MS, scaled)); } -export interface ConsumeStreamWithIdleTimeoutOptions { +export interface ConsumeStreamWithIdleTimeoutOptions { /** Silence budget per read; the timer resets on every event. */ readonly idleTimeoutMs: number; /** Total wall-clock budget for the whole stream; omit to leave it unbounded. */ @@ -93,6 +166,14 @@ export interface ConsumeStreamWithIdleTimeoutOptions { readonly onEvent?: (event: T) => void; /** Caller cancellation; an abort here ends the wait without an idle error. */ readonly signal?: AbortSignal; + /** + * Final settlement of the stream (its `result()`), awaited under the SAME + * timers as iteration. A provider whose iterator ends without pushing a + * terminal `done`/`error` event leaves `result()` pending forever; settling it + * after the watchdog's timers were cleared parked compaction with no timer + * armed at all (#1741). + */ + readonly settle?: () => Promise; } const IDLE_TRIP = "idle-trip" as const; @@ -104,19 +185,23 @@ const CALLER_ABORTED = "caller-aborted" as const; * event arrives within `idleTimeoutMs`. Caller aborts propagate as the * stream's own abort outcome, never masked as an idle timeout. */ -export async function consumeStreamWithIdleTimeout( +export function consumeStreamWithIdleTimeout( stream: AsyncIterable | PromiseLike>, - options: ConsumeStreamWithIdleTimeoutOptions, -): Promise { - const { idleTimeoutMs, maxDurationMs, abort, onEvent, signal } = options; + options: ConsumeStreamWithIdleTimeoutOptions & { settle?: undefined }, +): Promise; +export function consumeStreamWithIdleTimeout( + stream: AsyncIterable | PromiseLike>, + options: ConsumeStreamWithIdleTimeoutOptions & { settle: () => Promise }, +): Promise; +export async function consumeStreamWithIdleTimeout( + stream: AsyncIterable | PromiseLike>, + options: ConsumeStreamWithIdleTimeoutOptions, +): Promise { + const { idleTimeoutMs, maxDurationMs, abort, onEvent, signal, settle } = options; let iterator: AsyncIterator | undefined; let removeAbortListener: (() => void) | undefined; let callerAbortPromise: Promise | undefined; - if (signal?.aborted) { - return; - } - // One absolute deadline for the whole stream, not a per-read budget. Created - // only after the already-aborted early return so no timer is ever leaked. + // One absolute deadline for the whole stream, not a per-read budget. let budgetPromise: Promise | undefined; let budgetTimer: ReturnType | undefined; let budgetMs = 0; @@ -127,14 +212,46 @@ export async function consumeStreamWithIdleTimeout( budgetTimer.unref?.(); budgetPromise = promise; } - if (signal !== undefined) { + if (signal !== undefined && !signal.aborted) { const { promise, resolve } = Promise.withResolvers(); const onAbort = () => resolve(CALLER_ABORTED); signal.addEventListener("abort", onAbort, { once: true }); removeAbortListener = () => signal.removeEventListener("abort", onAbort); callerAbortPromise = promise; } + // Settle the stream under the timers this call already armed. Every exit that + // is not a thrown watchdog error goes through here, so `result()` can never be + // awaited with no deadline in force. + const settleUnderWatchdogs = async (): Promise => { + if (!settle) return undefined; + const { promise: idlePromise, resolve: resolveIdle } = Promise.withResolvers(); + const timer = setTimeout(() => resolveIdle(IDLE_TRIP), idleTimeoutMs); + timer.unref?.(); + const contenders: Array> = [ + settle().then((value) => ({ settled: value })), + idlePromise, + ]; + if (budgetPromise) contenders.push(budgetPromise); + let outcome: { settled: R } | typeof IDLE_TRIP | typeof BUDGET_TRIP; + try { + outcome = await Promise.race(contenders); + } finally { + clearTimeout(timer); + } + if (outcome === IDLE_TRIP) { + abort(); + throw new StreamIdleTimeoutError(idleTimeoutMs); + } + if (outcome === BUDGET_TRIP) { + abort(); + throw new StreamDurationBudgetError(budgetMs); + } + return outcome.settled; + }; try { + // A caller that already cancelled still settles its stream - the terminal + // aborted message is what callers return - but under the same watchdogs. + if (signal?.aborted) return await settleUnderWatchdogs(); let resolvedStream: AsyncIterable; if (Symbol.asyncIterator in stream) { resolvedStream = stream; @@ -149,7 +266,7 @@ export async function consumeStreamWithIdleTimeout( abort(); throw new StreamDurationBudgetError(budgetMs); } - if (resolution === CALLER_ABORTED) return; + if (resolution === CALLER_ABORTED) return await settleUnderWatchdogs(); resolvedStream = resolution; } iterator = resolvedStream[Symbol.asyncIterator](); @@ -181,9 +298,9 @@ export async function consumeStreamWithIdleTimeout( } if (result === CALLER_ABORTED) { void iterator?.return?.(); - return; + return await settleUnderWatchdogs(); } - if (result.done) return; + if (result.done) return await settleUnderWatchdogs(); onEvent?.(result.value); } } finally { diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md index b932e2e49..f10c03831 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/changes.md @@ -1,3 +1,28 @@ +## Authorize the deterministic fallback for a provider-killed summary stream (2026-09-16) + +### What changed + +- `packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts`: `RequiredCompactionFallbackFailure` gains `summarization-provider-failure`, and `classifyRequiredCompactionFallbackFailure` now recognizes a summary stream terminated by a provider or credential fault - any `CredentialFailoverError`, any error whose message carries the `senpi:no-turn-retry:` marker, and a non-transient, non-refused `SummaryRequestError` with no structured failure kind - plus `SummarizationTotalBudgetError` as `summarization-timeout`. User aborts, policy refusals, missing credentials and ordinary bugs stay unauthorized. Adds `stripTurnRetrySuppressionPrefix()` and `formatRequiredCompactionFallbackNotice()`. +- `packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts`: `SummaryRequestError` carries an explicit `refused` flag (set from `refusal`/`sensitive` stop details); `isRetryableSummaryAttempt` mirrors the new class so a marker-bearing or credential-failover error is never re-billed while genuinely transient failures still retry; `runExtensionCompaction` opens one `createSummarizationDeadline` for the whole compaction, re-clamps every attempt budget to what is left, and refuses a retry once the deadline passed. +- `packages/coding-agent/src/core/extensions/builtin/compaction/speculative-summary.ts`: `generateSummaryMessage` returns the message settled inside `consumeStreamWithIdleTimeout` instead of awaiting `responseStream.result()` after the watchdog cleared its timers. +- `packages/coding-agent/src/core/extensions/builtin/compaction/transient-failure.ts`: a compaction-wide total-budget trip degrades like the other watchdog trips. +- `packages/coding-agent/src/core/extensions/builtin/compaction/index.ts`: `recoverRequiredCompaction` takes the context and the causing error, notifies the user once through `ctx.ui.notify` when the deterministic checkpoint is applied, and every compaction message built from an error message is stripped of the `senpi:no-turn-retry:` prefix. + +### Why + +- Issue #1741: a summarization stream killed by a credential-rotation or provider error classified as `undefined`, so the deterministic fallback that exists precisely for "summarization did not complete" never ran. The blocking route rethrew, the marker disabled both session retry and model fallback, the context stayed above the threshold, and the next prompt repeated the identical failure forever; the circuit breaker never debited because the failure was non-transient. +- The marker is a session-internal replay-suppression signal. It must stay on the error object the session predicates read, and never appear in what the user is shown. + +### Why an extension could not handle it + +- The classification is this builtin's private authorization contract for destructive context reduction, and the retry predicate lives inside its own summarization loop; no external extension can observe a summary attempt's provenance or participate in required-compaction admission. + +### Expected merge conflict zones + +- LOW: `deterministic-fallback.ts` failure-kind union and classifier. +- LOW: `speculative.ts` `SummaryRequestError` shape, `isRetryableSummaryAttempt`, and the summarization while-loop. +- LOW: `speculative-summary.ts` stream settlement, `transient-failure.ts` predicate, `index.ts` `recoverRequiredCompaction` call sites. + # changes.md — builtin compaction policy ## Deterministic resume slice for an over-window restored context (2026-09-10) diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts index 8557d7ab0..89135462e 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts @@ -1,5 +1,10 @@ import { type CompactionPreparation, type CompactionResult, estimateTokens } from "../../../compaction/index.ts"; -import { StreamDurationBudgetError, StreamIdleTimeoutError } from "../../../compaction/stream-watchdog.ts"; +import { + StreamDurationBudgetError, + StreamIdleTimeoutError, + SummarizationTotalBudgetError, +} from "../../../compaction/stream-watchdog.ts"; +import { CredentialFailoverError, TURN_RETRY_SUPPRESSION_PREFIX } from "../../../credential-pool/failover.ts"; import { filterContextExcludedMessages } from "../../../messages.ts"; import { buildSessionContext, @@ -16,6 +21,7 @@ import { capUtf8Bytes } from "./task-intent.ts"; export type RequiredCompactionFallbackFailure = | "summarization-timeout" + | "summarization-provider-failure" | "upstream-stream-truncated" | "summarization-overflow-exhausted" | "summarization-empty-summary"; @@ -140,10 +146,42 @@ function isSafeBoundedValue(value: unknown, seen = new Set(), depth = 0) return true; } +/** + * A provider or credential fault that ended the summary stream with no usable + * summary and no cheaper recovery left. + * + * Credential rotation rethrows EVERY terminal outcome as `CredentialFailoverError`, + * and once any event past `start` reached the caller it prepends the + * `senpi:no-turn-retry:` marker so the session layer never replays a partially + * delivered turn. That marker also disables session retry and model fallback, so + * before #1741 such an error left required compaction with no recovery at all: + * it applied no summary, authorized no fallback, and repeated identically on the + * next prompt because the context stayed above the threshold. Single-key + * providers have no rotation wrapper and surface the same outage as a + * non-transient `SummaryRequestError`, so the authorization keys on the outcome + * ("the summary stream is dead") rather than on the marker alone. + * + * Deliberately NOT authorized: user aborts (they resolve `undefined` upstream), + * policy refusals (reducing context would not make the model comply), missing + * credentials (a configuration fault with an actionable message), and ordinary + * bugs - destructive context reduction must never be a bug's recovery path. + */ +function isTerminalSummarizationProviderFailure(error: unknown): boolean { + if (error instanceof CredentialFailoverError) return true; + if (error instanceof Error && error.message.startsWith(TURN_RETRY_SUPPRESSION_PREFIX)) return true; + return ( + error instanceof SummaryRequestError && !error.transient && !error.refused && error.failureKind === undefined + ); +} + export function classifyRequiredCompactionFallbackFailure( error: unknown, ): RequiredCompactionFallbackFailure | undefined { - if (error instanceof StreamDurationBudgetError || error instanceof StreamIdleTimeoutError) { + if ( + error instanceof StreamDurationBudgetError || + error instanceof StreamIdleTimeoutError || + error instanceof SummarizationTotalBudgetError + ) { return "summarization-timeout"; } if (error instanceof SummaryRequestError && error.transient && error.failureKind === "upstream-stream-truncated") { @@ -155,9 +193,49 @@ export function classifyRequiredCompactionFallbackFailure( if (error instanceof SummaryGenerationError && error.kind === "empty-summary") { return "summarization-empty-summary"; } + if (isTerminalSummarizationProviderFailure(error)) { + return "summarization-provider-failure"; + } return undefined; } +/** + * `senpi:no-turn-retry:` is a session-internal replay-suppression signal read by + * `_isRetryableError` / `_isHardErrorFallbackEligible`. It must stay on the error + * object those predicates inspect and must never reach user-visible text. + */ +export function stripTurnRetrySuppressionPrefix(message: string): string { + return message.replaceAll(TURN_RETRY_SUPPRESSION_PREFIX, ""); +} + +const FALLBACK_FAILURE_CAUSE: Record = { + "summarization-timeout": "the summary stream ran out of its time budget", + "summarization-provider-failure": "the provider ended the summary stream with an error", + "upstream-stream-truncated": "the provider truncated the summary stream", + "summarization-overflow-exhausted": "the summary input stayed over the provider's context limit", + "summarization-empty-summary": "the provider returned no summary text", +}; + +/** + * What the user is told when a required compaction recovered through the + * deterministic checkpoint. States the outcome plainly and never carries the + * internal retry-suppression marker. + */ +export function formatRequiredCompactionFallbackNotice( + failureKind: RequiredCompactionFallbackFailure, + cause?: unknown, +): string { + const detail = cause instanceof Error ? stripTurnRetrySuppressionPrefix(cause.message).trim() : ""; + return [ + `Compaction could not complete a provider summary: ${FALLBACK_FAILURE_CAUSE[failureKind]}.`, + "A deterministic checkpoint was applied and older transcript detail was dropped, so it is safe to continue.", + detail ? `Provider reported: ${capUtf8Bytes(detail, 512)}.` : "", + "Run /compact on a different model for a richer summary.", + ] + .filter((part) => part.length > 0) + .join(" "); +} + export function createRequiredCompactionFallback( preparation: CompactionPreparation, contextWindow: number, diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts index 8e1d71cc7..4a0058824 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts @@ -16,8 +16,10 @@ import { classifyRequiredCompactionFallbackFailure, createRequiredCompactionFallback, type DeterministicFallbackDiagnostic, + formatRequiredCompactionFallbackNotice, formatRequiredCompactionFallbackRejection, type RequiredCompactionFallbackFailure, + stripTurnRetrySuppressionPrefix, } from "./deterministic-fallback.ts"; import * as idle from "./idle.ts"; import * as idleRetry from "./idle-retry.ts"; @@ -372,8 +374,10 @@ export default function compactionExtension( } function recoverRequiredCompaction( + ctx: ExtensionContext, snapshot: SpeculativeCompactionSnapshot, failureKind: RequiredCompactionFallbackFailure, + cause: unknown, ): { compaction?: CompactionResult; rejectionReason?: string } { const diagnostics: DeterministicFallbackDiagnostic = {}; const compaction = createRequiredCompactionFallback( @@ -384,7 +388,12 @@ export default function compactionExtension( snapshot.branchEntries, diagnostics, ); - return compaction ? { compaction } : { rejectionReason: formatRequiredCompactionFallbackRejection(diagnostics) }; + if (!compaction) return { rejectionReason: formatRequiredCompactionFallbackRejection(diagnostics) }; + // The compaction itself succeeds, so nothing else tells the user their + // transcript was reduced without a provider summary. Say it plainly here, + // and never with the internal retry-suppression marker (#1741). + ctx.ui.notify(formatRequiredCompactionFallbackNotice(failureKind, cause), "warning"); + return { compaction }; } async function applyBlockingCompaction( @@ -474,7 +483,12 @@ export default function compactionExtension( pendingJob.snapshot.generation === speculativeGeneration && pendingJob.snapshot.expectedRevision === ctx.getMessageRevision() ) { - const recovery = recoverRequiredCompaction(pendingJob.snapshot, failureKind); + const recovery = recoverRequiredCompaction( + ctx, + pendingJob.snapshot, + failureKind, + inheritedFailure, + ); compaction = recovery.compaction; if (!compaction) { const result = { applied: false, reason: "failed" } as const; @@ -489,7 +503,7 @@ export default function compactionExtension( reason: "extension", signal: feedbackSignal, aborted: feedbackSignal?.aborted, - errorMessage: `Compaction failed: ${inheritedFailure.message}`, + errorMessage: `Compaction failed: ${stripTurnRetrySuppressionPrefix(inheritedFailure.message)}`, }); state = breaker.recordFailure(state, Date.now(), { route: "extension" }); return { applied: false, reason: "failed" }; @@ -544,7 +558,7 @@ export default function compactionExtension( } catch (error) { const failureKind = classifyRequiredCompactionFallbackFailure(error); if (failureKind !== undefined && !feedbackSignal?.aborted) { - const recovery = recoverRequiredCompaction(snapshot, failureKind); + const recovery = recoverRequiredCompaction(ctx, snapshot, failureKind, error); compaction = recovery.compaction; if (!compaction) { const result = { applied: false, reason: "failed" } as const; @@ -580,7 +594,7 @@ export default function compactionExtension( reason: "extension", signal: feedbackSignal, aborted: feedbackSignal?.aborted, - errorMessage: `Compaction failed: ${message}`, + errorMessage: `Compaction failed: ${stripTurnRetrySuppressionPrefix(message)}`, }); const transient = isTransientSummarizationFailure(error, message); if (transient) { @@ -712,7 +726,7 @@ export default function compactionExtension( failureKind !== undefined && !event.signal.aborted ) { - const recovery = recoverRequiredCompaction(snapshot, failureKind); + const recovery = recoverRequiredCompaction(ctx, snapshot, failureKind, error); if (recovery.compaction) return { compaction: recovery.compaction }; pendingMetadata.delete(event.requestId); return { @@ -722,9 +736,9 @@ export default function compactionExtension( } pendingMetadata.delete(event.requestId); if (error instanceof SummaryGenerationError) { - return { cancel: true, reason: error.message }; + return { cancel: true, reason: stripTurnRetrySuppressionPrefix(error.message) }; } - return { cancel: true, reason: `compaction generator failed: ${message}` }; + return { cancel: true, reason: `compaction generator failed: ${stripTurnRetrySuppressionPrefix(message)}` }; } if (!compaction) { pendingMetadata.delete(event.requestId); diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative-summary.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative-summary.ts index a69d284bd..5de25b8df 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative-summary.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative-summary.ts @@ -159,7 +159,9 @@ export async function generateSummaryMessage(options: { ...summarizationReasoningOptions(options.snapshot.model), ...(options.forbidToolCalls ? { toolChoice: "none" as const } : {}), }); - await consumeStreamWithIdleTimeout(responseStream, { + // Settlement rides inside the watchdog: a provider whose iterator ends + // without a terminal event used to park here with every timer cleared. + return await consumeStreamWithIdleTimeout(responseStream, { idleTimeoutMs: DEFAULT_SUMMARIZATION_IDLE_TIMEOUT_MS, maxDurationMs, abort: () => requestController.abort(), @@ -169,8 +171,8 @@ export async function generateSummaryMessage(options: { options.onProgress?.(event.delta); } }, + settle: async (): Promise => await responseStream.result(), }); - return await responseStream.result(); } finally { if (options.signal) options.signal.removeEventListener("abort", onCallerAbort); } diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts index 168005031..35c51780e 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts @@ -19,10 +19,14 @@ import { prepareCompaction, } from "../../../compaction/index.ts"; import { + createSummarizationDeadline, StreamDurationBudgetError, StreamIdleTimeoutError, + SummarizationTotalBudgetError, summarizationMaxDurationMs, + summarizationTotalBudgetMs, } from "../../../compaction/stream-watchdog.ts"; +import { CredentialFailoverError, TURN_RETRY_SUPPRESSION_PREFIX } from "../../../credential-pool/failover.ts"; import { createWarmAnchorSnapshot, isWarmSummaryAnchorValid, @@ -125,12 +129,20 @@ export type SummaryRequestFailureKind = "upstream-stream-truncated"; export class SummaryRequestError extends Error { readonly transient: boolean; readonly failureKind?: SummaryRequestFailureKind; - - constructor(message: string, transient: boolean, failureKind?: SummaryRequestFailureKind) { + /** + * The provider refused the request (refusal/sensitive stop details) rather + * than failing it. Carried explicitly because the message text cannot encode + * it, and because a refusal must never authorize destructive context + * reduction: dropping older detail would not make the model comply. + */ + readonly refused: boolean; + + constructor(message: string, transient: boolean, failureKind?: SummaryRequestFailureKind, refused = false) { super(message); this.name = "SummaryRequestError"; this.transient = transient; this.failureKind = failureKind; + this.refused = refused; } } @@ -140,7 +152,8 @@ const UPSTREAM_STREAM_TRUNCATED_PATTERN = /(?:^|[^A-Za-z0-9_])upstream_stream_tr * Only failures with no cheaper recovery earn another billed request. * * Every class that `classifyRequiredCompactionFallbackFailure` recognizes - * (watchdog timeouts, `upstream-stream-truncated`, overflow exhaustion, + * (watchdog timeouts, the compaction-wide total budget, terminal provider and + * credential failures, `upstream-stream-truncated`, overflow exhaustion, * empty-summary generation failures) already * has a deterministic zero-LLM recovery, and context overflow is answered by * shrinking the input in the surrounding loop - replaying those would pay for a @@ -150,15 +163,27 @@ const UPSTREAM_STREAM_TRUNCATED_PATTERN = /(?:^|[^A-Za-z0-9_])upstream_stream_tr */ function isRetryableSummaryAttempt(error: unknown): boolean { if (error instanceof StreamDurationBudgetError || error instanceof StreamIdleTimeoutError) return false; + if (error instanceof SummarizationTotalBudgetError) return false; if (error instanceof SummarizationOverflowExhaustedError) return false; if (error instanceof SummaryGenerationError) return false; + // Mirrors the `summarization-provider-failure` class: credential rotation has + // already spent every slot it may spend, and the marker means output was + // committed, so another billed attempt buys nothing the fallback cannot + // rebuild for free. Message text alone must not re-authorize it - the wrapped + // provider detail can read as transient (#1741). + if (error instanceof CredentialFailoverError) return false; + if (error instanceof Error && error.message.startsWith(TURN_RETRY_SUPPRESSION_PREFIX)) return false; if (error instanceof SummaryRequestError) return error.failureKind === undefined && error.transient; if (error instanceof Error) return isRetryableErrorMessage(error.message); return false; } +function isRefusalStop(response: AssistantMessage): boolean { + return response.stopDetails?.type === "refusal" || response.stopDetails?.type === "sensitive"; +} + function summaryRequestFailureKind(response: AssistantMessage): SummaryRequestFailureKind | undefined { - if (response.stopDetails?.type === "refusal" || response.stopDetails?.type === "sensitive") return undefined; + if (isRefusalStop(response)) return undefined; return UPSTREAM_STREAM_TRUNCATED_PATTERN.test(response.errorMessage ?? "") ? "upstream-stream-truncated" : undefined; } @@ -273,6 +298,12 @@ export async function runExtensionCompaction( requestSnapshot.contextWindow, promptTokens, ); + // One deadline for the whole compaction. The per-attempt budget scales with the + // input and every retry re-arms it, so without this a large session could hold + // the turn for attempt-budget x attempts with no bound the user can predict. + const deadline = createSummarizationDeadline( + summarizationTotalBudgetMs(requestSnapshot.preparation.settings.summarizationMaxDurationMs), + ); const overflowRetryStartMs = Date.now(); let overflowAttempts = 0; const summarizationToolsOffered = (requestSnapshot.tools?.length ?? 0) > 0; @@ -307,7 +338,9 @@ export async function runExtensionCompaction( const attempt = await generateSummaryMessage({ context, forbidToolCalls: toolUseRetrySpent, - maxDurationMs: attemptBudgetMs, + // Re-clamped per attempt, not per loop turn: a retry that starts + // near the deadline gets only what is left, and none starts past it. + maxDurationMs: deadline.attemptBudgetMs(attemptBudgetMs), messages: currentMessages, onProgress, prompt, @@ -330,12 +363,14 @@ export async function runExtensionCompaction( attempt.errorMessage || "Compaction summary request failed", failureKind !== undefined || isRetryableAssistantError(attempt), failureKind, + isRefusalStop(attempt), ); } return attempt; }, (error) => retryEligible && + deadline.remainingMs() > 0 && allowSummarizationRetry(Date.now() - retryStartedMs, attemptBudgetMs) && isRetryableSummaryAttempt(error), DEFAULT_SUMMARIZATION_RETRY_POLICY, @@ -375,6 +410,7 @@ export async function runExtensionCompaction( response.errorMessage || "Compaction summary request failed", failureKind !== undefined || isRetryableAssistantError(response), failureKind, + isRefusalStop(response), ); } diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/transient-failure.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/transient-failure.ts index c7005c330..471a012f2 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/transient-failure.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/transient-failure.ts @@ -9,17 +9,23 @@ * * Provider `error` stops carry metadata-aware classification through * `SummaryRequestError` (a refusal whose text merely looks retryable stays - * loud). Watchdog trips — a stalled stream or a stream that outlived its - * wall-clock budget — are infrastructure-slowness outcomes and always degrade. + * loud). Watchdog trips — a stalled stream, a stream that outlived its + * wall-clock budget, or a compaction that outlived its total budget — are + * infrastructure-slowness outcomes and always degrade. * Everything else falls back to the shared message classifier. */ import { isRetryableErrorMessage } from "@earendil-works/pi-ai"; -import { StreamDurationBudgetError, StreamIdleTimeoutError } from "../../../compaction/stream-watchdog.ts"; +import { + StreamDurationBudgetError, + StreamIdleTimeoutError, + SummarizationTotalBudgetError, +} from "../../../compaction/stream-watchdog.ts"; import { SummarizationOverflowExhaustedError } from "./overflow-retry.ts"; import { SummaryRequestError } from "./speculative.ts"; export function isTransientSummarizationFailure(error: unknown, message: string): boolean { if (error instanceof StreamDurationBudgetError || error instanceof StreamIdleTimeoutError) return true; + if (error instanceof SummarizationTotalBudgetError) return true; if (error instanceof SummaryRequestError) return error.transient; if (error instanceof SummarizationOverflowExhaustedError) return true; return isRetryableErrorMessage(message); From 8e288fd2140d433dfcd2fc914e393c96f844e722 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 16 Sep 2026 12:26:28 +0900 Subject: [PATCH 3/4] test(compaction): bind loud summarization failures to the refusal signal Three characterizations expressed a "policy rejection" only as error-message prose, so they pinned the exact contract #1741 changes: a terminal provider error that is not a refusal used to propagate and wedge the session. Bind them to the real `stopDetails.type` refusal signal, which still fails closed, and add the recovery case for a terminal provider error. Refs #1741 --- .../compaction/deterministic-fallback.ts | 4 +-- .../extensions/builtin/compaction/index.ts | 7 +--- .../builtin/compaction/speculative.ts | 2 +- .../before-compact-error-surfacing.test.ts | 3 ++ ...locking-compaction-network-degrade.test.ts | 32 +++++++++++++++++-- .../blocking-compaction-shared-retry.test.ts | 23 +++++++++++-- ...-compaction-deterministic-fallback.test.ts | 7 ++-- 7 files changed, 62 insertions(+), 16 deletions(-) diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts index 89135462e..b6e3cc211 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts @@ -169,9 +169,7 @@ function isSafeBoundedValue(value: unknown, seen = new Set(), depth = 0) function isTerminalSummarizationProviderFailure(error: unknown): boolean { if (error instanceof CredentialFailoverError) return true; if (error instanceof Error && error.message.startsWith(TURN_RETRY_SUPPRESSION_PREFIX)) return true; - return ( - error instanceof SummaryRequestError && !error.transient && !error.refused && error.failureKind === undefined - ); + return error instanceof SummaryRequestError && !error.transient && !error.refused && error.failureKind === undefined; } export function classifyRequiredCompactionFallbackFailure( diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts index 4a0058824..ead8f8bc5 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/index.ts @@ -483,12 +483,7 @@ export default function compactionExtension( pendingJob.snapshot.generation === speculativeGeneration && pendingJob.snapshot.expectedRevision === ctx.getMessageRevision() ) { - const recovery = recoverRequiredCompaction( - ctx, - pendingJob.snapshot, - failureKind, - inheritedFailure, - ); + const recovery = recoverRequiredCompaction(ctx, pendingJob.snapshot, failureKind, inheritedFailure); compaction = recovery.compaction; if (!compaction) { const result = { applied: false, reason: "failed" } as const; diff --git a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts index 35c51780e..2fcd5c38d 100644 --- a/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts +++ b/packages/coding-agent/src/core/extensions/builtin/compaction/speculative.ts @@ -26,12 +26,12 @@ import { summarizationMaxDurationMs, summarizationTotalBudgetMs, } from "../../../compaction/stream-watchdog.ts"; -import { CredentialFailoverError, TURN_RETRY_SUPPRESSION_PREFIX } from "../../../credential-pool/failover.ts"; import { createWarmAnchorSnapshot, isWarmSummaryAnchorValid, type WarmAnchorSnapshot, } from "../../../compaction/warm-anchor.ts"; +import { CredentialFailoverError, TURN_RETRY_SUPPRESSION_PREFIX } from "../../../credential-pool/failover.ts"; import { convertToLlm } from "../../../messages.ts"; import type { ModelRegistry } from "../../../model-registry.ts"; import type { ReadonlySessionManager } from "../../../session-manager.ts"; diff --git a/packages/coding-agent/test/compaction/before-compact-error-surfacing.test.ts b/packages/coding-agent/test/compaction/before-compact-error-surfacing.test.ts index caa31c320..de3c7d8fb 100644 --- a/packages/coding-agent/test/compaction/before-compact-error-surfacing.test.ts +++ b/packages/coding-agent/test/compaction/before-compact-error-surfacing.test.ts @@ -145,9 +145,12 @@ describe("session_before_compact error surfacing", () => { // Given const harness = createHarness(); harness.registration.setResponses([ + // A refusal: it stays unauthorized for the deterministic fallback, so the + // provider detail is what reaches the cancel reason (issue #1741). fauxAssistantMessage("", { stopReason: "error", errorMessage: "faux: request blocked by provider policy", + stopDetails: { type: "refusal" }, }), ]); diff --git a/packages/coding-agent/test/compaction/blocking-compaction-network-degrade.test.ts b/packages/coding-agent/test/compaction/blocking-compaction-network-degrade.test.ts index 0fa05da57..490167924 100644 --- a/packages/coding-agent/test/compaction/blocking-compaction-network-degrade.test.ts +++ b/packages/coding-agent/test/compaction/blocking-compaction-network-degrade.test.ts @@ -79,8 +79,10 @@ describe("blocking compaction network-failure degradation", () => { }); describe("Given a non-transient summarization failure", () => { - it("Then the failure still surfaces loudly as an extension error", async () => { - // Given: a deterministic provider rejection that retrying cannot fix. + it("Then a provider refusal still surfaces loudly as an extension error", async () => { + // Given: a refusal. Reducing context would not make the model comply, so + // this class keeps propagating instead of authorizing the deterministic + // fallback (issue #1741). const { beforeAgentStart } = createCompactionHandlers(); const harness = createBlockingContext({ usageTokens: 9_950 }); registrations.push(harness.registration); @@ -88,6 +90,7 @@ describe("blocking compaction network-failure degradation", () => { fauxAssistantMessage("", { stopReason: "error", errorMessage: "request blocked by provider policy", + stopDetails: { type: "refusal" }, }), ]); @@ -97,6 +100,31 @@ describe("blocking compaction network-failure degradation", () => { "request blocked by provider policy", ); }); + + // Issue #1741: a terminal provider error that is NOT a refusal used to + // propagate the same way, which left the context above the threshold and + // made every following prompt fail identically. It now recovers through the + // deterministic checkpoint, and the provider detail reaches the user as a + // warning instead of an unrecoverable turn. + it("Then a terminal provider error recovers deterministically and warns the user", async () => { + const { beforeAgentStart } = createCompactionHandlers(); + const harness = createBlockingContext({ usageTokens: 9_950 }); + registrations.push(harness.registration); + const notify = vi.fn(); + (harness.ctx as unknown as { ui: { notify: typeof notify } }).ui = { notify }; + harness.registration.setResponses([ + fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "Codex error: stream ended with an error response", + }), + ]); + + await expect(beforeAgentStart(createBeforeAgentStartEvent(), harness.ctx)).resolves.toBeUndefined(); + + expect(harness.registration.state.callCount).toBe(1); + expect(notify).toHaveBeenCalledTimes(1); + expect(notify.mock.calls[0]?.[0]).toContain("Codex error: stream ended with an error response"); + }); }); describe("Given summarization credentials are unavailable", () => { diff --git a/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts b/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts index aeb4ff928..db7714b25 100644 --- a/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts +++ b/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts @@ -80,12 +80,16 @@ describe("blocking compaction shares the bounded summarization retry", () => { describe("Given a non-transient summarization failure", () => { it("Then the route does not spend a single retry", async () => { - // Given: a deterministic provider rejection retrying cannot fix. + // Given: a deterministic provider refusal retrying cannot fix. const { beforeAgentStart } = createCompactionHandlers(); const harness = createBlockingContext({ usageTokens: 9_950 }); registrations.push(harness.registration); harness.registration.setResponses([ - fauxAssistantMessage("", { stopReason: "error", errorMessage: "request blocked by provider policy" }), + fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "request blocked by provider policy", + stopDetails: { type: "refusal" }, + }), ]); // When / Then: unchanged - it surfaces loudly on attempt one. @@ -94,5 +98,20 @@ describe("blocking compaction shares the bounded summarization retry", () => { ); expect(harness.registration.state.callCount).toBe(1); }); + + // Issue #1741: a terminal provider error is recovered by the deterministic + // checkpoint rather than rethrown, and that recovery must not be billed as a + // retry either. + it("Then a terminal provider error is recovered without a second request", async () => { + const { beforeAgentStart } = createCompactionHandlers(); + const harness = createBlockingContext({ usageTokens: 9_950 }); + registrations.push(harness.registration); + harness.registration.setResponses([ + fauxAssistantMessage("", { stopReason: "error", errorMessage: "Codex error: stream ended with an error response" }), + ]); + + await expect(beforeAgentStart(createBeforeAgentStartEvent(), harness.ctx)).resolves.toBeUndefined(); + expect(harness.registration.state.callCount).toBe(1); + }); }); }); diff --git a/packages/coding-agent/test/compaction/required-compaction-deterministic-fallback.test.ts b/packages/coding-agent/test/compaction/required-compaction-deterministic-fallback.test.ts index 437597f43..67a083c97 100644 --- a/packages/coding-agent/test/compaction/required-compaction-deterministic-fallback.test.ts +++ b/packages/coding-agent/test/compaction/required-compaction-deterministic-fallback.test.ts @@ -133,10 +133,13 @@ describe("required compaction deterministic fallback", () => { }); }); - it("does not recover aborted or unrelated failures", async () => { + // Issue #1741 narrowed this: a terminal provider error that is NOT a refusal now + // authorizes the deterministic checkpoint instead of wedging the session. Aborts + // and refusals stay fail-closed. + it("does not recover aborted requests or provider refusals", async () => { for (const testCase of [ { reason: "threshold" as const, message: "upstream_stream_truncated", aborted: true, refusal: false }, - { reason: "threshold" as const, message: "unrelated provider refusal", aborted: false, refusal: false }, + { reason: "threshold" as const, message: "unrelated provider refusal", aborted: false, refusal: true }, { reason: "threshold" as const, message: "upstream_stream_truncated", aborted: false, refusal: true }, ]) { const handlers = createCompactionHandlers(); From f03f0c21eeca036842c6eea6cb9026f8c8e3fa82 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 16 Sep 2026 12:28:24 +0900 Subject: [PATCH 4/4] style(compaction): format the recovery regression setup --- .../test/compaction/blocking-compaction-shared-retry.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts b/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts index db7714b25..d28cdc8d0 100644 --- a/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts +++ b/packages/coding-agent/test/compaction/blocking-compaction-shared-retry.test.ts @@ -107,7 +107,10 @@ describe("blocking compaction shares the bounded summarization retry", () => { const harness = createBlockingContext({ usageTokens: 9_950 }); registrations.push(harness.registration); harness.registration.setResponses([ - fauxAssistantMessage("", { stopReason: "error", errorMessage: "Codex error: stream ended with an error response" }), + fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "Codex error: stream ended with an error response", + }), ]); await expect(beforeAgentStart(createBeforeAgentStartEvent(), harness.ctx)).resolves.toBeUndefined();