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
6 changes: 3 additions & 3 deletions scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ The fleet's checks and review tooling, run by hand, with the deterministic ones

Python only, and standard library only, with markdown-it-py the one exception. `build_dist.py`, `canonical_review.py`, `carry.py`, `pr_review.py`, `pr_review_copilot.py`, and `pr_cleanup.py` declare it in their inline script metadata. `build_dist.py` and `carry.py` import it only on the path that reads a region, since `skills_install.py` and `handoff.py` import them under a plain `python3`. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, `canonical_review.py`, `handoff.py`, and `skills_install.py` below are the exceptions, since a generator, two recorders, an installer, and the handoff chain all exist to write. `build_dist.py --check` and `skills_install.py --report` are read-only modes for CI and for asking without changing anything. `local_review.py status` and `check` answer without recording anything, but they are not read-only in that same sense. Reading the working tree stages it, which writes inside `.git` and runs any `filter.*.clean` the repository configures. Only the receipt is left untouched. `canonical_review.py` reads files rather than staging them, so its `list`, `status`, `sweep`, and `report` are read-only outright, and `record` alone writes.

The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `backlog_metrics.py`, `build_dist.py`, `canonical_review.py`, `handoff.py`, `local_review.py`, `pr_cleanup.py`, `pr_review.py`, `pr_review_copilot.py`, `skills_install.py`. `gh_graphql.py` is a library with no command line, the GraphQL transport and cursor walker that `backlog_metrics.py`, `handoff.py`, `pr_cleanup.py`, and `pr_review.py` import from beside them. `pr_review_checks.py`, `pr_review_rounds.py`, and `pr_review_digest.py` are libraries with no command line as well. They are the pure readers `pr_review.py` imports for the head's checks, its review rounds, and the digest rendered from both. The unit tests live apart at the repository root under [`tests/`][tests], one `test_<script>.py` per script, driving the canonical implementations against input they must reject. The reader modules `pr_review.py` imports are tested through `test_pr_review.py`. A gate nobody has watched fail is a gate nobody knows works. Where a case covers a table, it reads the live table rather than restating it. Each such case also asserts a floor on what a healthy run reaches. That is because a check whose scan matches nothing reports zero findings and reads exactly like a pass.
The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `backlog_metrics.py`, `build_dist.py`, `canonical_review.py`, `handoff.py`, `local_review.py`, `pr_cleanup.py`, `pr_review.py`, `pr_review_copilot.py`, `skills_install.py`. `gh_graphql.py` is a library with no command line, the GraphQL transport and cursor walker that `backlog_metrics.py`, `handoff.py`, `pr_cleanup.py`, and `pr_review.py` import from beside them. `pr_review_checks.py`, `pr_review_check_lines.py`, `pr_review_rounds.py`, and `pr_review_digest.py` are libraries with no command line as well. They are the pure readers `pr_review.py` imports for the head's checks, its review rounds, and the digest rendered from both. The digest takes its lines about the checks from `pr_review_check_lines.py`. The unit tests live apart at the repository root under [`tests/`][tests], one `test_<script>.py` per script, driving the canonical implementations against input they must reject. The reader modules `pr_review.py` imports are tested through `test_pr_review.py`. A gate nobody has watched fail is a gate nobody knows works. Where a case covers a table, it reads the live table rather than restating it. Each such case also asserts a floor on what a healthy run reaches. That is because a check whose scan matches nothing reports zero findings and reads exactly like a pass.

```sh
python3 tests/test_prose_lint.py
Expand Down Expand Up @@ -190,7 +190,7 @@ uv run --no-project --script scripts/pr_review.py attest 452 --repo ptr727/Proje
uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/ProjectTemplate
```

