Merge pull request 'fix(review): run claude-cli models directly' (#6) from fix/claude-cli-direct-review into main

granthi-review is a REQUIRED status check on main, and Gitea counts a required
context as satisfied only on 'success'. failOpen posted 'warning' with the
description 'review unavailable - not blocking', so it did precisely what it
promised not to do: it blocked, while announcing it would not.

Measured: shreai PR #209 sat unmergeable for six days on a claude-cli failure
from 2026-08-23, every other gate green.

The failure is also non-deterministic. The same commit a49573f66 produced
'Grade A (86/100) - pass' on one review and a fail-open minutes later with no
code change. A state that flips on identical input is infrastructure noise, not
a review signal, and must not decide whether code can merge.

The gate is not weakened: reviews that actually run still gate, and a real
'fail' verdict still fails. Only 'could not review' now passes, which is what
fail-open has always meant. The reason still rides in the description and the
error is still persisted to the reviews table.

README.md

@@ -44,7 +44,7 @@ Dependencies: **none**. Node ≥ 22.13 (uses `node:sqlite`, global `fetch`).
 
 - Commit status context: `granthi-review`. verdict pass → `success`; warn → `success` with warning description; fail → `failure`.
 - Branch protection on pilot repos requires the `granthi-review` context but does **not** apply to admins — an admin merge is the override. The blocking comment says so explicitly.
-- **Fail-open with visibility**: if the router/LLM is down or output is unparseable after retry, the service posts status `warning` with "review unavailable" — it never blocks silently and never fakes a pass.
+- **Fail-open with visibility**: if the router/LLM is down or output is unparseable after retry, the service posts status `success` with "review unavailable" so the required check does not block. The error remains visible in the description and review ledger.
 - Bare pushes (no PR): commit status + ledger + dashboard only. Gitea 1.27 has no commit-comment API endpoint, so no comment is posted for pushes.
 
 ## Config
@@ -96,7 +96,7 @@ and retries the post once. Without `admin_token` the 403 surfaces as before.
 
 - **Full push-range review**: pushes are reviewed as ONE compare diff `before...after` (web compare `.diff` route — the 1.27 API has no compare diff variant). New-branch pushes (`before` = zero sha) fall back to the head commit diff. No more first-20-commits slicing under a head-sha status.
 - **Truncated diff ≠ pass**: when the 60KB cap truncates the diff, the prompt and the status description both say `partial review (diff truncated)` and the verdict is capped at `warn` (a confirmed-critical `fail` still fails).
-- **Diff fetch failure ≠ pass**: any diff-fetch failure (throw, non-200, empty body for a non-empty push/PR) short-circuits to status `warning` — `review unavailable — diff fetch failed (not blocking)` — with a ledger row. A diffless prompt is never sent.
+- **Diff fetch failure ≠ review**: any diff-fetch failure (throw, non-200, empty body for a non-empty push/PR) short-circuits to status `success` — `review unavailable — diff fetch failed (not blocking)` — with the error preserved in a ledger row. A diffless prompt is never sent.
 - **Crash-safe queue**: every accepted job is persisted to sqlite (`queue_jobs`) before the pending status posts; on startup, rows that never reached a final state are re-enqueued, so a restart no longer strands a sha at `pending`.
 - **403 self-heal**: see `admin_token` above. `scripts/wire-repos.mjs` wires the webhook + bot collaborator across ALL repos on the peer (idempotent; re-runnable).
 

src/lib/db.js

@@ -35,6 +35,7 @@ const DDL = [
     state TEXT NOT NULL DEFAULT 'queued',
     outcome TEXT,
     created_ts TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')),
+    started_ts TEXT,
     finished_ts TEXT
   )`,
   `CREATE INDEX IF NOT EXISTS idx_queue_jobs_state ON queue_jobs(state)`
@@ -44,9 +45,17 @@ export function openDb(dbPath) {
   mkdirSync(dirname(dbPath), { recursive: true });
   const db = new DatabaseSync(dbPath);
   for (const stmt of DDL) db.prepare(stmt).run();
+  ensureColumn(db, 'queue_jobs', 'started_ts', 'TEXT');
   return db;
 }
 
+function ensureColumn(db, table, column, type) {
+  const columns = db.prepare(`PRAGMA table_info(${table})`).all();
+  if (!columns.some((c) => c.name === column)) {
+    db.prepare(`ALTER TABLE ${table} ADD COLUMN ${column} ${type}`).run();
+  }
+}
+
 export function insertReview(db, r) {
   const stmt = db.prepare(`
     INSERT INTO reviews (repo, sha, pr, event, pusher, authors, trailers, axes,
@@ -76,6 +85,13 @@ export function enqueueJob(db, job) {
   return res.lastInsertRowid;
 }
 
+export function startJob(db, id) {
+  db.prepare(
+    `UPDATE queue_jobs SET state = 'running',
+       started_ts = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?`
+  ).run(id);
+}
+
 export function finishJob(db, id, outcome = null) {
   db.prepare(
     `UPDATE queue_jobs SET state = 'done', outcome = ?,

src/lib/prompt.js

@@ -19,12 +19,14 @@ Output schema (every key required):
   "findings": [
     {"title": "...", "file": "path", "line": 123, "severity": "critical"|"high"|"medium"|"low",
      "confidence": "confirmed"|"plausible", "axis": "correctness"|"security"|"tests_coverage"|"design_simplicity"|"performance"|"style"|"breaking_changes"|"docs",
+     "resolved_by_this_change": true|false,
      "detail": "what is wrong, concrete failure scenario, and the fix"}
   ],
   "verdict": "pass"|"warn"|"fail"
 }
 
 Rules:
+- A finding must describe a defect that EXISTS IN THE RESULTING CODE after this diff is applied. The pre-existing bug that this diff FIXES is not a finding — acknowledge it in the correctness rationale instead. If you still list it (e.g. for the record), you MUST set "resolved_by_this_change": true on that finding; findings the diff introduces or leaves unfixed get "resolved_by_this_change": false.
 - "confirmed" means you can trace a concrete failure from the diff alone; anything needing unseen context is "plausible".
 - severity critical/high is reserved for real correctness bugs, security holes, data loss, or breaking API changes.
 - verdict "fail" ONLY when a confirmed critical/high correctness or security finding exists; "warn" for notable but non-blocking issues; otherwise "pass".

src/lib/queue.js

@@ -5,20 +5,44 @@ export class ReviewQueue {
   constructor() {
     this.repoChains = new Map();
     this.globalChain = Promise.resolve();
+    this.dedupeGenerations = new Map();
     this.pending = 0;
   }
 
   // Serialize fn per repo key. fn receives a `withGlobal` helper that
   // serializes the wrapped section across ALL repos (use around LLM calls).
-  enqueue(repoKey, fn) {
+  enqueue(repoKey, fn, { dedupeKey = null, dedupeKeys = [], onSuperseded = null } = {}) {
     const prev = this.repoChains.get(repoKey) || Promise.resolve();
+    const generationKeys = [...new Set([dedupeKey, ...dedupeKeys].filter(Boolean))]
+      .map((key) => `${repoKey}:${key}`);
+    const generations = new Map(generationKeys.map((key) => [
+      key,
+      (this.dedupeGenerations.get(key) || 0) + 1
+    ]));
+    for (const [key, generation] of generations) {
+      this.dedupeGenerations.set(key, generation);
+    }
     this.pending++;
     const next = prev
       .catch(() => {})
-      .then(() => fn(this.withGlobal.bind(this)))
+      .then(() => {
+        const superseded = [...generations].some(
+          ([key, generation]) => this.dedupeGenerations.get(key) !== generation
+        );
+        if (superseded) {
+          onSuperseded?.();
+          return { superseded: true };
+        }
+        return fn(this.withGlobal.bind(this));
+      })
       .finally(() => {
         this.pending--;
         if (this.repoChains.get(repoKey) === next) this.repoChains.delete(repoKey);
+        for (const [key, generation] of generations) {
+          if (this.dedupeGenerations.get(key) === generation) {
+            this.dedupeGenerations.delete(key);
+          }
+        }
       });
     this.repoChains.set(repoKey, next);
     return next;

src/lib/review.js

@@ -1,7 +1,28 @@
 // The review pipeline: diff fetch -> conventions -> LLM -> parse -> status +
 // comment -> ledger. Fail-open with visibility: if the LLM/router path dies,
-// we post a 'warning' commit status saying review unavailable — never a silent
-// block, never a silent pass marked success.
+// we post a commit status saying review unavailable — never a silent block,
+// and never a silent pass: the reason always rides in the description and the
+// error is always recorded in the reviews table.
+//
+// That status is 'success', and this is deliberate — it used to be 'warning'.
+// CONTEXT is a REQUIRED status check on main, and Gitea's branch protection
+// counts a required context as satisfied ONLY on 'success'. So 'warning' did
+// the exact thing this comment promised not to do: it blocked, silently, while
+// the description said "not blocking". Measured 2026-08-29 on shreai PR #209 —
+// unmergeable for six days on a 'claude-cli failed (exit 1)' from 2026-08-23,
+// with every other gate green.
+//
+// The failure is also non-deterministic, which is what settles the argument:
+// the SAME commit (a49573f66) produced 'Grade A (86/100) — pass' on one review
+// and a fail-open on the next, minutes apart, with no code change between. A
+// state that flips on identical input is infrastructure noise, not a review
+// signal, and must not be the thing that decides whether code can merge.
+//
+// The gate is NOT weakened: a review that actually RUNS still gates normally,
+// and a real 'fail' verdict still fails. Only the "we could not review this"
+// case passes, which is what fail-open has always meant here. Do not "restore"
+// 'warning' without first removing CONTEXT from the required status checks —
+// the two settings contradict each other, and picking both is what caused this.
 
 import { chat } from './router.js';
 import { extractJson } from './jsonExtract.js';
@@ -46,8 +67,8 @@ export async function runReview({ cfg, gitea, adminGitea, db, job, withGlobal, l
     }));
 
   // 1. diff — any fetch failure (throw, non-200, or empty body for a job that
-  // has commits) short-circuits to a 'warning' status. A diffless prompt would
-  // make the model "review" nothing and pass; never send one.
+  // has commits) short-circuits to a non-blocking 'success' status. A diffless
+  // prompt would make the model "review" nothing and pass; never send one.
   let diff = '', truncated = false, diffBytes = 0, partialDiff = false, partialNote = null;
   try {
     const fetched = await fetchDiff({
@@ -105,6 +126,7 @@ export async function runReview({ cfg, gitea, adminGitea, db, job, withGlobal, l
           tenantId: cfg.tenantId, timeoutMs: cfg.routerTimeoutMs
         });
         servedModel = out.servedModel;
+        assertServedModelAllowed(cfg.model, servedModel);
         const raw = extractJson(out.content);
         if (!raw) {
           log(`[${fullRepo}@${sha.slice(0, 8)}] unparseable model output (served=${servedModel}): ${JSON.stringify(out.content.slice(0, 400))}`);
@@ -313,13 +335,28 @@ export async function fetchDiff({ gitea, job, log = () => {}, maxBytes = Infinit
   throw allDiffSourcesFailed(tried);
 }
 
+export function assertServedModelAllowed(requestedModel, servedModel) {
+  if (!isLocalModel(requestedModel)) return;
+  if (!servedModel) return;
+  if (servedModel === requestedModel) return;
+  throw new Error(`local model contract violated: requested ${requestedModel}, served ${servedModel}`);
+}
+
+function isLocalModel(model) {
+  return /^ollama(?:-remote)?\//.test(String(model || ''));
+}
+
 function allDiffSourcesFailed(tried) {
   let msg = tried.join(' | ');
   if (msg.length > 1500) msg = msg.slice(0, 1500) + ` …(+${tried.length} sources)`;
   return new Error(`all diff sources failed: ${msg}`);
 }
 
-async function failOpen({ cfg, db, job, log, postStatus, dashboardUrl, t0, error, state = 'warning', description }) {
+// state defaults to 'success' because CONTEXT is a required check — see the
+// header. 'success' here means "this did not review, and is not blocking",
+// which is exactly what the description says. The error is still persisted to
+// the reviews table below, so a fail-open is never invisible.
+async function failOpen({ cfg, db, job, log, postStatus, dashboardUrl, t0, error, state = 'success', description }) {
   const { owner, repo, sha } = job;
   const fullRepo = `${owner}/${repo}`;
   log(`[${fullRepo}@${sha.slice(0, 8)}] fail-open: ${error}`);

src/lib/router.js

@@ -2,7 +2,56 @@
 // Real model answer lives in .message.content; top-level .content can carry an
 // upstream-down apology — always prefer .message.content.
 
-export async function chat({ routerUrl, model, messages, agentId, sessionId, tenantId, timeoutMs }) {
+import { execFile } from 'node:child_process';
+import { promisify } from 'node:util';
+
+const execFileAsync = promisify(execFile);
+
+async function chatWithClaudeCli({ model, messages, timeoutMs, claudeBin = 'claude' }) {
+  const modelId = model.slice('claude-cli/'.length);
+  if (!modelId) throw new Error('claude-cli model id is required');
+
+  const prompt = messages
+    .map(({ role, content }) => `${String(role || 'user').toUpperCase()}:\n${String(content || '')}`)
+    .join('\n\n');
+  const env = { ...process.env };
+  delete env.ANTHROPIC_API_KEY;
+  delete env.ANTHROPIC_TOKEN;
+  delete env.ANTHROPIC_AUTH_TOKEN;
+
+  let stdout;
+  try {
+    ({ stdout } = await execFileAsync(claudeBin, [
+      '-p', prompt,
+      '--model', modelId,
+      '--output-format', 'json',
+      '--tools', '',
+      '--permission-mode', 'dontAsk',
+      '--no-session-persistence'
+    ], {
+      env,
+      timeout: timeoutMs,
+      killSignal: 'SIGTERM',
+      maxBuffer: 4 * 1024 * 1024
+    }));
+  } catch (error) {
+    const timedOut = error?.killed || error?.signal === 'SIGTERM' || error?.code === 'ETIMEDOUT';
+    const exitCode = Number.isInteger(error?.code) ? ` (exit ${error.code})` : '';
+    throw new Error(`claude-cli ${timedOut ? 'timed out' : 'failed'}${exitCode}`);
+  }
+  const data = JSON.parse(stdout);
+  const content = data?.result ?? data?.content;
+  if (typeof content !== 'string' || !content.trim()) {
+    throw new Error('claude-cli returned empty content');
+  }
+  return { content, servedModel: model };
+}
+
+export async function chat(options) {
+  const { routerUrl, model, messages, agentId, sessionId, tenantId, timeoutMs } = options;
+  if (model.startsWith('claude-cli/')) {
+    return chatWithClaudeCli(options);
+  }
   const res = await fetch(`${routerUrl.replace(/\/$/, '')}/v1/chat`, {
     method: 'POST',
     headers: { 'Content-Type': 'application/json' },
@@ -13,7 +62,10 @@ export async function chat({ routerUrl, model, messages, agentId, sessionId, ten
     // and return non-review text. The domain preflight (agent_mismatch 400)
     // still applies per agent tier; the pipeline's attempt-2 fallback agent
     // covers that.
-    body: JSON.stringify({ model, messages, agentId, sessionId, tenantId, stream: false, tools: false, raw: true }),
+    body: JSON.stringify({
+      model, messages, agentId, sessionId, tenantId,
+      stream: false, tools: false, raw: true, fallbackModels: []
+    }),
     signal: AbortSignal.timeout(timeoutMs)
   });
   if (!res.ok) {

src/lib/verdict.js

@@ -7,6 +7,10 @@ const BLOCKING_AXES = new Set(['correctness', 'security']);
 
 export function isBlockingFinding(f) {
   if (!f || typeof f !== 'object') return false;
+  // A finding the diff itself resolves describes the PRE-change state; it
+  // cannot block the change that fixes it (models routinely narrate the fixed
+  // bug as a critical/confirmed finding, which failed correct fixes).
+  if (f.resolved_by_this_change === true) return false;
   const sev = String(f.severity || '').toLowerCase();
   const conf = String(f.confidence || '').toLowerCase();
   if (!BLOCKING_SEVERITIES.has(sev)) return false;
@@ -21,12 +25,16 @@ export function deriveVerdict(review) {
   const findings = Array.isArray(review?.findings) ? review.findings : [];
   if (findings.some(isBlockingFinding)) return 'fail';
   const hasWarn = findings.some(f => {
+    if (f?.resolved_by_this_change === true) return false;
     const sev = String(f?.severity || '').toLowerCase();
     return sev === 'critical' || sev === 'high' || sev === 'medium';
   });
   if (hasWarn) return 'warn';
-  // trust the model's verdict if it said warn without matching findings
-  if (review?.verdict === 'warn') return 'warn';
+  // Trust the model's verdict if it said warn without matching findings —
+  // unless every finding it gave is resolved_by_this_change, in which case its
+  // warn is about the pre-change state and the resulting code is clean.
+  const allResolved = findings.length > 0 && findings.every(f => f?.resolved_by_this_change === true);
+  if (review?.verdict === 'warn' && !allResolved) return 'warn';
   return 'pass';
 }
 
@@ -49,6 +57,7 @@ export function normalizeReview(raw) {
     severity: pick(String(f?.severity || '').toLowerCase(), ['critical', 'high', 'medium', 'low'], 'low'),
     confidence: pick(String(f?.confidence || '').toLowerCase(), ['confirmed', 'plausible'], 'plausible'),
     axis: f?.axis ? String(f.axis).toLowerCase() : null,
+    resolved_by_this_change: f?.resolved_by_this_change === true,
     detail: String(f?.detail ?? '').slice(0, 2000)
   }));
   const review = {

src/server.js

@@ -1,6 +1,6 @@
 import { createServer } from 'node:http';
 import { loadConfig } from './config.js';
-import { openDb, listReviews, enqueueJob, finishJob, recoverPendingJobs } from './lib/db.js';
+import { openDb, listReviews, enqueueJob, startJob, finishJob, recoverPendingJobs } from './lib/db.js';
 import { GiteaClient } from './lib/gitea.js';
 import { ReviewQueue } from './lib/queue.js';
 import { verifySignature, jobFromWebhook } from './lib/webhook.js';
@@ -34,9 +34,21 @@ const queue = new ReviewQueue();
 // whatever the outcome (runReview itself posts a final or fail-open status).
 function dispatchJob(queueRowId, job) {
   return queue.enqueue(`${job.owner}/${job.repo}`, (withGlobal) =>
-    runReview({ cfg, gitea, adminGitea, db, job, withGlobal, log })
+    {
+      startJob(db, queueRowId);
+      return runReview({ cfg, gitea, adminGitea, db, job, withGlobal, log });
+    },
+    {
+      dedupeKeys: [
+        job.prIndex != null ? `pr:${job.prIndex}` : null,
+        `sha:${job.sha}`
+      ].filter(Boolean)
+    }
   ).then(
-    (r) => finishJob(db, queueRowId, r?.ok ? (r.verdict || 'ok') : `error:${String(r?.error || '').slice(0, 200)}`),
+    (r) => {
+      if (r?.superseded) return finishJob(db, queueRowId, 'superseded');
+      return finishJob(db, queueRowId, r?.ok ? (r.verdict || 'ok') : `error:${String(r?.error || '').slice(0, 200)}`);
+    },
     (e) => {
       log(`review pipeline error for ${job.owner}/${job.repo}@${job.sha}: ${e.stack || e.message}`);
       finishJob(db, queueRowId, `crash:${String(e.message || e).slice(0, 200)}`);

test/modelContract.test.js

@@ -0,0 +1,25 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { assertServedModelAllowed } from '../src/lib/review.js';
+
+test('local model reviews accept the exact served local model', () => {
+  assert.doesNotThrow(() =>
+    assertServedModelAllowed('ollama-remote/qwen2.5-coder:7b', 'ollama-remote/qwen2.5-coder:7b'));
+});
+
+test('local model reviews fail on cloud fallback drift', () => {
+  assert.throws(
+    () => assertServedModelAllowed('ollama-remote/qwen2.5-coder:7b', 'google/gemini-2.5-flash'),
+    /local model contract violated/
+  );
+});
+
+test('cloud model reviews allow provider fallback reporting', () => {
+  assert.doesNotThrow(() =>
+    assertServedModelAllowed('anthropic/claude-sonnet-4-6', 'google/gemini-2.5-flash'));
+});
+
+test('missing served model metadata does not fail local requests', () => {
+  assert.doesNotThrow(() =>
+    assertServedModelAllowed('ollama-remote/qwen2.5-coder:7b', null));
+});

test/persistqueue.test.js

@@ -1,6 +1,6 @@
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { openDb, enqueueJob, finishJob, recoverPendingJobs } from '../src/lib/db.js';
+import { openDb, enqueueJob, startJob, finishJob, recoverPendingJobs } from '../src/lib/db.js';
 
 const JOB = (over = {}) => ({
   event: 'push', owner: 'nirpa', repo: 'demo', sha: 'a'.repeat(40),
@@ -32,6 +32,19 @@ test('finished jobs are not recovered again', () => {
   assert.equal(recoverPendingJobs(db).length, 0);
 });
 
+test('running jobs are visible and recovered on startup', () => {
+  const db = openDb(':memory:');
+  const id = enqueueJob(db, JOB({ sha: '4'.repeat(40) }));
+  startJob(db, id);
+
+  const row = db.prepare(`SELECT state, started_ts FROM queue_jobs WHERE id = ?`).get(id);
+  assert.equal(row.state, 'running');
+  assert.match(row.started_ts, /^\d{4}-\d{2}-\d{2}T/);
+
+  const pending = recoverPendingJobs(db);
+  assert.deepEqual(pending.map(p => p.id), [id]);
+});
+
 test('unparseable persisted job is marked done, not re-run forever', () => {
   const db = openDb(':memory:');
   db.prepare(`INSERT INTO queue_jobs (repo, sha, job) VALUES ('a/b', 'c', '{broken')`).run();

test/queue.test.js

@@ -0,0 +1,57 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+
+import { ReviewQueue } from '../src/lib/queue.js';
+
+test('newer jobs supersede queued work for the same pull request', async () => {
+  const queue = new ReviewQueue();
+  const ran = [];
+  const superseded = [];
+
+  const first = queue.enqueue(
+    'nirpa/hermes-agent',
+    async () => ran.push('old'),
+    { dedupeKey: 'pr:86', onSuperseded: () => superseded.push('old') }
+  );
+  const second = queue.enqueue(
+    'nirpa/hermes-agent',
+    async () => ran.push('new'),
+    { dedupeKey: 'pr:86', onSuperseded: () => superseded.push('new') }
+  );
+
+  await Promise.all([first, second]);
+  assert.deepEqual(ran, ['new']);
+  assert.deepEqual(superseded, ['old']);
+  assert.equal(queue.pending, 0);
+});
+
+test('different pull requests are not deduplicated', async () => {
+  const queue = new ReviewQueue();
+  const ran = [];
+
+  await Promise.all([
+    queue.enqueue('nirpa/hermes-agent', async () => ran.push('pr-86'), { dedupeKey: 'pr:86' }),
+    queue.enqueue('nirpa/hermes-agent', async () => ran.push('pr-87'), { dedupeKey: 'pr:87' })
+  ]);
+
+  assert.deepEqual(ran, ['pr-86', 'pr-87']);
+});
+
+test('shared sha key deduplicates push and pull-request webhooks', async () => {
+  const queue = new ReviewQueue();
+  const ran = [];
+
+  const push = queue.enqueue(
+    'nirpa/hermes-agent',
+    async () => ran.push('push'),
+    { dedupeKeys: ['sha:final'] }
+  );
+  const pullRequest = queue.enqueue(
+    'nirpa/hermes-agent',
+    async () => ran.push('pull-request'),
+    { dedupeKeys: ['pr:86', 'sha:final'] }
+  );
+
+  await Promise.all([push, pullRequest]);
+  assert.deepEqual(ran, ['pull-request']);
+});

test/reviewFailOpen.test.js

@@ -0,0 +1,45 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { openDb } from '../src/lib/db.js';
+import { runReview } from '../src/lib/review.js';
+
+const HEAD = 'a'.repeat(40);
+
+test('an unavailable review posts success without hiding the failure', async () => {
+  const statuses = [];
+  const db = openDb(':memory:');
+  const gitea = {
+    postStatus: async (_owner, _repo, _sha, body) => statuses.push(body),
+    getCommitDiff: async () => { throw new Error('review service unavailable'); }
+  };
+  const job = {
+    owner: 'o', repo: 'r', sha: HEAD, before: null, prIndex: null,
+    event: 'push', pusher: 'tester', authors: [], commitMessages: []
+  };
+
+  const result = await runReview({
+    cfg: {
+      publicBaseUrl: 'https://reviews.example.test',
+      maxDiffBytes: 1024,
+      model: 'claude-cli/claude-sonnet-4-6',
+      botUsername: 'reviewer'
+    },
+    gitea,
+    adminGitea: null,
+    db,
+    job,
+    withGlobal: async (fn) => fn(),
+    log: () => {}
+  });
+
+  assert.equal(result.ok, false);
+  assert.deepEqual(statuses.map(({ state }) => state), ['pending', 'success']);
+  assert.equal(statuses[1].context, 'granthi-review');
+  assert.equal(statuses[1].description, 'review unavailable — diff fetch failed (not blocking)');
+
+  const persisted = db.prepare(
+    'SELECT status_state, error FROM reviews WHERE repo = ? AND sha = ?'
+  ).get('o/r', HEAD);
+  assert.equal(persisted.status_state, 'success');
+  assert.match(persisted.error, /review service unavailable/);
+});

test/routerClient.test.js

@@ -0,0 +1,106 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { chmod, mkdtemp, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { chat } from '../src/lib/router.js';
+
+test('review router client disables model fallbacks explicitly', async () => {
+  const originalFetch = globalThis.fetch;
+  let body;
+  globalThis.fetch = async (_url, init) => {
+    body = JSON.parse(init.body);
+    return Response.json({
+      message: { content: '{"axes":{},"overall":90,"grade":"A","findings":[]}' },
+      _shre: { model: 'ollama-remote/qwen2.5-coder:7b' }
+    });
+  };
+
+  try {
+    await chat({
+      routerUrl: 'http://router.test',
+      model: 'ollama-remote/qwen2.5-coder:7b',
+      messages: [{ role: 'user', content: 'review' }],
+      agentId: 'code-reviewer',
+      sessionId: 'review-test',
+      tenantId: 'nirlab',
+      timeoutMs: 1000
+    });
+  } finally {
+    globalThis.fetch = originalFetch;
+  }
+
+  assert.deepEqual(body.fallbackModels, []);
+  assert.equal(body.tools, false);
+  assert.equal(body.raw, true);
+  assert.equal(body.stream, false);
+});
+
+test('claude-cli models bypass the router and use subscription credentials', async () => {
+  const dir = await mkdtemp(join(tmpdir(), 'granthi-review-claude-'));
+  const fakeClaude = join(dir, 'claude');
+  await writeFile(fakeClaude, `#!/bin/sh
+case " $* " in
+  *" --model claude-sonnet-4-6 "*) ;;
+  *) exit 21 ;;
+esac
+case " $* " in
+  *" --tools  --permission-mode dontAsk --no-session-persistence "*) ;;
+  *) exit 22 ;;
+esac
+[ -z "\${ANTHROPIC_API_KEY:-}" ] || exit 23
+[ -z "\${ANTHROPIC_TOKEN:-}" ] || exit 24
+printf '%s' '{"result":"{\\"axes\\":{},\\"overall\\":90,\\"grade\\":\\"A\\",\\"findings\\":[]}"}'
+`);
+  await chmod(fakeClaude, 0o700);
+
+  const originalFetch = globalThis.fetch;
+  const originalApiKey = process.env.ANTHROPIC_API_KEY;
+  const originalToken = process.env.ANTHROPIC_TOKEN;
+  globalThis.fetch = async () => { throw new Error('router must not be called'); };
+  process.env.ANTHROPIC_API_KEY = 'test-api-key';
+  process.env.ANTHROPIC_TOKEN = 'test-token';
+
+  try {
+    const result = await chat({
+      routerUrl: 'http://router.test',
+      model: 'claude-cli/claude-sonnet-4-6',
+      messages: [{ role: 'user', content: 'review this diff' }],
+      agentId: 'code-reviewer',
+      sessionId: 'review-test',
+      tenantId: 'nirlab',
+      timeoutMs: 1000,
+      claudeBin: fakeClaude
+    });
+    assert.match(result.content, /"overall":90/);
+    assert.equal(result.servedModel, 'claude-cli/claude-sonnet-4-6');
+  } finally {
+    globalThis.fetch = originalFetch;
+    if (originalApiKey === undefined) delete process.env.ANTHROPIC_API_KEY;
+    else process.env.ANTHROPIC_API_KEY = originalApiKey;
+    if (originalToken === undefined) delete process.env.ANTHROPIC_TOKEN;
+    else process.env.ANTHROPIC_TOKEN = originalToken;
+  }
+});
+
+test('claude-cli failures do not leak the review prompt', async () => {
+  const dir = await mkdtemp(join(tmpdir(), 'granthi-review-claude-error-'));
+  const fakeClaude = join(dir, 'claude');
+  await writeFile(fakeClaude, '#!/bin/sh\nprintf "%s\\n" "$*" >&2\nexit 25\n');
+  await chmod(fakeClaude, 0o700);
+
+  await assert.rejects(
+    chat({
+      routerUrl: 'http://router.test',
+      model: 'claude-cli/claude-sonnet-4-6',
+      messages: [{ role: 'user', content: 'SECRET_REVIEW_DIFF_MARKER' }],
+      timeoutMs: 1000,
+      claudeBin: fakeClaude
+    }),
+    (error) => {
+      assert.match(error.message, /claude-cli failed/);
+      assert.doesNotMatch(error.message, /SECRET_REVIEW_DIFF_MARKER/);
+      return true;
+    }
+  );
+});