code-review

Files (5)

• SKILL.md
• codex-review.md
• live-verification.md · references/
• refactor.md · references/
• adversarial-review.js · workflows/

SKILL.md (first lines)

# Code Review

This skill **reviews and reports** - it never implements fixes. If a fix is obvious and one-line, mention it in the comment as a suggestion; hand the actual work back to `vd:cook` / `vd:fix`. Landing the branch afterwards is `vd:ship`.

> **Codex runtime:** this file uses Claude-Code tooling (`AskUserQuestion`, `Task(Explore)` subagents). Under `codex exec` use the self-contained `codex-review.md` in this skill dir instead; per-site fallbacks below also apply.

## Modes

Auto-detect from arguments. Ambiguous or empty → `AskUserQuestion` (Claude Code; plain-text question elsewhere).

| Argument | Mode | Source of diff |
|---|---|---|
| `#123`, `123`, or GitHub PR URL | **PR** *(default & polished path)* | `gh pr diff` |
| 7+ hex chars (`abc1234`) | **Commit** | `git show <sha>` |
| `--pending` | **Pending** | `git diff` (staged + unstaged) |
| `codebase` | **Codebase** | Broad scan via subagents |
| *(none, recent changes in context)* | **Recent** | Whatever was just edited |

Flags:
- `--dry-run` - print the review payload, do NOT post to GitHub (PR mode only)
- `--post` - opposite default; force posting even if other flags would skip
- `--no-inline` - skip inline comments, post only the top-level summary
- `--auto` - non-interactive, default answers, no prompts
- `--ultra` - adversarial review via a dynamic workflow: every finding is independently refuted before it ships (see [Ultra mode](#ultra-mode--adversarial-workflow)). Higher token cost; use for high-stakes diffs.
- `--cross-model` - add reviewers from other model families and weigh findings by agreement (see [Cross-model mode](#cross-model-mode)). Combine with `--ultra` only for release-critical diffs.
- `--refactor` - local reuse / composition / slop lens. Never posts. See [`references/refactor.md`](references/refactor.md). Combine with `--fix` / `--save`. Detect from "refactor review", "is this slop", "does this fit the codebase".

`--refactor` is a two-axis pass when paired with a landing review: run the reuse/slop lens locally first, then the default PR pass. Do not mix the two outputs in one GitHub review.

## Hard rules

1. **One review = one push to GitHub.** Don't dribble out comments across multiple `gh` calls. Build the full payload, then submit a single `POST /pulls/:n/reviews`. The author gets one notification, not 12.
2. **Inline beats top-level.** If a finding maps to a specific file:line, it MUST go inline. Top-level summary is for context, verdict, and orphan concerns that don't anchor to a hunk.
3. **No critical claim without evidence.** "Critical" means "merge would cause incident / data loss / security hole." If you can't point to the failure mode, downgrade to Important.
4. **Encouraging tone, firm content.** Lead with what works when it's true. When the user asks you to list the clean angles, name each angle that is clean. Be direct about problems. Never sarcastic. Never "as an AI…". The author is a peer.
5. **Read the file, not just the hunk.** Diff context lies. Open the file at the changed lines and 30 lines around to understand surrounding state before commenting.
6. **Intent vs mistake.** Review whether the change achieves the stated intent, not whether the intent is right. A finding that challenges a deliberate product decision, a named tradeoff, or two valid designs is `**Question:**`, never Critical/Important, and must not flip the verdict to Request changes. Relay it; let the author/user answer.
7. **Stay in the diff.** Comment on paths this change touches. Adjacent cleanup is out of scope unless this diff introduces a merge-blocking defect there.

## Review voice (the style guide)

These are the conventions for **every** comment this skill writes. Reference example: `<org>/<repo>#124`.

### Severity prefix on every finding

| Prefix | When | Notification cost |
|---|---|---|
| `**Critical:**` | Blocks merge. Bug, security, data loss, CI red. | High - author must act before next push. |
| `**Important - <topic>**:` | Should fix before merge. Correctness, design, perf. The `<topic>` makes the comment skimmable. | Medium. |
| `**Suggestion:**` | Nice-to-have. Style, minor refactor, doc nit. | Low - author can ignore. |
| `**Question:**` | You genuinely don't know, or the finding challenges a product/intent choice the author may have made on purpose. Asks; never blocks merge by itself. | Low - invites dialog, not blocking. |
| `**Nit:**` | Pure preference. Optional. | Zero - must be ignorable. |

### Comment shape

```
**<Severity prefix>**: <one-sentence problem statement>.

<1-3 sentences of evidence: what breaks, when, why it matters. Include the
actual symptom - not just "may cause issues">.

<Optional: suggested change, in fenced block or inline code>.

<Optional: question to author if the fix path is unclear>.
```

**Do:**
- Open with the problem, not the praise. (Praise goes in the summary.)
- Quote the offending fragment in fenced code when correcting a specific value.
- Name the failure mode explicitly: *"alert silently won't fire"*, *"PII leaks to logs on retry"*, *"locks the table for the duration of the migration"*.
- Suggest a concrete alternative when you have one: `CURRENT_DATE - 1` not "use the right date".
- Cross-link related comments when the issues compound: *"combined with the threshold on L81…"*.

**Don't:**
- Don't pad with hedges. "It might possibly be a small concern that perhaps…" → "This will fail when X."
- Don't restate the diff. The author already wrote the code.
- Don't moralize ("you should always…"). State the consequence: "this breaks X."
- Don't use emojis or icons (severity badges/PNGs) unless the repo's review conventions already use them. Plain `**Severity:**` ships everywhere.
- Don't comment on every line. If a file has 5 issues, the worst 2 are usually enough. The author will find the rest when they fix those.

### Top-level summary shape

Keep under 8 lines. Structure:

```
<One sentence on what the PR does + what works>.

<If CI is failing: Critical: CI <job> is red (link/run-id). Block merge until fixed:
exact command to debug>.

<One line pointing at inline comments>: "Inline comments cover the
<correctness / design / perf> concerns."

<Verdict - see below>.
```

**Verdict line** - exactly one of:
- `Approve` - no Critical, no Important. Suggestions OK.
- `Request changes` - at least one Critical or Important. List the topics.
- `Comment` - Only Suggestions / Questions / Nits. Author decides.

Tone: encouraging when earned (don't fake-praise), direct on real problems (no "maybe consider perhaps"), curious on unknowns, never condescending ("obviously" / "clearly" / "I'm surprised this passed").

## PR mode - the polished path

### 1. Fetch context

```bash
PR=$1   # accepts "#123", "123", or full URL - normalize to number
gh pr view  "$PR" --json title,body,author,baseRefName,headRefName,files,additions,deletions,state,reviewDecision,headRefOid
gh pr diff  "$PR"
# Exit 8 = pending (retry), not "no checks". Print the table; ignore the rc.
gh pr checks "$PR" || true
```

Capture `headRefOid` - every inline comment's `commit_id` MUST equal this so comments stay anchored if the author force-pushes mid-review.

### 2. Decide the scope of effort

From vanducng/skills, MIT.

More from vanducng/skills