Release v0.3.0

Token/cache/cost totals were accumulated and written to .runs/reviews.jsonl,
but that file is ephemeral in CI, so prompt-cache effectiveness was invisible
after a CI run. Emit a one-line usage summary (input/output/reasoning + cache
read/write, and cost when non-zero) via progress right before finalizing, so
it lands in the CI job log and the local terminal. Adds unit tests for the
formatter.

Co-authored-by: expo-tuft[bot] <288127324+expo-tuft[bot]@users.noreply.github.com>
Co-authored-by: brentvatne <[email protected]>

.github/workflows/expo-code-review.yml

@@ -17,9 +17,15 @@ concurrency:
 jobs:
   review:
     runs-on: ubuntu-latest
-    # Opt-in per PR: only run when the `ai-review` label is present. Remove this
-    # line to review every PR automatically.
-    if: contains(join(github.event.pull_request.labels.*.name, ','), 'ai-review')
+    # Trigger policy lives in .expo-code-review/config.jsonc (review.trigger); `ecr ci`
+    # self-gates on it (and honors the ai-review:skip label). This coarse gate just
+    # avoids spinning up a runner for a PR that explicitly opted out. Uses the array
+    # form of contains() for an EXACT label match (the old join()+substring form
+    # matched "ai-review:skip" as containing "ai-review").
+    # Prefer to gate entirely here instead? Set config trigger to "label" (or "all")
+    # and replace the line below with, e.g.:
+    #   if: contains(github.event.pull_request.labels.*.name, 'ai-review')
+    if: ${{ !contains(github.event.pull_request.labels.*.name, 'ai-review:skip') }}
     # Backstop so a stalled review fails fast instead of hanging.
     timeout-minutes: 60
     # A reviewer failure must never fail the PR's checks.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@expo/code-review-cli",
