feat: add /code-review built-in workflow command (#32)

README.md

@@ -76,7 +76,7 @@ return await agent('Synthesize and double-check these findings:\n' + findings.jo
 - **Interactive `/workflows` TUI** — drill runs → phases → agents → detail; inspect per-agent failures and compact subagent history; pause, stop, restart, and save runs from the keyboard.
 - **Quality patterns built in** — `verify()`, `judgePanel()`, `loopUntilDry()`, and `completenessCheck()` for adversarial review, best-of-N, and exhaustive discovery.
 - **Ultracode** — `/ultracode` is a standing opt-in that auto-arms an exhaustive multi-agent workflow for every substantive message, the way Claude Code's ultracode does. `/effort high` is the lighter tier.
-- **Bundled `/deep-research` + `/adversarial-review`** — real web search, source cross-checking, and cited reports.
+- **Bundled `/deep-research` + `/adversarial-review` + `/code-review`** — real web search, source cross-checking, cited reports, and a 7-angle parallel code review with a verify pass.
 - **Saved & nested workflows** — turn any run into a `/<name>` command, and compose saved workflows from inside other scripts.
 
 ## How it maps to Claude Code dynamic workflows
@@ -121,6 +121,8 @@ The same model — on Pi, plus the production pieces a real run needs:
 /adversarial-review <task>  findings vetted by skeptical reviewers
 /multi-perspective "<topic>" [angle …]
                             analyze a topic from several independent angles, then synthesize
+/code-review [target]       7 parallel finder angles (correctness, reuse, simplification, efficiency,
+                            altitude) + a verify pass → ranked findings
 /codebase-audit <scope> "<check>" …
                             run parallel checks over a scope, then cross-validate and report
 ```
@@ -135,6 +137,17 @@ The same model — on Pi, plus the production pieces a real run needs:
 
 `/multi-perspective` needs a topic; with fewer than two angles it defaults to `technical, product, security, user experience, maintainability`. `/codebase-audit` needs a scope and at least one check.
 
+`/code-review` reads its target from `[target]`, defaulting to your working diff when omitted:
+
+```
+/code-review                  review git diff HEAD (your working changes)
+/code-review HEAD~3..HEAD     review a git range
+/code-review src/foo.ts       review a git diff scoped to one path
+/code-review 42               review gh pr diff 42 (needs the gh CLI + auth)
+```
+
+It fans out 7 finder agents in parallel — 3 on correctness (line-by-line scan, removed-behavior audit, cross-file call-site tracing), 3 on cleanup (reuse, simplification, efficiency), and 1 on abstraction-level fit — dedupes their candidates, verifies each one, and returns a ranked markdown report (correctness first, cleanup next, abstraction last, capped at the top 10). A diff over ~200k characters is truncated with a clear notice rather than silently cut or blowing up the prompt.
+
 In the navigator: `↑/↓` select · `enter`/`→` open · `esc`/`←` back · `p` pause · `x` stop · `r` restart · `s` save · `q` quit. Each agent shows the model it ran on; the detail view shows its prompt, result, error diagnostics, and compact message/tool history.
 
 ## Storage

src/builtin-commands.ts

@@ -1,15 +1,29 @@
 /**
  * Bundled workflow commands: `/deep-research`, `/adversarial-review`,
- * `/multi-perspective`, and `/codebase-audit`.
+ * `/multi-perspective`, `/code-review`, and `/codebase-audit`.
  * They run a generated workflow script and print the final report.
  */
 
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
 import { createCodingTools, type ExtensionAPI, type ExtensionCommandContext } from "@earendil-works/pi-coding-agent";
 import { generateAdversarialReviewWorkflow, generateMultiPerspectiveWorkflow } from "./adversarial-review.js";
+import { generateCodeReviewWorkflow, MAX_DIFF_CHARS } from "./code-review.js";
 import { generateCodebaseAuditWorkflow, generateDeepResearchWorkflow } from "./deep-research.js";
 import { createWebTools } from "./web-tools.js";
 import { runWorkflow, type WorkflowRunResult } from "./workflow.js";
 
+const execFileAsync = promisify(execFile);
+
+/**
+ * Cap on the diff-source exec's stdout+stderr buffer. Node's default (1 MB)
+ * throws on anything but a small diff — `gh pr diff` on a sizeable PR routinely
+ * exceeds it. 64 MB comfortably covers any realistic diff while still bounding
+ * worst-case memory; the prompt-side cap (code-review.ts's MAX_DIFF_CHARS) is
+ * what actually protects the review from a huge diff, not this buffer.
+ */
+const DIFF_EXEC_MAX_BUFFER = 64 * 1024 * 1024;
+
 function alreadyRegistered(pi: ExtensionAPI, name: string): boolean {
   try {
     return (pi.getCommands?.() ?? []).some((c: { name: string }) => c.name === name);
@@ -85,6 +99,89 @@ export function registerBuiltinWorkflows(pi: ExtensionAPI, opts: { cwd: string }
     });
   }
 
+  if (!alreadyRegistered(pi, "code-review")) {
+    pi.registerCommand("code-review", {
+      description:
+        "Multi-angle parallel code review: 7 specialized finders (correctness, reuse, simplification, efficiency, altitude) + verify pass → ranked findings",
+      async handler(args: string, ctx: ExtensionCommandContext) {
+        const input = args.trim();
+        let diffSource = "git diff HEAD";
+        let diff = "";
+
+        try {
+          let cmd: string;
+          let cmdArgs: string[];
+          if (!input) {
+            diffSource = "git diff HEAD";
+            cmd = "git";
+            cmdArgs = ["diff", "HEAD"];
+          } else if (/^\d+$/.test(input)) {
+            diffSource = `gh pr diff ${input}`;
+            cmd = "gh";
+            cmdArgs = ["pr", "diff", input];
+          } else if (input.includes("..")) {
+            diffSource = `git diff ${input}`;
+            cmd = "git";
+            cmdArgs = ["diff", input];
+          } else {
+            diffSource = `git diff HEAD -- ${input}`;
+            cmd = "git";
+            cmdArgs = ["diff", "HEAD", "--", input];
+          }
+          // execFile (not exec/shell) + array args: input can't break out into a
+          // shell command. maxBuffer raised well past Node's 1MB default so a
+          // large `gh pr diff` doesn't throw ERR_CHILD_PROCESS_STDOUT_MAXBUFFER.
+          const { stdout } = await execFileAsync(cmd, cmdArgs, { cwd, maxBuffer: DIFF_EXEC_MAX_BUFFER });
+          diff = stdout;
+          if (!diff.trim()) {
+            return ctx.ui.notify(`No diff output from: ${diffSource}`, "warning");
+          }
+        } catch (err) {
+          const code = (err as NodeJS.ErrnoException | undefined)?.code;
+          if (code === "ERR_CHILD_PROCESS_STDOUT_MAXBUFFER") {
+            return ctx.ui.notify(
+              `Diff from ${diffSource} exceeds the ${Math.floor(DIFF_EXEC_MAX_BUFFER / (1024 * 1024))}MB capture limit — ` +
+                `narrow the target (e.g. a specific file or path) and try again.`,
+              "error",
+            );
+          }
+          return ctx.ui.notify(
+            `Failed to get diff (${diffSource}): ${err instanceof Error ? err.message : err}`,
+            "error",
+          );
+        }
+
+        // The workflow itself also caps prompt size (MAX_DIFF_CHARS), but truncating
+        // here lets us tell the user clearly rather than have it happen silently deep
+        // inside the generated script.
+        const originalLength = diff.length;
+        if (originalLength > MAX_DIFF_CHARS) {
+          diff = diff.slice(0, MAX_DIFF_CHARS);
+          ctx.ui.notify(
+            `Diff is ${originalLength.toLocaleString()} characters — truncated to the first ` +
+              `${MAX_DIFF_CHARS.toLocaleString()} for the review. Findings past the cut are not covered.`,
+            "warning",
+          );
+        }
+
+        ctx.ui.notify(`Reviewing diff (${diffSource}) — running 7 finder angles in parallel…`, "info");
+        try {
+          const result = await runWorkflow(generateCodeReviewWorkflow(), {
+            cwd,
+            args: { diff, diffSource },
+            tools: createCodingTools(cwd),
+            onPhase: (title) => ctx.ui.setStatus("code-review", `review: ${title}`),
+          });
+          ctx.ui.setStatus("code-review", undefined);
+          await pi.sendMessage({ customType: "code-review", content: reportText(result), display: true });
+        } catch (error) {
+          ctx.ui.setStatus("code-review", undefined);
+          ctx.ui.notify(`code-review failed: ${error instanceof Error ? error.message : error}`, "error");
+        }
+      },
+    });
+  }
+
   if (!alreadyRegistered(pi, "multi-perspective")) {
     pi.registerCommand("multi-perspective", {
       description: "Analyze a topic from several independent perspectives in parallel, then synthesize",

src/code-review.ts

@@ -0,0 +1,183 @@
+/**
+ * Multi-angle parallel code review workflow.
+ * 7 specialized finder agents → verify pass → ranked report.
+ */
+
+/**
+ * Hard cap on diff characters fed into the review. This bounds worst-case
+ * prompt size across 7 parallel finders + a per-candidate verify pass, even
+ * when the diff-source exec step (see builtin-commands.ts) already raised its
+ * own maxBuffer and successfully read a very large diff. Oversized diffs are
+ * truncated rather than rejected — findings in the untruncated prefix still
+ * have value — and the truncation is surfaced to the user, not silent.
+ */
+export const MAX_DIFF_CHARS = 200_000;
+
+/**
+ * Generate a code-review workflow script.
+ *
+ * The workflow expects `args` to be passed with shape:
+ *   { diff: string, diffSource: string }
+ *
+ * Model tier routing follows the spec:
+ *   Finders A/B/C → medium (correctness)
+ *   Finders D/E/F → small  (cleanup)
+ *   Finder  G     → big    (altitude / abstraction)
+ *   Synthesis     → big
+ */
+export function generateCodeReviewWorkflow(): string {
+  return `export const meta = {
+  name: 'code_review',
+  description: 'Multi-angle parallel code review: 7 finder angles + verify pass → ranked findings',
+  phases: [
+    { title: 'Find' },
+    { title: 'Verify' },
+    { title: 'Report' },
+  ],
+}
+
+const MAX_DIFF_CHARS = ${MAX_DIFF_CHARS}
+const rawDiff = (args && args.diff) || ''
+const diffSource = (args && args.diffSource) || 'git diff HEAD'
+const diffTruncated = rawDiff.length > MAX_DIFF_CHARS
+const diff = diffTruncated ? rawDiff.slice(0, MAX_DIFF_CHARS) : rawDiff
+if (diffTruncated) {
+  log(
+    'Diff truncated for review: showing the first ' + MAX_DIFF_CHARS + ' of ' + rawDiff.length +
+    ' characters (' + (rawDiff.length - MAX_DIFF_CHARS) + ' omitted). Findings past the cut are not covered.'
+  )
+}
+const candidateSchema = {
+  type: 'object',
+  properties: {
+    candidates: {
+      type: 'array',
+      items: {
+        type: 'object',
+        properties: {
+          file: { type: 'string' },
+          line: { type: 'number' },
+          summary: { type: 'string' },
+          failure_scenario: { type: 'string' },
+        },
+        required: ['file', 'line', 'summary', 'failure_scenario'],
+      },
+    },
+  },
+  required: ['candidates'],
+}
+
+const diffBlock = '\\n\\n<diff source=\\"' + diffSource + '\\"' + (diffTruncated ? ' truncated=\\"true\\"' : '') + '>\\n' +
+  diff + (diffTruncated ? '\\n\\n[... diff truncated: ' + (rawDiff.length - MAX_DIFF_CHARS) + ' more characters omitted ...]' : '') +
+  '\\n</diff>\\n'
+const base = 'Use the read/grep tools to pull in any additional file context you need.' + diffBlock
+
+phase('Find')
+const finders = await parallel([
+  () => agent(
+    'You are a line-by-line correctness scanner. Hunt ONLY for: inverted conditions, off-by-one errors, ' +
+    'null/nil dereferences, wrong variable used, swallowed errors. For each candidate name the exact file, ' +
+    'line number, a one-line summary, and the concrete failure scenario. Return ONLY issues you can justify ' +
+    'with a line in the diff.' + base,
+    { label: 'A-line-scan', tier: 'medium', schema: candidateSchema }
+  ),
+  () => agent(
+    'You are a removed-behavior auditor. For every deleted line or block in the diff: name the invariant ' +
+    'or contract it enforced, then find where (or prove) that contract is re-established elsewhere. ' +
+    'Report only gaps where the invariant is NOT re-established.' + base,
+    { label: 'B-removed-behavior', tier: 'medium', schema: candidateSchema }
+  ),
+  () => agent(
+    'You are a cross-file call-site tracer. For each function/method whose signature or behavior changed ' +
+    'in the diff: grep the codebase for callers, then check whether each call site is still correct after ' +
+    'the change. Report only call sites that are now broken or need updating.' + base,
+    { label: 'C-cross-file-tracer', tier: 'medium', schema: candidateSchema }
+  ),
+  () => agent(
+    'You are a reuse finder. Identify new code in the diff that duplicates existing helpers, utilities, ' +
+    'or patterns already present in the codebase. Propose the existing symbol that should be used instead.' + base,
+    { label: 'D-reuse', tier: 'small', schema: candidateSchema }
+  ),
+  () => agent(
+    'You are a simplification finder. Look for: redundant state that could be derived, copy-paste ' +
+    'variation that could be a shared function, and dead code introduced by the diff.' + base,
+    { label: 'E-simplification', tier: 'small', schema: candidateSchema }
+  ),
+  () => agent(
+    'You are an efficiency finder. Identify: redundant I/O or network calls, sequential work that could ' +
+    'be parallel, and blocking operations on the startup or hot path introduced by the diff.' + base,
+    { label: 'F-efficiency', tier: 'small', schema: candidateSchema }
+  ),
+  () => agent(
+    'You are an altitude reviewer. Assess whether the change is made at the RIGHT abstraction level. ' +
+    'Look for: bandaids on shared infrastructure that should be fixed at the root, fixes in the wrong ' +
+    'layer (e.g. compensating in the UI for a data model problem), or the change solving a symptom ' +
+    'rather than the cause.' + base,
+    { label: 'G-altitude', tier: 'big', schema: candidateSchema }
+  ),
+])
+
+// Collect and deduplicate candidates across all finders
+const allRaw = finders.flatMap((r, fi) => {
+  const label = ['A','B','C','D','E','F','G'][fi]
+  return ((r && r.candidates) || []).map((c) => ({ ...c, angle: label }))
+})
+
+// Deduplicate: same file + line + first 40 chars of summary → keep first
+const seen = new Set()
+const allCandidates = allRaw.filter((c) => {
+  const key = (c.file || '') + ':' + (c.line || 0) + ':' + (c.summary || '').slice(0, 40)
+  if (seen.has(key)) return false
+  seen.add(key)
+  return true
+})
+
+phase('Verify')
+// NOTE: deliberately NOT using the verify() stdlib helper here. verify() only
+// returns a boolean real/not-real vote; this phase needs the 3-way
+// CONFIRMED/PLAUSIBLE/REFUTED verdict so the synthesis report can hedge
+// ("worth a second look" vs "will break"). Since only REFUTED is filtered out
+// below, verify()'s boolean would collapse CONFIRMED and PLAUSIBLE into one
+// bucket and lose that signal for no behavioral gain — verify({reviewers: 1})
+// is already a single agent() call under the hood, same as this.
+const verdicts = allCandidates.length > 0
+  ? await parallel(allCandidates.map((c, i) => () =>
+      agent(
+        'You are a verifier. Determine whether this code review finding is CONFIRMED, PLAUSIBLE, or REFUTED. ' +
+        'CONFIRMED = you can trace the exact failure in the diff. PLAUSIBLE = concern is valid but not certain. ' +
+        'REFUTED = finding is wrong or already handled.\\n\\n' +
+        'FINDING:\\nFile: ' + c.file + '\\nLine: ' + c.line + '\\nSummary: ' + c.summary + '\\n' +
+        'Failure scenario: ' + c.failure_scenario + diffBlock,
+        {
+          label: 'verify-' + (i + 1),
+          schema: {
+            type: 'object',
+            properties: { verdict: { type: 'string', enum: ['CONFIRMED', 'PLAUSIBLE', 'REFUTED'] }, reason: { type: 'string' } },
+            required: ['verdict'],
+          },
+        }
+      )
+    ))
+  : []
+
+const surviving = allCandidates
+  .map((c, i) => ({ ...c, verdict: (verdicts[i] && verdicts[i].verdict) || 'PLAUSIBLE', verifyReason: (verdicts[i] && verdicts[i].reason) || '' }))
+  .filter((c) => c.verdict !== 'REFUTED')
+
+// Rank: correctness (A/B/C) before cleanup (D/E/F) before altitude (G), cap at 10
+const rankAngle = (a) => ['A','B','C'].includes(a) ? 0 : ['D','E','F'].includes(a) ? 1 : 2
+surviving.sort((a, b) => rankAngle(a.angle) - rankAngle(b.angle))
+const top = surviving.slice(0, 10)
+
+phase('Report')
+const synthesis = await agent(
+  'You are a senior code reviewer writing the final report. Below are the verified findings from a ' +
+  'multi-angle code review (already ranked by severity). Write a concise markdown report: ' +
+  '1 sentence per finding with file, line, and the failure scenario. Note the total found vs shown. ' +
+  'Correctness issues (A/B/C) come first, then cleanup (D/E/F), then altitude (G).\\n\\n' +
+  'FINDINGS JSON:\\n' + JSON.stringify(top, null, 2),
+  { label: 'synthesis', tier: 'big' }
+)
+
+return { total: allCandidates.length, surviving: surviving.length, findings: top, report: synthesis, diffTruncated }`;
+}

src/index.ts

@@ -7,6 +7,7 @@ export { compactAgentHistory } from "./agent-history.js";
 export type { AgentDefinition, AgentRegistry } from "./agent-registry.js";
 export { applyToolPolicy, listAgentTypes, loadAgentRegistry, resolveAgentType } from "./agent-registry.js";
 export { registerBuiltinWorkflows } from "./builtin-commands.js";
+export { generateCodeReviewWorkflow, MAX_DIFF_CHARS } from "./code-review.js";
 export * from "./config.js";
 export type { DeepResearchConfig } from "./deep-research.js";
 export { generateCodebaseAuditWorkflow, generateDeepResearchWorkflow } from "./deep-research.js";

tests/builtin-commands.test.ts

@@ -3,12 +3,18 @@ import test from "node:test";
 import { registerBuiltinWorkflows } from "../src/builtin-commands.js";
 import { makeCommandRegistryPi, makeNotifyCtx } from "./helpers/mock-pi.js";
 
-test("registerBuiltinWorkflows registers all four built-in workflow commands", () => {
+test("registerBuiltinWorkflows registers all five built-in workflow commands", () => {
   const { pi, commands } = makeCommandRegistryPi();
   registerBuiltinWorkflows(pi, { cwd: "/tmp" });
-  assert.equal(commands.length, 4);
+  assert.equal(commands.length, 5);
   const names = commands.map((c) => c.name).sort();
-  assert.deepEqual(names, ["adversarial-review", "codebase-audit", "deep-research", "multi-perspective"]);
+  assert.deepEqual(names, [
+    "adversarial-review",
+    "code-review",
+    "codebase-audit",
+    "deep-research",
+    "multi-perspective",
+  ]);
 });
 
 test("registerBuiltinWorkflows is idempotent — skips already registered commands", () => {
@@ -17,6 +23,7 @@ test("registerBuiltinWorkflows is idempotent — skips already registered comman
     "adversarial-review",
     "multi-perspective",
     "codebase-audit",
+    "code-review",
   ]);
   registerBuiltinWorkflows(pi, { cwd: "/tmp" });
   assert.equal(commands.length, 0, "should not re-register when already present");
@@ -27,7 +34,7 @@ test("registerBuiltinWorkflows registers only missing commands", () => {
   registerBuiltinWorkflows(pi, { cwd: "/tmp" });
   assert.deepEqual(
     commands.map((c) => c.name).sort(),
-    ["codebase-audit", "multi-perspective"],
+    ["code-review", "codebase-audit", "multi-perspective"],
     "should only register the commands that aren't already present",
   );
 });
@@ -102,4 +109,9 @@ test("registerBuiltinWorkflows creates handlers with expected structure", () =>
     "should contain Investigate",
   );
   assert.equal(typeof advReviewCmd.handler, "function");
+
+  const codeReviewCmd = commands.find((c) => c.name === "code-review");
+  assert.ok(codeReviewCmd, "code-review should be registered");
+  assert.ok(codeReviewCmd.description?.includes("Multi-angle"), "should describe the multi-angle review");
+  assert.equal(typeof codeReviewCmd.handler, "function");
 });