chore(skills): teach the PR skills that an open thread blocks the merge

.claude/skills/collapse-pr/SKILL.md

@@ -394,11 +394,13 @@ if [ -n "$PR_NUM" ]; then
   OPEN_THREADS="$(gh api graphql -f query='
   query($owner:String!,$name:String!,$pr:Int!){
     repository(owner:$owner,name:$name){
-      pullRequest(number:$pr){ reviewThreads(first:100){ nodes{ isResolved path line } } }
+      pullRequest(number:$pr){ reviewThreads(first:100){
+        pageInfo{ hasNextPage } nodes{ isResolved path line } } }
     }
   }' -f owner="$OWNER" -f name="$NAME" -F pr="$PR_NUM" \
-    --jq '[.data.repository.pullRequest.reviewThreads.nodes[]
-           | select(.isResolved == false)] | length')"
+    --jq '.data.repository.pullRequest.reviewThreads
+          | if .pageInfo.hasNextPage then error("more than 100 review threads — paginate before trusting this count")
+            else [.nodes[] | select(.isResolved == false)] | length end' || echo unknown)"
 fi
 ```
 
@@ -411,6 +413,14 @@ whole branch and costs them that diff.* This is `revise-pr` Step 5.1's rule; the
 decision of whether this round is the final one belongs to the caller, which is
 why the gate is soft.
 
+**Print the count even when the caller passes `--yes`, and say it holds the merge.**
+`main` carries `required_conversation_resolution: true`, so every one of these threads
+blocks the merge until someone resolves it — a collapsed, queue-ready branch with three
+open threads is not mergeable, and a caller who read only "collapsed ✓" will believe it
+is. The gate stays **soft** regardless: collapsing is not merging, and a final-round
+collapse alongside a thread the reviewer still owes an answer on is a legitimate state.
+Report it; never decide it here.
+
 #### 3c — Branch ownership (hard)
 
 ```bash

.claude/skills/pr-autopilot/SKILL.md

@@ -13,7 +13,8 @@ conflict, or a review comment back and forth by hand:
 ```
 resolve conflicts → loop( /pr-review → /revise-pr ) until bot-clean
   → /pr-ready (solicit a human) → read the outcome
-      ├─ approved, clean            → done: report MERGE READY, stop
+      ├─ approved, 0 open threads   → done: report MERGE READY, stop
+      ├─ approved, threads open     → /revise-pr to close them, then re-check
       ├─ changes requested          → fold back through the bot loop, re-solicit
       └─ silent / still reviewing   → done for this run: report the state, stop
 ```
@@ -174,11 +175,20 @@ From `/pr-ready`'s report for this PR:
 
 | `/pr-ready` state | What this skill does |
 |---|---|
