fix(pr-management-triage): stop misclassifying PRs on truncated and unsettled GitHub data

skills/pr-management-triage/actions.md

@@ -362,7 +362,10 @@ executed.
 # with `conclusion: "action_required"`. The query parameter
 # `?status=action_required` matches no runs and would silently
 # return an empty result — post-filter on `conclusion` instead.
-head_sha=$(gh api "repos/<owner>/<repo>/pulls/<N>" --jq '.head.sha')
+# One fetch covers both guards.
+read -r head_sha merge_state <<<"$(gh api "repos/<owner>/<repo>/pulls/<N>" \
+  --jq '"\(.head.sha) \(.mergeable_state)"')"
+
 pending=$(gh api "repos/<owner>/<repo>/actions/runs?head_sha=${head_sha}&per_page=20" \
   --jq '[.workflow_runs[] | select(.conclusion == "action_required")] | length')
 if [ "$pending" -gt 0 ]; then
@@ -371,10 +374,29 @@ if [ "$pending" -gt 0 ]; then
   exit 2
 fi
 
-# Guard passed — apply the label.
+# Mergeability guard — GraphQL `mergeable` is computed lazily and
+# reports UNKNOWN until a background job settles it, so a PR can
+# classify as `passing` and be conflicting by the time we mutate.
+# `mergeable_state == dirty` is the REST spelling of CONFLICTING.
+if [ "$merge_state" = "dirty" ]; then
+  echo "refuse mark-ready: <N> is conflicting — route to draft instead" >&2
+  exit 2
+fi
+if [ "$merge_state" = "unknown" ]; then
+  echo "refuse mark-ready: <N> mergeability not yet computed — retry next sweep" >&2
+  exit 2
+fi
+
+# Guards passed — apply the label.
 gh pr edit <N> --repo <repo> --add-label "ready for maintainer review"
 ```
 
+When the mergeability guard refuses with `dirty`, the PR belongs
+to row 9 (`mergeable == CONFLICTING` → `draft`) — route it there
+rather than dropping it. On a full sweep of a large `<upstream>`
+this guard refused **11 of 39** `mark-ready` candidates, every one
+genuinely conflicting despite reporting `UNKNOWN` at fetch time.
+
 When the guard refuses, the implementation should **reclassify
 the PR as `pending_workflow_approval`** (see
 [`classify-and-act.md#decision-table`](classify-and-act.md), row 1) and
@@ -592,6 +614,14 @@ if [ "$merg" = "CONFLICTING" ]; then
   echo "refuse: CONFLICTING — route to draft instead" >&2
   exit 2
 fi
