fix(observability): capture AI-CLI stderr + surface exhausted provider failures to Sentry

src/selfhost/ai.ts

@@ -216,7 +216,7 @@ type SpawnFn = (
   cmd: string,
   args: string[],
   opts: { env: Record<string, string | undefined>; input?: string; timeoutMs: number; cwd?: string },
-) => Promise<{ stdout: string; code: number | null }>;
+) => Promise<{ stdout: string; code: number | null; stderr?: string }>;
 
 async function defaultSpawn(): Promise<SpawnFn> {
   const cp = await import("node:child_process");
@@ -225,20 +225,24 @@ async function defaultSpawn(): Promise<SpawnFn> {
       const stdio: ["pipe", "pipe", "pipe"] = ["pipe", "pipe", "pipe"];
       const child = cp.spawn(cmd, args, { cwd: o.cwd, env: o.env as NodeJS.ProcessEnv, stdio });
       let stdout = "";
+      // Capture stderr too — the CLI's actual error (auth, rate limit, model-not-supported, OOM) lands here, and
+      // it's what makes a `claude_code_exit_1` / `codex_exit_1` diagnosable instead of an opaque exit code (#26).
+      let stderr = "";
       /* v8 ignore start */ // a 120s subprocess timeout is not unit-testable without a 2-minute wait
       const timer = setTimeout(() => {
         child.kill("SIGKILL");
         reject(new Error("subscription_cli_timeout"));
       }, o.timeoutMs);
       /* v8 ignore stop */
       child.stdout?.on("data", (d: Buffer) => (stdout += d.toString("utf8")));
+      child.stderr?.on("data", (d: Buffer) => (stderr += d.toString("utf8")));
       child.on("error", (e) => {
         clearTimeout(timer);
         reject(e);
       });
       child.on("close", (code) => {
         clearTimeout(timer);
-        resolve({ stdout, code });
+        resolve({ stdout, code, stderr });
       });
       if (o.input != null) {
         child.stdin?.write(o.input);
@@ -247,6 +251,30 @@ async function defaultSpawn(): Promise<SpawnFn> {
     });
 }
 
+/** Credential/token shapes that must never reach logs or Sentry. High-precision (prefixed key formats + JWT, each
+ *  anchored on a word boundary) so genuine diagnostics — auth/rate-limit/model errors — survive redaction. */
+const SECRET_PATTERNS: readonly RegExp[] = [
+  /\bsk-[A-Za-z0-9_-]{16,}/g, // OpenAI / Anthropic keys (sk-..., sk-ant-..., sk-proj-...)
+  /\bgh[oprsu]_[A-Za-z0-9]{20,}/g, // GitHub PAT / OAuth / server / refresh tokens
+  /\bgithub_pat_[A-Za-z0-9_]{20,}/g, // GitHub fine-grained PAT
+  /\beyJ[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}/g, // JWT (header.payload.signature)
+  /\bAKIA[0-9A-Z]{16}/g, // AWS access key id
+];
+
+/** Redact secrets from untrusted CLI stderr before it enters an error message that flows to logs/Sentry. The
+ *  claude/codex subprocesses can echo back the OAuth token we hand them via env (or a key from a config they read),
+ *  and the central Sentry forwarder only scrubs secret-KEYED fields, never free-text — so a token inside an error
+ *  string would otherwise leak. Strips the caller's known secret values exactly, then well-known token shapes. */
+export function redactSecrets(text: string, knownSecrets: readonly string[] = []): string {
+  let out = text;
+  for (const secret of knownSecrets) {
+    // Length-guard so a short/empty token (e.g. a stubbed "t") can't blank out unrelated diagnostic text.
+    if (secret.length >= 8) out = out.split(secret).join("[redacted]");
+  }
+  for (const pattern of SECRET_PATTERNS) out = out.replace(pattern, "[redacted]");
+  return out;
+}
+
 /** Claude Code subscription (CLAUDE_CODE_OAUTH_TOKEN via `claude setup-token`). Headless, read-only, JSON. */
 export function createClaudeCodeAi(parentEnv: Record<string, string | undefined>, spawnImpl?: SpawnFn): SelfHostAi {
   return {
@@ -262,12 +290,12 @@ export function createClaudeCodeAi(parentEnv: Record<string, string | undefined>
       const spawn = spawnImpl ?? (await defaultSpawn());
       const claudeModel = resolveModel(configuredModel(parentEnv), model, "claude-sonnet-4-6");
       const effort = resolveEffort(parentEnv.AI_EFFORT);
-      const { stdout, code } = await spawn(
+      const { stdout, code, stderr } = await spawn(
         "claude",
         ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "plan", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"],
         { env, input: prompt, timeoutMs: resolveCliTimeoutMs(parentEnv), cwd: await isolatedCliCwd() },
       );
-      if (code !== 0) throw new Error(`claude_code_exit_${code ?? "null"}`);
+      if (code !== 0) throw new Error(`claude_code_exit_${code ?? "null"}: ${redactSecrets(stderr ?? "", [token]).slice(0, 500)}`);
       const errStatus = claudeErrorStatus(stdout);
       if (errStatus) throw new Error(`claude_code_error_${errStatus}`);
       const text = extractCliText(stdout);
@@ -296,12 +324,12 @@ export function createCodexAi(parentEnv: Record<string, string | undefined>, spa
       const args = ["exec", "--json", "--skip-git-repo-check", "--sandbox", "read-only"];
       if (codexModel) args.push("--model", codexModel);
       args.push("--", prompt);
-      const { stdout, code } = await spawn("codex", args, {
+      const { stdout, code, stderr } = await spawn("codex", args, {
         env,
         timeoutMs: resolveCliTimeoutMs(parentEnv),
         cwd: await isolatedCliCwd(),
       });
-      if (code !== 0) throw new Error(`codex_exit_${code ?? "null"}`);
+      if (code !== 0) throw new Error(`codex_exit_${code ?? "null"}: ${redactSecrets(stderr ?? "").slice(0, 500)}`);
       const text = extractCliText(stdout);
       if (!text) throw new Error("codex_empty_output");
       return { response: text };

src/services/ai-review.ts

@@ -518,6 +518,9 @@ async function runWorkersOpinion(
   const extra: AiGatewayOptions | undefined = gatewayId
     ? { gateway: { id: gatewayId } }
     : undefined;
+  // Track the last provider error so we can fail-LOUD once ALL models × attempts are exhausted (below). Per-attempt
+  // logs are warn (noisy retries, skipped by the central Sentry forwarder); the exhausted summary is error (#26).
+  let lastError: unknown;
   for (const model of fallback && fallback !== primary
     ? [primary, fallback]
     : [primary]) {
@@ -550,9 +553,24 @@ async function runWorkersOpinion(
             error: errorMessage(error),
           }),
         );
+        lastError = error;
       }
     }
   }
+  // All models × attempts threw (vs "ran but returned unparseable output", where lastError stays undefined): the
+  // reviewer is genuinely DOWN. Emit one level:error log so the central Sentry forwarder surfaces the outage — the
+  // per-attempt warns above are invisible to it. (#26 fail-loud)
+  if (lastError !== undefined) {
+    console.log(
+      JSON.stringify({
+        level: "error",
+        event: "ai_review_provider_exhausted",
+        primary,
+        fallback,
+        error: errorMessage(lastError),
+      }),
+    );
+  }
   return null;
 }
 

test/unit/ai-review.test.ts

@@ -1188,6 +1188,43 @@ describe("pure helpers", () => {
     expect(run).toHaveBeenCalledTimes(1);
   });
 
+  it("logs ai_review_provider_exhausted at error level when every attempt throws (#26 fail-loud)", async () => {
+    const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const run = vi.fn(async () => {
+      throw new Error("ENOENT: claude binary not found");
+    });
+    const env = createTestEnv({ AI: { run } as unknown as Ai });
+    const result = await runWorkersOpinion(env, "primary-model", "", "sys", "user", 256);
+    expect(result).toBeNull();
+    const exhausted = logSpy.mock.calls
+      .map((c) => c[0])
+      .find((l) => typeof l === "string" && l.includes("ai_review_provider_exhausted"));
+    expect(exhausted).toBeDefined();
+    expect(JSON.parse(exhausted as string)).toMatchObject({
+      level: "error",
+      event: "ai_review_provider_exhausted",
+      primary: "primary-model",
+      error: expect.stringContaining("ENOENT"),
+    });
+    logSpy.mockRestore();
+    warnSpy.mockRestore();
+  });
+
+  it("does NOT log exhausted when the model runs but returns unparseable output (no provider error)", async () => {
+    const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
+    const run = vi.fn(async () => ({ response: "not json at all" }));
+    const env = createTestEnv({ AI: { run } as unknown as Ai });
+    const result = await runWorkersOpinion(env, "primary-model", "", "sys", "user", 256);
+    expect(result).toBeNull();
+    expect(
+      logSpy.mock.calls
+        .map((c) => c[0])
+        .some((l) => typeof l === "string" && l.includes("ai_review_provider_exhausted")),
+    ).toBe(false);
+    logSpy.mockRestore();
+  });
+
   it("applies the default daily neuron budget when none is configured", async () => {
     const run = vi.fn(async (_model: string) => ({ response: reviewJson() }));
     const env = createTestEnv({

test/unit/selfhost-ai.test.ts

@@ -2,7 +2,7 @@ import { chmodSync, mkdtempSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, resolveAiReviewerPlan, resolveCliTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, routeProviders, subscriptionCliEnv } from "../../src/selfhost/ai";
+import { buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, resolveAiReviewerPlan, resolveCliTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, redactSecrets, routeProviders, subscriptionCliEnv } from "../../src/selfhost/ai";
 
 describe("resolveModel (#979 — never leak the Workers-AI default to a self-host backend)", () => {
   const WORKERS_DEFAULT = "@cf/meta/llama-3.1-8b-instruct-fp8-fast";
@@ -52,7 +52,7 @@ describe("resolveCliTimeoutMs (#selfhost — subprocess timeout scales with effo
 
 afterEach(() => vi.unstubAllGlobals());
 
-type SpawnResult = { stdout: string; code: number | null };
+type SpawnResult = { stdout: string; code: number | null; stderr?: string };
 type StubSpawn = (
   cmd: string,
   args: string[],
@@ -456,6 +456,53 @@ describe("subscription CLI helpers + fail-safe", () => {
     await expect(createCodexAi({}, empty).run("gpt-5", { prompt: "x" })).rejects.toThrow(/codex_empty_output/);
   });
 
+  it("surfaces the CLI's stderr in the non-zero-exit error (diagnosable failures, #26)", async () => {
+    // Without stderr in the message, a `claude_code_exit_1` / `codex_exit_1` is an opaque dead-end; with it the real
+    // cause (auth, rate limit, model-not-supported) reaches the logs + Sentry. (stderr-present branch of `?? ""`.)
+    const claudeErr: StubSpawn = async () => ({ stdout: "", code: 1, stderr: "Invalid API key · auth_error" });
+    await expect(
+      createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, claudeErr).run("m", { prompt: "x" }),
+    ).rejects.toThrow(/claude_code_exit_1: Invalid API key/);
+    const codexErr: StubSpawn = async () => ({ stdout: "", code: 1, stderr: "stream error: rate limit reached" });
+    await expect(createCodexAi({}, codexErr).run("m", { prompt: "x" })).rejects.toThrow(
+      /codex_exit_1: stream error: rate limit reached/,
+    );
+  });
+
+  it("redacts the OAuth token and key-shaped tokens from claude stderr before they reach the error (#1605 sec)", async () => {
+    // The CLI can echo the token we hand it via env; it must never land in an error string forwarded to Sentry.
+    const token = "oauth-tok-abcdef123456";
+    const leaky: StubSpawn = async () => ({ stdout: "", code: 1, stderr: `fatal: rejected token ${token} (key sk-ant-api03-ABCDEFGHIJKLMNOPqrstuvwx)` });
+    const err = await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: token }, leaky).run("m", { prompt: "x" }).catch((e: Error) => e.message);
+    expect(err).toContain("claude_code_exit_1:");
+    expect(err).not.toContain(token);
+    expect(err).not.toContain("sk-ant-api03");
+    expect(err).toContain("[redacted]");
+  });
+
+  it("redacts key-shaped tokens from codex stderr (no env token to key off) (#1605 sec)", async () => {
+    const leaky: StubSpawn = async () => ({ stdout: "", code: 1, stderr: "auth failed: ghp_ABCDEFGHIJ0123456789KLMNOPQRSTUV" });
+    await expect(createCodexAi({}, leaky).run("m", { prompt: "x" })).rejects.toThrow(/codex_exit_1: auth failed: \[redacted\]/);
+  });
+
+  it("defaultSpawn captures a failing CLI's stderr and surfaces it on the exit error (#26)", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "fakecli-"));
+    const fake = join(dir, "claude");
+    // a fake `claude` that reads stdin (so the parent's write never EPIPEs), then writes to STDERR and exits non-zero
+    // — the real failure shape we previously couldn't diagnose.
+    writeFileSync(fake, "#!/usr/bin/env node\nlet i='';process.stdin.on('data',d=>i+=d);process.stdin.on('end',()=>{process.stderr.write('BOOM: auth failed');process.exit(1);});\n");
+    chmodSync(fake, 0o755);
+    const origPath = process.env.PATH;
+    process.env.PATH = `${dir}:${origPath ?? ""}`;
+    try {
+      await expect(
+        createClaudeCodeAi({ ...process.env, CLAUDE_CODE_OAUTH_TOKEN: "t" }).run("sonnet", { prompt: "x" }),
+      ).rejects.toThrow(/claude_code_exit_1: BOOM: auth failed/);
+    } finally {
+      process.env.PATH = origPath;
+    }
+  });
+
   it("defaultSpawn rejects when the CLI binary is missing (error handler)", async () => {
     const origPath = process.env.PATH;
     process.env.PATH = "/nonexistent-gittensory-empty";
@@ -471,3 +518,25 @@ describe("subscription CLI helpers + fail-safe", () => {
     expect(extractCliText("not json\nstill not json")).toBe("");
   });
 });
+
+describe("redactSecrets — strip credentials from untrusted CLI stderr before it reaches logs/Sentry (#1605 sec)", () => {
+  it("redacts caller-known secret values (>= 8 chars) and leaves short ones untouched", () => {
+    expect(redactSecrets("token=supersecretvalue used", ["supersecretvalue"])).toBe("token=[redacted] used");
+    // a short known value must NOT blank out unrelated text (length-guard false branch)
+    expect(redactSecrets("the cat sat", ["cat"])).toBe("the cat sat");
+  });
+
+  it("redacts well-known token shapes with no known-value list (default arg)", () => {
+    expect(redactSecrets("key sk-ant-api03-ABCDEFGHIJKLMNOPqrstuvwx12")).toBe("key [redacted]");
+    expect(redactSecrets("pat ghp_ABCDEFGHIJ0123456789KLMNOPQRSTUV")).toBe("pat [redacted]");
+    expect(redactSecrets("fine github_pat_ABCDEFGHIJ0123456789KLMNO")).toBe("fine [redacted]");
+    expect(redactSecrets("jwt eyJhbGciOi.eyJzdWIiOi.S1gnaTuRe99")).toBe("jwt [redacted]");
+    expect(redactSecrets("aws AKIAIOSFODNN7EXAMPLE here")).toBe("aws [redacted] here");
+  });
+
+  it("leaves benign diagnostics intact, including words that merely contain a token prefix", () => {
+    expect(redactSecrets("Invalid API key · auth_error")).toBe("Invalid API key · auth_error");
+    // "disk-usage-report-2024-summary" must survive — the \b anchor prevents an in-word `sk-` false positive
+    expect(redactSecrets("disk-usage-report-2024-summary failed")).toBe("disk-usage-report-2024-summary failed");
+  });
+});