From c66548027604af294a92a27c69945f7ef7d73060 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 05:34:45 -0700 Subject: [PATCH] fix(selfhost): validate AI reviewer-provider configuration and add a failure circuit breaker Closes #2540. Self-host's dual-AI review path resolved up to two reviewer slots from AI_PROVIDER with no distinctness check, so a config mistake (the same provider listed twice, e.g. a copy-paste artifact) silently collapsed "two independent reviewers reaching consensus" into "the same provider called twice" -- defeating dual-AI review and meaning one provider's outage/auth failure took down both slots at once. resolveAiReviewerPlan now throws a descriptive duplicate_ai_reviewer_provider error when the first two (distinctness-relevant) names match, mirroring assertNoLegacySharedAiEnv's existing fail-loud-at-boot pattern for other misconfigured env combinations. createChainAi also gained a per-provider circuit breaker: after 3 consecutive failures a provider is skipped (no network/CLI call at all) for a 5-minute cooldown, falling straight through to the next provider in the chain instead of repeating a doomed attempt on every single PR review during a sustained outage. In-process only, matching the module's existing global failure-streak pattern. Every provider circuit-open (no healthy fallback) throws a distinct all_ai_providers_circuit_open error instead of the misleading generic no_ai_providers default. New metrics (gittensory_ai_provider_circuit_total{provider,result}, gittensory_ai_review_inconclusive_total{mode,dual}) and two Prometheus alert rules give an operator visibility into a tripped provider and a spike in fail-closed HELD verdicts, which is often the correlated downstream symptom of the same outage. --- prometheus/rules/alerts.yml | 36 +++++ src/selfhost/ai.ts | 70 ++++++++- src/services/ai-review.ts | 5 + test/unit/ai-review.test.ts | 7 + test/unit/selfhost-ai.test.ts | 141 ++++++++++++++++++- test/unit/selfhost-grafana-dashboard.test.ts | 9 ++ 6 files changed, 263 insertions(+), 5 deletions(-) diff --git a/prometheus/rules/alerts.yml b/prometheus/rules/alerts.yml index 7d4f5bdd8b..d440beb663 100644 --- a/prometheus/rules/alerts.yml +++ b/prometheus/rules/alerts.yml @@ -369,3 +369,39 @@ groups: summary: "gittensory p95 request latency above 1s" description: "p95 HTTP request latency is {{ $value | printf \"%.2f\" }}s over the last 5m (sustained 10m), breaching the 1s SLO." runbook: "Check whether slowness is queue/DB/AI-bound: correlate with gittensory_queue_pending and Qdrant/AI latency. A rising p95 with flat error rate usually means a saturated dependency, not a bug." + + # ── AI review provider health (#2540) ─────────────────────────────────────── + - name: gittensory-ai + rules: + - alert: GittensoryAiProviderCircuitOpen + # A provider's per-provider circuit breaker (createChainAi) has been open (tripping repeated attempts, + # not just one) for a sustained window -- the provider is known-bad for this whole window, not a + # one-off blip. `increase(...) > 0` over the alert window plus `for: 15m` requires the breaker to + # still be actively tripping partway through, not a single trip that then recovered. + expr: increase(gittensory_ai_provider_circuit_total{result="tripped"}[15m]) > 0 + for: 15m + labels: + severity: warning + annotations: + summary: "gittensory AI provider {{ $labels.provider }} circuit breaker open" + description: "The {{ $labels.provider }} AI provider has tripped its circuit breaker and stayed open for over 15m -- reviews are falling through to the next configured provider (or failing if none remain)." + runbook: "Check the provider's credentials/quota/reachability (API key, CLI auth, network egress). gittensory_ai_provider_circuit_total{result=\"skipped\"} shows how many reviews are bypassing it. Clears automatically once a call succeeds." + + - alert: GittensoryAiReviewInconclusiveSpike + # A HELD (fail-closed, no usable AI output) verdict is not itself an error -- it's the SAFE outcome + # when review output can't be trusted. But a SUSTAINED spike means dual-AI review is providing + # materially less signal than configured, often correlated with a provider circuit trip above. + # `> 0` denominator guard avoids 0/0 = NaN when no block-mode reviews ran in the window. + expr: | + ( + sum(rate(gittensory_ai_review_inconclusive_total[15m])) + / + sum(rate(gittensory_ai_requests_total[15m])) > 0 + ) > 0.25 + for: 15m + labels: + severity: warning + annotations: + summary: "gittensory AI review inconclusive-verdict ratio above 25%" + description: "{{ $value | humanizePercentage }} of AI review activity resulted in an inconclusive (held) verdict over the last 15m (sustained 15m)." + runbook: "Correlate with GittensoryAiProviderCircuitOpen and selfhost_ai_providers_exhausted logs. A spike usually means one or more configured reviewers are down; check credentials and provider status." diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 091311a987..53435ff36b 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -634,21 +634,70 @@ export function markAiProviderUnhealthyAtBoot(): void { aiConsecutiveFailures = AI_UNHEALTHY_FAILURE_STREAK; } -/** Try each provider in order until one returns; if all throw, rethrow the last error so the caller degrades - * (AI summary → "unavailable"; the review still runs deterministically). The fallback chain is what makes a - * BYOK setup robust — e.g. AI_PROVIDER="anthropic,ollama" uses the API first and a local model if it's down. */ +// Per-provider circuit breaker (#2540): the readiness streak above only tracks "the WHOLE chain exhausted" -- +// it says nothing about ONE provider within a multi-provider chain (e.g. AI_PROVIDER="anthropic,ollama") being +// known-bad, so every single PR review still pays that provider's full failure latency (a slow API timeout or +// CLI hang) before falling through to the next, every time, for the whole outage. Trip a provider's breaker +// after a short run of CONSECUTIVE failures and skip it (no network/CLI call at all) for a cooldown window, +// falling straight through to the next provider in the chain -- degrading review latency/cost gracefully +// instead of repeating a doomed attempt on every review. In-process only (no persistence layer), matching this +// module's existing aiConsecutiveFailures streak. +const AI_PROVIDER_CIRCUIT_FAILURE_STREAK = 3; +const AI_PROVIDER_CIRCUIT_COOLDOWN_MS = 5 * 60_000; +type ProviderCircuitState = { consecutiveFailures: number; cooldownUntilMs: number }; +const providerCircuits = new Map(); + +function isProviderCircuitOpen(providerName: string, nowMs = Date.now()): boolean { + const state = providerCircuits.get(providerName); + return state !== undefined && state.cooldownUntilMs > nowMs; +} + +function recordProviderCircuitSuccess(providerName: string): void { + if (providerCircuits.delete(providerName)) { + incr("gittensory_ai_provider_circuit_total", { provider: providerName, result: "recovered" }); + } +} + +function recordProviderCircuitFailure(providerName: string, nowMs = Date.now()): void { + const state = providerCircuits.get(providerName) ?? { consecutiveFailures: 0, cooldownUntilMs: 0 }; + state.consecutiveFailures += 1; + if (state.consecutiveFailures >= AI_PROVIDER_CIRCUIT_FAILURE_STREAK) { + state.cooldownUntilMs = nowMs + AI_PROVIDER_CIRCUIT_COOLDOWN_MS; + incr("gittensory_ai_provider_circuit_total", { provider: providerName, result: "tripped" }); + } + providerCircuits.set(providerName, state); +} + +/** Test-only reset so circuit-breaker state from one test can't leak into the next (module-level Map). */ +export function resetAiProviderCircuitsForTest(): void { + providerCircuits.clear(); +} + +/** Try each provider in order until one returns; if all throw (or are circuit-open), rethrow the last real error + * so the caller degrades (AI summary → "unavailable"; the review still runs deterministically). The fallback + * chain is what makes a BYOK setup robust — e.g. AI_PROVIDER="anthropic,ollama" uses the API first and a local + * model if it's down. */ export function createChainAi(providers: Array<{ name: string; ai: SelfHostAi }>): SelfHostAi { return { async run(model, options) { let lastError: unknown = new Error("no_ai_providers"); + let attempted = false; const failures: Array<{ provider: string; error: string }> = []; for (const p of providers) { + if (isProviderCircuitOpen(p.name)) { + failures.push({ provider: p.name, error: "circuit_open: skipped after repeated recent failures" }); + incr("gittensory_ai_provider_circuit_total", { provider: p.name, result: "skipped" }); + continue; + } + attempted = true; try { const result = await runProviderWithOtel(p, model, options); aiConsecutiveFailures = 0; + recordProviderCircuitSuccess(p.name); return result; } catch (error) { lastError = error; + recordProviderCircuitFailure(p.name); failures.push({ provider: p.name, error: errorMessage(error) }); console.error(JSON.stringify({ level: "warn", event: "selfhost_ai_provider_failed_in_chain", provider: p.name, error: errorMessage(error) })); } @@ -665,6 +714,9 @@ export function createChainAi(providers: Array<{ name: string; ai: SelfHostAi }> error: errorMessage(lastError), }), ); + // Every provider was circuit-open (a real attempt never ran) -- surface a distinct, actionable error + // instead of the generic "no_ai_providers" default, which would misleadingly imply NOTHING was configured. + if (!attempted && providers.length > 0) throw new Error("all_ai_providers_circuit_open"); throw lastError instanceof Error ? lastError : new Error("all_ai_providers_failed"); }, }; @@ -798,6 +850,18 @@ export function resolveAiReviewerPlan( const names = resolveProviderNames(env); if (names.length === 0) return undefined; if (names.length === 1) return { reviewers: [{ model: names[0] as string }], combine: "single", onMerge: undefined }; + // #2540: a duplicate AI_PROVIDER entry feeding BOTH dual-review slots (e.g. "claude-code,claude-code" -- a + // copy-paste config mistake) would otherwise silently collapse "two independent reviewers reaching consensus" + // into "the same provider called twice", defeating dual-AI review entirely AND meaning a single provider's + // outage/auth failure takes down both slots at once. Fail loud at boot, mirroring + // assertNoLegacySharedAiEnv's pattern for other misconfigured env combinations, rather than silently + // degrading review quality. Only the first two names feed the two slots, so only THOSE need to differ -- + // a harmless third duplicate further down the fallback chain (unused here) is not this problem. + if (names[0] === names[1]) { + throw new Error( + `duplicate_ai_reviewer_provider: AI_PROVIDER lists "${names[0]}" for both dual-review slots -- configure two DISTINCT providers (e.g. AI_PROVIDER="claude-code,codex") or a single provider (dual review needs two).`, + ); + } const rawCombine = (env.AI_COMBINE ?? "").trim().toLowerCase() as CombineStrategy; const combine: CombineStrategy = COMBINE_STRATEGIES.has(rawCombine) ? rawCombine : "synthesis"; const rawOnMerge = (env.AI_ON_MERGE ?? "").trim().toLowerCase() as OnMerge; diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 22d04a3969..9c2ae27a94 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -25,6 +25,7 @@ import { sanitizePublicComment } from "../queue-intelligence"; import { defangReviewInput } from "../review/safety"; import { convergedFeatureActive } from "../review/feature-activation"; import { labelSelfHostReviewerModels, labelSelfHostReviewerNames, resolveConfiguredProviderNames } from "../selfhost/ai-config"; +import { incr } from "../selfhost/metrics"; import { errorMessage } from "../utils/json"; import type { ReviewProfile } from "../signals/focus-manifest"; @@ -1228,6 +1229,10 @@ export async function runGittensoryAiReview( reviewDiagnostics.some((diagnostic) => diagnostic.status === "unparseable_output")) ) inconclusive = true; + // #2540: observability for a fail-closed HOLD (no gate finding either way, but no usable AI output to certify + // clean) -- a correlated spike here alongside gittensory_ai_provider_circuit_total{result="tripped"} is the + // "dual-AI review is silently degrading" signal the alert rule watches for. + if (inconclusive) incr("gittensory_ai_review_inconclusive_total", { mode: input.mode, dual: String(dual) }); const advisoryNotes = reviewsForNotes.length > 0 ? (composeAdvisoryNotes(reviewsForNotes) ?? composeFallbackAdvisoryNotes(fallbackNotes)) diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 8d03225f0f..cf86618f59 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -6,6 +6,7 @@ import { type GittensoryAiReviewInput, } from "../../src/services/ai-review"; import { createTestEnv } from "../helpers/d1"; +import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; const { parseModelReview, @@ -87,6 +88,7 @@ const baseInput: GittensoryAiReviewInput = { afterEach(() => { vi.unstubAllGlobals(); + resetMetrics(); }); describe("runGittensoryAiReview gating", () => { @@ -474,6 +476,9 @@ describe("runGittensoryAiReview block mode (consensus)", () => { expect(result.consensusDefect).toBeNull(); expect(result.inconclusive).toBe(true); // FAIL-CLOSED: a missing second opinion holds the PR, never passes it expect(result.advisoryNotes).not.toBeNull(); // notes still come from the one parseable opinion + // #2540: an inconclusive HOLD is observable, so a sustained spike (often correlated with a provider outage) + // is alertable independent of any one PR's outcome. + expect(await renderMetrics()).toContain('gittensory_ai_review_inconclusive_total{dual="true",mode="block"} 1'); }); it("a clean dual review is NOT inconclusive (both models parsed, neither blocks → passes)", async () => { @@ -486,6 +491,8 @@ describe("runGittensoryAiReview block mode (consensus)", () => { }); expect(result.status === "ok" && result.consensusDefect).toBeNull(); expect(result.status === "ok" && result.inconclusive).toBe(false); + // #2540: no inconclusive metric on the happy path — the counter must stay silent, not just correct-valued. + expect(await renderMetrics()).not.toContain("gittensory_ai_review_inconclusive_total"); }); it("block mode with BYOK: provider writes the advisory, the free Workers-AI pair drives consensus", async () => { diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 8124c90fc4..9c6d0b1f32 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -2,7 +2,7 @@ import { chmodSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, subscriptionCliEnv } from "../../src/selfhost/ai"; +import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, resetAiProviderCircuitsForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, subscriptionCliEnv } from "../../src/selfhost/ai"; import { labelSelfHostReviewerModel } from "../../src/selfhost/ai-config"; import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; @@ -67,6 +67,7 @@ afterEach(() => { vi.unstubAllGlobals(); resetMetrics(); resetAiProviderHealthForTest(); + resetAiProviderCircuitsForTest(); }); type SpawnResult = { stdout: string; code: number | null; stderr?: string }; @@ -195,9 +196,123 @@ describe("createChainAi (fallback)", () => { }); }); +describe("createChainAi — per-provider circuit breaker (#2540)", () => { + function countingProvider(name: string, behavior: "fail" | "succeed") { + let calls = 0; + return { + provider: { name, ai: { run: async () => { calls += 1; if (behavior === "fail") throw new Error(`${name}_down`); return { response: `from ${name}` }; } } }, + calls: () => calls, + }; + } + + it("does NOT open the circuit before the failure streak threshold — every attempt still reaches the provider", async () => { + const { provider: a, calls } = countingProvider("a", "fail"); + for (let i = 0; i < 2; i += 1) { + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + } + expect(calls()).toBe(2); // both attempts reached the real provider — breaker not yet tripped + }); + + it("opens the circuit after 3 consecutive failures and SKIPS the provider on the next chain run — falls through to a healthy sibling with zero calls to the broken one", async () => { + const { provider: a, calls: aCalls } = countingProvider("a", "fail"); + const { provider: b, calls: bCalls } = countingProvider("b", "succeed"); + for (let i = 0; i < 3; i += 1) { + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + } + expect(aCalls()).toBe(3); + + const result = await createChainAi([a, b]).run("m", { prompt: "x" }); + + expect(result.response).toBe("from b"); + expect(aCalls()).toBe(3); // UNCHANGED — the 4th "attempt" against `a` never happened, it was skipped + expect(bCalls()).toBe(1); + }); + + it("REGRESSION: every provider circuit-open (no healthy fallback) throws a distinct, actionable error instead of the generic no_ai_providers default", async () => { + const { provider: a } = countingProvider("a", "fail"); + for (let i = 0; i < 3; i += 1) { + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + } + + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/all_ai_providers_circuit_open/); + }); + + it("a success resets that provider's failure streak — it does not carry over into a later, separate failure run", async () => { + let behavior: "fail" | "succeed" = "fail"; + const flaky = { name: "a", ai: { run: async () => { if (behavior === "fail") throw new Error("a_down"); return { response: "ok" }; } } }; + // Two failures (below the 3-streak threshold), then one success. + await expect(createChainAi([flaky]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + await expect(createChainAi([flaky]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + behavior = "succeed"; + await expect(createChainAi([flaky]).run("m", { prompt: "x" })).resolves.toEqual({ response: "ok" }); + behavior = "fail"; + // Two MORE failures after the reset -- still below the streak threshold, so the circuit must NOT be open yet. + await expect(createChainAi([flaky]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + await expect(createChainAi([flaky]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + // A THIRD (this run's) consecutive failure -- would be the 3rd since the reset, tripping fresh, not carried over. + behavior = "fail"; + await expect(createChainAi([flaky]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + expect(await renderMetrics()).toContain('gittensory_ai_provider_circuit_total{provider="a",result="tripped"} 1'); + }); + + it("REGRESSION: a cooldown-expired provider is tried again (skipped=false) rather than staying open forever", async () => { + vi.useFakeTimers(); + try { + const { provider: a, calls: aCalls } = countingProvider("a", "fail"); + for (let i = 0; i < 3; i += 1) { + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + } + expect(aCalls()).toBe(3); + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/all_ai_providers_circuit_open/); + expect(aCalls()).toBe(3); // still skipped, circuit open + + vi.advanceTimersByTime(5 * 60_000 + 1); // past the cooldown window + + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(/a_down/); + expect(aCalls()).toBe(4); // cooldown expired -- the provider was actually called again + } finally { + vi.useRealTimers(); + } + }); + + it("emits skipped/tripped/recovered metrics with the provider name as a label", async () => { + const { provider: a } = countingProvider("a", "fail"); + const { provider: b } = countingProvider("b", "succeed"); + for (let i = 0; i < 3; i += 1) { + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(); + } + await createChainAi([a, b]).run("m", { prompt: "x" }); // a: skipped; b: succeeds + + const metrics = await renderMetrics(); + expect(metrics).toContain('gittensory_ai_provider_circuit_total{provider="a",result="tripped"} 1'); + expect(metrics).toContain('gittensory_ai_provider_circuit_total{provider="a",result="skipped"} 1'); + }); + + it("a provider that recovers after cooldown clears its circuit and emits a recovered metric", async () => { + vi.useFakeTimers(); + try { + const { provider: a } = countingProvider("a", "fail"); + for (let i = 0; i < 3; i += 1) { + await expect(createChainAi([a]).run("m", { prompt: "x" })).rejects.toThrow(); + } + vi.advanceTimersByTime(5 * 60_000 + 1); + const recovered = { name: "a", ai: { run: async () => ({ response: "back up" }) } }; + await expect(createChainAi([recovered]).run("m", { prompt: "x" })).resolves.toEqual({ response: "back up" }); + expect(await renderMetrics()).toContain('gittensory_ai_provider_circuit_total{provider="a",result="recovered"} 1'); + } finally { + vi.useRealTimers(); + } + }); +}); + describe("isAiProviderHealthy (readiness streak, #2497)", () => { const failing = { name: "a", ai: { run: async () => { throw new Error("down"); } } }; - const working = { name: "a", ai: { run: async () => ({ response: "ok" }) } }; + // A DISTINCT provider name from `failing` (#2540): the per-provider circuit breaker persists failure state by + // name across separate createChainAi(...) calls (that's the point -- a cooldown that resets every call would + // do nothing), so reusing "a" here would itself trip "a"'s circuit and this success would never actually run. + // This block tests the GLOBAL isAiProviderHealthy() streak, which is provider-name-agnostic — any success + // resets it, so a different provider name preserves that intent without colliding with the circuit breaker. + const working = { name: "b", ai: { run: async () => ({ response: "ok" }) } }; it("reports healthy before any AI call has happened", () => { expect(isAiProviderHealthy()).toBe(true); @@ -381,6 +496,28 @@ describe("resolveProviderNames + resolveAiReviewerPlan (#dual-ai-combiner)", () it("labels explicit provider:model reviewer ids without consulting env defaults", () => { expect(labelSelfHostReviewerModel(" CODEX:gpt-5.5 ", { CODEX_AI_MODEL: "ignored" })).toBe("codex:gpt-5.5"); }); + + describe("duplicate reviewer-slot validation (#2540)", () => { + it("throws when the SAME provider would fill both dual-review slots", () => { + expect(() => resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,claude-code" })).toThrow(/duplicate_ai_reviewer_provider/); + }); + + it("throws regardless of casing/whitespace (dedup happens on the normalized, lowercased name)", () => { + expect(() => resolveAiReviewerPlan({ AI_PROVIDER: " Claude-Code , claude-code " })).toThrow(/duplicate_ai_reviewer_provider/); + }); + + it("does NOT throw when the duplicate is only a THIRD entry beyond the two slots actually used", () => { + expect(resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex,claude-code" })).toEqual({ + reviewers: [{ model: "claude-code" }, { model: "codex" }], + combine: "synthesis", + onMerge: undefined, + }); + }); + + it("does NOT throw for two genuinely distinct providers (regression: the happy path stays byte-identical)", () => { + expect(() => resolveAiReviewerPlan({ AI_PROVIDER: "claude-code,codex" })).not.toThrow(); + }); + }); }); describe("branch coverage — defaults + edge inputs", () => { diff --git a/test/unit/selfhost-grafana-dashboard.test.ts b/test/unit/selfhost-grafana-dashboard.test.ts index 6f58b78021..9e92873af5 100644 --- a/test/unit/selfhost-grafana-dashboard.test.ts +++ b/test/unit/selfhost-grafana-dashboard.test.ts @@ -146,6 +146,15 @@ describe("Gittensory Self-Host Grafana dashboard", () => { expect(alerts).toContain("alert: GittensoryBackupStale"); expect(alerts).toContain('time() - gittensory_backup_latest_timestamp_seconds{target=~"postgres|sqlite"} > 93600'); }); + + it("ships AI provider circuit-breaker and inconclusive-verdict alerts (#2540)", () => { + const alerts = readFileSync(selfhostAlertsPath, "utf8"); + + expect(alerts).toContain("alert: GittensoryAiProviderCircuitOpen"); + expect(alerts).toContain('increase(gittensory_ai_provider_circuit_total{result="tripped"}[15m]) > 0'); + expect(alerts).toContain("alert: GittensoryAiReviewInconclusiveSpike"); + expect(alerts).toContain("sum(rate(gittensory_ai_review_inconclusive_total[15m]))"); + }); }); describe("maintainer Reviews & PRs Grafana dashboard", () => {