diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 5b3c3d3..793ecf0 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -19,6 +19,7 @@ "usage-budget", "harness", "code-graph", + "code-review", "codex", "cursor", "opencode", diff --git a/AGENTS.md b/AGENTS.md index 0d7ffd6..8139922 100644 --- a/AGENTS.md +++ b/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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8935f6c..2aa000f 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/README.md b/README.md index c4fb904..2a16149 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ A harness is listed as supported only after every eval fixture passes on it. | 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 | ## 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. diff --git a/skills/builder/SKILL.md b/skills/builder/SKILL.md index 016d99f..f129db4 100644 --- a/skills/builder/SKILL.md +++ b/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 | diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md new file mode 100644 index 0000000..44ac1d2 --- /dev/null +++ b/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 /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: + +``` +**** `path/to/file:line` — +What goes wrong: . +Evidence: confirmed by running: , → . | from reading. +Fix: . +Test: — asserts . +``` + +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. diff --git a/tests/evals/review/allow b/tests/evals/review/allow new file mode 100644 index 0000000..8b333e3 --- /dev/null +++ b/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:*) diff --git a/tests/evals/review/expect.md b/tests/evals/review/expect.md new file mode 100644 index 0000000..91a4003 --- /dev/null +++ b/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. diff --git a/tests/evals/review/prompt.md b/tests/evals/review/prompt.md new file mode 100644 index 0000000..5861d5b --- /dev/null +++ b/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. diff --git a/tests/evals/review/repo/README.md b/tests/evals/review/repo/README.md new file mode 100644 index 0000000..af7a669 --- /dev/null +++ b/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. diff --git a/tests/evals/review/repo/config.example.json b/tests/evals/review/repo/config.example.json new file mode 100644 index 0000000..f3d88e1 --- /dev/null +++ b/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" +} diff --git a/tests/evals/review/repo/jotter.py b/tests/evals/review/repo/jotter.py new file mode 100644 index 0000000..5a29ea8 --- /dev/null +++ b/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:])) diff --git a/tests/evals/review/repo/pr/DESCRIPTION.md b/tests/evals/review/repo/pr/DESCRIPTION.md new file mode 100644 index 0000000..8948d46 --- /dev/null +++ b/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. diff --git a/tests/evals/review/repo/pr/backup.diff b/tests/evals/review/repo/pr/backup.diff new file mode 100644 index 0000000..8ad46df --- /dev/null +++ b/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() diff --git a/tests/evals/review/repo/test_jotter.py b/tests/evals/review/repo/test_jotter.py new file mode 100644 index 0000000..3f8e85d --- /dev/null +++ b/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()