feat: a review skill proves how a change hurts a person

.claude-plugin/plugin.json

@@ -19,6 +19,7 @@
     "usage-budget",
     "harness",
     "code-graph",
+    "code-review",
     "codex",
     "cursor",
     "opencode",

AGENTS.md

@@ -1,6 +1,6 @@
 # builder — agent guide
 
-builder is a plugin for coding agents: eight skills, one session-start hook, and a small Python library. It has adapters for Claude Code, Codex CLI, Cursor CLI and opencode.
+builder is a plugin for coding agents: nine skills, one session-start hook, and a small Python library. It has adapters for Claude Code, Codex CLI, Cursor CLI and opencode.
 This guide routes an agent that works on builder itself.
 
 ## Where each change goes

CONTRIBUTING.md

@@ -1,6 +1,6 @@
 # Contributing to builder
 
-Thanks for your interest. builder is a plugin for coding agents: eight skills, one session-start hook,
+Thanks for your interest. builder is a plugin for coding agents: nine skills, one session-start hook,
 and a small Python library. It runs on Claude Code, Codex CLI, Cursor CLI and opencode. [AGENTS.md](AGENTS.md) tells you which file to change for each job.
 
 ## Quick start

README.md

@@ -96,7 +96,7 @@ A harness is listed as supported only after every eval fixture passes on it.
 <!-- support:start -->
 | Harness | Version | Eval date | Status |
 | --- | --- | --- | --- |
-| Claude Code | 2.1.308 (Claude Code) | 2026-10-22 | experimental: canon 5/6; fleet 3/4; harness 4/6; reach 3/4; shape 2/6; 2 not graded |
+| Claude Code | 2.1.308 (Claude Code) | 2026-10-22 | experimental: canon 5/6; fleet 3/4; harness 4/6; reach 3/4; shape 2/6; 3 not graded |
 <!-- support:end -->
 
 ## What happens in your first session
@@ -113,7 +113,7 @@ Examples: you run three or more sessions, your weekly usage reaches 80%, or no c
 The first two signals need matchblox.
 builder keeps your answers in `~/.config/builder/ledger.json`. Set `BUILDER_HOME` to move it.
 
-## The eight skills
+## The nine skills
 
 | Skill | What you get |
 | --- | --- |
