ci: give a lost runner a name, a last file, and a row (B5 of #110)

[mobileskyfi]

B5 of #110 — part 2 of #77, the half B4 explicitly could not do. B4 bounded a file that hangs; this makes a lost runner diagnosable.

A leg whose runner stops talking left nothing behind: no artifact, no metrics record, no row anywhere. The 2026-07-31 sweep planned 15 integration legs and ci-data received 12 ndjson files, with no marker for the missing three. A reader comparing platforms saw silence and had to guess whether macos-x86 had never run or had died trying.

Both open design decisions got a maintainer call first

Recorded on #110 before any code was written, per the bite's instruction not to pick silently.

Decision 1 — a separate ledger, not tested-versions.json. The version scheduler reads that file as a presence test (ros-versions.yml:99), so writing incomplete into conclusion would make an aborted run look tested and silently stop rescheduling that version, forever, with no error. Today's macos-x86 case would not trip it (the scheduler only reads linux-x86), but the contract is one platform away from breaking. So: a new ci-data/attempted-legs.json, and that jq is untouched. foldTestedVersions's scope:"full" rule is unchanged.

Decision 2 — a per-leg check run. Rejected alternatives, one of them on evidence gathered before claiming.

What survives losing a runner — measured, not assumed

The test loop already prints ::notice::Running <file> per file, so scraping the job log would have cost nothing. It does not survive. On run 30665449265:

job 91271309625  macos/x86_64 · stable     → BlobNotFound
job 91271309655  macos/x86_64 · testing    → BlobNotFound
job 91271309687  macos/x86_64 · long-term  → BlobNotFound
job 91271309635  linux-x86 (green)         → 868 lines
job 91271309686  windows-x86 (RED, normal) → 790 lines

The contrast with the normally failing windows leg is the load-bearing half: failure preserves the log; runner loss destroys it. The jobs API step ledger, by contrast, does survive — a vanished leg still records its test step as in_progress with a start time. That is free and now used, but it stops at step granularity and can never name the file. This table is recorded in DESIGN.md so the next instrument does not re-derive it.

The design

scripts/ci-leg-checkpoint.ts	Opens a check run per leg, PATCHes it after every file — last completed file, what is running now, outcome/duration/cap, and a resource sample (free memory, load, free disk, live QEMU count). A leg that dies leaves it un-closed; the absence of a terminal is the signal.
scripts/ci-leg-ledger.ts	Runs in aggregate: compares plan's matrix against the legs that reached ci-data/runs/, writes attempted-legs.json, annotates the run summary, and closes check runs the runner never got to.
scripts/ci-host-snapshot.ts	The host readings, extracted from B4's watchdog into one home — same rule COLD_DOWNLOAD_FLOOR_BYTES_PER_S got in B13. These numbers get compared across the two instruments, so a second copy free to drift would make the comparison noise.

The artifact outranks both instruments. A leg is complete because it produced a metrics record — never because a check run says so. The checkpoint is best-effort (warns and exits 0 on any API failure, so a fork PR's read-only token or a dropped PATCH cannot red a leg), which means trusting it first would let instrument failure fabricate a runner-lost for a leg that finished cleanly. There is an anchor test for exactly that.

Verdicts: complete, runner-lost, attempted-incomplete, not-started. That last one is not in #77's vocabulary and is deliberate — without it a leg that died in Install QEMU is indistinguishable from one whose runner vanished mid-suite, and mislabelling a setup failure as runner-lost sends #76 chasing a mechanism that was never involved.

Verification

1. Replay against the real sweep (committed as an anchor test). The classifier, fed the actual job records from run 30665449265 — 15 planned, 12 complete, the three macos-x86 legs named runner-lost, all 15 jobs matched by name. Fixture trimmed to the fields classification reads. This is the only test driven by a genuine runner-loss event rather than one written to match its own expectations.

2. Happy path on a real runner — run 30750868899, green. Check run posted, marked and closed; ledger wrote 1/1 legs complete and committed to ci-data with no incomplete map.

3. An intentionally killed runner — the bite's stated exit criterion, which cannot be verified locally. Run 30750979859, dispatched from a temporary commit that sudo halt -f -ps the runner after the first file. That commit was dropped and the branch force-pushed before this PR was opened — it is not in the diff, and no such lever ships.

The reproduction matched #76 exactly: step 17 stuck in_progress, steps 18+ never started, if: always() never fired, no artifact, and the job log blob returns BlobNotFound — a second, self-controlled confirmation of the log finding above. What the ledger recorded anyway:

"linux-x86|stable": {
  "terminal": "runner-lost",
  "last_file": "disk.test.ts",
  "current_file": "exec.test.ts",
  "last_checkpoint_ts": "2026-08-02T13:57:25.584Z",
  "files_reported": 1, "files_planned": 2,
  "stalled_step": "Run integration tests (sequential per-file)",
  "job_matched": true, "job_elapsed_s": 2760
}

and the aggregate closed the orphaned check run as runner-lost — last file disk.test.ts.

The gap between the wedge and the verdict is ~45 minutes, and the checkpoint closes it

This run measured something #110 had listed as uncharacterized. The runner was halted at 13:57:26; GitHub declared the job failed at 14:42:03. The service waited ~44.6 minutes before deciding the runner was gone.

last_checkpoint_ts is 13:57:25 — within two seconds of the actual wedge, and 44.6 minutes earlier than completed_at. That is the whole value of the instrument: job_elapsed_s: 2760 is an upper bound that overstates the wedge by ~16x here, and the checkpoint replaces it with a timestamp.

Do not carry the 44.6 min figure across platforms or loss modes (#110 rule 6). It is one measurement, on ubuntu-latest, of a clean halt — a silent network partition or a wedged-but-alive runner may well get a different verdict latency. What it does establish is that the interval is tens of minutes, not seconds, so reading completed_at as the wedge time is wrong by a lot. Whether #76's macos-x86 legs go quiet far earlier than their 62-65 min completed_at is now a measurable question rather than an unanswerable one — B8a is the bite that should answer it, and it now has an instrument that can.

Notes for review

• Permissions cross a reusable-workflow boundary. The integration job needs checks: write; aggregate needs checks: write + actions: read. Reusable-workflow permissions are capped by the calling job, so main.yml and sweep.yml grant both. A caller that forgets them gets warnings and an empty ledger, not a red run.
• One silent-degradation risk, deliberately fenced. The jobs API exposes no matrix values, so joining a job to a planned leg goes through the display-name string. integrationJobName() is its one home and is asserted against production job names by a test; if integration.yml's name: is edited without it, every entry degrades to job_matched: false.
• hostUptimeS replaces B4's uptimeS, which reported process.uptime() — the watchdog process's own life, duplicating its elapsed_s and reading ~0 in every checkpoint. Renamed rather than reused so nothing reads a checkpoint's number as if it meant the watchdog's.
• No CHANGELOG entry — CI-internal, nothing user-facing, same as B4 (ci: bound each integration test file, and name the one that wedged (B4 of #110) #122) and B3 (ci: key the CHR image cache by resolved version, with one owner (B3 of #110) #117).
• ci-data README updated — done, 54a2192 on the data branch. attempted-legs.json has been written by the aggregate job since run 30750868899 but was undocumented; the README now carries the four terminals, the artifact-outranks-the-instrument rule, why last_checkpoint_ts rather than job_elapsed_s is the timestamp to read, and a query recipe verified against the real file.

This PR is deliberately linked to #77 — check that before merging

Per the #112 → #79 hazard, a PR should link its sub-issue only when it satisfies that issue's whole done-when. This one does, so the link is intentional rather than accidental:

#77 done-when	Bite
A hung file fails before the runner-loss boundary with name, cap, host snapshot, cleanup result	B4 (#122) ✅
Timeout cleanup verified; CI does not continue through a poisoned environment	B4 (#122) ✅
Runner disappearance leaves a server-visible runner-lost and last completed/current file	this PR ✅
Per-file timeout outcomes flow into the rollup, and expected incomplete legs are visible	B4 (first half) + this PR (second half) ✅
Caps derived from cited post-2899be4 data, checked in, documented	B4 (#122) ✅

The one judgement call is resolved: the link stays, and the residue is filed as #125. #77's Proposed approach §4 also lists operation-timeout, infra-download / infra-cache and cancelled / superseded, which are not implemented — they need output classification neither bite does, and ci-file-watchdog.ts:113 says so rather than leaving it silent. They are not in the done-when, and #110's rule is that a PR links its sub-issue when it satisfies that issue's whole done-when — which this does, all five rows above. So closing #77 here is correct, and #125 exists so the unbuilt half does not evaporate with it.

Closes #77.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added CI progress checkpoints for individual test files, with persistent status and timing information.
  • Added recovery tracking for interrupted or incomplete integration runs.
  • Added per-run reporting that distinguishes completed, interrupted, attempted-incomplete, and not-started test legs.
  • CI now captures available host and accelerator details during execution.

• Documentation

  • Added guidance for investigating lost-runner and incomplete-run scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

CI runner-loss recovery

Layer / File(s)	Summary
Host snapshots and watchdog sidecar scripts/ci-host-snapshot.ts, scripts/ci-file-watchdog.ts, .github/instructions/ci.instructions.md, DESIGN.md, project-words.txt	Shared host metrics support cross-platform disk and QEMU readings. The watchdog writes per-file outcome sidecars for skipped, completed, and timed-out files. CI documentation defines the ledger verdicts and evidence model.
Leg checkpoint lifecycle scripts/ci-leg-checkpoint.ts, test/unit/ci-leg-checkpoint.test.ts	The checkpoint CLI manages persisted leg state and GitHub check runs through open, mark, and close. Tests cover IDs, payloads, rendering, malformed input, and leg keys.
Workflow checkpoint wiring .github/workflows/integration.yml, .github/workflows/main.yml, .github/workflows/sweep.yml	Integration jobs receive check and action permissions, initialize and update checkpoints, close them after testing, upload state, and pass ledger output to metrics commits.
Leg ledger reconciliation scripts/ci-leg-ledger.ts, test/unit/ci-leg-ledger.test.ts, test/unit/fixtures/sweep-30665449265-jobs.json	The ledger classifies legs as complete, runner-lost, attempted-incomplete, or not-started using metrics, check runs, and jobs. Tests cover reconciliation, job matching, elapsed times, filesystem data, and runner-loss fixtures.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant IntegrationWorkflow
  participant ci-file-watchdog
  participant ci-leg-checkpoint
  participant GitHubChecks
  participant ci-leg-ledger
  participant MetricsArtifacts
  IntegrationWorkflow->>ci-leg-checkpoint: Open leg checkpoint
  ci-leg-checkpoint->>GitHubChecks: Create in-progress check run
  ci-file-watchdog->>IntegrationWorkflow: Write per-file sidecar
  IntegrationWorkflow->>ci-leg-checkpoint: Mark file outcome and host snapshot
  ci-leg-checkpoint->>GitHubChecks: Update check-run output
  IntegrationWorkflow->>ci-leg-checkpoint: Close leg checkpoint
  IntegrationWorkflow->>ci-leg-ledger: Reconcile planned legs
  ci-leg-ledger->>MetricsArtifacts: Read completed leg records
  ci-leg-ledger->>GitHubChecks: Inspect checkpoint runs
  ci-leg-ledger->>ci-leg-ledger: Classify and persist attempted legs

Loading

Possibly related issues

• CI: evidence-preserving per-file watchdog and incremental run ledger #77 — Covers the same per-file watchdog, persistent checkpoint, and attempted-leg ledger behavior.

Possibly related PRs

• tikoci/quickchr#122 — The watchdog changes extend its timing and outcome behavior with sidecar records.
• tikoci/quickchr#108 — The checkpoint flow surrounds the integration test execution changed by this PR.
• tikoci/quickchr#10 — Both changes address QEMU handling and runner-loss recovery in CI.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.10% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding runner-loss identification, last-file tracking, and ledger reporting.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/77-incomplete-leg-ledger

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Adds CI instrumentation to make “runner lost communication” cases diagnosable by (1) pushing per-file progress to a server-side check run during the leg, and (2) writing a planned-vs-completed ledger (attempted-legs.json) during aggregation so missing legs get an explicit terminal classification instead of vanishing from ci-data.

Changes:

• Introduces per-leg check-run checkpointing (ci-leg-checkpoint.ts) and an aggregate reconciliation/ledger (ci-leg-ledger.ts) persisted to ci-data/attempted-legs.json.
• Refactors host resource snapshotting into a shared module (ci-host-snapshot.ts) and wires the watchdog to emit a sidecar consumed by the checkpoint marker.
• Updates integration workflows and callers to grant the needed checks: write / actions: read permissions and to run/close the checkpoints.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/unit/fixtures/sweep-30665449265-jobs.json	Adds real jobs-API fixture data from the cited sweep run for replay tests.
test/unit/ci-leg-ledger.test.ts	Anchor tests for leg classification and ledger building (including replay of the real runner-loss fixture).
test/unit/ci-leg-checkpoint.test.ts	Anchor tests for the checkpoint external_id and payload round-trip contract.
scripts/ci-leg-ledger.ts	Implements planned-vs-completed reconciliation, runner-loss classification, and attempted-legs.json writing + orphan check-run finalization.
scripts/ci-leg-checkpoint.ts	Implements per-leg check-run open/mark/close with machine-readable payload embedded in check output.
scripts/ci-host-snapshot.ts	Extracts and centralizes host snapshot + QEMU process counting used by watchdog and checkpoints.
scripts/ci-file-watchdog.ts	Writes a “last file” sidecar for each file run and consumes shared host snapshot/QEMU counting.
project-words.txt	Adds new vocabulary used by the new CI/docs text for cSpell.
DESIGN.md	Documents what evidence survives runner loss and the resulting instrumentation design constraints.
.github/workflows/sweep.yml	Grants required permissions to the reusable workflow for checkpoints/ledger.
.github/workflows/main.yml	Grants required permissions to the reusable workflow for checkpoints/ledger.
.github/workflows/integration.yml	Wires checkpoint open/mark/close into each leg, and runs the ledger step in aggregate; adjusts permissions accordingly.
.github/instructions/ci.instructions.md	Documents the new incomplete-leg ledger, sidecar, and new data flow.

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/ci-leg-checkpoint.ts`:
- Around line 254-256: Update saveState to use asynchronous Bun.write() instead
of writeFileSync, and make it return/await the write promise. Update every
saveState call site in open(), mark(), and close() to await the operation,
preserving the existing state serialization and execution order.

In `@scripts/ci-leg-ledger.ts`:
- Line 463: Update the summary write in the step-summary flow to use
appendFileSync directly, preserving the existing `${lines.join("\n")}\n` content
and avoiding the read-modify-write pattern currently using readFileSync and
writeFileSync.
- Line 437: Replace the full-file write using writeFileSync in the async build()
flow with awaited Bun.write(), preserving the existing path and serialized
content, including indentation and trailing newline.
- Around line 350-354: The JSON parsing in scripts/ci-leg-ledger.ts lines
350-354 and 435-435 must not abort reconciliation. In the per-line parser within
the NDJSON loop, catch parse failures, emit a ::warning::, and continue
processing subsequent lines; in the attempted-legs.json parsing flow, catch
failures, emit a warning, and fall back to an empty object so build() can still
record the ledger and run finalizeLostCheckRuns.
- Around line 370-398: Update the PATCH request in the check-run completion flow
to retain the fetch response, inspect its ok status, and log the HTTP status
when the response is unsuccessful. Keep the existing request payload and timeout
behavior unchanged, and ensure non-2xx responses produce diagnostic output
instead of being silently ignored.

In `@test/unit/ci-leg-ledger.test.ts`:
- Around line 285-301: Extract the duplicated join logic from realJobMap into an
exported joinJobsByLeg function in ci-leg-ledger.ts, accepting the job-name map
and planned legs and returning the leg-keyed job map. Update fetchJobs to call
joinJobsByLeg and update the test helper to call the same production function,
so the assertion exercises the actual join implementation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 54ee2331-b9b0-4213-802f-7eda3a7d3f08

📥 Commits

Reviewing files that changed from the base of the PR and between 63254e7 and 9e96525.

📒 Files selected for processing (13)

• .github/instructions/ci.instructions.md
• .github/workflows/integration.yml
• .github/workflows/main.yml
• .github/workflows/sweep.yml
• DESIGN.md
• project-words.txt
• scripts/ci-file-watchdog.ts
• scripts/ci-host-snapshot.ts
• scripts/ci-leg-checkpoint.ts
• scripts/ci-leg-ledger.ts
• test/unit/ci-leg-checkpoint.test.ts
• test/unit/ci-leg-ledger.test.ts
• test/unit/fixtures/sweep-30665449265-jobs.json

📜 Review details

⏰ Context from checks skipped due to timeout. (6)

• GitHub Check: copilot-pull-request-reviewer
• GitHub Check: Unit Tests (windows-latest)
• GitHub Check: Unit Tests & Coverage
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (actions)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In Bun-based TypeScript code, use Bun.spawn(), Bun.write(), Bun.sleep(), bun:test, and ESM imports with .ts extensions.

Files:

• test/unit/ci-leg-checkpoint.test.ts
• scripts/ci-leg-ledger.ts
• test/unit/ci-leg-ledger.test.ts
• scripts/ci-host-snapshot.ts
• scripts/ci-leg-checkpoint.ts
• scripts/ci-file-watchdog.ts

test/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not turn a red integration test green by broadening timeouts, skipping it, or platform-gating it before reproducing and root-causing the failure.

Files:

• test/unit/ci-leg-checkpoint.test.ts
• test/unit/ci-leg-ledger.test.ts

**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.ts: Use Bun APIs and tooling rather than Node.js equivalents: Bun.spawn(), Bun.write(), Bun.sleep(), bun test, and bun:test. Use ESM with .ts extensions in imports; do not use CommonJS.
For ARM64 virt machines, never use if=virtio for drives; use an explicit -device virtio-blk-pci,drive=drive0 configuration.
When using HVF acceleration, use -cpu host, not cortex-a710.
For arm64 guests on macOS, automatically select TCG with -cpu cortex-a710; HVF cannot run the CHR image's 32-bit ARM userspace on Apple Silicon. --accel and QUICKCHR_ACCEL must override this selection for testing.
UEFI pflash code and vars units must be identical in size.
QGA is x86-only; do not assume the guest agent starts for arm64 CHR.
Use tabs for indentation.
Do not add unnecessary comments to obvious code.
Errors must be thrown as QuickCHRError(code, message, installHint?).
Preserve the documented public API types and behavior: QuickCHR.start(opts) returns ChrInstance; ChrInstance provides stop(), remove(), rest(), monitor(), serial(), and qga(); and MachineState represents persisted machine.json state.

Files:

• test/unit/ci-leg-checkpoint.test.ts
• scripts/ci-leg-ledger.ts
• test/unit/ci-leg-ledger.test.ts
• scripts/ci-host-snapshot.ts
• scripts/ci-leg-checkpoint.ts
• scripts/ci-file-watchdog.ts

test/unit/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Unit tests must be fast and must not require QEMU.

Files:

• test/unit/ci-leg-checkpoint.test.ts
• test/unit/ci-leg-ledger.test.ts

🧠 Learnings (4)

📚 Learning: 2026-07-28T00:12:59.340Z

Learnt from: mobileskyfi
Repo: tikoci/quickchr PR: 101
File: .github/workflows/integration.yml:342-342
Timestamp: 2026-07-28T00:12:59.340Z
Learning: For tikoci/quickchr, review workflow changes to ensure GitHub Actions are pinned to immutable commit SHAs as a repository-wide policy. If a PR introduces SHA pinning for only part of a workflow (or only some workflows) without extending the same policy across the relevant workflow files, flag it. When adding/changing SHA pinning, also verify Dependabot is configured to update the `github-actions` ecosystem so action version bumps are managed consistently (e.g., in the repo’s Dependabot configuration), rather than doing an isolated pinning change in an unrelated PR.

Applied to files:

• .github/workflows/main.yml
• .github/workflows/sweep.yml
• .github/workflows/integration.yml

📚 Learning: 2026-07-31T11:56:27.561Z

Learnt from: mobileskyfi
Repo: tikoci/quickchr PR: 117
File: .github/workflows/integration.yml:188-188
Timestamp: 2026-07-31T11:56:27.561Z
Learning: For the tikoci/quickchr repository, do not request isolated SHA pinning of first-party GitHub Actions in a single workflow or pull request. Treat action pinning as repository-wide work: first configure Dependabot for the github-actions ecosystem, then mechanically pin all workflow action references, and finally decide whether to enforce zizmor in CI. Apply this guidance to workflow files while issue `#118` tracks the rollout.

Applied to files:

• .github/workflows/main.yml
• .github/workflows/sweep.yml
• .github/workflows/integration.yml

📚 Learning: 2026-07-31T11:56:40.455Z

Learnt from: mobileskyfi
Repo: tikoci/quickchr PR: 117
File: scripts/ci-cache-key.ts:68-68
Timestamp: 2026-07-31T11:56:40.455Z
Learning: In Bun TypeScript files, do not flag use of `node:fs.appendFileSync` when append semantics are required and `Bun.write()` cannot provide them. This applies to cases such as appending multiple entries to `$GITHUB_OUTPUT` or writing to the boot log; use append-capable file operations rather than overwriting existing content.

Applied to files:

• test/unit/ci-leg-checkpoint.test.ts
• scripts/ci-leg-ledger.ts
• test/unit/ci-leg-ledger.test.ts
• scripts/ci-host-snapshot.ts
• scripts/ci-leg-checkpoint.ts
• scripts/ci-file-watchdog.ts

📚 Learning: 2026-07-31T11:56:38.870Z

Learnt from: mobileskyfi
Repo: tikoci/quickchr PR: 117
File: scripts/ci-cache-key.ts:110-110
Timestamp: 2026-07-31T11:56:38.870Z
Learning: In Bun CI scripts under scripts/, use a plain Error for validation failures when the script catches the error and renders it as a GitHub ::error:: annotation. QuickCHRError is intended for programmatic library callers and is not required for this CLI-only error path.

Applied to files:

• scripts/ci-leg-ledger.ts
• scripts/ci-host-snapshot.ts
• scripts/ci-leg-checkpoint.ts
• scripts/ci-file-watchdog.ts

🪛 zizmor (1.28.0)

.github/workflows/main.yml

[warning] 46-46: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)

.github/workflows/sweep.yml

[warning] 39-39: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)

.github/workflows/integration.yml

[warning] 427-427: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)

---

[warning] 869-869: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🔇 Additional comments (22)

scripts/ci-leg-ledger.ts (5)

44-135: LGTM!

---

141-221: LGTM!

---

226-254: LGTM!

---

258-286: LGTM!

---

296-334: LGTM!

test/unit/ci-leg-ledger.test.ts (5)

1-61: LGTM!

---

63-151: LGTM!

---

153-218: LGTM!

---

220-259: LGTM!

---

352-364: LGTM!

test/unit/fixtures/sweep-30665449265-jobs.json (1)

1-569: LGTM!

scripts/ci-host-snapshot.ts (1)

1-113: LGTM!

scripts/ci-file-watchdog.ts (1)

65-67: LGTM!

Also applies to: 206-233, 289-289, 319-319, 349-349

.github/instructions/ci.instructions.md (1)

153-272: LGTM!

DESIGN.md (1)

230-256: LGTM!

project-words.txt (1)

59-59: LGTM!

Also applies to: 429-429

scripts/ci-leg-checkpoint.ts (1)

1-243: LGTM!

Also applies to: 258-350

test/unit/ci-leg-checkpoint.test.ts (1)

1-149: LGTM!

.github/workflows/integration.yml (2)

422-427: LGTM! Verified the checks: write/actions: read scoping matches the aggregate job's needs, mark/close shell calls use || true correctly under set -eo pipefail, and the reconcile step runs after the fold, consistent with the artifact-first precedence documented in ci.instructions.md. The zizmor undocumented-permissions hint (line 427) and template-injection hint (line 869, steps.run.outcome is a fixed GitHub-generated enum) are false positives here — the permissions block already carries an explanatory comment, and steps.run.outcome is not attacker-controllable.

Also applies to: 771-790, 809-809, 834-842, 855-870, 1045-1046, 1067-1068, 1097-1103, 1127-1146, 1155-1158

---

583-583: LGTM!

Also applies to: 594-597

.github/workflows/main.yml (1)

42-48: LGTM! The zizmor undocumented-permissions hint on line 46 is a false positive — the block already carries an explanatory comment directly above it.

.github/workflows/sweep.yml (1)

34-41: LGTM! The zizmor undocumented-permissions hint on line 39 is a false positive — the block already carries an explanatory comment directly above it.

[mobileskyfi]

All six CodeRabbit findings applied in c048186; the earlier CodeQL and code-quality findings are in cda0562 / 96bc413.

The one that mattered. completedLegs parsed every ndjson line unguarded. A single truncated record would throw out of build() — so no ledger would be written and finalizeLostCheckRuns would never run, leaving the lost-runner check runs stuck in_progress. The script would have failed shut in precisely the circumstances it exists for, and it would have looked like the instrument simply had nothing to say. Now a bad line costs that line, with a warning naming the file.

The same guard now covers attempted-legs.json, which is worse in one respect: it persists and accumulates, so one corrupt write would have broken the ledger permanently rather than for a run. On a parse failure it warns and starts fresh, so the current run's verdict is still recorded.

My own parsePayload was already guarded and has a test for it — so this was an inconsistency in my defensive posture, not an oversight about whether it mattered. Two anchor tests added: a truncated trailing record leaves the good legs intact, and an all-garbage file yields no legs and no throw.

The rest:

• finalizeLostCheckRuns now checks the PATCH response. fetch resolves on a 403, so a rejected finalize was completely silent — again leaving the check run in the state that function exists to clear. The checkpoint script's api() helper already did this; the ledger did not.
• mapJobsToLegs is exported and the anchor test now drives it instead of rebuilding the join by hand. Worth doing beyond tidiness: a broken join degrades silently to job_matched: false, so a test using a private copy could have stayed green while production quietly stopped matching anything.
• Step summary appends instead of read-modify-write; Bun.write per the repo's Bun-API rule.

Re-verified: bun run check clean, full unit suite 942 pass / 0 fail, and the ledger re-smoked end-to-end against a synthetic ci-data tree.

One note for the record — this PR is deliberately linked to Closes #77; see the mapping table in the description, including the one judgement call (§4's unimplemented outcome vocabulary) worth confirming before merge.

[mobileskyfi]

Copilot's two findings applied in d606198. Both were correct, and the first was the more interesting of the whole review round.

classifyLeg was making the exact mistake this design was built to prevent. It treated any check run left in_progress as runner-lost — including on a job whose steps all reached a terminal state. But the close PATCH is best-effort and can fail transiently, so a plain API hiccup was being reported as runner loss. That is the same class of error I used to justify adding not-started to the vocabulary (don't let a setup failure masquerade as a lost runner), and I had left the mirror-image case open.

The rule is now explicit: when the two instruments disagree, the jobs API wins. It is server-side and unconditional; the checkpoint posts best-effort and silently tolerates its own failures, so it is the weaker witness by construction. An un-closed check run means runner loss only when no job corroborates otherwise — and that case now reports attempted-incomplete with "a failed close, not a lost runner", keeping the forensics (last file, current file) intact. Anchor test added; .github/instructions/ci.instructions.md records the precedence rule.

The sidecar's cap_s: 0 on the not-run path would have rendered in the checkpoint table as a 0-second timeout — reading as a wedge that never happened rather than "the step budget was already spent". The field is now omitted when no cap was ever applied, with a test asserting the property is absent rather than zero.

Full suite: 945 pass / 0 fail, bun run check clean.

All eight review threads resolved. Ready for your call on merge — the only open question is the deliberate Closes #77 link and the §4 judgement call flagged in the description.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/ci-leg-ledger.ts:8

• The header comment documents a --artifacts flag, but build() doesn’t accept it and the workflow invokes this script without it. This can mislead maintainers when running the script locally; please align the documented invocation with the actual CLI.

 *   bun scripts/ci-leg-ledger.ts build --data <ci-data-dir> \
 *     --artifacts <dir> --matrix <plan matrix json file>
 *

scripts/ci-leg-ledger.ts:415

• finalizeLostCheckRuns() forces the check run conclusion to failure, which contradicts the checkpoint script’s stated goal of avoiding a second red check for an already-red leg (see ci-leg-checkpoint.ts close()). Consider closing these orphaned check runs as neutral and relying on the job itself + the check title/summary to carry the signal, so PR/status views don’t double-count failures.

				body: JSON.stringify({
					status: "completed",
					conclusion: "failure",
					completed_at: new Date().toISOString(),

[mobileskyfi]

Final pass before merge. Three things closed, one of them a new finding.

Copilot's two suppressed comments were both correct — eaf24d1. Suppressed comments do not appear in the thread list, so a review that reads "generated no new comments" is not the same as a clean review; worth knowing for the next PR here.

The load-bearing one: finalizeLostCheckRuns() closed orphaned check runs as failure, while close() in ci-leg-checkpoint.ts:327 uses neutral and states exactly why — "the leg's own job already carries that verdict, and a second red check on the commit would double-count one failure in every branch-protection and PR-status view." A lost runner looks like the exception where the job carries no verdict, but the evidence on ci-data says otherwise: run 30750979859's own ledger entry records "job_conclusion": "failure". GitHub does conclude the job — ~45 minutes late, but red. So failure here showed one dead leg as two reds. Now neutral, with the measurement cited in the comment. What this check run carries is its title and summary, not its color.

That is the third finding in this PR of the same shape: a rule I wrote down and applied in one place but not the neighbouring one. The other two were parsePayload guarded while completedLegs was not, and not-started existing to stop one failure masquerading as another while the failed-close case did precisely that. Three for three — worth stating plainly, because "I already decided this" is evidently not evidence that I applied it. The header also documented an --artifacts flag build() never accepted.

The ci-data README is done — 54a2192, pushed to the data branch. It was listed as a merge-time follow-up, but attempted-legs.json has existed on that branch since run 30750868899, so the README was already describing an incomplete reality. It now documents the four terminals, the artifact-outranks-the-instrument rule, the job_matched: false degradation mode, why incomplete legs deliberately stay out of tested-versions.json, and why last_checkpoint_ts is the timestamp to read. The query recipe in it was run against the real file, not composed and hoped for:

30750979859  linux-x86|stable  runner-lost  disk.test.ts  2026-08-02T13:57:25.584Z

Closes #77 stays, and the residue is filed as #125. All five done-when rows are satisfied. §4's operation-timeout / infra-* / cancelled / superseded are not implemented, are not in the done-when, and were disclaimed in code by both bites rather than silently — ci-file-watchdog.ts:113 names the reason (the watchdog leaves stdio inherited so the workflow's tee keeps producing byte-identical logs, so classification cannot be bolted on without changing what CI logs look like). #110's rule is that a PR links its sub-issue when it satisfies the whole done-when; this does. #125 carries the unbuilt half so it does not close along with #77.

Unit tests green locally (31 pass), all 8 checks green on the pushed head.