# Reviewing Changes

> Five-pass review of a diff: code, security, architecture, acceptance, AI-native.

- **Type:** Skill
- **Install:** `agentstack add skill-swell-agents-coding-skills-reviewing-changes`
- **Verified:** Yes — security-reviewed for prompt injection and unsafe behavior
- **Seller:** [swell-agents](https://agentstack.voostack.com/s/swell-agents)
- **Installs:** 0
- **Category:** [AI & ML](https://agentstack.voostack.com/c/ai-and-ml)
- **Latest version:** 0.1.0
- **License:** MIT
- **Upstream author:** [swell-agents](https://github.com/swell-agents)
- **Source:** https://github.com/swell-agents/coding-skills/tree/main/skills/reviewing-changes

## Install

```sh
agentstack add skill-swell-agents-coding-skills-reviewing-changes
```

Requires the [AgentStack CLI](https://agentstack.voostack.com/docs/cli). Works with Claude Code, Cursor, and any MCP-compatible agent.

## About

## Process

Always run the five passes in order. Findings flow into one combined verdict.

### 1. Read the rules

Before any pass, internalise the language-rule skill that matches the diff (python-conventions, go-conventions, solidity-conventions, …) and engineering-philosophy. The rule skills are the source of truth — don't invent additional standards.

### 2. See the change

```
git diff ...HEAD
git log ..HEAD --oneline
```

For a GitHub PR:

```
gh pr view 
gh pr diff 
```

### 3. Pass 1 — Code quality

Check, in order:

- **Philosophy violations** — over-engineering (KISS, YAGNI), duplication (DRY), magic behaviour (No Magic), copy-paste-modified blocks.
- **Redundant entity & local-pattern reuse**: when the diff adds a function, accessor, or code path the repo already provides (an auto-generated getter, a wider getter that subsumes a narrower one, an idiom a sibling file implements), reuse the existing form instead of a second variant. DRY and YAGNI span the whole repo, not just this diff.
- **SOLID violations** — Single Responsibility first; flag classes/files that grew a second responsibility.
- **Naming, readability, complexity** — function lengths, parameter lists, deeply nested conditionals, clever one-liners that hide intent.
- **Test coverage** — was the change tested? If TDD discipline applied, was the failing test committed first?
- **Tooling compliance** — ruff/mypy strict for Python, golangci-lint for Go, solhint:all for Solidity, forge fmt --check for Solidity formatting.
- **Configuration safety** — production timeouts, connection pools, missing retries, missing rate limits.

### 4. Pass 2 — Security audit

Check, in order, against OWASP Top 10:

- **Injection** — SQL, command, LDAP, template, header injection via unsanitised input.
- **Broken authentication** — weak token handling, missing MFA, fragile session management.
- **Broken access control** — missing authz checks, privilege escalation, IDOR (insecure direct object reference).
- **Sensitive data exposure** — secrets in logs, error messages, or response bodies; missing TLS; weak ciphers.
- **Misconfiguration** — overly permissive CORS, missing security headers, debug endpoints exposed.
- **Vulnerable components** — `pip-audit` / `npm audit` / `govulncheck` / dep CVEs.
- **XSS** — unencoded output rendered as HTML/JS; missing CSP.
- **Insecure deserialisation** — `pickle.loads` on untrusted input, similar in JS/Java.
- **Insufficient logging and monitoring** — security-relevant events not logged, no alerting.
- **Cryptographic issues** — weak algorithms, hardcoded keys, missing key rotation, predictable IVs.
- **Smart-contract specific (if Solidity)** — reentrancy, integer over/underflow, unchecked external calls, access control on `onlyOwner`-style modifiers, front-running, MEV exposure, signature replay.

See `reference/owasp-checklist.md` for the canonical mapping with attack-vector notes.

### 5. Pass 3 — Architecture consistency

- **Architecture map** — does the diff respect the `docs/architecture.md` (or equivalent) responsibility split? See `reference/architecture-map-pattern.md`.
- **Layer violations** — dependencies pointing the wrong way (e.g., domain importing infrastructure).
- **Boundary erosion** — public methods sneaking into private packages; circular dependencies.
- **Missing abstractions** — same logic implemented twice with minor variations.
- **Custom code where a library exists** — presumptive Critical when the diff reinvents primitives the ecosystem already solves (cryptography, encoding, standard-format parsers, wire codecs, retry/rate-limiting, ORMs, validators). Major for general utility code with a battle-tested equivalent. Three sub-checks:
  - **Already in tree** — if the project's lockfile already pulls in a library that exports the function being hand-rolled, the hand-rolled version is Critical regardless of LoC. Don't import one symbol and reinvent the others.
  - **Justification still valid** — comments that justified hand-rolling earlier ("avoid coupling", "keep dep tree small", "minimise binary size") must still hold for *this* diff. Once the dep is in the tree, the original reason has expired.
  - **What to grep for** — custom encoders for standard formats, raw wire-protocol bytes as constants, hand-rolled crypto primitives, hand-written auth-token verification, custom retry-with-backoff loops.
- **Pattern compliance** — clean architecture / DDD bounded contexts, only when the project documents a pattern.

### 6. Pass 4 — Acceptance / intent alignment

Does the diff actually solve the contract — linked GitHub issue, PR description, or active Spec Kit `specs/-/tasks.md` Block? Cover the three axes:

- **Drift** — diff implements something related but not the asked feature.
- **Partial** — diff covers some required behaviours but misses others.
- **Overreach** — diff includes changes the issue did not request.

See `agents/acceptance-auditor.md` for the full procedure.

### 7. Pass 5 — AI-Native-Coding Practices

Validates the diff and the surrounding project against the empirically-grounded rubric for working with AI coding agents. Eight rules, citation-grounded:

- **R1** — Comments WHY not WHAT (load-bearing for LLM coding; inaccurate comments harmful).
- **R2** — Durable agent context belongs in instruction files. At least one of AGENTS.md / CLAUDE.md / `.cursor/rules/` must exist at repo root; section structure is guidance, not a graded checklist.
- **R3** — Tests prefer real objects over mocks; mock only at I/O boundaries.
- **R4** — Architectural decisions need ADRs — **only for projects using Spec Kit** (detected via `specs/` + `plan.md`/`tasks.md` or `.specify/`). Non-Spec-Kit projects skip R4.
- **R5** — Code review and PR hygiene survive AI throughput (small commits, decomposed PRs).
- **R6** — Conversational interaction is iterative, not one-shot.
- **R7** — Minimize context: delete, don't tombstone. Every line in always-loaded files costs every turn — remove rather than mark removed.
- **R8** — Mechanical-rubric subset belongs in CI; bundled templates ship with the plugin.

The rubric, with citations, lives at `reference/ai-native-rubric.md`. The mechanical-check templates ship at `reference/ai-native-templates/`. See `agents/ai-native-reviewer.md` for the full procedure.

## Output

```
## Quality Gate Summary

| Review              | Verdict        | Critical | Major | Minor |
|---------------------|----------------|----------|-------|-------|
| Code                | pass/warn/fail | N        | N     | N     |
| Security            | pass/warn/fail | N        | N     | N     |
| Architecture        | pass/warn/fail | N        | N     | N     |
| Acceptance          | pass/warn/fail | N        | N     | N     |
| AI-Native Practices | pass/warn/fail | N        | N     | N     |

**Overall**: PASS / NEEDS WORK / FAIL

### Action items
1. 
```

For each individual finding:

- **Rule** — which rule was violated (with the language-rule skill or engineering-philosophy reference) or "best practice" if no codified rule.
- **Severity** — Critical / Major / Minor.
- **Location** — `file:line`.
- **Issue** — what's wrong and (for security) the attack vector.
- **Fix** — concrete suggestion, with a short code example when it clarifies the change.

## Behavioural traits

- Constructive, educational tone. Teach; don't just flag.
- Specific, actionable feedback. "This is too complex" without a fix is useless.
- Severity matches reality. Critical for "this could ship a bug or a CVE today"; Major for "this will hurt within six months"; Minor for style and polish.
- Practical over theoretical security risks. If an attack requires three impossible preconditions, mark Minor.
- Defence in depth. Multiple weak controls beat one perfect control.
- Read-only. This skill never edits the diff itself; it reports.

## Cross-references

- `running-tdd-cycles` — preceding workflow; review confirms TDD discipline.
- `committing-changes` — commit-message + branch hygiene checks fold into the code-quality pass.
- `python-conventions` / `go-conventions` / `solidity-conventions` — the language rule the diff is being checked against.
- `engineering-philosophy` — KISS, YAGNI, DRY, SOLID weights for code-quality and architecture passes.

## Reference

- [reference/owasp-checklist.md](reference/owasp-checklist.md) — canonical OWASP Top 10 mapping with attack vectors and fix patterns.
- [reference/architecture-map-pattern.md](reference/architecture-map-pattern.md) — the optional `docs/architecture.md` convention this skill expects.

The live subagent shims that wrap this skill for parallel execution live one level up:

- `agents/code-reviewer.md` — Pass 1 (code quality)
- `agents/security-auditor.md` — Pass 2 (security)
- `agents/architect-review.md` — Pass 3 (architecture)
- `agents/acceptance-auditor.md` — Pass 4 (acceptance / intent)
- `agents/ai-native-reviewer.md` — Pass 5 (AI-native-coding practices)

Each is a thin shim that reads this `SKILL.md` (and, for Pass 5, also `reference/ai-native-rubric.md`) and applies its scoped pass; the `/coding-skills:review` slash command spawns the five in parallel under `model: opus`.

## Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

- **Author:** [swell-agents](https://github.com/swell-agents)
- **Source:** [swell-agents/coding-skills](https://github.com/swell-agents/coding-skills)
- **License:** MIT

Install and usage instructions live in the source repository linked above.

## Pricing

- **Free** — Free

## Security capabilities

Automated source analysis of v0.1.0 — what this tool can access:

- **Network access:** no
- **Filesystem access:** no
- **Shell / process execution:** no
- **Environment & secrets:** no
- **Dynamic code execution:** no

*"Yes" means the capability is present in the source — more access means more to trust, not that it is unsafe.*


## Versions

- **0.1.0** — security scan: passed — Imported from the upstream source.

## Links

- Listing page: https://agentstack.voostack.com/l/skill-swell-agents-coding-skills-reviewing-changes
- Seller: https://agentstack.voostack.com/s/swell-agents
- Browse the marketplace: https://agentstack.voostack.com/browse

---
Listed on AgentStack — the marketplace for AI agent skills and MCP servers. Every listing is security-reviewed. Creators keep 70%.