/ship-check — Is this finished change actually safe to merge?

The code is written. Tests maybe pass. mr-review maybe found no bugs. And it can still be wrong — because it fixes the symptom instead of the cause, because it left half the problem unpatched, or because it breaks on the first null/empty/concurrent input the happy path never met. This is the last gate before merge: step back from the lines and ask whether the change, as a whole, correctly and completely solves the problem it was made for.

What this is NOT — so you invoke the right thing:

• Not validate-plan — that stress-tests a plan before code exists. This is its bookend: same adversarial spine, pointed at the finished diff before merge.
• Not mr-review / nitpick — those are line-level bug/quality sweeps ("is the code good?"). This is holistic ("does the change solve the right problem, completely?"). Clean, well-tested code that fixes the wrong thing passes mr-review and fails ship-check.
• Not run — that runs the app to observe behavior. This is static reasoning against the code, the schema, and real data. (They pair well — run both before a risky merge.)
• Not second-opinion — that's a head-to-head on one decision. This audits the whole change against its goal, and borrows second-opinion's rework-cost weighing for the approach pass.

The restraint gate — read first. The failure mode is manufacturing gaps: inventing missing cases that aren't real and edge cases that can't happen, to look thorough on a change that's genuinely done. Don't. A confident "✅ ship it — solid, complete, edges covered" is a first-class outcome. Only flag a gap you can name concretely (this caller, this row, this input). Three real findings beat ten filler ones.

Before the passes: re-anchor to the problem

You cannot judge whether a fix is complete without knowing exactly what it was supposed to do. Restate the original problem in one line — from the ticket, the root-cause diagnosis, the user's ask, the MR description. If you can't state it crisply, go read it before judging. Every pass below is measured against this line, not against "does the code look reasonable."

A limit worth knowing about this skill

You are judging code you probably wrote, so your judgment is contaminated — you will re-derive the reasoning that produced the bug and find it sound. On the record, /ship-check has cleared branches that /mr-review then found CRITICALs in, with pass 2's "did fixing X break Y?" present and followed.

The fix is not to spawn your own reviewer here — /mr-review runs five independent reviewers on the same diff minutes later, so doing it in this skill just pays twice for one answer. The fix is to know that a clean ship-check is not sufficient evidence to merge, and to route the risky case to review deliberately — see "When this runs after review fixes" below.

The four passes

Run in order. Each is skippable-with-a-sentence if it genuinely yields nothing — but you must have looked, against ground truth (code, schema, real data), not assumed.

First, the floor check (free, deterministic). Run node ~/agent-skills/scripts/floor-guard.mjs --base <merge target> from the repo. It lists the moves that lower the bar to get to green: .skip, a deleted test or assertion, a new suppression (@ts-ignore, eslint-disable, istanbul ignore), a stub that throws, an empty catch. Exit 1: each line is a finding unless the change justifies it. A reworded assertion shows as removed, so read the hunk before reporting it. Exit 2 means it could not run; say so, never read it as clean. (adapted from addyosmani/agent-skills constraint-driven-development floor guard, MIT)

1. Completeness — what's MISSING? (the headline pass)

The most dangerous bug in a finished fix is the part that isn't there. A missing branch throws no error and fails no test — it just silently doesn't happen. Hunt for it:

• Sibling callers / the shared source. If the fix patched one call site of a shared function, grep every caller — are the others still broken? The right fix is usually at the source; a per-caller patch leaves siblings bleeding.
• Unhandled branches. Every enum case, every status, every if without its else, every type in the union — does the fix cover all of them, or just the one in the report?
• Already-broken data. A forward fix stops new breakage; it doesn't heal rows already poisoned. Does this need a backfill / migration, not just a code change? (Especially for wrong balances, bad statuses, corrupt derived values.)
• The other side. Fixed the write but not the read? The API but not the consumer? The happy path but not the failure/rollback path? Backend but not the mobile/web client that also calls it?
• Other tenants / orgs. Does the fix assume org 1's shape? Multi-tenant blind spots hide here.
• Loose ends from the original ask. Re-read the problem line. Is every part of it addressed, or did one sub-requirement quietly get dropped?
• The fix you just wrote. If a pass produced a fix, that fix is new code that nothing has checked — the passes ran against the diff as it stood, and the verdict is about to be issued on code that didn't exist when they ran. Re-run this pass against the fix itself: name every layer and path the defect can reach, and confirm the fix covers each one. A defect that crosses layers (capture → serialization → transport → storage) patched at exactly one of them is the default failure, and it looks complete from inside the layer you patched. Measured 2026-09-16: a malformed-byte bug was correctly found and then fixed at the storage encoder, which left the queue path — where the value is serialized long before storage — still broken; /mr-review found it minutes later.

2. Where it breaks — edge cases the happy path ignores

Attack the finished code as an adversary who wants it to fail in production:

• Degenerate inputs — null, empty collection, zero, negative, huge, duplicate, first-run (no prior state), retried (already ran once).
• Concurrency & ordering — two requests at once, the row that changed between read and write, the job that runs before its dependency.
• Did fixing X break Y? — the regression the change introduces. What used to work that this now touches?
• Failure modes — external call times out, transaction rolls back, cache is stale, queue worker holds old code (needs restart?). Name the specific input and the specific line it breaks at — not "consider edge cases."

3. Correctness of approach — is this even the right fix?

Step back from "does it work" to "should it be done this way." Would a materially different approach be better — a different layer, reusing something already in the codebase, a simpler mechanism that covers the real requirement, a native feature it reinvents? But it's already built — so weigh the rework cost honestly (like second-opinion): "better in theory but not worth redoing" is a valid, useful call. Only raise this if a genuinely different approach exists with a real trade-off; otherwise one line: "approach is right because …" and move on.

3½. Persona step — UI diffs only

Static passes can't tell you a screen is confusing. If the diff touches what a user sees, a blind persona has to see it before the verdict.

• Detect: ~/.claude/skills/ship-check/scripts/ui-touched.sh [-C repo] <base>...HEAD. Exit 0 = UI touched (prints the files); exit 1 = not. UI means a non-test file that is a view/style/template (.vue .blade.php .jsx .tsx .svelte .html .css .scss .arb), sits under a strings or view folder (lang/ locales/ l10n/ i18n/ Pages/ Components/ screens/ widgets/ views/…), or is a .dart file defining a widget. The script is the rule; don't re-decide it by eye.
• Not UI → one line in the output, Persona: skipped — backend-only (ui-touched: no UI files), and move on.
• UI → run /persona-walkthrough diff on the screens those files render, before the verdict. If a walkthrough already ran on this diff before commit and no UI file changed since, reuse its report instead of running it again. If you are a subagent, don't run it nested (it spawns its own personas): return Persona: pending — UI touched (<files>) and let the lead run it; the verdict waits for it. A blocker from it makes the verdict 🔧 fix first. A major goes in the fix-first list unless the user accepts it in so many words. Minors are listed, not gating.
• UI → also the design audits. Personas are lay users on an errand: they reliably miss what only a designer voices — tiles of uneven height, a two-item nav bar that leads nowhere, contrast, screen-reader semantics, motion settings. So on the same changed screens run /ui-audit (technical: accessibility, responsive, theming, states) and /ux-audit (does the screen serve its one goal: hierarchy, friction, wasted affordances). A subagent may run these (they don't spawn agents). Their P1/High findings go in the fix-first list unless the user accepts them; P2/Med are listed; the rest are noted. Calibrated 2026-10-11 on the Esoq vendor app home screen: personas (6 runs) missed both the ragged tile rows and the 2-tab bottom nav; ui-audit flagged the first and ux-audit the second, unprompted.
• Screenshot gate — every user-visible UI diff: Check the PR/MR description or linked comment for rendered before and after images of each affected screen/state. Local files and private artifact links do not count. If the UI changed after capture, refresh the images. Missing, broken, or stale images make the verdict 🔧 fix first; keep the PR/MR draft until they are present. For a new screen, an accurately labeled image of the previous entry point or absence is the before state.
• Evidence table — when the change came from persona findings (the MR, commits or task cite a persona finding): the MR description must carry, per finding, one row: finding ID · persona (name + one-line who) · persona's words (verbatim FRICTION/FEEL line, quoted) · BEFORE screenshot (the persona's step-NNN.png) · AFTER screenshot (same screen, fixed build). Check it's there and the images render; a missing row or a missing AFTER shot is 🔧 fix first. It is what lets a reviewer judge the UX reason, not just the diff.

4. Verdict — a merge decision, no hedging

A regression test does not cover a finding until you have watched it fail. Before any verdict that leans on a test written for a finding here, revert the fix (git stash push -- <file>), run that test, confirm it goes red, restore. If it passes without the fix it is pinning nothing, and the finding is still open. Measured 2026-09-16: three tests written in one session passed against deliberately broken code — one was satisfied by a fallback row the failure path wrote, which met an assertion that named only the field under test. When a test surprises you by passing, find out which line it actually pins before believing it. See /prove-the-test.