**The gate and its readers are separate files.** This script holds the GraphQL and REST plumbing, the verdict, the wait loop, and the writes. The head's checks are read by `pr_review_checks.py` and its review rounds by `pr_review_rounds.py`, and `pr_review_digest.py` renders the `status` and `wait` digest from both. Each reads the payload the gate already fetched, and none makes a call or imports the gate. The digest's header is an ordered list of fields, and its fixed advisory lines are an ordered table. What a reviewer's body says is read by `pr_review_copilot.py`, whose result is a small typed reading rather than a body. The reviewer's wording has changed many times and has no programmatic API. A change in it is therefore a data edit in `reviewers/` plus one fixture, and never reaches the gate. A part of a head round that the reader cannot place holds the gate, `shapes=HELD` and exit `50`. A finding can sit in it where no count reaches. Its text prints under `UNPLACED ON THE HEAD`, each part cut to `EXCERPT_LINES` lines of `EXCERPT_WIDTH` characters. Beside it prints a fingerprint, the first `FINGERPRINT_HEX` hex characters of a SHA-256 over the set's text. An owner, member, or collaborator clears the hold, `shapes=ACKNOWLEDGED`, with written outcomes posted by `comment --fingerprint`. That comment carries `<!-- fleet-shape-ack: fingerprint=<fp> -->` as an HTML line of its own. A new head carrying new unplaced text has a new fingerprint and owes a new acknowledgment. A part on an earlier round prints under `REVIEWER OUTPUT NOTED` with a one-line note. So does a label nested in a list, a quote, or a placed part. It decides no exit code. `shapes=NOTED` shows only where no head hold takes the field.
**The gate and its readers are separate files.** This script holds the GraphQL and REST plumbing, the verdict, the wait loop, and the writes. The head's checks are read by `pr_review_checks.py` and its review rounds by `pr_review_rounds.py`, and `pr_review_digest.py` renders the `status` and `wait` digest from both. Its lines about the checks come from `pr_review_check_lines.py`, so a new check shape's wording is one table entry there. Each reads the payload the gate already fetched, and none makes a call or imports the gate. The digest's header is an ordered list of fields, and its fixed advisory lines are an ordered table. What a reviewer's body says is read by `pr_review_copilot.py`, whose result is a small typed reading rather than a body. The reviewer's wording has changed many times and has no programmatic API. A change in it is therefore a data edit in `reviewers/` plus one fixture, and never reaches the gate. A part of a head round that the reader cannot place holds the gate, `shapes=HELD` and exit `50`. A finding can sit in it where no count reaches. Its text prints under `UNPLACED ON THE HEAD`, each part cut to `EXCERPT_LINES` lines of `EXCERPT_WIDTH` characters. Beside it prints a fingerprint, the first `FINGERPRINT_HEX` hex characters of a SHA-256 over the set's text. An owner, member, or collaborator clears the hold, `shapes=ACKNOWLEDGED`, with written outcomes posted by `comment --fingerprint`. That comment carries `<!-- fleet-shape-ack: fingerprint=<fp> -->` as an HTML line of its own. A new head carrying new unplaced text has a new fingerprint and owes a new acknowledgment. A part on an earlier round prints under `REVIEWER OUTPUT NOTED` with a one-line note. So does a label nested in a list, a quote, or a placed part. It decides no exit code. `shapes=NOTED` shows only where no head hold takes the field.

**A Copilot round is requested at defined moments rather than on every push.** The one moment is the head of a pull request into the default branch, a promotion among them. A promotion re-reviews content its feature pull requests already carried, so the hosted review runs once per change, at the promotion. On a pull request into any other branch, every head is covered by the recorded local pass the push already owes. That pass lives in the checkout's git directory, where nothing on GitHub can read it, so `attest` publishes it as a comment carrying `<!-- fleet-local-review: head=<sha> base=<branch> findings=<n> -->`. It does so only after confirming the checkout is the pushed head and holds no other change. It does so only after also confirming that the checkout measures the pull request's own merge base and has a covering pass in `local_review.py status`. `status` then reads that head as `review_on_head=local` with `coverage=local`, where the attestation comes from an owner, member, or collaborator. The reading also requires that the attestation stands as an HTML line of its own outside any code. It further requires that no round on record states partial coverage with the whole history in view. A hidden attestation comment does not count. `wait` requests nothing on such a head and polls its checks instead, while they can still move the merge. Where it is not attested, `wait` exits `49`, and `--request` asks for a round anyway. The comment's findings count prints beside the reading and gates nothing, since a local pass's findings are advisory. A default branch the payload does not name reads as a promotion. An unreadable field therefore costs a Copilot request rather than passing a head on a local pass alone.

Expand Down Expand Up @@ -234,7 +234,7 @@ The digest reports the completed head review's effective effort as `lite`, `bala

`status` and `wait` exit `50` where the round covering the head opens on a verdict other than the clean one. This applies while that round states no open finding, has no open entry, opened no thread, and collapsed nothing, and no thread is open. Its headline prints under `VERDICT WITH NO COUNTED FINDING`, since it names what that round flags beside any findings list or file table it carries. The code repeats on that head after the answer, since nothing here reads one. Where a check on that head has not concluded, `wait` names a bounded `gh pr checks --watch` to watch it with. A clean verdict on the same terms prints its headline under `CLEAN VERDICT HEADLINE`, advisory and gating nothing. A clean round whose findings list or file table names a finding prints none, since `FINDINGS WITH NO THREAD` names those.

