Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ python3 scripts/pr_review.py attest 452 --repo ptr727/ProjectTemplate --checkout

**Do not infer one field's spelling from another's.** Several states print upper-case, `merge=` reproduces GitHub's own enum verbatim, and a field can arrive behind a prefix, so a pattern built by analogy with a neighboring field silently never fires. `review_on_head` is the one that catches a watcher, rendering `yes` against `NO`, so a pattern written for `YES` waits out its whole bound over a review that landed. A watcher that cannot fire reads exactly like a review that has not landed, so run the command once, read a real line, and write the pattern against that. Anchor the pattern on what is unique to the field being read, meaning that field's own name with its value, and for a pair nested in a parenthesized group the group's name ahead of it, since values repeat across fields and the groups carry the same inner names as each other. A watcher testing for a bare negative matches a line a covered review wrote, so it reports no review over a review that landed and waits out its whole bound saying so. That one matches on the wrong evidence rather than failing to match, which is worse than the casing trap above, since its answer is the digest's own inverted rather than absent. Give the watcher a branch that exits non-zero where the digest cannot be read at all, or a pattern that never fires and a command that never ran are equally silent.

`wait` exits `30` when the review is still pending at the timeout, which is pending rather than failed. On an attested held head it exits `30` with `status=CHECKS_PENDING` at the timeout. That is a required check still settling, one running long included, or any check while no required one has posted. It is also no check posted on a `BLOCKED`, `BEHIND`, or `DRAFT` merge, or the merge still `UNKNOWN`. A merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll ahead of all of these. A round requested while that poll runs ends it with `status=PENDING`, so the next `wait` polls for that round. A quota reading outranks that line with its own `46` or `47`, and an answer outside a review with `40`. `wait`'s failure mode is a wrong answer rather than a crash, so the cases feed crafted GraphQL payloads: a review attributed to the wrong login, a review counted against a stale head, an unknown author's open thread left uncounted, and a wait that returns success while nothing landed. One case reads the reviewer login out of the runbook rather than restating it, since GraphQL drops the `[bot]` suffix REST carries, and another holds the script to exactly the four mutation documents named `addComment`, `requestReviews`, `addPullRequestReviewThreadReply`, and `resolveReviewThread`, so a fifth arriving is a write nobody reviewed as one.
`wait` exits `30` when the review is still pending at the timeout, which is pending rather than failed. On an attested held head it exits `30` with `status=CHECKS_PENDING` at the timeout. That is a required check still settling, one running long included, or any check while no required one has posted. It is also no check posted on a `BLOCKED`, `BEHIND`, or `DRAFT` merge, or the merge still `UNKNOWN`. A merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll ahead of all of these. On a `BLOCKED` merge three shapes of required check end the poll at once even while another required check still runs, and the exit is then `44` unless a review verdict outranks it. A failed check ends it on the first read. A status expected and never posted, and a check queued with no runner, end it only once each is past `--check-grace`. A required check merely running long ends nothing. A round requested while that poll runs ends it with `status=PENDING`, so the next `wait` polls for that round. A quota reading outranks that line with its own `46` or `47`, and an answer outside a review with `40`. `wait`'s failure mode is a wrong answer rather than a crash, so the cases feed crafted GraphQL payloads: a review attributed to the wrong login, a review counted against a stale head, an unknown author's open thread left uncounted, and a wait that returns success while nothing landed. One case reads the reviewer login out of the runbook rather than restating it, since GraphQL drops the `[bot]` suffix REST carries, and another holds the script to exactly the four mutation documents named `addComment`, `requestReviews`, `addPullRequestReviewThreadReply`, and `resolveReviewThread`, so a fifth arriving is a write nobody reviewed as one.

`wait` exits `64` when the target is out of scope, the same refusal `comment` and `reply` raise, checked before either the auto-request or the backoff loop reaches GitHub. The `reply` paragraph below states what that refusal covers.

Expand Down
6 changes: 5 additions & 1 deletion scripts/pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -3203,7 +3203,11 @@ def head_commit(pr: dict) -> dict:


def check_nodes(pr: dict) -> list[dict]:
"""The head commit's checks, each as {name, state, conclusion, since}.
"""The head commit's checks, each a dict with `name`, `state`, `conclusion`, and `since`.

A readable node also carries `required`, true unless the rollup marks it `isRequired: false`,
which `held_checks_open` and `wait`'s exit `44` decide on. A node of an unrecognized union
member carries empty values for the four and an `unreadable` key naming its type instead.

A rollup carries two node shapes and they spell every field differently: a CheckRun has a
`name`, a `status` and a `conclusion`, while a StatusContext has a `context` and a single
Expand Down
Loading