Skip to content

fix(compaction): recover a session from a provider-killed summary stream - #1745

Merged
code-yeongyu merged 5 commits into
mainfrom
fix/1741-compaction-fallback-authorization
Sep 16, 2026
Merged

code-yeongyu merged 5 commits into
mainfrom
fix/1741-compaction-fallback-authorization

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

A session could become permanently uncontinuable when the compaction summarizer was killed by a
provider or credential failure. This makes that impossible: any summary stream that dies without a
usable summary now recovers through the deterministic checkpoint on every required-compaction route,
the turn continues, the user is told plainly what happened, and one compaction is bounded in wall
clock regardless of how large its input is.

Fixes #1741

Root cause

Two independent facts combined into the wedge.

The 690s the reporter saw was a legal budget, not a watchdog that failed to fire.
packages/coding-agent/src/core/compaction/stream-watchdog.ts:57 scales the per-attempt budget at
2 ms per estimated input token, :65 caps it at 1,800,000 ms and :78-84 clamps it against a
120,000 ms floor, so a 345k-token input licenses exactly 690,000 ms and a 900k-token input licenses
the full 30 minutes. Every retry re-arms that budget from scratch
(.../builtin/compaction/summarization-retry.ts:19-27,47), and nothing bounded the operation as a
whole.

When that stream finally failed, the error class escaped every recovery mechanism.
packages/coding-agent/src/core/extensions/builtin/compaction/deterministic-fallback.ts:143-159
(classifyRequiredCompactionFallbackFailure) was a four-class enumeration. Credential rotation
rethrows every terminal outcome as CredentialFailoverError
(packages/coding-agent/src/core/credential-pool/failover.ts:145-167) and, once any event past
start reached the caller, prepends the senpi:no-turn-retry: marker. That class matched none of
the four, so it classified as undefined: the deterministic fallback that exists precisely for
"summarization did not complete" never ran, the error was rethrown
(.../builtin/compaction/index.ts:555, :590), the marker disabled both session retry and model
fallback (packages/coding-agent/src/core/agent-session.ts:7876-7878, :7949-7951), the context
stayed above the threshold, and the next prompt re-entered required compaction and failed
identically. The circuit breaker never debited because the failure is classified non-transient
(.../builtin/compaction/index.ts:585-589). Single-key providers have no rotation wrapper and reach
the same undefined through a non-transient SummaryRequestError.

Two further holes on the same path: the marker prefix
(packages/coding-agent/src/core/agent-session.ts:252) reached user-visible compaction text, and
final result() settlement sat outside the watchdog -
packages/coding-agent/src/core/compaction/compaction.ts:767 awaited result() after the timer's
finally had cleared it (:746-750 armed it only around iteration), same shape at
.../builtin/compaction/speculative-summary.ts:162/173. A provider whose iterator ends without a
terminal event parked compaction forever with no timer armed at all.

What changed

  1. Authorization by outcome, not by enumeration. classifyRequiredCompactionFallbackFailure
    gains summarization-provider-failure for a summary stream terminated by a provider or credential
    fault: any CredentialFailoverError, any error whose message carries the retry-suppression
    marker, and a non-transient, non-refused SummaryRequestError with no structured failure kind.
    This reaches every required route, because all four call sites (blocking route, inherited warm
    job, core route, warm-inherit gate) go through that one classifier. User aborts, policy refusals
    (SummaryRequestError now carries an explicit refused flag set from refusal/sensitive stop
    details), missing credentials and ordinary bugs stay unauthorized - destructive context reduction
    must never be a bug's recovery path.
  2. Mirrored in the retry predicate. isRetryableSummaryAttempt refuses the same classes, so a
    marker-bearing failure is never re-billed as another summarization request (its wrapped provider
    detail can read as transient), while a genuinely transient failure still retries unchanged.
  3. Settlement inside the watched iterator. consumeStreamWithIdleTimeout takes an optional
    settle() and awaits the stream's final result() under the same idle and wall-clock timers as
    iteration; both summarization call sites return that value instead of awaiting result() after
    the watchdog cleared its timers.
  4. One compaction, one deadline. New SUMMARIZATION_TOTAL_BUDGET_MS (900,000 ms) and
    createSummarizationDeadline() bound a whole compaction across every attempt, retry and overflow
    shrink. The bound never scales with the input; each attempt budget is re-clamped to what is left,
    and no retry starts past the deadline. An explicit compaction.summarizationMaxDurationMs larger
    than the total still wins, clamped to the existing 30-minute ceiling. The trip classifies as
    summarization-timeout, so it recovers through the same checkpoint and surfaces in the same
    compaction end message.
  5. A message the user can act on. When the checkpoint is applied, the extension says so once:
    that a provider summary could not be completed, that a deterministic checkpoint was applied and
    older detail was dropped, that it is safe to continue, and what the provider reported. Every
    compaction message built from an error message is stripped of the senpi:no-turn-retry: prefix;
    the prefix stays on the error object the session-level retry-suppression predicates read.

