ci: fleet review lanes show up as commit statuses on the PR head

[askalf]

What does this PR do?

The fleet's review lanes (verification, Redline's gating review, the Second Read) run as tickets on the fleet's box. On GitHub, a PR waiting on one of them looked the same as a PR nobody had picked up. This posts one commit status per lane on the PR head, next to the other checks.

status	pending	green	red
fleet/verify	waiting on required CI (or the Breaker) at the head	verified at the head, or not required (docs, assets, .github config, bot branch)	a required check failed at the head
fleet/review	waiting on Redline at the head, or on verification	Redline approved the head	Redline requested changes at the head
fleet/second-read	waiting on the Second Read at the head, or on verification	READY at the head, or not gating (non-code PR)	NOT READY at the head, with its reason

Here the base branch requires test, analyze (javascript-typescript), docker-build. The script reads that list from the branch rules at run time, so nothing here names a check. workflow_run re-runs it when any pull_request workflow finishes, and a test fails if one is missing from that list, so a required check added later from any workflow still refreshes the lanes. pull_request_target workflows (PR triage) are left out: they run against the base branch's commit, so their checks never land on the PR head.

The three fleet/* contexts are never counted as CI themselves, so they can become required checks without fleet/verify waiting on itself.

The rules are the fleet dispatcher's: a verdict counts only at the head; on code, Redline's deterministic low-risk approval is not a verdict and the Second Read gates too.

• The status job runs the default branch's copy of scripts/fleet-status.mjs, never the PR's code. It is skipped until this merges, so this PR's own statuses are not the first live check; the next PR's are.
• It runs on a GitHub-hosted runner with the workflow's own token (checks: read, statuses: write, reads only otherwise). It does not use the fleet's GitHub quota.
• Fork PRs are skipped; the fleet does not review them.
• No concurrency group: a cancelled run would roll up as a failed check. Instead each run posts only what differs from the head's newest statuses, then re-reads the PR and corrects what differs (up to three passes), so the run that acts last leaves statuses matching data at least as new as anything posted.
• The statuses are informational. None is added to the ruleset's required checks.
• self-test runs the script's 111 unit tests on the PR's code, read-only.
• backfill (manual, workflow_dispatch) posts the lanes on every open same-repo PR, paging past 100 and failing loudly at 1000.

Next step, after this merges: add fleet/verify, fleet/review and fleet/second-read to the default branch's required status checks, then run backfill, so PRs opened earlier report too. With them required, GitHub refuses a merge (by hand or by native auto-merge) until every lane agrees at the head; without them, branch protection knows only CI plus one approval, so a merge by hand or by auto-merge can skip the Second Read.

How to test

• node scripts/fleet-status.test.mjs: 111 pass, 0 fail.
• The script and tests are plain ASCII. Tests build the Second Read's own separator character at run time with String.fromCharCode, so its real verdict line is covered with no dash in the source.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: approve — no blocking issues found in the fleet-status workflow, status computation, or its regression coverage.

What's good: the workflow executes the default branch copy with scoped read/status-write permissions, separates untrusted fork PRs, and the status logic pins verdicts to the live head. I reviewed the added workflow, the 240-line status computation, and its unit-test coverage; required CI is green at ee56d8eecab827e73692fe6b4a0ad534ac39f694. I did not run the local suite, per review policy.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

Verdict: changes requested — public test text includes patch-history narration and generated-writing tells. rule:reads-as-generated

Blocking finding — generated-writing tell

scripts/fleet-status.test.mjs:63:

console.log("\n the dario#1403 morning: verdicts on an older head");

This is public patch-history narration in a regression test. It describes a prior PR/event rather than the behavior the test exercises, and exposes the fleet's review history in the shipped test suite. The same newly added test file also frames a test comment around what the Second Read did (scripts/fleet-status.test.mjs:275). Remove the history references and name/comment the case solely for its observable stale-head behavior.

console.log("\n  verdicts on an older head");

What's good: all required CI checks passed at 238c2f9ba68529ce0fe107214ffd8fbb5446dd33; I reviewed the workflow, lane-status implementation, and its unit tests. I did not run the local suite, per review policy.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).

Verdict: no blocking issues at d1a302d. The lane rules match the stated table, the boundaries I rebuilt each have a test that can fail, and every claim in the PR body I checked matches the diff.

Note on the head: the ticket named ee56d8e, but the live head is d1a302d (5 commits ahead: dismissed reads, >100 files, own-context exclusion, post-then-verify, test renames). This read covers the whole diff at d1a302d.

Boundaries rebuilt from the diff

Predicate	Input	What the code does	Pinned by
needsVerify facts.files.length > 100 (scripts/fleet-status.mjs:55)	99 / 100 / 101 docs files	not code / not code / code	"99 docs files…", "exactly 100 docs files…", "more than 100 files is code…"
same	101 files on a bot branch	isBotPr returns first, so exempt	"a bot PR with 100 files is not code" (100, not 101; the bot early return means the count never matters)
isBotPr author AND branch	person on bot/, release/; askalf on a feature branch; dependabot on any branch	not bot / not bot / bot	the isBotPr checks at test:48-50, 198-203
isCodePath .github/	workflow yml, .github/actions/*, .github/workflows/helper.MJS	not code / code / code	"what counts as code"
verifiedAtHead label AND last askalf heading	label only; comment only; older head; [HEAD, OLD]; [OLD, HEAD]; 7-char prefix; "findings"/"blocked" headings	false, false, false, false, true, true, false	the verifiedAtHead blocks (the [HEAD, OLD] case fails if first-wins, so it holds)
requiredCiState	[] required; only own contexts required; one unreported; one in_progress; failure + pending; failure then success rerun; skipped/neutral	none, none, pending, pending, failed, passed, passed	the requiredCiState block (lines 646-655 of the test) plus the own-lanes block
laneStatuses ci === 'none' && verifiedAtHead	CI pending with a stale Breaker label+comment	stays pending, so the label cannot short-circuit CI	"CI pending: an old Breaker label and comment do not count"
redlineVerdict deterministic marker, code flag	code / docs	skipped / counts	"deterministic approvals"
rv.commitId === facts.head	verdict on OLD	pending with (its last verdict was on 34b7875)	"an old CHANGES_REQUESTED is not a red…", docs variant
secondReadAtHead regex	no line; READY, mostly; READY then NOT READY in one body; bare NOT READY; other login; older head + lineless at head; em-dash reason	none, none, NOT READY, red with no trailing colon, none, none, reason with the dash stripped	"the Second Read, review by review" and the verdict tests
fit 140	140 / 141 chars	kept / 137 + ...	"the 140-character edge"
statusesToPost	identical; state differs; description differs; missing	skip / post / post / post	the post-then-verify block

I did not find a reachable row where the code does the wrong thing, or a new assertion that holds whether or not the code it names is there. I spot-checked "the latest comment wins", "dismissed NOT READY at head: waiting, not red" and "own lanes required and pending, CI green: still passed": each one fails if the guard it names is removed.

PR body claims checked against the diff

• The status job checks out default_branch with a sparse scripts/fleet-status.mjs and runs the script only when hashFiles finds it (.github/workflows/fleet-status.yml:49-67). The PR's code never runs with statuses: write. Holds.
• Job-scoped permissions are contents/pull-requests/issues/checks: read and statuses: write, with top-level permissions: {}. Holds.
• self-test has contents: read only and runs the PR's tests. Holds.
• Fork PRs are filtered in the job if and again in readFacts (p.head?.repo?.full_name !== repo), and this second check also covers issue_comment on fork PRs. Holds.
• There is no concurrency: block. The 3-pass post-then-reread loop is at scripts/fleet-status.mjs:401-418. Holds.
• Required checks are read at run time from /rules/branches/<base>. On main today these return test, analyze (javascript-typescript) and docker-build, which come from the ci, CodeQL and build workflows named in workflow_run.workflows. Holds.
• 94 tests: I count 94 check( calls. CI self-test passes at this head. Holds.
• "plain ASCII throughout": I grepped the diff for bytes ≥ 0x80 and found none. Holds.
• Not checked: that the files are byte-identical to askalf/dario#1419.

Non-blocking notes

• workflow_run.workflows: [ci, build, CodeQL] (.github/workflows/fleet-status.yml:24) is a static list, but the required checks are read dynamically. If a ruleset later requires a check from another workflow, fleet/verify will sit on pending until some other event re-runs the job. If that happens, adding the workflow name to this list is the fix. Today's rules are covered.
• r.state === 'DISMISSED' in secondReadAtHead (scripts/fleet-status.mjs:242) can't happen for this lane today, because GitHub refuses to dismiss a COMMENTED review. The filter is harmless and still correct if the lane ever posts a different review type.

What's good

All GitHub I/O is kept out of pure functions, so the whole rule table can be tested without the network. The fail-closed choices are correct: unreadable rules fall back to the Breaker rule, and unreadable checks count as pending, so neither can make fleet/verify green. Leaving the fleet/* contexts out of the CI count removes the obvious self-wait before those contexts become required.

CI at d1a302d: test, analyze (javascript-typescript), docker-build, self-test, status and triage pass.

SECOND READ: READY

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: CHANGES_REQUESTED — a required check produced by a workflow not in the static trigger list can leave fleet/verify pending indefinitely. rule:none

Blocking — correctness

.github/workflows/fleet-status.yml:22-25

  workflow_run:
    workflows: [ci, build, CodeQL]
    types: [completed]

The implementation derives the required check names from the branch rules at runtime, but the workflow only wakes for completions from these three hard-coded workflow names. If the ruleset requires a check from another workflow (for example, a new lint workflow), its completion will not run this job. On a PR with no subsequent review, label, comment, or push event, the last fleet-status run still sees that check pending and leaves fleet/verify pending forever, even after GitHub reports every required check passing. That blocks the PR once the fleet contexts are required.

Use a trigger/reconciliation mechanism that runs after any relevant required-check completion, or otherwise make the completion source match the dynamically discovered required checks rather than maintaining a divergent static list.

# Reconcile fleet statuses from an event source that cannot omit a newly required
# workflow, rather than limiting workflow_run to a fixed workflow-name allowlist.

What's good

I reviewed the live-head diff at d1a302d5ed3c79462ce75b07bc2ac0447777b8fa, including the post-then-reread status loop, own-context exclusion from required CI, and the head-scoped Redline/Second Read verdict rules. Required CI is green at this head. The focused self-test coverage is a strong fit for the pure rule functions.

I did not run the local suite; CI is the test signal and its required checks passed.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: changes requested — the manual backfill does not cover every open same-repository PR as documented. rule:none

Blocking — correctness / coverage

.github/workflows/fleet-status.yml:112-113

prs="$(gh pr list --repo "$REPO" --state open --limit 100 --json number,isCrossRepository \
  --jq '.[] | select(.isCrossRepository | not) | .number')"

gh pr list --limit 100 returns at most 100 results; the loop consequently never invokes fleet-status.mjs for open same-repository PRs after that cap. For example, if the repository has 101 eligible open PRs when the documented post-merge backfill is run, one PR receives none of the three lane contexts. If those contexts have been made required as this PR proposes, that PR remains blocked until a later event happens to refresh it. The PR body promises that backfill posts lanes on every open same-repo PR, so the changed implementation needs to paginate rather than silently truncate. Add a regression test (or equivalent invocation-level coverage) for multiple pages as well.

# Page through all open PRs rather than limiting the backfill to one page.
gh pr list --repo "$REPO" --state open --limit 100000 --json number,isCrossRepository \
  --jq '.[] | select(.isCrossRepository | not) | .number'

I reviewed the live-head diff at cd94b53, including the new workflow, lane-status logic, and unit tests. Required CI is green (test, analyze (javascript-typescript), and docker-build); I did not run the suite locally.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: approve — no blocking issues found in the live-head change.

I reviewed the three-file implementation, with particular attention to the manual backfill pagination/cap guard and the separator parsing added at this head. Required CI is green at 3eda07a37965239f910947350911d913f9e3ab73 (test, analyze (javascript-typescript), and docker-build). I did not run the local test suite, per review-lane policy.

What's good: the backfill now obtains the full bounded list before filtering same-repository PRs and fails rather than silently omitting work at its stated cap. The new test coverage exercises each supported separator, including the Second Read's runtime-built form, and asserts the externally visible failure description.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).

Verdict: no blocking issues at 3eda07a. I found one low-severity convergence edge and one low-severity backfill robustness point. Neither is a wrong line on a path this repo hits in practice.

Note on head: the dispatch ticket named cd94b53. The live head is 3eda07a (pushed 04:25Z), and this review is of 3eda07a.

Findings

Low: scripts/fleet-status.mjs:272-288: the third pass posts without re-reading afterwards

  for (let pass = 1; pass <= 3; pass++) {
    const read = await readFacts();
    ...
    const todo = statusesToPost(want, have);
    if (!todo.length) break;
    ...
    for (const s of todo) {
      await gh(`/repos/${repo}/statuses/${read.facts.head}`, token, {

Scenario: run A has differed from the head on passes 1 and 2 because other runs kept posting. On pass 3 it reads facts D_old. Run B then reads D_new, posts, re-reads, finds a match and exits. A's pass-3 POST lands after that and the loop ends, so D_old stays on the head until the next event. The comment at :268-271 states the bound ("the next event covers anything after that"). The body's "the run that acts last leaves statuses matching data at least as new as anything posted" therefore holds only for passes 1 and 2. It takes three contested passes in a row on one PR, and since the lanes are informational and not required, I'm not blocking on it. Once fleet/* become required checks, a stale pending would block a merge until the next event.

Suggested fix (optional): let the last pass verify only, so a run never ends right after a write.

  for (let pass = 1; pass <= 4; pass++) {
    ...
    const todo = statusesToPost(want, have);
    if (!todo.length) break;
    if (pass === 4) { console.log(`still differs after 3 posts: ${todo.map((s) => s.context).join(', ')}`); break; }

Low: .github/workflows/fleet-status.yml:112,122: one failing PR stops the rest of the backfill

          set -euo pipefail
          ...
            PR="$pr" node scripts/fleet-status.mjs

gh() throws on any non-2xx. A top-level rejection exits node non-zero, and set -e then skips every PR after the first one that hits a transient 5xx or secondary rate limit. It fails loudly, which fits the rest of the job, but a re-run starts again from the first PR. Suggested (optional):

          failed=0
          for pr in $prs; do
            echo "== #$pr"
            PR="$pr" node scripts/fleet-status.mjs || { echo "::error::#$pr failed"; failed=1; }
          done
          exit "$failed"

Boundaries I rebuilt from the diff

predicate	input	behaviour	pinned by
facts.files.length > 100	99 / 100 / 101 docs files	not code / not code / code	99 docs files are not code, exactly 100 docs files is not code, more than 100 files is code whatever they are
isBotPr before the file count	bot branch with 101 files	exempt	a bot PR with more than 100 files is still not code
isBotPr identity AND branch	person on bot/, release/1.2	not a bot PR	a person on bot/ is not, a person on release/1.2 is not a bot PR
fit, s.length <= 140	140 / 141 chars	kept / cut to 137 + ...	the 140-character edge pair
requiredCiState, empty after filtering own contexts	[], only fleet/*	none (Breaker rule)	no required checks -> none, only own lanes required: none
missing / waiting / failed / skipped-neutral check	each	pending / pending / failed (failure wins over pending) / passed	four requiredCiState cases
last result per name	failure then success	passed	a rerun that passed
rules or checks unreadable	throws	pending, never green	code at :241,251; not unit-tested (CLI path), and it fails closed
verifiedAtHead label AND comment	label only / comment only / older sha / other login / findings or blocked heading / 7-char prefix	all false except the prefix	six verifiedAtHead cases
r.commitId !== facts.head for both reviewers	verdict at an older head	pending, names the old sha	verdicts on an older head
r.state === 'DISMISSED'	dismissed NOT READY at head	pending; a later READY counts	two dismissed cases (each fails if the guard is removed)
verdict regex READY[ \t\r]*$	READY, mostly, no line, Redline carrying the line	none	three cases
separator strip	-, :, U+2013, U+2014, none, leading backtick or bracket	the separator is dropped, a leading code span or bracket is kept	seven cases
deterministic approval	on code / on docs	ignored / counts	deterministic approvals
own contexts required, three rounds	fleet/* pending in the check list	converges to all green	own lanes required, three rounds

Every reachable row has a test that fails if its guard is removed. I found no assertion that holds either way.

Checked against the PR body

• The required list really is test, analyze (javascript-typescript), docker-build (from GET /rules/branches/main). None of the fleet/* contexts is required yet.
• The workflow_run list [build, ci, CodeQL, labels] matches every workflow on the branch with a pull_request trigger. PR triage is pull_request_target only and is correctly left out. The self-test enforces both.
• The status job checks out only the default branch's script (sparse, persist-credentials: false). The hashFiles guard skips it before merge, which matches the skipped step behind status pass 5s. Fork PRs are skipped in both the job if and readFacts. The token permissions are as stated.
• The script, tests and workflow are plain ASCII (0 non-ASCII chars). The test count is 111, and self-test is green at this head. I also ran the test file once locally: 111 pass, 0 fail.
• Backfill uses --limit 1000 and fails with -ge 1000, as described.
• No concurrency group; the post-then-verify loop is as described, apart from the pass-3 edge above.

What's good: the lane rules are pure functions with boundary-level tests. Unreadable rules or checks fail closed. Own-context exclusion stops fleet/verify from waiting on itself once it is required. The workflow-list test protects against a future required check that would never refresh the lanes.

Not read: the dispatcher sources (review-dispatch.sh, review-events.ts, public-automerge-sweep.ts) that the comments cite as the rules being mirrored. I checked the rules as they are written here, not whether they match the dispatcher.

SECOND READ: READY

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).

Verdict: no blocking issues at cc76931.

What changed since my last read (3eda07a): only cc76931 Merge branch 'main' into ci/fleet-status, which brings in #81 (src/config.ts, _test_unit.mjs, _test_proxy.mjs). The compare 3eda07a...cc76931 touches none of this PR's three files, so .github/workflows/fleet-status.yml, scripts/fleet-status.mjs and scripts/fleet-status.test.mjs are byte-for-byte what I read before. I re-read the full diff at this head anyway.

Boundaries, rebuilt from the diff, with the test that pins each:

predicate	input	behaviour	pinned by
needsVerify: files.length > 100	99 / 100 / 101 docs files	not code / not code / code	"99 docs files are not code", "exactly 100 docs files is not code", "more than 100 files is code whatever they are"
isBotPr before the file count	bot branch, 101 files	exempt	"a bot PR with more than 100 files is still not code"
isBotPr identity AND branch	person on bot/, release/1.2	not a bot PR	two cases in the bot block
requiredCiState own contexts filtered	only fleet/* required	none (falls back to the Breaker rule)	"only own lanes required: none"; the "still passed" case fails if the filter at scripts/fleet-status.mjs:217 is removed
requiredCiState empty/unreported	required check absent	pending	"a required check not reported yet"
requiredCiState failure beats pending	one failed, one missing	failed	"failed, even with another pending"
last result per name	failure then success	passed	"a rerun that passed"
verifiedAtHead 7-char prefix / older comment last		counts / does not	two cases
secondReadAtHead DISMISSED, older head, lineless later review, READY, mostly		none / none / keeps prior / none	each has a case
NOT READY with no reason		NOT READY at 4753643, no trailing colon	exact-string case
fit at 140 / 141		whole / 137 + ...	exact-string cases
unreadable rules or checks (catch)	API error	pending, never green	not unit-tested (CLI path); fails closed by construction at scripts/fleet-status.mjs:376 and :386

Tests that could not fail: none found. I checked each assertion with its guard removed in mind; "checks nobody requires do not hold it" would pass without the OWN_CONTEXTS filter, but the filter is pinned separately by "own lanes required and pending, CI green: still passed".

Body claims checked against the diff and CI:

• 111 unit tests: the self-test log at this head ends 111 pass, 0 fail.
• The base branch's required checks (test, analyze (javascript-typescript), docker-build) match what gh api rules/branches/main returns; nothing in the script names them.
• The workflow_run list covers every pull_request workflow at this head (build, ci, CodeQL, labels) and leaves out PR triage (pull_request_target). The self-test checks this and it passes.
• The status and backfill jobs check out only the default branch's scripts/fleet-status.mjs (sparse-checkout, persist-credentials: false). self-test runs PR code with contents: read only. No PR-controlled code runs with statuses: write.
• Backfill uses --limit 1000 and fails at -ge 1000 with ::error::, as the body says.

Not blocking, for the record: check-runs?per_page=100 at scripts/fleet-status.mjs:382 reads one page. A head with more than 100 check runs could leave a required check unread, and that shows as pending, not as a wrong green. The head has 14 today.

What's good: the lane logic is pure and exported, so the 111 cases drive the real functions rather than fakes. Every error path fails closed to pending. The post-then-re-read loop gives a bounded answer to the ordering race without a concurrency group.

SECOND READ: READY