From 2db4675dc2068b6176b2d1ef6f99d5d0eed2f855 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:58:37 -0700 Subject: [PATCH 1/2] feat(observability): distinguish a rejected credential from an exhausted quota A 401 and a 429 from an AI provider need opposite first responses -- rotate the credential, versus wait for the quota window -- but both surfaced identically as an opaque error string under a single reason-less counter. On the hosted box a burst of claude_code_error_429 read as a dead token and prompted a rotation that could not have helped. classifyProviderFailure maps a failure to credential_invalid / quota_exhausted / timeout / not_configured / other, matching on the structured error shapes this module itself throws rather than free-text provider prose. Anything unrecognised stays 'other': a confidently wrong label sends an operator to the wrong runbook. Exposed as a reason label on a NEW counter rather than a new label on loopover_ai_provider_failures_total, which shipped alert rules and dashboards already query, plus a reason field on the failure log. Two alert rules encode the split: a rejected credential is critical and its runbook points at the rotation path, an exhausted quota is a warning whose runbook says not to rotate. Closes #9549 --- prometheus/rules/alerts.yml | 31 ++++++++++++ src/selfhost/ai.ts | 33 +++++++++++++ test/unit/selfhost-ai.test.ts | 88 ++++++++++++++++++++++++++++++++++- 3 files changed, 151 insertions(+), 1 deletion(-) diff --git a/prometheus/rules/alerts.yml b/prometheus/rules/alerts.yml index acbb3fc144..e226d53dda 100644 --- a/prometheus/rules/alerts.yml +++ b/prometheus/rules/alerts.yml @@ -604,6 +604,37 @@ groups: description: "Provider {{ $labels.provider }} has failed repeatedly and its circuit breaker is skipping calls fast during its cooldown (sustained 5m)." runbook: "Check that provider's credentials/reachability (CLI auth for claude-code/codex, or the configured API key/base URL for HTTP providers) via loopover_ai_provider_failures_total{provider=\"...\"} and recent selfhost_ai_provider_failed logs." + # The next two rules exist to separate the only two provider failures whose FIRST RESPONSE differs + # (#9549). Both previously surfaced as an opaque `claude_code_error_NNN` under the same counter, so a + # quota exhaustion looked exactly like a dead credential -- and rotating a perfectly good token is the + # natural, wasted next move. Distinct severities encode the difference: one needs a human NOW, the + # other resolves itself when the window rolls over. + - alert: LoopoverAiProviderCredentialInvalid + # A 401/403 from a provider: the credential is rejected outright. This does NOT self-heal -- every + # review degrades to the fallback provider (or fails) until someone rotates it. Any occurrence in + # 15m is actionable; `for: 5m` only filters a single blip during an in-flight rotation. + expr: increase(loopover_ai_provider_failure_reason_total{reason="credential_invalid"}[15m]) > 0 + for: 5m + labels: + severity: critical + annotations: + summary: "loopover AI provider {{ $labels.provider }} credential is being rejected" + description: "Provider {{ $labels.provider }} returned {{ $value | printf \"%.0f\" }} auth failure(s) in 15m (sustained 5m). The credential is invalid or expired — this will not recover on its own." + runbook: "Rotate the credential: `./scripts/rotate-secret.sh claude_code_oauth_token` (or the loopover_admin_rotate_secret MCP tool). No restart is needed — the token is re-read per AI call. Confirm recovery via loopover_ai_provider_failure_reason_total{reason=\"credential_invalid\"} going flat." + + - alert: LoopoverAiProviderQuotaExhausted + # A 429: the credential is fine, the plan's limit is spent. Rotating it changes NOTHING, so this is a + # warning and the runbook says so explicitly. Sustained 15m rather than any-occurrence, because a + # short burst against a rate limit is normal and self-correcting. + expr: increase(loopover_ai_provider_failure_reason_total{reason="quota_exhausted"}[15m]) > 0 + for: 15m + labels: + severity: warning + annotations: + summary: "loopover AI provider {{ $labels.provider }} is out of quota" + description: "Provider {{ $labels.provider }} returned {{ $value | printf \"%.0f\" }} quota/rate-limit rejection(s) in 15m, sustained for 15m. The credential is valid; the plan's limit is spent." + runbook: "Do NOT rotate the credential — a 429 is a quota signal and a new token on the same account behaves identically. Wait for the window to roll over, raise the plan/limit, or configure a fallback in AI_PROVIDER so reviews degrade instead of failing." + # ── Ops anomaly scan (review burst / review failure burst, #ops-anomaly-metric) ──── - name: loopover-ops-anomalies rules: diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index d0e1dd5d6e..69afc6366d 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -997,6 +997,32 @@ function errorMessage(error: unknown, knownSecrets: readonly string[] = []): str return redactSecrets(message, knownSecrets).slice(0, 500); } +/** Why a provider call failed, at the granularity an operator actually acts on (#9549). + * `credential_invalid` → rotate the credential. `quota_exhausted` → wait, or raise the plan/limit. + * Conflating the two is the whole problem: both surface today as an opaque `claude_code_error_NNN`, so a + * 429 (a quota signal that no rotation can clear) is indistinguishable from a 401 (a dead credential) + * without grepping raw logs -- and rotating a token that was never the problem is the natural next move. */ +export type ProviderFailureReason = "credential_invalid" | "quota_exhausted" | "timeout" | "not_configured" | "other"; + +/** Classify a provider failure from its error. Deliberately matches on the STRUCTURED error shapes this + * module itself throws (`claude_code_error_`, `codex_error_`, the auth/timeout sentinels) + * rather than free-text provider prose, which is not a stable contract. Anything unrecognised stays + * `other` -- a wrong confident label is worse than an honest unknown, because it would send an operator + * to the wrong runbook. */ +export function classifyProviderFailure(error: unknown): ProviderFailureReason { + const message = error instanceof Error ? error.message : String(error ?? ""); + // HTTP status carried by the CLI's own structured error envelope, e.g. `claude_code_error_401`. + const status = /(?:^|[^0-9])(?:error_)(\d{3})\b/.exec(message)?.[1]; + if (status === "401" || status === "403") return "credential_invalid"; + if (status === "429") return "quota_exhausted"; + if (/_no_oauth_token\b|_auth_not_configured\b/.test(message)) return "not_configured"; + // The HTTP providers surface an auth failure as a plain status line rather than a `*_error_NNN` code. + if (/\b(?:401|403)\b/.test(message) && /unauthor|forbidden|invalid[_ ]api[_ ]key|authentication/i.test(message)) return "credential_invalid"; + if (/\b429\b/.test(message) || /rate[_ ]limit|quota|too many requests/i.test(message)) return "quota_exhausted"; + if (/timeout\b|_timed_out\b|stalled_no_output\b/.test(message)) return "timeout"; + return "other"; +} + function logSelfHostAiProviderFailed(input: { provider: string; model: string; @@ -1018,6 +1044,11 @@ function logSelfHostAiProviderFailed(input: { // a single-shot caller that never sets this field keeps today's always-loud behavior. const level = input.finalAttempt === false ? "warn" : "error"; const log = level === "warn" ? console.warn : console.error; + const reason = classifyProviderFailure(input.error); + // A separate counter rather than a new label on loopover_ai_provider_failures_total: that series is + // already referenced by shipped alert rules and dashboards, and silently multiplying it into one series + // per reason changes what those existing queries return. This is additive and breaks nothing. + incr("loopover_ai_provider_failure_reason_total", { provider: input.provider, reason }); log( JSON.stringify({ level, @@ -1030,6 +1061,8 @@ function logSelfHostAiProviderFailed(input: { repoFullName: input.repoFullName, pullNumber: input.pullNumber, attempt: input.attempt, + // Greppable alongside the metric: `reason` answers "rotate or wait?" without decoding the raw error. + reason, error: errorMessage(input.error, input.knownSecrets), }), ); diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index b23c521e59..e7b2862dc8 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -18,7 +18,7 @@ const posthogMocks = vi.hoisted(() => { }); vi.mock("posthog-node", () => ({ PostHog: posthogMocks.PostHog })); -import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, codexErrorFromStdout, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, providerNameFromBaseUrl, resetAiProviderCircuitBreakerForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveClaudeFirstOutputTimeoutMs, resolveCodexAuthPath, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveCodexFirstOutputTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, shouldWarnRagEmbedUnavailable, subscriptionCliEnv, withAdvisoryAiEnv, withAiGenerationCapture, __selfHostAiInternals } from "../../src/selfhost/ai"; +import { assertNoLegacySharedAiEnv, buildProvider, classifyProviderFailure, claudeErrorStatus, codexErrorFromStdout, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, providerNameFromBaseUrl, resetAiProviderCircuitBreakerForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveClaudeFirstOutputTimeoutMs, resolveCodexAuthPath, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveCodexFirstOutputTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, shouldWarnRagEmbedUnavailable, subscriptionCliEnv, withAdvisoryAiEnv, withAiGenerationCapture, __selfHostAiInternals } from "../../src/selfhost/ai"; import { labelSelfHostReviewerModel, labelSelfHostReviewerModels } from "../../src/selfhost/ai-config"; import { setFileSourcedSecrets } from "../../src/selfhost/file-sourced-secrets"; import { setProviderCredentialResolver } from "../../src/selfhost/provider-credential-registry"; @@ -2635,3 +2635,89 @@ describe("resolveClaudeOauthToken (#9543 — rotate the credential without recre await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN_FILE: "/nonexistent/nope" }, spawn).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_no_oauth_token/); }); }); + +describe("classifyProviderFailure (#9549 — rotate or wait?)", () => { + it("classifies a rejected credential from the CLI's structured envelope", () => { + expect(classifyProviderFailure(new Error("claude_code_error_401"))).toBe("credential_invalid"); + expect(classifyProviderFailure(new Error("claude_code_error_403"))).toBe("credential_invalid"); + expect(classifyProviderFailure(new Error("codex_error_401"))).toBe("credential_invalid"); + }); + + it("REGRESSION: classifies a 429 as quota, NOT as a credential problem", () => { + // The production incident this exists for: a burst of claude_code_error_429 read as "the token is + // dead", when rotating it could not possibly have helped. + expect(classifyProviderFailure(new Error("claude_code_error_429"))).toBe("quota_exhausted"); + }); + + it("classifies a missing credential distinctly from a rejected one", () => { + // Different first response: nothing to rotate, something to CONFIGURE. + expect(classifyProviderFailure(new Error("claude_code_no_oauth_token"))).toBe("not_configured"); + expect(classifyProviderFailure(new Error("codex_auth_not_configured: /root/.codex/auth.json not found"))).toBe("not_configured"); + }); + + it("classifies timeouts", () => { + expect(classifyProviderFailure(new Error("subscription_cli_timeout"))).toBe("timeout"); + expect(classifyProviderFailure(new Error("claude_stalled_no_output: no stdout within firstOutputTimeoutMs"))).toBe("timeout"); + }); + + it("classifies HTTP providers, which report auth failures as prose rather than an error_NNN code", () => { + expect(classifyProviderFailure(new Error("anthropic 401: invalid api key"))).toBe("credential_invalid"); + expect(classifyProviderFailure(new Error("HTTP 403 Forbidden"))).toBe("credential_invalid"); + expect(classifyProviderFailure(new Error("openai 429 Too Many Requests"))).toBe("quota_exhausted"); + expect(classifyProviderFailure(new Error("rate limit exceeded for this organization"))).toBe("quota_exhausted"); + }); + + it("INVARIANT: an unrecognised failure stays 'other' rather than being confidently mislabelled", () => { + // A wrong label sends an operator to the wrong runbook, which is worse than an honest unknown. + expect(classifyProviderFailure(new Error("claude_code_exit_1: something unexpected"))).toBe("other"); + expect(classifyProviderFailure(new Error("ECONNREFUSED"))).toBe("other"); + expect(classifyProviderFailure(new Error("claude_code_error_500"))).toBe("other"); + expect(classifyProviderFailure(new Error(""))).toBe("other"); + }); + + it("handles a non-Error and a nullish rejection without throwing", () => { + expect(classifyProviderFailure("claude_code_error_429")).toBe("quota_exhausted"); + expect(classifyProviderFailure(undefined)).toBe("other"); + expect(classifyProviderFailure(null)).toBe("other"); + }); + + it("does not read a status out of an unrelated number in the message", () => { + // `error_` is required before the digits, so a PR number or byte count never masquerades as a status. + expect(classifyProviderFailure(new Error("failed after 401 bytes of output"))).toBe("other"); + }); +}); + +describe("provider failure reason metric + log field (#9549)", () => { + afterEach(() => { + resetMetrics(); + }); + + it("counts the reason separately from the existing failures counter, and logs it", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const quota: StubSpawn = async () => ({ stdout: JSON.stringify({ is_error: true, api_error_status: 429 }), code: 1 }); + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, quota).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_error_429/); + + expect(await renderMetrics()).toMatch(/loopover_ai_provider_failure_reason_total\{[^}]*reason="quota_exhausted"[^}]*\} 1/); + const logged = JSON.parse(errorSpy.mock.calls.at(-1)![0] as string) as { reason: string; event: string }; + expect(logged.event).toBe("selfhost_ai_provider_failed"); + expect(logged.reason).toBe("quota_exhausted"); + errorSpy.mockRestore(); + }); + + it("labels an auth failure as credential_invalid, so the two are distinguishable in metrics", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const unauthorized: StubSpawn = async () => ({ stdout: JSON.stringify({ is_error: true, api_error_status: 401 }), code: 1 }); + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, unauthorized).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_error_401/); + expect(await renderMetrics()).toMatch(/loopover_ai_provider_failure_reason_total\{[^}]*reason="credential_invalid"[^}]*\} 1/); + errorSpy.mockRestore(); + }); + + it("never leaks the credential into the reason metric's labels", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const token = "sk-ant-oat01-should-never-appear"; + const unauthorized: StubSpawn = async () => ({ stdout: JSON.stringify({ is_error: true, api_error_status: 401 }), code: 1 }); + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: token }, unauthorized).run("m", { prompt: "x" })).rejects.toThrow(); + expect(await renderMetrics()).not.toContain(token); + errorSpy.mockRestore(); + }); +}); From 9244d4d90019fa6cf75de659729d3d080703ca8b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 01:32:35 -0700 Subject: [PATCH 2/2] fix(observability): register the new failure-reason metric and correct the credential runbook --- prometheus/rules/alerts.yml | 2 +- src/selfhost/metrics.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/prometheus/rules/alerts.yml b/prometheus/rules/alerts.yml index e226d53dda..cb2e8b15c7 100644 --- a/prometheus/rules/alerts.yml +++ b/prometheus/rules/alerts.yml @@ -620,7 +620,7 @@ groups: annotations: summary: "loopover AI provider {{ $labels.provider }} credential is being rejected" description: "Provider {{ $labels.provider }} returned {{ $value | printf \"%.0f\" }} auth failure(s) in 15m (sustained 5m). The credential is invalid or expired — this will not recover on its own." - runbook: "Rotate the credential: `./scripts/rotate-secret.sh claude_code_oauth_token` (or the loopover_admin_rotate_secret MCP tool). No restart is needed — the token is re-read per AI call. Confirm recovery via loopover_ai_provider_failure_reason_total{reason=\"credential_invalid\"} going flat." + runbook: "Rotate the credential by writing the new value into its secret file (e.g. /run/secrets/claude_code_oauth_token) as a BARE value — no comments, no trailing prose. No restart is needed: resolveClaudeOauthToken (#9543) re-reads the file on every AI call, and an empty or unreadable file falls back to the last known good value rather than dropping a working credential mid-rewrite. Confirm recovery by watching the credential_invalid series in loopover_ai_provider_failure_reason_total go flat." - alert: LoopoverAiProviderQuotaExhausted # A 429: the credential is fine, the plan's limit is spent. Rotating it changes NOTHING, so this is a diff --git a/src/selfhost/metrics.ts b/src/selfhost/metrics.ts index 4d7cc99d1d..206ff96ca1 100644 --- a/src/selfhost/metrics.ts +++ b/src/selfhost/metrics.ts @@ -120,6 +120,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [ ["loopover_ai_total_tokens_total", { help: "AI provider total tokens observed.", type: "counter" }], ["loopover_ai_provider_circuit_open_total", { help: "AI provider circuit-open events.", type: "counter" }], ["loopover_ai_provider_failures_total", { help: "AI provider failures by provider.", type: "counter" }], + ["loopover_ai_provider_failure_reason_total", { help: "AI provider failures by provider AND classified reason (credential_invalid | quota_exhausted | other) — a rejected credential needs a rotation, an exhausted quota needs waiting, and loopover_ai_provider_failures_total alone cannot tell them apart.", type: "counter" }], ["loopover_ai_provider_request_duration_seconds", { help: "AI provider request duration in seconds, by provider and request kind.", type: "histogram" }], ["loopover_ai_provider_request_errors_total", { help: "AI provider request errors, by provider and request kind (excludes expected embedding-routing fallbacks).", type: "counter" }], ["loopover_ai_review_cache_hit_total", { help: "AI review cache hits.", type: "counter" }],