ums: the blocked-request test false-positives on auto-requesting repos

[d-morrison]

shared/workflow/pr-on-claim.md contains a test and, a few lines later, the condition that makes that test wrong.

The test (line 70): if the pending review request disappears and no current-head review appears after a short poll, treat the reviewer request as blocked and fall back.

The condition (the section immediately above it): some repos carry a ruleset rule copilot_code_review with review_on_push: true, which auto-requests Copilot on every push.

On such a repo the pending request disappears by design — the POST returns success naming the reviewer, and reviewRequests reads empty moments later because the auto-management consumed it. That is the exact signature the test calls blocked, so the two paragraphs contradict each other, and the blocked reading is the wrong one there.

The cost is not a wasted API call. It routes you to the self-review fallback while a working reviewer is queued, which fully-clean.md reserves for when no external reviewer is reachable — so the PR ends up carrying a weaker verdict than it could have.

This PR adds the disambiguating check (read the repo's rulesets for a copilot_code_review rule), a note that the next push re-requests automatically, and a Do/Don't pair.

Evidence

From Morrison-Lab/ai-config#1077, 2026-08-03:

• Two explicit requested_reviewers POSTs each returned ["Copilot"]; reviewRequests read [] within a minute of each. Both were reported mid-session as a possible blocked or silent reviewer.
• That repo's main ruleset (id 17712474) carries copilot_code_review with {"review_draft_pull_requests": false, "review_on_push": true}, so neither request was needed.

Kept deliberately separate: Copilot did also stay silent on #1077 — a 4m11s copilot-pull-request-reviewer check concluded success with zero reviews at that head. That is the distinct third state fully-clean.md already documents, and #1079 covered it well enough this morning that I dropped my planned entry there as a duplicate rather than restating it. The point here is narrower: the empty pending-list was never the evidence for it.

Scope

Touches one file, disjoint from #1077 (batch-merge-and-resolve.md, post-merge/SKILL.md, r-quarto.md), so there is no merge-order constraint between them.

Verification

check-new-line-breaks clean over added lines (it flagged one mid-line semicolon on the first pass, fixed by a reflow); banned-glyph scan clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XcGPxpt5ZqyWvajP8Nse3v

[github-actions]

Claude finished review — View run

---

Code Review — #1084

Scope: Single-file, 28-line addition to shared/workflow/pr-on-claim.md, adding a caveat that the "blocked-request" test for a Copilot review request false-positives on repos where a copilot_code_review ruleset with review_on_push: true auto-manages the pending-request list.

What I checked

• PR state: open, not draft, no prior claude[bot] comment — review applies.
• ASCII punctuation (CLAUDE.md → "no em-dashes or non-ASCII punctuation in source files"): clean. All dash usage is the ASCII --- triple-hyphen form; no curly quotes or other banned glyphs in the added lines.
• Semantic line breaks: each added line carries one sentence/clause; consistent with the file's existing convention and the PR's stated verification.
• Markdown links: [fully-clean](fully-clean.md) resolves (file exists at shared/workflow/fully-clean.md).
• API/technical claim: verified via web search that GitHub's repository-ruleset "Automatically request Copilot code review" rule (copilot_code_review type) with an on-push toggle is a real, documented GitHub feature — not a hallucination. This matches prior corpus evidence already in memories/github.md (probed on ucdavis/bcs), so the new snippet is consistent with previously-established fact rather than inventing new API surface.
• Bash snippet correctness: the two-step for id in $(gh api .../rulesets --jq '.[].id'); do gh api .../rulesets/$id --jq '...'; done is technically correct — GitHub's list-rulesets endpoint returns abbreviated objects without the rules array, so fetching each ruleset by id to get .rules[] is the right approach (not a bug).
• Cited PR (Morrison-Lab/ai-config#1077) exists and is a real, disjoint PR as claimed in the Scope section.

Non-blocking observations (not filed as findings — below the "high signal" bar)

1. Mild overlap with memories/github.md. That file already documents gh api "repos/<o>/<r>/rulesets/<id>" --jq '.rules[] | select(.type=="copilot_code_review") | .parameters', and the paragraph immediately above this diff's insertion already cites it. The new snippet adds real value (enumerating ruleset ids when you don't already have one), so per challenge-redundant-content.md's litmus test ("only flag when consolidating would lose nothing") this doesn't clearly qualify as a duplicate — just worth knowing the two snippets will need to be kept in sync if either changes.
2. Slightly more confident causal language than the corpus's own existing hedge. memories/github.md labels the "POST succeeds, then reviewRequests reads empty" pattern as an untested, "likeliest reconciliation." This PR states flatly that it's "because the auto-management consumes it." The correlational evidence in the PR body (ruleset confirmed present on the repo where both empty-reads were observed) supports the claim, but it's a somewhat stronger assertion than the neighboring file's own epistemic hedge. Not a factual error, just a stylistic tightening worth being aware of.

Neither rises to a blocking issue under this review's high-signal bar (no syntax errors, no wrong logic, no unambiguous CLAUDE.md violation, nothing fabricated).

No --comment argument was provided, so no GitHub comments were posted.

Verdict

Ready for merge.

[github-actions]

💰 Cost: $4.2954 (review) — run

[d-morrison]

ARD round 1

Verdict was Ready for merge with two items filed as "non-blocking observations (not filed as findings) — below the high signal bar." Both Addressed in eedb9529, because a severity label is prioritization rather than a pass, and "not a finding" retires an item more firmly than "nit" does.

#	Observation	Disposition	Detail
1	Overlaps memories/github.md's ruleset snippet	✅ Address	Cited it beside the loop and said to keep the two in sync.
2	States the mechanism more confidently than the corpus's own hedge	✅ Address	Restated as observation; kept the untested hedge.

Observation 2 is the one that mattered, and the reviewer was right in a way worth spelling out. memories/github.md records the same 201-then-empty sequence and calls auto-requesting the "likeliest reconciliation, untested" — untested on purpose, since probing consumes the per-user quota that is usually the actual reason Copilot is absent. My text asserted "because the auto-management consumes it" flatly.

That is a cause claim, which shared/workflow/metacognitive-monitoring.md names as one of the four types owing a check ("ask what else explains it"). My evidence was correlational — ruleset present on the repo where the empty reads happened — and I wrote it as mechanism. Worse, I contradicted an existing hedge in this same corpus, which is the failure mode where two fragments start disagreeing about the same fact.

The fix keeps the observation (empty read, two repos) and the hedge (mechanism unestablished), and notes the operational advice does not depend on resolving it: an empty pending-list is uninformative on such a repo whatever the mechanism is.

One correction to my own PR body while I am here: it said the ruleset "explains" the empty read. It is consistent with it; it does not establish it.

Self-review before pushing: check-new-line-breaks clean, glyph scan clean.

The push carries the re-review.

[d-morrison]

Heads-up: the fix for this PR's own review findings did not ship in the merge.

The ARD summary posted at 04:16:36Z and fde4eb0a merged at 04:17:04Z, 28 seconds later, without commit eedb9529. Verified against main rather than inferred: git show origin/main:shared/workflow/pr-on-claim.md | grep -c 'explicitly untested' returns 0.

So the merged text still asserts the mechanism flatly ("because the auto-management consumes it") where memories/github.md marks it explicitly untested. Carried forward as #1086.