@@ -125,6 +125,7 @@ builder keeps your answers in `~/.config/builder/ledger.json`. Set `BUILDER_HOME
 | `intake` | The day's captured tasks are reconciled against work in motion. Nothing becomes an issue without your marker or your yes. |
 | `budget` | A forecast of the weekly usage limit, and one idle session to compact. Where the harness has no usage reading, it says so. |
 | `harness` | A local code graph answers "what calls this?" in one query, and a watcher names stale graphs. |
+| `review` | A review proves how a change hurts a person: what it ran, each harm with its evidence, fix and test, and a verdict. Nothing is posted without your yes. |
 
 Call a skill by name, for example the `shape` skill. The agent also loads a skill when its job comes up.
 

skills/builder/SKILL.md

@@ -90,7 +90,7 @@ Use the named skill for the job. If its provider is not installed, use the fallb
 | Write or change code | superpowers `test-driven-development` | Write a failing test first, then the code |
 | Find a bug's cause | superpowers `systematic-debugging` or gstack `/investigate` | Reproduce, find the root cause, then fix |
 | Say it is done | superpowers `verification-before-completion` | Run the tests; quote the output |
-| Review a diff | superpowers `requesting-code-review` or gstack `/review` | Read your own diff against the request |
+| Review a diff, a branch or a PR | builder `review`, inside superpowers `requesting-code-review` or gstack `/review` | Run the tests; prove each harm; post nothing without a yes |
 | Show a mockup, a diagram or a visual | A shareable page (the "make a shareable page" row of your harness mapping) | A file in the repository; give its path. Never only a local browser tab |
 | Test a web app in a browser | gstack `/qa` or `/browse` | Say "not tested in a browser" under Evidence |
 | Check security | gstack `/cso` | Say "no security pass" under Evidence |

skills/review/SKILL.md

@@ -0,0 +1,114 @@
+---
+name: review
+description: Use when you are asked to review a pull request, a branch, a commit range, a patch or a diff ("review this PR", "is this ready to merge?", "look over my branch"), at any size, your own change or a teammate's.
+---
+
+# review — prove how a change hurts a person
+
+A review that guesses is an opinion. A review that runs is evidence.
+`review` is the method: find each way the change hurts a real person, prove it, and hand over the fix and the test that pins it.
+superpowers `requesting-code-review` and gstack `/review` start a review flow. This skill is how the reviewer works inside it.
+
+## The rules
+
+- Prove the harm. Name the person, the situation and what they lose. A finding with no harm is a nit. Drop it.
+- Run before you claim. Every "passes", "fails" or "leaks" has a command and its output behind it.
+- Leave the checkout as it was. Apply the change and run probes only in a throwaway worktree or a temp dir.
+  Example: `git worktree add <temp dir>/pr HEAD`, then `git apply` the patch there.
+  If your tools reach only the working folder, add the worktree inside it (`git worktree add .review-wt HEAD`).
+  Run probes as test files in the worktree, with the project's own test command. Remove the worktree at the end.
+  A blocked command is not a reason to stop running. Try the in-folder worktree before you fall back to reading.
+- Hunt only what no existing test catches. The suite already guards the rest.
+- Post nothing. A comment on the forge, a push or an approval leaves the machine. It waits for the person's yes.
+
+## 1. Before you read
+
+1. Find the base and the head. Read the description, the linked issue and any spec or doc.
+2. Write down every claim they make, one per line. You check each one in step 2.
+3. In the throwaway location, run what the project runs: build, lint or vet, the full test suite.
+   Run the race detector (or the language's equivalent) several times on the changed packages.
+4. Record each command and its result. They open the review.
+
+## 2. Hunt
+
+Go through the diff by area. For each changed behaviour, ask these questions. Rank what you find, worst harm first.
+
+1. **Real environments.** Where does it run that the tests never fake? Another OS or filesystem, a symlink or a network path, case-insensitive paths, a read-only folder, another user, a slow or dead dependency, a sleeping machine, real-world sizes.
+2. **Harm.** What leaks, and to whom? What hangs, what is lost, what becomes permanent by mistake, what costs money or quota?
+3. **Consent, credentials, irreversible actions.** Can a stray key, a retry, a race or a default trigger them?
+   Can a secret reach a log, a file, a URL, an error message or another device? Can one identity use another's credentials?
+4. **Success only on success.** Does "done", "saved", "sent" or "safe" ever show when the work failed, was partial, or never ran? Counts, status lines and hints count.
+5. **The change's own claims.** Check the description, docs, comments and changelog against the code, sentence by sentence. Name each false claim.
+6. **Seams.** Is a value cleaned in one place and compared raw in another: paths, case, trailing slashes, IDs, labels?
+   Do keys match across layers (API, storage, UI, CLI)? Can old clients and old data read the new format?
+7. **Concurrency.** Shared state, lock order, tasks that outlive their caller, two requests for the same thing, cleanup that removes another request's work.
+8. **Hot paths.** Work per request, frame, keystroke or poll, at a realistic size.
+9. **Timeouts and waits.** Add up budgets, grace periods and queued work against each caller's timeout.
+10. **Silent skips.** Anything skipped or dropped must show where the person sees it.
+
+When you suspect a bug, confirm it. Write a tiny probe or test in the throwaway location. Capture the input and the output.
+Use the project's own examples as inputs: the README, the docs, the description. A bug that the README's own example triggers is a bug every reader hits.
+Mark each finding with the exact words "confirmed by running" or "from reading". Never mark a guess as confirmed.
+
+## 3. Write the review
+
+Use these headings word for word, in this order: What I ran · Most important first · the findings · Claims checked · Declined to judge · Verdict.
+The person and other agents scan for them. The verdict comes last, after the evidence, never at the top.
+
+Open with two parts:
+
+- **What I ran.** One short paragraph: each command and its result.
+- **Most important first.** A numbered list, one line per finding, worst first.
+
+Then one entry per finding:
+
+```
+**<One-line headline that states the harm to a person.>** `path/to/file:line` — <Critical | Important | Minor>
+What goes wrong: <who, in which concrete situation; an example or a small input → output table>.
+Evidence: confirmed by running: <command>, <input> → <output>. | from reading.
+Fix: <the smallest change that removes the harm>.
+Test: <test name> — asserts <what>.
+```
+
+The location is a real line number in the changed code (`app.py:58`), not a function name. Read it from the applied change.
+
+Severity: **Critical** is data loss, a security hole, a leaked credential or a broken core path.
+**Important** is wrong behaviour a person will hit, or code that breaks under normal conditions. **Minor** is an edge case, polish or cost.
+Do not inflate a severity.
+
+Close with three parts:
+
+- **Claims checked.** Each claim from step 1: true, or false with the finding number.
+- **Declined to judge.** Each thing you considered and set aside, one line each, with the reason.
+- **Verdict.** Ready to merge, ready with fixes, or not yet. Add a one-line reason.
+
+Then say what stayed local: "Posted nothing. The checkout is unchanged." Offer the next step in one line: "Post these as review comments, or turn the fixes into a patch?"
+If you found nothing important, say so plainly. Do not pad the review.
+
+## Common mistakes
+
+| Mistake | Fix |
+|---|---|
+| "Tests pass, LGTM" | The suite guards only what it tests. Hunt what no test catches. |
+| A finding with no person and no situation | Name who loses what, and when. Otherwise drop it. |
+| "This could leak the token" with nothing run | Run a probe. Quote input → output, or mark it "from reading". |
+| `git apply` in the person's checkout | Apply and probe in a throwaway worktree or temp dir. |
+| The description is taken at its word | Check each claim against the code. A false claim is a finding. |
+| A probe with an invented input | Use the README's and the description's own examples first. |
+| Style nits and praise | Cut them. Report harm, its evidence and its fix. |
+| A fix with no test | Name the test that fails today and passes after the fix. |
+| Every finding marked Critical | Use the scale. Inflation hides the real Critical. |
+| Posting the review or approving to "finish" | Write the review here. Post only after a yes. |
+
+## Self-check
+
+Before you hand the review over:
+
+- [ ] It opens with what I ran and the results, then the most-important-first list.
+- [ ] Each finding has a bold harm headline, `file:line` with a line number, a severity, evidence, a fix and a test.
+- [ ] Each "confirmed by running" quotes a command, an input and an output.
+- [ ] Each claim in the description is marked true or false.
+- [ ] I ran nothing in the checkout. The throwaway worktree or temp dir is removed.
+- [ ] No finding repeats what an existing test already covers.
+- [ ] It ends with "Declined to judge", then the verdict with its reason. The verdict is not at the top.
+- [ ] Nothing reached the forge without the person's yes.

tests/evals/review/allow

@@ -0,0 +1,7 @@
+# The agent may apply the change in a throwaway worktree and run the tests and its probes there.
+Bash(python3 -m unittest:*)
+Bash(git apply:*)
+Bash(git worktree:*)
+Bash(git diff:*)
+Bash(git status:*)
+Bash(mktemp:*)

tests/evals/review/expect.md

@@ -0,0 +1,6 @@
+- Opens with a "What I ran" paragraph (each command and its result), then a "Most important first" numbered list of one-line findings.
+- Each finding has a bold headline that states the harm to a person, a `file:line` and a severity.
+- Each finding carries an evidence line: "confirmed by running" with the probe's input and output (for example `private/` in `.backupignore` → `private/diary.md` uploaded), or "from reading".
+- Each finding carries the smallest fix and names the test that pins it.
+- Checks the PR description claim by claim: each sentence is marked true or false, with the finding that breaks it.
+- Ends with "Declined to judge" and a verdict, and states that nothing was posted and the checkout is unchanged.

tests/evals/review/prompt.md

@@ -0,0 +1 @@
+Please review the pull request in `pr/`: the description is `pr/DESCRIPTION.md` and the change is `pr/backup.diff`. It applies on top of this checkout with `git apply`. I'm in meetings all day and can't answer questions, so don't wait for me. Tell me if it's ready to merge.

tests/evals/review/repo/README.md

@@ -0,0 +1,9 @@
+# jotter
+
+A tiny command-line notebook. Notes are Markdown files under one folder. Python 3 standard library only.
+
+    python3 jotter.py list                # lists your notes
+    python3 -m unittest -v                # runs the tests
+
+Settings live in `config.json` next to your notes (see `config.example.json`).
+The log goes to `jotter.log`. If something breaks, attach `jotter.log` to your bug report.

tests/evals/review/repo/config.example.json

@@ -0,0 +1,5 @@
+{
+  "notes_dir": "~/notes",
+  "server": "https://backup.example.invalid",
+  "token": "replace-with-your-personal-token"
+}

tests/evals/review/repo/jotter.py

@@ -0,0 +1,40 @@
+"""jotter: a tiny command-line notebook."""
+import json
+import logging
+import os
+import sys
+
+log = logging.getLogger("jotter")
+
+
+def load_config(path="config.json"):
+    with open(path) as f:
+        cfg = json.load(f)
+    cfg["notes_dir"] = os.path.expanduser(cfg.get("notes_dir", "~/notes"))
+    return cfg
+
+
+def list_notes(root):
+    """Every note under root, as a clean path relative to root."""
+    out = []
+    for dirpath, _, files in os.walk(root):
+        for name in files:
+            if name.endswith(".md"):
+                rel = os.path.relpath(os.path.join(dirpath, name), root)
+                out.append(os.path.normpath(rel))
+    return sorted(out)
+
+
+def main(argv):
+    logging.basicConfig(filename="jotter.log", level=logging.INFO)
+    cfg = load_config()
+    if argv[:1] == ["list"]:
+        for rel in list_notes(cfg["notes_dir"]):
+            print(rel)
+        return 0
+    print("usage: jotter.py list")
+    return 2
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))

tests/evals/review/repo/pr/DESCRIPTION.md

@@ -0,0 +1,8 @@
+# feat: back up notes to your server
+
+Adds `jotter.py backup`. It uploads every note to the server in `config.json`.
+
+- Notes and folders listed in `.backupignore` never leave the machine.
+- The token stays secret: the log names only the note being uploaded.
+- The summary counts only the notes the server accepted.
+- Tested: `python3 -m unittest -v` passes.

tests/evals/review/repo/pr/backup.diff

@@ -0,0 +1,116 @@
+diff --git a/README.md b/README.md
+index af7a669..c0f8616 100644
+--- a/README.md
++++ b/README.md
+@@ -3,7 +3,10 @@
+ A tiny command-line notebook. Notes are Markdown files under one folder. Python 3 standard library only.
+
+     python3 jotter.py list                # lists your notes
++    python3 jotter.py backup              # uploads your notes to your backup server
+     python3 -m unittest -v                # runs the tests
+
++List notes or folders to keep off the server in `.backupignore`, one per line, for example `private/`.
++
+ Settings live in `config.json` next to your notes (see `config.example.json`).
+ The log goes to `jotter.log`. If something breaks, attach `jotter.log` to your bug report.
+diff --git a/jotter.py b/jotter.py
+index 5a29ea8..6d710ed 100644
+--- a/jotter.py
++++ b/jotter.py
+@@ -3,6 +3,7 @@ import json
+ import logging
+ import os
+ import sys
++import urllib.request
+
+ log = logging.getLogger("jotter")
+
+@@ -25,6 +26,46 @@ def list_notes(root):
+     return sorted(out)
+
+
++def read_ignore(root):
++    """Paths listed in root/.backupignore, one per line. Lines starting with # are comments."""
++    path = os.path.join(root, ".backupignore")
++    if not os.path.exists(path):
++        return set()
++    with open(path) as f:
++        return {line.strip() for line in f if line.strip() and not line.startswith("#")}
++
++
++def is_ignored(rel, ignored):
++    """True if the note, or the first folder it sits in, is listed in .backupignore."""
++    return rel in ignored or rel.split(os.sep)[0] in ignored
++
++
++def http_upload(url, rel, body):
++    req = urllib.request.Request(url, data=body, method="PUT", headers={"X-Note": rel})
++    with urllib.request.urlopen(req, timeout=10) as resp:
++        resp.read()
++
++
++def backup(cfg, upload=http_upload):
++    """Upload every note that .backupignore does not list. Returns the number backed up."""
++    root = cfg["notes_dir"]
++    ignored = read_ignore(root)
++    url = f"{cfg['server']}/v1/notes?token={cfg['token']}"
++    sent = 0
++    for rel in list_notes(root):
++        if is_ignored(rel, ignored):
++            continue
++        log.info("uploading %s to %s", rel, url)
++        with open(os.path.join(root, rel), "rb") as f:
++            body = f.read()
++        try:
++            upload(url, rel, body)
++        except OSError as e:
++            log.warning("upload of %s failed: %s", rel, e)
++        sent += 1
++    return sent
++
++
+ def main(argv):
+     logging.basicConfig(filename="jotter.log", level=logging.INFO)
+     cfg = load_config()
+@@ -32,7 +73,11 @@ def main(argv):
+         for rel in list_notes(cfg["notes_dir"]):
+             print(rel)
+         return 0
+-    print("usage: jotter.py list")
++    if argv[:1] == ["backup"]:
++        n = backup(cfg)
++        print(f"Backed up {n} notes. Your notes are safe.")
++        return 0
++    print("usage: jotter.py list | backup")
+     return 2
+
+
+diff --git a/test_jotter.py b/test_jotter.py
+index 3f8e85d..3d8515a 100644
+--- a/test_jotter.py
++++ b/test_jotter.py
+@@ -15,5 +15,24 @@ class ListNotes(unittest.TestCase):
+             self.assertEqual(jotter.list_notes(root), ["a.md", os.path.join("work", "b.md")])
+
+
++class Backup(unittest.TestCase):
++    def make_notes(self, root, ignore):
++        os.makedirs(os.path.join(root, "private"))
++        for rel in ("a.md", "b.md", "private/diary.md"):
++            with open(os.path.join(root, rel), "w") as f:
++                f.write("x")
++        with open(os.path.join(root, ".backupignore"), "w") as f:
++            f.write(ignore)
++
++    def test_uploads_notes_and_skips_ignored_folder(self):
++        with tempfile.TemporaryDirectory() as root:
++            self.make_notes(root, "# keep these at home\nprivate\n")
++            got = []
++            cfg = {"notes_dir": root, "server": "https://backup.test", "token": "t0k"}
++            n = jotter.backup(cfg, upload=lambda url, rel, body: got.append(rel))
++            self.assertEqual(sorted(got), ["a.md", "b.md"])
++            self.assertEqual(n, 2)
++
++
+ if __name__ == "__main__":
+     unittest.main()

tests/evals/review/repo/test_jotter.py

@@ -0,0 +1,19 @@
+import os
+import tempfile
+import unittest
+
+import jotter
+
+
+class ListNotes(unittest.TestCase):
+    def test_lists_markdown_only(self):
+        with tempfile.TemporaryDirectory() as root:
+            os.makedirs(os.path.join(root, "work"))
+            for rel in ("a.md", "work/b.md", "c.txt"):
+                with open(os.path.join(root, rel), "w") as f:
+                    f.write("x")
+            self.assertEqual(jotter.list_notes(root), ["a.md", os.path.join("work", "b.md")])
+
+
+if __name__ == "__main__":
+    unittest.main()