-  "version": "0.2.3",
+  "version": "0.3.0",
   "description": "Generic, config-driven AI code reviewer engine. Repos supply their agents via .expo-code-review/.",
   "license": "MIT",
   "repository": {

src/__tests__/auth.test.ts

@@ -1,10 +1,61 @@
 import { test, expect } from 'bun:test';
 
-import { prepareAuth } from '../core/auth.js';
+import { prepareAuth, checkProviderAuth } from '../core/auth.js';
 import type { LoadedConfig } from '../config/schema.js';
 
 const cfg = (auth: unknown): LoadedConfig => ({ auth }) as unknown as LoadedConfig;
 
+test('checkProviderAuth: api-key is ready when the configured tokenEnv is set', () => {
+  const r = checkProviderAuth(cfg({ mode: 'api-key', provider: 'anthropic', tokenEnv: 'MY_KEY' }), {
+    MY_KEY: 'sk-xxx',
+  });
+  expect(r.ok).toBe(true);
+});
+
+test('checkProviderAuth: api-key is ready via the provider key env when no tokenEnv value', () => {
+  const r = checkProviderAuth(cfg({ mode: 'api-key', provider: 'openai', tokenEnv: 'MY_KEY' }), {
+    OPENAI_API_KEY: 'sk-xxx',
+  });
+  expect(r.ok).toBe(true);
+});
+
+test('checkProviderAuth: api-key fails fast when no credential is set anywhere', () => {
+  const r = checkProviderAuth(cfg({ mode: 'api-key', provider: 'anthropic', tokenEnv: 'MY_KEY' }), {});
+  expect(r.ok).toBe(false);
+  expect(r.detail).toMatch(/MY_KEY/);
+});
+
+test('checkProviderAuth: oauth requires the token env to be set', () => {
+  expect(
+    checkProviderAuth(cfg({ mode: 'oauth', provider: 'anthropic', tokenEnv: 'OAUTH' }), {}).ok
+  ).toBe(false);
+  expect(
+    checkProviderAuth(cfg({ mode: 'oauth', provider: 'anthropic', tokenEnv: 'OAUTH' }), {
+      OAUTH: 'tok',
+    }).ok
+  ).toBe(true);
+});
+
+test('checkProviderAuth: forbidden tokenEnv is refused', () => {
+  const r = checkProviderAuth(cfg({ mode: 'api-key', provider: 'anthropic', tokenEnv: 'GITHUB_TOKEN' }), {
+    GITHUB_TOKEN: 'ghp_xxx',
+  });
+  expect(r.ok).toBe(false);
+  expect(r.detail).toMatch(/non-provider secret/);
+});
+
+test('checkProviderAuth: REVIEWER_MODEL override is always ready (own login)', () => {
+  const r = checkProviderAuth(cfg({ mode: 'oauth', provider: 'anthropic' }), {
+    REVIEWER_MODEL: 'openai/some-model',
+  });
+  expect(r.ok).toBe(true);
+});
+
+test('checkProviderAuth: no tokenEnv and unknown provider defers to OpenCode login', () => {
+  const r = checkProviderAuth(cfg({ mode: 'api-key', provider: 'mystery' }), {});
+  expect(r.ok).toBe(true);
+});
+
 test('prepareAuth refuses to forward a well-known non-provider secret as tokenEnv', async () => {
   const prev = process.env.REVIEWER_MODEL;
   delete process.env.REVIEWER_MODEL; // ensure the guard path runs (override would skip it)

src/__tests__/ci-trigger.test.ts

@@ -0,0 +1,35 @@
+import { test, expect } from 'bun:test';
+
+import { shouldReview } from '../commands/ci.js';
+
+const policy = { trigger: 'all' as const, label: 'ai-review', skipLabel: 'ai-review:skip' };
+const labelMode = { ...policy, trigger: 'label' as const };
+
+test('trigger "all": reviews every PR when not skipped', () => {
+  expect(shouldReview([], policy).review).toBe(true);
+  expect(shouldReview(['something-else'], policy).review).toBe(true);
+});
+
+test('skipLabel always wins (exact match), even in "all" mode', () => {
+  expect(shouldReview(['ai-review:skip'], policy).review).toBe(false);
+  // present alongside an opt-in label → still skipped
+  expect(shouldReview(['ai-review', 'ai-review:skip'], labelMode).review).toBe(false);
+});
+
+test('trigger "label": requires the opt-in label or a label:<agent> variant', () => {
+  expect(shouldReview([], labelMode).review).toBe(false);
+  expect(shouldReview(['ai-review'], labelMode).review).toBe(true);
+  expect(shouldReview(['ai-review:correctness'], labelMode).review).toBe(true);
+  expect(shouldReview(['unrelated'], labelMode).review).toBe(false);
+});
+
+test('the skip label does not count as the opt-in label (no substring match)', () => {
+  // ai-review:skip must not satisfy the ai-review opt-in in label mode.
+  expect(shouldReview(['ai-review:skip'], labelMode).review).toBe(false);
+});
+
+test('custom label names are honored', () => {
+  const custom = { trigger: 'label' as const, label: 'please-review', skipLabel: 'no-review' };
+  expect(shouldReview(['please-review'], custom).review).toBe(true);
+  expect(shouldReview(['no-review'], custom).review).toBe(false);
+});

src/__tests__/opencode.test.ts

@@ -0,0 +1,43 @@
+import { test, expect } from 'bun:test';
+
+import { isTransientApiError, AgentTimeoutError } from '../core/opencode.js';
+
+test('classifies rate-limit / 5xx / network errors as transient', () => {
+  for (const message of [
+    'HTTP 429 Too Many Requests',
+    'rate limit exceeded',
+    'Overloaded',
+    'status 503 Service Unavailable',
+    'server error 500',
+    'fetch failed',
+    'read ECONNRESET',
+    'connect ETIMEDOUT 1.2.3.4:443',
+    'getaddrinfo ENOTFOUND api.anthropic.com',
+    'socket hang up',
+  ]) {
+    expect(isTransientApiError(new Error(message))).toBe(true);
+  }
+});
+
+test('does not treat deterministic failures as transient', () => {
+  for (const message of [
+    'Agent "correctness" did not return parseable JSON after retries',
+    'HTTP 400 Bad Request',
+    'HTTP 401 Unauthorized',
+    'HTTP 403 Forbidden',
+    'invalid model id',
+  ]) {
+    expect(isTransientApiError(new Error(message))).toBe(false);
+  }
+});
+
+test('never treats a timeout as transient (timeouts must abandon, not retry)', () => {
+  // Its message mentions "timed out", but a timeout means abandon the pass.
+  expect(isTransientApiError(new AgentTimeoutError('correctness', 15))).toBe(false);
+});
+
+test('tolerates non-Error thrown values', () => {
+  expect(isTransientApiError('429 rate limit')).toBe(true);
+  expect(isTransientApiError(undefined)).toBe(false);
+  expect(isTransientApiError({ nope: true })).toBe(false);
+});

src/__tests__/review-internals.test.ts

@@ -6,6 +6,8 @@ import {
   runGrowableQueue,
   decisionAfterVerification,
   reconcileSummary,
+  isAuthError,
+  formatUsageSummary,
 } from '../core/review.js';
 import type { PatchWorkspaceFile } from '../core/noise.js';
 import type { CoordinatorOutput, Finding } from '../core/schema.js';
@@ -120,3 +122,49 @@ test('decisionAfterVerification: re-derives after drops', () => {
     'request_changes'
   );
 });
+
+test('isAuthError flags provider auth/permission failures', () => {
+  for (const message of [
+    'HTTP 401 Unauthorized',
+    'status 403 Forbidden',
+    'authentication_error: invalid x-api-key',
+    'permission denied for this model',
+    'Invalid API key provided',
+    'the OAuth token has expired',
+    'missing api key',
+  ]) {
+    expect(isAuthError(new Error(message))).toBe(true);
+  }
+});
+
+test('isAuthError does not flag non-auth failures', () => {
+  for (const message of [
+    'HTTP 429 Too Many Requests',
+    'server error 500',
+    'did not return parseable JSON after retries',
+    'ECONNRESET',
+    'request timed out',
+  ]) {
+    expect(isAuthError(new Error(message))).toBe(false);
+  }
+});
+
+test('formatUsageSummary reports token + cache totals (and cost when present)', () => {
+  const s = formatUsageSummary(
+    { input: 1200, output: 340, reasoning: 50, cache: { read: 108_000, write: 900 } },
+    0.0123
+  );
+  expect(s).toContain('input 1200');
+  expect(s).toContain('output 340');
+  expect(s).toContain('reasoning 50');
+  expect(s).toContain('cache read 108000');
+  expect(s).toContain('cache write 900');
+  expect(s).toContain('$0.0123');
+});
+
+test('formatUsageSummary omits cost when zero and reasoning when absent', () => {
+  const s = formatUsageSummary({ input: 10, output: 5 }, 0);
+  expect(s).toContain('cache read 0');
+  expect(s).not.toContain('reasoning');
+  expect(s).not.toContain('cost');
+});

src/commands/ci.ts

@@ -1,7 +1,8 @@
 import { readFile } from 'node:fs/promises';
 
+import type { LoadedConfig } from '../config/schema.js';
 import { loadReviewConfig } from '../config/load.js';
-import { repoRoot } from '../core/exec.js';
+import { repoRoot, run } from '../core/exec.js';
 import { errorMessage } from '../core/util.js';
 import { runReview } from '../core/review.js';
 import { GitHubPRSource } from '../sources/github-pr.js';
@@ -75,6 +76,27 @@ export async function ciCommand(argv: string[] = []): Promise<void> {
     return;
   }
 
+  // Config-driven trigger policy (.expo-code-review/config.jsonc → review): decide
+  // whether this PR should be reviewed at all. Fetch current labels via gh (more
+  // authoritative than the possibly-stale event payload); on failure, default to
+  // reviewing so a label-read hiccup never silently skips a PR.
+  let labels: string[] = [];
+  try {
+    const { stdout } = await run(
+      'gh',
+      ['pr', 'view', String(prNumber), '--repo', repo, '--json', 'labels', '--jq', '.labels[].name'],
+      { cwd: process.cwd() }
+    );
+    labels = stdout.split('\n').map(name => name.trim()).filter(Boolean);
+  } catch (error) {
+    process.stderr.write(`CI reviewer: could not read PR labels (continuing): ${errorMessage(error)}\n`);
+  }
+  const gate = shouldReview(labels, config.review);
+  if (!gate.review) {
+    process.stderr.write(`CI reviewer: skipping — ${gate.reason}.\n`);
+    return;
+  }
+
   const reporter = new GitHubReporter({
     prNumber,
     repo,
@@ -124,6 +146,31 @@ export async function ciCommand(argv: string[] = []): Promise<void> {
 }
 
 
+/**
+ * Decide whether a PR should be reviewed, given its labels and the repo's trigger
+ * policy. `skipLabel` always wins (write-gated opt-out). In "label" mode a PR must
+ * carry `label` or a `label:<agent>` variant; in "all" mode every non-skipped PR
+ * is reviewed. Pure so it's unit-testable and matches exact label names (no
+ * substring surprises like `ai-review:skip` satisfying an `ai-review` check).
+ */
+export function shouldReview(
+  labels: string[],
+  review: LoadedConfig['review']
+): { review: boolean; reason: string } {
+  if (labels.includes(review.skipLabel)) {
+    return { review: false, reason: `the ${review.skipLabel} label is set` };
+  }
+  if (review.trigger === 'label') {
+    const optedIn = labels.some(
+      name => name === review.label || name.startsWith(`${review.label}:`)
+    );
+    return optedIn
+      ? { review: true, reason: `the ${review.label} label is set` }
+      : { review: false, reason: `trigger is "label" and no ${review.label} label is set` };
+  }
+  return { review: true, reason: 'trigger is "all"' };
+}
+
 /** Parse `--agents a,b,c` from argv (undefined = all agents). */
 function parseAgents(argv: string[]): string[] | undefined {
   const index = argv.indexOf('--agents');

src/commands/doctor.ts

@@ -1,4 +1,5 @@
 import { loadReviewConfig, hasConfig } from '../config/load.js';
+import { checkProviderAuth } from '../core/auth.js';
 import { onPath, repoRoot, run } from '../core/exec.js';
 import { errorMessage } from '../core/util.js';
 
@@ -74,21 +75,8 @@ export async function doctorCommand(argv: string[] = []): Promise<void> {
         'all agent prompt files resolved and non-empty'
       );
 
-      const { mode, provider, tokenEnv } = config.auth;
-      if (tokenEnv) {
-        const present = Boolean(process.env[tokenEnv]);
-        line(
-          present,
-          present
-            ? `auth: ${mode} for ${provider}; token env ${tokenEnv} is set`
-            : `auth: ${mode} for ${provider}; token env ${tokenEnv} is NOT set`
-        );
-      } else {
-        line(
-          true,
-          `auth: ${mode} for ${provider}; no tokenEnv configured — relying on OpenCode's own login or REVIEWER_MODEL`
-        );
-      }
+      const readiness = checkProviderAuth(config);
+      line(readiness.ok, `auth: ${readiness.detail}`);
     } catch (error) {
       line(false, `config invalid: ${errorMessage(error)}`);
     }

src/commands/review.ts

@@ -18,7 +18,8 @@ Source (pick one):
   (default)          diff the working tree against the merge-base
   --base <ref>       base ref to diff against
   --head <ref>       head ref to diff
-  --staged           review only staged changes
+  --staged           review only staged changes (index vs HEAD; not combinable
+                     with --base/--head)
   --pr <n>           review GitHub PR #n by number (diff fetched via \`gh\`, no
                      checkout needed); can't be combined with --base/--head/--staged
 
@@ -212,6 +213,11 @@ function validateArgs(args: ReviewArgs): void {
   if (args.pr == null && (args.repo || args.post)) {
     throw new Error('--repo/--post only apply together with --pr.');
   }
+  // --staged diffs the index against HEAD, so --base/--head have no effect. Reject
+  // the combination rather than silently ignoring the range the user asked for.
+  if (args.staged && (args.base || args.head)) {
+    throw new Error('--staged reviews the staged changes (index vs HEAD) and cannot be combined with --base/--head.');
+  }
 }
 
 /** Resolve owner/repo from the current checkout via gh (for --post). */

src/config/load.ts

@@ -105,6 +105,7 @@ export async function loadReviewConfig(repoRoot: string): Promise<LoadedConfig>
       provider: parsed.auth.provider,
       tokenEnv: parsed.auth.tokenEnv,
     },
+    review: parsed.review,
   };
 }
 

