Skip to content

Commit 6e892da

Browse files
committed
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
1 parent 8751b9a commit 6e892da

3 files changed

Lines changed: 151 additions & 1 deletion

File tree

prometheus/rules/alerts.yml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,37 @@ groups:
604604
description: "Provider {{ $labels.provider }} has failed repeatedly and its circuit breaker is skipping calls fast during its cooldown (sustained 5m)."
605605
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."
606606

607+
# The next two rules exist to separate the only two provider failures whose FIRST RESPONSE differs
608+
# (#9549). Both previously surfaced as an opaque `claude_code_error_NNN` under the same counter, so a
609+
# quota exhaustion looked exactly like a dead credential -- and rotating a perfectly good token is the
610+
# natural, wasted next move. Distinct severities encode the difference: one needs a human NOW, the
611+
# other resolves itself when the window rolls over.
612+
- alert: LoopoverAiProviderCredentialInvalid
613+
# A 401/403 from a provider: the credential is rejected outright. This does NOT self-heal -- every
614+
# review degrades to the fallback provider (or fails) until someone rotates it. Any occurrence in
615+
# 15m is actionable; `for: 5m` only filters a single blip during an in-flight rotation.
616+
expr: increase(loopover_ai_provider_failure_reason_total{reason="credential_invalid"}[15m]) > 0
617+
for: 5m
618+
labels:
619+
severity: critical
620+
annotations:
621+
summary: "loopover AI provider {{ $labels.provider }} credential is being rejected"
622+
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."
623+
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."
624+
625+
- alert: LoopoverAiProviderQuotaExhausted
626+
# A 429: the credential is fine, the plan's limit is spent. Rotating it changes NOTHING, so this is a
627+
# warning and the runbook says so explicitly. Sustained 15m rather than any-occurrence, because a
628+
# short burst against a rate limit is normal and self-correcting.
629+
expr: increase(loopover_ai_provider_failure_reason_total{reason="quota_exhausted"}[15m]) > 0
630+
for: 15m
631+
labels:
632+
severity: warning
633+
annotations:
634+
summary: "loopover AI provider {{ $labels.provider }} is out of quota"
635+
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."
636+
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."
637+
607638
# ── Ops anomaly scan (review burst / review failure burst, #ops-anomaly-metric) ────
608639
- name: loopover-ops-anomalies
609640
rules:

src/selfhost/ai.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -995,6 +995,32 @@ function errorMessage(error: unknown, knownSecrets: readonly string[] = []): str
995995
return redactSecrets(message, knownSecrets).slice(0, 500);
996996
}
997997

