fix: graceful handling of transient API connection errors - #47
Merged
Conversation
A lost API connection (laptop asleep / no internet) during generation was silently swallowed: connection errors were unclassified, a single blip could falsely mark a phase completed, and a total failure still reported "complete". - classify CONNECTION_ERROR in model_routing via a narrow pattern list - abort a connection-lost phase before the checkpoint write so a resume re-runs it instead of skipping it - surface all-workspace failures (generation + deploy) so the run fails cleanly with a "run retry_generation once you're back online" message; partial success still continues (fail only when every workspace fails) Connection retries remain the Claude SDK default (no custom wait-loop). Backend: 2016 passed / 36 skipped.
Remove the multi-line comment on the CONNECTION_ERROR abort branch to match the terser style of the sibling TOOL_CALL_FAILURE case, and drop _raise_if_all_workspaces_failed (and its test) — surfacing an all-workspace failure through the orchestrator is out of scope for this PR.
awrobel-gd
approved these changes
Jul 15, 2026
awrobel-gd
left a comment
Contributor
There was a problem hiding this comment.
Approved with optional comment
mkonopelski-gd
added a commit
that referenced
this pull request
Jul 23, 2026
) test_connection_error_aborts_without_checkpoint (added in #47) passed a Mock generation_session_service without configuring db_adapter. Because execute_all_phases derives `db_adapter = generation_session_service.db_adapter` and calls raise_if_cancelled() before each phase, the auto-Mock adapter made raise_if_cancelled await a non-awaitable Mock (get_generation_session), raising TypeError before the connection-error abort under test was ever reached. Set svc.db_adapter = None so raise_if_cancelled hits its documented DB-less no-op path (matching test/state/test_cancellation.py::test_noop_when_db_adapter_is_none), leaving the test to exercise only the checkpoint-abort behavior it asserts. Code under test is unchanged — this was a test-mocking gap at the seam with the pre-existing cancellation check, not a regression in error handling. Backend: 2040 passed / 36 skipped. mcp_server: 671 passed.
mkonopelski-gd
added a commit
that referenced
this pull request
Jul 27, 2026
…#53) * feat: surface agent errors; progress-aware resume Agent/phase-level failures (SDK crashes, resume give-ups, model fallbacks, workspace aborts) existed only in per-phase log files — nothing durable, nothing in /status, nothing in the TUI. Incident est-e2dfad7b42d7/ws-03-1-phase12: three transient socket errors ~2h apart exhausted the fixed 2-resume budget despite each attempt saving real work; the workspace stalled 3 days and a later retry finished the "failed" phase in 4 minutes. Durable event channel: - agent_error_events (bounded, 200) on the session doc, written only via new state-machine methods (Commandment VII); best-effort recorder wired through TelemetryContext like the totals handler, so deep agent code records events with no signature changes and never raises. - /status ships the newest 50 events + per-workspace agent_state badge; TUI renders a yellow "Agent warnings" panel (mirror of the Error panel), bare RETRYING/ABORTED badges + warning markers on workspace rows, a per-workspace warnings block on the drill-in (model switches pinned first), crash/retry lines in the live feed (new "error" stream kind), a full Events screen (e), and a check_status warnings clause. Unified resume loop (retries are the main logic, not an appendix): - one attempt loop (coding + validator per attempt) with a single classify_error-driven except handler; ResumeGeneration is the sole source for loop condition, log lines, events, and the TUI badge. - every crash waits on one schedule (1/3/5/10/30/60 min); saved progress (new git commit OR >=20 tool uses) resets the schedule; six consecutive zero-progress crashes give up; no total cap. Validator budget stays 2. A dead validator now retries instead of silently marking the phase complete, and the give-up warning can no longer fire on success. Minimum-survivors gate (un-drops what #47 deferred, strengthened): - codegen fails the run when survivors < min(2, total) — variance estimates need two samples; a total wipeout previously completed GREEN with an empty estimate. Deploy keeps the all-failed rule. Aggregated message names per-workspace causes and the retry path; model-caused failures recommend changing the model in Settings. Also fixes the Recent-activity panel tailing macOS .DS_Store garbage (logs live nested; now rglob("*.log") only). * fix(tui): hide the noisy Recent activity panel The earlier `.DS_Store`/nested-log discovery fix made this panel actually locate the agent log — which exposed that it renders a raw tail of DEBUG-level records (full `UserMessage(content=[ToolResultBlock(...)])` dumps with escaped newlines): a wall of text that isn't useful at a glance. Hide it from the dashboard for now. `_activity_panel` and `tui/activity.py` are kept intact (and still point at the correct nested log dir), so a proper filtered/prettified view can be revived by re-appending the panel in build_dashboard. * fix(tui): drop the agent-warnings list from the session view The session dashboard should show only the run's outcome — the estimate when complete, or the fatal Error — not a running list of per-workspace warnings. The "Agent warnings" panel violated that: it grew a scrolling log right under Workspaces (and, with Recent activity now hidden, became the prominent panel there). Remove it from build_dashboard. Nothing is lost: the Workspaces panel already carries bare RETRYING/ABORTED/⚠ markers at a glance, and the full per-event detail lives in the workspace drill-in warnings block and the Events screen (`e`). `_event_row_text` stays (shared by both of those). * fix(retry): clear per-workspace agent_state badge on retry Manual/auto retry (reset_for_retry) reset retry_count/error but left the transient per-workspace agent_state (RETRYING/ABORTED) untouched. Because that badge is cleared only as a side-effect of the resume loop's crash→backoff path, a retried run left stale ABORTED/RETRYING markers on any workspace that didn't happen to re-enter that path — so after pressing "r", one workspace's badge cleared while its siblings kept a phantom ABORTED. reset_for_retry now strips agent_state from every workspace_phases entry (preserving last_completed_phase / planning_data so the retry still resumes from the right phase), giving all workspaces a clean slate. * fix(resume): surface is_error on crash give-up so #47 abort still fires agent_query_with_resume derived its final AgentResult from last_returned regardless of stop_reason. Because the crash branch only populates last_returned when it is still None, a single clean-but-incomplete attempt before a sustained outage left last_returned holding that stale success (is_error=False). On a NO_PROGRESS_BUDGET (or TOOL_CALL_FAILURE) give-up, final returned it verbatim, so execute_all_phases took the non-error branch: it checkpointed the incomplete phase as complete and never hit the CONNECTION_ERROR abort path (#47). On retry the phase was silently skipped. Return an is_error=True result carrying last_error for crash-based give-ups; COMPLETED and VALIDATOR_BUDGET (the anti-stuck contract) still return the last coding result as-is. Adds two regression tests covering the mixed clean-incomplete -> outage and clean-incomplete -> tool-call sequences. * refactor(claude_code): rename _run_attempt to _code_and_validate The helper runs a coding query then a validator query; the new name says what it does (coding pass + completion check) rather than the generic 'attempt'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A lost API connection during generation (laptop asleep / no internet) was silently swallowed: connection errors went unclassified, and a single blip could falsely mark a phase
completedthat never actually ran.Fix
CONNECTION_ERRORinmodel_routing(narrow pattern list).retry_generationre-runs it instead of skipping it.Connection retries are left to the Claude SDK default (no custom wait-loop, no new config).
Tests:
test_classify_error.py::TestConnectionError,test_execute_all_phases_connection_error.py— backend 2011 passed / 36 skipped. No new lint/type errors.