src/config/schema.ts

@@ -63,6 +63,21 @@ export const ReviewConfigSchema = z.object({
       tokenEnv: z.string().optional(),
     })
     .default({ mode: 'api-key', provider: 'anthropic' }),
+  review: z
+    .object({
+      // Which PRs `ecr ci` acts on — the source of truth for trigger policy (a
+      // workflow `if:` gate, if any, is an optional coarse filter layered on top):
+      //   "all"   — review every PR, unless it carries the `skipLabel`.
+      //   "label" — review only PRs carrying `label` (e.g. `ai-review`) or a
+      //             `label:<agent>` variant. `skipLabel` still wins.
+      trigger: z.enum(['all', 'label']).default('all'),
+      // Opt-in label (and prefix for `label:<agent>`) used when trigger is "label".
+      label: z.string().default('ai-review'),
+      // Opt a single PR out of review. A label (not a config flag) because labels
+      // are write-gated to maintainers — a PR author can't add one to dodge review.
+      skipLabel: z.string().default('ai-review:skip'),
+    })
+    .default({ trigger: 'all', label: 'ai-review', skipLabel: 'ai-review:skip' }),
 });
 export type RawReviewConfig = z.infer<typeof ReviewConfigSchema>;
 
@@ -110,4 +125,9 @@ export interface LoadedConfig {
     provider: string;
     tokenEnv?: string;
   };
