fix(selfhost): stop retrying a CLI provider after it times out once - #3987
Merged
Conversation
runWorkersOpinion/runDualAiTieBreakJudgeCall retried every model up to 3 times regardless of failure type, including subscription_cli_timeout -- a non-transient signal that the same model will time out again on the same diff. Short-circuit remaining retries of THAT model on a timeout; the fallback model still gets its own full retry budget. Closes #3927
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
loopover-orb
Bot
added
the
gittensor:bug
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3987 +/- ##
=======================================
Coverage 93.70% 93.70%
=======================================
Files 373 373
Lines 35076 35079 +3
Branches 12863 12866 +3
=======================================
+ Hits 32868 32871 +3
Misses 1584 1584
Partials 624 624
🚀 New features to boost your workflow:
|
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…nd two subprocess leaks (#9476, #9479) (#9505) claude --output-format json buffers its whole response, so ANY run exceeding its effort timeout has produced zero stdout when the deadline lands -- which trips the first-output watchdog and throws claude_stalled_no_output rather than subscription_cli_timeout. The retry-break tested for the latter by strict string equality, so it never matched: every timed-out review burned all three attempts (3x180s at default effort, 3x600s at the top tier) before the fallback model was tried at all. With QUEUE_CONCURRENCY defaulting to 8 that parks the whole queue during a provider slowdown, and the per-provider circuit breaker needs three FULL-LENGTH failures before it trips. Matched by prefix, since these errors carry a detail suffix -- the strict equality is the original bug. subscription_cli_timeout was effectively unreachable for claude-code, making #3987's fix dead code on this deployment. child.on("error") catches spawn failures only; it never receives stdio stream errors. A CLI that exits before draining stdin (an unknown flag on an upgraded binary, an auth abort, an OOM kill) made the ~250KB stdin write fail with EPIPE on an emitter with no error listener, which Node escalates to an uncaught exception -> exit(1), taking down every in-flight queue job in the container. Per-call temp dirs were never removed. Every AI review minted one and, where repo review instructions are configured, wrote the composed system prompt into it -- so they accumulated on the container's writable overlay layer until recreation, leaving those instructions on disk indefinitely. Removed in the same finally that records CLI usage metrics, best-effort so a cleanup failure can never turn a completed review into a thrown error. The systemAppend test now reads the appended-prompt file inside the spawn stub rather than after the call returns, and additionally asserts the directory does not outlive the call -- which is both the new behaviour and a closer match to how the file is actually used.
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.
Summary
runWorkersOpinion/runDualAiTieBreakJudgeCallretried every model in the fallback chain up to 3 times regardless of failure type, includingsubscription_cli_timeout(a SIGKILL after the effort-based timeout on a self-hostclaude-code/codexsubprocess) -- a non-transient signal that the same model will time out again on the same diff.env.AIbinding, so this path never executes there.Scope
Closes #3927above.Validation
npm run typechecknpx vitest run test/unit/ai-review.test.ts test/unit/ai-review-advisory.test.ts-- 203/203 pass, including 2 new tests proving the timeout short-circuit and that transient errors still get full retries.Safety