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
5 changes: 3 additions & 2 deletions scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,13 @@ 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, which does not prove its 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. |
| `42` | `status`, `wait` | The round covering the head states partial coverage. Or a non-refusal head round exists while an earlier round states partial, and one reads `UNKNOWN` or none states coverage. |
| `44` | `wait` | The review loop closed, the merge reads `BLOCKED`, and a required check is in a shape no wait clears. |
| `45` | `wait` | An open attested held head that nothing holds, its merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. Its rollup was unreadable, a `DIRTY` merge has none posted, or a `DIRTY` or `BEHIND` merge lacks a required check or has it from another app, judged on rules read and a whole rollup. |
| `49` | `wait` | No Copilot round covers the head and none was requested, on a pull request into a branch other than the default with no attestation. |
| `50` | `status`, `wait` | The round covering the head opens on a verdict other than the clean one while it counts no finding. Or a head round carries a part the reader cannot place, and nothing acknowledges its fingerprint. |
| `60` to `63` | `reply` | No thread matched, more than one did, the reply returned no comment url, or the resolve did not report the thread resolved. |
Expand All @@ -214,7 +215,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj

`1` is a GraphQL read that failed and ended the run, and `2` is argparse's own usage error.

`wait` exits `30` with `status=CHECKS_PENDING` on an attested held head while its checks can still move the merge at the timeout. It reads them in this order. A pull request no longer open holds nothing, and a merge reading `UNKNOWN` holds. A `BLOCKED` merge ends the poll on the first read where a required check is stuck in a shape waiting does not clear. A check running long or gated is not such a shape. A failed one is the exception while a newer run of its workflow on the head has not finished, which the digest names `CHECK SUPERSEDED`. Next, a workflow run on the head that has not completed holds, where the read reaches it. The read reaches the head's last 50 check suites and those of its first `CHECKS_WINDOW` rollup contexts. A run's later jobs enter the rollup only as their `needs:` finish, so a green rollup proves nothing while one is open. The status line names those runs. Next, a merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll. Otherwise a required check still settling holds, one running long included, and so does any settling check while no required one has posted. A `BLOCKED` merge also holds while a check the base branch's rules require has not registered. That hold needs the base branch's rules read and a rollup that is not `CHECKS TRUNCATED`. An empty rollup holds under `BLOCKED`, `BEHIND`, or `DRAFT`. So exit `0` on a held head does not prove its required checks ran. A payload with no commit matching the head, which the digest names `CHECKS UNREADABLE`, reads no run and no check. Short of those holds, a merge with some check posted, or a `DIRTY` one with none, can release with a required check unregistered. `--min-rounds N` counts what `status` prints as `rounds=`, every round on every head, refusals included, so N is the `rounds=` a `status` read before a re-request.
`wait` exits `30` with `status=CHECKS_PENDING` on an attested held head while its checks can still move the merge at the timeout. It reads them in this order. A pull request no longer open holds nothing, and a merge reading `UNKNOWN` holds. A `BLOCKED` merge ends the poll on the first read where a required check is stuck in a shape waiting does not clear. A check running long or gated is not such a shape. A failed one is the exception while a newer run of its workflow on the head has not finished, which the digest names `CHECK SUPERSEDED`. Next, a workflow run on the head that has not completed holds, where the read reaches it. The read reaches the head's last 50 check suites and those of its first `CHECKS_WINDOW` rollup contexts. A run's later jobs enter the rollup only as their `needs:` finish, so a green rollup proves nothing while one is open. The status line names those runs. Next, a merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll. Otherwise a required check still settling holds, one running long included, and so does any settling check while no required one has posted. A `BLOCKED` merge also holds while a check the base branch's rules require has not registered. That hold needs the base branch's rules read and a rollup that is not `CHECKS TRUNCATED`. Where the rollup holds a type the reader cannot parse, its status line says that check may sit in the unread part. An empty rollup holds under `BLOCKED`, `BEHIND`, or `DRAFT`. An open head released short of those holds exits `45` with `status=CHECKS_MISSING` where a check it is owed is missing from the read. Each such case falls under a merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. One is a payload with no commit matching the head, which the digest names `CHECKS UNREADABLE` and which reads no run and no check. Another is a rollup with no check posted, which only a `DIRTY` merge releases, since a conflicted pull request runs no `pull_request` workflow. The last is a `DIRTY` or `BEHIND` merge whose rollup lacks a context the base branch's rules require, or carries it from another app. That one needs the rules read and a rollup that is not `CHECKS TRUNCATED`. Where the rollup holds a type the reader cannot parse, which the digest names `CHECKS PARTIALLY UNREAD`, the status says the check may sit there. Exit `0` can then leave a required check unregistered only under unread rules, a cut rollup, a `DRAFT` merge, or a pull request not open. `--min-rounds N` counts what `status` prints as `rounds=`, every round on every head, refusals included, so N is the `rounds=` a `status` read before a re-request.

