argus-review

name

argus-review

description

Use when asked to "argus review", "review this PR", "review this branch", "build an annotated review site", "explain this diff file by file", or to produce a browsable, evidence-backed review of a change — a reading journey, per-file pages, per-unit notes in plain language, and verified findings with a fix direction. Accepts a PR URL or number, a branch, or the current checkout including uncommitted work. Builds a self-contained static site and serves it locally.

ARGUS review

Turn a change into a browsable review that leads with plain language and keeps the code one click away: a reading journey ordered by dependency, a page per file, a note per unit, and — where a claim was actually checked — a finding with its evidence.

Two principles hold the whole thing up.

The filesystem is the accumulator. Every stage writes its result to disk before the next begins. That is what makes this resumable after a crash or a context compaction, incremental on re-runs, and runnable on agents that cannot fan out parallel workers.

Verify before warning. A note that says "consider whether this is safe" costs the reader time and tells them nothing. Check the claim and state the fact, or name the exact human decision. verify.mjs enforces this — it is not advice.

Three hard stops. Scope, cost, and checks. Do not continue past any of them without an answer.

Layout and paths

Resolving the plugin root, referred to below as <plugin>:

1. ${CLAUDE_PLUGIN_ROOT} when it is set — the plugin is installed.
2. Otherwise resolve symlinks first, then take two levels up: dirname(dirname(readlink -f <path to this SKILL.md>/..)). This skill is commonly reached through ~/.claude/skills/argus-review, which is a symlink; walking up from the link path lands in ~/.claude and every script below would be missing. readlink -f is what makes the scripts findable.
3. Verify before running anything: <plugin>/scripts/gen-data.mjs must exist. If it does not, you resolved the wrong root — stop and say so rather than improvising a pipeline without the scripts.

<plugin>/scripts/resolve-target.mjs  # PR/branch/dirty tree -> isolated checkout + target.json
<plugin>/scripts/gen-data.mjs        # diff -> data.js + buckets.json + review.json
<plugin>/scripts/merge.mjs           # bucket JSONs -> annotations.js / summaries.js / overview.js / notes/
<plugin>/scripts/order.mjs           # summaries + import graph -> order.js (the reading order)
<plugin>/scripts/checks.mjs          # approved commands -> checks.js + checks/<id>.log
<plugin>/scripts/verify.mjs          # structural + evidence checks, non-zero exit on failure
<plugin>/assets/viewer/index.html    # the viewer — a shared, versioned asset; copy it, never edit
                                     # it per review (any change belongs in the plugin, for everyone)
<plugin>/references/review-model.md  # the shapes every stage reads and writes
<plugin>/references/language-rules.md
<plugin>/skills/argus-review/references/{annotation,summary,overview}.md

Output goes outside the repository under review:

~/artifacts/argus-review/<date>-<repo>-<target>/
  target.json  data.js  annotations.js  summaries.js  overview.js  order.js  checks.js
  review.json  buckets.json  order.json  checks.json  index.html
  annotations/<bucket-id>.json    # one file per bucket, named for its id in buckets.json
  summaries/batch-NN.json         # one file per summary batch
  notes/<path__>.json             # per-file notes, written by merge.mjs for the summaries pass
  checks/<check-id>.log           # full output of each check that ran

Outside the repository on purpose: the review runs lint, type and test commands, and doing that inside the user's own checkout writes caches and races their editor. It also keeps the review out of their git status.

Pipeline

Run the stages in order. Do not skip a stage because it "looks done" — check its output file on disk instead.

1. Preflight

Check, and stop with a clear message if any is missing:

• git --version
• node --version — must be ≥ 18
• gh --version — only when the target is a PR
• a static server: python3 --version, else npx --version
• the current directory is inside a git work tree

No npm installs. macOS/Linux only — say so and stop if the platform is Windows.

2. Resolve the target

node <plugin>/scripts/resolve-target.mjs <repoDir> [--target <pr-url|pr-number|branch>] --out <reviewDir>

One --target covers every intake: a PR URL, a PR number, a branch name, or nothing at all for the current checkout. It fetches, resolves the base from the remote, creates a throwaway worktree, and writes target.json.

Two behaviours to know:

• Uncommitted work is included when reviewing the current checkout. The script folds it in with git stash create, which builds a commit whose tree is the working tree without touching the working tree. target.json records isDirty and dirtyFileCount.
• The checkout is throwaway. Everything downstream reads target.checkoutDir, not the user's repo. Remove it at the end with --cleanup, and remove it too when the run fails or is interrupted.

3. Scope gate — HARD STOP

The worst failure this pipeline can have is diffing against a stale local base: a colleague's already-merged work shows up as the branch's own. resolve-target fetches and resolves from the remote every time, so report what it resolved and let the user confirm the file list.

Report in one message:

• the resolved range, and the base branch it came from
• whether uncommitted work is included, and how many files — required, never left implicit
• the changed file count and total +adds −dels
• the file list; if longer than ~60 files, group by top-level directory with counts and offer the full list
• proposed exclusions that gen-data.mjs applies by default: lockfiles, _generated/ paths, binaries, and mode-only changes. Name them.
• a request for the user's own exclusions, with concrete suggestions from the actual list — e.g. "exclude apps/ (frontend, 41 files)?", "exclude tests (76 files, +34k lines)?"

Then stop and wait.

The file list must be confirmed before any model tokens are spent.

Do not proceed on an assumed yes, on silence, or on a general "go ahead" given before the list was shown. If the confirmed scope is 0 files, stop and say so.

4. Generate data

node <plugin>/scripts/gen-data.mjs <target.checkoutDir> <mergeBase> <headRef> \
  --exclude <comma,separated,paths> --out <reviewDir>

Writes data.js (window.FILES), buckets.json (the work packing — a file with ≥300 changed lines gets its own bucket, the rest packs to ≤600 lines / ≤8 files), and review.json. fileHash is sha256 of mergeBaseOid + ":" + headBlobOid, so a merge-base move invalidates every modified file. That is intentional.

Read buckets.json for the bucket count and ids. Do not re-derive the buckets — the constants live in the script. buckets.json is an object; the list is buckets.buckets.

5. Cost gate — HARD STOP

• annotation buckets = the bucket count
• summary batches = ceil(non-deleted file count / 8)
• token estimate = (buckets + batches) × ~38k — the measured average per worker. Label it an estimate.
• name the model that will spend them. A token count is not a cost until the model is known, and the same estimate can mean very different spend. Re-prompt if the model changes after this gate.
• time estimate = a range, not a number.
• no dollar figures.

Then stop and wait for explicit go-ahead.

The 300-file rule: at ≥300 files, refuse to fan out. Say the scope is too large to review well or cheaply, offer concrete narrowings, and continue only if the user narrows it or types an explicit yes (a plain "ok" is not enough — ask them to type e.g. yes, review all 340 files).

6. Check gate — HARD STOP

Before running anything in the checkout, propose the commands: type-check, lint, the test command, whatever the repository actually has. Show the exact command lines and get approval once.

node <plugin>/scripts/checks.mjs <reviewDir> <target.checkoutDir> \
  --cmd "typecheck=pnpm -w typecheck" --cmd "lint=pnpm -w lint"

Never run a repository command that was not approved here. If the user declines every command, run checks.mjs with no --cmd so the empty record exists, and say that findings will be limited to what can be established by reading.

Run this before the annotation pass — the workers cite check results as evidence, so the results have to exist first.

7. Annotation pass

One unit of work = one bucket. Every bucket ends by writing its JSON to disk and forgetting its contents.

The prompt is references/annotation.md. Hand each worker:

• the bucket id verbatim, and its file list inlined as literal text
• target.checkoutDir, the range <mergeBase>..<headRef>, and the review dir
• the fileHash of each file, copied from data.js
• the contents of <plugin>/references/language-rules.md and the unit and finding shapes from <plugin>/references/review-model.md (inline them)
• the list of recorded check ids from checks.json, so a finding can cite one

Parallel branch. One worker per bucket, several at a time. Each writes annotations/<id>.json itself and returns only {file_written, units, findings}. Never let a worker return unit bodies — that defeats the whole design.

Sequential branch. Otherwise loop buckets one at a time: read the bucket's files, write annotations/<id>.json, then drop the contents from working memory before the next bucket. A crash resumes by skipping buckets whose JSON exists.

8. Merge

node <plugin>/scripts/merge.mjs <reviewDir>

Concatenates the bucket JSONs (tolerating unparseable files — it logs them), writes notes/<path__>.json per annotated path for the summaries pass, and guarantees overview.js, order.js and checks.js exist so the viewer's script tags never 404. Run it after every pass.

9. Summaries pass

One summary per non-deleted file, {path, purpose, functions, decisions, fileHash}. The prompt is references/summary.md.

Notes first, code second. merge.mjs has already written the per-file notes, so a worker reads those and only opens code when unsure. If notes/ is empty the annotation pass or the merge did not finish, and the summary workers will fall back to reading whole files at full cost.

~8 files per batch with parallel workers; 3–4 sequentially. Each batch writes summaries/batch-NN.json. Then run merge.mjs again.

10. Ordering pass

node <plugin>/scripts/order.mjs <reviewDir>

Computes the reading order — foundations, backend, frontend consumers, tests, supporting — with in-change import edges overriding the default rank.

This runs after annotation on purpose. Reading order is a property of what the files do, and nothing knows that until the summaries exist. It is a different job from buckets.json, which packs work by line count and which the reader never sees.

11. Overview pass

One worker, using references/overview.md. It reads the merged summaries, annotations, order and checks — not the code — and writes overview.json as the Flow journey. Run merge.mjs again so overview.js picks it up.

Skipping this is fine on a tiny change: merge.mjs still writes window.OVERVIEW = null and the viewer shows the findings index alone. Say when you skip it.

