[jayden1711]

What

The test-run helper's GPU line (worklane-testrun: gpu <N> (devices …; max <s> s; waited <s> s)) now reports as waited only the seconds spent in flock, summed across the run's GPUs. Before, it was the span from before gpu_lock, so it also counted each lock file's setup (about 14 forks: id, stat, mkdir, chgrp, chmod…).

Closes #968.

Why

CI flake on PR #920 (run 39555627878): "GPU on for the instance" saw waited 3 s on an uncontended per-test lock because the runner stalled during lock-file setup. The docs and the test describe waited as the wait for the lock alone. No check is weakened: the [01] assertion stays as it is, and the wait still counts against the run's timeout (runtime is still computed from started).

How verified

• Measured, fail before: I added the test with the script unchanged. gpuHelper()'s stat stub sleeps 0.6 s for each stat of a lock path under the test's temp run/ (about 3 s in total). node --import ./dist/test/isolate.js --test dist/test/testrun-gpu-helper.test.js gave 7 pass, 1 fail (… max 57 s; waited 3 s)).
• Measured, pass after: with the fix, testrun-gpu-helper, testrun-gpu, helper-protocol and testrun-sandbox gave 59/59 pass.
• Measured: bash -n on the script passed; npm run typecheck was clean; npm run check:denylist was clean (1628 files). shellcheck isn't installed on this machine, so it was not run.
• Inferred: the lock order, the deadline and the timeout fail path are unchanged; only the reported number moves.

What remains

• Inferred: with N GPUs, the sum of per-GPU whole-second deltas can be off by up to about N seconds. CI covers only one GPU.
• Inferred: the "a second run waits for it" runtime check (left <= 40 - waited) is looser, because waited no longer includes setup time. The direction is safe.

Critical set

In: scripts/machine/ root helper (setup and machine scripts). Needs an independent final check and an adversarial second review, both naming this head.

Built in a Cursor-coordinated session (normal development, not the installed coordinator).

Round 2 (head 89964cd)

• Measured, margin as on main: "GPU on for the instance" is unchanged from main (no slow stat). Its line showed max 60 s, 60 s and 59 s in three runs, against the >= 55 check.
• Measured, the slow-setup case on its own: a separate test makes the lock's five stats take about 3 s on a free lock and asserts only waited [01] s. With main's script: 9 pass, 1 fail (max 57 s; waited 3 s). At head: 10/10.
• Measured, the sum across GPUs: a two-GPU test holds GPU 0 for 3 s and asserts waited >= 2. With GPU_WAITED= instead of +=: 9 pass, 1 fail (waited 0 s). At head: 10/10.
• Measured, CI parity: merge-policy 56/56, change-files-renames 63/63, changelog 27/27, owner-terminal-removed 8/8. The push gate is clean.
• Inferred: the two-GPU test needs the helper to reach GPU 0's lock well within the 3 s hold; that took about 0.5 s here.

Round 3 (f3e3c26)

Answers R2c and R2d (both FAIL at 89964cd).

• Experiments helper, the same bug: worklane-exp.sh's gpu_lock is byte-identical to the test-run helper's again, which fixes CI shard 1/4's "one GPU lock…" test. The job record's waited_s is now GPU_WAITED. Measured: with the helper reverted, 10 pass and 2 fail (waited_s 3, plus the byte-identity test); at head, 12/12.
• Two-GPU test, robust to a slow setup: both GPUs are held. Each is released 3 s after the helper's last stat of its lock file (the stat stub logs each one), and the test asserts waited >= 5. Measured: the = and keep-the-longest mutants both fail (waited 3 s); with a 3 s setup stall per lock file, 3 of 3 runs pass (waited 6 s, from an uncommitted probe copy of the compiled test).
• The coordinator names the cap correctly: src/testrun/run.ts names the GPU's max_minutes as a timeout's cause only when the opt-in is on, the unit's max equals max_minutes × 60, and it is under timeout_s. Measured: a new uncapped-timeout test fails with run.ts reverted (23 pass, 1 fail) and passes at head (24/24).
• Remaining (inferred): when root's file can't be read, the old comparison can still misname a cap after a slow setup. Fixing that exactly needs the helper to report when it capped.
• Measured: the push gate is clean; the source-text lints, the experiments and testrun tests, coordinator (142 pass, 0 fail, 4 skip, run with --test-timeout=400000 and the sandbox cache variables unset), typecheck and denylist all pass.

Round 3 (R2f)

