fix(opencode): publish overflow error when recovery is abandoned, not attempted

Merged
AL
4mo ago
+70-1

Problem

From cloud v4 production (reported via #123): a long run's context exceeds the provider limit, the provider rejects the request with HTTP 413, auto-compaction kicks in, the retry succeeds, the agent finishes the work — and the run is reported as failed. The CLI exits 1, the TUI shows an error toast, the desktop app fires an OS push notification, and cloud's worker (which tails --format json stdout) marks the run run.failed.

Root cause

processor.ts halt, auto-compact branch:

ctx.needsCompaction = true
yield* events.publish(Session.Event.Error, { sessionID, error })
return

This publish is the outlier among all Session.Event.Error producers, on three counts:

  1. Event contradicts state. It's the only publisher that does not set message.error. The durable record says "no error"; only the ephemeral event claims one.
  2. Event contradicts lifecycle. Every other sessionful publish is followed by idle or a throw. This one continues working toward the same goal.
  3. The event does nothing. Compaction is triggered by the "compact" return value; nothing consumes this event to make recovery happen.

And it breaks the codebase's own precedent for recoverable provider errors: retried 429/5xx never publish (retry.ts contains zero publishes), and the proactive overflow check (processor.ts:507) sets needsCompaction silently. Only the reactive path announced itself — on the channel all four consumers read as "the run's outcome is failure."

Fix

Move the announcement from overflow occurred to recovery abandoned.

  • processor.ts — delete the publish in the auto-compact branch. A 413 the system is about to compact-and-retry is the same grade as a retried 429.
  • compaction.ts — the result === "compact" branch ("too large to compact even after stripping media") is where overflow actually becomes an outcome. It already writes the durable failure (message.error, finish = "error") but published nothing — its error event previously arrived only as a side effect of the halt re-entering during summary generation. Publish there, with its clearer message.

Net contract: session.error with a sessionID fires iff the outcome is affected. No consumer changes needed anywhere — the CLI's existing latch, the TUI toast, the app notification, and cloud's seen_error gate all become correct as written.

Relationship to #123

#123 fixed the same symptom consumer-side: run.ts cleared its latched error when a later assistant message finished cleanly. That worked for the CLI exit code, but taught one of four consumers to distrust the channel — the TUI toast, app notification, and cloud's stdout latch would all still fire failure UX on recovered runs. It also required stateful clear-on-later-success logic (with a subtle interaction: the clear actually fires on the summary message, not the recovered turn) and opened a behavioral divergence in run.ts, which is otherwise identical to upstream here.

This PR supersedes it at the producer. The recovered-413 test fixture is Magnus's from #123, credited — flipped to assert the stronger property (exit 0 and no error event on the stream).

Tests

Two tests in run-process.test.ts, pinning both directions of the move:

  • 413 → compact → retry succeeds: exit 0, no error event, recovered output present.
  • 413 → compaction itself overflows: exit 1, exactly one error event, carrying "too large to compact". This guards the regression that a bare deletion would cause — the failed-compaction path only got its error event via the halt re-entry, so deleting the publish without adding the compaction.ts one would make failed recovery exit 0 silently.

Verified both fail with the source change stashed and the tests kept:

  • recovered case: expected exit 0, got 1
  • abandoned case: expect(errors.length).toBe(1) fails (pre-fix the stream carries two overflow events and neither has the compaction message)

Full test/cli/run/run-process.test.ts: 15/15. bun typecheck clean. (test/session/compaction.test.ts has 2 failures and processor-effect.test.ts 15, identical on clean main — pre-existing, unrelated.)

Behavior matrix

ScenarioBeforeAfter
413 → compact → retry succeedsexit 1, toast, notification, orchestrator failureexit 0; recovery visible via the compaction message and Event.Compacted
413 → compaction also overflowsexit 1 (event emitted from the halt re-entry, raw 413 text)exit 1, single event with the clearer "too large to compact" message
Retried 429/5xxsilentunchanged
Fatal provider error / content filterexit 1unchanged
Overflow with compaction.auto: falseexit 1 (fatal branch)unchanged

Zone

Yellow — edits packages/opencode/src/session/{processor,compaction}.ts, both of which already carry Yellow-zone exceptions. EXCEPTIONS.md entry added maintainer-side. The bug is verbatim in anomalyco/opencode dev (processor.ts:616); an upstream PR with this patch is being opened in parallel, and this exception retires when it merges and syncs back.

Supersedes #123.


Summary by cubic

Publish overflow errors only when recovery is abandoned. Recoverable 413s (auto-compaction then retry) no longer emit Session.Event.Error, so successful runs report success across CLI/TUI/app/cloud.

  • Bug Fixes
    • packages/opencode/src/session/processor.ts: removed error publish in the auto-compact path; only set ctx.needsCompaction.
    • packages/opencode/src/session/compaction.ts: when result === "compact" (cannot shrink), set durable message.error and finish = "error", then publish Session.Event.Error with a clear "too large to compact" message.
    • Behavior: recovered overflow exits 0 with no error event; unrecoverable overflow exits 1 with a single error event.
    • Tests: added two cases in test/cli/run/run-process.test.ts to pin both paths.
    • Relates to #123: supersedes the consumer-side workaround by fixing at the source; no consumer changes needed.