From b15f2862bfe280e013c6d814bf0833aca3263cd2 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 16:40:36 -0700 Subject: [PATCH 1/3] Restore the Caveat That a Review-Present Exit 0 Reads No Check The fourth promotion review of #3484 found the rewrite of the exit 0 text dropping its caveat that exit 0 does not prove the required checks ran, though a review-present exit 0 reads no check at all. The help and the README's exit 0 row say so again. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index eeae0d7a..a9e114d1 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -199,7 +199,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj | Code | Command | Meaning | | --- | --- | --- | -| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. | +| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. A review present reads no check, so neither proves the required checks ran. | | `30` | `wait` | Still pending at the timeout, which is pending rather than failed. On a held head, `status=CHECKS_PENDING`. A round past `--min-rounds` not yet landed. | | `40` | `wait` | No Copilot review covers the head: the reviewer answered outside a formal review, the request returned success and recorded nothing, or the reviewer writes under a login no query filters on. | | `41` | `wait` | The newest round is a refusal or a quota state, its kind printed in the status line, and in `refusal=` for a refusal on this pull request. | diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 5a47703a..87bd4165 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -122,7 +122,8 @@ DIRTY, none posted. A DIRTY or BEHIND one also exits 45 where a required check is unregistered or from another app. Exit 0 can then leave a required check unregistered only under unread rules, a `CHECKS TRUNCATED` rollup, a DRAFT merge, or a pull request - not open. + not open. Exit 0 on a review present reads no check, so it does not prove the required + checks ran. 30 = still pending at the timeout. On a held head it is any hold `status=CHECKS_PENDING` names, a required check still settling and a workflow run still open among them. Where the rollup holds a type this cannot read, a BLOCKED hold's note From 45e6d715e437bfd7187251765733d90cf32532b1 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 16:42:35 -0700 Subject: [PATCH 2/3] Say a Review-Present Exit 0 Does Not Poll the Checks The local pass found "reads no check" false, since the review-present path still reads the rollup for exit 44, and the help's completed-runs claim unscoped though only a held head polls its runs. The caveat now says it does not poll the checks, the run claim names the held head, and the README states the caveat for the review-present case alone. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index a9e114d1..59af2b54 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -199,7 +199,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj | Code | Command | Meaning | | --- | --- | --- | -| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. A review present reads no check, so neither proves the required checks ran. | +| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. Exit `0` on a review present does not poll the checks, so it does not prove the required checks ran. | | `30` | `wait` | Still pending at the timeout, which is pending rather than failed. On a held head, `status=CHECKS_PENDING`. A round past `--min-rounds` not yet landed. | | `40` | `wait` | No Copilot review covers the head: the reviewer answered outside a formal review, the request returned success and recorded nothing, or the reviewer writes under a login no query filters on. | | `41` | `wait` | The newest round is a refusal or a quota state, its kind printed in the status line, and in `refusal=` for a refusal on this pull request. | diff --git a/scripts/pr_review.py b/scripts/pr_review.py index 87bd4165..cf263351 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -114,16 +114,16 @@ carries any Copilot activity. The loop runs in-process, so a 45-minute wait costs one agent turn, not 90. Exit 0 = review present, or on a held head an attested local pass whose checks no longer - hold the poll. On an open pull request whose head commit the payload carries, every - workflow run the read reaches on that head has then completed. The read reaches the + hold the poll. On an open held head whose commit the payload carries, every workflow + run the read reaches on that head has then completed. The read reaches the head's last 50 check suites and those of its first CHECKS_WINDOW rollup contexts. A CLEAN, UNSTABLE, or HAS_HOOKS merge owes no required check. Under any other merge, an open attested held head nothing holds exits 45 where its rollup was unreadable or, under DIRTY, none posted. A DIRTY or BEHIND one also exits 45 where a required check is unregistered or from another app. Exit 0 can then leave a required check unregistered only under unread rules, a `CHECKS TRUNCATED` rollup, a DRAFT merge, or a pull request - not open. Exit 0 on a review present reads no check, so it does not prove the required - checks ran. + not open. Exit 0 on a review present does not poll the checks, so it does not prove the + required checks ran. 30 = still pending at the timeout. On a held head it is any hold `status=CHECKS_PENDING` names, a required check still settling and a workflow run still open among them. Where the rollup holds a type this cannot read, a BLOCKED hold's note From 8c65e4d85d34e1ed10ccc8eef7a045a00211609a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 16:44:20 -0700 Subject: [PATCH 3/3] Place the Review-Present Caveat Before the Held-Head Sentences The second local pass found the completed-runs claim covering a held head no attestation covers, which polls nothing, and the caveat landing after an exhaustive "only" list it then contradicted. The claim names the attested held head, and the caveat now follows the opening line. Co-Authored-By: Claude Opus 5.5 --- scripts/README.md | 2 +- scripts/pr_review.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/README.md b/scripts/README.md index 59af2b54..6b54bfeb 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -199,7 +199,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj | Code | Command | Meaning | | --- | --- | --- | -| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. Exit `0` on a review present does not poll the checks, so it does not prove the required checks ran. | +| `0` | `status`, `wait` | No Copilot review covers the head yet (`status`), or the review covering it states no partial coverage and its verdict counts what it flags. On a held head, an attested local pass whose checks no longer hold the poll, short of `45`. Exit `0` on that review does not poll the checks, so it does not prove the required checks ran. | | `30` | `wait` | Still pending at the timeout, which is pending rather than failed. On a held head, `status=CHECKS_PENDING`. A round past `--min-rounds` not yet landed. | | `40` | `wait` | No Copilot review covers the head: the reviewer answered outside a formal review, the request returned success and recorded nothing, or the reviewer writes under a login no query filters on. | | `41` | `wait` | The newest round is a refusal or a quota state, its kind printed in the status line, and in `refusal=` for a refusal on this pull request. | diff --git a/scripts/pr_review.py b/scripts/pr_review.py index cf263351..d37e2249 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -114,16 +114,16 @@ carries any Copilot activity. The loop runs in-process, so a 45-minute wait costs one agent turn, not 90. Exit 0 = review present, or on a held head an attested local pass whose checks no longer - hold the poll. On an open held head whose commit the payload carries, every workflow - run the read reaches on that head has then completed. The read reaches the + hold the poll. Exit 0 on a review present does not poll the checks, so it does not prove + the required checks ran. On an open attested held head whose commit the payload carries, + every workflow run the read reaches on that head has then completed. The read reaches the head's last 50 check suites and those of its first CHECKS_WINDOW rollup contexts. A CLEAN, UNSTABLE, or HAS_HOOKS merge owes no required check. Under any other merge, an open attested held head nothing holds exits 45 where its rollup was unreadable or, under DIRTY, none posted. A DIRTY or BEHIND one also exits 45 where a required check is unregistered or from another app. Exit 0 can then leave a required check unregistered only under unread rules, a `CHECKS TRUNCATED` rollup, a DRAFT merge, or a pull request - not open. Exit 0 on a review present does not poll the checks, so it does not prove the - required checks ran. + not open. 30 = still pending at the timeout. On a held head it is any hold `status=CHECKS_PENDING` names, a required check still settling and a workflow run still open among them. Where the rollup holds a type this cannot read, a BLOCKED hold's note