One deliberate contract change in existing tests

Three characterizations expressed "a policy rejection surfaces loudly" only as error-message prose
(errorMessage: "request blocked by provider policy" with no stopDetails), which is exactly the
class this issue says must stop wedging the session. They are now bound to the real refusal signal
(stopDetails.type), which still fails closed and still surfaces loudly, and each gained a companion
case pinning the new recovery. No assertion was weakened or removed.

Relationship to #1735

PR #1735 (fix/astra-preprompt-compaction-recovery) touches deterministic-fallback.ts too, but a
different function: it adds a later-safe-boundary scan to the retained-suffix search and adds
pre_prompt to isRequiredCompactionFallbackReason in extension-wiring.ts. This PR does not touch
either, is branched from main rather than rebased onto #1735, and changes only the classification,
watchdog and message paths. The two are complementary: #1735 widens which routes may recover,
this PR widens which failures authorize a recovery at all - #1735's trigger is a classified idle
timeout, and a marker-bearing provider error still escaped it. Whichever lands first, the other
applies cleanly.

Verification

Run on a second machine in a disposable checkout, which was removed afterwards and verified gone.

RED (tests only, before the production change)

$ cd packages/coding-agent && bun run test -- test/compaction/summarization-provider-failure-recovery.test.ts test/compaction/summarization-total-budget.test.ts
$ vitest --run test/compaction/summarization-provider-failure-recovery.test.ts test/compaction/summarization-total-budget.test.ts

 RUN  v4.1.11 /private/tmp/discord7-1741/senpi/packages/coding-agent

xxxxxx·xxxxx

⎯⎯⎯⎯⎯⎯ Failed Tests 11 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  test/compaction/summarization-provider-failure-recovery.test.ts > summarization provider failure authorizes the deterministic fallback > classifies a marker-bearing credential failover error as a provider failure
AssertionError: expected undefined to be 'summarization-provider-failure' // Object.is equality

- Expected:
"summarization-provider-failure"

