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 (4 OS/Node combinations), analyze (javascript-typescript). 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 110 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: 110 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: changes requested — bot-branch detection disagrees with the fleet exemption rule and can incorrectly hold eligible PRs. rule:none

Blocking — correctness — scripts/fleet-status.mjs:39-43

/** Dependabot, or a bot-shaped branch opened by askalf or github-actions: verification-exempt. */
export function isBotPr(author, headRef) {
if (/^(app\/)?dependabot(\[bot\])?$/i.test(author ?? '')) return true;
return /^(askalf|(app\/)?github-actions(\[bot\])?)$/i.test(author ?? '') && BOT_BRANCH.test(headRef ?? '');
}

The dispatcher's exemption is independent: a head branch beginning bot/, release, chore/release, dependabot/, or receipts- is exempt regardless of author. For example, a same-repository PR opened by a person from bot/maintenance reaches this predicate with author === 'contributor'; it returns false, so needsVerify() classifies the PR as code and laneStatuses() leaves fleet/verify and fleet/second-read pending. Once these contexts become required, that otherwise exempt bot-branch PR cannot merge. The branch expression also only recognizes chore/release-v<digits> rather than the specified chore/release prefix.

export function isBotPr(author, headRef) {
  return /^(app\/)?dependabot(\[bot\])?$/i.test(author ?? '') || BOT_BRANCH.test(headRef ?? '');
}

I reviewed the workflow event/permission boundaries, status computation, and unit-test coverage in the diff. All required CI checks are passing at 02c80c9; I did not run the suite locally.

