fix(task): auto-retry transient "Connection closed mid-response" like chat (MUL-4910)#5565
Merged
Conversation
Claude Code's "API Error: Connection closed mid-response" is a transient network cut. In unattended issue runs it fell through to agent_error.unknown / process_failure — neither in retryableReasons — so the task terminated. Interactive chat only appeared resilient because the CLI's own in-process retry usually recovers first; there is no chat-specific retry in Multica. Both paths share the same finalizeStreamResult -> Classify -> retryableReasons pipeline. - classify: route "connection closed" / "mid-response" to agent_error.provider_network, before the exit-status rule so the "exited with error: exit status N" variant also lands here. - retry: add provider_network to retryableReasons. It is resume-safe, so the retry child inherits the session and continues the truncated conversation instead of restarting. Tests cover the new classification (incl. exit-status variant) and the retry/resume flags. Note: mirror the substrings into the MUL-1949 offline backfill SQL to keep in-flight and historical taxonomies aligned. Co-authored-by: multica-agent <github@multica.ai>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…L-4910) Follow-up to the immediate-retry fix: make the connection-closed retry a three-tier schedule — first run + immediate retry + one retry deferred ~5s — so a blip that survives the immediate retry gets a short cooldown before the final attempt instead of firing back-to-back. Reuses the existing deferred/fire_at primitive (the comment-routing escalation mechanism) rather than adding new infrastructure: CreateRetryTask gains an optional fire_at; when set, the child is inserted 'deferred' and the existing PromoteDueDeferredTasksForRuntime sweeper — already run promote-first on every claim poll — flips it to 'queued' at fire_at. No migration, no claim-query change, no daemon change. - retryAttemptCeiling: raise provider_network's ceiling to 3 (other reasons keep max_attempts=2); applied in both retryEligible and MaybeRetryFailedTask's budget pre-check so the primary and sweeper paths agree. - retryDelayForAttempt: only provider_network's final attempt is deferred (5s); every other retry — including provider_network's first — stays immediate. - FailTask + MaybeRetryFailedTask pass fire_at and skip the queued broadcast/notify for a deferred child (promotion emits them at fire time). Timing: the deferred child fires on the first claim poll at/after fire_at, so >= 5s; on an otherwise-idle runtime it can stretch to the poll interval — the same behaviour deferred escalations already have. Tests: pure schedule/eligibility coverage (TestProviderNetworkRetrySchedule) plus a DB test asserting fire_at controls deferred-vs-queued, attempt=3, and resume-safety. Co-authored-by: multica-agent <github@multica.ai>
…disable (MUL-4910) Addresses the pre-merge review must-fix: retryAttemptCeiling raised provider_network to 3 unconditionally, which (1) overrode the max_attempts=1 "auto-retry disabled" contract and (2) persisted a self-contradictory child (attempt=3, max_attempts=2) that leaks to the task API, so a naive attempt < max_attempts consumer would misjudge the budget. - retryAttemptCeiling now returns taskMaxAttempts unchanged when it is <= 1 (disabled stays disabled) and only ever WIDENS otherwise (max(col, 3) for provider_network) — a higher configured budget is kept. - CreateRetryTask takes an optional max_attempts; FailTask and MaybeRetryFailedTask write the reason-aware effective ceiling into the child so the whole retry chain self-describes (attempt=3, max_attempts=3). NULL inherits the parent column, so non-provider_network reasons are unchanged. Tests: - pure: ceiling widens to 3, keeps a higher budget, and never revives a disabled (max_attempts=1) task; eligibility rejects the disabled case. - DB (end-to-end FailTask): default budget → deferred final child at attempt=3/max_attempts=3; first failure → immediate child; max_attempts=1 → no child. Plus CreateRetryTask persists the passed budget / inherits on NULL. Co-authored-by: multica-agent <github@multica.ai>
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
1 participant
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 (MUL-4910)
Claude Code agents running issue tasks frequently terminate mid-run with:
In chat the same transient error is invisible — it "retries once and recovers." There is no chat-specific retry in Multica: chat and issue tasks share the same
finalizeStreamResult → taskfailure.Classify → retryableReasonspipeline. The string is emitted by the Claude Code CLI, which has its own in-process retry; chat just happens to recover before the error surfaces, while longer unattended issue runs surface it and — because it classified to a non-retryableagent_error.*— terminated with no platform retry.What this PR does
Makes the platform recover from the transient cut itself, on a three-tier schedule:
A 4th failure stops and reports normally. All retries resume the session, so the truncated conversation continues rather than restarting.
Implementation
taskfailure/classify.go): routeconnection closed/mid-responseinto the provider-network bucket →agent_error.provider_network, before theexit statusrule so the exit-status variant lands there too.service/task.go): addagent_error.provider_networktoretryableReasons. It is resume-safe, soCreateRetryTaskinherits the session and continues the conversation.deferredprimitive — no new infra:CreateRetryTaskgains an optionalfire_at; when set, the child is inserteddeferredand the existingPromoteDueDeferredTasksForRuntimesweeper (already run promote-first on every claim poll) flips it toqueuedat fire time. No migration, no claim-query change, no daemon change.retryAttemptCeiling): provider_network's ceiling is 3 (max(col, 3)— a higher configured budget is kept), and the effective ceiling is written into the child'smax_attemptsso the chain self-describes (attempt=3, max_attempts=3), never a contradictoryattempt=3, max_attempts=2.max_attempts=1still means "auto-retry disabled" and is never revived.Behaviour notes for the merger
fire_at, so it is ≥5s; on an otherwise-idle runtime it can stretch toward the poll interval — the same behaviour deferred escalations already have. A tighter (scheduled-wake) variant is a possible follow-up.agent_error.provider_networkbucket three-tier (existingdial tcp/ DNS /connection refusedmembers too, not only the newly-addedconnection closed/mid-response). This is consistent with the "transient provider network error is recoverable" boundary, but is a deliberately wider blast radius than the literal string.provider_server_error/capacity_or_rate_limit) — retrying rate-limits needs backoff/Retry-Aftersemantics; left as a follow-up.Tests
pkg/taskfailure/classify_test.go: plain +exit status-wrapped variants classify toprovider_network.service/task_complete_race_test.go(TestProviderNetworkRetrySchedule): the tier/delay/eligibility schedule, incl. the ceiling widening to 3, keeping a higher budget, and never reviving amax_attempts=1task.service/retry_deferred_test.go: DB tests —CreateRetryTaskdeferred-vs-queued + persisted budget, and an end-to-endFailTasktest asserting default budget → deferredattempt=3/max_attempts=3, first failure → immediate child, andmax_attempts=1→ no child.Verified locally (build/vet + service & taskfailure suites against a clean migrated DB); GitHub CI green.
Follow-up noted separately
Mirror the
connection closed/mid-responsesubstrings into the MUL-1949 offline backfill SQL so historical rows share the taxonomy (flagged in a code comment + commit).