fix(selfhost): validate AI reviewer-provider configuration and add a failure circuit breaker - #2626
Conversation
…failure circuit breaker Self-host's dual-AI review resolves up to two reviewer slots from AI_PROVIDER with no distinctness check. Listing the same provider twice (a copy-paste mistake) silently collapses "two independent reviewers" into "one provider called twice" -- routeProviders' name-keyed map collapses duplicates to a single runtime instance, so a provider outage takes down both slots at once. resolveAiReviewerPlan now throws loudly at boot when the two slots the plan actually uses are identical, matching the existing fail-loud pattern used for other self-host misconfigurations. Also adds a per-provider circuit breaker: after 3 consecutive failures a provider enters a 60s cooldown during which further calls are skipped fast (no real HTTP/CLI cost) instead of retrying a known-broken provider from a cold state on every single PR review. Shared by both the fallback chain and the dual-review named-slot path via their one common call site. New Prometheus counters (gittensory_ai_review_inconclusive_total, gittensory_ai_provider_failures_total, gittensory_ai_provider_circuit_open_total) plus two alert rules for a sustained inconclusive-verdict spike or an open circuit breaker.
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-02 19:39:48 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 1 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2626 +/- ##
=======================================
Coverage 96.05% 96.05%
=======================================
Files 234 234
Lines 26228 26247 +19
Branches 9517 9522 +5
=======================================
+ Hits 25192 25211 +19
Misses 425 425
Partials 611 611
🚀 New features to boost your workflow:
|
Advances #2540
Problem
Self-host's dual-AI review resolves up to two reviewer slots from
AI_PROVIDERwith no deduplication check. Listing the same provider twice collapses "two independent reviewers reaching consensus" into "one provider called twice in parallel" —routeProviders' name-keyed map collapses duplicate provider names to a single runtime instance, so a single provider's outage takes down both slots. Separately, there was no circuit breaker: a provider already known to be failing paid the full cost of a fresh attempt (a real HTTP call or CLI subprocess spawn) on every single PR review during a sustained outage.Changes
resolveAiReviewerPlan(src/selfhost/ai.ts) now throws loudly at boot when the two dual-review slots it would actually use are the same provider, matching the existing fail-loud pattern already used for other self-host misconfigurations (assertNoLegacySharedAiEnv). A third+ duplicate further down an otherwise-fine provider list does not trigger it — only the two slots actually consumed matter.runProviderWithOtelchokepoint (used by both the fallback chain and the dual-review named-slot path): after 3 consecutive failures a provider enters a 60s cooldown; further calls during that window are skipped fast without touching the real provider. A success resets the count; a real failure rethrows unchanged so existing fallback/chain behavior is untouched.gittensory_ai_review_inconclusive_total{mode},gittensory_ai_provider_failures_total{provider},gittensory_ai_provider_circuit_open_total{provider}.GittensoryAiReviewInconclusiveSpikeandGittensoryAiProviderCircuitOpeninprometheus/rules/alerts.yml, following the existing absolute-increase-threshold convention.Test plan
npm run typecheckcleannpm run test:coverage(unsharded) green, 100% branch coverage on all new/changed linesnpm run test:cifull local gate greennpm audit --audit-level=moderate— 0 vulnerabilities