[nissa-seru]

Two of the tool-result guard's batch-deadline tests fail now and then on a loaded machine, with no change to the code they test. They now hold the clock still, so the host's load can't spend a batch's deadline before its write starts, and so does a third test with the same window. It changes tests only.

What was wrong

The batch's deadline is a Date.now() time taken when the batch is queued: Date.now() plus withheldSpillDeadlineMs (src/framework.ts:4175). spillWithheld skips the write once deadline - Date.now() is spent (:14545–:14547). These two tests set the deadline to 50 ms, and both drive whole runs, so on a busy host the steps between the batch's queueing and its first write can take all of it:

• "a write still under way at the deadline is named as unfinished, and nothing waits on it longer" saw no write start, since the 50 ms ran out first.
• "batches behind one stalled write cost one deadline: no write starts until it settles" saw its third batch, whose write has nothing stalling it, run out of its 50 ms before the write began. Its log says the batch "used its deadline before all its originals were written".

A longer deadline only makes that rarer.

Changes

Both tests hold Date still with node:test's mock timers, mocking Date alone. No batch's deadline can then be spent before its write starts, however long the steps before it take. The spill's own timer is a real setTimeout, so the first write's deadline still passes 50 ms after the write starts, while the stalled write is under way, which is what both tests check. The second test's third batch, whose write is a real one with nothing stalling it, gets a deadline of 60 s, far beyond what a write there takes, so a slow write on a loaded host isn't named unfinished.

The file has three other short-deadline tests:

• "writes that stalled together each hold the workspace as unanswered until it settles" calls spillWithheld directly with a 30 ms deadline. spillWithheld reads the clock for the deadline and again before the write starts, with the write's place found between, so a pause of the process there on a loaded host can spend it. It holds Date still too.
• "slow successful writes share one batch deadline" and "a withheld deadline and an unanswered skip are observable" already hold Date and setTimeout still, and advance them by hand.

The tests' own wall-clock bounds stay as they are: bounded() fails a run only once it has taken 5 s, and until() a wait after about 2 s, so a run that hangs fails rather than hanging the suite. No assertion about the guard rests on them.

What each part is for

The change is for one thing: three tests of the guard's batch deadline give the same verdict on a loaded host as on an idle one, and still check what they checked.

• Holding Date still in the three tests whose deadline is a Date.now() time, one line each. What each test checks depends on its deadline not being spent before the write starts, and with the clock held it can't be, however long the steps before the write take. The spill's own timer stays real, so a deadline still passes while a stalled write is under way. Weighed: a longer deadline, which was this change's first form. It only makes the failure rarer.
• The third batch's 60 s deadline in the second test. That batch's write is a real one with nothing stalling it, timed by the spill's real timer, so a 50 ms deadline could name a slow write on a loaded host unfinished. 60 s is beyond the run's own 5 s bound, so no verdict rests on it.
• The comments say why each test holds the clock, so a later change to the deadline code knows what the hold is for.

Left as they are: bounded()'s 5 s and until()'s 400 polls 5 ms apart, which turn a hang into a failure, and whose lengths decide no verdict on a working guard. Cut: nothing.

Evidence

• Before: in our runs on a loaded Linux machine, at heads that don't touch the guard's deadline code, the first test failed in a full run on Node 24 with --test-force-exit, both failed in one on Node 20, and the second failed in a cross-package check of fix(stop): a stop at a tool boundary stops the turn #382 and in a run of the file on Node 24 at loads of 55 to 66.
• A probe that makes the race certain: 150 ms of synchronous work added between a deadline's reading and its write's start (in toolResultFilePlace, which spillWithheld calls between its two readings of the clock). With it, at this head, all three tests pass in this form and all three fail in main's, the direct-call test included, on Node 24.22.0 and on Node 20.20.2.
• At this head, on Linux, from a fresh install at a clean checkout: test/tool-result-guard.test.ts passed 124/124 in each of 3 runs on Node 24.22.0 without --test-force-exit and 3 on Node 20.20.2 with it, at 1-minute load averages of 39 to 51. Typecheck and build are clean.
• An earlier version of this change, without the direct-call test's hold: before it was committed, the file passed 124/124 in each of 8 runs on Node 24.22.0 without --test-force-exit and 8 on Node 20.20.2 with it, at 1-minute load averages of 31 to 61. Committed, it passed in 3 of 3 more runs on Node 24.22.0 at a clean checkout, starting at a 1-minute load average of 47.

@antra-tess, @Anarchid: it will merge once its checks pass, and not before 18:00 UTC on October 17, unless you ask for changes before then.

— Ada-1017 (Claude Opus 5.5)

[greptile-apps]

Confidence Score: 5/5

[Low impact] This test-only change appears safe to merge.

Summary

Makes three guard tests less sensitive to host load by freezing Date while leaving setTimeout real.

• Three guard tests keep short deadlines from expiring before writes start.

_{Reviews (1) · Last reviewed commit: "test(guard): hold the clock still in thr..." · Reviewed by Greptile}