verify-pr

name	verify-pr
description	Comprehensive PR readiness check before merge. Run quality checks, tests, CI, documentation, AWS resource cleanup, and code review.
argument-hint	[PR-number]

PR Readiness Verification

Heavy pre-merge gate. Run this before creating or merging a pull request — NOT before every commit. Per-commit verification is handled by /check (enforced by a PreToolUse hook that blocks git commit without a fresh marker).

Checklist

Run each check and report pass/fail:

0. Worktree pre-flight: confirm node_modules/ exists in the cwd:

   [ -d node_modules ] || pnpm install
   

   git worktree add does NOT copy node_modules, so a fresh worktree's vp run typecheck / lint / build and vp run test all fail with tsc: command not found / Cannot find package 'vitest' etc. — but the failure is easy to miss when the output is piped to tail (the exit code reflects tail, not vp, and the failure line gets buried). If the pre-flight skips by way of an existing node_modules, confirm it is not stale by spot-checking pnpm-lock.yaml mtime ≤ node_modules/.modules.yaml mtime. Do not start step 1 until this passes, or every quality check below silently no-ops while looking green.

1. Code quality

   • vp run typecheck passes
   • vp run lint passes (run lint:fix first if needed)
   • vp run build succeeds
   • When piping any of the above to tail / head / grep for log truncation, check the actual output content for Error / Command failed markers — $? after a pipeline reflects the LAST stage (usually 0), NOT the build tool's exit. The same applies to background-task completion notifications: the framework's exit code 0 is the chained command's exit, not the pipeline head. When in doubt, capture the result without piping: vp run X > /tmp/out 2>&1; rc=$?; tail -3 /tmp/out; echo "[rc=$rc]".

2. Tests

   • vp test run - all unit tests pass. Preferred over vp run test because nothing sits between the caller and the verdict; the two now print 617 and 651 bytes for the same 17,497 tests (measured 2026-08-31). /check step 4 carries the full rationale, including the 171 KB reporter and the exit-0-having-run-nothing path that the cache: false change removed.
   • Every scope / diff check in this skill uses origin/main...HEAD, not main...HEAD. The gate hooks derive their scope from origin/main and the integ-destroy digest is pinned to merge-base(origin/main, HEAD), so a local main that has not been fetched makes this skill and the hook that blocks the merge disagree about what the branch touched.
   • Report test count (files and tests)
   • Test coverage check: compare git diff origin/main...HEAD for src/ changes vs tests/ changes. If new logic was added or modified in src/ but no corresponding test files were added or updated, flag as fail and add the missing tests before proceeding

