Pre-Landing Review — Find What CI Misses
You are a staff engineer. Find the bugs that pass CI but blow up in production. Auto-fix the obvious ones. Flag the rest.
Related skills: plan-eng-review | ship | investigate
Step 0: Detect Base Branch
- Check for existing PR:
gh pr view --json baseRefName -q .baseRefName
- If no PR:
gh repo view --json defaultBranchRef -q .defaultBranchRef.name
- Fallback:
main
Step 1: Check Branch
git branch --show-current — if on base branch, nothing to review.git fetch origin --quiet && git diff origin/<base> --stat — if no diff, nothing to review.
Step 1.5: Scope Drift Detection
Before reviewing code quality, check: did they build what was requested?
- Read commit messages, PR description, and TODOS.md
- Compare files changed against stated intent
Scope Check: [CLEAN / DRIFT DETECTED / REQUIREMENTS MISSING]
Intent: <what was requested>
Delivered: <what the diff actually does>
This is informational — does not block the review.
Step 2: Get the Diff
git fetch origin <base> --quiet
git diff origin/<base>
Step 3: Two-Pass Review
Pass 1 (CRITICAL)
| Category |
What to look for |
| SQL & Data Safety |
Raw SQL injection, missing parameterization, unguarded DELETE/UPDATE |
| Race Conditions |
Concurrent access to shared state, read-modify-write without locks |
| LLM Trust Boundary |
LLM output used in SQL, eval, system commands, or rendered as HTML without sanitization |
| Enum Completeness |
New enum value added but not handled in all switch/case/if-else chains |
Pass 2 (INFORMATIONAL)
| Category |
What to look for |
| Conditional Side Effects |
Side effects (DB writes, API calls, emails) inside conditional branches that might not execute |
| Magic Numbers |
Hardcoded values that should be constants |
| Dead Code |
Unreachable code, unused imports, commented-out blocks |
| Test Gaps |
New code paths without corresponding tests |
| Performance |
N+1 queries, missing indexes, unbounded queries, large bundle imports |
Step 4: Fix-First Review
Every finding gets action — not just critical ones.
Classify each finding as AUTO-FIX or ASK
- AUTO-FIX: Mechanical fixes with one correct answer (unused imports, missing
await, obvious typos)
- ASK: Fixes requiring judgment (architecture changes, behavior changes, security decisions)
Apply AUTO-FIX items directly
For each: [AUTO-FIXED] [file:line] Problem → what you did
Batch-ask about ASK items
Present in one question:
I auto-fixed N issues. M need your input:
1. [CRITICAL] file:line — Race condition in status transition
Fix: Add WHERE clause to UPDATE
→ A) Fix B) Skip
2. [INFO] file:line — LLM output not validated before DB write
Fix: Add schema validation
→ A) Fix B) Skip
Verification of Claims
- If you claim "this is handled elsewhere" → read and cite the handling code
- If you claim "tests cover this" → name the test file and method
- Never say "likely handled" or "probably tested" — verify or flag as unknown
Step 5: Documentation Staleness Check
For each .md file in the repo root — if code changes affect described features but the doc wasn't updated, flag as informational.
Step 6: Output
Pre-Landing Review: N issues (X critical, Y informational)
Auto-fixed: [list]
Needs input: [list]
Scope: [CLEAN / DRIFT / MISSING]
After review, proceed to ship to create the PR.
Important Rules
- Read the FULL diff before commenting. Don't flag issues already addressed.
- Fix-first, not read-only. AUTO-FIX items are applied directly.
- Be terse. One line problem, one line fix.
- Only flag real problems. Skip anything that's fine.
- Never commit, push, or create PRs — that's ship's job.
1---2name: review3description: Pre-landing PR review. Analyzes diff for SQL safety, race conditions, LLM trust boundary violations, conditional side effects, and structural issues. Auto-fixes obvious issues, asks about complex ones. Use when reviewing code before merge, or when asked for "code review".4---56# Pre-Landing Review — Find What CI Misses78You are a staff engineer. Find the bugs that pass CI but blow up in production. Auto-fix the obvious ones. Flag the rest.910**Related skills:** [plan-eng-review](../plan-eng-review/SKILL.md) | [ship](../ship/SKILL.md) | [investigate](../investigate/SKILL.md)1112---1314## Step 0: Detect Base Branch15161. Check for existing PR: `gh pr view --json baseRefName -q .baseRefName`172. If no PR: `gh repo view --json defaultBranchRef -q .defaultBranchRef.name`183. Fallback: `main`1920---2122## Step 1: Check Branch23241. `git branch --show-current` — if on base branch, nothing to review.252. `git fetch origin --quiet && git diff origin/<base> --stat` — if no diff, nothing to review.2627---2829## Step 1.5: Scope Drift Detection3031Before reviewing code quality, check: **did they build what was requested?**32331. Read commit messages, PR description, and TODOS.md342. Compare files changed against stated intent3536```37Scope Check: [CLEAN / DRIFT DETECTED / REQUIREMENTS MISSING]38Intent: <what was requested>39Delivered: <what the diff actually does>40```4142This is informational — does not block the review.4344---4546## Step 2: Get the Diff4748```bash49git fetch origin <base> --quiet50git diff origin/<base>51```5253---5455## Step 3: Two-Pass Review5657### Pass 1 (CRITICAL)5859| Category | What to look for |60|----------|-----------------|61| **SQL & Data Safety** | Raw SQL injection, missing parameterization, unguarded DELETE/UPDATE |62| **Race Conditions** | Concurrent access to shared state, read-modify-write without locks |63| **LLM Trust Boundary** | LLM output used in SQL, eval, system commands, or rendered as HTML without sanitization |64| **Enum Completeness** | New enum value added but not handled in all switch/case/if-else chains |6566### Pass 2 (INFORMATIONAL)6768| Category | What to look for |69|----------|-----------------|70| **Conditional Side Effects** | Side effects (DB writes, API calls, emails) inside conditional branches that might not execute |71| **Magic Numbers** | Hardcoded values that should be constants |72| **Dead Code** | Unreachable code, unused imports, commented-out blocks |73| **Test Gaps** | New code paths without corresponding tests |74| **Performance** | N+1 queries, missing indexes, unbounded queries, large bundle imports |7576---7778## Step 4: Fix-First Review7980**Every finding gets action — not just critical ones.**8182### Classify each finding as AUTO-FIX or ASK8384- **AUTO-FIX**: Mechanical fixes with one correct answer (unused imports, missing `await`, obvious typos)85- **ASK**: Fixes requiring judgment (architecture changes, behavior changes, security decisions)8687### Apply AUTO-FIX items directly8889For each: `[AUTO-FIXED] [file:line] Problem → what you did`9091### Batch-ask about ASK items9293Present in one question:9495```96I auto-fixed N issues. M need your input:97981. [CRITICAL] file:line — Race condition in status transition99 Fix: Add WHERE clause to UPDATE100 → A) Fix B) Skip1011022. [INFO] file:line — LLM output not validated before DB write103 Fix: Add schema validation104 → A) Fix B) Skip105```106107### Verification of Claims108109- If you claim "this is handled elsewhere" → read and cite the handling code110- If you claim "tests cover this" → name the test file and method111- Never say "likely handled" or "probably tested" — verify or flag as unknown112113---114115## Step 5: Documentation Staleness Check116117For each `.md` file in the repo root — if code changes affect described features but the doc wasn't updated, flag as informational.118119---120121## Step 6: Output122123```124Pre-Landing Review: N issues (X critical, Y informational)125Auto-fixed: [list]126Needs input: [list]127Scope: [CLEAN / DRIFT / MISSING]128```129130After review, proceed to [ship](../ship/SKILL.md) to create the PR.131132---133134## Important Rules135136- **Read the FULL diff before commenting.** Don't flag issues already addressed.137- **Fix-first, not read-only.** AUTO-FIX items are applied directly.138- **Be terse.** One line problem, one line fix.139- **Only flag real problems.** Skip anything that's fine.140- Never commit, push, or create PRs — that's [ship](../ship/SKILL.md)'s job.
aspectrr