+# Same lazy-computation caveat as the mark-ready guard: this live
+# re-query can itself return UNKNOWN, and UNKNOWN is not "no
+# conflict". Proceeding spends a round-trip that 422s on exactly the
+# PRs this guard exists to catch.
+if [ "$merg" = "UNKNOWN" ]; then
+  echo "refuse: mergeability not yet computed — retry next sweep" >&2
+  exit 2
+fi
 ```
 
 When the guard passes, single mutation via `gh`:

skills/pr-management-triage/classify-and-act.md

@@ -100,10 +100,10 @@ Action verbs are defined in [`actions.md`](actions.md).
 | 16 | No real CI ran (see [Real-CI guard](#real-ci-guard)) AND `mergeable != CONFLICTING` AND author NOT first-time | `deterministic_flag` | `rebase`            | No real CI checks triggered, branch mergeable — rebase to re-trigger |
 | 17 | [`has_deterministic_signal`](#has_deterministic_signal) (fallback)                             | `deterministic_flag`       | `draft`                | Has quality issues — convert to draft with violations comment |
 | 18 | `latestReviews` has CHANGES_REQUESTED AND author committed after AND NOT [`follow_up_ping`](#follow_up_ping) | `stale_review`         | `ping`                 | Author pushed commits after CHANGES_REQUESTED from <reviewers> but no follow-up — ping |
-| 19 | All of: `statusCheckRollup.state == SUCCESS`, `mergeable != CONFLICTING`, no unresolved **collaborator** threads (see [`unresolved_threads_only`](#unresolved_threads_only) for the collaborator-author qualifier), [Real-CI guard](#real-ci-guard) passes, label `ready for maintainer review` already present | `passing` | `skip` | Already marked ready for review |
-| 20 | All of: `statusCheckRollup.state == SUCCESS`, `mergeable != CONFLICTING`, no unresolved **collaborator** threads (see [`unresolved_threads_only`](#unresolved_threads_only) for the collaborator-author qualifier), [Real-CI guard](#real-ci-guard) passes | `passing` | `mark-ready` | All checks green, no conflicts, no unresolved collaborator threads — mark for deeper review |
+| 19 | All of: `statusCheckRollup.state == SUCCESS`, `mergeable == MERGEABLE` (**not** merely `!= CONFLICTING` — see [hard rules](#hard-rules-cross-cutting-the-table)), no unresolved **collaborator** threads (see [`unresolved_threads_only`](#unresolved_threads_only) for the collaborator-author qualifier), [Real-CI guard](#real-ci-guard) passes, label `ready for maintainer review` already present | `passing` | `skip` | Already marked ready for review |
+| 20 | All of: `statusCheckRollup.state == SUCCESS`, `mergeable == MERGEABLE` (**not** merely `!= CONFLICTING` — see [hard rules](#hard-rules-cross-cutting-the-table)), no unresolved **collaborator** threads (see [`unresolved_threads_only`](#unresolved_threads_only) for the collaborator-author qualifier), [Real-CI guard](#real-ci-guard) passes | `passing` | `mark-ready` | All checks green, no conflicts, no unresolved collaborator threads — mark for deeper review |
 | 21 | Stale-sweep candidate (see [`stale-sweeps.md`](stale-sweeps.md)) AND no row 1–20 matched in this session | `stale_draft` / `inactive_open` / `stale_workflow_approval` | (per sweep) | (per sweep) |
-| 22 | Data inconsistency: rollup `SUCCESS` with `failed_checks` non-empty, OR rollup `FAILURE` with `failed_checks` empty (e.g. only CANCELLED contexts visible, or rollup hasn't yet propagated the failing check-run). Evaluated **before** rows 17, 19-20 — see [hard rules](#hard-rules-cross-cutting-the-table) | n/a | `skip` | Data anomaly — rollup not yet settled, retry next page |
+| 22 | Unsettled server-side state, either: (a) data inconsistency — rollup `SUCCESS` with `failed_checks` non-empty, OR rollup `FAILURE` with `failed_checks` empty (e.g. only CANCELLED contexts visible, or rollup hasn't yet propagated the failing check-run); or (b) `mergeable == UNKNOWN` — GitHub has not finished computing mergeability. Evaluated **before** rows 17, 19-20 — see [hard rules](#hard-rules-cross-cutting-the-table) | n/a | `skip` | State not yet settled, retry next sweep |
 
 ### Hard rules cross-cutting the table
 
@@ -116,6 +116,52 @@ Action verbs are defined in [`actions.md`](actions.md).
   [`mark-ready` action](actions.md#mark-ready--add-ready-for-maintainer-review-label) re-checks the
   REST `action_required` index immediately before mutating
   (Golden rule 1b in [`SKILL.md`](SKILL.md)).
+- **`mergeable == UNKNOWN` is not "no conflict".** GitHub
+  computes mergeability lazily: the first query after a base-branch
+  move returns `UNKNOWN` while a background job runs. Written as
+  `mergeable != CONFLICTING`, rows 19/20 evaluate **true** for
+  `UNKNOWN` — so an unsettled PR reads as green and earns
+  `ready for maintainer review`.
+  Treat `UNKNOWN` as *undetermined*, never as *mergeable*:
+  - Rows 19/20 require `mergeable == MERGEABLE` explicitly, and
+    **row 22 catches the `UNKNOWN` case** — without that the PR
+    would match no row at all, since `UNKNOWN` also fails
+    [`has_deterministic_signal`](#has_deterministic_signal) and so
+    never reaches the row 17 fallback. Row 22 is the right home:
+    it already means *"server-side state not yet settled, retry"*
+    and is already evaluated before rows 17 and 19-20. The PR
+    skips with reason *"mergeability not yet computed — retry next
+    sweep"*; GitHub settles it within seconds and the next sweep
+    classifies it properly.
+  - The [`mark-ready` action](actions.md#mark-ready--add-ready-for-maintainer-review-label)
+    re-reads `mergeable_state` from the REST PR object immediately
+    before applying the label and refuses on `dirty`, in the same
+    pre-mutation block as the `action_required` check.
+
+  F4 keeps the looser `!= CONFLICTING` deliberately: it only
+  decides whether an *already-labelled* PR is skipped, so an
+  `UNKNOWN` there costs one sweep of delay rather than a wrong
+  label.
+
+  Row 16 also keeps `!= CONFLICTING`, but for a different reason:
+  it routes to `rebase`, whose own
+  [pre-flight guard](actions.md#rebase--update-the-pr-branch-with-base)
+  re-queries `mergeable` live and refuses on both `CONFLICTING`
+  and `UNKNOWN`. The classification stays loose because the
+  mutation is guarded; the guard has to handle `UNKNOWN` for that
+  to hold, since the live re-query is subject to the same lazy
+  computation as the batch fetch.
+
+  `unresolved_threads_only` also reads `!= CONFLICTING`. That one
+  is diagnostic — it decides which *reason* is reported, not which
+  action fires — so an `UNKNOWN` mislabels a reason string rather
+  than producing a wrong outcome.
+
+  Observed on a full sweep of a large `<upstream>`: **11 of 39**
+  `mark-ready` candidates reported `UNKNOWN` at fetch time and
+  `dirty` at mutation time. All 11 were genuinely conflicting; the
+  pre-mutation guard refused every one. Without that guard they
+  would have entered the maintainer review queue unmergeable.
 - **Collaborator-authored PRs never get `draft`.** When
   `authors:collaborators` is active, fall back to `comment` with
   the same body. Row 9 / 17 / etc. emit `comment`, not `draft`,

skills/pr-management-triage/fetch-and-batch.md

@@ -53,6 +53,15 @@ query(
               committedDate
               statusCheckRollup {
                 state          # SUCCESS / FAILURE / PENDING / ERROR
+                # NOTE: this page is TRUNCATED on large repos and the
+                # truncation is silent — see #failed-check-lists-are-truncated.
+                # `state` is always authoritative; the derived
+                # failed-check *list* is not, and must be re-derived from
+                # the check-runs REST API before any row that reads it.
+                # Kept at 50 deliberately: raising it cannot close the
+                # truncation window, and the REST re-derivation makes this
+                # page a fast path rather than a source of truth — so the
+                # extra complexity would buy nothing. See #batch-size.
                 contexts(first: 50) {
                   nodes {
                     __typename
@@ -180,6 +189,78 @@ the rate-limit budget. The inner `first:` arguments are the
 dominant factor; if you need to widen them, *lower* the outer
 batch size first — never raise above 25 without measuring.
 
+Note the interaction with
+[Failed-check lists are truncated](#failed-check-lists-are-truncated):
+trimming `contexts(first:)` to buy complexity headroom widens
+that truncation window. That is an acceptable trade **only**
+because the REST re-derivation is mandatory before any row reads
+`failed_checks` — the rollup page is a fast path, never the
+source of truth.
+
+The same reasoning cuts the other way, and is why `contexts(first:)`
+stays at 50 rather than being raised: a larger page cannot close the
+truncation window either (a repo can always exceed any fixed page),
+so raising it would spend complexity budget — the dominant factor
+per the paragraph above — to buy a list nothing is allowed to trust.
+Widen it only alongside a measured `cost=` figure and a matching
+reduction in `$batchSize`.
+
+### Failed-check lists are truncated
+
+**`statusCheckRollup.contexts` is a paginated connection, and the
+page it returns is a silent prefix — not the whole set.** A
+`<upstream>` whose PRs run more check-runs than the page holds (a
+large matrix-heavy CI easily does) overflows it. Nothing in the
+response signals the truncation: you get a well-formed list that
+happens to be missing entries.
+
+`statusCheckRollup.state` is unaffected — it is computed
+server-side over *all* contexts, so `SUCCESS` / `FAILURE` stays
+authoritative. What is **not** authoritative is the derived
+`failed_checks` list, and that list is what the decision table
+reads for every CI-shaped row.
+
+Observed on a full 333-PR sweep of a large `<upstream>` at
+`first: 50`, three PRs were misrouted by the truncated list:
+
+| Failures per rollup page | Actual failures | Effect |
+|---|---|---|
+| 1 (a static check) | 16, including a whole provider-test sweep | routed to `comment` instead of `draft` |
+| 2 | 4 — the same suite failing on **all four** DB backends | routed to `rerun`; a consistent cross-backend failure treated as a flake |
+| 1 | 3 | routed to `comment` instead of `draft` |
+
+Raising the page size shrinks the window but does not close it —
+a repo can always exceed it. **Before evaluating any row that
+reads `failed_checks`** (rows 10, 11, 12, 12b, 13, and the
+[Real-CI guard](classify-and-act.md#real-ci-guard)), re-derive the
+list from the paginated check-runs REST endpoint:
+
+```bash
+# Walk every page — stop when a page returns < 100 entries.
+# Fetch each page ONCE and derive both the failure names and the page
+# length from that single response; re-querying the same page to count
+# it doubles the call budget this section advertises.
+page=1
+while :; do
+  page_json=$(gh api "repos/<owner>/<repo>/commits/${head_sha}/check-runs?per_page=100&page=${page}")
+  jq -r '.check_runs[] | select(.conclusion == "failure" or .conclusion == "timed_out") | .name' \
+    <<<"$page_json"
+  [ "$(jq '.check_runs | length' <<<"$page_json")" -lt 100 ] && break
+  page=$((page + 1))
+done
+```
+
+Cost is one REST call per ~100 check-runs per PR, and only for
+PRs whose `rollup.state` is `FAILURE` — green PRs skip it
+entirely. On the 333-PR sweep above that was ~50 extra calls,
+negligible against the 5000/h budget and far cheaper than posting
+a wrong violations list to a contributor.
+
+**Do not** report a violations list built from the rollup page
+alone. A contributor told they have "one lint failure" when they
+have sixteen will fix the lint, push, and land back in triage —
+having been actively misled by us.
+
 ### `gh` invocation
 
 ```bash
