fix(ship,finalize-pr): enforce explicit loops, async waits, and result categorization

github-workflows/skills/finalize-pr/SKILL.md

@@ -18,6 +18,21 @@ No manual intervention required. For manual review-focused workflows, use `/revi
 > asynchronously. CI may have re-run. Merge conflicts may have appeared.
 > Re-fetch live PR state from Step 1.
 
+## Load-bearing rules
+
+These three rules govern the entire Phase 2 → Phase 3 loop. Apply them as written.
+
+1. **NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE.** (See
+   `superpowers:verification-before-completion`.) Subagent self-reports are not
+   state. Re-query the live count from `/gh-cli-patterns` after every `/resolve-*`
+   call.
+2. **PENDING IS NOT FAILURE.** If any required check is still running, wait — don't
+   abort. Phase 2.6 drains pending checks before Phase 3 fires.
+3. **If a `/resolve-*` call did not strictly decrease the corresponding count, the
+   subagent did nothing.** Track no-ops independently per `/resolve-*` invocation
+   type. Two consecutive no-ops on the same fix type → exit Phase 5 with
+   `Blocked on: <gate>`.
+
 ## Critical Rules
 
 1. **Wait for user approval to merge** - Report ready status, then pause for user merge command
@@ -85,13 +100,25 @@ start of each iteration. For org-wide mode, use `repository.nameWithOwner` from
 
 Steps 2.1 and 2.2 start concurrently (2.1 is non-blocking). Steps 2.3 and 2.4 run sequentially after 2.2.
 
+### 2.0 Iteration Counter (REQUIRED)
+
+Initialize `phase_2_iteration = 0` at first entry to Phase 2. Increment on every
+re-entry from a failed Phase 3 gate. **Hard cap: 5.** On the 6th entry, emit a
+state dump (last gate result, CodeQL count, unresolved threads, CI rollup, HEAD
+SHA) and exit straight to Phase 5 with `Blocked on: iteration-cap`.
+
 ### 2.1 Start CI Monitoring (BACKGROUND)
 
 Launch CI monitoring in a background Task agent (`run_in_background: true` on the Task tool).
 Monitor CI checks using `--watch` so the agent blocks until all complete.
 
 Do NOT wait for the agent to finish — proceed to 2.2 immediately.
 
+**Background-agent fallback**: if the Task agent exits without a final status
+(non-zero, network error, or no result), fall back to direct polling —
+`gh pr checks <PR_NUMBER> --watch` with a 10-minute timeout. Log the fallback
+visibly; treat the direct-poll output as authoritative for Step 2.3.
+
 ### 2.2 Parallel Fixes
 
 Run these checks simultaneously. Launch independent fixes in parallel via
@@ -102,12 +129,12 @@ Task agents when they touch different files. Invoke `superpowers:dispatching-par
 Run the **canonical code-scanning alert count** from /gh-cli-patterns.
 Replace `<OWNER>` and `<REPO>` per the placeholder convention in that skill.
 
-**If violations found**: Invoke `/resolve-codeql fix`, validate locally.
+**If violations found**: Invoke `/resolve-codeql fix`, validate locally. Verify in Step 2.2.5.
 
 #### Review Threads
 
 Invoke `/resolve-pr-threads`. It exits cleanly when no threads exist.
-After completion, validate locally.
+After completion, validate locally. Verify in Step 2.2.5.
 
 #### Merge Conflicts
 
@@ -117,6 +144,19 @@ attempt merge, report unresolvable conflicts for user. After resolution, validat
 readiness verification (including `mergeStateStatus`, CI, CodeQL, review decision, threads) happens
 in Phase 3.
 
+### 2.2.5 Post-Fix Verification (REQUIRED — applies to every `/resolve-*` call)
+
+After any `/resolve-codeql` or `/resolve-pr-threads` invocation, re-query the
+corresponding count from `/gh-cli-patterns` (canonical code-scanning alert count,
+canonical unresolved thread count). Per the load-bearing rules:
+
+- Count strictly decreased → continue.
+- Count unchanged → the subagent did nothing. Increment that fix type's local
+  no-op counter. At 2 consecutive no-ops for the same fix type, exit Phase 5
+  with `Blocked on: <fix type>`.
+
+Track `codeql_noop_count` and `threads_noop_count` independently; do not merge.
+
 ### 2.3 CI Failure Fixes
 
 Check background CI results from 2.1:
@@ -138,6 +178,23 @@ and push before proceeding to 2.4.
 
 Verify final PR state, mergeability, and check status. If fixes introduced new issues, loop back to 2.2.
 