• root lowers max_minutes mid-run: the helper reads root's file for each GPU command, but the coordinator only read it once, at the run's start. When a granted GPU tier times out, src/testrun/run.ts now reads root's file again. The GPU's max_minutes is named as the cap when the unit's max equals either reading × 60 and is under timeout_s. With neither reading on, it uses the old comparison.
• Why not have the helper report its cap (inferred): the GPU line's optional ; <note> field would swallow a new marker. The helper is versioned (PROTOCOL=11), so a format change would need a bump and a root reinstall on every machine. Already-installed helpers would never send it, so the fallback is needed regardless.
• Measured: a new testrun-runs test, with root going from 60 to 10 minutes and the unit capped at 600 s, fails on f3e3c26c's run.ts (25 pass, 1 fail: "after 30 min (its timeout_s …, 1800)") and passes at head (26/26).
  • New cases also cover an uncapped timeout with max_minutes below timeout_s, and the unknown-file fallback. On main's run.ts, 2 tests fail ("after 23.3 min (the GPU runs' max_minutes …, 1400)").
• Measured: the push gate over origin/main..HEAD is clean. The CI-parity lints, the testrun and experiments tests, coordinator (146 tests, 142 pass, 4 skipped, with the cache variables unset), typecheck and denylist all pass.

Round 4 (R2h)

The cap the helper read. A new testrun-runs test covers root changing max_minutes after the helper read it, with timeout_s 1800:

• Lowered from 25 to 10, with the unit capped at 1500 s: it expects "after 25 min (… 1500)".
• Raised from 10 to 60, with the unit capped at 600 s: it expects "after 10 min".
• Measured: the end-of-tier-reading-only mutant ([d.gpuOptIn?.()]) and the minimum mutant (g.maxS === Math.min(...rootMaxS)) each fail it, with 26 pass, 1 fail ("after 30 min (its timeout_s …, 1800)").

A held experiment lock. The experiments-helper flock stub now sleeps 3 s on -w, and a new test asserts waited_s >= 2.

• Measured: waited=0 in worklane-exp.sh fails it, with 12 pass, 1 fail (waited_s 0).

The unit's max after a slow setup. The slow-setup test also asserts max <= 57.

• Measured: runtime=$((timeout - GPU_WAITED)) fails it (max 60 s).

Other checks (measured). Head passes all of these. The push gate over origin/main..HEAD is clean. The CI-parity lints, the testrun, experiments and helper tests, typecheck and denylist all pass.

[jayden1711]

Final check (independent, first review): PASS ae46476

Reviewer: R2a, a Cursor-session review agent. It is neither the builder nor the orchestrator.

• Fail-before (measured): the base script with the head's test gives 7 pass, 1 fail ("GPU on for the instance", waited 3 s). At this head, testrun-gpu-helper and testrun-gpu give 22/22. bash -n passes and the denylist is clean.
• Unchanged logic (inferred, from reading gpu_lock and exec_run at base and head): the lock order, the fd numbering, the left deadline, the -n fallback, the timeout exit and runtime from started are unchanged.
• GPU_WAITED is initialised once and has one call site, not inside $(...) (measured, rg). A direct two-GPU harness, holding the locks for 2 s and 4 s, reported 4 (measured).
• Mutants (measured): timestamping before setup is caught, and deleting the sum is caught. Overwriting instead of adding survives, since no test sums several GPUs (optional follow-up).
• Required changes: none.

[jayden1711]

Second review (adversarial): FAIL ae46476

Adversarial second review (R2b; did not build this change). The script fix is sound. The test change re-creates the CI flake in a different assertion.

Required change

• What breaks (measured): the new 3 s slow-stat stub goes into the same test that asserts max >= 55 (test/testrun-gpu-helper.test.ts:200). Without the stub that test reports max 60 s and max 59 s; with it, max 57 s. That leaves 2 s of slack where there were 4–5 s.
• Why it matters (inferred): a stall like testrun GPU helper: 'waited' counts lock-file setup, not only the lock wait (CI flake) #968's (~2.8 s) anywhere before the GPU line would fail it.
• Fix: keep the existing test without the slow stat, and put the slow-stat case in its own test that asserts only waited [01]. Don't widen >= 55; that loosens a check and needs the owner's OK.

Checked (measured)

• Build and tests: tsc is clean. testrun-gpu-helper 8/8, testrun-gpu 14/14, helper-protocol 4/4, testrun-sandbox 33/33, and bash -n passes.
• Fail-before: with the script from HEAD~1, the new test fails (waited 3 s).
• Mutants:
  • timestamp taken before gpu_lock_file: killed;
  • GPU_WAITED=0: killed;
  • = instead of +=: survived, because no test uses more than one GPU (optional: add a two-GPU case).