12. Adversarial verification

Every finding gets a second reader that did not write it, prompted to refute it. Prefer a different vendor from the annotation worker.

Give the refuter the finding, its evidence, and the file. It returns one of:

• CONFIRMED — the finding stands
• WRONG — with the killing evidence. Delete the finding from its bucket JSON.
• UNCLEAR — demote it to a decision naming what a person must resolve

Then re-run merge.mjs. Skip this stage only when there are no findings, and say so. A finding nobody tried to break is a guess with a severity attached.

13. Verify

node <plugin>/scripts/verify.mjs <reviewDir>

Non-zero exit means the site is not shippable. It checks that the data files parse, every non-deleted file has a unit and a summary, anchors land inside the rendered range, every snippet matches within ±3 lines (re-snapping where it can), shapes match, every finding has a severity and evidence that resolves, no statement is a vague caution, and every failed check is cited by some finding.

Act on failures, do not narrate them:

• drifted anchor — fix line/endLine/snippet in the bucket JSON, re-merge
• missing coverage — re-run just that bucket
• unparseable bucket JSON — re-run that bucket
• finding with no evidence — either verify it and cite the line, or convert it to a decision, or delete it
• failed check nobody cited — the check failed and the review is silent about it. Read the log, and either write the finding that explains it or drop the check from the run

Loop until it exits 0.

14. Copy the viewer, serve, clean up

cp <plugin>/assets/viewer/index.html <reviewDir>/index.html
cd <reviewDir> && (nohup python3 -m http.server <port> --bind 127.0.0.1 >/dev/null 2>&1 &)

Detached matters: a normal background job dies with the agent session. Probe ports upward from 8600 until one is free, and confirm the server answers (curl -sSf -o /dev/null http://127.0.0.1:<port>/index.html).

Fallbacks: no python3 → npx serve -l <port> .; neither works → print the exact command for the user and say the site is complete on disk either way.

Then remove the throwaway checkout:

node <plugin>/scripts/resolve-target.mjs --cleanup <reviewDir>

Record the port in review.json, then print:

• the URL
• coverage stats, e.g. 34 files, 187 units, 6 findings (1 blocker), 34 summaries, verify green
• one line on how to read it: the overview is the reading journey; the sidebar is ordered by dependency, not by folder; each file opens as notes with code behind Inspect; j/k next/prev, i inspect all, d full diff, v mark viewed

Resume and incremental re-runs

Re-invoking on a target that already has a review directory is a resume.

1. Always re-resolve and regenerate data.js. Re-run resolve-target.mjs and gen-data.mjs, passing excludePatterns from the existing review.json back in via --exclude — the patterns, not the concrete paths in exclusions, or a file added under an excluded directory leaks into scope. The merge base moves when the base branch moves, and a stale data.js silently reviews the wrong range. Re-confirm scope only if the file list changed.
2. Skip finished buckets. A bucket whose JSON exists and parses is done.
3. Re-annotate changed files. Compare each file's new fileHash against the one recorded on its existing units. Where they differ, delete that file's units and re-run the bucket, and re-do its summary.
4. Then merge → order → verify → re-copy the viewer → serve.

Re-run checks.mjs whenever the head moved; a check result from a previous head is not evidence about this one. The cost gate applies to the re-run, estimating only what you intend to re-run.

When things break

• "Way more files than I expected." Stale base. Re-run resolve-target.mjs; it fetches every time. This is the #1 failure of this pipeline.
• Uncommitted work missing from the review. The target was a branch or PR name, which means "review that ref", not "review this desk". Re-run with no --target to include the working tree.
• A worker reports an empty file list. Argument passing through the dispatch layer is unreliable. Inline the bucket's file list as literal text.
• Anchors drifted a few lines. Notes are anchored at generation time; if the branch moves the notes on files the base also touched drift. Regenerate data.js always; re-run buckets only for the drifted files. verify.mjs finds these via the snippet check.
• verify.mjs fails on a file with no units. Its bucket worker errored or wrote invalid JSON. Check annotations/ for a missing or truncated file.
• The served page is blank. index.html was not copied, or a .js data file is not valid JavaScript. Re-run merge.mjs; it is the only thing that should ever write those files.
• Syntax highlighting looks plain. Expected. The tokenizer is TypeScript-oriented and degrades to plain text elsewhere.

Red flags — stop and fix

• Proceeding past the scope, cost, or check gate without an explicit answer
• Running a repository command that was not approved at the check gate
• Diffing against a local base branch instead of the remote merge base
• Reviewing in the user's own checkout instead of the throwaway one
• Writing annotations.js / summaries.js / overview.js / order.js / checks.js by hand instead of via the scripts
• A worker returning unit bodies into the main context
• Filing a finding with no evidence, or a caution that names nothing
• Calling the site done while verify.mjs exits non-zero
• Leaving the throwaway checkout behind
• Inventing note content from the diff header without reading the code