1// Agent reviews: forge critique runs a reviewer (.forge/review, from the2// review's target branch) over a review's change on your machine, posts what3// it finds as comments, and signs a statement into refs/notes/review on the4// tip, like a CI attestation:5//6// forge-review 17// review cat/greet#78// commit 2aa8af0...9// tree 58945d0...10// since 3c1e2aa... (what it reviewed from: the base, or the last reviewed tip)11// scope full|incremental|resolved (resolved: findings dismissed by hand, the reviewer not run)12// script .forge/review 99a0d5e...13// rules .forge/review.md 1b2c3d4... (or none)14// model claude-opus-5-515// reviewer cat@gentle-wren-4c16// started 2026-11-04T10:00:00Z17// findings 3 (new this run)18// open 1 (blocking findings still open, carried ones too)19// |{"id":"2aa8af0-1","severity":"blocking","path":"a.js","line":12,"body":"..."}20// |{"resolved":"9f8e7d6-2","by":"agent"}21// -----BEGIN SSH SIGNATURE----- (ssh-keygen -Y sign -n forge-review)22//23// The | lines are the open blocking findings and this run's notes, as JSON,24// and what was resolved since the last statement. Each statement is25// self-contained: "open 0" is what an approval rule's "+ agent" asks for26// (lib/rules.ts). Like CI, a statement is the signer's claim about a tree.27//28// The reviewer protocol: .forge/review gets the diff on stdin and runs in a29// clean worktree of the tip. Environment:30//31// FORGE_REVIEW_ID 732// FORGE_REVIEW_BASE the commit the diff is from33// FORGE_REVIEW_TIP the commit it is to34// FORGE_REVIEW_SCOPE full, or incremental (only what changed since the last agent review)35// FORGE_REVIEW_RULES a file with .forge/review.md from the target branch, or empty36// FORGE_REVIEW_OPEN a file of the open blocking findings, a JSON object a line37// FORGE_REVIEW_SKIPPED a file of paths left out of the diff (mechanical, by Jev), a line each38//39// It prints JSON objects, a line each (anything else is ignored):40//41// {"model":"claude-opus-5-5"}42// {"severity":"blocking","path":"lib/a.ts","line":12,"body":"markdown"}43// {"severity":"note","body":"about the whole change"}44// {"resolved":"9f8e7d6-2"} (an open finding the change has dealt with)45//46// node lib/critique.ts prepare --git-dir . --review <owner>/<repo>#<id> --base <c> --tip <c> --out <dir> [--full] [--signers <f>]47// node lib/critique.ts finish --git-dir . --out <dir> [--resolve id,id] [--by <user>]48import fs from 'node:fs';49import path from 'node:path';50import { fileURLToPath } from 'node:url';51import { catFileBatch, isAncestor, tryRun } from './git.ts';52import { Verifier } from './attest.ts';53import type { Verification } from './types.ts';5455export const REVIEW_NAMESPACE = 'forge-review';56export const NOTES = 'refs/notes/review';57const BLOCK = /^forge-review 1\n[\s\S]*?^-----END SSH SIGNATURE-----$/gm;58// Caps on what a reviewer may say in one run.59export const MAX_FINDINGS = 50;60export const MAX_BODY = 4000;6162export type Severity = 'blocking' | 'note';6364export interface Finding {65 id: string;66 severity: Severity;67 path: string | null;68 line: number | null;69 body: string;70}7172export interface AgentReview {73 payload: string;74 signature: string;75 review: string;76 commit: string;77 tree: string;78 since: string | null;79 scope: string;80 script: string | null;81 rules: string | null;82 model: string;83 reviewer: string | null;84 started: string;85 findings: number;86 open: number;87 items: Finding[];88 resolved: { id: string; by: string }[];89 verification: Verification;90 ok: boolean;91}9293export function parseReviewNote(text: string): AgentReview[] {94 const out: AgentReview[] = [];95 for (const m of text.matchAll(BLOCK)) {96 const block = m[0];97 const at = block.search(/^-----BEGIN SSH SIGNATURE-----$/m);98 if (at < 0) continue;99 const payload = block.slice(0, at);100 const f: Record<string, string> = {};101 const items: Finding[] = [];102 const resolved: { id: string; by: string }[] = [];103 for (const line of payload.split('\n').slice(1)) {104 if (line.startsWith('|')) {105 try {106 const o = JSON.parse(line.slice(1));107 if (typeof o.resolved === 'string') resolved.push({ id: o.resolved, by: String(o.by ?? 'agent') });108 else if (typeof o.id === 'string' && typeof o.body === 'string') {109 items.push({ id: o.id, severity: o.severity === 'blocking' ? 'blocking' : 'note', path: typeof o.path === 'string' ? o.path : null, line: Number.isInteger(o.line) ? o.line : null, body: o.body });110 }111 } catch {112 // Not one of ours; the signature still covers it.113 }114 continue;115 }116 const sp = line.indexOf(' ');117 if (sp > 0) f[line.slice(0, sp)] = line.slice(sp + 1);118 }119 if (!f.commit || !f.tree) continue;120 const verification: Verification = { ok: false, signer: null, reason: 'not checked' };121 out.push({122 payload, signature: block.slice(at) + '\n', review: f.review ?? '', commit: f.commit, tree: f.tree, since: f.since ?? null,123 scope: f.scope ?? 'full', script: f.script ?? null, rules: f.rules && f.rules !== 'none' ? f.rules : null, model: f.model ?? 'unknown',124 reviewer: f.reviewer ?? null, started: f.started ?? '', findings: Number(f.findings) || 0, open: Number.isInteger(Number(f.open)) ? Number(f.open) : NaN,125 items, resolved, verification, ok: false,126 });127 }128 return out;129}130131// Every agent review in a repo's notes (or in the notes commit `notes`, for132// a push that brings them), verified, by tree.133export function loadAgentReviews(gitDir: string, verifier: Verifier, notes: string = NOTES): Map<string, AgentReview[]> {134 const byTree = new Map<string, AgentReview[]>();135 const list = tryRun(gitDir, ['ls-tree', '-r', notes]);136 if (!list) return byTree;137 const blobs = list.split('\n').filter(Boolean).map((l) => l.split(/\s+/)).filter((x) => x[1] === 'blob').map((x) => x[2]);138 for (const b of catFileBatch(gitDir, blobs).values()) {139 for (const r of parseReviewNote(b.data.toString('utf8'))) {140 r.verification = verifier.verify(r);141 r.ok = r.verification.ok && Number.isFinite(r.open);142 byTree.set(r.tree, [...(byTree.get(r.tree) ?? []), r]);143 }144 }145 for (const l of byTree.values()) l.sort((a, b) => b.started.localeCompare(a.started));146 return byTree;147}148149export function agentReviews(gitDir: string, tree: string, allowedSigners: string | null, notes: string = NOTES): AgentReview[] {150 const v = new Verifier({ allowedSigners, cacheFile: null, namespace: REVIEW_NAMESPACE });151 return loadAgentReviews(gitDir, v, notes).get(tree) ?? [];152}153154// The latest verified agent review of a review (owner/repo#id) among them.155export function latestFor(all: Iterable<AgentReview[]>, review: string): AgentReview | null {156 let best: AgentReview | null = null;157 for (const l of all) for (const r of l) if (r.ok && r.review === review && (!best || r.started > best.started)) best = r;158 return best;159}160161// ---------------------------------------------------------------- prepare and finish162163const write = (dir: string, f: string, s: string): void => fs.writeFileSync(path.join(dir, f), s);164const read = (dir: string, f: string): string => (fs.existsSync(path.join(dir, f)) ? fs.readFileSync(path.join(dir, f), 'utf8') : '');165166interface State {167 review: string;168 base: string;169 tip: string;170 tree: string;171 since: string;172 // same: the last agent review was of this tree; nothing new to review.173 scope: 'full' | 'incremental' | 'same';174 carried: Finding[];175 paths: string[];176 // The last agent review's model, for a statement that only dismisses.177 model: string | null;178}179180// What to review: everything from base, or, after an earlier agent review of181// this review whose tip is still an ancestor, only what came since. Writes182// the diff, the open findings and the skipped paths into out for the183// reviewer, and state.json for finish.184export function prepare(o: { gitDir: string; review: string; base: string; tip: string; out: string; full: boolean; signers: string | null; skipped: string[] }): State {185 const tree = tryRun(o.gitDir, ['rev-parse', `${o.tip}^{tree}`])?.trim() ?? '';186 const v = new Verifier({ allowedSigners: o.signers, cacheFile: null, namespace: REVIEW_NAMESPACE });187 const all = loadAgentReviews(o.gitDir, v);188 const last = latestFor(all.values(), o.review);189 const carried = last ? last.items.filter((f) => f.severity === 'blocking') : [];190 let since = o.base;191 let scope: State['scope'] = 'full';192 if (!o.full && last && last.tree === tree) {193 since = last.since ?? o.base;194 scope = 'same';195 } else if (!o.full && last && last.tree !== tree && tryRun(o.gitDir, ['cat-file', '-e', `${last.commit}^{commit}`]) != null && isAncestor(o.gitDir, o.base, last.commit) && isAncestor(o.gitDir, last.commit, o.tip)) {196 since = last.commit;197 scope = 'incremental';198 }199 const skip = o.skipped.map((p) => `:(exclude,literal)${p}`);200 const diff = tryRun(o.gitDir, ['diff', '--no-color', '-M', since, o.tip, '--', '.', ...skip]) ?? '';201 const paths = (tryRun(o.gitDir, ['diff', '--name-only', '-z', '-M', o.base, o.tip]) ?? '').split('\0').filter(Boolean);202 fs.mkdirSync(o.out, { recursive: true });203 write(o.out, 'diff', diff);204 write(o.out, 'open.jsonl', carried.map((f) => JSON.stringify(f)).join('\n') + (carried.length ? '\n' : ''));205 write(o.out, 'skipped', o.skipped.map((p) => p + '\n').join(''));206 const state: State = { review: o.review, base: o.base, tip: o.tip, tree, since, scope, carried, paths, model: last?.model ?? null };207 write(o.out, 'state.json', JSON.stringify(state));208 return state;209}210211export interface Post {212 finding: Finding;213 where: string | null;214}215216// Read what the reviewer printed, check it, and write the statement to sign217// (out/statement) and the comments to post (out/post/<n>: a header of218// "where <path>:<line>" and "severity <s>", a blank line, the body).219export function finish(o: { gitDir: string; out: string; resolve: string[]; by: string; reviewer: string; started: string; script: string; rules: string | null }): { posts: Post[]; open: number; warnings: string[]; model: string } {220 const st = JSON.parse(read(o.out, 'state.json')) as State;221 const warnings: string[] = [];222 let model = st.scope === 'same' && st.model ? st.model : 'unknown';223 const fresh: Finding[] = [];224 const resolvedByAgent = new Set<string>();225 const changed = new Set(st.paths);226 const lines = new Map<string, number>();227 const lineCount = (p: string): number => {228 if (!lines.has(p)) {229 const blob = tryRun(o.gitDir, ['cat-file', 'blob', `${st.tip}:${p}`]);230 lines.set(p, blob == null ? 0 : blob.split('\n').length - (blob.endsWith('\n') ? 1 : 0));231 }232 return lines.get(p)!;233 };234 read(o.out, 'out.jsonl').split('\n').forEach((raw, i) => {235 const t = raw.trim();236 if (!t.startsWith('{')) return;237 let x: Record<string, unknown>;238 try {239 x = JSON.parse(t);240 } catch {241 return void warnings.push(`line ${i + 1} isn't JSON; left out`);242 }243 if (typeof x.model === 'string' && Object.keys(x).length === 1) {244 if (/^[A-Za-z0-9._:/@-]{1,64}$/.test(x.model)) model = x.model;245 else warnings.push(`model "${x.model.slice(0, 40)}" isn't a plain name; left out`);246 return;247 }248 if (typeof x.resolved === 'string') {249 if (st.carried.some((f) => f.id === x.resolved)) resolvedByAgent.add(x.resolved);250 else warnings.push(`resolved "${x.resolved}" isn't an open finding`);251 return;252 }253 if (typeof x.body !== 'string' || !x.body.trim()) return void warnings.push(`line ${i + 1} has no body; left out`);254 if (fresh.length >= MAX_FINDINGS) {255 if (fresh.length === MAX_FINDINGS) warnings.push(`more than ${MAX_FINDINGS} findings; the rest are left out`);256 return;257 }258 let p = typeof x.path === 'string' && x.path ? x.path : null;259 let line = Number.isInteger(x.line) && (x.line as number) > 0 ? (x.line as number) : null;260 if (p && (!changed.has(p) || /[\0-\x1f]/.test(p))) {261 warnings.push(`${p.slice(0, 80)} isn't a file the review changes; posted as a comment on the whole review`);262 p = null;263 line = null;264 }265 if (p && !lineCount(p)) {266 warnings.push(`${p} isn't in the tip (deleted, or empty); posted as a comment on the whole review`);267 p = null;268 line = null;269 }270 if (p && line && line > lineCount(p)) {271 warnings.push(`${p}:${line} is past the end of the file; posted on the file's first line`);272 line = 1;273 }274 if (p && !line) line = 1;275 fresh.push({ id: `${st.tip.slice(0, 7)}-${fresh.length + 1}`, severity: x.severity === 'blocking' ? 'blocking' : 'note', path: p, line, body: x.body.trim().slice(0, MAX_BODY) });276 });277 const byHand = new Set(o.resolve.filter(Boolean));278 for (const id of byHand) if (!st.carried.some((f) => f.id === id)) warnings.push(`--resolve ${id}: no such open finding`);279 const resolved = [280 ...[...resolvedByAgent].filter((id) => !byHand.has(id)).map((id) => ({ resolved: id, by: 'agent' })),281 ...[...byHand].filter((id) => st.carried.some((f) => f.id === id)).map((id) => ({ resolved: id, by: o.by })),282 ];283 const gone = new Set(resolved.map((r) => r.resolved));284 const stillOpen = st.carried.filter((f) => !gone.has(f.id));285 const items = [...stillOpen, ...fresh];286 const open = items.filter((f) => f.severity === 'blocking').length;287 const statement = [288 'forge-review 1',289 `review ${st.review}`,290 `commit ${st.tip}`,291 `tree ${st.tree}`,292 `since ${st.since}`,293 `scope ${st.scope === 'same' ? 'resolved' : st.scope}`,294 `script ${o.script}`,295 `rules ${o.rules ?? 'none'}`,296 `model ${model}`,297 `reviewer ${o.reviewer}`,298 `started ${o.started}`,299 `findings ${fresh.length}`,300 `open ${open}`,301 ...items.map((f) => '|' + JSON.stringify(f)),302 ...resolved.map((r) => '|' + JSON.stringify(r)),303 ].join('\n') + '\n';304 write(o.out, 'statement', statement);305 const posts = fresh.map((f) => ({ finding: f, where: f.path ? `${f.path}:${f.line}` : null }));306 fs.mkdirSync(path.join(o.out, 'post'), { recursive: true });307 posts.forEach((p, i) => write(path.join(o.out, 'post'), String(i + 1).padStart(3, '0'), `severity ${p.finding.severity}\nwhere ${p.where ?? ''}\n\n${p.finding.body}\n`));308 write(o.out, 'model', model);309 return { posts, open, warnings, model };310}311312// ---------------------------------------------------------------- cli313314// Run directly (realpath: /tmp is a symlink on macOS, and import.meta.url is resolved).315if (process.argv[1] && fs.realpathSync(process.argv[1]) === fileURLToPath(import.meta.url)) {316 const args = process.argv.slice(2);317 const opt = (k: string): string | null => {318 const i = args.indexOf(k);319 return i >= 0 ? (args[i + 1] ?? null) : null;320 };321 const need = (k: string): string => {322 const v = opt(k);323 if (v == null) {324 process.stderr.write(`critique: ${args[0]} needs ${k}\n`);325 process.exit(2);326 }327 return v;328 };329 if (args[0] === 'prepare') {330 const out = need('--out');331 const skipped = fs.existsSync(path.join(out, 'skip.in')) ? fs.readFileSync(path.join(out, 'skip.in'), 'utf8').split('\n').filter(Boolean) : [];332 const signers = opt('--signers');333 const st = prepare({ gitDir: need('--git-dir'), review: need('--review'), base: need('--base'), tip: need('--tip'), out, full: args.includes('--full'), signers: signers && fs.existsSync(signers) ? signers : null, skipped });334 process.stdout.write(`${st.scope} ${st.since} ${st.carried.length}\n`);335 } else if (args[0] === 'finish') {336 const r = finish({ gitDir: need('--git-dir'), out: need('--out'), resolve: (opt('--resolve') ?? '').split(','), by: need('--by'), reviewer: need('--reviewer'), started: need('--started'), script: need('--script'), rules: opt('--rules') });337 for (const w of r.warnings) process.stderr.write(`forge: critique: ${w}\n`);338 process.stdout.write(`${r.posts.length} ${r.open}\n`);339 } else {340 process.stderr.write('usage: node lib/critique.ts prepare|finish ...\n');341 process.exit(2);342 }343}