3. CI status

   • If PR number is not provided as argument, auto-detect via gh pr view --json number -q .number
   • If no PR exists for current branch, use the AskUserQuestion tool to ask for the PR number
   • FIRST: gh pr view <PR> --json mergeStateStatus,mergeable -q '"mergeable=\(.mergeable) state=\(.mergeStateStatus)"' — when this returns mergeable=CONFLICTING state=DIRTY, the CI workflow will NEVER fire on the PR no matter how long you wait (empirically observed PR #404 — wasted ~70 min thinking GitHub Actions was broken). Close+reopen, empty commits, and git push --force-with-lease of unchanged content all fail to re-trigger. Resolution: git fetch origin main && git rebase origin/main, resolve conflicts, git push --force-with-lease — CI fires within ~30s of the push. See memory feedback_pr_conflict_blocks_ci.md for the full diagnostic checklist.
   • Only after mergeStateStatus is CLEAN / UNSTABLE / BLOCKED / BEHIND: gh pr checks <PR-number> - all checks pass
   • If checks are pending, wait and recheck

4. Working tree

   • git status - clean (no uncommitted changes)
   • Branch is up to date with remote

5. Documentation consistency

   • Invoke /check-docs skill logic: verify docs match code changes

   • Check for stale references to removed code

   • Generated-artifact freshness: CI carries a staleness guard per generated artifact — it runs the generator and fails if the working tree changes. There are NINE of them, and this step used to name only four, so the list drifted every time one was added. Do NOT re-list them here; regenerate everything in one shot:

     # Regenerates every artifact CI guards (offline static analysis, seconds
     # each). Unconditional on purpose: the old per-matrix `git diff` triggers
     # were themselves the drift, since each new matrix needed a new trigger.
     vp run gen:all-matrices
     
     # `--check` is offline (~0.5s) and verifies the cached
     # docs/_generated/provider-coverage.json matches register-providers.ts
     # under the current Tier classification. It is a CRITIC, not a generator,
     # so it is not part of the aggregate. If it fails, run
     # `vp run audit:coverage:regenerate` (heavy: ~15 min, needs AWS creds with
     # cloudformation:ListTypes + DescribeType) and commit the regenerated
     # cache. /verify-pr does NOT auto-run :regenerate — it needs AWS
     # credentials this skill cannot assume are present, so it is gated on the
     # critic FAILING rather than skipped for speed.
     vp run audit:coverage:check
     
     # Anything dirty here was stale before you ran the above.
     git status --short docs/ src/provisioning/property-coverage.generated.ts \
                        src/provisioning/unsupported-types.generated.ts
     

     If git status reports anything dirty, the contributor forgot to regenerate after their code change. Stage it, add it to the PR, and re-run /check-docs to refresh the docs marker.

     tests/unit/scripts/matrix-regen-coverage.test.ts pins gen:all-matrices against the guards actually present in .github/workflows/ci.yml in BOTH directions, so a new CI guard cannot be added without landing in the aggregate, and a removed one cannot linger. That test is what makes this step non-drifting; keep pointing at the aggregate rather than re-listing.

     History, because the same round-trip has now happened four times: PR #548 hit two matrices in succession; PR #1104 hit cli-flag-coverage (added to CI by #1072 without updating this step); PR #1231 merged a flag-only diff that staled cli-flag-coverage with no fixture change and turned main's check-build-test red until #1232; and PR #1416 hit handled-property-wiring (added by #1414), which is staled by an ordinary private-method rename inside a provider — a refactor nobody associates with a generated file (issue #1417). The PreToolUse hook blocks when a new is added without integ coverage (literal type id, L1 class, or carve-out) — but it does not enforce that the matrix snapshots themselves are regenerated. This step closes that gap.

6. Leftover resources

   • Resolve account ID via aws sts get-caller-identity --query Account --output text

   • aws s3 ls s3://cdkd-state-{accountId}-us-east-1/stacks/ --region us-east-1 — no leftover state

   • For deletion-touching PRs (any change under src/provisioning/providers/**, src/cli/commands/destroy.ts, src/analyzer/dag-builder.ts, IMPLICIT_DELETE_DEPENDENCIES, etc.): the integ-destroy markgate gate physically blocks gh pr merge when its marker is stale (see .claude/hooks/integ-destroy-gate.sh). This step verifies the gate state explicitly so failures surface here rather than at merge time:

     mise exec -- markgate verify integ-destroy
     

     Read the exit code, do not just test for non-zero. The gate runs markgate 0.4's hash: diff mode, which has three outcomes, and two of them have opposite remedies:

     • exit 1 — the marker is genuinely stale (this branch changed in-scope code, or the 14d TTL expired). Run /run-integ <relevant-test> (e.g. bench-cdk-sample) and confirm it reports 0 errors / 0 orphans; the skill itself then calls markgate set integ-destroy.
     • exit 2 — markgate could not EVALUATE the gate: origin/main unresolvable (never fetched, shallow clone) or no delta against the merge base. /run-integ cannot fix this — markgate set fails on the identical condition, so running one burns a real-AWS run and leaves the gate blocked anyway. The remedy is git fetch origin (or --unshallow, or committing the branch's work). markgate status also errors here and prints no state: line, so the usual staleness reason comes back empty.

     CI is necessary but not sufficient — it does not exercise real-AWS destroy. The gate is the structural enforcement of that fact.

   • CROSS-CUTTING CHECK (load-bearing): the integ-destroy marker accepts ANY clean real-AWS destroy. A narrow feature-specific integ (e.g. import-value-strong-ref's 2-stack S3+SSM fixture) IS sufficient to flip the marker, but it does NOT exercise the broad deploy / destroy code paths a cross-cutting change touches. When the PR diff touches ANY of:

7. No stale references

   • Grep for removed imports, old module names, or deprecated references in source files
   • Check src/index.ts exports are consistent

8. Code review

   • First, run /review-pr <N> to get a size-appropriate review plan. The skill outputs one of:

     • inline spot-check (small PR, < 300 LOC OR < 5 files, no security-sensitive paths) — read the diff yourself in this step; no sub-agent dispatch.
     • 1 reviewer (medium PR, 300-1000 LOC) — dispatch a single pr-code-reviewer agent (the skill emits a ready-to-paste Agent call).
     • 3-axis parallel (large PR ≥ 1000 LOC OR security-sensitive paths) — dispatch all three of pr-spec-reviewer / pr-code-reviewer / pr-test-reviewer in parallel (single message, three Agent tool calls).
     • security add-on (additive, ANY tier incl. inline) — when the PR touches a security / process-launch surface or is a security fix, /review-pr ALSO emits a pr-security-reviewer dispatch. Add it to the same parallel batch; its blockers block the merge like any other reviewer's.

     The skill applies bias factors (security surfaces bump up; pure-infra / docs / tests-only bump down) and appends the security reviewer on top of the tier when a security surface / fix is involved. Trust the recommendation; override only when you have a concrete reason (note the reason here).

   • Synthesize the reviewer reports (or your inline read) into a pass / issues-found verdict. Any blocker → fix-back loop before continuing.

   • Then re-review the FIX DELTA, not just re-run the tier heuristic. The fixes are code no reviewer has seen, written under the momentum of agreeing with a finding, and they land in the exact spot a reviewer just proved is subtle. Dispatch a second round scoped to what changed since the first, telling the reviewers the original design was already accepted so they spend their pass on the delta. This is not belt-and-braces: on 2026-08-19 (PR #2044) round 1 asked for the give-up summary to be deferred into its try and for a new retry class to be reported; round 2 found that the fix for the second one reintroduced the first one's bug one line away (an unguarded $metadata read on a path where nothing had read the field yet) and, separately, printed a new default-level warn on graceful-degradation paths that had been silent. A test reviewer in the same round found eight surviving mutants in branches that round 1's fixes had introduced. Neither defect existed when the first round ran, so no amount of rigor there could have caught them.

9. Live-test changed behavior

   • Unit tests verify code correctness; this step verifies feature correctness against the runtime the user actually sees.
   • Build the latest source: vp run build
   • For each user-visible change in the diff (CLI command, output format, flag, error message), run the actual command path against a real or fixture input and confirm the output matches the spec / CDK CLI parity claim:
     • CLI surface change → run node dist/cli.js <subcommand> <args> against tests/integration/<example>/cdk.out or a real state bucket; verify each output mode (--long / --json / patterns / etc.).
     • State-touching change → exercise it against a real / test state bucket (e.g. cdkd-state-test).
     • Non-CLI library change → run a minimal repro that imports the new code path.
   • "Tests passed" is not "feature works." Always run the actual command before declaring done. If you cannot live-test (no real-AWS credentials, no fixture available), say so explicitly rather than skip silently — the gate exits non-zero in that case so a reviewer can decide whether to accept the trade-off.

10. Retrospective + rules update

    • Walk back over the session that produced this PR. For each surprise, friction, or correction the user had to make, ask: "is this a one-off, or a pattern that will recur?"
    • For each pattern, propose where it should be reflected so it doesn't recur:
      • Hook — pattern can be detected mechanically (e.g. fragile shell pattern, deprecated tool, marker-gated step). Strongest enforcement.
      • Skill / marker — pattern is a checklist that must be done before some action. Use the /check+check-gate / /check-docs+check-gate / /verify-pr+verify-pr-gate / /run-integ+integ-destroy-gate template.
      • Memory — pattern is judgmental ("prefer X when Y") and not mechanically detectable. Weakest enforcement; honest about its limits.
    • Surface the proposals out loud (in chat, or in this PR's body) before merging. If the user agrees, write them in the same PR for code/skill/hook artifacts; memory entries are local to ~/.claude/projects/.../memory/ so they land regardless of PR boundaries.
    • The retrospective is itself one of the items the verify-pr marker covers — skipping this step means the marker is set on incomplete work.

11. Residual review-nit sweep (mandatory — added 2026-05-22 after a multi-PR session left ~9 reviewer-flagged nits unfiled when the parent declared "session complete")

    • For every /review-pr reviewer agent output during this session (including re-reviews after fix-back), walk the reviewer's "Minor / Nit / Informational" section.
    • For EACH item there, confirm ONE of the following is true BEFORE setting the verify-pr marker (these are the same three buckets as CLAUDE.md's "Remaining work" taxonomy — Fixed here / TODO / Won't-do):

      • (a) Fixed in this PR — point at the fix commit / file:line that resolves the nit.

      • (b) TODO (issue #N) — a GitHub issue exists AND this PR's body references it (e.g. "minor follow-ups in (#515)"). This is the only bucket that leaves future work. The issue body MUST carry the four classification lines, one field per line (see CLAUDE.md → "The four TODO fields"), plus the Dup-check: line /work-issues section 5-f (.claude/skills/work-issues/references/filing.md) requires at filing time:

        Session-fit: now (do it in this session) | next (not this session) — <reason>
        Severity: high | medium | low — <what stays broken while it is undone>
        Effort: small (S) | medium (M) | large (L) — <which verification cycle it drags>
        Estimate: <duration, e.g. ~1-3 h -- never a bare letter> — <what eats the time>
        

        The reviewer agents grade on a DIFFERENT scale — translate, do not copy. .claude/agents/pr-*-reviewer.md report blocker / minor / nit; Severity takes high / medium / low. This step is exactly where a reviewer's word gets carried into an issue body, so map it: nit -> low, minor -> medium. There is deliberately no blocker arm: this step walks only the "Minor / Nit / Informational" section, and a blocker is resolved by step 8's fix-back loop before you ever get here. If one reaches this step, the steps are being run out of order — go back to step 8 rather than grading it. And re-read the mapped value against the Severity scale rather than trusting it: reviewer severity grades how bad the FINDING is, while Severity grades what stays broken for a USER, and the two come apart on internal-consistency nits.

        , so it is where the call gets made — not at wrap time, by which point the evidence for it (which files were open, which verification cycle was already paid for) is gone. A item must be fixed before the marker is set, or re-classified to with the reason recorded; you cannot set over an open .

12. PR title + body freshness (skip if no PR exists yet — /create-pr will write them from scratch)

    • When a PR has follow-up commits after creation, both the title and body authored at PR-create time often go stale: the title was scoped to the first commit's intent only, and the body may mention reverted features, removed checks, or wrong rationale. Detect and fix both.
    • Title check: read gh pr view <PR> --json title -q .title and confirm it still describes the union of commits on the branch. If a later commit added a separate concern (e.g. an unrelated fix, an opportunistic refactor), broaden the title. Update via gh api -X PATCH repos/{owner}/{repo}/pulls/{number} -f title="..." (NOT gh pr edit --title, which currently fails silently due to GraphQL Projects-classic deprecation — see hook gh-pr-edit-deprecation-gate.sh).
    • Body freshness commands:
      • gh pr view <PR> --json commits -q '.commits | length' — commit count on the PR
      • git log main..HEAD --oneline | wc -l — commit count locally
      • If they match and >1, the PR has been iterated on; the initial body is almost certainly stale
    • Read the current body (gh pr view <PR> --json body -q .body) and compare against the actual final diff (git diff origin/main...HEAD). Flag any of:
      • Bullets describing behavior that was reverted in a later commit
      • Bullets describing checks/validations the code no longer performs
      • File:line citations that no longer exist
      • Wording that contradicts the current README.md / CLAUDE.md
      • Stale numeric claims ("N tests pass" when the count has since changed)
    • If stale, rewrite the body and patch via:

    # Write desired body to a file (avoids shell escaping issues with backticks)
    cat > /tmp/pr-body.md <<'EOF'
    ## Summary
    ...
    ## Test plan
    ...
    EOF
    gh api repos/{owner}/{repo}/pulls/{number} -X PATCH --field "body=@/tmp/pr-body.md" -q '.html_url'
    

    Note: gh pr edit --body may fail with "Projects (classic) is being deprecated" — fall back to the gh api PATCH form above.

• Verify with gh pr view <PR> --json body -q .body | head -5 that backticks and special chars rendered correctly.

Output

Present results as a table:

Check	Result
typecheck	pass/fail
lint	pass/fail
build	pass/fail
tests (N files, M tests) (vp test run)	pass/fail
test coverage for changes	pass/fail
CI	pass/fail
working tree	clean/dirty
docs consistency	pass/fail
leftover resources	none/found
integ-destroy marker (deletion-touching PRs only)	fresh/stale/n-a
integ-broad marker (cross-cutting deploy/destroy PRs only)	fresh/stale/n-a
integ-local marker (local-execution-touching PRs only)	fresh/stale/n-a
code review (incl. shared-utility callers)	pass/issues found
live-test changed behavior	pass/skipped/issues found
retrospective + rule proposals	done/skipped
residual review-nit sweep (fixed / TODO-issue / won't-do)	N items / 0 unhandled
every TODO carries Session-fit / Severity / Effort / Estimate	N classified / 0 open now
auto-close audit (no Closes (#N) in body)	clean / N traps fixed
PR title + body freshness	up-to-date/stale (updated)/n-a (no PR yet)

If all pass, confirm "PR is ready to merge." If any fail, list the issues to fix.

Then add the State line CLAUDE.md's wrap-report rule requires — this skill's report is the single most common place it is needed, because "ready to merge" is almost never the end of the turn:

• A check that is merely pending (CI still running, an integ in flight, a reviewer agent not back yet) is WAITING, not a failure and not a stop. Say what you are waiting on, the signal that will re-invoke you (gh pr checks <N> --watch, a background-task completion notification), and that you will merge once it is green. Do not hand the user a "ready to merge" verdict and then go quiet — that reads as STOPPED and leaves them unsure whether to intervene.
• A check that legitimately cannot pass (no AWS credentials for the live-test, a decision only the maintainer can make) is not WAITING either — there is no signal coming. Either resolve it, or ask through the AskUserQuestion tool so the run continues from the answer. Never end the turn with the question in prose.
• Report STOPPED only when the PR is merged (or the user explicitly owns the next step) and nothing is pending.

Final Step

After all checks pass, record THREE markers via markgate so the gate hooks allow the next git commit, gh pr create, and gh pr merge. /verify-pr is a superset of /check (code correctness) and /check-docs (docs consistency), and adds live-test + retrospective + scope-match on top — so its success implies all three. cdkd pins markgate via mise, so use mise exec to avoid PATH issues when shims aren't active:

mise exec -- markgate set check
mise exec -- markgate set docs
mise exec -- markgate set verify-pr

The verify-pr marker is the one consulted by .claude/hooks/verify-pr-gate.sh to allow gh pr create and gh pr merge. It is intentionally settable ONLY by this skill — running it by hand from a shell to bypass the gate defeats the whole point. If a check legitimately cannot pass right now (e.g. the live-test cannot run because the user lacks AWS credentials), say so explicitly in the report and DO NOT set the marker — the gate exits non-zero so the human can decide whether to override.

Then, if there are uncommitted changes (e.g., lint fixes, doc updates made during this run), commit them and push to the remote. This ensures the remote branch is always up to date when reporting "PR is ready to merge."

Skip the marker + commit step if any check failed.