-| `REVIEWED (APPROVED)`, mergeable, checks green, no unresolved threads | **MERGE READY** — go to Phase 5 (converged). |
+| `REVIEWED (APPROVED)`, mergeable, checks green, **0 unresolved threads** | **MERGE READY** — go to Phase 5 (converged). |
+| `REVIEWED (APPROVED)` with **≥1 unresolved thread** | Approved but **not mergeable** — `main` requires conversation resolution. Nobody rejected anything, so this does **not** spend a human cycle. Run `/revise-pr <N> [--repo <REPO>]` once, then re-check. See the dismissal note below: if that run *pushed*, the approval is gone, and the pushed branch goes through Phase 1 (recheck conflicts) → Phase 2 (a fresh bot pass, bot-round counter reset to 0) → Phase 3 to re-solicit — the same route the `CHANGES_REQUESTED` row takes after the same push, because a human must not be asked to re-approve code no bot pass or conflict check has seen; if it closed the threads with replies alone, the approval stands and this goes straight to **MERGE READY**. Allow this path **once per human cycle** — a second pass through it with threads still open ends the run as **APPROVED · THREADS OPEN** (Phase 5), not a third attempt. It is not BOT STUCK: the usual cause is a `/revise-pr` pushback, which that skill leaves open for the reviewer by rule, so nothing is wrong with the bot and the next move is whoever each thread is waiting on. |
 | `REVIEWED (CHANGES_REQUESTED)` | A human found something. Increment the **human-cycle counter**. If `≤ --max-human-cycles`: **run `/revise-pr <N> [--repo <REPO>]` directly** — not `/pr-review` first (see below) — reset the bot-round counter to 0, then go through Phase 1 (recheck conflicts) → Phase 2 (a fresh bot pass — now meaningful, since the human's thread is already addressed) → Phase 3 again to re-solicit. If the counter now exceeds the bound: **HUMAN STUCK** — go to Phase 5 (stuck). |
 | `SILENT`, `ACKED`, `REVIEWING`, or `HELD BUT STALE` | Nobody rejected anything — there is nothing for the bot loop to act on, and `/pr-ready`'s own wait for this run is already over. Go to Phase 5 and report this state as-is; re-running `/pr-autopilot` later will pick it up. |
 | `REVIEWED (COMMENTED)` with no `CHANGES_REQUESTED` from anyone | Not a blocking review. Treat like the row above — report and stop; don't manufacture a revision cycle out of a non-blocking comment. |
 
+**A push after an approval dismisses it — `main` has `dismiss_stale_reviews: true`.** That
+is why the approved-with-threads row branches on whether `/revise-pr` pushed rather than on
+whether it "succeeded": a run that only replied and resolved leaves the approval intact and
+converges immediately, while a run that committed a fix has silently put the PR back to
+`REVIEW_REQUIRED`. Read `reviewDecision` after `/revise-pr` returns rather than assuming
+either outcome — treating a dismissed approval as still-standing is how this skill would
+report `MERGE READY` on a PR that needs a whole new reviewer.
+
 **Why `/revise-pr` runs first, not Phase 2's `/pr-review`.** `/pr-review` never
 reads existing review threads — its findings come from the issue, the diff,
 and the offlinecv gates alone (see its own Steps 0–4). Re-entering through
@@ -212,11 +222,56 @@ approved, no conflicts, checks green, no unresolved threads. Merge is yours
 whenever you're ready.
 ```
 
-**BOT STUCK / HUMAN STUCK / not-yet-engaged:** post the equivalent short status
-(round/cycle count, what's outstanding, and — for the not-yet-engaged case — that
-`/pr-ready`'s wait already ran once and re-running `/pr-autopilot` will check
-again) and say so in the session. Don't dress up an unresolved state as
-converged.
+**Verify that last clause before posting it; never carry it over from Phase 4.** Every
+claim in the line is re-checkable in one call, and the thread count is the one that moves
+most — `/revise-pr` resolving threads, or a reviewer opening new ones, both land between
+the two phases:
+
+```bash
+# notGreen uses pr-ready's allowlist: a running check (empty/null conclusion) is not green
+gh pr view <N> --repo <REPO> --json reviewDecision,mergeable,mergeStateStatus,statusCheckRollup \
+  -q '{reviewDecision, mergeable, mergeStateStatus,
+       notGreen: [.statusCheckRollup[]
+                  | select(((.conclusion // .state) as $s | ["SUCCESS","NEUTRAL","SKIPPED"] | index($s)) | not)
+                  | "\(.name // .context): \(.conclusion // .state // "running")"]}'
+gh api graphql -f query='
+  query($owner:String!,$repo:String!,$pr:Int!){
+    repository(owner:$owner,name:$repo){ pullRequest(number:$pr){
+      reviewThreads(first:100){ pageInfo{ hasNextPage } nodes{ isResolved } } } } }' \
+  -f owner=<OWNER> -f repo=<NAME> -F pr=<N> \
+  --jq '.data.repository.pullRequest.reviewThreads
+        | if .pageInfo.hasNextPage then error("more than 100 review threads — paginate before trusting this count")
+          else [.nodes[] | select(.isResolved==false)] | length end'
+```
+
+`<OWNER>`/`<NAME>` are `<REPO>` split on the `/`, and `-F pr=` is typed — `-f` 422s an
+`Int!`. Past 100 threads the jq errors rather than undercounting — a count that decides
+MERGE READY fails closed. The line makes four claims, so four conditions each mean **not
+converged**:
+
+- the thread count is not 0, or `reviewDecision` is no longer `APPROVED` → take the matching
+  Phase 4 row;
+- `mergeable` is not `MERGEABLE` (re-poll `UNKNOWN` up to 5 times, ~5s apart, as
+  `/pr-ready`'s preflight does) → report the conflict;
+- `notGreen` is non-empty → report each check and its state; a check re-run between Phase 4
+  and now is exactly what this catches.
+
+Only when all four pass may the line be posted. `mergeStateStatus` is then the cross-check,
+not a substitute: it reads `BLOCKED` alike for a thread, a dismissed approval and a red
+required check, so it cannot name a cause — but once those have all been checked, a
+`BLOCKED` that remains is a real, unexplained block. Report it with the raw value instead of
+posting MERGE READY over it.
+
+**BOT STUCK / HUMAN STUCK / APPROVED · THREADS OPEN / not-yet-engaged:** post the
+equivalent short status (round/cycle count, what's outstanding, and — for the
+not-yet-engaged case — that `/pr-ready`'s wait already ran once and re-running
+`/pr-autopilot` will check again) and say so in the session. Don't dress up an
+unresolved state as converged.
+
+For **APPROVED · THREADS OPEN**, the outstanding part is a per-thread list, not a count:
+each open thread's `path:line`, who opened it, and whose reply it is waiting on (the
+opener, if the last comment is someone else's; otherwise the PR author). The blocker count
+the STUCK forms report is 0 here by construction, so reporting it would say nothing.
 
 Either way, print the same summary in this session as well — the PR comment is
 for the PR's audience, the session output is for whoever ran this.
@@ -234,6 +289,8 @@ for the PR's audience, the session output is for whoever ran this.
   a human on bot-flagged work.
 - **`--max-human-cycles` reached** → HUMAN STUCK, Phase 5, never re-solicit past
   the bound in the same run.
+- **Approved, and threads still open after the one `/revise-pr` pass** → APPROVED ·
+  THREADS OPEN, Phase 5, each thread named with the person it waits on.
 - **`/pr-ready`'s config is missing** (its own Phase 0 setup) → that failure
   surfaces from Phase 3 verbatim; this skill doesn't have a fallback channel of
   its own.

.claude/skills/pr-ready/SKILL.md

@@ -212,6 +212,17 @@ against the login string, not the object):
 for PR_NUM in <every PR in the list>; do
   gh pr view "$PR_NUM" --repo "$REPO" \
     --json state,mergeable,statusCheckRollup,commits,author,url,title -q '.'
+
+  # unresolved review threads — a merge gate since `required_conversation_resolution`,
+  # and invisible to every field above (see the note under the table)
+  gh api graphql -f query='
+    query($owner:String!,$repo:String!,$pr:Int!){
+      repository(owner:$owner,name:$repo){ pullRequest(number:$pr){
+        reviewThreads(first:100){ pageInfo{ hasNextPage } nodes{ isResolved } } } } }' \
+    -f owner="${REPO%%/*}" -f repo="${REPO##*/}" -F pr="$PR_NUM" \
+    --jq '.data.repository.pullRequest.reviewThreads
+          | if .pageInfo.hasNextPage then error("more than 100 review threads — paginate before trusting this count")
+            else [.nodes[] | select(.isResolved==false)] | length end'
 done
 AUTHOR=$(gh pr view <the first PR in the list> --repo "$REPO" --json author -q .author.login)
 ```
@@ -249,6 +260,26 @@ is no ask, and the abort message is every PR's reason, not just the first.
 | A check is not green | any `.statusCheckRollup[]` whose `(.conclusion // .state)` is not in `SUCCESS` / `NEUTRAL` / `SKIPPED` | "PR #N has a check that isn't green: `<check name>` is `<conclusion or "still running">` — wait for it or fix it; a red or half-finished PR trains reviewers to skip the ask." |
 | More than one commit | `.commits \| length > 1` | "PR #N has `<n>` commits — collapse to one before asking for review: `/collapse-pr <N>`." |
 
+**Unresolved threads are disclosed, never excluded.** `required_conversation_resolution:
+true` makes an open thread a merge gate, but unlike a red check it is not a sign the PR is
+unready for a reviewer — often it is waiting *on* one. The sibling skills leave threads open
+on purpose: `/pr-review` Step 6.5 leaves a suggestion block for the author's Apply click and
+a question for the author's answer, and `/revise-pr` Step 6 leaves a pushback open for the
+reviewer to adjudicate. Excluding on the count would withhold the ask from exactly the PRs
+that need a reviewer next — and `/pr-autopilot` Phase 3 passes a single PR, so one exclusion
+there is an aborted ask with no Phase 4 row to catch it.
+
+So the count goes in the Phase 2 pre-send print next to each PR (`#N · 2 threads open`), the
+ping is sent unchanged, and Phase 6 re-counts. What it never does is drop the PR.
+
+**No `gh pr view` field reports this.** `.mergeable` is only ever
+`MERGEABLE` / `CONFLICTING` / `UNKNOWN` — it answers "do the diffs conflict", not "does
+branch protection allow the merge", and a PR with five open threads reads `MERGEABLE`.
+`.mergeStateStatus` *does* go `BLOCKED`, but it conflates unresolved threads with a
+missing approval, a failing required check, and a stale branch, so it cannot name the
+cause and must not be reported as if it did — which is why the fetch above does not
+request it. Count the threads directly.
+
 **`UNKNOWN` is not a conflict.** GitHub computes mergeability asynchronously,
 so a freshly-pushed PR — exactly the state `/pr-ready` runs in, right after
 `/open-pr` — commonly reads `UNKNOWN` with zero conflicts. Re-poll the
@@ -698,17 +729,26 @@ Then Phase 6.
 
 Re-evaluate the ack predicate one last time and print a terminal summary: the
 state reached per PR, by whom and through which signal, each PR's current
-`state`/`mergeable`/check status, and the author's options. Stop there. Nothing
-after this phase runs automatically.
+`state`/`mergeable`/check status **and its unresolved-thread count**, and the author's
+options. Stop there. Nothing after this phase runs automatically.
+
+**Re-count the threads here; don't reuse Phase 1's number.** The whole point of the ask
+was to get someone to review, and a review that arrived during the wait opens threads —
+so the count that mattered at preflight is exactly the one most likely to be stale by
+Phase 6. A PR that reaches `REVIEWED (APPROVED)` with open threads is **approved and not
+mergeable**, and reporting only the approval is the failure this sweep exists to fix:
+`/pr-review` leaves open precisely the threads it decided the author owes an answer to,
+so an open thread beside an approval is a worklist, not a contradiction. Say
+`APPROVED · 2 threads open → /revise-pr <N>`, never `APPROVED` alone.
 
 **Report per PR — one row each, never one verdict for the set.**
 
-| PR | State | Who | Signal | Since | State / checks |
+| PR | State | Who | Signal | Since | State / checks / threads |
 |---|---|---|---|---|---|
-| #606 | REVIEWED (`APPROVED`) | `<login>` | submitted review | 15:42 | OPEN / green |
-| #607 | REVIEWING | `<login>` | 👀 on the PR | 16:10 | OPEN / green |
-| #608 | HELD BUT STALE | `<login>` | 👀 at 14:05, grace lapsed 15:05 | — | OPEN / green |
-| #605 | SILENT | — | — | — | OPEN / green |
+| #606 | REVIEWED (`APPROVED`) | `<login>` | submitted review | 15:42 | OPEN / green / **2 open → `/revise-pr 606`** |
+| #607 | REVIEWING | `<login>` | 👀 on the PR | 16:10 | OPEN / green / 0 open |
+| #608 | HELD BUT STALE | `<login>` | 👀 at 14:05, grace lapsed 15:05 | — | OPEN / green / 0 open |
+| #605 | SILENT | — | — | — | OPEN / green / 0 open |
 
 Above the table, on their own line, name the **ask-level** acks — a 👍 or a thread
 reply on the ping. They say someone picked up the ask, but they cannot say which