Skip to content

Carry forward #1084's unshipped review fix: hedge the mechanism claim #1086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
d-morrison merged 1 commit into from
Aug 3, 2026
Merged

Carry forward #1084's unshipped review fix: hedge the mechanism claim #1086

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions shared/workflow/pr-on-claim.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,25 @@ A POST response alone is not enough; if the pending request disappears and no cu
Do not leave it as "review owed".

**That blocked-request test has a false positive, and it is on exactly the repos the section above describes.**
Where a ruleset auto-requests Copilot, the pending request disappears *by design*: the POST returns success naming the reviewer, and `reviewRequests` reads **empty** moments later, because the auto-management consumes it.
Where a ruleset auto-requests Copilot, the POST returns success naming the reviewer and `reviewRequests` reads **empty** moments later.
That is the literal signature the test above calls blocked --- request gone, no review yet --- so the two paragraphs contradict each other on any repo with `review_on_push: true`, and the earlier one is the one that is wrong there.

The empty read is observed, on two repos; *why* it comes back empty is not.
[`memories/github.md`](../../memories/github.md) records the same
201-then-empty sequence and offers auto-requesting as the **likeliest
reconciliation, explicitly untested** --- deliberately so, since probing it
consumes the per-user quota that is usually the real reason Copilot is absent.
Keep that hedge: what matters operationally is that an empty pending-list is
uninformative on such a repo, which holds whatever the mechanism turns out to
be.

Reading it as blocked costs more than a wasted call.
It routes you to the self-review fallback while a working reviewer is queued, which [`fully-clean`](fully-clean.md) treats as a fallback for when *no* external reviewer is reachable --- so the PR ends up carrying a weaker verdict than it could have.

Settle it by reading the ruleset rather than by polling harder:
Settle it by reading the ruleset rather than by polling harder.
[`memories/github.md`](../../memories/github.md) gives the single-ruleset form;
this loop is the same query when you do not already know the id, so keep the
two in sync if either changes:

```bash
for id in $(gh api "repos/<owner>/<repo>/rulesets" --jq '.[].id'); do
Expand Down
Loading