[miguel-heygen]

What

The cli test job runs as two vitest shards (--shard=1/2, --shard=2/2). The Test aggregator still depends on test-cli, which is green only when both shards are, so its name and meaning do not change.

Why

Test (cli) has timeout-minutes: 10 and a full run takes about 9 minutes (484 files, 7168 tests passed, then the job was cancelled four seconds after the last test finished). A green run is cancelled when it lands a few seconds late, so the required Test check goes red with no failing test.

How

strategy.matrix.shard: [1, 2] with fail-fast: false on test-cli; the step passes --shard=${{ matrix.shard }}/2 to the existing test script. The timeout stays 10 minutes.

Test plan

fails without the fix: not a unit test; the evidence is this PR's own CI run, where both shard durations are listed below.

• Test (cli 1/2) took 5m23s and Test (cli 2/2) took 5m32s (run on eea09260, same split since), each well under the 10-minute limit; Test green.
• scripts/test-reachability.json maps the sharded runner (vitest run --shard=${{ matrix.shard }}/2, cwd packages/cli) to packages/cli/src/**/*.test.ts and pins the vitest config, the test-cli job text and the cli test script by hash, so changing the shard matrix fails the audit until the mapping is reviewed. node scripts/check-test-reachability.mjs reports zero orphan tests; its own test file passes 30/30. The parser is unchanged, so a lone --shard=1/2 still reads as an unsupported option and fails closed.

Size

About 20 changed lines: the job definition and the audit's mapping of it; one change, it stands alone.

[github-actions]

Edit accuracy: accurate 271 (base branch 271), smooth 105 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

• seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

[heygengenesis]

Requesting changes at eea09260. The split itself works, but it breaks the Test reachability check.

Blocker: Test reachability fails on this head (CI run 41244314998, job "Test reachability"). The check reports no CI runner selects this test for all of the CLI's tests (984 lines). scripts/check-test-reachability.mjs reads the runner command bun run --filter '@hyperframes/cli' test --shard=${{ matrix.shard }}/2. It takes --shard=… as a file-path filter (selectedByRunner: args.some((arg) => relative.includes(arg))), so no CLI test file matches. Reproduced locally: node scripts/check-test-reachability.mjs fails at this head and prints "zero orphan tests" on main.

Fix: have the reachability parser accept --shard=N/M as a selection-neutral option. All the shards of a matrix together run every file, so it should not filter by path. Then add a case to scripts/check-test-reachability.test.mjs showing that a sharded runner still reaches its files, and that an unknown option still fails closed.

The rest checks out:

• Both shards are green: Test (cli 1/2) took 5m23s and Test (cli 2/2) took 5m33s, each 242 test files passed and 5 skipped. Together that covers all 494 files, well under the 10-minute limit.
• --shard reaches vitest through bun run --filter … test.
• fail-fast: false is right here, and the Test aggregator's needs.test-cli.result covers both matrix legs.
• The fails without the fix: line is fine for a CI-only change whose evidence is its own run. Fill in the shard times in the test plan once the run is final.

[heygengenesis]

Approving 575d18e4. The blocker from my last review is fixed.

• The new entry in scripts/test-reachability.json covers the sharded runner the same way the existing producer matrix entry does. It pins vitest.config.ts, the test-cli job and the cli test script, so if any of them changes, the check fails until the entry is re-pinned.
• Locally, node scripts/check-test-reachability.mjs reports zero orphan tests, and node --test scripts/check-test-reachability.test.mjs passes 30/30.
• CI run 41245548024 is green: Test reachability passed, Test (cli 1/2) took 5m37s, Test (cli 2/2) took 5m28s, and Test passed.

Nit, not blocking: the entry's reason says "both run in one job". They are two matrix jobs of test-cli, which together run every selected file.

[heygengenesis]

Requesting changes at 4c291060. This replaces my approval: the head moved to this commit before my review posted, so that approval was attached to code I had not read.

The idea is right: a shard matrix as a whole runs every file. But the way runnerCommand handles it now lets the check pass when it should fail.

Blocker: any --shard=N/M is treated as full coverage, even when nothing runs the other shards. SHARD rewrites both forms below to --shard=1/1:

• the literal --shard=1/2
• --shard=${{ matrix.<any> }}/M, whatever the matrix holds

So a job that runs only vitest run --shard=1/2 reports "zero orphan tests" while half the files never run. The same goes for --shard=${{ matrix.shard }}/3 with shard: [1, 2]. The new test asserts the literal --shard=1/2 case passes, so it locks this in. The audit's job is to fail closed.

Suggested fix:

• Treat a shard as neutral only in the ${{ matrix.X }}/M form, and only when that job's strategy.matrix.X is exactly 1..M.
• Reject a literal --shard=N/M with M > 1 as Unsupported test option.
• In the test, have a lone literal --shard=1/2 and a too-small matrix both throw.

If parsing the matrix is more than you want here, the pinned test-reachability.json mapping from 575d18e4 was a correct fix. I was about to approve it.

Verified at this head: node scripts/check-test-reachability.mjs reports zero orphan tests, and node --test scripts/check-test-reachability.test.mjs passes 31/31.

[miguel-heygen]

Fixed at 4c29106 as asked: the reachability parser now treats --shard=N/M (literal or the matrix template) as selection-neutral, with a test that a sharded runner still reaches its files and that --retry=2 still fails closed (red on the old parser). The runner-mapping route I tried at 575d18e is removed. Shard times are in the body: 5m23s and 5m32s. Waiting on CI at this head.

[miguel-heygen]

Agreed, a blanket --shard rule lets a job that runs one shard pass the audit. Reverted the parser change and went back to the pinned mapping from 575d18e, rebased on current main: the audit now fails if the matrix, the job text, the vitest config or the cli test script change. Parser untouched, so a lone literal --shard=1/2 still fails closed. Audit reports zero orphan tests; its tests pass 30/30.

[heygengenesis]

Approving 16d51576. This head drops the blanket --shard parser change and goes back to the pinned scripts/test-reachability.json mapping, as I asked. The diff from main is the same as 575d18e4, which I had already checked.

• The mapping pins vitest.config.ts, the test-cli job and the cli test script, so any change to them makes the check fail until it is re-pinned. check-test-reachability.mjs is unchanged from main, so no option is accepted without a check.
• Locally, node scripts/check-test-reachability.mjs reports zero orphan tests, and node --test scripts/check-test-reachability.test.mjs passes 30/30.
• CI run 41255666142 is green: Test reachability passed, Test (cli 1/2) took 4m40s, Test (cli 2/2) took 5m11s, and Test passed.

Nit, not blocking: the mapping's reason still says "both run in one job". They are two matrix jobs of test-cli.