• Locking, deadline and timeout paths (inferred, from reading): unchanged. The coordinator's parse format (src/testrun/gpu.ts:129) is unchanged, and the real-systemd script's waited >= 3 check is still met.
• Optional: scripts/machine/worklane-exp.sh:559-562 has the same "waited includes setup" pattern. That needs its own issue.

[jayden1711]

FAIL 89964cd

Independent final check (R2c; did not build this change). The fix to worklane-testrun.sh is correct, and R2b's item is resolved. But CI is red because of this PR.

Required changes

1. CI shard 1/4 (Node 22) fails (measured, run 40062178383): experiments-helper.test.js test 451 requires gpu_lock in scripts/machine/worklane-exp.sh to be byte-identical to the test-run helper's copy. Locally it gives 10 pass, 1 fail.
   • Fix: make the same change in worklane-exp.sh, including GPU_WAITED=0. Have its caller (about line 559) use waited=$GPU_WAITED.
   • Add a changelog line for the experiments record's waited_s.
2. The two-GPU test (3 s hold, waited >= 2) fails with a 1.5 s setup stall (measured): about 1 s gives waited 2 and passes; about 1.5 s or 2 s gives waited 1 and fails. That is less than testrun GPU helper: 'waited' counts lock-file setup, not only the lock wait (CI flake) #968's 2.8 s.
   • Fix: hold longer (for example 6 s, keeping >= 2), or gate the hold on the helper actually waiting.

Checked (measured)

• Tests: testrun-gpu-helper 10/10, testrun-gpu 14/14, helper-protocol 4/4, testrun-sandbox 33/33. The CI-parity set passes.
• Fail-before: 9 pass, 1 fail.
• GPU-on test: byte-identical to main's.
• Mutants: =, t0 before setup, and t0 after flock: all killed.

[jayden1711]

Second review (adversarial): FAIL 89964cd

Adversarial second review (R2d; did not build this change). R2b's item is resolved (measured). Three changes are required.

1. CI is red (measured): experiments-helper test 451 needs gpu_lock in scripts/machine/worklane-exp.sh to match the test-run helper's. Make the same change there. Also report waited_s from GPU_WAITED (lines 559–562) instead of the span from locked.
2. The two-GPU test has testrun GPU helper: 'waited' counts lock-file setup, not only the lock wait (CI flake) #968's weakness (measured):
   • The helper normally reaches GPU 0's lock in 48–152 ms (12 runs at load about 18), giving waited=3.
   • With a slowed setup: 1.1 s and 1.6 s give waited=2 (pass), 2.1 s gives 1 (fail), 2.9 s gives 0 (fail).
   • Fix: gate the hold on the helper actually waiting (for example, the stat stub touches a marker on 0.lock, and the hold runs 3 s from then).
   • A "max instead of sum" mutant survives 10/10. Contend both GPUs one after the other to kill it.
3. The coordinator's timeout cause is now misreported more often (measured):
   • src/testrun/run.ts:350 blames root's max_minutes whenever maxS + waitedS < timeout_s, which assumes waited covers setup.
   • In 25 uncapped two-GPU runs, max + waited < 60 happened 0/25 times on main and 3/25 on head with normal setup; 1/25 on main and 25/25 on head with setup slowed about 1.5 s.
   • Fix: compare maxS with the GPU's max_minutes (gpuOptIn knows it), or have the helper say when it capped. Add a test for an uncapped timeout on a GPU tier.

Checked (measured)

• Tests: tsc clean; testrun-gpu-helper 10/10 (3 times), testrun-gpu 14/14, testrun-runs 23/23. The CI-parity set passes, bash -n passes and the denylist is clean.
• Fail-before: 9/10.
• Mutants: counting from started, and a local GPU_WAITED: killed.

[jayden1711]

PASS f3e3c26

Independent final check (R2e; I did not build this change).

