fix(runtime): retry transport interruptions after HTTP 2xx - #5663
sunsunsun-java wants to merge 1 commit into
Conversation
Recognize bounded response-body transport causes without losing successful HTTP metadata. Preserve existing retry budgets and replay barriers, and cover SDK wrapping, durable tool results, cancellation, and continuation safety. Fixes apache#5656 Generated-by: Maka (gpt-6-astra)
hqhq1025
left a comment
There was a problem hiding this comment.
I found no substantiated P0–P3 issue in this head. The runtime classifier recognizes a narrow set of transport-interruption causes even when an SDK error reports HTTP 2xx, while excluding cancellation and nested non-2xx errors. The existing retry path retains its budget, cancellation, and tool-side-effect guards. Focused and affected local tests passed (301/301 and 392/392); a local HTTP server that sent 200 and then broke the response produced the expected retryable SDK cause chain.
The current head has only a successful label check. Its CI run is action_required with no test job, so merge readiness is not established. The original production incident did not preserve the error cause, and I did not test a real provider, Desktop E2E, or the full monorepo. No schema migration is involved.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Summary
Fixes #5656
An HTTP 200 response only confirms that the headers arrived; reading its body can still fail. Maka currently treats an SDK-wrapped response-body disconnect as
unknownand declines retry, even when its cause identifies a socket reset.This only addresses Goal 1 from the issue investigation. It does not change failure attribution, retry budgets, or Desktop presentation. The original incident did not retain its underlying cause, so this fixes the independently reproduced classification gap rather than claiming to prove that incident's exact root cause.
Verification
Rebased onto
apache:mainatbd8661fand reran:npm run build— passed.npm run lint— passed.npm run format:check— passed.npm run typecheck— passed.npx --no-install knip --workspace apps/desktop— passed.npx --no-install knip --workspace packages/ui— passed.git diff --check— passed.The new positive recovery tests failed before the production fix. Negative controls cover parsing/certificate errors, explicit HTTP/provider failures, cancellation, bounded cause inspection, tool activity, and continuation metadata. The backend tests verify that a durable tool effect occurs once, including when subsequent model attempts recover or exhaust their retry budget.
Additional local HTTP check: a server sent HTTP 200 plus a partial body, then disconnected. The real SDK produced:
Affected-suite command (after building)
Not run: the entire monorepo test suite, Desktop E2E, or live model-provider verification. No packaged Desktop application was installed or replaced.
AI use
Select exactly one:
Tool(s) and scope: Maka (gpt-6-astra) investigated the reported failure, authored the implementation and tests, performed automated review and local verification, and prepared this PR. The commit includes
Generated-by: Maka (gpt-6-astra).This is an automated submission by Maka at the contributor's request. Automated review is not an independent human approval.
Checklist
Does this PR entail a change in behavior?