[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: One blocking defect. fleet/verify counts its own context as a required check, so the "next step" in the PR body (make the fleet/* contexts required) locks every code PR at pending permanently.

Read at live head cf18a53 (the ticket named 02c80c9; cf18a53 adds the edited/deleted triggers, the isBotPr doc comment and five bot-rule tests, and I read all of it).

Findings

1. High: fleet/verify deadlocks once the fleet/* contexts are required checks

scripts/fleet-status.mjs:212-213 builds the required list from the branch rules with no exclusions:

    required = rules.filter((r) => r.type === 'required_status_checks')
      .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context));

scripts/fleet-status.mjs:223 then evaluates that list against the head's statuses, and those statuses include the ones this script posts:

      requiredCi = requiredCiState(required, [...statuses, ...checks]);

The PR body says the step after merge is to "add fleet/verify, fleet/review and fleet/second-read to the default branch's required status checks", and the backfill comment at .github/workflows/fleet-status.yml says to run it "right after the fleet/* contexts become required checks". Once that is done:

• required = [test (...) x4, analyze (javascript-typescript), fleet/verify, fleet/review, fleet/second-read].
• Before the first run, fleet/verify has no status, so requiredCiState returns 'pending'. After the first run it is pending, so it returns 'pending' again.
• required.length > 0, so ci is never 'none', and the label-and-comment path at laneStatuses (ci === 'none' && verifiedAtHead(facts)) is also closed.
• verified stays false, so gated stays true, and fleet/review and fleet/second-read stay on "reads ... once it is verified" whatever Redline and the Second Read post.

I ran this against the head's module:

requiredCiState([...CI, 'fleet/verify','fleet/review','fleet/second-read'], CI all success)            -> pending
same, plus fleet/verify|review|second-read = pending                                                  -> pending
laneStatuses(... Redline APPROVED at head, requiredCi from the above) -> verify pending "Waiting on required CI",
                                                                         review/second-read pending "once it is verified"

With the three contexts required, GitHub refuses every code-PR merge from then on. Unwinding it means an admin editing the ruleset. The test at scripts/fleet-status.test.mjs:226, 'checks nobody requires do not hold it', only covers the case where a fleet/* status is present but not required. The case the PR plans for has no test.

Suggested fix:

const OWN = new Set(Object.values(CONTEXTS));
// ...
    required = rules.filter((r) => r.type === 'required_status_checks')
      .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context))
      .filter((c) => !OWN.has(c));

Suggested test, matching the file's shape (better still, move the filter into an exported helper so the test goes through the same code the CLI runs):

check('the lanes\' own contexts are not required CI',
  requiredCiState(requiredCiContexts(['test', 'fleet/verify', 'fleet/review', 'fleet/second-read']), [ok('test')]) === 'passed');

2. Low: the test count in the body is stale

The body says "node scripts/fleet-status.test.mjs: 73 pass, 0 fail" and "self-test runs the script's 73 unit tests". At cf18a53 the file has 78 check( calls, because cf18a53 added five. This is a doc fix only.

What I checked

• Head-scoping: the redlineVerdict + commitId === facts.head pair, and secondReadAtHead filtering on commitId before it parses. Old-head verdicts stay pending and say where they were (tests at the "dario#1403 morning" block and the docs older-head case).
• Boundaries: 99 vs 100 files (both pinned). 7-char vs full sha prefix (pinned). Description at exactly 140 characters and at 141 (pinned). Empty required returns 'none' (pinned). NOT READY with no reason gets no trailing colon (pinned). READY, mostly is not READY (pinned). A dismissal or comment at head does not replace an older verdict (pinned). A person on a bot-shaped branch is still verified (pinned at cf18a53).
• Ordering: statuses are reversed from newest-first to oldest-first and check-runs are sorted by id, so "last result per name" really is the newest one.
• Security: status/backfill check out the default branch's script only, with persist-credentials: false. Permissions are scoped per job with permissions: {} at the top. Fork PRs are filtered in both the if: and the script. No PR-controlled string reaches a shell. The PR number is validated ^\d+$.
• CI at cf18a53: all green, including self-test and status.

Not read: the dario#1419 twin, and pr-triage.yml interplay.

SECOND READ: NOT READY — fleet/verify requires its own context once the fleet/* checks are required (fleet-status.mjs:212-213), deadlocking every code PR at pending

[sprayberry-redline]

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

Verdict: changes requested — the replacement ordering guard still permits a stale status to overwrite a fresh one. rule:none

Blocking — correctness — scripts/fleet-status.mjs:122-124, scripts/fleet-status.mjs:249-257

return statuses.some((s) => s.context === context && Date.parse(s.created_at) > readAtMs);

const posted = dryRun ? [] : await ghAll(/repos/${repo}/commits/${facts.head}/statuses, token);
for (const s of laneStatuses(facts)) {
...
if (postedSince(posted, s.context, readAt)) { console.log(' (a newer run already posted this; skipped)'); continue; }

readAt is parsed from the HTTP Date header and GitHub status created_at values are second-granularity timestamps. If run A reads at 03:00:10.1, a review changes, and run B reads the new review and posts fleet/review at 03:00:10.8, both timestamps parse to 03:00:10Z. The strict > returns false, so A can then post its stale status after B. Because B has already completed, nothing overwrites A's result; the lane can remain green/red for the superseded review. The newly added test explicitly treats this same-second case as safe, so CI does not cover the failing ordering boundary.

export function postedSince(statuses, context, readAtMs) {
  if (!Number.isFinite(readAtMs)) return false;
  return statuses.some((s) => s.context === context && Date.parse(s.created_at) >= readAtMs);
}

Please also change the same-second test to require skipping and consider using a server-provided value with finer precision or an ordering token if GitHub cannot distinguish a status that predates this run within that second.

I reviewed the workflow event changes, the stale-status ordering path, and the corresponding unit tests. Required CI is green at 0ad08f7; I did not run the suite locally.

What's good: expanding the review/comment triggers and replacing cancellation with an explicit freshness check addresses the right failure mode, and the added tests cover the normal before/after ordering cases.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: changes requested — the added public test comments narrate the patch/history rather than using the test file's direct idiom. rule:reads-as-generated

Blocking — generated-writing tell — scripts/fleet-status.test.mjs:275

// Feed each run's statuses back in as the next run's checks, three rounds, as the Second Read did.

This is patch-history narration in a test comment (and names the review process), not an explanation needed to understand the test. First-party strict mode requires public test text to avoid generated-style patch narration. Remove the comment or replace it with the file's concise, behavior-focused idiom; the test's arrange/act/assert code already makes the setup clear.

  let posted = [];
  let states = [];
  for (let round = 0; round < 3; round++) {

What's good: I reviewed the live-head delta that excludes the fleet contexts before computing required CI, including its self-cycle and pending-real-CI coverage. Required CI is green at 9e4ea5d; I did not run the local suite.

[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 9137339. One low-severity divergence from the dispatcher, on an error path. The comment at that line is wrong.

Scope: the ticket named 0ad08f7, but the live head is 9137339 (three commits later), so I reviewed 9137339. I read the full diff (workflow, script, tests), compared the script and tests against askalf/dario#1419 at d8225d9, and compared the rules against platform's review-events.ts, ci-verify.ts and tools/review-dispatch.sh. CI is green at head (gh pr checks). I did not run the suite locally.

Findings

Low: unreadable branch rules fall back to "none", but the dispatcher waits

scripts/fleet-status.mjs:233-240 (diff lines 363-370):

    // Unreadable rules count as none (the label-and-comment rule applies); unreadable checks as
    // pending. Neither can turn fleet/verify green.
    let required = [];
    try {
      const rules = await (await gh(`/repos/${repo}/rules/branches/${encodeURIComponent(p.base.ref)}?per_page=100`, token)).json();
      ...
    } catch { required = []; }

Trace: the rules GET fails (5xx or a secondary rate limit). required = [] → requiredCi = 'none' → laneStatuses computes verified = code && (ci === 'none' && verifiedAtHead(facts)). If the PR has the verified label and a ## Verification at <head> comment, fleet/verify goes green ("Verified at …"). That can happen even while a required check is failing at that head. In that case the comment's "Neither can turn fleet/verify green" is false.

The dispatcher it says it mirrors does the opposite in both places. ci-verify.ts returns 'pending' when requiredContexts is null. Its comment says a failed read "waits", after the 2026-09-25 01:45Z rate-limit incident. review-dispatch.sh ci_state() sets CI_STATE="pending" when the rules read fails.

Impact is small. The real CI checks are still required on their own, so this can't merge a red head. It also needs a Breaker verification at head on a repo that verifies by CI, and the next event corrects it. That is why this is not blocking. Still, it's a fail-open on an error path that contradicts the comment directly above it. It would matter more once fleet/verify is itself a required check.

Suggested fix:

    let required = null;
    try {
      const rules = await (await gh(`/repos/${repo}/rules/branches/${encodeURIComponent(p.base.ref)}?per_page=100`, token)).json();
      required = rules.filter((r) => r.type === 'required_status_checks')
        .flatMap((r) => (r.parameters?.required_status_checks ?? []).map((c) => c.context));
    } catch { required = null; }
    let requiredCi = required === null ? 'pending' : 'none';
    if (required?.length) {

Also update the comment to "Unreadable rules or checks count as pending".

What I checked (the boundary ledger, rebuilt from the diff)

• requiredCiState: required list empty → none; only own fleet/* contexts required → none; a required check missing, or PENDING/IN_PROGRESS → pending; any failure, even alongside a pending check → failed; a failure followed by a passing rerun → passed; SKIPPED/NEUTRAL → pass. Each case is pinned by a named check. The three-round self-reference test really does fail without the OWN_CONTEXTS filter: in round 0 the own contexts are missing, so the result is pending.
• needsVerify at 99, 100 and 101 docs files, a bot branch with 100 files, and 101 docs files with required CI passed: all pinned. > 100 matches readPrFacts (hasNextPage on files(first: 100)). Note that review-dispatch.sh:616 uses >= 100, so at exactly 100 docs files the two platform dispatchers disagree with each other. That predates this PR, and the script follows the one it cites.
• secondReadAtHead: tested cases are no verdict line, READY followed by text, NOT READY with no reason (no trailing colon), the last verdict line in a body, the latest review at head, a lineless follow-up, an older-head verdict, another login carrying the line, and dismissed-then-READY. I also checked by hand that CRLF bodies work: READY\r matches via [ \t\r]*, and NOT READY — why\r\n captures NOT READY — why.
• redlineVerdict: DISMISSED and COMMENTED reviews are skipped, deterministic approvals don't count on code but do on docs, and CHANGES_REQUESTED after APPROVED at the same head is red. All pinned.
• The 140-character cap: 140 is kept whole, 141 is cut to 137 plus .... Both pinned.
• Every new assertion discriminates against the rule it names. I found none that holds either way.
• Workflow safety: status and backfill check out only the default branch's scripts/fleet-status.mjs, and persist-credentials: false. The only job that runs PR code is self-test, with contents: read and no secrets. PR comes in through env and is validated with /^\d+$/. Fork PRs are excluded both in the if: and in readFacts. workflow_run is limited to the same head_repository and has a non-null pull_requests[0]. I found no pwn-request path.
• PR body claims: 94 check() calls (counted). scripts/fleet-status.mjs has the same blob as dario#1419 (8b338ee). The test file differs from dario's test/fleet-status.mjs only in its import path. No non-ASCII bytes anywhere in the diff. No concurrency group. Permissions match the list in the body. CI and CodeQL are the two workflows that produce the required contexts on master.

What's good: the pure laneStatuses core is separate from the I/O. The post-then-verify loop is bounded and has tests for stale overwrites. The required-CI rule is a line-for-line port of ci_state(), reading statuses in reverse and check runs sorted by id.

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 — bot-shaped branches that the fleet must exempt are still routed into verification. rule:none

Blocking — correctness — scripts/fleet-status.mjs:39-43

const BOT_BRANCH = /^(bot\/|release\/|release-v?[0-9]|chore\/release-v?[0-9]|dependabot\/|receipts-)/;
export function isBotPr(author, headRef) {
if (/^(app\/)?dependabot(\[bot\])?$/i.test(author ?? '')) return true;
return /^(askalf|(app\/)?github-actions(\[bot\])?)$/i.test(author ?? '') && BOT_BRANCH.test(headRef ?? '');
}

The first-party gate exempts any head branch beginning bot/, release, chore/release, dependabot/, or receipts-; it does not require the PR author to be askalf or GitHub Actions. For example, a same-repository PR from bot/maintenance opened by contributor makes this predicate return false, so needsVerify() treats it as code and leaves fleet/verify pending. After these lane contexts are required, that exempt PR is blocked waiting for verification it must not need. The narrower chore/release-v?[0-9] pattern also excludes valid chore/release/... branches.

export function isBotPr(author, headRef) {
  return /^(app\/)?dependabot(\[bot\])?$/i.test(author ?? '') || BOT_BRANCH.test(headRef ?? '');
}

Update the tests that currently assert !isBotPr('contributor', 'bot/maintenance') to pin the exemption rule.

I reviewed the live-head rework, including the status reconciliation loop, review-state handling, required-CI filtering, workflow permissions and triggers, and the new unit coverage. All required CI checks are passing at 9137339; I did not run the suite locally.

What's good: replacing the timestamp comparison with post-then-re-read reconciliation removes the same-second ordering ambiguity in the previous implementation, and the dismissed-review handling is explicitly covered.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: approved — the live-head rework fixes the prior gating finding; required CI is green and the added workflow coverage is consistent with the implementation.

No blocking issues found in the live-head diff.

What I checked: all required CI checks are passing at e5c6db5. I reviewed the full workflow, status computation, and unit-test diff, then specifically re-read the rework from 9137339 to e5c6db5. requiredCiState() now excludes the three fleet/* contexts before evaluating required CI, preventing fleet/verify from waiting on itself when those contexts become required. The live-head change adds Arena to .github/workflows/fleet-status.yml:20 and the test dynamically verifies that every workflow using pull_request is included while the pull_request_target-only workflow remains excluded (scripts/fleet-status.test.mjs:332-361). I also checked workflow trigger types, scoped permissions, same-repository filtering, status reconciliation, and the first-party generated-text pass. I did not run the suite locally.

[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 3bcfab8. One low-severity gap in test coverage, noted below.

Scope: I read the whole PR diff at 3bcfab8, plus the delta since my last read at 9137339 (commits a9c4ad6, 01f9f4f, e5c6db5, 3bcfab8: fleet-status.yml +8/-3, fleet-status.mjs +8/-7, fleet-status.test.mjs +70/-4). CI at this head: self-test 110 pass / 0 fail; test x4, analyze and CodeQL are green. I did not run anything locally.

Finding

Low: scripts/fleet-status.mjs:236-243, the unreadable-rules branch has no test

    let required = null;
    ...
    } catch { required = null; }
    let requiredCi = required === null ? 'pending' : 'none';
    if (required?.length) {

The behaviour is correct, and it now matches the dispatcher (ci-verify.ts returns pending when required === null and none only when the rules were read and came back empty). The problem is that nothing would catch a regression. Here's the failure scenario: someone restores catch { required = [] }, and the rules API then returns a 5xx on a code PR that carries verified plus a verification comment at the head. requiredCi becomes 'none', laneStatuses takes the label-and-comment path, and fleet/verify turns green while required CI is still unknown. All 110 tests would still pass, because this logic sits in the I/O block that the unit tests never reach. It fails closed today, so I'm not blocking on it.

Suggested fix:

export function ciFromRules(required, checks) {
  if (required === null) return 'pending';
  return requiredCiState(required, checks);
}
// test
check('unreadable branch rules wait instead of falling back to the label', ciFromRules(null, []) === 'pending');

Boundaries rebuilt from the delta

Row	Code at 3bcfab8	Pinned by
needsVerify, 100 docs files	not code (matches the dispatcher's GraphQL files(first:100) with hasNextPage false)	exactly 100 docs files is not code
101 docs files, a person's PR	code, pending	a person with more than 100 docs files is code
101 files, bot branch	exempt, because the bot rule is checked before the count	a bot PR with more than 100 files is still not code
Reason strip, NOT READY followed by U+2014 / U+2013 / - / :	only the separator is removed	separator U+2014 is dropped and the three siblings
Reason starting with `, (, [	first character kept	code-span, colon and no-separator cases. These fail under the old \W* pattern, so they discriminate; the U+2014-only cases would pass either way
Bare NOT READY with no reason	empty reason, description NOT READY at <h>	existing verdict-line tests
Rules read fails	pending	not pinned (finding above)
workflow_run list against the repo's on: pull_request workflows	Arena, CI and CodeQL listed; PR triage (pull_request_target only) left out	the generated per-workflow checks. They read the real .github/workflows directory, so a new PR workflow that is missing from the list fails the suite
Backfill with more than 100 open PRs	--limit 1000, -ge 1000 exits with ::error::	backfill reads more than one page and fails at its cap (both fail if the old --limit 100 comes back)

PR body claims checked against the diff

• 110 tests pass: confirmed in the self-test log at this head.
• "The script and tests are plain ASCII": confirmed. There are 0 non-ASCII characters in the three files. The em dash is built with String.fromCharCode(0x2014).
• pull_request_target workflows left out: confirmed (fleet-status.yml:18, [Arena, CI, CodeQL]).
• fleet/* contexts never counted as CI: OWN_CONTEXTS filtering in requiredCiState is unchanged since 9137339.
• No concurrency group; post-then-reverify up to three passes: unchanged, and the header comment still says so.
• Backfill pages past 100 and fails at 1000: confirmed.

What's good

The fail-closed change to the rules read brings the lanes in line with the dispatcher. The separator-only strip fixes a real defect: the old \W* pattern turned `x` is null into x` is null. The workflow-list test is derived from the repo rather than hard-coded, so it keeps holding as workflows are added.

SECOND READ: READY