Review Changes
ASecurityReview the changes since a fixed point (commit, branch, tag, or merge-base), or an uncommitted change, along three axes — Standards (does the code follow this repo's documented coding standards?), Spec (does the code match what the originating ticket or spec asked for?) and Architecture (is it in the right module, pointing the right way?). Runs each axis in a parallel sub-agent that follows the same axis steps as the loop's review, and reports them side by side. Use when the user wants to rev...
- 2 stars
- 0 votes
- 0 copies
- 0 views
- Added October 20, 2026
Works with
Security analysis
100/100Pro scans all 4 files and shows the line behind each finding
npx -y skills add MalcolmMcNeely/skillworks --skill review-changes --agent claude-codeAre you the author of Review Changes?
Add the live security badge to your README. It updates with every re-scan.
[](https://www.skillsdirectory.com/skills/malcolmmcneely-review-changes)---
name: review-changes
argument-hint: "[fixed point] [ticket]"
description: >
Review the changes since a fixed point (commit, branch, tag, or merge-base), or an uncommitted
change, along three axes — Standards (does the code follow this repo's documented coding
standards?), Spec (does the code match what the originating ticket or spec asked for?) and
Architecture (is it in the right module, pointing the right way?). Runs each axis in a parallel
sub-agent that follows the same axis steps as the loop's review, and reports them side by side.
Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to "review
since X".
Not for a check on correctness alone: that is Claude Code's own /code-review.
---
Three-axis review of a change:
- **Standards** — does the code conform to this repo's documented coding standards?
- **Spec** — does the code faithfully implement the originating ticket or spec?
- **Architecture** — is the code in the right module, and do its dependencies point the way this repo says they should?
Each axis runs as a **parallel sub-agent** so they don't pollute each other's context, then this skill sets their reports side by side.
Each axis keeps its steps in an axis file in this skill's folder, `${CLAUDE_SKILL_DIR}`. The spec loop's review step follows the same files, so a review by hand checks what the loop checks:
| Axis | Axis file, in `${CLAUDE_SKILL_DIR}` |
|---|---|
| Standards | `standards.md` |
| Spec | `spec.md` |
| Architecture | `architecture.md` |
`docs/agents/issue-tracker.md` says how this repo names and reads a ticket. If it is missing, stop. Tell the user that it is missing, that `/skillworks:skillworks-setup` writes it, and that nothing was reviewed.
## Pin the fixed point
Whatever the user said is the fixed point — a commit SHA, branch name, tag, `main`, `HEAD~5`, etc. If they didn't specify one, ask for it, unless the review is of an uncommitted change.
Capture the diff command once: `git diff <fixed-point>...HEAD` (three-dot, so the comparison is against the merge-base). Also capture `git diff <fixed-point>...HEAD --stat -M`, and the list of commits via `git log <fixed-point>..HEAD --oneline`.
**An uncommitted change** has no commits for a three-dot diff to see, and needs no fixed point. Run `git add -N .` so new files show, then use `git diff HEAD` and `git diff HEAD --stat -M` in place of the commands above. There is no commit list.
Before going further, confirm the fixed point resolves (`git rev-parse <fixed-point>`) and the diff is non-empty. If the ref does not resolve, or the diff is empty, stop. Tell the user which, and start no sub-agent: three of them would each fail on it apart.
## Find the spec
Look for the ticket or spec the change was built for, in this order:
1. The `Ticket:` trailer on the commits, read back as `docs/agents/issue-tracker.md` says. Fetch the ticket and its parent spec through the same docs.
2. An issue number, or a ticket named as its Tracker names it, passed as this skill's argument. An uncommitted change carries no trailer, so this is where its ticket comes from.
3. With neither, ask the user which ticket or spec the change was built for. If they say there isn't one, skip the Spec sub-agent and note that in the report.
## Spawn the sub-agents in parallel
Start the sub-agents in one message, one sub-agent call for each axis, so they run at the same time. Use the general-purpose type, because an axis runs git and its checks in a shell, and reads the whole files its sources name.
Give each sub-agent the same four things, and let it read the rest itself:
- The full path to its axis file: `${CLAUDE_SKILL_DIR}/` and the file name from the table above. Tell it to read that file and follow it in report-only mode.
- The diff command and the `--stat -M` command.
- The commit list, or a line saying the change is uncommitted.
- The ticket or spec: its number and Tracker, or the fetched contents.
Say in each prompt that the sub-agent edits nothing. A review by hand reports, and the user decides what to change.
## Aggregate
Present each report under its `## Standards`, `## Spec` or `## Architecture` heading, as it came. Keep every finding, with its file, its line and its quote, because a finding cut here is lost with nothing to show it. Tidy the layout only, and write each heading once. Do **not** merge or rerank findings — the axes are deliberately separate (see _Why three axes_).
If an axis stops because a file it needs is missing, give its message under its heading as it came.
End with a one-line summary: total findings per axis, and the worst issue _within each axis_ (if any). Don't pick a single winner across axes — that's the reranking the separation exists to prevent.
## Why three axes
A change can pass on one axis and fail on another, so each masks the others when merged:
- Code that follows every standard but implements the wrong thing → **Standards pass, Spec fail.**
- Code that does exactly what the issue asked but breaks the project's conventions → **Spec pass, Standards fail.**
- Code that is well named, well tested, and exactly what the issue asked for, in a module that should never have imported it → **Standards and Spec pass, Architecture fail.**
The third axis is separate from Standards for two further reasons. Its evidence is different — Standards reads the diff, Architecture needs the module graph the diff sits in. And its remedies cost differently: "rename this" and "invert this dependency" do not belong in one ranked list, because the cheap findings crowd out the expensive ones.
Files in this skill
- SKILL.md
- architecture.md
- spec.md
- standards.md
Attribution
Comments
Loading comments…
Newsletter
Get the weekly list of safe new skills, and the ones we pulled.