chore(loop): isolated double review for pr-review

scripts/loop/README.md

@@ -300,6 +300,31 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w
   `tokens`/`durationMs`/`model`/`outcome`); full transcripts go to the
   artifact store (90 days).
 
+### Isolated double review (`pr-review`)
+
+`skills/review/SKILL.md` requires two independent reviewers, but pi has no
+sub-agent mechanism — one process can only do two passes over one context. The host
+(`shared/review.mjs`) therefore runs **two `pi` processes** for the `pr-review`
+stage:
+
+| | Reviewer A | Reviewer B |
+| --- | --- | --- |
+| session | `<LOOP_SESSION_DIR>/reviewer-a` | `<LOOP_SESSION_DIR>/reviewer-b` |
+| model | `LOOP_MODEL` | `LOOP_REVIEW_MODEL`, else the first different DeepSeek model |
+| tools | `read,grep,find,ls,bash` | `read,grep,find,ls,bash` |
+| result | `<rid>.reviewer-a.result.json` / `.md` | `<rid>.reviewer-b.result.json` / `.md` |
+
+They run in sequence (one shared wall-clock budget) and cannot see each other's
+session or result file; `mergeReviews` is the only place their verdicts meet. The
+changeset passes only when **both** approve (`accepted`); otherwise `rejected`. A
+reviewer that produced no verdict is never counted as approval — a transient exit
+becomes `retry`, anything else `failed`. The read-only allowlist has no `edit`/`write`
+tool, so a reviewer writes its own result file with a `bash` heredoc; "read-only"
+means "no edit/write tools", not a filesystem sandbox.
+
+Each reviewer prompt carries its own lens and review slot, and the product-stage
+commit/push instructions are suppressed for it (the reviewer's job is the verdict).
+
 ### Trigger → action (workflow)
 
 | Event | Action |
@@ -860,8 +885,26 @@ the first `auto` run is a verification step rather than a milestone.
       `skipped` while step 6 stays `failure`, so the only red step is the one
       that actually failed.
 
+- [x] D35 (#71) The `pr-review` stage claimed two isolated reviewers but ran one
+      `pi` process in one session, so both lenses shared a context and a model —
+      the isolation `skills/review/SKILL.md` promised was nominal. pi has no
+      sub-agent mechanism, so the promise was unfulfillable as written. The host
+      now launches two `pi` processes for `pr-review` (`shared/review.mjs`): separate
+      `--session-dir`, a different `--model` for the second lens, and a read-only
+      `--tools read,grep,find,ls,bash` allowlist; `mergeReviews` accepts only when
+      both approve and never counts a missing verdict as approval. The skill was
+      reworded to describe the host mechanism instead of telling the agent to spawn
+      sub-agents. Demonstrated with a fake-engine harness: two invocations, distinct
+      session dirs and models, both carrying the allowlist, and the accept/reject/
+      inconclusive merge paths.
+
 ## Environment facts
 
+- **Double review (#71):** `pr-review` runs two isolated `pi` processes
+  (`shared/review.mjs`) — separate session dirs, different models, read-only tools —
+  and merges their verdicts. `LOOP_REVIEW_MODEL` overrides the second model;
+  without it the host picks the first DeepSeek catalogue entry that is not
+  `LOOP_MODEL` (default: `deepseek-v4-flash` → `deepseek-v4-pro`).
 - Phase (2026-09-10): **A1 implementation landed; A0 remains the running host
   until the 2026-09-15 switchover.** Write boundary in A1 = `report`.
 - Engine: pi (verified) for A1; A0 runs were driven by an interactive opencode
@@ -880,7 +923,8 @@ the first `auto` run is a verification step rather than a milestone.
     the agent wanted to do arrived as a proposal checklist in the report.
   - pi's built-in DeepSeek catalogue (credential-gated, printed per run):
     `deepseek-v4-flash`, `deepseek-v4-flash-vision-exp`, `deepseek-v4-pro`.
-    There is no `deepseek-flash` — see D19.
+    There is no `deepseek-flash` — see D19. `LOOP_REVIEW_MODEL` defaults to a
+    different entry from this catalogue so the second reviewer lens is heterogeneous.
 - **Infrastructure smoke**: `gh workflow run loop.yml -f smoke=true` exercises
   checkout → toolchain → pi install → R2 pull → push and **skips the agent
   entirely**, so "is the pipeline healthy?" is answerable without spending

scripts/loop/entry.mjs

@@ -18,6 +18,7 @@
  *   LOOP_EVENT_NAME / LOOP_EVENT_ACTION / LOOP_EVENT_JSON / LOOP_REPO
  *   LOOP_WRITE_LEVEL (report|auto, default report)
  *   LOOP_MODEL (pi's --model, e.g. deepseek/deepseek-flash)
+ *   LOOP_REVIEW_MODEL (optional second-lens model; defaults to a different DeepSeek model)
  *   LOOP_RUN_TIMEOUT_MS, LOOP_SESSION_DIR, GITHUB_STEP_SUMMARY
  */
 
@@ -35,6 +36,7 @@ import {
 } from './shared/state.mjs';
 import { route, eventKey, stageForIssue } from './shared/route.mjs';
 import { buildPrompt, runPi, parseEvents, summarize, classify } from './shared/agent.mjs';
+import { runDoubleReview } from './shared/review.mjs';
 
 const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
 const S = makeState(ROOT);
@@ -290,35 +292,69 @@ async function doRun({ decision, ctx, repo, writeLevel }) {
     + `${task ? `task #${task.id}, ` : 'system-level, '}stage=${stage}`
     + `${decision.mode === 'readonly' ? ', readonly' : ''})`);
 
-  const prompt = buildPrompt({
-    task, stage, runId: rid, writeLevel, mode: decision.mode, repo,
-    lead: [
-      `trigger: ${ctx.eventName}.${ctx.action || '(none)'} — ${decision.reason}`,
-      decision.loopTask ? `linked issue: #${decision.loopTask}` : null,
-    ].filter(Boolean),
-  });
+  const lead = [
+    `trigger: ${ctx.eventName}.${ctx.action || '(none)'} — ${decision.reason}`,
+    decision.loopTask ? `linked issue: #${decision.loopTask}` : null,
+  ].filter(Boolean);
+  const promptOptions = { task, stage, runId: rid, writeLevel, mode: decision.mode, repo, lead };
 
   const sessionDir = process.env.LOOP_SESSION_DIR ?? path.join(os.tmpdir(), 'loop-pi', rid);
   const timeoutMs = Number(process.env.LOOP_RUN_TIMEOUT_MS ?? 55 * 60 * 1000);
 
-  const res = await runPi({
-    prompt, cwd: ROOT, sessionDir,
-    model: process.env.LOOP_MODEL,
-    timeoutMs,
-    writeLevel,
-    log,
-  });
-  const events = parseEvents(res.stdout);
-  const { usage, stopReason, toolCalls, turns } = summarize(events);
-
-  // the agent's result file (the orchestration layer is the single writer for finish)
-  const result = await readJson(path.join(S.reportsDir, `${rid}.result.json`));
-  const cls = classify({ code: res.code, signal: res.signal, stderr: res.stderr, timedOut: res.timedOut, stopReason });
+  let usage = null;
+  let toolCalls = 0;
+  let turns = 0;
+  let result;
+  let cls;
+  if (stage === 'pr-review') {
+    // Two isolated reviewer processes (#71): separate sessions, different models,
+    // read-only tools. `runDoubleReview` returns the merged verdict as `result`.
+    const review = await runDoubleReview({
+      promptOptions, reportsDir: S.reportsDir, sessionDir, timeoutMs, writeLevel,
+      cwd: ROOT, log, warn,
+    });
+    result = review.result;
+    usage = review.usage;
+    toolCalls = review.toolCalls;
+    turns = review.turns;
+    // Keep `cls` shaped like `classify()` so the shared logging below stays uniform;
+    // `mergeReviews` is the judge for this stage, not a single agent's exit code.
+    cls = {
+      exit: result.outcome === 'retry' ? 3 : result.outcome === 'failed' ? 1 : 0,
+      kind: 'review',
+      reason: result.note,
+    };
+  } else {
+    const prompt = buildPrompt(promptOptions);
+    const res = await runPi({
+      prompt, cwd: ROOT, sessionDir,
+      model: process.env.LOOP_MODEL,
+      timeoutMs,
+      writeLevel,
+      log,
+    });
+    const summary = summarize(parseEvents(res.stdout));
+    usage = summary.usage;
+    toolCalls = summary.toolCalls;
+    turns = summary.turns;
+    // the agent's result file (the orchestration layer is the single writer for finish)
+    result = await readJson(path.join(S.reportsDir, `${rid}.result.json`));
+    cls = classify({
+      code: res.code, signal: res.signal, stderr: res.stderr,
+      timedOut: res.timedOut, stopReason: summary.stopReason,
+    });
+  }
 
   let outcome; let note; let push = null;
-  if (result?.outcome && outcomeAllowed(stage, result.outcome)) {
+  if (stage === 'pr-review') {
+    // `mergeReviews` already collapsed the two isolated verdicts (accepted/rejected),
+    // or classified an inconclusive review (retry/failed). The single-agent
+    // `result.outcome` path must not apply here.
+    outcome = result.outcome;
+    note = result.note;
+  } else if (result?.outcome && outcomeAllowed(stage, result.outcome)) {
     outcome = result.outcome;
-    note = result.note ?? `agent decision (exit=${res.code})`;
+    note = result.note ?? `agent decision (exit=${cls.exit})`;
   } else if (result?.outcome) {
     outcome = 'failed';
     note = `agent returned outcome=${result.outcome}, but stage=${stage} does not allow it `