• CI (measured): complete, 25 success and 2 skipped (bootstrap, which isn't required). Every required check passes, including shard 1/4, which was red at 89964cd.
• Push gate (measured): "5 commit(s) and 1 tip(s) clean".
• CI-parity tests (measured): change-files-renames 63/63, merge-policy 56/56, changelog 27/27, owner-terminal-removed 8/8.
• Tests at head (measured):
  • tsc is clean, and bash -n passes on all the scripts.
  • experiments-helper 12/12, testrun-runs 24/24, testrun-gpu 14/14, testrun-gpu-helper 10/10, helper-protocol 4/4, testrun-sandbox 33/33.
  • coordinator: 142 pass, 0 fail, 4 skip.
  • gpu_lock is identical in both helpers once the helper name is normalised.
• Fail-before (measured):
  • Both helpers taken from base: testrun-gpu-helper 9/10 (waited 3 s) and experiments-helper 11/12 (waited_s 3).
  • run.ts taken from base: testrun-runs 23/24, failing the new uncapped-timeout test.
• Mutants (measured):
  • Killed: overwrite instead of sum; keep only the longest wait; t0 before setup; the experiments caller back to now - locked; no GPU_WAITED=0 in exp.sh; no maxS === maxMinutes*60; never naming max_minutes when the opt-in is on.
  • Survived: dropping && g.maxS < t.timeout_s (equivalent, inferred: both caps are the same value then), and the unreadable-root-file branch set to false (main's old comparison, untested, already listed as remaining).

Required: none.

Optional: a test for the branch where root's file is unreadable.

[jayden1711]

Second review (adversarial): FAIL f3e3c26

Adversarial second review (R2f; I did not build this change). Every item R2c and R2d required is resolved and CI is green, but the new timeout-cause check in src/testrun/run.ts regresses one case main gets right.

Required

• A mid-run change to max_minutes (measured, with probe tests in a scratch copy of testrun-runs):
  • The coordinator reads gpuOpt once, when the run starts. The helper reads root's file again for each GPU tier.
  • Case: timeout_s is 1800 and max_minutes is 60 at the start. Root lowers it to 10, so the helper caps the unit at 600 s.
  • This head reports "timed out after 30 min (its timeout_s in tests.yaml test_run, 1800)". That's wrong.
  • Main reports "timed out after 10 min (the GPU runs' max_minutes …, 600)". That's right.
• Fix: read root's file again when the tier ends and count either reading × 60 as the cap, or have the helper's line say when it capped. Add a test for the lowered-mid-run case.

Prior items (measured)

• gpu_lock is identical in both scripts, and experiments-helper passes 12/12.
• Two-GPU test with setup stalls of 1, 2, 3 and 4 s, 5 runs each: 20/20 pass. With 20 s stalls: 3/3 pass.
• The uncapped-timeout case is fixed.

Checked (measured)

• tsc is clean and bash -n passes.
• Every related testrun and experiments suite passes. coordinator: 142 pass, 4 skipped.
• The push gate is clean and the CI-parity tests pass.
• Fail-before: each of the 3 new tests fails on main's code.

Mutants (measured)

• Killed: max instead of sum, = instead of the sum, t0 before setup, locked back as the wait's start, and naming the GPU cap whenever maxS < timeout_s.
• Survived: the unknown/off fallback never naming the cap (untested), and dropping the maxS < timeout_s guard (harmless).

Optional

• Test the unknown/off fallback.
• Add an uncapped case where max_minutes is below timeout_s.

[jayden1711]

PASS 31f1e4a

Independent final check (R2g; I did not build this change). R2f's required item is fixed. When root lowers max_minutes mid-run, the timeout names the GPU's max_minutes again. The new test fails without the fix, and CI is fully green at this head.

R2f's required item: root lowers max_minutes mid-run

• The fix (inferred, from reading the diff f3e3c26c..31f1e4a6):
  • src/testrun/run.ts calls d.gpuOptIn() again, only for a granted GPU tier that timed out.
  • It names the GPU's cap when g.maxS equals either reading × 60 and is under timeout_s. When neither reading is on, it uses the old maxS + waitedS < timeout_s comparison.
  • The extra read can't throw: gpuOptIn returns off or unknown on any error (read in src/testrun/gpu.ts).
  • Its one production caller, src/testrun/setup.ts, passes a fresh gpuOptIn(s.instance) each call.
  • Adding r.timedOut && to g changes nothing, because cap is only used when r.timedOut.
• Fail-before (measured): in my scratch worktree I ran git checkout f3e3c26c -- src/testrun/run.ts, rebuilt with heavy.sh npx tsc, then ran dist/test/testrun-runs.test.js.
  • Result: 25 pass, 1 fail. The failing test is "testrun GPU helper: 'waited' counts lock-file setup, not only the lock wait (CI flake) #968: root lowers max_minutes while the run goes on…", which got "timed out after 30 min (its timeout_s…, 1800)".
  • With origin/main's run.ts: 24 pass, 2 fail (the uncapped test and the cap-cases test).
  • With head's run.ts restored: 26/26.

Mutants against run.ts (measured; each one rebuilt, then testrun-runs run)

• Killed:
  • g.maxS <= Math.max(...rootMaxS) instead of includes: 2 fail.
  • The old fallback ORed in with includes: 2 fail.
  • The unknown-file fallback hard-coded to false: 1 fail. This closes the gap R2e and R2f listed as optional.
  • Dropping the second read (the f3e3c26 code): 1 fail.
• Survived (optional finding):
  • Using only the end-of-tier reading ([d.gpuOptIn?.()]): 26/26.
  • Using only the last on reading (.slice(-1)): 26/26.
  • What's missing (inferred): no test has root raising max_minutes after the helper read it. Example: 10 minutes at the start and for the helper, 60 by the end of the tier. With either mutant that run would be misnamed as timeout_s; head names it correctly.
  • Optional: a test for that case would cover the start-of-run reading.

Whole-PR checks (measured)

• Build: heavy.sh npx tsc -p tsconfig.json is clean, and bash -n passes on both worklane-testrun.sh and worklane-exp.sh.
• Suites at head (each run as TMPDIR=… heavy.sh node --import ./dist/test/isolate.js --test …):

Suite	Result
testrun-runs	26/26
testrun-gpu	14/14
testrun-gpu-helper	10/10
experiments-helper	12/12
helper-protocol	4/4

• CI parity:
  • node scripts/push-gate.mjs --range origin/main..31f1e4a6: "6 commit(s) and 1 tip(s) clean".
  • change-files-renames 63/63, merge-policy 56/56, changelog 27/27, owner-terminal-removed 8/8.
  • npm run check:denylist: 1629 files clean.
• PR CI at 31f1e4a: I waited with gh pr checks 1037 --watch and then read gh pr view 1037 --json statusCheckRollup. Everything completed with success, including all 8 shards, test (22/24) and every real-systemd job. Only the two bootstrap jobs are skipped, and those aren't required.

Diff hygiene (inferred, from reading git diff origin/main...HEAD)

• No assertion is weakened and no test is skipped: the [01] and >= 55 checks are unchanged.
• The changelog fragment changelog.d/968-gpu-waited.md covers all three behaviour changes.
• No brand name is hardcoded in src/ and there is no scope creep.
• The new run.ts comment says why the file is read again, not what the code does.

Required changes: none.

Optional: add a test where root raises max_minutes after the helper's read, which would kill the two surviving mutants.

[jayden1711]

Second review (adversarial): FAIL 31f1e4a

Adversarial second review by R2h. I did not build this change. The fixes in both helpers and in run.ts are correct (measured: tests, fail-before and mutants below). It fails because two plausible wrong implementations of this PR's own changes still pass every test. Each gap is closed by one small test, and I measured that both work.

Required

1. The start-of-run reading in the cap naming is untested (measured).
   • The mutant: change [gpuOpt, d.gpuOptIn?.()] to [d.gpuOptIn?.()], so only the end-of-tier reading is used. testrun-runs still gives 26 pass, 0 fail.
   • The existing lowered-mid-run test reads 60 at the start and 10 at the end, so the end reading alone already passes it.
   • The case only the start reading covers: root lowers max_minutes after the helper read it, while the command runs. The unit was capped at the old value, but this mutant names timeout_s. That is the mirror of the case R2f made required. The changelog promises "as root's file said when the run started or when the tier ended".
   • Fix: add a test for it. My uncommitted probe (25 then 10 minutes, unit capped at 1500 s, tier timeout_s 1800, expecting "after 25 min (the GPU runs' max_minutes …, 1500)") passed at head, 27/27. It killed this mutant (26 pass, 1 fail) and a keep-the-minimum mutant (g.maxS === Math.min(...rootMaxS), also 26 pass, 1 fail).
2. The experiments record's waited_s has only a one-sided test (measured).
   • The mutant: waited=0 instead of waited=$GPU_WAITED in worklane-exp.sh. experiments-helper still gives 12 pass, 0 fail.
   • Why: the stub flock exits 0 at once, so no experiments test ever waits. The new test only asserts waited_s <= 1, which 0 passes. So does reading GPU_WAITED before gpu_lock runs.
   • Fix: add a test where the lock is really held. My probe made the flock stub sleep 3 s on -w and asserted waited_s >= 2. It passed at head (13/13) and killed the mutant (12 pass, 1 fail).

Optional

• The unit's max still counts setup time, but no test pins it (measured, a pre-existing gap). The mutant runtime=$((timeout - GPU_WAITED)) would stop counting the helper's own setup against the run's timeout. It survives testrun-gpu-helper (10/10) and testrun-gpu (14/14). The slow-setup test could also assert max <= 57 to catch it. That tightens a check rather than loosening one.
• Two changes of root's file can still misname the cap (inferred). If root changes max_minutes both before the helper reads it and again before the tier ends (for example 60, then 10, then 30), neither of the coordinator's readings is the helper's value, and the cap is named as timeout_s. Only the helper reporting that it capped would fix this exactly, and the builder has explained why that needs a protocol bump. This is acceptable as listed remaining work.
• Equivalent mutants (inferred): dropping && g.maxS < t.timeout_s and dropping r.timedOut && both survived (26/26). The first is harmless because both caps are the same value then. The second only adds a file read, because the cap is used only on a timeout. Moving t0 before the fd open also survived, and it doesn't matter because opening the fd costs nothing.

What I checked

• Build and tests (measured): tsc is clean, and bash -n passes on scripts/machine/*.sh. At head:
  • testrun-runs 26/26
  • testrun-gpu-helper 10/10
  • testrun-gpu 14/14
  • experiments-helper 12/12
  • helper-protocol 4/4
• CI parity (measured):
  • change-files-renames 63/63
  • merge-policy 56/56
  • changelog 27/27
  • owner-terminal-removed 8/8
  • node scripts/push-gate.mjs --range origin/main..HEAD: "6 commit(s) and 1 tip(s) clean".
• Fail-before (measured): with run.ts and both helpers taken from origin/main:
  • testrun-runs 24 pass, 2 fail
  • testrun-gpu-helper 9 pass, 1 fail
  • experiments-helper 11 pass, 1 fail
• Mutants killed (measured):
  • only the start reading (25 pass, 1 fail)
  • counting an unknown file as on (25 pass, 1 fail)
  • maxS <= cap instead of equal (24 pass, 2 fail)
  • the fallback set to false (25 pass, 1 fail)
  • the fallback ignoring the wait (25 pass, 1 fail)
  • only the largest reading (25 pass, 1 fail)
• Mutants that survived: keeping only the end reading and waited=0 in the experiments helper, which are the two required items, plus the setup-time mutant and the equivalent ones above.
• Protocol drift (inferred, from reading): the helper and parseGpuFile both accept max_minutes from 1 to 120, and both scale it by 60. The GPU line's format is unchanged, so PROTOCOL=11 needs no bump. Helpers already installed with the old wait count fall back to main's comparison. The re-read in production goes through gpuOptIn(s.instance) in src/testrun/setup.ts, which isn't memoised, so it really reads root's file again. readGpuFile catches its own read errors, so a re-read at the tier's end can't throw.
• CI at head (measured, gh pr view 1037 --json statusCheckRollup): every check had finished green except "test-run sandbox (real systemd)", which was still running. The two bootstrap checks were skipped. I didn't wait for it, because the verdict is FAIL either way.
• Diff hygiene (measured, by reading the diff): there is a changelog fragment. No assertion is weakened or skipped, there's no brand string in src/, and nothing is out of scope.

[jayden1711]

PASS 3a807e7

Final check (independent): R2i. I did not build this change. I reviewed the whole diff against origin/main (merge base b1c6fe22) in a scratch detached worktree. The new commit 3a807e79 changes only the three test files. The worktree has been removed.

Both of R2h's required items are fixed (measured). Each mutant below was applied in my worktree, rebuilt with heavy.sh npx tsc -p tsconfig.json, then the suite was run.

• Start-of-run reading, [gpuOpt, d.gpuOptIn?.()] changed to [d.gpuOptIn?.()]: testrun-runs gave 26 pass, 1 fail. The failing test was the new "root changes max_minutes after the helper read it", which got "timed out after 30 min (its timeout_s …, 1800)".
• rootMaxS.includes(g.maxS) changed to g.maxS === Math.min(...rootMaxS): 26 pass, 1 fail, the same test.
• waited=0 in worklane-exp.sh: experiments-helper gave 12 pass, 1 fail. The new held-lock test failed with waited_s 0, so waited_s >= 2 is enforced.
• runtime=$((timeout - GPU_WAITED)) in worklane-testrun.sh: testrun-gpu-helper gave 9 pass, 1 fail (the slow-setup test reported max 60 s). testrun-gpu stayed at 14/14, so only the new max <= 57 check catches this mutant.

At this head (measured). Every suite was run as TMPDIR=… heavy.sh node --import ./dist/test/isolate.js --test ….

• Build: tsc is clean, and bash -n passes on both helpers.
• Suites:
  • testrun-runs 27/27
  • testrun-gpu-helper 10/10
  • experiments-helper 13/13
  • testrun-gpu 14/14
  • helper-protocol 4/4
• CI parity:
  • change-files-renames 63/63
  • merge-policy 56/56
  • changelog 27/27
  • owner-terminal-removed 8/8
  • node scripts/push-gate.mjs --range origin/main..3a807e79: "7 commit(s) and 1 tip(s) clean"
  • check:denylist: 1629 files clean
• Sign-off: all 7 commits are signed off.

The new tests don't look timing-flaky.

• Measured: I ran testrun-runs, testrun-gpu-helper and experiments-helper three times, with the three suites running in parallel each round. Load average was 30 to 55 throughout. All passed every time (27/27, 10/10, 13/13).
• Inferred, from reading the code:
  • The testrun-runs case is a pure unit test with no timing in it.
  • The held-lock test's flock stub sleeps 3 s between the two date +%s calls, so the whole-second difference is always at least 3. It has no upper bound, so load can't break it.
  • In the slow-setup test, all five slowed stats (0.6 s each) run in gpu_lock_file, after started is taken. So at least 3 s always counts against the 60 s, and max <= 57 only gets easier to meet under load. The pre-existing waited [01] check is unchanged.

CI at this head (measured, gh pr checks 1037 after waiting with --watch).

• 24 checks succeeded, including all shards and the real-systemd jobs.
• The two bootstrap jobs were skipped. They are not required.
• The rollup shows one new-deps run (40136466387) as cancelled, after 1 s. The workflow sets cancel-in-progress: true, and a second new-deps run (40136468023) at the same head completed with success. Inferred: the PR body edit triggered the second run, which replaced the first. This is unrelated to the change.

Diff hygiene (inferred, from reading the diff).

• No assertion is loosened. The new max <= 57 makes a check stricter, and the [01] check is unchanged.
• No test is skipped beyond the existing platform guards.
• The changelog fragment is present.
• No brand name is hardcoded in src/, and there is no scope creep.
• The new flock stub only sleeps on -w when flockwait is set, so the other experiments tests behave as before.

Required changes: none.

Optional (inferred, carried from R2h): if root changes max_minutes twice within one tier, the cap can still be misnamed. Fixing that needs the helper to report its cap, which needs a protocol bump. It is acceptable as listed remaining work.

[jayden1711]

Second review (adversarial): PASS 3a807e7

Adversarial second review by R2j. I did not build this change. I reviewed head 3a807e79 in a fresh detached worktree, which I have since removed.

Required changes: none.

The changes to both helpers and to run.ts are correct. Every mutant I wrote that is a plausible wrong version of this PR's change was caught by a test. The one that survived is equivalent except in tie cases (listed under Optional). R2h's two required items are fixed: the start-of-run reading is now tested (my mutant RT3 is caught), and so is the experiments record's held lock (mutant E1 is caught).

Mutants (measured). For each one I applied the change in my scratch worktree, rebuilt with heavy.sh npx tsc if it was TypeScript, ran the suite, then restored the file from the head commit.

• Test-run helper (worklane-testrun.sh), run against testrun-gpu-helper:
  • Killed (A). The wait doesn't count against the run's timeout: runtime=$((timeout - (now - started) + GPU_WAITED)). Result: 9 pass, 1 fail. The failing test is "a second waits for it, its wait counts against its timeout".
  • Killed (B). waited is the time since the run started: GPU_WAITED=$((now - started)). Result: 9 pass, 1 fail (the slow-setup test).
  • Killed (C). runtime=$((timeout - GPU_WAITED)). Result: 9 pass, 1 fail. The new max <= 57 assertion catches it.
• Experiments helper (worklane-exp.sh), run against experiments-helper:
  • Killed (E1). waited=0. Result: 12 pass, 1 fail (the held-lock test, waited_s >= 2).
  • Killed (E2). = instead of +=, changed in the experiments helper only. Result: 12 pass, 1 fail, from the test that requires both helpers' lock functions to be identical. The same mutant in the test-run helper is caught by the two-GPU test, as earlier reviewers measured.
• Coordinator (src/testrun/run.ts), run against testrun-runs:
  • Killed (RT1). rootMaxS.includes(g.maxS + g.waitedS). Result: 26 pass, 1 fail ("stopped at its cap").
  • Killed (RT2). Root's file counts unless it is off, so an unknown reading also counts. Result: 26 pass, 1 fail (the unread-file cases).
  • Killed (RT3). The start reading is used only when the end reading is missing: [d.gpuOptIn?.() ?? gpuOpt]. Result: 26 pass, 1 fail (the new start-reading test).
  • Killed (RT4). The fallback ignores the wait: g.maxS < t.timeout_s. Result: 26 pass, 1 fail.
  • Survived (RT5). In the branch where root's file was read, g.maxS < t.timeout_s changed to g.maxS + g.waitedS < t.timeout_s. Result: 27/27. It is only an Optional item (see below).

Fail-before (measured). I took run.ts and both helpers from origin/main, rebuilt and ran the three suites:

• testrun-runs: 25 pass, 2 fail.
• testrun-gpu-helper: 9 pass, 1 fail.
• experiments-helper: 12 pass, 1 fail.

After restoring the head's files, all three pass.

Timing flakiness

• Measured, one full run: heavy.sh runs everything at nice 19 and idle IO priority. The machine's load average was about 33 on 24 cores. Every suite passed:
  • testrun-runs 27/27
  • testrun-gpu-helper 10/10
  • testrun-gpu 14/14
  • experiments-helper 13/13
  • helper-protocol 4/4
• Measured, the four new timing tests: I ran them 3 at a time, in 2 rounds, at load 27 to 33. That gave 6 of 6 passes; each pass covered all 4 tests.
• Inferred, the max <= 57 test cannot flake from load. All five stubbed stats of paths under run/ happen in gpu_lock_file, after started is taken. Those are the parent directory, the lock directory twice and the lock file twice. So at least 3 s really passes. Whole-second timestamps can't make a difference of 3 s or more come out smaller. Load only makes max smaller, which still passes.
• Inferred, the held-lock test in the experiments helper is robust and not vacuous. The flock stub sleeps 3 s whenever its second argument is -w. flock -w is called only in gpu_lock. A 3 s sleep always gives a timestamp difference of at least 3, against the >= 2 check.

CI parity (measured)

• node scripts/push-gate.mjs --range origin/main..3a807e79: "7 commit(s) and 1 tip(s) clean".
• change-files-renames 63/63, merge-policy 56/56, changelog 27/27, owner-terminal-removed 8/8.
• bash -n passes on both helpers.

PR CI at head (measured, from gh pr view 1037 --json statusCheckRollup). Every check has finished, and all succeeded except three:

• The two bootstrap jobs were skipped, and they aren't required.
• One new-deps run was cancelled. A second new-deps run started 4 s later at the same head and succeeded. Inferred: the second run superseded the first (CI concurrency), so this is unrelated to the change.

Other checks (inferred, from reading the code)

• The experiments helper's waited is used only in the record. It is never subtracted from a step's time budget, so leaving out setup time doesn't lengthen any job.
• gpu_lock declares t0 local. Without that, it would overwrite the job's t0, which feeds gpu_seconds. Even so, removing it would make no difference: job() sets its own t0 again before training, and nothing can fail in between.
• The re-read of root's file in production goes through gpuOptIn(s.instance) in setup.ts, which isn't cached.
• The diff has a changelog fragment. No assertion is weakened or skipped, there is no brand string in src/, and nothing is out of scope.

Optional

• RT5 is equivalent except in tie cases (inferred). The two conditions differ only when root's max_minutes × 60 is under timeout_s but adding the wait reaches timeout_s. In that case the leftover of timeout_s equals root's cap exactly (both caps are the same number), or root's file changed and one of the coordinator's two readings happens to equal the leftover. This is the same kind of survivor as R2h's equivalent && g.maxS < t.timeout_s mutant.
• A remaining, unavoidable flake risk (guess). The waited [01] checks still include two date calls and the free flock call. A stall longer than 1 s right there could still flake. That window is the smallest the issue's fix allows.
• Two-GPU test margin (inferred). Each per-GPU wait shrinks by however long the helper takes between its last stat of the lock file and its t0 timestamp. The check >= 5 needs each wait to round to at least 2 or 3 s. It held in all my runs under load.
• Still open (inferred). R2h's double-change case remains: root changes max_minutes both before the helper reads it and again before the tier ends. Fixing it needs a protocol bump; the builder has already listed it as remaining work.

[jayden1711]

Owner decision (recorded by the coordinator, at the owner's instruction): the engine weakening scan's flag on this PR at 3a807e7 is not a weakening. The flag is for platform skips on 8 new tests (bash 4/GNU tools, POSIX or perl missing), which Linux CI runs. No existing test or assertion changed. It may merge under docs/process.md once its required checks pass on a combined check built on main's tip. This covers this head only.