+### 2.6 Drain Pending Checks (REQUIRED before Phase 3)
+
+PENDING IS NOT FAILURE. CodeQL, required-reviewer hooks, and some third-party checks complete async after push. Phase 3 must not see PENDING; drain first.
+
+Poll every 30 seconds until no PENDING checks remain, OR for 5 minutes:
+
+```bash
+end=$(($(date +%s) + 300))
+while [ "$(date +%s)" -lt "$end" ]; do
+  pending=$(gh pr checks <PR_NUMBER> --json bucket --jq '[.[] | select(.bucket == "pending")] | length')
+  [ "$pending" = "0" ] && break
+  sleep 30
+done
+```
+
+CodeQL is NOT in `pr checks`. Also re-run the canonical code-scanning alert count from /gh-cli-patterns before advancing.
+
 ## Phase 3: Pre-Handoff Verification
 
 > ⛔ **NO SHORT-CIRCUIT — EVERY INVOCATION, EVERY TIME.**
@@ -153,7 +210,7 @@ Run the **canonical PR-readiness gate** from /gh-cli-patterns against
 `<PR_NUMBER>`. Replace `<OWNER>`, `<REPO>`, `<PR_NUMBER>` per the placeholder convention in
 that skill.
 
-**Required values — if any fail, return to Phase 2:**
+**Required values — if any fail, loop back to Phase 2 (subject to the iteration cap from Step 2.0):**
 
 | Field | Required | Abort message |
 |-------|---------|---------------|
@@ -171,6 +228,13 @@ that skill.
 > status check), `DIRTY` (conflicts), `DRAFT`, `UNKNOWN` (GitHub computing),
 > `UNSTABLE` (checks failed or pending). Any of these = return to Phase 2.
 
+If a failed field is something the AI cannot fix (e.g.,
+`reviewDecision = REVIEW_REQUIRED`, `isDraft = true`, `state ≠ OPEN`, or
+`mergeStateStatus = BLOCKED` with no resolvable cause), skip the loop and exit
+Phase 5 with `Blocked on: <field name and value>`. Loop only when the failure has
+an in-skill fix path (CI failure, CodeQL alert, unresolved thread, merge
+conflict, BEHIND).
+
 ### 3.2 CodeQL Gate (REST — separate from CI, re-run now)
 
 `statusCheckRollup` does NOT include CodeQL alert state. Run the **canonical code-scanning
@@ -226,24 +290,39 @@ Proceed to Phase 5.
 
 ## Phase 5: Record Result
 
-**Single/current-branch mode**: Emit the **Canonical PR Status Summary** (Section 1 =
-this PR, Section 2 = all open PRs in current repo) as defined in /gh-cli-patterns,
-titled `PR Status`. Then append:
+**Single/current-branch mode**: Emit the **Canonical PR Status Summary**
+(Section 1 = this PR, Section 2 = all open PRs in current repo) as defined in
+/gh-cli-patterns, titled `PR Status`.
+
+If every gate in Phase 3 passed, the PR row in Section 1 reads `Ready for review` (no extra annotation). Then append:
 
 ```text
 IMPORTANT: Do NOT merge this PR. Wait for the human to review and invoke
   /squash-merge-pr    # Squash all commits into one
   /rebase-pr          # Rebase commits onto main (preserves history)
 ```
 
-**Multi-PR mode**: Record the per-PR result (ready / blocked / needs-human). Restore the original
-branch and continue to the next PR. Do NOT emit a ready report — that happens in Phase 6.
+If any gate failed, the PR row in Section 1 includes `Blocked on: <gate>` naming
+the specific failed gate (e.g., `Blocked on: REVIEW_REQUIRED`, `Blocked on:
+CodeQL`, `Blocked on: iteration-cap`, `Blocked on: threads`). State the single
+manual action that would unblock it — never a generic "review manually."
+
+**Multi-PR mode**: Record the per-PR row (`Ready for review` or
+`Blocked on: <gate>`). Restore the original branch and continue to the next PR.
+Do NOT emit a ready report — that happens in Phase 6.
 
 ## Stop Condition
 
-MUST NOT return until ALL conditions pass for EVERY targeted PR:
-CI green, CodeQL clean, threads resolved, no conflicts, code simplified, local linters and tests pass, metadata updated.
-If ANY fails, loop back to Phase 2. CRITICAL: CodeQL is SEPARATE from CI — check both independently.
+Loop Phase 2.0 → 2.6 → 3.1 → 3.2 → 3.3 until either:
+
+- All three Phase 3 gates pass → run Phase 4, then Phase 5 reports `Ready for review`.
+- A failed gate has no in-skill fix path (per Phase 3.1) → exit Phase 5 with `Blocked on: <gate>`.
+- `phase_2_iteration` reaches 5 → exit Phase 5 with `Blocked on: iteration-cap` and emit the state dump from Step 2.0.
+- A `codeql_noop_count` or `threads_noop_count` reaches 2 → exit Phase 5 with `Blocked on: <fix type>`.
+
+Pending checks (`mergeStateStatus = UNKNOWN`, only PENDING checks in `UNSTABLE`,
+or `statusCheckRollup.state = PENDING`) do NOT increment the iteration counter —
+re-run Phase 2.6 and re-evaluate Phase 3.
 
 **MERGE PROHIBITION**: FORBIDDEN from merging, auto-merging, enabling auto-merge, or approving any PR.
 
