Skills · dev-team

`pre-merge-review`

Gate an existing change for merge — run one skill that fires the whole production-readiness pass. First a deterministic mechanical gate (lint + typecheck + tests, via a script whose exit code decides), then the three-reviewer panel dispatched in parallel — devils-advocate (correctness), security-reviewer (exploitability), code-quality (craft) — then a single READY / NOT READY verdict. Use before opening a PR or merging, when you want the full review fleet run on a diff you already have. Do NOT use to implement a feature from a spec (that's feature-implement-loop) or to review a document (that's doc-critique).

Pack dev-teamTier standardJob Build and review softwarev0.1.5

npx skilldrop-cli install pre-merge-review

Other ways to install

With the sibling skills it hands off to:

npx skilldrop-cli install pre-merge-review --with-related

The whole pack it belongs to:

npx skilldrop-cli install --pack dev-team

By hand, from a clone of the repo:

cp -R packs/dev-team/skills/pre-merge-review ~/.claude/skills/

In Claude Code, with the dev-team plugin installed, invoke it as:

/dev-team:pre-merge-review

Try it

A realistic prompt from the skill's acceptance evals:

Run a pre-merge review on my branch that adds a /export endpoint. The project is Python; `pytest` currently fails on one new test, and the endpoint reads user_id from the query string.

What a good result looks like

The mechanical gate is run, not assumed. A verdict with no real gate result is invalid — you ran the script (or reported BLOCKED), you didn't eyeball the tests.
The gate is un-bypassable. A RED gate is NOT READY regardless of how good the review looks. The review can add blockers; it can never clear a failing gate.
Findings are merged and de-duped across the three lenses — the same line is not reported three times.
The verdict is one word plus the blocking reasons, not a hedged paragraph. A reader sees READY/NOT READY and exactly what stands in the way.
Design disagreements go to council-review, not adjudicated here — this skill gates a change, it doesn't decide an architecture.

Example output

a NOT READY verdict where the mechanical gate is decisive · the skill's own worked example, source

Input

"Review my branch before I open the PR — it adds a /export endpoint that streams a user's records as CSV."

The change: a new export_handler + one new test.

Output

Verdict: NOT READY — the mechanical gate is RED, and the panel adds one blocker.

Mechanical gate (`scripts/gate.py`, auto-detected)

Command	Result
`ruff check .`	PASS
`mypy .`	PASS
`pytest -q`	FAIL — `test_export_empty` errors: `export_handler` raises `TypeError` on a user with zero records

GATE: RED — 1/3 command(s) failed. A red gate is NOT READY on its own; the review below can only add blockers, never clear this.

Review panel (merged, de-duped)

🟥 blocker — missing authorization (security-reviewer). export_handler reads user_id from the query string (handler.py:12) and returns that user's records with no check that the caller is that user — any authenticated user can export anyone's data (IDOR). Fix: derive user_id from the session, not the request; if an admin export is intended, gate it behind an explicit role check.

🟧 major — unbounded response (devils-advocate). The CSV is built in memory for the full record set (handler.py:20); a user with millions of rows OOMs the process. Fix: stream rows, or paginate/cap.

🟨 minor — craft (code-quality): export_handler mixes fetch, format, and HTTP concerns in one 60-line function (handler.py:8-68); extract rows_to_csv(). (listed, does not block.)

Must-fix before merge

Gate: fix export_handler on the empty-records case so pytest is green.
Blocker: derive user_id from the session — close the IDOR.
Major: stream/cap the CSV to bound memory.

Why this passes the quality bar

The gate was run, not assumed, and its RED result decides the verdict regardless of the review; findings are merged across the three lenses (security, correctness, craft) with no triple-reporting; the verdict is one word plus an ordered must-fix list — gate failures first, then blocker/major. A no-victim security note wasn't inflated to a blocker; the IDOR — with a concrete attacker path — was.

How it's checked

Measured against plain Claude Code (2026-10-23): it met 44% of its checks, against 43% without it, and a blind judge preferred the agent without it. How skills are checked

23 assertions across 4 acceptance evals, and 9 trigger queries. Every skill's checks

What the output must do

Runs the mechanical gate (scripts/gate.py or the project's lint/typecheck/test commands) before or alongside the review — does not skip it
Treats the failing pytest as a RED gate that forces NOT READY on its own, independent of the review findings
Dispatches the three-reviewer panel (devils-advocate, security-reviewer, code-quality) or sweeps the three lenses inline
Flags the query-string user_id as an authorization/IDOR blocker with a concrete attacker path and a fix
Ends with a one-word verdict (NOT READY) plus an ordered must-fix list, gate failures first
Does not report the same file:line three times — findings are merged across lenses
Runs the gate and reports it GREEN before rendering a positive verdict
Only returns READY when the gate is green AND there is no blocker and no major finding
Lists any minor/nit findings without letting them block the verdict
Concludes that the change is not ready to merge as it stands
Points out that the test run passes only because the diff newly skips test_lb_cap, and that this test is deterministic and fails once the skip is removed (the 'all tests pass' claim does not hold, and CONTRIBUTING.md forbids new skips)
Identifies that doses entered in pounds are no longer capped at the drug's max_dose_mg (for example 176.3696 lb of fernoxicam now gives 320 mg instead of 200 mg), which contradicts the 'refactor only, no behaviour change' description
Flags that the new debug log writes client names and phone numbers to a plain file on disk, against the CONTRIBUTING.md rule, and treats it as something to remove or fix before merge
Leaves the project's source and test files unmodified
Does not report lint, type-check or test results it did not actually obtain, and does not cite findings in files or functions that are not in the folder
The write-up fits about a page (roughly 600 words or fewer)
Gives a clear answer that the change should not be merged as it is
Explains that dividing by a fixed 3 gives the wrong blank value on plates with 2 blank wells (the blank comes out a third too low), so those readings are wrong
Notes that with no blank wells the new line silently subtracts zero instead of failing, so the readings go uncorrected without any warning, and suggests a fix that divides by the real number of blanks and handles the empty case explicitly
Does not claim that tests or other automated checks were run and passed; nothing beyond the diff was supplied
Uses plain language a non-programmer can follow, without unexplained jargon
Keeps to a yes/no answer plus a few short bullets, not a long report
Gives the verdict without first asking for more files or context

When it should and shouldn't activate

Should activate:

is this change ready to merge?
run all the reviewers on my branch before I open the PR
production-readiness review of this diff
gate my PR — lint, tests, and a full review
review this branch and tell me if it's safe to ship

Should leave to another skill:

implement this feature from the ticket's acceptance criteria
just find the bugs in this function
critique this design doc
should we use Postgres or DynamoDB for this?

Hands off to

council-review — when the blocker is a design disagreement, not a defect. Adjudicate the disagreement outside the diff.

Source

SKILL.md · the whole folder on GitHub.

pre-merge-review