feat(observability): distinguish a rejected credential from an exhausted quota

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 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
+        # 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:

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_<status>`, `codex_error_<status>`, 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),
     }),
   );

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" }],

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();
+  });
+});