fix(selfhost): only escalate the final AI-provider retry attempt to Sentry (#5046) - #5048
Conversation
…entry (#5046) logSelfHostAiProviderFailed fired console.error on every subscription- CLI attempt, not just the final one -- contradicting ai-review.ts's own documented "per-attempt=warn, exhausted=error" policy (#26). GITTENSORY-K (claude_stalled_no_output) hit 2077+ events: pulled raw events sharing the same jobId/trace_id at attempt 0, 1, 2, confirming one review's 3x-retry-per-model loop was independently escalating each attempt to Sentry instead of just the exhausted outcome. Thread finalAttempt through AiRunOptions (mirroring the existing attempt/jobId correlation fields) so the provider's own log can tell a retried attempt (quiet) from a genuinely exhausted one (loud). runWorkersOpinion, the dual-AI tie-break judge, the issue planner, and the AI slop advisory all compute it as "last attempt of the last model" and pass it through. Two of those callers (planner.ts, ai-slop.ts) have no logging of their own -- their retry loops rely entirely on this log for visibility, so a blanket downgrade would have silenced them; unset stays loud by default for exactly that reason.
|
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 #5048 +/- ##
=======================================
Coverage 94.16% 94.16%
=======================================
Files 463 463
Lines 39429 39435 +6
Branches 14384 14390 +6
=======================================
+ Hits 37128 37134 +6
Misses 1646 1646
Partials 655 655
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 10:47:49 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 2 non-blocking
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
GITTENSORY-K(selfhost_ai_provider_failed: claude_stalled_no_output) is still climbing (2077+ events) despite Fix fix(review): claude-code subscription-CLI hangs/times out repeatedly (4,030 Sentry events, ongoing) #4994/PR fix(review): add a stalled-no-output fast-fail deadline for claude-code #5013 already shipping and deploying — confirmed the deployed self-host release (gittensory-selfhost@6b52b207) is a descendant of that fix commit, so this is not a deploy-lag artifact.jobId/trace_id, withattempt: 0,attempt: 1, etc. — one single review job, retrying internally, with every individual attempt independently reaching Sentry as its ownconsole.error.runWorkersOpinion(src/services/ai-review.ts) retries up to 3× per model × up to 2 models (6 attempts max) — and its own per-attempt log (ai_review_provider_attempt_failed) is deliberatelyconsole.warn, with an explicit comment citing feat(agent): serve stale decision packs with freshness marker and background rebuild #26: "Per-attempt logs are warn (noisy retries, skipped by the central Sentry forwarder); the exhausted summary is error." ButlogSelfHostAiProviderFailed(src/selfhost/ai.ts), called from insidecreateClaudeCodeAi/createCodexAi's own catch blocks on every throw, ignored that policy entirely and escalated every attempt — the two layers disagreed, and the lower one always wins since Sentry sees it either way. Fix fix(review): claude-code subscription-CLI hangs/times out repeatedly (4,030 Sentry events, ongoing) #4994 made each individual attempt fail faster (good, real, needed) but never touched this separate over-escalation.src/review/planner.tsandsrc/services/ai-slop.tshave their own retry loops (2× and 3× respectively) with zero logging of their own — they rely entirely on this same low-level log for any Sentry visibility into a persistent failure. A blanket downgrade would have made them silently blind.finalAttempt?: booleanthroughAiRunOptions, mirroring the existingattempt/jobIdcorrelation fields (#codex-timeout-fields— same optional, purely-observational pattern).logSelfHostAiProviderFailedlogs atwarnonly whenfinalAttempt === false; unset (any single-shot caller) ortruestayserror, so nothing loses visibility by default.runWorkersOpinion, the dual-AI tie-break judge call,runPlannerModel, andrunWorkersSlopOpinionall compute it asattempt === maxAttempts - 1 && modelIndex === models.length - 1and pass it through.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 #5046).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 --coveragefor the five touched test files (453 tests total:selfhost-ai.test.ts,ai-review.test.ts,ai-review-advisory.test.ts,planner.test.ts,ai-slop.test.ts) and confirmed via lcov that every changed line and both branches of every newfinalAttemptexpression are covered — theai-review.ts/planner.ts/ai-slop.tsboolean expressions were already naturally exercised (both truthy and falsy) by the existing retry/exhaustion test suites; three new dedicated regression tests inselfhost-ai.test.tspin the actual behavior change (warn vs error vs unset-stays-loud).actionlint/test:workers/build:mcp/test:mcp-pack/ui:openapi:check/ui:lint/ui:typecheck/ui:build/npm audit: not run — this change touches only existing internal AI-provider plumbing (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
Follow-up from this session's earlier 13-bug Sentry triage batch (#4994–#5006) — found while re-checking the live dashboard after that batch shipped. The
claude_stalled_no_outputevents themselves are expected/correct (Fix #4994 working as designed: fast-detecting a real, occasional external CLI hang); this PR addresses the separate, over-eager escalation of every retry within that detection.