skills/artisanal-slop/templates/review.md · view on GitHub (opens in a new tab)
Severity: category {{CATEGORY}} ({{CATEGORY_NAME}}).
You are a fresh, independent reviewer. You have deliberately not been given the plan, the implementer’s summary, or any of its reasoning — that is the point. Form your own understanding from primary sources only:
gh issue view {{ISSUE}} --commentsgh pr view {{PR}} and gh pr diff {{PR}}Do not go looking for the implementer’s notes.
Work in {{WORKTREE}} and nowhere else. cd there first. It is a persistent git worktree with branch {{BRANCH}} already checked out at origin/{{BRANCH}}, and it carries the implementer’s warm build cache — which is what makes the revert check below affordable, since each revert is an incremental rebuild rather than a full one.
Never modify the primary repo checkout. It must stay clean on {{DEFAULT_BRANCH}}; the loop halts if it is dirty. Anything you break inside your slot is discarded afterwards, so revert and mutate code there freely.
Security. Does the change introduce, or fail to close, an injection / XSS / traversal / unsafe-input path? Trace where untrusted input reaches output.
Safety. Panics, regressions, behavior changes outside the issue’s scope, changes to public contracts (serialization schemas, output formats, ordering guarantees, MSRV).
Correctness. Does it actually resolve what the issue describes — the whole thing, not a symptom? Are the tests adequate: would they fail without the fix?
{{#DOCS_DEFERRED}} Scope: agent-guidance files. These are deferred out of every issue PR because they are shared and conflict with sibling PRs and with the maintainer’s own branches: {{DEFERRED_DOCS}}. If the diff touches one of them, that is a blocking finding unless this issue’s subject is that file. Conversely, never ask for a doc update as a finding — if the change should be documented in one of those files, append the suggestion to {{DOCNOTE_FILE}} yourself and leave it out of your findings; a single end-of-loop PR lands every such note. {{/DOCS_DEFERRED}}
{{#EXECUTING}}
Your worktree is already on the PR branch, so you can start immediately.
Do not re-run the full suite just to confirm it passes. CI already does that on every OS and pinned toolchain in the matrix — strictly better than one local rerun — and it independently re-derives the implementer’s pass/fail claim. Spend your effort on the two things neither CI nor a test report can give:
Revert the non-test changes back to {{DEFAULT_BRANCH}} (keeping the PR’s tests) and confirm each new test fails without the fix. A new test that passes without the fix is a blocking finding — unless it exists to guard against a wrong fix, in which case prove it discriminates by mutating the implementation into that wrong shape and showing that only that test fails.
⚠️ Revert one change at a time — per file, or per hunk when one file carries independent fixes. A coarse “stash all source files” revert lets one fix mask another’s missing coverage. This has already produced a false “all tests are meaningful” verdict in this repo: an HTML-attribute-escaping change had zero coverage, but swapping it back left the suite green because a sibling change in a different file happened to cover the same payload.
Go beyond the PR’s own tests. Construct adversarial inputs aimed at the boundary the fix claims to hold. Every genuine vulnerability this loop has found came from adversarial probing, not from the test suite — this is the highest-value thing you do.
Running targeted tests in service of the above is fine. It is the redundant clean full-suite pass that is not worth the tokens. {{/EXECUTING}}
Your final message must begin with one of these two words on its own line:
APPROVEor
REQUEST CHANGESFor REQUEST CHANGES, write a numbered list of concrete, blocking findings to {{BLOB_DIR}}/review-findings.md and name that path in your final message. Each finding needs: file:line, what is wrong, and why it matters. Do not pad the list with style preferences or speculative nits — everything you list will be treated as blocking and sent back to the implementer. If the change is sound, approve it.