fix: make agent and review failure handling resilient

src/audit/events.ts

@@ -47,7 +47,7 @@ export type AuditEvent =
   | {
       type: "review_posted";
       prNumber: number;
-      verdict: "approve" | "request_changes";
+      verdict: "approve" | "request_changes" | "errored";
       iteration: number;
     }
   | {

src/runners.ts

@@ -17,6 +17,11 @@ import { parseIssueMetadata } from "./state/metadata.js";
 import type { RouterDecision } from "./state/types.js";
 
 const CONFIDENCE_THRESHOLD = 60;
+// Consecutive errored review runs (every lens failed to start) tolerated before
+// the review escalates to review-stuck and pages a human. Each run already
+// retries the CLI install internally, so this guards against a persistent
+// infrastructure failure parking the pipeline silently.
+const MAX_CONSECUTIVE_REVIEW_ERRORS = 3;
 
 // Each stage handler assembles its own extras (revision context, progress
 // comment, PR/file lookups) so the orchestrator stays generic. Returning the
@@ -217,6 +222,10 @@ export const RUNNERS: Record<Stage, StageHandler> = {
       // stateless reviewer: no iteration counter is persisted, every push
       // gets a fresh review, and the iteration cap never fires.
       const iteration = ctx.reviewOnly ? 0 : (routed.reviewIteration ?? 0);
+      const errorCount = ctx.reviewOnly ? 0 : (routed.reviewErrorCount ?? 0);
+      const maxConsecutiveReviewErrors = ctx.reviewOnly
+        ? Number.POSITIVE_INFINITY
+        : MAX_CONSECUTIVE_REVIEW_ERRORS;
       const maxIterations = ctx.reviewOnly
         ? Number.POSITIVE_INFINITY
         : ctx.config.maxReviewIterations;
@@ -277,6 +286,8 @@ export const RUNNERS: Record<Stage, StageHandler> = {
         currentIteration: iteration,
         maxIterations,
         confidenceThreshold: CONFIDENCE_THRESHOLD,
+        currentErrorCount: errorCount,
+        maxConsecutiveReviewErrors,
       });
 
       // Pipeline mode (Shopfloor-authored impl PRs) writes pipeline-state

src/setup/ensure-claude-cli.ts

@@ -1,13 +1,21 @@
-import { spawn, spawnSync } from "node:child_process";
+import { spawnSync } from "node:child_process";
 import { existsSync } from "node:fs";
 import { homedir } from "node:os";
 import { join } from "node:path";
 import * as core from "@actions/core";
+import { retryWithBackoff, runCapturing } from "./installer-support.js";
 
 // Pinned to match @anthropic-ai/claude-agent-sdk@0.2.141 in package.json.
 // Bump in lockstep with the SDK dependency.
 const CLI_VERSION = "2.1.141";
 
+// The install runs `curl | bash` against claude.ai/install.sh, which itself
+// pulls the native build from downloads.claude.ai. Both legs are network calls
+// on a CI runner and fail transiently (DNS SERVFAIL, connection resets). Retry
+// a few times with linear backoff before giving up.
+const INSTALL_ATTEMPTS = 3;
+const INSTALL_RETRY_BASE_DELAY_MS = 2000;
+
 let cached: Promise<string> | null = null;
 
 export function ensureClaudeCli(): Promise<string> {
@@ -47,40 +55,53 @@ function whichClaude(): string | null {
   return path && existsSync(path) ? path : null;
 }
 
+// Run under bash with `set -o pipefail` so a curl failure (e.g. DNS SERVFAIL
+// before install.sh is even fetched) propagates as the pipeline's exit code
+// instead of being masked by bash exiting 0 on empty stdin. The command already
+// pipes into bash, so requiring bash here adds no new dependency. Without
+// pipefail the captured diagnostic tail would be discarded and the failure
+// would surface as a misleading "no binary found".
+function runInstallScript(): Promise<void> {
+  return runCapturing(
+    "bash",
+    [
+      "-c",
+      `set -o pipefail; curl -fsSL https://claude.ai/install.sh | bash -s -- ${CLI_VERSION}`,
+    ],
+    "Claude CLI installer",
+  );
+}
+
 async function installClaudeCli(): Promise<string> {
   core.info(
     `Claude CLI not found on PATH. Installing v${CLI_VERSION} via claude.ai/install.sh`,
   );
-  await new Promise<void>((resolve, reject) => {
-    const proc = spawn(
-      "sh",
-      [
-        "-c",
-        `curl -fsSL https://claude.ai/install.sh | bash -s -- ${CLI_VERSION}`,
-      ],
-      { stdio: ["ignore", "inherit", "inherit"] },
-    );
-    proc.on("error", reject);
-    proc.on("close", (code) => {
-      if (code === 0) resolve();
-      else reject(new Error(`Claude CLI installer exited with code ${code}`));
-    });
-  });
-
   // install.sh defaults to ~/.local/bin/claude.
   const defaultPath = join(homedir(), ".local", "bin", "claude");
-  if (existsSync(defaultPath)) {
-    core.info(`Installed Claude CLI at ${defaultPath}`);
-    return defaultPath;
-  }
-
-  const onPath = whichClaude();
-  if (onPath) {
-    core.info(`Installed Claude CLI resolved via PATH: ${onPath}`);
-    return onPath;
-  }
 
-  throw new Error(
-    `Claude CLI installer succeeded but no binary found at ${defaultPath} or on PATH`,
+  return retryWithBackoff(
+    async () => {
+      await runInstallScript();
+      if (existsSync(defaultPath)) {
+        core.info(`Installed Claude CLI at ${defaultPath}`);
+        return defaultPath;
+      }
+      const onPath = whichClaude();
+      if (onPath) {
+        core.info(`Installed Claude CLI resolved via PATH: ${onPath}`);
+        return onPath;
+      }
+      throw new Error(
+        `Claude CLI installer succeeded but no binary found at ${defaultPath} or on PATH`,
+      );
+    },
+    {
+      attempts: INSTALL_ATTEMPTS,
+      baseDelayMs: INSTALL_RETRY_BASE_DELAY_MS,
+      onRetry: (attempt, error) =>
+        core.warning(
+          `Claude CLI install attempt ${attempt}/${INSTALL_ATTEMPTS} failed: ${error.message}. Retrying...`,
+        ),
+    },
   );
 }

src/setup/ensure-codex-cli.ts

@@ -1,11 +1,18 @@
-import { spawn, spawnSync } from "node:child_process";
+import { spawnSync } from "node:child_process";
 import { existsSync } from "node:fs";
 import * as core from "@actions/core";
+import { retryWithBackoff, runCapturing } from "./installer-support.js";
 
 // Pinned to match @openai/codex-sdk / @openai/codex in package.json.
 // Bump in lockstep with those dependencies.
 const CLI_VERSION = "0.139.0";
 
+// `npm install -g` is a network call on a CI runner and fails transiently
+// (DNS SERVFAIL, registry connection resets). Retry with linear backoff,
+// mirroring the Claude installer.
+const INSTALL_ATTEMPTS = 3;
+const INSTALL_RETRY_BASE_DELAY_MS = 2000;
+
 let cached: Promise<string> | null = null;
 
 export function ensureCodexCli(): Promise<string> {
@@ -49,26 +56,30 @@ async function installCodexCli(): Promise<string> {
   core.info(
     `Codex CLI not found on PATH. Installing @openai/codex@${CLI_VERSION} globally via npm.`,
   );
-  await new Promise<void>((resolve, reject) => {
-    const proc = spawn(
-      "npm",
-      ["install", "-g", `@openai/codex@${CLI_VERSION}`],
-      { stdio: ["ignore", "inherit", "inherit"] },
-    );
-    proc.on("error", reject);
-    proc.on("close", (code) => {
-      if (code === 0) resolve();
-      else reject(new Error(`Codex CLI installer exited with code ${code}`));
-    });
-  });
-
-  const onPath = whichCodex();
-  if (onPath) {
-    core.info(`Installed Codex CLI resolved via PATH: ${onPath}`);
-    return onPath;
-  }
 
-  throw new Error(
-    "Codex CLI installer succeeded but no `codex` binary was found on PATH",
+  return retryWithBackoff(
+    async () => {
+      await runCapturing(
+        "npm",
+        ["install", "-g", `@openai/codex@${CLI_VERSION}`],
+        "Codex CLI installer",
+      );
+      const onPath = whichCodex();
+      if (onPath) {
+        core.info(`Installed Codex CLI resolved via PATH: ${onPath}`);
+        return onPath;
+      }
+      throw new Error(
+        "Codex CLI installer succeeded but no `codex` binary was found on PATH",
+      );
+    },
+    {
+      attempts: INSTALL_ATTEMPTS,
+      baseDelayMs: INSTALL_RETRY_BASE_DELAY_MS,
+      onRetry: (attempt, error) =>
+        core.warning(
+          `Codex CLI install attempt ${attempt}/${INSTALL_ATTEMPTS} failed: ${error.message}. Retrying...`,
+        ),
+    },
   );
 }

src/setup/installer-support.ts

@@ -0,0 +1,84 @@
+import { spawn } from "node:child_process";
+
+// Shared helpers for the native-CLI installers (Claude and Codex). Both pull a
+// binary over the network on a CI runner, so both need transient-failure retry
+// and diagnostics that surface the real cause instead of a bare exit code.
+
+export interface RetryOptions {
+  attempts: number;
+  baseDelayMs: number;
+  sleep?: (ms: number) => Promise<void>;
+  onRetry?: (attempt: number, error: Error) => void;
+}
+
+function defaultSleep(ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
+export async function retryWithBackoff<T>(
+  fn: (attempt: number) => Promise<T>,
+  opts: RetryOptions,
+): Promise<T> {
+  const sleep = opts.sleep ?? defaultSleep;
+  let lastError: Error | undefined;
+  for (let attempt = 1; attempt <= opts.attempts; attempt++) {
+    try {
+      return await fn(attempt);
+    } catch (err) {
+      lastError = err instanceof Error ? err : new Error(String(err));
+      if (attempt < opts.attempts) {
+        opts.onRetry?.(attempt, lastError);
+        await sleep(opts.baseDelayMs * attempt);
+      }
+    }
+  }
+  throw lastError ?? new Error("retryWithBackoff: no attempts were made");
+}
+
+// Installers stream their own progress, but a thrown Error that carries only
+// the exit code surfaces as the opaque "exited with code 1". Append the tail of
+// the captured output (e.g. "getaddrinfo ESERVFAIL downloads.claude.ai") so the
+// real cause is visible wherever the error is reported, including the review
+// lens failure summary.
+export function formatInstallerError(
+  label: string,
+  code: number | null,
+  output: string,
+): string {
+  const base = `${label} exited with code ${code}`;
+  const tail = output
+    .trim()
+    .split(/\r?\n/)
+    .map((line) => line.trim())
+    .filter(Boolean)
+    .slice(-5)
+    .join(" | ");
+  return tail ? `${base}: ${tail}` : base;
+}
+
+// Spawn a command, capturing stdout/stderr for diagnostics while still
+// streaming it to the action log, and reject with a formatted error (carrying
+// the captured tail) on a non-zero exit.
+export function runCapturing(
+  command: string,
+  args: string[],
+  label: string,
+): Promise<void> {
+  return new Promise((resolve, reject) => {
+    const proc = spawn(command, args, { stdio: ["ignore", "pipe", "pipe"] });
+    let output = "";
+    proc.stdout?.on("data", (chunk: Buffer) => {
+      output += chunk.toString();
+      process.stdout.write(chunk);
+    });
+    proc.stderr?.on("data", (chunk: Buffer) => {
+      output += chunk.toString();
+      process.stderr.write(chunk);
+    });
+    proc.on("error", reject);
+    proc.on("close", (code) => {
+      if (code === 0) resolve();
+      else reject(new Error(formatInstallerError(label, code, output)));
+    });
+  });
+}

src/stages/review/aggregate.ts

@@ -12,6 +12,13 @@ export interface AggregateInput {
   currentIteration: number;
   maxIterations: number;
   confidenceThreshold: number;
+  // Consecutive prior review runs that ended in `errored` (read from the PR
+  // footer). Defaults to 0. When the incremented count reaches
+  // `maxConsecutiveReviewErrors`, the errored outcome escalates so a human is
+  // paged instead of the pipeline spinning on a persistent infrastructure
+  // failure. Both are irrelevant (0 / infinity) in reviewOnly mode.
+  currentErrorCount?: number;
+  maxConsecutiveReviewErrors?: number;
 }
 
 export type AggregateOutcome =
@@ -31,10 +38,28 @@ export type AggregateOutcome =
   | {
       kind: "iteration_cap";
       maxIterations: number;
+    }
+  | {
+      kind: "errored";
+      body: string;
+      failures: Array<{ lens: LensName; kind: string; message: string }>;
+      // Consecutive errored runs including this one.
+      errorCount: number;
+      // True once errorCount reaches maxConsecutiveReviewErrors: stop retrying
+      // silently and page a human via the review-stuck label.
+      escalate: boolean;
     };
 
 const SHOPFLOOR_REVIEW_MARKER = "<!-- shopfloor-review -->";
 
+function renderLensFailure(f: {
+  lens: LensName;
+  kind: string;
+  message: string;
+}): string {
+  return `- \`${f.lens}\` failed (${f.kind}): ${f.message}`;
+}
+
 const SOURCE_CATEGORY: Record<LensName, Category> = {
   compliance: "compliance",
   bugs: "bug",
@@ -112,6 +137,37 @@ export function aggregateFindings(input: AggregateInput): AggregateOutcome {
     (s) => s.decision.verdict === "blocked",
   );
 
+  // Every lens failed before returning a verdict (e.g. the Claude CLI never
+  // installed, all agents timed out). There is no review result at all, so this
+  // is an operational error rather than a "changes requested" verdict. Surface
+  // it as such instead of blocking an unevaluated PR with a misleading
+  // REQUEST_CHANGES review.
+  if (succeeded.length === 0 && failedLenses.length > 0) {
+    const errorCount = (input.currentErrorCount ?? 0) + 1;
+    const maxErrors =
+      input.maxConsecutiveReviewErrors ?? Number.POSITIVE_INFINITY;
+    const escalate = errorCount >= maxErrors;
+    const closing = escalate
+      ? `This has now failed ${errorCount} consecutive times. A human should take over this PR.`
+      : "This is an infrastructure error, not a code-review result. The pull request has not been evaluated and will be re-reviewed.";
+    const body = [
+      SHOPFLOOR_REVIEW_MARKER,
+      "**Shopfloor agent review: could not complete.** Every reviewer failed before returning a verdict.",
+      "",
+      closing,
+      "",
+      "**Reviewer failures:**",
+      ...failedLenses.map(renderLensFailure),
+    ].join("\n");
+    return {
+      kind: "errored",
+      body,
+      failures: failedLenses,
+      errorCount,
+      escalate,
+    };
+  }
+
   const allComments = succeeded.flatMap((s) => s.decision.comments);
   const deduped = dedupeComments(allComments);
   const filtered = deduped.filter(
@@ -167,7 +223,7 @@ export function aggregateFindings(input: AggregateInput): AggregateOutcome {
     bodyParts.push("");
     bodyParts.push("**Lens failures:**");
     for (const f of failedLenses) {
-      bodyParts.push(`- \`${f.lens}\` failed (${f.kind}): ${f.message}`);
+      bodyParts.push(renderLensFailure(f));
     }
   }
   if (blockedLenses.length > 0) {