alamops skills

---
name: code-review
description: (alamops) Performs a thorough, read-only code review of any change source — a pull request, a git diff, uncommitted edits, recent commits, or code just produced in the conversation — and re-reviews a fix round, scoring each earlier finding fixed / partial / not fixed. Flags bugs; security and trust-boundary gaps (tenant isolation, authz, approvals bound to the wrong version, untrusted input driving privileged state, atomicity, retry safety, RLS self-recursion); reliability under the real deployment (two instances, crashes mid-step, network calls inside transactions, unbounded retries); code that is written and tested but never wired into production; PR-body and doc claims the code doesn't back; tests that cannot fail; performance, config and schema drift; and blast-radius gaps — checked against the repo's own rules and specs. Returns findings with severity, file:line, a failure scenario and a fix; never edits code. Use for any code review, PR review, re-review, diff review, or critique of recent changes.
---

# Code review

A read-only review skill: produce findings the author can act on without reopening the code, and never edit code. Tone: a direct, friendly senior teammate — plain words, concrete scenarios, firm where you have evidence, a question where you don't.

## Where the expensive defects hide

The defects that cost the most in a finished change are rarely typos in the changed lines. They are:

- **Code that is written but not wired.** Exported, unit-tested, listed as done in the PR body — and called by nothing in production. Sometimes it's the feature's main output that never gets produced.
- **A guard on one path and not its sibling.** The main path checks authorization; the batch action, the retry, the fallback job, or the path a fix just added does not.
- **Behavior that holds with one process.** Two instances, a rolling deploy, a crash between two steps, or a dependency outage breaks it.
- **A test whose name promises more than it checks.** It passes with the bug still in, because it asserts something that was always true, or it silently returns when its database is missing.
- **A mistake the repo already wrote down.** A lessons-learned entry, a checklist, an ADR or a tech-debt item that the change repeats.
- **Claims that drifted from the code.** The PR body, a generated report, the plan's ledger or a commit message describes work that isn't there.

The diff alone shows none of these. Each one needs you to look past the changed lines, at callers, entry points, the deployment shape and the repo's own docs. Spend most of the review's effort there.

## Rules

1. **Read-only.** Even if an editing tool is available, do not modify code. Fixes, refactors and tests are findings, not commits.
2. **Every finding is concrete.** State the mechanism, a **scenario** (who does what, in which state, and what goes wrong), and the fix. If you can't build a plausible scenario, write it as a question instead.
3. **Verify before you report.** Re-check every blocker and must-fix yourself at the reviewed head by reading the code, running a grep, or tracing the call. A sub-agent's finding sounds equally confident whether it's right or wrong, and a wrong blocker costs the author a day.
4. **Complete the entire review.** Walk every changed file. Don't stop after the first few hits.
5. **Promise discipline.** Don't defer work you can do now. If you spot something, write it down as a finding immediately. Don't say "I'll come back to this."
6. **Explain the why.** Every finding states what is wrong and why it matters. No bare assertions.
7. **No positive findings.** Findings are for problems. Don't emit "looks good" or "nice touch" items. A clean area gets a one-line mention in the summary. One exception: on a large review, a short **Keep** list is allowed (see *Report*). Each Keep item names a specific pattern, with file:line, that the fix round must not break. It qualifies because the fixer can act on it. Ask: "Is there an action the author should take?"
8. **Say what you didn't do.** Note it if CI wasn't re-run, the spec wasn't found, migrations weren't applied, or the plan/apply step wasn't run. Put this in the header so the reader knows what the verdict rests on.

## 1. Pick and pin the review target

| Trigger | How to gather the diff |
| --- | --- |
| User names a PR (URL or number) | `gh pr view <n> --json title,body,headRefOid,baseRefName,commits,files` and `gh pr diff <n>`; read the review comments too (`gh api repos/{owner}/{repo}/pulls/<n>/comments`, `gh pr view <n> --comments`) since an earlier review changes the job (see §4). |
| "Review this branch" / "my changes" / "what I'm about to push" | `git status`, `git diff <base>...HEAD`, `git log <base>..HEAD --stat`. Default `<base>` to `origin/main` (or `origin/master`); fall back to `main`. |
| Uncommitted / pending / staged work | `git status`, `git diff` and `git diff --cached`. |
| "Recent commits" / "the last commit" | `git log -n <N> --stat`, `git show <sha>` for each. |
| Code we just wrote or discussed in this conversation | The files edited this session — re-read them from disk; don't trust your memory of the patch. |
| "Re-review" / "check the fixes" / a prior review is pasted or exists on the PR | Re-review mode — §4. |
| Ambiguous ("review my code") | Ask once: "The open PR, your uncommitted changes, the diff against `main`, recent commits, or the code we just wrote?" Then proceed. |