"Edges hold" needs an edge you saw. When the change reads or parses something it doesn't produce — another program's files, logs, API payloads, real rows — and you never looked at a real one, the size and shape it assumes are unverified. Name the assumption (a byte limit, a field's presence, a format) and make it a fix-first item: check one real instance before merging. Measured 2026-10-22 on a replay: a ship-check said "✅ ship it, the edges hold" on a scan that read 8 KB of Codex rollouts whose first line runs ~22 KB; a fix had to follow 30 minutes later.

• ✅ Ship it — solves the real problem, complete, edges hold. Name the one thing to keep an eye on post-merge (there's always one).
• 🔧 Fix these first — fundamentally right, but these specific gaps must land before merge. List them, most-important first, each with where.
• 🛑 Reconsider — it fixes the wrong thing, leaves the core problem unsolved, or a materially better approach is worth the rework. State the single most important reason and the next step.

5. Does a human need to look? — decide it, so nobody reads a diff for nothing

After the verdict, make one review call for the human who merges. 🔧 and 🛑 go back to the author first; make the call on the ✅ that follows.

• 👀 Review — a human should read the parts named, and only those. Required if any of these hold:
  1. It touches money, tax, payments, auth, permissions, secrets, personal data, or tenant isolation.
  2. It can't be undone cleanly: a destructive or rewriting migration, a backfill on real rows, deletions, or external side effects (messages sent, webhooks, a published API).
  3. It changes a contract others consume: a public or cross-repo API, a shared schema, event shapes, config defaults.
  4. The agent made a product or UX call the approved issue or plan didn't: new behaviour, changed copy, a new default, a new screen.
  5. A gate didn't run or ran degraded, a finding was accepted instead of fixed, or a regression test couldn't be proven red.
  6. Review took ≥2 rounds of structural fixes, or this check found a gap the author missed.
  7. Project policy names a human reviewer for these paths (CODEOWNERS, contributing rules, recorded merge policy).
  8. You wouldn't bet on it. Low confidence is a reason on its own; say what it is.
• 👁 Glance — none of the above, but end users will see a difference that matches the approved issue. The before/after screenshots in /mr-guide are enough; about a minute.
• ✅ No review needed — none of the above, nothing user-visible changed, the change matches the approved issue, and every gate ran clean.

When unsure between two levels, pick the higher one. The call says what is worth a human's time; it never grants merge authority. Who may merge is unchanged.

When this runs after review fixes

Fixes written in response to a review are the highest-risk code on the branch: written fast, on the assumption the reviewer already framed the problem correctly, and landing in code the existing tests were written against. Split them by kind, because only one kind bites:

• Structural — deleted a branch, moved a decision across a lock/transaction/guard, changed who owns a lock, changed a constructor's visibility, collapsed two paths into one. Re-run /mr-review before merging. Every self-introduced CRITICAL on the record followed a change of this shape.
• Additive — added a test, tightened a validation, added a null guard, renamed for clarity. Merge on the tests. A fresh review round buys nothing here.

If a third round still finds something, stop iterating and split the MR. Three rounds of structural fixes means the change is too large to converge — each fix is landing in code the previous round just moved, and the next round will find the next one.

Output shape

Skimmable — this is a merge decision, not an essay.

## Ship-check: <the change, one line>

**Problem it must solve:** <the original ask, one line — everything below is judged against this>

**Missing / incomplete**
- <the caller/branch/row/tenant/backfill that's not covered> — <where> — <why it matters>
  (or: "Complete — checked callers X/Y, all enum branches, no backfill needed because …")

**Where it breaks**
- <specific input> → <specific line/behavior> — <fix or accept>
  (or: "Edges hold — null/empty/concurrent/retry all handled at …")

**Approach**
- <materially-better alternative + rework-cost call>  (or: "Right approach because …")

**Persona** — <skipped: backend-only | pending: lead runs it | N personas: blockers/majors with IDs | evidence table present/missing>

**Design audit** — <skipped: backend-only | ui-audit: score + P1s | ux-audit: High findings>  (UI diffs only)

**Verdict: ✅ ship / 🔧 fix first / 🛑 reconsider** — <the decisive reason + the one thing to watch>

**Human review: 👀 review <what, where> / 👁 glance at screenshots / ✅ not needed** — <the rule that triggered it, or why none did>

Guardrails

• Judge against the problem, not the code's looks. Re-anchor first; a change that "looks fine" but doesn't fully solve the stated problem fails here.
• Missing > present. The absent branch, the un-patched sibling, the skipped backfill — these fail no test and are the whole reason this skill exists. Spend the most effort in pass 1.
• Ground every finding. file:line, a query result, a real row, a named caller. No abstract worries — if you can't name where it breaks, it's not a finding.
• Grep the callers. A shared-source fix is only complete if every caller is checked. "The ticket named one" is not "there is one."
• Weigh rework honestly. For approach objections on built code, compare switching cost, not just abstract superiority.
• A clean ship-check is not sufficient evidence to merge. It has cleared branches that review then found CRITICALs in. Say "✅ ship it" when it's true, but don't present it as the last word on a change you wrote yourself.
• Don't spawn reviewers here. /mr-review runs five on the same diff minutes later; duplicating one costs ~55k tokens to re-find what it finds anyway.
• Restraint gate. "✅ ship it" is a real verdict. Never invent a missing case or a marginal alternative to justify running the skill. If it's done, say so and hand back the one thing to watch.
• Decide, don't survey. End on one merge verdict, not a neutral list of trade-offs.