998+
/** Why a provider call failed, at the granularity an operator actually acts on (#9549).
999+
* `credential_invalid` → rotate the credential. `quota_exhausted` → wait, or raise the plan/limit.
1000+
* Conflating the two is the whole problem: both surface today as an opaque `claude_code_error_NNN`, so a
1001+
* 429 (a quota signal that no rotation can clear) is indistinguishable from a 401 (a dead credential)
1002+
* without grepping raw logs -- and rotating a token that was never the problem is the natural next move. */
1003+
export type ProviderFailureReason = "credential_invalid" | "quota_exhausted" | "timeout" | "not_configured" | "other";
1004+
1005+
/** Classify a provider failure from its error. Deliberately matches on the STRUCTURED error shapes this
1006+
* module itself throws (`claude_code_error_<status>`, `codex_error_<status>`, the auth/timeout sentinels)
1007+
* rather than free-text provider prose, which is not a stable contract. Anything unrecognised stays
1008+
* `other` -- a wrong confident label is worse than an honest unknown, because it would send an operator
1009+
* to the wrong runbook. */
1010+
export function classifyProviderFailure(error: unknown): ProviderFailureReason {
1011+
const message = error instanceof Error ? error.message : String(error ?? "");
1012+
// HTTP status carried by the CLI's own structured error envelope, e.g. `claude_code_error_401`.
1013+
const status = /(?:^|[^0-9])(?:error_)(\d{3})\b/.exec(message)?.[1];
1014+
if (status === "401" || status === "403") return "credential_invalid";
1015+
if (status === "429") return "quota_exhausted";
1016+
if (/_no_oauth_token\b|_auth_not_configured\b/.test(message)) return "not_configured";
1017+
// The HTTP providers surface an auth failure as a plain status line rather than a `*_error_NNN` code.
1018+
if (/\b(?:401|403)\b/.test(message) && /unauthor|forbidden|invalid[_ ]api[_ ]key|authentication/i.test(message)) return "credential_invalid";
1019+
if (/\b429\b/.test(message) || /rate[_ ]limit|quota|too many requests/i.test(message)) return "quota_exhausted";
1020+
if (/timeout\b|_timed_out\b|stalled_no_output\b/.test(message)) return "timeout";
1021+
return "other";
1022+
}
1023+
9981024
function logSelfHostAiProviderFailed(input: {
9991025
provider: string;
10001026
model: string;
@@ -1016,6 +1042,11 @@ function logSelfHostAiProviderFailed(input: {
10161042
// a single-shot caller that never sets this field keeps today's always-loud behavior.
10171043
const level = input.finalAttempt === false ? "warn" : "error";
10181044
const log = level === "warn" ? console.warn : console.error;
1045+
const reason = classifyProviderFailure(input.error);
1046+
// A separate counter rather than a new label on loopover_ai_provider_failures_total: that series is
1047+
// already referenced by shipped alert rules and dashboards, and silently multiplying it into one series
1048+
// per reason changes what those existing queries return. This is additive and breaks nothing.
1049+
incr("loopover_ai_provider_failure_reason_total", { provider: input.provider, reason });
10191050
log(
10201051
JSON.stringify({
10211052
level,
@@ -1028,6 +1059,8 @@ function logSelfHostAiProviderFailed(input: {
10281059
repoFullName: input.repoFullName,
10291060
pullNumber: input.pullNumber,
10301061
attempt: input.attempt,
1062+
// Greppable alongside the metric: `reason` answers "rotate or wait?" without decoding the raw error.
1063+
reason,
10311064
error: errorMessage(input.error, input.knownSecrets),
10321065
}),
10331066
);

test/unit/selfhost-ai.test.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const posthogMocks = vi.hoisted(() => {
1818
});
1919
vi.mock("posthog-node", () => ({ PostHog: posthogMocks.PostHog }));
2020

21-
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";
21+
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";
2222
import { labelSelfHostReviewerModel, labelSelfHostReviewerModels } from "../../src/selfhost/ai-config";
2323
import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics";
2424
import { initPostHog, resetPostHogForTest } from "../../src/selfhost/posthog";
@@ -2521,3 +2521,89 @@ describe("ollama context window (#9478)", () => {
25212521
expect(ollamaNumCtx()).toBeGreaterThan(8_192); // must exceed the truncation-prone stock defaults
25222522
});
25232523
});
2524+
2525+
describe("classifyProviderFailure (#9549 — rotate or wait?)", () => {
2526+
it("classifies a rejected credential from the CLI's structured envelope", () => {
2527+
expect(classifyProviderFailure(new Error("claude_code_error_401"))).toBe("credential_invalid");
2528+
expect(classifyProviderFailure(new Error("claude_code_error_403"))).toBe("credential_invalid");
2529+
expect(classifyProviderFailure(new Error("codex_error_401"))).toBe("credential_invalid");
2530+
});
2531+
2532+
it("REGRESSION: classifies a 429 as quota, NOT as a credential problem", () => {
2533+
// The production incident this exists for: a burst of claude_code_error_429 read as "the token is
2534+
// dead", when rotating it could not possibly have helped.
2535+
expect(classifyProviderFailure(new Error("claude_code_error_429"))).toBe("quota_exhausted");
2536+
});
2537+
2538+
it("classifies a missing credential distinctly from a rejected one", () => {
2539+
// Different first response: nothing to rotate, something to CONFIGURE.
2540+
expect(classifyProviderFailure(new Error("claude_code_no_oauth_token"))).toBe("not_configured");
2541+
expect(classifyProviderFailure(new Error("codex_auth_not_configured: /root/.codex/auth.json not found"))).toBe("not_configured");
2542+
});
2543+
2544+
it("classifies timeouts", () => {
2545+
expect(classifyProviderFailure(new Error("subscription_cli_timeout"))).toBe("timeout");
2546+
expect(classifyProviderFailure(new Error("claude_stalled_no_output: no stdout within firstOutputTimeoutMs"))).toBe("timeout");
2547+
});
2548+
2549+
it("classifies HTTP providers, which report auth failures as prose rather than an error_NNN code", () => {
2550+
expect(classifyProviderFailure(new Error("anthropic 401: invalid api key"))).toBe("credential_invalid");
2551+
expect(classifyProviderFailure(new Error("HTTP 403 Forbidden"))).toBe("credential_invalid");
2552+
expect(classifyProviderFailure(new Error("openai 429 Too Many Requests"))).toBe("quota_exhausted");
2553+
expect(classifyProviderFailure(new Error("rate limit exceeded for this organization"))).toBe("quota_exhausted");
2554+
});
2555+
2556+
it("INVARIANT: an unrecognised failure stays 'other' rather than being confidently mislabelled", () => {
2557+
// A wrong label sends an operator to the wrong runbook, which is worse than an honest unknown.
2558+
expect(classifyProviderFailure(new Error("claude_code_exit_1: something unexpected"))).toBe("other");
2559+
expect(classifyProviderFailure(new Error("ECONNREFUSED"))).toBe("other");
2560+
expect(classifyProviderFailure(new Error("claude_code_error_500"))).toBe("other");
2561+
expect(classifyProviderFailure(new Error(""))).toBe("other");
2562+
});
2563+
2564+
it("handles a non-Error and a nullish rejection without throwing", () => {
2565+
expect(classifyProviderFailure("claude_code_error_429")).toBe("quota_exhausted");
2566+
expect(classifyProviderFailure(undefined)).toBe("other");
2567+
expect(classifyProviderFailure(null)).toBe("other");
2568+
});
2569+
2570+
it("does not read a status out of an unrelated number in the message", () => {
2571+
// `error_` is required before the digits, so a PR number or byte count never masquerades as a status.
2572+
expect(classifyProviderFailure(new Error("failed after 401 bytes of output"))).toBe("other");
2573+
});
2574+
});
2575+
2576+
describe("provider failure reason metric + log field (#9549)", () => {
2577+
afterEach(() => {
2578+
resetMetrics();
2579+
});
2580+
2581+
it("counts the reason separately from the existing failures counter, and logs it", async () => {
2582+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
2583+
const quota: StubSpawn = async () => ({ stdout: JSON.stringify({ is_error: true, api_error_status: 429 }), code: 1 });
2584+
await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, quota).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_error_429/);
2585+
2586+
expect(await renderMetrics()).toMatch(/loopover_ai_provider_failure_reason_total\{[^}]*reason="quota_exhausted"[^}]*\} 1/);
2587+
const logged = JSON.parse(errorSpy.mock.calls.at(-1)![0] as string) as { reason: string; event: string };
2588+
expect(logged.event).toBe("selfhost_ai_provider_failed");
2589+
expect(logged.reason).toBe("quota_exhausted");
2590+
errorSpy.mockRestore();
2591+
});
2592+
2593+
it("labels an auth failure as credential_invalid, so the two are distinguishable in metrics", async () => {
2594+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
2595+
const unauthorized: StubSpawn = async () => ({ stdout: JSON.stringify({ is_error: true, api_error_status: 401 }), code: 1 });
2596+
await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, unauthorized).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_error_401/);
2597+
expect(await renderMetrics()).toMatch(/loopover_ai_provider_failure_reason_total\{[^}]*reason="credential_invalid"[^}]*\} 1/);
2598+
errorSpy.mockRestore();
2599+
});
2600+
2601+
it("never leaks the credential into the reason metric's labels", async () => {
2602+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
2603+
const token = "sk-ant-oat01-should-never-appear";
2604+
const unauthorized: StubSpawn = async () => ({ stdout: JSON.stringify({ is_error: true, api_error_status: 401 }), code: 1 });
2605+
await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: token }, unauthorized).run("m", { prompt: "x" })).rejects.toThrow();
2606+
expect(await renderMetrics()).not.toContain(token);
2607+
errorSpy.mockRestore();
2608+
});
2609+
});

0 commit comments

Comments
 (0)