`wait` exits `40` when no Copilot review covers the head and the reviewer has said why in another shape. A plain comment newer than its newest review carries no commit, so it satisfies no coverage check whatever it says, and its body prints whole because its wording is the only thing separating a refusal from a remark. A comment older than the newest review is spent. Every connection reads the newest `WINDOW` nodes rather than the reviewer's own, so `window_blind` guards both sides: blind on comments reads as `answered_outside_review=unknown`, and blind on reviews reports nothing and lets the wait keep polling. A request that returns success and adds neither a pending reviewer nor a review-request event also exits `40`, as `status=REQUEST_NOT_RECORDED`, as does a reviewer writing under a renamed login, `status=REVIEWER_LOGIN_DRIFT`.

Expand Down
58 changes: 43 additions & 15 deletions scripts/pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,21 @@
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
head's last 50 check suites and those of its first CHECKS_WINDOW rollup contexts. It
does not prove the required checks ran.
A payload with no commit matching the head, named `CHECKS UNREADABLE` in the digest,
reads no run and no check. A required check not yet registered holds only a BLOCKED
merge whose base branch rules were read and whose rollup is not `CHECKS TRUNCATED`. An
empty rollup holds only a BLOCKED, BEHIND, or DRAFT one. Short of those holds, a merge with
some check posted, or a DIRTY one with none, can exit 0 with a required check unregistered.
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.
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. It is also a round past --min-rounds not yet landed.
open among them. Where the rollup holds a type this cannot read, a BLOCKED hold's note
says an unmet required check may sit in its unread part. It is also a round past
--min-rounds not yet landed.
40 = no Copilot review covers the head: the reviewer
answered outside a formal review, the request returned success and recorded nothing on
the pull request, or the reviewer writes under a login no query filters on. 41 = the
Expand All @@ -140,6 +143,17 @@
is in a shape no wait clears. It is queued with nothing acting on it, expected and
never posted, running far past the job's cost, held by its environment, or failed. On
a held head a check running long or held by its environment polls to 30 instead.
45 = an open attested held head that nothing holds, its merge other than CLEAN,
UNSTABLE, or HAS_HOOKS. Either the payload carries no commit matching the head, named
`CHECKS UNREADABLE` in the digest, or a DIRTY merge has no check posted. Or the merge
reads DIRTY or BEHIND and a check the base branch's rules require has not registered,
or came from another app. That last needs the rules read and a rollup not
`CHECKS TRUNCATED`. Where the rollup holds a type this cannot read, named
`CHECKS PARTIALLY UNREAD` in the digest, the status says the check may sit there.
`status=CHECKS_MISSING` names which. A DIRTY merge runs no pull_request workflow until
its conflict is resolved. A required check not yet registered holds a BLOCKED merge to
30 instead, where the rules and the head's rollup were read, not `CHECKS TRUNCATED`. An
empty rollup read off the head holds a BLOCKED, BEHIND, or DRAFT one.
49 = no Copilot round covers this head and none was requested, the pull request
merging into a branch other than the default with no attestation vouching for the
head. 64 = the write scope could not
Expand Down Expand Up @@ -181,6 +195,7 @@
_unmet_note,
check_lines,
check_nodes,
checks_never_ran,
checks_settling,
checks_stuck,
checks_tally,
Expand Down Expand Up @@ -664,9 +679,11 @@ def parsed_json(proc: subprocess.CompletedProcess) -> object:
return None


def required_for(pr: dict, owner: str, repo: str) -> frozenset[tuple[str, int | None]]:
"""The required entries a BLOCKED merge can be short of, empty wherever none can matter."""
if pr.get("mergeStateStatus") != "BLOCKED":
def required_for(
pr: dict, owner: str, repo: str, gated: frozenset[str] = frozenset({"BLOCKED"})
) -> frozenset[tuple[str, int | None]]:
"""The required entries a merge in `gated` can be short of, empty wherever none can matter."""
if pr.get("state", "OPEN") != "OPEN" or pr.get("mergeStateStatus") not in gated:
return frozenset()
return required_contexts(owner, repo, pr.get("baseRefName") or "") or frozenset()