@@ -270,3 +349,4 @@ For `all`/`org` modes: Phases 2-5 loop per PR, Phase 6 aggregates results.
 - pr-standards (git-standards) — PR authoring and review standards
 - code-quality-standards (code-standards) — code quality guidelines applied during fixes
 - gh-cli-patterns (github-workflows) — canonical gh CLI command shapes, placeholder convention, PR gate, code-scanning query
+- verification-before-completion (superpowers) — iron-law verification before claiming work complete

github-workflows/skills/gh-cli-patterns/SKILL.md

@@ -250,6 +250,26 @@ Append after the URL, separated by ` | `. Omit when no issues exist ("Ready for
 gh pr view <PR_NUMBER> --json url --jq '.url'
 ```
 
+**Canonical Unresolved Thread Count** (post-fix verification after
+`/resolve-pr-threads`):
+
+```bash
+QUERY='query($owner:String!,$repo:String!,$prNumber:Int!){
+  repository(owner:$owner,name:$repo){
+    pullRequest(number:$prNumber){
+      reviewThreads(first:100){nodes{id isResolved}}
+    }
+  }
+}'
+gh api graphql -f query="$QUERY" \
+  -f owner="<OWNER>" -f repo="<REPO>" -F prNumber=<PR_NUMBER> \
+  --jq '[.data.repository.pullRequest.reviewThreads.nodes[]
+         | select(.isResolved == false)] | length'
+```
+
+Caller compares this against the pre-fix count: strict decrease → continue,
+unchanged → subagent did nothing.
+
 **Fetch all open PRs** (for Section 2 — one GraphQL call per affected repo):
 
 `gh pr list --json` does NOT support `mergeStateStatus` — use GraphQL instead:

github-workflows/skills/ship/SKILL.md

@@ -177,18 +177,40 @@ any `reviewThreads.isResolved` = `false`,
 `reviewDecision` = `CHANGES_REQUESTED`/`REVIEW_REQUIRED`,
 `statusCheckRollup.state` ≠ `SUCCESS`, or CodeQL count > 0.
 
-If any abort condition hits: re-invoke `/finalize-pr <PR_NUMBER>`, wait for completion,
-then re-run both gates. Only list a PR as "Ready to merge" after both gates pass.
+### 3.1 Post-Finalize Wait (REQUIRED before re-verify)
 
-Then emit the **Canonical PR Status Summary** as defined in /gh-cli-patterns, titled
-`Ship Summary`. Affected repos = current repo. Fetch each PR's full URL via:
+If the most recent commit on the PR's `headRefName` landed within the last 60 seconds:
 
 ```bash
-gh pr view <PR_NUMBER> --json url --jq '.url'
+lastCommitTs=$(gh api repos/<OWNER>/<REPO>/commits/<HEAD_SHA> --jq '.commit.committer.date | fromdate')
+now=$(date +%s)
+[ $((now - lastCommitTs)) -lt 60 ] && sleep $((60 - (now - lastCommitTs)))
 ```
 
-Section 1 lists the PRs targeted by this `/ship` invocation. Section 2 lists all open
-PRs in the current repo (including unrelated ones).
+GitHub recomputes `mergeStateStatus` and branch protection asynchronously after a push.
+A gate query within the first 60 seconds frequently returns `UNKNOWN` or stale state.
+Waiting once here is dramatically cheaper than burning a `/finalize-pr` retry on a
+transient pending check.
+
+### 3.2 Retry Cap
+
+If any abort condition hits: re-invoke `/finalize-pr <PR_NUMBER>`, wait for completion,
+then re-run both gates.
+
+**Hard cap: 3 re-invocations per PR per `/ship` invocation.** After the 3rd, do NOT
+silently report the PR as "Ready to merge." Instead, categorize the result and report
+it explicitly per Step 3.3 below.
+
+### 3.3 Surface Per-PR Result
+
+Surface whatever `/finalize-pr` reported. Do not re-classify. If `/finalize-pr`
+returned `Blocked on: <gate>`, the Section 1 row includes that exact string and
+states the single manual action that would unblock it.
+
+Then emit the **Canonical PR Status Summary** as defined in /gh-cli-patterns,
+titled `Ship Summary`. Affected repos = current repo. Section 1 lists the PRs
+targeted by this `/ship` invocation. Section 2 lists all open PRs in the
+current repo (including unrelated ones).
 
 ## Safety