+ Received:
undefined

 ❯ test/compaction/summarization-provider-failure-recovery.test.ts:84:78
     82| describe("summarization provider failure authorizes the deterministic …
     83|  it("classifies a marker-bearing credential failover error as a provid…
     84|   expect(classifyRequiredCompactionFallbackFailure(markerBearingFailov…
       |                                                                              ^
     85|   // A pool that never committed output rethrows without the marker; i…
     86|   // as terminal for this summary, so it authorizes the same recovery.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/11]⎯

 FAIL  test/compaction/summarization-provider-failure-recovery.test.ts > summarization provider failure authorizes the deterministic fallback > classifies a non-transient summary request error as a provider failure
AssertionError: expected undefined to be 'summarization-provider-failure' // Object.is equality

- Expected:
"summarization-provider-failure"

+ Received:
undefined

 ❯ test/compaction/summarization-provider-failure-recovery.test.ts:102:106
    100|
    101|  it("classifies a non-transient summary request error as a provider fa…
    102|   expect(classifyRequiredCompactionFallbackFailure(new SummaryRequestE…
       |                                                                                                          ^
    103|    "summarization-provider-failure",
    104|   );

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/11]⎯

 FAIL  test/compaction/summarization-provider-failure-recovery.test.ts > summarization provider failure authorizes the deterministic fallback > applies a deterministic checkpoint when the blocking route's summary stream throws a marker error
AssertionError: expected { cancel: true, …(1) } to not have property "cancel"

- Expected:
undefined

+ Received:
true

 ❯ test/compaction/summarization-provider-failure-recovery.test.ts:150:22
    148|   // The wedge was `{ cancel: true }`: compaction never applied, so th…
    149|   // stayed over threshold and the next prompt repeated the identical …
    150|   expect(result).not.toHaveProperty("cancel");
       |                      ^
    151|   expect(result).toMatchObject({
    152|    compaction: {

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/11]⎯

 FAIL  test/compaction/summarization-provider-failure-recovery.test.ts > summarization provider failure authorizes the deterministic fallback > recovers the blocking route from a non-transient provider error stop
AssertionError: expected { cancel: true, …(1) } to not have property "cancel"

- Expected:
undefined

+ Received:
true

 ❯ test/compaction/summarization-provider-failure-recovery.test.ts:205:22
    203|
    204|   if (!result) throw new Error("Expected a compaction handler result");
    205|   expect(result).not.toHaveProperty("cancel");
       |                      ^
    206|   expect(result).toMatchObject({
    207|    compaction: { details: { failureKind: "summarization-provider-failu…

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/11]⎯

 FAIL  test/compaction/summarization-provider-failure-recovery.test.ts > summarization provider failure authorizes the deterministic fallback > tells the user plainly what happened without the internal retry-suppression marker
AssertionError: expected "vi.fn()" to be called 1 times, but got 0 times
 ❯ test/compaction/summarization-provider-failure-recovery.test.ts:238:18
    236|   // predicates read, and never reaches what the user is shown.
    237|   expect(failure.message.startsWith(TURN_RETRY_SUPPRESSION_PREFIX)).to…
    238|   expect(notify).toHaveBeenCalledTimes(1);
       |                  ^
    239|   const [message] = notify.mock.calls[0] as [string, string?];
    240|   expect(message).not.toContain(TURN_RETRY_SUPPRESSION_PREFIX);

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/11]⎯

[... 6 further failures: the user-visible message, both settlement bounds, and the whole total-budget file ...]


 Test Files  2 failed (2)
      Tests  11 failed | 1 passed (12)
   Start at  12:16:09
   Duration  17.29s (transform 24.24s, setup 49ms, import 30.81s, tests 121ms, environment 0ms)

error: script "test" exited with code 1

GREEN (after the production change, on the branch head)

$ cd packages/coding-agent && bun run test -- test/compaction/summarization-provider-failure-recovery.test.ts test/compaction/summarization-total-budget.test.ts
$ vitest --run test/compaction/summarization-provider-failure-recovery.test.ts test/compaction/summarization-total-budget.test.ts

 RUN  v4.1.11 /private/tmp/discord7-1741/senpi/packages/coding-agent

············

 Test Files  2 passed (2)
      Tests  12 passed (12)
   Start at  12:48:01
   Duration  3.25s (transform 4.02s, setup 134ms, import 5.78s, tests 20ms, environment 0ms)

The whole test/compaction directory is green on this branch:

$ cd packages/coding-agent && bun run test -- test/compaction
RUN  v4.1.11 /private/tmp/discord7-1741/senpi/packages/coding-agent

·····························································································································································································································································································································································································································································································································································································

 Test Files  77 passed (77)
      Tests  573 passed (573)
   Start at  12:48:04
   Duration  36.14s (transform 159.22s, setup 2.81s, import 355.15s, tests 42.30s, environment 9ms)

biome check --error-on-warnings over the changed compaction trees reports no errors and
tsc --noEmit is clean on the same checkout. test/suite/regressions plus the credential-failover
suites fail identically on this branch and on main in that environment (5 files / 3 tests: RPC
session grants, internal-socket mkdir, fswatch crash, multi-session theme init, cross-project
resume) - pre-existing and unrelated to compaction.

main moved while this was in progress, so origin/main is merged into the branch and the
verification above was re-run on the merged head. This PR is branched from main, not rebased onto
#1735.

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
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
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
…allback-authorization

# Conflicts:
#	packages/coding-agent/CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant