--- name: pr-review-merge description: > Drive a single pull request to merge-ready across all six criteria (sync, CI, inline comments, conversation, threads, bot re-review of the head commit), then smart-merge it. Source-agnostic library skill invoked by the /tm, /issues, /fix-pr, and /fix-develop commands and by marathon teammates. TRIGGER when a command or agent needs the PR review-to-green loop or the smart-merge (stale-bot-CR dismissal, auto-merge criteria, UNSTABLE/UNKNOWN handling, merge ordering), or when the user asks to take a PR to green/merge it. --- # PR Review-to-Green + Smart Merge Source-agnostic. Consumers pass: PR number, base branch, and bot-reviewer/CI rules from the project's `## Marathon Configuration` (defaults if absent). ## Run-completion is gated elsewhere (floor) This skill takes one PR to merge — a **process** signal. A green, merged PR is not proof the run's assembled product works, and this skill never certifies run-completion. That is gated by the acceptance-contract scripts the marathon engine invokes, not here: `scripts/contract/start_gate.py` fails closed at run start unless the contract is frozen before decomposition, `scripts/contract/spawn_verifier.py` is the sole custody chokepoint that spawns the cold non-implementing verifier against the assembled product, and `scripts/contract/complete_gate.py` fails closed unless that verifier's completion record validates. Merging here never substitutes for those gates. This note is part of the constitutional floor (`FLOOR.md`); the retro may propose changes but never self-apply them. ## Ready Criteria (ALL must be true) The PR is merge-ready only when all six are simultaneously true. Re-check from the top after every push — a fix can reopen an earlier criterion. 1. **Branch in sync** — no merge conflicts with base branch 2. **CI passing** — all checks succeed (or skipped) 3. **All inline comments addressed** — see thread resolution rules 4. **No unaddressed conversation comments** — actionable feedback responded to 5. **All review threads resolved** — no unresolved threads remain 6. **Re-reviewing bots have reviewed the head SHA** - every bot the project's Marathon Configuration flags `Re-reviews on push: yes` has completed its pass on the current head SHA. For a bot without `Re-review check name`, at least one of its reviews must carry that head SHA as its `commit_id` (a review of an earlier commit does not count), and that is the limit of this path: any review on the head SHA satisfies it, a placeholder or rate-limited one included, so a bot whose pass must be proven belongs on `Re-review check name`. For a bot configured with `Re-review check name`, only the check run of that name on the head SHA decides (its submitted reviews do not: the check run is what turns red when the reviewer exits with an error after posting), and that run has four observable states: **in progress** (or not yet created) - keep waiting until the max wait expires; **completed with conclusion `success`** - the criterion is satisfied for that bot, and the marathon skill's `Commit:`-line spot check still applies before merging on it; **completed with conclusion `skipped`** - the bot is not applicable to this PR (a reviewer job that skips some actors, such as Dependabot, by design), so the criterion is satisfied for that bot with no warning; **completed with any other conclusion** (failure, cancelled, neutral, timed_out) - a settled verdict that the bot did not complete a green pass, so the criterion takes the warning path at once. Any push restarts the wait. The wait is bounded per bot by that bot's `Max wait for re-review`, measured from when the head commit was pushed (the earliest check-suite creation on the head SHA, not the commit's committer date, which predates the push and resets on rebase); when `Re-reviews on push: yes` and `Max wait for re-review` is absent, the bound is 15m. If no check suite exists on the head SHA, fall back to the head commit's committer date as a floor - explicitly not the push time, it only guarantees the bound always expires. When the max wait expires before the bot completes, or a check-name bot's run settles on a conclusion other than `success` or `skipped`, the criterion passes with a warning recorded in the merge record (the PR comment or report that accompanies the merge) naming the bot, the head SHA, and what was observed: for a check-name bot, the check run's conclusion on that SHA (or that no completed run exists) and, so the record never contradicts the reviews endpoint, whether path (a) shows a review of that head SHA anyway; for a review bot, that no review of that SHA was found. The record states that the bot did not complete a green pass, not that it did not review. Merge proceeds with that warning; nothing holds forever on an advisory bot. Bots without `Re-reviews on push: yes` are never waited on, so a project with no such flags sees no change. **Checking criterion 6** (per flagged bot; `` and the optional `` from its `Re-review check name` field in the Marathon Configuration): ```bash HEAD_SHA=$(gh pr view $PR --json headRefOid | jq -r '.headRefOid') # Push time of the head commit: GitHub creates check suites on push. # --paginate emits one JSON page after another; jq -s gathers them so min spans all pages. PUSHED_AT=$(gh api --paginate "repos///commits/$HEAD_SHA/check-suites?per_page=100" \ | jq -r -s '[.[].check_suites[].created_at] | min // empty') # No check suite on the head SHA (min of an empty list is null): use the committer date as a # floor so the max wait always expires. This is not the push time; it predates the push. if [ -z "$PUSHED_AT" ]; then PUSHED_AT=$(gh api "repos///commits/$HEAD_SHA" | jq -r '.commit.committer.date') fi # (a) Bot that submits reviews: its reviews whose commit_id is the head SHA. # Reviews page in ascending order, so the newest head-SHA review sits on the last page: # --paginate reads every page and jq streams matches from each one. gh api --paginate "repos///pulls/$PR/reviews?per_page=100" \ | jq -r --arg sha "$HEAD_SHA" --arg bot "" '.[] | select(.commit_id == $sha and .user.login == $bot) | .submitted_at' # (b) Bot hosted as a check run (Re-review check name set): matched by check-run name. # Prints every run of that name as " ", whatever its outcome. gh api --paginate "repos///commits/$HEAD_SHA/check-runs?per_page=100" \ | jq -r --arg n "" '.check_runs[] | select(.name == $n) | "\(.status) \(.conclusion // "in_progress") \(.completed_at // "")"' ``` Complete when, for a bot with `Re-review check name` set, path (b) prints a line whose conclusion is `success` or `skipped` (`skipped` means the bot does not apply to this PR, so no warning is recorded; path (a) does not decide for that bot, and its output is recorded only on the warning path); a `completed` line with any other conclusion (failure, cancelled, neutral, timed_out) is the settled warning-path case, which takes the warning path at once; no line, or only lines whose status is not `completed`, means keep waiting until the max wait expires. For a bot without one, complete when path (a) prints at least one line (a head-SHA review), and otherwise keep waiting until the max wait expires. The max wait is `Max wait for re-review` since `PUSHED_AT`, 15m when that field is absent. The filters use `--arg` and pipe to `jq` rather than `gh api --jq`, per the Shell Pitfalls below. Never match check runs by app slug: every GitHub Actions job, the reviewer included, reports as `github-actions`, so an unrelated job completing would pass the criterion. Merge proceeds with the warning on expiry or on a settled run whose conclusion is neither `success` nor `skipped`. A green reviewer check still warrants the spot-check the marathon skill describes (the reviewer's summary cites the head SHA) before merging on the strength of it. **Thread resolution rules:** Follow bot reviewer rules from the project's CLAUDE.md Marathon Configuration. Generic defaults: - **Bot threads**: Fix the code and push. Resolve via GraphQL if addressed. Use jq JSON builder (avoids zsh `$` escaping): ```bash jq -n --arg tid "$THREAD_ID" '{"query": "mutation { resolveReviewThread(input: {threadId: \"\($tid)\"}) { thread { isResolved } } }"}' | gh api graphql --input - ``` - **Human threads**: Fix the code, reply inline explaining the fix, `@mention` the reviewer. Do NOT resolve human threads — let the reviewer confirm. ## Shell Pitfalls **Never use `gh ... --jq` with complex filters.** Always pipe to `jq` separately. **Use positive jq filters, not negative.** zsh escapes `!=` to `\!=`, breaking filters silently: ```bash # WRONG: gh pr view --json reviews --jq '.reviews[] | select(.state != "APPROVED")' # WRONG: gh pr view --json reviews | jq '.reviews[] | select(.state != "APPROVED")' # RIGHT: gh pr view --json reviews | jq '.reviews[] | select(.state == "CHANGES_REQUESTED")' ``` ## Review Loop (each iteration) ### Step 1: Sync with base branch (FIRST, every iteration) ```bash BASE=$(gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name' 2>/dev/null || echo "main") git fetch origin $BASE && git merge origin/$BASE --no-edit # If conflicts: resolve them, commit, push # If can't auto-resolve: report blocked with details ``` **Resolving a `.claude-plugin/plugin.json` conflict (the common one in multi-PR waves).** The conflict is **not always confined to `.version`** — a sibling PR may also have reframed the marketplace `description` or another field. A version-only line edit (`sed`-ing just the version line, or a regex that targets only `.version`) silently keeps the stale side of *every other* field and can leave merge markers behind. Resolve the **whole file** deterministically: take the side carrying the siblings' already-merged field changes (usually base), then overwrite only the version with `jq`: ```bash git checkout --theirs .claude-plugin/plugin.json # base side, which has the merged siblings' description/other-field edits jq --arg v "" '.version = $v' .claude-plugin/plugin.json > /tmp/pj && mv /tmp/pj .claude-plugin/plugin.json git add .claude-plugin/plugin.json ``` Always `git diff` the full `plugin.json` before resolving — never assume the only divergence is the version line. **Conflict resolution patterns:** - **Import/route files** (e.g., App.tsx, index.ts): Accept BOTH sides — additions are additive - **Barrel exports** (e.g., shared/index.ts): Accept both sides — each adds its own export - **Config/manifest files**: Accept both sides unless the same key is modified differently (then ask) ### Step 2: Check all criteria - delegate CI to background agent ```bash PR=$(gh pr view --json number --jq '.number') ``` **Spawn a background agent to watch CI** (never block on CI yourself): ``` Agent( run_in_background: true, prompt: """ Monitor PR #$PR for CI completion and review state. 1. Block on CI: `gh pr checks $PR --watch --fail-fast` 2. When CI settles, gather: - CI result: `gh pr checks $PR --json name,state,conclusion` - Unresolved threads: `gh api graphql ...` (all unresolved with path/line/author/body) - Conversation comments: `gh pr view $PR --comments --json comments` - Pending checks count 3. Return structured report. """ ) ``` **While CI runs (you are FREE)**, do an immediate check for threads and comments: ```bash # Quick thread check - fix what you can now # Substitute the repo owner/name the consumer passed in. gh api graphql -f query='query { repository(owner: "", name: "") { pullRequest(number: '$PR') { reviewThreads(first: 50) { nodes { id isResolved path line comments(first: 1) { nodes { author { login } body } } }}}}}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {id, author: .comments.nodes[0].author.login, path, line, body: .comments.nodes[0].body[0:200]}' # For each unresolved thread with a path, check local code to see if already fixed. # If addressed, resolve bot threads via GraphQL immediately (no push needed). # Conversation comments gh pr view $PR --comments ``` ### Step 3: Fix and batch **Fix issues locally while CI runs** - stage changes but don't push yet: - Unresolved bot threads - **Check local code first** at the referenced path:line. If already addressed, resolve via GraphQL immediately (no push needed). If not, fix the code and `git add`. Follow bot reviewer rules from CLAUDE.md Marathon Configuration. - Unresolved human threads - Fix code, reply inline, @mention reviewer - Actionable conversation comments - Respond or fix - Merge conflicts - Resolve using patterns above, or report blocked if ambiguous **When background agent notification arrives** (CI settled): - CI passed + no local fixes staged: evaluate whether all six criteria are met, including criterion 6 (every bot flagged `Re-reviews on push: yes` has reviewed the head SHA, or its `Max wait for re-review` has expired) - CI passed + local fixes staged: push once (batches all thread fixes into one CI cycle) - CI failed: fix CI issues too, then push everything together, spawn new background watcher - **All six criteria met** - Report ready (name any bot whose max wait expired so the merge record carries the warning), STOP **This keeps you responsive.** While CI runs, you process threads and comments. When CI settles, you act on the full report. No blocking waits. ## Smart Merge Invoked to merge a PR that has been reported merge-ready. Always verify via the API before merging — never trust the report. **Known: Stale bot CHANGES_REQUESTED.** Some bot reviewers submit CR reviews that GitHub does not auto-dismiss on re-review. Check the project's Marathon Configuration for bot-specific patterns. Default: dismiss any stale bot CHANGES_REQUESTED before merging. ```bash PR= # Step 1: Dismiss stale bot CRs STALE_REVIEWS=$(gh api repos///pulls/$PR/reviews \ --jq '[.[] | select(.state == "CHANGES_REQUESTED" and (.user.login | endswith("[bot]")))]') echo "$STALE_REVIEWS" | jq -r '.[].id' | while read REVIEW_ID; do gh api repos///pulls/$PR/reviews/$REVIEW_ID/dismissals \ --method PUT -f message="Stale bot review — verified findings addressed" -f event="DISMISS" done # Step 2: Check merge state gh pr view $PR --json mergeStateStatus,mergedAt,reviews \ | jq '{ mergeStateStatus, mergedAt, approvals: [.reviews[] | select(.state == "APPROVED")] | length, changesRequested: [.reviews[] | select(.state == "CHANGES_REQUESTED")] | length }' ``` **Auto-Merge Criteria (ALL must be true):** 1. `mergeStateStatus` is `"CLEAN"` — OR `"UNSTABLE"` with only non-required checks failing 2. At least `$REQUIRED_APPROVALS` approvals — OR `$MARKDOWN_APPROVALS` for markdown-only PRs (some bot reviewers skip them) 3. Zero non-dismissed changes-requested reviews 4. **Re-review of the head SHA complete** - Ready Criterion 6 holds on the head SHA being merged: every bot flagged `Re-reviews on push: yes` has completed its review of that commit, or its `Max wait for re-review` expired and the merge record names the bot with a warning. Re-check at merge time; a push after the ready report restarts the wait. 5. **Base branch is healthy** — if the PR's CI failures exist on `$BASE_BRANCH` too (pre-existing), do NOT merge and compound the problem. Instead, spawn a separate worktree/PR to fix the failing tests on `$BASE_BRANCH` first, then rebase and merge the original PR. **UNSTABLE handling:** If `mergeStateStatus == "UNSTABLE"`, check failing checks against `meta.flaky_checks` and any CI patterns from the project's Marathon Configuration. If ALL failing checks are non-required AND not pre-existing on `$BASE_BRANCH`, treat as merge-eligible. Report: "Merging with UNSTABLE — only non-required checks failing: ". If failures ARE pre-existing on `$BASE_BRANCH`, fix it first (criterion 5). **UNKNOWN handling:** GitHub sometimes returns `mergeStateStatus: "UNKNOWN"` even when all checks pass. If UNKNOWN but CI all green and 0 unresolved threads, retry up to 3 times with 30s backoff. If still UNKNOWN after retries, treat as CLEAN and proceed (log the override). **The merge command — use `--admin` on solo-maintainer repos.** When `$REQUIRED_APPROVALS` is 0 and only the named required checks gate, a plain `gh pr merge --squash` can be **refused** ("base branch policy prohibits the merge") whenever a *non-required* check (CodeRabbit, an advisory AI review, a regression gate that re-runs on base advance) is PENDING or re-running at the exact merge instant — even though `mergeStateStatus` reported CLEAN/UNSTABLE a moment earlier. Once the named required checks are all SUCCESS, merge with admin override so a mid-run non-required check can't bounce you: ```bash gh pr merge $PR --squash --delete-branch --admin ``` Only do this once the *required* checks are green (admin override bypasses branch policy, not your own merge criteria). On multi-PR waves, a gate that re-runs on every base advance (each merge re-triggers it on the pending PRs) makes the plain-merge bounce recurring — `--admin` avoids a wait-and-retry cycle per PR. **Verify before merging** — never trust the caller's claim that a PR is ready: ```bash gh pr view $PR --json state,mergedAt,mergeStateStatus | jq '{state, mergedAt, mergeStateStatus}' ``` Trust the API, not the message. **After merge — gate cleanup on a VERIFIED merge.** A merge call can be rejected (see the `--admin` note above) while a chained one-liner blindly runs cleanup anyway, deleting the worktree and branch of a PR that never merged. **Never chain cleanup unconditionally after the merge command.** Confirm `state == "MERGED"` (or `mergedAt != null`) first, then close the unit via the consumer's **close on merge** adapter operation and remove its worktree + branch: ```bash MERGED=$(gh pr view $PR --json state --jq '.state') if [ "$MERGED" = "MERGED" ]; then git worktree remove --force git branch -D else echo "MERGE NOT CONFIRMED ($MERGED) — skipping cleanup, retry merge" fi ``` Report the merge to the user. Any wave/next-task orchestration after a merge is the caller's responsibility (the marathon engine handles waves and teammate lifecycle) — this skill's job ends at a clean merge + cleanup. **If not merge-ready:** - BLOCKED → report blocked to the caller - DIRTY (merge conflict) → resolve using the Review Loop conflict-resolution patterns, or report blocked if genuinely ambiguous - Human changes requested → report to the caller; do not merge #### Merge Order (multiple PRs) 1. **Batch-dismiss stale CRs** across ALL eligible PRs first: ```bash for PR in ; do gh api repos///pulls/$PR/reviews \ --jq '.[] | select(.state == "CHANGES_REQUESTED" and (.user.login | endswith("[bot]"))) | .id' \ | while read REVIEW_ID; do gh api repos///pulls/$PR/reviews/$REVIEW_ID/dismissals \ --method PUT -f message="Stale bot review — batch dismissed" -f event="DISMISS" done done ``` 2. **Sort by hot-file impact**: Fewer hot files first. Hot-file PRs merge LAST. 3. **Merge sequentially**: One at a time. 10-20s between for conflict detection. 4. **Re-check merge state** after each — CLEAN can flip to DIRTY from cascade.