fix(selfhost): stop killing legitimate slow claude-code reviews (#5053) - #5055
Conversation
#4994 added a 30s "first output" fast-fail deadline to claude-code, copying codex's own mechanism -- but codex's comment explains why THAT one is safe ("real JSONL progress from codex --json always lands on stdout"); claude-code's --output-format json is documented by `claude --help` as a "single result": fully buffered, not streamed. Confirmed live: a realistic 274KB/effort:high prompt took 116s to complete successfully with zero stdout bytes the entire time, then the full response arrived at once. Every review slower than 30s was being killed and mislabeled claude_stalled_no_output. Combined with the per-provider circuit breaker (opens after 3 consecutive failures) and a self-host box running AI_PROVIDER= claude-code with no fallback, this caused a total AI-review outage: every attempt failed deterministically, keeping the circuit open fleet-wide. resolveClaudeFirstOutputTimeoutMs's default/ceiling now make the fast-fail window equal the full timeoutMs by default (the call site's own Math.min(this, timeoutMs - 1) does the clamping) -- matching pre-#4994 behavior exactly. An operator can still opt into a shorter, riskier window via CLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MS. Codex's own mechanism is untouched -- its streaming premise still holds.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5055 +/- ##
=======================================
Coverage 94.13% 94.13%
=======================================
Files 465 465
Lines 39542 39542
Branches 14431 14431
=======================================
Hits 37222 37222
Misses 1664 1664
Partials 656 656
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-11 11:06:30 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 2 non-blocking
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Summary
claude-code, mirroringcodex's existing mechanism. Codex's own code comment explains why that one is safe: "real JSONL progress fromcodex --jsonalways lands on stdout." I never verified the equivalent claim for claude-code before copying the pattern — it doesn't hold.claude --helpdocuments--output-format json(what this codebase uses) as returning a "single result" — fully buffered, not streamed. Confirmed live on the box: ran the exact CLI invocation the app makes with a realistic 274KB prompt ateffort:high— it took 116 seconds to complete successfully, producing zero stdout bytes for the entire run, then the full 9KB response arrived at once. A 30-second (or even 120-second) "no output yet" signal cannot distinguish that from a genuine hang for this CLI mode — it's a coin flip that fix(review): claude-code subscription-CLI hangs/times out repeatedly (4,030 Sentry events, ongoing) #4994 mis-set badly.AI_PROVIDER=claude-codewith no fallback provider configured, andsrc/selfhost/ai.ts's per-provider circuit breaker opens after 3 consecutive failures (60s cooldown). Once every review started failing deterministically at the 30s mark, the circuit tripped repeatedly and stayed open, blocking essentially all AI reviews fleet-wide — a total outage, not just noise.Fix
resolveClaudeFirstOutputTimeoutMs's default and ceiling now matchresolveCliTimeoutFrom's own outer clamp (30 minutes) — since the call site already doesMath.min(resolveClaudeFirstOutputTimeoutMs(...), timeoutMs - 1), this means the fast-fail window equals the real timeout budget by default, exactly the pre-#4994 behavior.claude_stalled_no_outputnow only fires when nothing arrived for the entire configured budget — a genuine hang. An operator can still opt into a shorter, riskier window viaCLAUDE_AI_FIRST_OUTPUT_TIMEOUT_MSif they understand the tradeoff.codex's identical-looking mechanism is untouched — its streaming premise still holds, confirmed by its own existing code comment.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #5053).Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocallynpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
test:coverage(full unsharded): not run end-to-end — ran scopedvitest --coveragefortest/unit/selfhost-ai.test.ts(157 tests) and confirmed via lcov that both branches of the changed function are covered. Also confirmed against the realclaudeCLI on the affected box (not just unit tests) given the severity.actionlint/test:workers/build:mcp/test:mcp-pack/ui:openapi:check/ui:lint/ui:typecheck/ui:build/npm audit: not run — this change touches onlysrc/selfhost/ai.ts(existing internal timeout resolution, no new API/schema/binding/dependency surface) and its tests; no workflow, MCP, UI, or dependency-manifest surface changed.Safety
UI Evidencesection. (N/A.)Notes
This is a fix for a bug my own earlier fix in this session (#4994 / PR #5013) introduced. Needs to be merged and deployed to edge-nl-01 urgently — it's currently the live production configuration causing the outage.