Repository navigation
fix(ci): read the test shard jobs in the Bun crash census and fix stale CI test job docs - #476
Merged
Merged
Conversation
…le CI test job docs Since #442 the `test` job is only the required fan-in of the four `test shard i/4` jobs. It runs no Bun, but `ci:bun-crash-census` still read it by default and reported zero crashes without saying so, and `workflowJobBunVersion(ci.yml, "test")` found the fan-in block, which has no setup-bun. - With no --job, the census reads every `test shard i/n` job of a run, or the `test` job of a run that has no shard job (a run from before the split, whose `test` job ran the suite). - --job accepts `/` and a `*` pattern, so the documented `--job "test shard 1/4"` and `--job "test shard *"` work; before, both were refused as an invalid argument. - The workflow lookup matches a job by its `name:` template, so `test shard 2/4` finds the `test-shard` matrix job's Bun pin. - The report prints a closed `warning:` line (also on stderr, and in --json) when no job matched or a selected job shows no Bun setup in its log or workflow block. - docs/security/env-secrets-threat-model.md and docs/reference/shared-host-lifecycle.md now say the tests run in a `test shard i/4` job and that `test` is the fan-in. Refs HACK-1424 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 24, 2026
roodboi
marked this pull request as ready for review
October 24, 2026 18:06
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
…d skip comment lines in jobs (#489) Refs HACK-1441 Follow-ups from the #476 verifier on `scripts/ci-bun-crash-census.ts`. **Stacked on #476:** #476 is still open, so this branch merges its head (`e3814e87`) on top of `next` (`1281184b`). Until #476 lands, this PR's diff also shows #476's changes. Only commit `159df1e0` belongs to this PR. ## Root cause 1. **Cache key had no test.** If you change ``const cacheKey = `${sha}\n${job}` `` to `const cacheKey = sha`, all 16 existing tests still pass. With a wildcard selector, a sha-only key gives every job at that sha the version of whichever job resolves first. For example, the `test` fan-in (no Bun) and a shard (pinned Bun) would share one result. 2. **Column-0 comments ended the job scan.** `workflowJobBunVersion` stopped at the first non-indented line after `jobs:`. A column-0 `# comment` inside `jobs:` therefore hid every later job, and the function returned null for those jobs. 3. **One fetch per job name.** The only cache was keyed by (sha, job), so the same `ci.yml` was fetched once for each distinct job name at a commit. That is up to 4 GETs per sha for the shards, plus 1 for the fan-in. ## Fix - Add a per-sha cache of the decoded workflow text. Each sha now costs one contents GET, whatever the number of jobs, runs or concurrent workers. The cache stores the promise, so concurrent runs at the same sha share the in-flight request. - Keep the per-(sha, job) version memo on top of that cache. A comment explains why the key must never be the sha alone. - `workflowJobBunVersion` now skips blank lines and lines that start with `#` in column 0. Any other column-0 line is still the next top-level key and still ends the jobs. - The header doc now says the workflow is fetched once per commit. ## Tests (`tests/ci-bun-crash-census.test.ts`, new `workflow reads (HACK-1441)` block) - **Comment and blank lines:** a workflow with a column-0 comment, blank lines, a commented-out job and a top-level `env:` key after `jobs:`. Shard, lint, fan-in, commented-out-job and post-`jobs:` lookups each return the right value. - **Per-job version under `--job "test*"`:** three runs, two of them at the same sha. In one run the fan-in is listed first; in another a shard is listed first. Every shard gets `1.4.2 workflow`, the fan-in gets `unknown`, and the expected warning is `no_bun_setup 2/9`. - **Fetch count:** the same fixture at concurrency 1 and 3 makes exactly one contents GET per sha (2 in total). ## Evidence - Focused tests: `bun test tests/ci-bun-crash-census.test.ts` gives 19 pass, 0 fail (16 existing + 3 new), on Bun 1.4.2. - Mutation results. Each mutation was applied to the fixed script, tested, and then the file was restored: | Mutation | Result | |---|---| | M1: `cacheKey = sha` | killed by the per-job version test | | M2a: column-0 comment ends the scan again | killed by the comment test and the version test | | M2b: full revert to break-only behavior | killed | | M2c: skip every column-0 line (never end `jobs:`) | killed by the comment test (`late` lookup) | | M3: no per-sha text cache (the pre-fix behavior) | killed by the fetch-count test | - Gates: root `bun run check` passes (bun-version, privacy:check, turbo check; existing warnings only). `bun run --cwd packages/cli typecheck` has 0 errors and covers `tests/`, which imports the script. Biome is clean on both changed files. gitleaks 8.30.1 (`gitleaks dir` over a `git log -p origin/next..HEAD` export) found no leaks. - No CLI surface or consumer agent guidance changed, so I did not run `docs:cli-reference` or `generate:agent-plugins`. - Full suite: hosted CI `test` shards at this PR's exact head. ## Gaps - No live `gh` census was run against GitHub. The behavior is proven with the fake `GhReader` only. - If `#476` is squash-merged before this PR, `next` has to be merged into this branch again. Expect a possible conflict at the end of the test file, because both sides add lines after the same last block. ## Round 2: merge `next` after #476 landed #476 was squash-merged into `next` as `101d2638`. This branch still carried #476 as the merge commit `0482673b`, so the PR conflicted with `next` (`d09e1b73`) in `scripts/ci-bun-crash-census.ts` and `tests/ci-bun-crash-census.test.ts`. - **Fix:** merge commit `499e8019` merges `next` (`d09e1b73`) into the branch and keeps the PR side of both census files. Before resolving, I checked that #476's five files are byte- identical between `0482673b` and `d09e1b73` (`git diff` is empty), so the PR side differs from `next` only by the HACK-1441 changes. - **Diff now:** `git diff next HEAD` is only the two HACK-1441 files (+177/-20). The "Stacked on #476" note above no longer applies, because #476's changes are already in `next`. - **Focused tests at `499e8019`:** 19 pass, 0 fail, 87 expects (Bun 1.4.2). - **Mutations re-run at `499e8019`:** each was applied, tested and then restored. M1 (`cacheKey = sha`) was killed by the per-job version test. M2 (skip block removed) was killed by the comment test and the per-job test. M3 (per-sha text cache never hits) was killed by the fetch- count test. - **Gates at `499e8019`:** root `bun run check` exits 0 (privacy-check ok). `bun run --cwd packages/cli typecheck` exits 0 with 0 errors. Biome is clean on both files. gitleaks 8.30.1 (`gitleaks dir` over a `git log -p origin/next..HEAD` export plus the net diff) found no leaks. - **Hosted CI at exactly `499e8019`:** run 40120172542 passed all 20 checks, including `test` shards 1/4 to 4/4, `docker-e2e` shards 1/4 to 4/4, secret-scan, Candidate core, Config compiler (all platforms), Runtime state models, linux-process-lifetime and runtime-images. This replaces the earlier evidence from `159df1e0`. - **Remaining:** after CI started, `next` gained one more commit, `78487f72` (#487). `git merge- tree` reports a clean merge, and that commit touches no census files. GitHub shows `BEHIND`, not a conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups to #442 (CI sharding, merged as
6e4bcf2b). Parts (1) and (2) are fixed here. Part (3), the release workflow, is a proposed diff only: it is not part of this PR. The reason is in that section below.Root cause
Since #442, the hosted suite runs in four
test shard i/4jobs. Thetestjob is now only the required fan-in. It needstest-shardand runs no Bun.testjob".bun run ci:bun-crash-censusdefaulted to--job test, so it read the fan-in. A failed fan-in log has no Bun output, so the census reported zero crashes for it and gave no warning.workflowJobBunVersion(ci.yml, "test")found the fan-in's block, which has nobun-version, and"test shard 1/4"matched no workflow key at all. The workaround in.ai/skills/hack-repo-verify/SKILL.md(--job "test shard i/4") did not work either. The--jobcheck refused/, so the census exited withInvalid argument(reproduced onorigin/next)..github/workflows/release-prepare.ymlstill runs the serialbun run turbo:test. It has notimeout-minutes, so GitHub's 360 min default applies, and Turbo buffers the output.Fix
(1) Docs.
docs/security/env-secrets-threat-model.md:99anddocs/reference/shared-host-lifecycle.md:230now say the test runs in one of the hostedtest shard i/4jobs, and that the requiredtestcheck passes only when every shard passes.(2) Crash census (
scripts/ci-bun-crash-census.ts):--job, the census reads everytest shard i/njob of a run. A run with no shard job is from before the split, so itstestjob ran the suite and is read instead. The same window can span both kinds of run.--jobvalues:--jobnow accepts/and a*pattern.--job "test shard 1/4"and--job "test shard *"both work. Other characters are still refused.workflowJobBunVersionnow matches a job by itsname:template, read only underjobs:. A${{ ... }}expression in the template matches any text. Sotest shard 2/4finds thetest-shardmatrix job's pin, andteststill returns null.warning:line when no job matched the selector, or when a selected job shows no Bun setup in its log or workflow block. The same line goes to stderr, and--jsongets awarningsarray. The warning contains no log content..ai/skills/hack-repo-verify/SKILL.mdnow describes the new default.Tests and evidence
Focused tests:
tests/ci-bun-crash-census.test.ts(16 pass). New cases:docker-e2e shardjobs are skipped, and a run from before the split is still read.--job teston a sharded run warns, and the fan-in log content is not printed.on:keys..github/workflows/ci.yml: all four shard names resolve to thepackage.jsonpackageManagerBun pin, andtestresolves to none.--jobwith/and*is accepted;;and values over 100 characters are refused.Mutation proof: each of these 8 mutations makes the suite fail:
test;no_bun_setupwarning is dropped;--jobcheck is restored;testagain;Live read-only census (GitHub API GET requests only; window 2026-10-24 05:00Z to 12:00Z, 17 CI runs):
origin/nexttest)bun=unknown; no warning.log:1,workflow:4,unknown:0); no warning.--job testwarning: 1 of 2 selected jobs show no Bun setup ...line.--job "test shard 1/4"Other checks:
bun run checkpasses, includingprivacy:check.bun run --cwd packages/cli typecheckpasses with 0 errors.bunx ultracite checkon the changed files passes.shared-host-lifecycle,aes-gcm-v1-context-binding,bun-crash-loopandagent-instruction-source, which reads the verify skill.origin/next..HEADfound no leaks.Full suite: the hosted CI
testcheck (all four shards) at this PR's head.(3) release-prepare: proposed diff, not applied
Why it is not applied. The change cannot be validated without a release dispatch or a push to
main, and both are outside this unit.release-prepare.ymlruns only onpush: mainandworkflow_dispatch. A pull request never runs it, so no hosted check in this PR would exercise the change. Two findings make a run-free change unsafe to claim:next's release-prepare cannot pass as written. Onnext,turbo:testisbun run test:preflight && turbo run test. The preflight needs the matching native config compiler (dist/hack-config-compiler).release-prepare.ymlinstalls no Rust toolchain and never runsbun run build:config-compiler, so the preflight fails before any test starts. Onmain,turbo:testhas no preflight. The lastmainrelease-prepare (40050242500) ran 165 files in 30 s.nexthas 579 files, planned at about 42 min on the macOS shard runner.next's full suite on Linux. Thetest shardjobs run onblacksmith-6vcpu-macos-15, andlinux-process-lifetimeruns only 6 files. release-prepare runs onblacksmith-4vcpu-ubuntu-2404. So the Linux duration and the Linux pass/fail result of the whole suite are both unmeasured.The change only matters when
nextis merged intomain. The workflow that runs is the copy onmain, andscripts/test-shard.tsexists only onnext, so this diff must not be applied tomainby itself. It applies cleanly tonextat6e4bcf2b:What stays the same:
semantic-releaseruns only if every earlier step passes.concurrencygroup,cancel-in-progress: false.semantic-releasestep with the same token. No new outputs, artifacts or permissions.What is new:
check:bun-versionnow runs before the tests. It already runs before everybun run testand every CI shard.git ls-filescoverage check:verify --count=1on this head reports "579 files in 1 shards cover the full test set exactly once", planned at 42m22s.Validated statically:
bun scripts/test-shard.ts verify --count=1andplan --shard=1/1succeed on this head.Not validated: the run itself on the Linux release runner (compiler build, Linux suite result and duration). That needs one
workflow_dispatchofrelease-prepare.ymlagainstnext, or the next-to-main merge. Onnext, a dispatched run would also reachbunx semantic-releaseif the tests pass, so a dry run would need--dry-runor a temporary branch filter. I did not dispatch it.Gaps
Refs HACK-1424
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.