@@ -519,6 +600,11 @@ query($owner: String!, $repo: String!) {
             commit {
               oid
               statusCheckRollup {
+                # Same truncation caveat as the main query, and kept at 50
+                # for the same reason. Here it only under-populates
+                # `recent_main_failures`, which makes rows 10/11 fire less
+                # often — a PR gets `draft`/`comment` instead of `rerun`.
+                # That is the safe direction to fail.
                 contexts(first: 50) {
                   nodes {
                     __typename

skills/pr-management-triage/stale-sweeps.md

@@ -154,7 +154,7 @@ above.
 
 Two sub-cases, both resulting in `close`:
 
-### 1a. Triaged draft with no author reply ≥ 7 days
+### 1a. Triaged draft with no author response ≥ 7 days
 
 **Trigger.**
 
@@ -163,13 +163,34 @@ Two sub-cases, both resulting in `close`:
   [Ready-label exclusion](#ready-label-exclusion-applies-to-sweeps-13) — Sweep 4's domain)
 - `last_triage_comment_at` is not null
 - `<now> - last_triage_comment_at >= 7 days`
-- No comment by the author after `last_triage_comment_at`
+- `last_author_activity_at <= last_triage_comment_at` — i.e. **no
+  author response of any kind** since we asked. Use the
+  [`last_author_activity_at`](#inputs) input defined above, which
+  already folds in pushes and review-thread replies alongside
+  issue comments.
+
+**A push is a response.** Many contributors answer review feedback
+with code and never write a comment. Testing this trigger against
+*comments only* marks those authors silent while they are actively
+working — and this sweep's action is `close`, the least reversible
+thing the skill does.
+
+Observed on a full sweep of a large `<upstream>`: of 3 PRs
+matching a comments-only reading of this trigger, **one had author
+pushes 5, 12, and 25 days after the triage comment** — actively
+worked, zero comments. It would have been closed. A second had
+last pushed 32 days earlier; only the third was genuinely
+silent.
+
+The `last_author_activity_at` input exists precisely for this and
+costs no extra fetch — the trigger must not fall back to a
+bare comment scan.
 
 **Action.** `close` — post the
 [stale-draft-close](comment-templates.md#stale-draft-close) comment,
 then close. No label (these are not quality-violation closes).
 
-**Reason string.** *"Draft triaged N days ago, no author reply — close with stale-draft notice"*.
+**Reason string.** *"Draft triaged N days ago, no author reply or push — close with stale-draft notice"*.
 
 ### 1b. Untriaged draft with no activity ≥ 2 weeks
 

tools/skill-evals/evals/pr-management-triage/decision-table/fixtures/assertions.json

@@ -1,3 +1,4 @@
 {
-  "has_reason_no_real_ci": {"type": "regex", "field": "reason", "pattern": "(no|not|without).{0,30}(real )?ci|ci.{0,20}(not|never).{0,20}(trigger|ran|run)|re-?trigger", "flags": "i"}
+  "has_reason_no_real_ci": {"type": "regex", "field": "reason", "pattern": "(no|not|without).{0,30}(real )?ci|ci.{0,20}(not|never).{0,20}(trigger|ran|run)|re-?trigger", "flags": "i"},
+  "has_reason_mergeability_unsettled": {"type": "regex", "field": "reason", "pattern": "unknown|not (yet )?(been )?(computed|determined|settled)|undetermined|unsettled|still computing|retry next sweep", "flags": "i"}
 }

tools/skill-evals/evals/pr-management-triage/decision-table/fixtures/case-19-unknown-mergeable/expected.json

@@ -0,0 +1,5 @@
+{
+  "classification": null,
+  "action": "skip",
+  "has_reason_mergeability_unsettled": true
+}

tools/skill-evals/evals/pr-management-triage/decision-table/fixtures/case-19-unknown-mergeable/report.md

@@ -0,0 +1,15 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+PR #14931
+Author: rowan-contributor
+AuthorAssociation: CONTRIBUTOR
+StatusCheckRollup: SUCCESS
+FailedChecks: []
+RecentMainFailures: []
+Mergeable: UNKNOWN
+UnresolvedThreads: 0
+IsDraft: false
+CommitsBehind: 3
+RealCIRan: true
+Labels: ["area:core"]

tools/skill-evals/evals/pr-management-triage/decision-table/fixtures/system-prompt.md

@@ -72,10 +72,14 @@ and reason.
 | 16 | No real CI checks triggered AND `mergeable != CONFLICTING` AND author is NOT first-time (`authorAssociation` NOT IN {`FIRST_TIME_CONTRIBUTOR`, `FIRST_TIMER`}) | `deterministic_flag` | `rebase` |
 | 17 | `has_deterministic_signal` (fallback) | `deterministic_flag` | `draft` |
 | 18 | `latestReviews` has CHANGES_REQUESTED AND author pushed commits after that review AND NOT `follow_up_ping` | `stale_review` | `ping` |
-| 19 | `statusCheckRollup == SUCCESS` AND `mergeable != CONFLICTING` AND `unresolved_threads == 0` AND real CI ran AND labels contain `ready for maintainer review` | `passing` | `skip` |
-| 20 | `statusCheckRollup == SUCCESS` AND `mergeable != CONFLICTING` AND `unresolved_threads == 0` AND real CI ran | `passing` | `mark-ready` |
+| 19 | `statusCheckRollup == SUCCESS` AND `mergeable == MERGEABLE` AND `unresolved_threads == 0` AND real CI ran AND labels contain `ready for maintainer review` | `passing` | `skip` |
+| 20 | `statusCheckRollup == SUCCESS` AND `mergeable == MERGEABLE` AND `unresolved_threads == 0` AND real CI ran | `passing` | `mark-ready` |
 | 21 | Stale sweep candidate — no row 1–20 matched AND PR meets stale criteria: `isDraft == true` AND triage marker exists AND `(now - triage_comment_at) >= 7 days` AND `(now - last_author_activity) >= 14 days` | `stale_draft` | `close` |
-| 22 | Data anomaly — `statusCheckRollup == SUCCESS` but `failed_checks` is non-empty, OR `statusCheckRollup == FAILURE` but `failed_checks` is empty. Evaluated before rows 17, 19, 20. | n/a | `skip` |
+| 22 | Unsettled server-side state — `statusCheckRollup == SUCCESS` but `failed_checks` is non-empty, OR `statusCheckRollup == FAILURE` but `failed_checks` is empty, OR `mergeable == UNKNOWN`. Evaluated before rows 17, 19, 20. | n/a | `skip` |
+
+Note on rows 19/20: `mergeable == UNKNOWN` means GitHub has not
+finished computing mergeability. It is **not** the same as "no
+conflict" — treat it as undetermined and let row 22 handle it.
 
 ## Output