`wait` exits `44` when the review loop has closed but a required check sits in a shape no wait clears. The digest carries `checks=N/M` beside the merge word and names the shape as `stuck=`. It does so because `BLOCKED` alone is worn by a red check, a check nothing is running, an unresolved thread, and a missing approval alike. `NOT_POSTED` is a required status nothing has posted, and `NOT_PICKED_UP` is a check queued with no runner past `--check-grace`. `RUNNING_LONG` is a check running past `--check-stall`, and `FAILED` is a verdict. `GATED` is a check its environment's protection rule holds past `--check-grace`. On the Copilot path, a check merely still running is none of these and exits `0`. `wait` returns the moment the round lands, and the checks are nearly always still going then. A held head polls its checks instead. There a check running long or gated holds the poll to `30` rather than ending it at `44`. `44` also requires that the stuck check be required, read from each rollup node's `isRequired`, and that the merge read `BLOCKED`. A rollup member of an unknown type is named on `CHECKS PARTIALLY UNREAD`, and a rollup past `CHECKS_WINDOW` contexts is named on `CHECKS TRUNCATED`. A payload with no commit matching the head is named on `CHECKS UNREADABLE`. None of those three reads as a clean `checks=0/0`. A `CheckRun`'s `PENDING` is dispatched and not begun while a `StatusContext`'s is under way, so the two are translated rather than renamed. `--check-grace` must be less than `--check-stall`, and a negative value for either is rejected by name.
`wait` exits `44` when the review loop has closed but a required check sits in a shape no wait clears. The digest carries `checks=N/M` beside the merge word and names the shape as `stuck=`. It does so because `BLOCKED` alone is worn by a red check, a check nothing is running, an unresolved thread, and a missing approval alike. `NOT_POSTED` is a required status nothing has posted, and `NOT_PICKED_UP` is a check queued with no runner past `--check-grace`. `RUNNING_LONG` is a check running past `--check-stall`, and `FAILED` is a verdict. `GATED` is a check its environment's protection rule holds past `--check-grace`. `STALE_RECORD` is a job still in progress inside a workflow run that has completed. No wait ends it, and re-running that job or pushing a new head replaces it. On the Copilot path, a check merely still running is none of these and exits `0`. `wait` returns the moment the round lands, and the checks are nearly always still going then. A held head polls its checks instead. There a check running long or gated holds the poll to `30` rather than ending it at `44`. `44` also requires that the stuck check be required, read from each rollup node's `isRequired`, and that the merge read `BLOCKED`. A rollup member of an unknown type is named on `CHECKS PARTIALLY UNREAD`, and a rollup past `CHECKS_WINDOW` contexts is named on `CHECKS TRUNCATED`. A payload with no commit matching the head is named on `CHECKS UNREADABLE`. None of those three reads as a clean `checks=0/0`. A `CheckRun`'s `PENDING` is dispatched and not begun while a `StatusContext`'s is under way, so the two are translated rather than renamed. `--check-grace` must be less than `--check-stall`, and a negative value for either is rejected by name.

`comment` posts one answer in the pull request conversation. A suppressed finding has no thread, so this is the path that records its disposition. The command reads the pull request node ID in the same run and exposes no ID argument. It confirms the returned comment URL and exact body before it reports success. A missing target exits `65`, and an unconfirmed response exits `66`, after which the conversation is inspected before any retry. With `--fingerprint` the body is the acknowledgment of an unplaced set, posted with its marker beneath it.

Expand Down
8 changes: 5 additions & 3 deletions scripts/pr_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@
refusal whose one re-request is pending exits 30 instead. 42 and 50 = the review landed
and `status`'s readings apply to it. 44 = the review loop closed, the merge reads BLOCKED, and a required check
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.
never posted, running far past the job's cost, held by its environment, still in
progress inside a run that has completed, 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. One case is a payload with no commit matching the head, named
`CHECKS UNREADABLE` in the digest. Another is a DIRTY merge with no check posted that
Expand Down Expand Up @@ -1737,7 +1738,8 @@ def read_live() -> tuple[bool, dict | None, list[str], bool]:
"BLOCKED, and a required check is in a shape waiting does not clear, "
f"required stuck {', '.join(repr(g) for g in gates)}: read their lines in the "
"block above, since a starved check wants a re-run, an unposted one its poster, "
"a gated one its approval or its timer, a long one a judgment, and a failed one a fix. It need "
"a gated one its approval or its timer, a long one a judgment, a stale record a "
"re-run of its job or a new head, and a failed one a fix. It need "
"not be all that blocks "
"the merge, because BLOCKED is also worn by a thread or a missing approval"
)
Expand Down
Loading
Loading