fix(cli): clear recovered provider errors

[MagMueller]

Summary

• clear a remembered provider error when a later assistant completion finishes successfully
• allow a non-interactive run to exit successfully after automatic compaction recovers
• cover a 413 overflow followed by compaction and recovered output

Test plan

• bun test test/cli/run/run-process.test.ts --test-name-pattern "exits 0 when compaction recovers a provider size error"
• bun typecheck from packages/opencode
• repository pre-push typecheck (16 packages)

---

Summary by cubic

Fixes CLI run error recovery so a recovered provider error no longer forces a failure. Non-interactive runs now exit 0 after automatic compaction successfully recovers from a size error.

• Bug Fixes
  • Clear a remembered provider error when an assistant completion finishes with finish and no error.
  • Allow non-interactive runs to exit 0 after auto-compaction recovers from a 413 provider size error.
  • Add a test covering 413 overflow → compaction → recovered output.

^{Written for commit 066b4b8. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

[Alezander9]

Reviewed and verified — this is the right fix at the right layer, and it's the first of the eight that I'd have written the same way. Merging. Three things worth adding to the record, one of which means the production symptom will not go away when this lands.

Root cause confirmed, and the codebase already contains the correct shape

processor.ts has two overflow paths:

// 507 — proactive: usage came back over the limit
if (!ctx.assistantMessage.summary && isOverflow({ ... })) {
  ctx.needsCompaction = true          // silent
}

// 641 — reactive: the provider already rejected the request (the 413)
ctx.needsCompaction = true
yield* events.publish(Session.Event.Error, { sessionID: ctx.sessionID, error })   // broadcasts
return

Same recovery, same next step, but one path announces a failure on the error channel and the other doesn't. The CLI then treats every session.error as terminal (error = error ? error + EOL + err : err → if (error) process.exitCode = 1), so the reactive path exits 1 on a run that fully recovered and produced correct output.

I considered deleting the 641 publish instead, since that would fix every consumer at once. Decided against it and I think your choice is right: the 413 genuinely happened, it has real latency and cost, and cloud's _friendly_provider_error() shows we do want provider errors visible. The defect isn't that the error is reported — it's that a consumer equated "an error occurred" with "the run failed." That's a consumer-side fix, which is what this is.

Verification

• The new test is a genuine regression pin, not a tautology. Reverted run.ts to 066b4b805~1 with the test in place: opencode: expected exit 0, got 1. Restored: pass.
• Full run-process.test.ts — 14/14 pass. This matters because you restructured the outer condition, and the > agent · model banner (previously gated by args.format !== "json" && toggles.get("start") !== true in the same if) is covered by four of those tests. The inner re-gate preserves it.
• bun typecheck from packages/opencode clean.
• Rebase status: clean against current main. Only SKILL.md moved under you (docs(browser): add CDP recovery boundaries #115), no run.ts overlap.

The part that isn't finished: cloud still fails the run

This fixes the exit code. The exit code is not what cloud reads.

# app.py:2179
if isinstance(event_data, dict) and _event_carries_error(event_data):
    seen_error = json.dumps(_extract_event_error(event_data))    # latched, never cleared
...
# app.py:2227
if rc == 0 and not seen_error:

emit("error", { error: props.error }) is untouched here — deliberately, and your test asserts it (events.some(e => e.type === "error") is true). That's the correct call for a JSON stream consumer. But it means stdout still carries {"type":"error",...}, _extract_event_error still matches it, seen_error latches, and the run is reported run.failed with rc: 0. A user watching cloud sees no change from this PR.

Worth noting the shape sitting immediately above seen_error in that same function:

# Finish reason of the most recent LLM call; a truncated call exits rc=0
# with no error event. Overwritten, not accumulated: a run that truncates
# early but finishes cleanly is a success.
last_stop_reason: str | None = None

That's exactly the reasoning this PR applies to error in the CLI — overwritten, not accumulated. seen_error, three lines away, does the opposite.

So the sequencing is: this PR is the precondition, not the fix. Before it, cloud couldn't simply trust rc, because rc was 1 on a recovered run — the latch was masking a real CLI bug. After it, rc == 0 means what it says, and the gate can become if rc == 0: while seen_error keeps its legitimate job of sourcing the message in the failure branch (_friendly_provider_error(seen_error)), rather than deciding the outcome.

I'd want that cloud change reasoned through rather than done reflexively — the question is whether any genuinely-failed run reaches rc == 0 with nothing but seen_error to catch it. Truncation is already handled separately via last_stop_reason + decide_terminal, and a missing result is caught by extract_result_from_db, so I think the answer is no. But that's the analysis that has to be written down before the gate changes.

One subtlety to be aware of

error = undefined fires on the first assistant message that finishes cleanly after the error — and on the compaction path that's the summary message, not the recovered turn. So the clear happens slightly earlier than "recovery succeeded."

I traced it and the outcome is still correct: every fatal continuation re-publishes Session.Event.Error and goes idle, including a second overflow (processor.ts:635 takes the fatal branch once ctx.assistantMessage.summary is set), so anything that actually fails after the clear re-sets error. No !info.summary guard needed. Recording it because the predicate reads as "the work recovered" and technically means "something later finished."

Also confirmed info.finish && !info.error is a sound success test rather than a coincidence: every site that sets finish = "error" sets error in the same statement block (processor.ts:636, compaction.ts:410, message-updater.ts:226).

Upstream

upstream/dev has this bug verbatim — identical if block at run.ts:703-713, identical publish at processor.ts:616. Our run.ts currently diverges from upstream by two branding strings and nothing else, so this is the first behavioral divergence in a hot upstream file, and every future sync pays for it.

Opening the upstream PR against anomalyco/opencode the same day this lands. Needs a memory/browsercode/EXCEPTIONS.md entry as a Yellow-zone edit, with "can we upstream this?" = yes, PR pending.

[Alezander9]

Superseding this with #125, which fixes the same bug at the producer. Full reasoning there; the short version of why we changed course after the earlier review approved this:

Your diagnosis was correct and your test caught the real regression — both are carried forward into #125 (fixture credited). What changed our mind is that the CLI is one of four consumers of session.error, and all four treat it as the run's outcome: your fix repaired the exit code but left the TUI error toast, the desktop OS notification, and cloud's seen_error stdout latch all still firing failure UX on recovered runs. Cloud in particular would have needed its own follow-up patch (app.py:2227 gates on not seen_error, so the production symptom survives an exit-0 fix).

Looking at the producer instead, the publish at processor.ts's auto-compact branch turned out to be internally inconsistent three ways: it's the only Session.Event.Error publisher that doesn't set message.error (durable state disagrees with the event), the only one not followed by idle or a throw, and nothing consumes it — compaction is triggered by the "compact" return value. It also breaks the codebase's own precedent: retried 429s are silent, and the proactive overflow check sets needsCompaction silently. #125 deletes that publish and moves the announcement to where failure is actually decided — compaction's "too large to compact" branch, which already writes the durable error but only got its event as a side effect of the halt re-entering during summary generation.

That makes every consumer correct as written: your run.ts latch, the toast, the notification, and cloud's gate — zero consumer changes, run.ts stays identical to upstream, and no clear-on-later-success state to reason about (the clearing in this PR fires on the summary message rather than the recovered turn, which worked out safe but took a full trace to prove).

One thing your PR got right that we kept as a hard requirement: the failed-recovery case must still exit 1. That only worked pre-#125 by accident (the halt re-entry during summary published the event), so #125 adds an explicit test pinning exit 1 with exactly one error event when compaction can't shrink the session.

The same patch is going upstream to anomalyco/opencode, where this bug exists verbatim — your consumer-side approach remains the right fallback there if they insist on keeping the blip event. Thanks for the report and the fixture; this one was a real production bug with a clean diagnosis.