--- name: reviewing-changes description: Run a layered quality gate over a code change — code quality, security audit, and architecture consistency, in that order. Use after writing or modifying code, before opening or merging a PR, when reviewing a diff or branch, or when asked for a code review, security audit, or architecture review. Produces severity-ranked findings (Critical / Major / Minor) tied to file:line, each with a concrete fix. Covers OWASP Top 10, SOLID, KISS / YAGNI / DRY, ruff + mypy for Python, golangci-lint for Go, solhint for Solidity, and common perf pitfalls (N+1, unbounded loops, leaks, missing indexes). For diffs over ~500 lines or 20 files, scopes the linter sweep to touched packages first and widens on demand. Read-only: never edits the diff, never runs unscoped Bash — every tool is a fixed command pattern (Bash(git diff *), Bash(uv run ruff *), Bash(golangci-lint *)). Composes with python-conventions, go-conventions, solidity-conventions, engineering-philosophy. allowed-tools: Read, Grep, Glob, Bash(git diff *), Bash(git log *), Bash(git show *), Bash(git status *), Bash(git rev-parse *), Bash(gh pr view *), Bash(gh pr diff *), Bash(gh pr list *), Bash(uv run ruff *), Bash(uv run mypy *), Bash(uv run pip-audit *), Bash(npm audit *), Bash(golangci-lint *), Bash(govulncheck *), Bash(gosec *), Bash(solhint *), Bash(forge fmt --check), WebSearch, WebFetch --- ## Process Always run the three 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. - **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** — flag reinvented validators, parsers, ORMs, retry logic, etc. - **Pattern compliance** — clean architecture / DDD bounded contexts, only when the project documents a pattern. ## 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 | **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/code-reviewer-agent.md](reference/code-reviewer-agent.md) — original Claude-Code code-reviewer agent verbatim (model: opus). - [reference/security-auditor-agent.md](reference/security-auditor-agent.md) — original Claude-Code security-auditor agent verbatim (model: opus). - [reference/architect-review-agent.md](reference/architect-review-agent.md) — original Claude-Code architect-review agent verbatim (model: opus). - [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.