Expand Down Expand Up @@ -707,7 +724,7 @@ def required_contexts(owner: str, repo: str, base: str) -> frozenset[tuple[str,
):
print(
f"note: the rules for {base} were unread, so which checks they require is unknown and "
"this wait cannot hold for one that has not registered yet."
"this wait cannot judge one that has not registered."
)
return None
return frozenset((e[0], e[1]) for e in entries)
Expand Down Expand Up @@ -2013,7 +2030,7 @@ def held_read() -> dict:
"had not completed, a check the base branch requires had not registered or came "
"from another app, or the merge was still UNKNOWN, so the merge is not ready yet: "
"wait again, or read the checks above"
+ _unmet_note(required_for(final, owner, repo), checks)
+ _unmet_note(final, required_for(final, owner, repo), checks)
+ (f" (not completed: {', '.join(runs)})" if runs else "")
)
return 30
Expand All @@ -2030,6 +2047,17 @@ def held_read() -> dict:
"the merge, because BLOCKED is also worn by a thread or a missing approval"
)
return 44
if covered and (
unrun := checks_never_ran(
final, checks, required_for(final, owner, repo, frozenset({"DIRTY", "BEHIND"}))
)
):
print(
"status=CHECKS_MISSING an attested local pass covers this head and nothing holds "
f"the poll, but {unrun}. The merge reads {final.get('mergeStateStatus')}, so it is "
"not ready to merge"
)
return 45
return 0
# Every refusal and quota state is 41, the kind named by its status line.
refusal = refusing_review(final)
Expand Down
55 changes: 43 additions & 12 deletions scripts/pr_review_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
# SKIPPED and NEUTRAL count as passed, since the fleet aggregator skips conditional jobs.
CHECK_OK = frozenset({"SUCCESS", "SKIPPED", "NEUTRAL"})

MERGE_UNGATED = frozenset({"CLEAN", "UNSTABLE", "HAS_HOOKS"})

WORKFLOW_KEYS_UNAVAILABLE = "workflowKeysUnavailable"

# How long a queued check is simply starting, the pickup grace's five minutes.
Expand Down Expand Up @@ -283,23 +285,47 @@ def held_checks_open(
return False
if runs_open(pr):
return True
if merge in ("CLEAN", "UNSTABLE", "HAS_HOOKS"):
if merge in MERGE_UNGATED:
return False
settling = checks_settling(nodes, now, grace, stall)
if any(n.get("required") for n in settling):
return True
if settling and not any(n.get("required") for n in nodes):
return True
if (
merge == "BLOCKED"
and not checks_unreadable(pr)
and not checks_truncated(pr)
and required_unmet(required, nodes)
):
if merge == "BLOCKED" and required_unregistered(pr, nodes, required):
return True
return not nodes and merge in ("BLOCKED", "BEHIND", "DRAFT") and not checks_unreadable(pr)


def checks_never_ran(
pr: dict, nodes: list[dict], required: frozenset[tuple[str, int | None]]
) -> str:
"""Why an open head's gated merge lacks a check it is owed, with its remedy, or "" for none.

A CLEAN, UNSTABLE, or HAS_HOOKS merge is owed nothing. The caller passes `required` only for a
merge whose release judges registration, and a context is judged only on the head's own
rollup, not cut.
"""
if pr.get("state", "OPEN") != "OPEN" or pr.get("mergeStateStatus") in MERGE_UNGATED:
return ""
if checks_unreadable(pr):
return "the payload carries no commit matching the head, so read the check runs on the head commit"
if not nodes:
return (
"no check has posted, so resolve the head's conflict with its base, since a DIRTY "
"merge runs no pull_request workflow until then"
)
unmet = ", ".join(required_unregistered(pr, nodes, required))
unread = checks_unread(nodes)
where = ", sits in the rollup's unread part," if unread else ""
read = "read the head commit's checks and statuses first, then " if unread else ""
return unmet and (
f"a required check has not registered{where} or came from another app, {unmet}, so "
f"{read}update a BEHIND head, resolve a DIRTY one's conflict, or correct the check's "
"poster or its rule"
)


def runs_open(pr: dict) -> list[str]:
"""The head's workflow runs whose suite has not completed, each named `run <id>`, sorted.

Expand All @@ -322,8 +348,12 @@ def runs_open(pr: dict) -> list[str]:
return sorted(named)


def required_unmet(required: frozenset[tuple[str, int | None]], nodes: list[dict]) -> list[str]:
"""The contexts among `required` that no rollup node satisfies, sorted."""
def required_unregistered(
pr: dict, nodes: list[dict], required: frozenset[tuple[str, int | None]]
) -> list[str]:
"""The `required` contexts no node satisfies, sorted, on the head's own rollup, not cut."""
if checks_unreadable(pr) or checks_truncated(pr):
return []
seen = [n for n in nodes if not n.get("unreadable")]
return sorted(
{
Expand All @@ -337,10 +367,11 @@ def required_unmet(required: frozenset[tuple[str, int | None]], nodes: list[dict
)


def _unmet_note(required: frozenset[tuple[str, int | None]], nodes: list[dict]) -> str:
def _unmet_note(pr: dict, required: frozenset[tuple[str, int | None]], nodes: list[dict]) -> str:
"""Names the required contexts still unmet, so a reader sees what the hold waits on."""
unmet = required_unmet(required, nodes)
return f" (required, unmet: {', '.join(unmet)})" if unmet else ""
unmet = required_unregistered(pr, nodes, required)
where = ", which may sit in the rollup's unread part" if checks_unread(nodes) else ""
return f" (required, unmet: {', '.join(unmet)}{where})" if unmet else ""


def checks_truncated(pr: dict) -> bool:
Expand Down
Loading
Loading