|
export const meta = { |
|
name: 'magpie-review', |
|
description: 'Multi-agent debate code review (claude, codex, agy) in the style of magpie', |
|
whenToUse: 'Launched by the /magpie-review skill after it prepared the run directory with a context file containing the diff.', |
|
phases: [ |
|
{ title: 'Analyze', detail: 'summarize the change, pick review focus areas' }, |
|
{ title: 'Review', detail: 'up to 5 debate rounds, reviewers in parallel, convergence check per round' }, |
|
{ title: 'Conclude', detail: 'prose conclusion over the debate' }, |
|
{ title: 'Structurize', detail: 'extract findings as structured issues' }, |
|
{ title: 'Audit', detail: 'verify every issue against the real code' }, |
|
], |
|
} |
|
|
|
// args: { |
|
// runDir: absolute dir for prompt/output files (skill created it) |
|
// repoDir: absolute path of the checkout under review |
|
// contextFile: absolute path of the context file (task + full diff) |
|
// label: short human label of the target, e.g. "pr-123" or a branch name |
|
// agents?: subset of reviewer ids to run (default: all registered) |
|
// maxRounds?: debate rounds cap (default 5; forced to 1 for a single reviewer) |
|
// } |
|
// The harness may deliver args as a JSON string instead of an object. |
|
const argv = typeof args === 'string' ? JSON.parse(args) : args |
|
const { runDir, repoDir, contextFile, label } = argv |
|
if (!runDir || !repoDir || !contextFile || !label) { |
|
throw new Error('args must include runDir, repoDir, contextFile, label') |
|
} |
|
|
|
// --------------------------------------------------------------------------- |
|
// Reviewer registry. To add a reviewer, add ONE entry here. |
|
// |
|
// kind 'native' runs the review as a Claude subagent (the session model). |
|
// kind 'cli' runs a shell command via a cheap haiku wrapper agent that writes |
|
// the prompt file, starts the command detached, waits for it, and relays its |
|
// stdout behind a sentinel line (see runReviewer for the protocol and why). |
|
// |
|
// Every reviewer must run with FULL access (no sandbox, no permission |
|
// prompts): claude native subagents already have it, codex gets --yolo, |
|
// agy gets --dangerously-skip-permissions. When adding a new CLI reviewer, |
|
// give it that CLI's equivalent full-access flags. |
|
// |
|
// A 'cli' command receives the prompt file path and must deliver the prompt |
|
// itself: codex reads stdin via `- <file`, agy takes the prompt as the |
|
// --print argument (agy does NOT read stdin — `--print -` silently reviews |
|
// nothing). It runs with the repo visible: codex/claude see it via cwd, agy |
|
// only via --add-dir (it ignores cwd). runDir must also be readable because |
|
// the context file lives there. |
|
// --------------------------------------------------------------------------- |
|
const REVIEWERS = { |
|
claude: { kind: 'native' }, |
|
codex: { |
|
kind: 'cli', |
|
command: (promptFile) => `cd '${repoDir}' && codex exec --yolo - < '${promptFile}'`, |
|
}, |
|
agy: { |
|
kind: 'cli', |
|
command: (promptFile) => |
|
`agy --dangerously-skip-permissions --add-dir '${repoDir}' --add-dir '${runDir}' --print-timeout 30m --print "$(cat '${promptFile}')"`, |
|
}, |
|
} |
|
|
|
const active = argv.agents && argv.agents.length ? argv.agents : Object.keys(REVIEWERS) |
|
for (const id of active) { |
|
if (!REVIEWERS[id]) throw new Error(`unknown reviewer '${id}' — registered: ${Object.keys(REVIEWERS).join(', ')}`) |
|
} |
|
const maxRounds = active.length === 1 ? 1 : argv.maxRounds || 5 |
|
|
|
// --------------------------------------------------------------------------- |
|
// Prompts (adapted from magpie) |
|
// --------------------------------------------------------------------------- |
|
const CONTEXT_NOTE = [ |
|
`First read ${contextFile} — it contains the task description and the FULL diff under review.`, |
|
`That diff is all you need for the review, but you may read files in the repository at ${repoDir} whenever you need surrounding context to verify a claim.`, |
|
].join('\n') |
|
|
|
const EVIDENCE_RULES = `For every issue you raise, you MUST include: |
|
1. The specific \`file:line\` — only lines inside diff hunks |
|
2. A quote of the offending code (1-3 lines max) |
|
3. The concrete failure scenario — what input or state triggers it |
|
4. A self-assessed severity (use these definitions exactly): |
|
- critical = data corruption, security hole, guaranteed crash on common input |
|
- high = will trigger under realistic conditions, observable user-facing breakage |
|
- medium = edge case with plausible trigger, missing error handling |
|
- low = code quality, minor concern |
|
- nitpick = style-only preference (won't be reported) |
|
|
|
DO NOT REPORT: |
|
- Build script / CI polish |
|
- Missing comments / docstrings unless load-bearing |
|
- "Forward-compat risk" without a concrete trigger |
|
- Dead code unless it carries real risk |
|
- Style preferences |
|
- Issues outside the diff hunks unless severity >= high |
|
- Theoretically-correct-but-impossible cases |
|
|
|
If a file has nothing meaningful wrong, skip it. Do NOT produce filler. |
|
Brevity is a feature — 5 well-evidenced issues beat 20 weak ones. |
|
Verify your claims against the actual code before reporting.` |
|
|
|
function round1Prompt(id, analysis, focusAreas) { |
|
return [ |
|
`You are reviewer [${id}] in a multi-agent code review of "${label}".`, |
|
'', |
|
CONTEXT_NOTE, |
|
'', |
|
`An analyzer summarized the change as follows:`, |
|
'', |
|
analysis, |
|
'', |
|
`The analyzer suggests focusing on: ${focusAreas.join('; ')}.`, |
|
`These are suggestions — also flag anything else you notice beyond these areas.`, |
|
'', |
|
`Review the change systematically.`, |
|
'', |
|
EVIDENCE_RULES, |
|
].join('\n') |
|
} |
|
|
|
function roundNPrompt(id, round, othersPrev, ownPrev) { |
|
return [ |
|
`You are reviewer [${id}] in round ${round} of a multi-agent code review of "${label}".`, |
|
'', |
|
CONTEXT_NOTE, |
|
'', |
|
`Your own findings from the previous round:`, |
|
'', |
|
ownPrev || '(you produced no output last round)', |
|
'', |
|
`Here is what the OTHER reviewers said in the previous round:`, |
|
'', |
|
othersPrev, |
|
'', |
|
`Do this:`, |
|
`1. If the others' findings are correct and you have nothing substantive to add, say "I agree with [reviewer]'s findings, no additional issues." That is a fine outcome — do not pad.`, |
|
`2. If you disagree with any of their claims, challenge with code evidence — quote the line that disproves their concern.`, |
|
`3. ONLY add new issues if they are concrete (file:line + code quote + failure scenario) AND genuinely missed by the others. Do not manufacture issues to look productive — padding hurts review quality.`, |
|
'', |
|
EVIDENCE_RULES, |
|
].join('\n') |
|
} |
|
|
|
// --------------------------------------------------------------------------- |
|
// Reviewer execution |
|
// |
|
// CLI reviewers run detached from their wrapper agent: the wrapper starts the |
|
// command with setsid+nohup, then blocks on `tail --pid` in repeatable |
|
// foreground Bash calls until the exit-code file appears. run_in_background |
|
// is deliberately NOT used — a wrapper that ends its turn kills its background |
|
// tasks, and haiku wrappers did exactly that, feeding "still waiting..." |
|
// placeholders into the debate as if they were reviews. The sentinel lets the |
|
// round loop tell a relayed review from any such placeholder. |
|
// --------------------------------------------------------------------------- |
|
const REVIEW_SENTINEL = '<<<REVIEW>>>' |
|
|
|
function runReviewer(id, round, prompt) { |
|
const opts = { label: `${id} r${round}`, phase: 'Review' } |
|
const r = REVIEWERS[id] |
|
if (r.kind === 'native') { |
|
return agent(`${prompt}\n\nWork from the repository checkout at ${repoDir}. Your final message must be ONLY your review findings (raw data, no meta commentary).`, opts) |
|
} |
|
const base = `${runDir}/round-${round}-${id}` |
|
const promptFile = `${base}-prompt.md` |
|
const outFile = `${base}-out.md` |
|
const errFile = `${base}-err.log` |
|
const runScript = `${base}-run.sh` |
|
const pidFile = `${base}.pid` |
|
const exitFile = `${base}.exit` |
|
return agent( |
|
[ |
|
`You are a mechanical CLI runner. Do exactly these steps and nothing else:`, |
|
`1. Write the prompt text below (everything between the BEGIN/END markers, markers excluded) VERBATIM to ${promptFile} with the Write tool. Do not alter, trim, or reformat it.`, |
|
`2. Write exactly these three lines to ${runScript} with the Write tool:`, |
|
`#!/bin/sh`, |
|
`${r.command(promptFile)} > '${outFile}' 2> '${errFile}'`, |
|
`echo $? > '${exitFile}'`, |
|
`3. Start it detached with one foreground Bash call (do NOT use run_in_background):`, |
|
` setsid nohup sh '${runScript}' >/dev/null 2>&1 & echo $! > '${pidFile}'`, |
|
`4. Wait for it with this foreground Bash call (set timeout to 600000):`, |
|
` tail --pid="$(cat '${pidFile}')" -f /dev/null 2>/dev/null; cat '${exitFile}' 2>/dev/null || echo RUNNING`, |
|
` Repeat this exact call until it prints a number instead of RUNNING. The review can take 20+ minutes; if the call times out or prints RUNNING, run it again. NEVER end your turn while it still prints RUNNING.`, |
|
`5. When it prints a number: Read ${outFile}. If the number is not 0 or the file is empty, Read ${errFile} and make your final message exactly "ERROR: " followed by the last ~20 lines of stderr.`, |
|
`6. Otherwise your final message is the line ${REVIEW_SENTINEL} followed by the full content of ${outFile}, verbatim. No summary, no commentary, nothing before the sentinel.`, |
|
``, |
|
`----- BEGIN PROMPT -----`, |
|
prompt, |
|
`----- END PROMPT -----`, |
|
].join('\n'), |
|
{ ...opts, model: 'haiku', effort: 'low' }, |
|
) |
|
} |
|
|
|
// --------------------------------------------------------------------------- |
|
// Phase 1: analysis |
|
// --------------------------------------------------------------------------- |
|
phase('Analyze') |
|
const ANALYSIS_SCHEMA = { |
|
type: 'object', |
|
properties: { |
|
analysis: { type: 'string', description: 'What the change does, its architecture and purpose, trade-offs, things reviewers should note' }, |
|
focusAreas: { type: 'array', items: { type: 'string' }, minItems: 2, maxItems: 4 }, |
|
}, |
|
required: ['analysis', 'focusAreas'], |
|
} |
|
const analyzed = await agent( |
|
[ |
|
`You are the analyzer preparing a multi-agent code review of "${label}".`, |
|
`Read ${contextFile} (task + full diff) and, where useful, files in ${repoDir}.`, |
|
`Produce: (a) an analysis of what the change does — architecture, purpose, trade-offs, things to note — that lets reviewers work without re-deriving intent; (b) 2-4 suggested review focus areas.`, |
|
].join('\n'), |
|
{ label: 'analyzer', phase: 'Analyze', schema: ANALYSIS_SCHEMA }, |
|
) |
|
if (!analyzed) throw new Error('analyzer failed') |
|
log(`Analysis done. Focus: ${analyzed.focusAreas.join('; ')}`) |
|
|
|
// --------------------------------------------------------------------------- |
|
// Phase 2: debate rounds |
|
// --------------------------------------------------------------------------- |
|
phase('Review') |
|
const CONVERGE_SCHEMA = { |
|
type: 'object', |
|
properties: { |
|
reasoning: { type: 'string' }, |
|
converged: { type: 'boolean' }, |
|
}, |
|
required: ['reasoning', 'converged'], |
|
} |
|
|
|
const rounds = [] // [{ round, messages: { id: text }, failed: [id] }] |
|
let convergedAtRound = null |
|
let prevMessages = null |
|
|
|
for (let round = 1; round <= maxRounds; round++) { |
|
log(`Round ${round}/${maxRounds}: ${active.join(', ')}`) |
|
// Build every prompt BEFORE execution so all reviewers in a round see identical info. |
|
const prompts = {} |
|
for (const id of active) { |
|
if (round === 1) { |
|
prompts[id] = round1Prompt(id, analyzed.analysis, analyzed.focusAreas) |
|
} else { |
|
const others = active |
|
.filter((o) => o !== id && prevMessages[o]) |
|
.map((o) => `[${o}]:\n${prevMessages[o]}`) |
|
.join('\n\n---\n\n') |
|
prompts[id] = roundNPrompt(id, round, others || '(no other reviewer produced output last round)', prevMessages[id]) |
|
} |
|
} |
|
|
|
const outs = await parallel(active.map((id) => () => runReviewer(id, round, prompts[id]))) |
|
const messages = {} |
|
const failed = [] |
|
active.forEach((id, i) => { |
|
let text = outs[i] |
|
if (text && REVIEWERS[id].kind === 'cli') { |
|
// Only sentinel-prefixed output is a relayed review; anything else is a |
|
// wrapper that broke protocol (e.g. quit early with "still waiting..."). |
|
if (text.startsWith(REVIEW_SENTINEL)) text = text.slice(REVIEW_SENTINEL.length).trim() |
|
else if (!text.startsWith('ERROR:')) text = `ERROR: wrapper broke protocol: ${text.slice(0, 200)}` |
|
} |
|
if (!text || text.startsWith('ERROR:')) { |
|
failed.push(id) |
|
log(`Round ${round}: reviewer '${id}' failed${text ? ` — ${text.slice(0, 200)}` : ''}`) |
|
} else { |
|
messages[id] = text |
|
} |
|
}) |
|
if (Object.keys(messages).length === 0) { |
|
throw new Error(`round ${round}: all reviewers failed — check ${runDir}/round-${round}-*-err.log (auth/quota are the usual causes)`) |
|
} |
|
rounds.push({ round, messages, failed }) |
|
prevMessages = messages |
|
|
|
// Convergence check (skipped on the last round and for solo reviews) |
|
if (round < maxRounds && active.length > 1) { |
|
const transcript = Object.entries(messages) |
|
.map(([id, text]) => `[${id}]:\n${text}`) |
|
.join('\n\n---\n\n') |
|
const verdict = await agent( |
|
[ |
|
`You are a strict consensus judge for a multi-agent code review. Below are all reviewer messages from round ${round}.`, |
|
'', |
|
transcript, |
|
'', |
|
`TRUE CONSENSUS requires ALL of: the reviewers reach the same overall verdict; every critical/high issue raised by anyone is acknowledged by the others; no concern is left unaddressed; there is explicit agreement on what should happen.`, |
|
round === 1 ? `This was an independent round — reviewers could not see each other. Independent agreement (same issues, same verdict) still counts as consensus.` : '', |
|
`Be very conservative: if there is ANY doubt, it is NOT converged.`, |
|
].join('\n'), |
|
{ label: `converge r${round}`, model: 'haiku', phase: 'Review', schema: CONVERGE_SCHEMA }, |
|
) |
|
if (verdict && verdict.converged) { |
|
convergedAtRound = round |
|
log(`Converged at round ${round}: ${verdict.reasoning.slice(0, 200)}`) |
|
break |
|
} |
|
} |
|
} |
|
|
|
// --------------------------------------------------------------------------- |
|
// Phase 3: prose conclusion |
|
// --------------------------------------------------------------------------- |
|
phase('Conclude') |
|
const lastRound = rounds[rounds.length - 1] |
|
const finalTranscript = rounds |
|
.map((r) => |
|
Object.entries(r.messages) |
|
.map(([id, text]) => `[round ${r.round}] [${id}]:\n${text}`) |
|
.join('\n\n---\n\n'), |
|
) |
|
.join('\n\n=== next round ===\n\n') |
|
|
|
const conclusion = await agent( |
|
[ |
|
`You are the moderator of a multi-agent code review of "${label}". Full debate transcript:`, |
|
'', |
|
finalTranscript, |
|
'', |
|
`Write the final conclusion: overall assessment and merge-risk, points of consensus, points of unresolved disagreement (with each side's reasoning — do not paper over them), and recommended actions. Prose, concise. Your final message is ONLY the conclusion text.`, |
|
].join('\n'), |
|
{ label: 'conclusion', phase: 'Conclude' }, |
|
) |
|
|
|
// --------------------------------------------------------------------------- |
|
// Phase 4: structurize issues |
|
// --------------------------------------------------------------------------- |
|
phase('Structurize') |
|
const CATEGORIES = [ |
|
'correctness', 'security', 'performance', 'concurrency', 'resource-leak', 'error-handling', |
|
'build', 'testing', 'documentation', 'architecture', 'compatibility', 'style', |
|
] |
|
const ISSUE_PROPS = { |
|
severity: { type: 'string', enum: ['critical', 'high', 'medium', 'low', 'nitpick'] }, |
|
category: { type: 'string', enum: CATEGORIES }, |
|
file: { type: 'string' }, |
|
line: { type: 'integer' }, |
|
endLine: { type: 'integer' }, |
|
title: { type: 'string' }, |
|
description: { type: 'string', description: 'Senior-engineer inline comment: WHAT is wrong, WHY, the FAILURE scenario, and the FIX' }, |
|
suggestedFix: { type: 'string' }, |
|
codeSnippet: { type: 'string' }, |
|
raisedBy: { type: 'array', items: { type: 'string' } }, |
|
} |
|
const ISSUES_SCHEMA = { |
|
type: 'object', |
|
properties: { |
|
issues: { |
|
type: 'array', |
|
items: { |
|
type: 'object', |
|
properties: ISSUE_PROPS, |
|
required: ['severity', 'category', 'file', 'line', 'title', 'description', 'raisedBy'], |
|
}, |
|
}, |
|
}, |
|
required: ['issues'], |
|
} |
|
const lastRoundTranscript = Object.entries(lastRound.messages) |
|
.map(([id, text]) => `[${id}]:\n${text}`) |
|
.join('\n\n---\n\n') |
|
|
|
const structured = await agent( |
|
[ |
|
`Based on these code review discussions (final round of a multi-agent debate on "${label}"), extract ALL concrete issues that are still standing (drop ones that were refuted during the debate).`, |
|
'', |
|
lastRoundTranscript, |
|
'', |
|
`Rules:`, |
|
`- The diff is in ${contextFile}; \`line\` MUST point inside a diff hunk of \`file\` — drop issues you cannot anchor to a changed line.`, |
|
`- Merge duplicates raised by several reviewers into one issue listing every reviewer id in raisedBy.`, |
|
`- description: WHAT is wrong, WHY, the concrete FAILURE scenario, and the FIX.`, |
|
`- Apply the severity rubric strictly (critical = data corruption / security hole / guaranteed crash; high = realistic user-facing breakage; medium = plausible edge case; low = minor; nitpick = style).`, |
|
`- Do not invent issues that no reviewer raised.`, |
|
].join('\n'), |
|
{ label: 'structurizer', phase: 'Structurize', schema: ISSUES_SCHEMA }, |
|
) |
|
const issues = structured ? structured.issues : [] |
|
log(`Structurized ${issues.length} issue(s)`) |
|
|
|
// --------------------------------------------------------------------------- |
|
// Phase 5: audit — verify every issue against the real code |
|
// --------------------------------------------------------------------------- |
|
phase('Audit') |
|
let auditedIssues = issues |
|
let droppedIssues = [] |
|
let auditRan = false |
|
if (issues.length > 0) { |
|
const AUDIT_SCHEMA = { |
|
type: 'object', |
|
properties: { |
|
issues: { |
|
type: 'array', |
|
items: { |
|
type: 'object', |
|
properties: { |
|
...ISSUE_PROPS, |
|
verdict: { type: 'string', enum: ['keep', 'rewrite', 'new'] }, |
|
evidence: { type: 'string', description: 'file:line plus the actual code line proving the issue' }, |
|
}, |
|
required: ['severity', 'category', 'file', 'line', 'title', 'description', 'raisedBy', 'verdict', 'evidence'], |
|
}, |
|
}, |
|
dropped: { |
|
type: 'array', |
|
items: { |
|
type: 'object', |
|
properties: { |
|
title: { type: 'string' }, |
|
reason: { |
|
type: 'string', |
|
enum: ['codebase-convention', 'pre-existing', 'theoretically-correct-but-impossible', 'style-out-of-scope', 'false-claim'], |
|
}, |
|
explanation: { type: 'string' }, |
|
}, |
|
required: ['title', 'reason', 'explanation'], |
|
}, |
|
}, |
|
}, |
|
required: ['issues', 'dropped'], |
|
} |
|
const audited = await agent( |
|
[ |
|
`You are the audit judge of a multi-agent code review of "${label}". The reviewers' issues, already structurized:`, |
|
'', |
|
JSON.stringify(issues, null, 2), |
|
'', |
|
`The diff is in ${contextFile}; the repository checkout is at ${repoDir}. For EVERY issue, open the cited file:line and re-read the diff. Then:`, |
|
`- keep: the issue is real as written — cite evidence (file:line + the actual code line).`, |
|
`- rewrite: real but wrong severity/description/anchor — return the corrected issue, with evidence.`, |
|
`- drop: false positive — list it under "dropped" with a reason category and a short explanation.`, |
|
`- new: while verifying you found a genuinely missed concrete issue — add it with verdict "new" and evidence.`, |
|
`Also check whether flagged patterns repeat elsewhere in the changed files, and whether callers/consumers of changed code break.`, |
|
`Every kept/rewritten/new issue MUST carry evidence you actually read. Do not keep an issue on trust.`, |
|
].join('\n'), |
|
{ label: 'auditor', phase: 'Audit', schema: AUDIT_SCHEMA }, |
|
) |
|
if (audited) { |
|
auditedIssues = audited.issues |
|
droppedIssues = audited.dropped |
|
auditRan = true |
|
log(`Audit: ${audited.issues.length} kept/rewritten/new, ${audited.dropped.length} dropped`) |
|
} else { |
|
log('Audit agent failed — returning unaudited issues') |
|
} |
|
} |
|
|
|
return { |
|
label, |
|
analysis: analyzed.analysis, |
|
focusAreas: analyzed.focusAreas, |
|
conclusion, |
|
issues: auditedIssues, |
|
dropped: droppedIssues, |
|
audited: auditRan, |
|
rounds: rounds.map((r) => ({ round: r.round, reviewers: Object.keys(r.messages), failed: r.failed })), |
|
convergedAtRound, |
|
runDir, |
|
} |