+  review: {
+    trigger: 'all' | 'label';
+    label: string;
+    skipLabel: string;
+  };
 }

src/core/auth.ts

@@ -42,6 +42,84 @@ const FORBIDDEN_TOKEN_ENVS = new Set([
 
 const YEAR_MS = 365 * 24 * 60 * 60 * 1000;
 
+export interface AuthReadiness {
+  /** True when the configured provider has (or plausibly has) a usable credential. */
+  ok: boolean;
+  /** Human-readable detail for `doctor` output and fail-fast error messages. */
+  detail: string;
+}
+
+/**
+ * Decide whether the configured model provider has a usable credential, WITHOUT
+ * mutating the environment. Shared by `prepareAuth` (fail fast before spinning up
+ * the server and every pass) and `doctor` (report), so the two never drift.
+ *
+ * We only report `ok: false` when we're confident there is no credential — a
+ * missing OAuth token, a forbidden tokenEnv, or an api-key run with neither the
+ * configured tokenEnv nor the provider's own key env set. When nothing is
+ * configured and no known key env is present, we assume OpenCode's own login may
+ * cover it and don't hard-fail. `REVIEWER_MODEL` bypasses provider auth entirely.
+ */
+export function checkProviderAuth(
+  config: LoadedConfig,
+  env: NodeJS.ProcessEnv = process.env
+): AuthReadiness {
+  const { mode, provider, tokenEnv } = config.auth;
+
+  if (env.REVIEWER_MODEL) {
+    return {
+      ok: true,
+      detail: `REVIEWER_MODEL override (${env.REVIEWER_MODEL}); using OpenCode's own login for that model`,
+    };
+  }
+
+  if (tokenEnv && FORBIDDEN_TOKEN_ENVS.has(tokenEnv)) {
+    return {
+      ok: false,
+      detail:
+        `auth.tokenEnv is "${tokenEnv}", a well-known non-provider secret; refusing to ` +
+        `forward it to the model provider (that would leak it). Point auth.tokenEnv at a ` +
+        `token minted for the provider instead.`,
+    };
+  }
+
+  if (mode === 'oauth') {
+    if (!tokenEnv) {
+      return {
+        ok: false,
+        detail: 'auth.mode "oauth" requires auth.tokenEnv to name the env var holding the OAuth token.',
+      };
+    }
+    if (!env[tokenEnv]) {
+      return { ok: false, detail: `auth is oauth for ${provider} but token env "${tokenEnv}" is not set.` };
+    }
+    return { ok: true, detail: `oauth for ${provider}; token env ${tokenEnv} is set` };
+  }
+
+  // api-key: usable if the configured tokenEnv is set, or the provider's own key
+  // env is already present in the environment.
+  const providerKeyEnv = PROVIDER_KEY_ENV[provider];
+  if (tokenEnv && env[tokenEnv]) {
+    return { ok: true, detail: `api-key for ${provider}; token env ${tokenEnv} is set` };
+  }
+  if (providerKeyEnv && env[providerKeyEnv]) {
+    return { ok: true, detail: `api-key for ${provider}; ${providerKeyEnv} is set` };
+  }
+  if (!tokenEnv && !providerKeyEnv) {
+    return {
+      ok: true,
+      detail: `api-key for ${provider}; no tokenEnv configured and no known key env — relying on OpenCode's own login`,
+    };
+  }
+  const names = [tokenEnv, providerKeyEnv].filter(Boolean).join(' or ');
+  return {
+    ok: false,
+    detail:
+      `configured api-key for ${provider} but no credential is set — set ${names}, or set ` +
+      `REVIEWER_MODEL to a model you're already logged into.`,
+  };
+}
+
 /**
  * Prepare model credentials for the OpenCode server based on the repo's auth mode.
  * Must run before the server starts (it mutates env). Returns a cleanup handle.
@@ -65,14 +143,13 @@ export async function prepareAuth(config: LoadedConfig): Promise<PreparedAuth> {
     return noop;
   }
 
-  // Refuse to forward a well-known unrelated secret as the provider credential,
-  // even if the (repo/PR-controlled) config names one — that would leak it.
-  if (tokenEnv && FORBIDDEN_TOKEN_ENVS.has(tokenEnv)) {
-    throw new Error(
-      `auth.tokenEnv is set to "${tokenEnv}", a well-known non-provider secret. Refusing ` +
-        `to forward it to the model provider (that would leak the secret). Point auth.tokenEnv ` +
-        `at a token minted for the model provider instead.`
-    );
+  // Fail fast, before starting the server and every pass, if the configured
+  // provider has no usable credential — otherwise it surfaces as N failed passes
+  // mid-run. This is the same readiness check `doctor` reports, and it also covers
+  // the forbidden-secret guard (refusing to forward a well-known unrelated secret).
+  const readiness = checkProviderAuth(config);
+  if (!readiness.ok) {
+    throw new Error(readiness.detail);
   }
 
   if (mode === 'api-key') {
@@ -88,15 +165,11 @@ export async function prepareAuth(config: LoadedConfig): Promise<PreparedAuth> {
     return noop;
   }
 
-  // oauth
-  if (!tokenEnv) {
-    throw new Error(
-      'auth.mode "oauth" requires auth.tokenEnv to name the env var holding the OAuth token.'
-    );
-  }
-  const token = process.env[tokenEnv];
+  // oauth — checkProviderAuth guarantees tokenEnv is set and present; read
+  // defensively so TypeScript narrows and this stays correct if called directly.
+  const token = tokenEnv ? process.env[tokenEnv] : undefined;
   if (!token) {
-    throw new Error(`OAuth token env "${tokenEnv}" is not set.`);
+    throw new Error('auth.mode "oauth" requires auth.tokenEnv to name a set OAuth token env.');
   }
 
   const dir = await mkdtemp(path.join(tmpdir(), 'ecr-auth-'));

src/core/opencode.ts

@@ -2,7 +2,7 @@ import { createOpencode } from '@opencode-ai/sdk';
 
 import type { LoadedConfig } from '../config/schema.js';
 import { toolMap } from './tools.js';
-import { sleep } from './util.js';
+import { errorMessage, sleep } from './util.js';
 
 export interface OpencodeHandle {
   client: any;
@@ -320,6 +320,71 @@ const CORRECTIVE =
 // it should return almost immediately.
 const CORRECTIVE_WAIT_MS = 2 * 60 * 1000;
 
+/** Backoff (ms) before the 2nd and 3rd attempt of a transient-failing model call. */
+const TRANSIENT_BACKOFF_MS = [2_000, 8_000];
+
+/**
+ * A transient, retryable API failure — a one-off rate-limit (429), server error
+ * (5xx), or network blip — as opposed to a timeout (which means "abandon", see
+ * AgentTimeoutError) or a JSON-parse failure (handled by the corrective re-emit in
+ * promptAndParse). We match on the error text because the OpenCode SDK surfaces
+ * these as plain Errors; an AgentTimeoutError is never transient.
+ */
+const TRANSIENT_PATTERNS: RegExp[] = [
+  /\b429\b/,
+  /\b50[0-9]\b/,
+  /rate.?limit/i,
+  /overloaded/i,
+  /too many requests/i,
+  /temporarily unavailable/i,
+  /ETIMEDOUT/i,
+  /ECONNRESET/i,
+  /ECONNREFUSED/i,
+  /ENOTFOUND/i,
+  /EAI_AGAIN/i,
+  /socket hang ?up/i,
+  /network error/i,
+  /fetch failed/i,
+];
+
+export function isTransientApiError(error: unknown): boolean {
+  if (error instanceof AgentTimeoutError) {
+    return false;
+  }
+  const message = errorMessage(error);
+  return TRANSIENT_PATTERNS.some(pattern => pattern.test(message));
+}
+
+/**
+ * Run a model call, retrying with bounded backoff on a transient API error. This
+ * is deliberately separate from the timeout path (abandon, never retry) and the
+ * parse-failure path (corrective re-emit): a one-off 429/5xx/network error used to
+ * drop the whole pass with no retry, reported as a coverage gap. Non-transient
+ * errors (incl. AgentTimeoutError) propagate immediately.
+ */
+async function withTransientRetry<T>(
+  label: string,
+  onActivity: ((line: string) => void) | undefined,
+  fn: () => Promise<T>
+): Promise<T> {
+  for (let attempt = 0; ; attempt++) {
+    try {
+      return await fn();
+    } catch (error) {
+      const waitMs = TRANSIENT_BACKOFF_MS[attempt];
+      if (waitMs === undefined || !isTransientApiError(error)) {
+        throw error;
+      }
+      onActivity?.(
+        `${label}: transient API error (${errorMessage(error)}); retry ${attempt + 1}/${
+          TRANSIENT_BACKOFF_MS.length
+        } in ${Math.round(waitMs / 1000)}s`
+      );
+      await sleep(waitMs);
+    }
+  }
+}
+
 /**
  * Prompt an agent and parse its reply. On a JSON-parse failure, first retry in
  * the SAME session: the model still holds all the file context it read, so the
@@ -353,7 +418,9 @@ export async function promptAndParse<T>(
     addTokenUsage(tokens, result.tokens);
   };
 
-  const first = await promptAgent(handle, args);
+  const first = await withTransientRetry(`Agent "${args.agent}"`, args.onActivity, () =>
+    promptAgent(handle, args)
+  );
   record(first);
   try {
     return { value: parse(first.text), cost, truncated, tokens };

src/core/review.ts

@@ -348,8 +348,13 @@ export async function runReview(
         if (!(error instanceof AgentTimeoutError)) {
           failedPasses++;
           progress(`  ${task.label}: FAILED (${errorMessage(error)})`);
+          // An auth/permission failure hits every pass identically; push one shared,
+          // actionable note (deduped into a single coverage line) instead of N generic
+          // per-pass failures that bury the real, fixable cause.
           incomplete.push(
-            `${capitalize(task.coverageLabel)} failed to run; those changes were not reviewed.`
+            isAuthError(error)
+              ? AUTH_FAILURE_NOTE
+              : `${capitalize(task.coverageLabel)} failed to run; those changes were not reviewed.`
           );
           return;
         }
@@ -510,6 +515,8 @@ export async function runReview(
       output = { ...output, summary: reconcileSummary(output.summary, output.findings.length) };
     }
 
+    progress(formatUsageSummary(tokenTotals, sum(agentCosts)));
+
     await safeLog(logPath, {
       ...baseRecord,
       agentCosts,
@@ -644,6 +651,34 @@ function capitalize(text: string): string {
   return text.length > 0 ? text[0]!.toUpperCase() + text.slice(1) : text;
 }
 
+/**
+ * An authentication/authorization failure from the model provider (401/403, a
+ * rejected/expired/missing credential) — distinct from a transient blip or a real
+ * code finding. Every pass hits the same wall, so the caller collapses it into one
+ * actionable coverage note instead of N generic "failed to run" lines.
+ */
+export function isAuthError(error: unknown): boolean {
+  const message = errorMessage(error).toLowerCase();
+  const cred = /(api.?key|token|credential)/;
+  const problem = /(invalid|expired|revoked|missing|rejected|no)/;
+  return (
+    /\b401\b|\b403\b/.test(message) ||
+    /unauthor/.test(message) ||
+    /\bforbidden\b/.test(message) ||
+    /authentication/.test(message) ||
+    /permission denied/.test(message) ||
+    /invalid x-api-key/.test(message) ||
+    // a credential noun and a problem word near each other, in either order
+    new RegExp(`${problem.source}\\b[^.]{0,20}${cred.source}`).test(message) ||
+    new RegExp(`${cred.source}[^.]{0,20}${problem.source}`).test(message)
+  );
+}
+
+const AUTH_FAILURE_NOTE =
+  'The model provider rejected the request (authentication or permission). Check the ' +
+  'configured credential (auth.tokenEnv, or REVIEWER_MODEL for a local run) and re-run — ' +
+  'those changes were not reviewed.';
+
 function selectAgents(all: LoadedAgent[], filter?: string[]): LoadedAgent[] {
   if (!filter?.length) {
     return all;
@@ -734,6 +769,22 @@ function sum(costs: Record<string, number>): number {
   return Object.values(costs).reduce((total, value) => total + value, 0);
 }
 
+/**
+ * One-line usage summary for the run. Emitted via progress so it lands in the CI
+ * job log (and the local terminal) — `.runs/reviews.jsonl` is ephemeral in CI, so
+ * this is the only place the token/cache totals are visible after a CI run, which
+ * is how prompt-cache effectiveness gets confirmed there.
+ */
+export function formatUsageSummary(tokens: TokenUsage, totalCost: number): string {
+  const parts = [`input ${tokens.input ?? 0}`, `output ${tokens.output ?? 0}`];
+  if (tokens.reasoning) {
+    parts.push(`reasoning ${tokens.reasoning}`);
+  }
+  parts.push(`cache read ${tokens.cache?.read ?? 0}`, `cache write ${tokens.cache?.write ?? 0}`);
+  const cost = totalCost > 0 ? ` (cost $${totalCost.toFixed(4)})` : '';
+  return `Token usage — ${parts.join(', ')}${cost}`;
+}
+
 
 async function safeLog(logPath: string, record: RunLogRecord): Promise<void> {
   try {

templates/config.jsonc

@@ -25,6 +25,16 @@
   // full-context pass. Defaults shown; raise/lower per your model + PR sizes.
   // "chunk": { "maxChangedLines": 1000, "maxFiles": 20, "concurrency": 4 },
 
+  // Which PRs `ecr ci` reviews. This is the source of truth for trigger policy;
+  // the scaffolded workflow always calls `ecr ci` and lets this decide. (If you'd
+  // rather gate in the workflow instead, add an `if:` to the job — see the
+  // workflow template — and this still applies on top.)
+  //   "trigger": "all"   — review every PR, unless it has the `skipLabel`.
+  //   "trigger": "label" — review only PRs labeled `label` (or `label:<agent>`).
+  // The `skipLabel` is write-gated (only maintainers can add labels), so a PR
+  // author can't opt their own PR out.
+  "review": { "trigger": "all", "label": "ai-review", "skipLabel": "ai-review:skip" },
+
   // A maintainer comment containing this marker skips the CI review.
   "breakGlass": { "marker": "/skip-review" },
 

templates/workflow.yml

@@ -17,9 +17,14 @@ concurrency:
 jobs:
   review:
     runs-on: ubuntu-latest
-    # Opt-in per PR: only run when the `ai-review` label is present. Remove this
-    # line to review every PR automatically.
-    if: contains(join(github.event.pull_request.labels.*.name, ','), 'ai-review')
+    # Trigger policy lives in .expo-code-review/config.jsonc (review.trigger); `ecr ci`
+    # self-gates on it (and honors the ai-review:skip label). This coarse gate just
+    # avoids spinning up a runner for a PR that explicitly opted out. Uses the array
+    # form of contains() for an EXACT label match ("ai-review:skip" is not "ai-review").
+    # Prefer to gate entirely here instead? Set config trigger to "label" and replace
+    # the line below with, e.g.:
+    #   if: contains(github.event.pull_request.labels.*.name, 'ai-review')
+    if: ${{ !contains(github.event.pull_request.labels.*.name, 'ai-review:skip') }}
     # Backstop so a stalled review fails fast instead of hanging.
     timeout-minutes: 60
     # A reviewer failure must never fail the PR's checks.