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 tsgo: 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 run test - all unit tests pass
   • Report test count (files and tests)
   • Test coverage check: compare git diff 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
   • Coverage matrix freshness: CI runs three independent matrix checks (integ-coverage, scenario-coverage, audit:coverage:check) and hard-fails on staleness. PR #548 hit two of these in succession because /verify-pr only regenerated integ-coverage locally — the contributor pushed, CI failed on audit:coverage:check, the contributor regenerated provider-coverage and pushed again, CI failed on scenario-coverage matrix is up-to-date. The block below covers all three so the same round-trip can't happen again:

     fixtures_changed=$(git diff main...HEAD --name-only | grep -qE '^src/provisioning/register-providers\.ts$|^tests/integration/[^/]+/(lib|bin)/.+\.ts$|^tests/integration/[^/]+/\.scenarios\.json$' && echo yes)
     providers_changed=$(git diff main...HEAD --name-only | grep -qE '^src/provisioning/register-providers\.ts$' && echo yes)
     
     if [ "$fixtures_changed" = "yes" ]; then
       vp run integ-coverage
       vp run scenario-coverage
     fi
     
     if [ "$providers_changed" = "yes" ]; then
       # `--check` is offline (~0.5s) and verifies the cached
       # docs/_generated/provider-coverage.json matches register-providers.ts
       # under the current Tier classification. If it fails, the contributor
       # must run `vp run audit:coverage:regenerate` (heavy: ~15 min, requires
       # AWS creds with cloudformation:ListTypes + DescribeType) and commit
       # the regenerated cache. /verify-pr does NOT auto-run :regenerate
       # because the cost is too high and AWS creds may not be present in
       # the local dev environment.
       vp run audit:coverage:check
     fi
     
     git status --short docs/integ-coverage.md docs/_generated/integ-coverage.json \
                        docs/scenario-coverage.md docs/_generated/scenario-coverage.json \
                        docs/_generated/provider-coverage.json docs/_generated/provider-coverage.md
     

     If git status reports any of these files as dirty, the contributor forgot to regenerate after their code change. Stage the regenerated output, amend / new-commit it onto the PR, and re-run /check-docs to refresh the docs marker. If vp run audit:coverage:check exits non-zero, run vp run audit:coverage:regenerate (heavy — see above) and commit the regenerated docs/_generated/provider-coverage.{json,md}. The provider-integ-gate.sh PreToolUse hook blocks git commit when a new registry.register('AWS::Foo::Bar', ...) is added without integ coverage (literal type id, Cfn<Type>( L1 class, or // allow-no-integ: <rationale> 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
     

     If this exits non-zero, run /run-integ <relevant-test> (e.g. bench-cdk-sample) and confirm it reports 0 errors / 0 orphans — the skill itself will then call markgate set integ-destroy. 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:

     • src/deployment/deploy-engine.ts
     • src/deployment/intrinsic-function-resolver.ts
     • src/cli/commands/destroy-runner.ts
     • src/cli/commands/destroy.ts
     • src/cli/commands/deploy.ts
     • src/analyzer/dag-builder.ts
     • src/analyzer/template-parser.ts
     • src/provisioning/register-providers.ts

     ...you MUST run a broad integ in addition to whatever feature-specific integ the change came with. The canonical broad set (keep in sync with .claude/hooks/integ-broad-gate.sh, .claude/skills/run-integ/SKILL.md step 11, .markgate.yml integ-broad gate, CLAUDE.md "integ-broad" entry):

     • bench-cdk-sample (39-resource VPC+NAT+CF+Lambda+SQS)
     • lambda
     • microservices
     • drift-revert
     • drift-revert-vpc
     • multi-stack-deps
     • multi-resource
     • remove-protection
     • export

     These exercise multi-resource VPC / Lambda / IAM / CFn-Custom paths that narrow integs leave uncovered. Cross-cutting code paths affect EVERY user's deploy/destroy, not just the feature you added — broad integs are the only structural defense against shipping a regression that only surfaces in production on stacks unlike your fixture. Bypassing this is the PR #348 trap from 2026-05-13 (Issue #343 shipped without bench-cdk-sample validation; surfaced post-merge as an incident).

     # Detection: only fires when the diff actually touches cross-cutting code.
     if git diff main...HEAD --name-only | grep -qE '^src/deployment/(deploy-engine|intrinsic-function-resolver)\.ts$|^src/cli/commands/(destroy-runner|destroy|deploy)\.ts$|^src/analyzer/(dag-builder|template-parser)\.ts$|^src/provisioning/register-providers\.ts$'; then
       echo "Cross-cutting code touched — broad integ required (bench-cdk-sample / lambda / microservices / drift-revert)."
       # Then run the broad integ via /run-integ and confirm 0 errors / 0 orphans.
     fi
     

     The narrow feature integ stays valuable for testing the FEATURE; the broad integ is the regression backstop. Both must pass; both refresh the same integ-destroy marker.

   • For local-execution-touching PRs (any change under src/local/**, src/cli/commands/local-*.ts, tests/integration/local-*/**): the integ-local markgate gate physically blocks gh pr merge when its marker is stale (see .claude/hooks/integ-local-gate.sh). The merge-time gate has a known blind spot: it reads the local working tree digest, and when gh pr merge runs from a parent worktree still on pre-PR main, the digest matches the old content and the gate passes silently — so an unverified local-execution change can reach main via the merge-from-parent path. /verify-pr runs in the PR's own worktree (post-PR content), so verifying the marker here closes that gap structurally:

     # Only check when the PR diff actually touches the gate scope.
     # Mirrors how `gh pr merge` is checked, but in the worktree that has the PR's content.
     if git diff main...HEAD --name-only | grep -qE '^src/local/|^src/cli/commands/local-|^tests/integration/local-'; then
       mise exec -- markgate verify integ-local
     fi
     

     If this exits non-zero (digest differs OR expired by TTL), run /run-integ local-<test> against a test that exercises the changed surface — local-start-api for HTTP-server / route-discovery / authorizer / container-pool changes, local-invoke for Lambda-runtime / ZIP-asset changes, local-run-task for ECS changes, local-invoke-container for container-Lambda changes, local-invoke-layers for Lambda Layers changes. The integ skill calls markgate set integ-local itself when the Docker-side check passes. As with integ-destroy, CI is necessary but not sufficient — it does not exercise Docker-based local execution.

   • For each region this PR may have created resources in (typically us-east-1), spot-check the most failure-prone resource types — VPCs (describe-vpcs --filters "Name=tag:Name,Values=Cdkd*/Vpc"), Lambda hyperplane ENIs (describe-network-interfaces --filters "Name=description,Values=AWS Lambda VPC ENI-*"), CloudFront Distributions, NAT Gateways. Any match against a stack name in this PR's diff = orphan, must be cleaned up before merge.

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).

     The skill applies bias factors (security surfaces bump up; pure-infra / docs / tests-only bump down). 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.

   • git diff main...HEAD — confirm the diff is what you reviewed (no last-minute commits slipped through).

   • For each change: is it correct? complete? necessary?

   • Check for:

     • Logic errors or unhandled edge cases
     • Unnecessary changes (reverted code still in diff, dead code, unrelated changes)
     • Inconsistencies between changed files

   • Verify all callers of changed functions handle the new behavior

   • Verify type definitions are consistent with implementation

   • Shared-utility regression check: if any file under src/utils/** (or another widely-imported module) changed, list every importer (grep -rl "from '\.\./.*utils/<file>'" src tests) and walk through each one to confirm the new behavior is correct for them. A change to a shared helper is only "done" when every caller has been considered.

   • Internal-interface contract change check: if the diff changes the semantics of arguments an interface receives — even if the type signature is unchanged — list every implementer and walk through each one. Examples that count as a contract change: provider.update's newProperties shifting from "full desired state" to "partial / overlay / etc."; an intrinsic resolver's input format changing; a state schema field's invariant changing. The risk is implementers that silently treat the old shape's invariants as load-bearing (e.g. SNSTopicProvider.update treating newProps[K] === undefined as "remove K from AWS"). PR #161 hit this — the first-pass design ("drifted-only partial newProperties") had to be reworked after audit found SNSTopicProvider and IAMRoleProvider.updateManagedPolicies would silently clear non-drifted attrs. Audit BEFORE writing tests against the new design, not after — discovering the breaks via tests-after-design forces a design rework and invalidates the tests already written.

     # For provider interface changes:
     grep -rln "implements ResourceProvider" src/provisioning/providers/
     # For each implementer, read the body of the affected method and write
     # down what it assumes about the argument's shape — truthy gates,
     # diff-based "absent = remove" semantics, JSON.parse on stringly-typed
     # input, etc. The new contract must preserve every assumption that
     # is load-bearing, OR every implementer must be updated in the same PR.
     

     See feedback_internal_contract_audit_first.md for the full pattern.

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:
      • (a) Addressed in this PR — point at the fix commit / file:line that resolves the nit.
      • (b) Filed as a follow-up issue — a GitHub issue exists AND this PR's body references it (e.g. "minor follow-ups in (#515)").
      • (c) Explicitly accepted as known cost — the PR body or a comment names the nit and explains why it's acceptable to ship as-is.
    • If NONE of (a) / (b) / (c) is true for any nit, file a bundled follow-up issue NOW (one issue per session, listing every uncovered nit) and update the PR body to reference it. Do not set the verify-pr marker until every reviewer-flagged item is on one of those three paths.
    • Also walk the session transcript for surfaced memory-rule candidates (surprising traps, repeated friction, "I should remember this for next time" moments). Each MUST be either written as a memory file in ~/.claude/projects/-Users-goto-pc-github-cdkd/memory/ (with a MEMORY.md index entry) OR explicitly de-prioritized in the chat / PR body.
    • Auto-close audit (added 2026-05-22 — counter-trap to the closing-paren disambig convention in memory feedback_pr_body_no_hash_for_item_numbers.md):
      • Read the PR body (gh pr view <PR> --json body -q .body). For every (#N) parens-form reference, check whether it's adjacent to a close keyword (closes / fixes / resolves, case-insensitive).
      • If yes: the merge will NOT auto-close the target issue. Either rewrite to parens-free Closes #N (auto-close fires), OR add a manual gh issue close <N> step to the merge sequence and note it in the PR body.
      • The mechanical closes-paren-form-gate.sh hook ALREADY blocks gh pr merge for the Closes (#N) pattern — this skill step is the human-readable backup that catches the issue BEFORE the merge attempt.
    • This step is the structural enforcement of memory feedback_session_completion_audit_required.md — claiming "session complete" / "残作業なし" without running this sweep is the exact violation that surfaced this rule.

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 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)	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 (filed / addressed / accepted)	N items / 0 unhandled
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.

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.