test(core): make wait-queue's re-arm test prove the mechanism, not just the timing (H5)

The test titled "re-arms only the time actually remaining on a
re-enqueue before the deadline" asserted only on waiter.state at
various clock ticks and when its promise finally rejected -- every one
of those observations comes out identical whether #armTimeout actually
cancels the original timer and starts a fresh one for the recomputed
remainder, or simply leaves the still-live original timer alone. Both
hit the same fixed deadlineAt.

Retitled it and added clock.setTimer/cancel spies asserting on the
mechanism directly: exactly one timer is armed across the whole
queued -> processing -> queued cycle, never two. Documented in
#armTimeout's own comment why that is the only thing that can happen:
its `waiter.timer !== undefined` guard means a genuine partial-value
recompute-and-re-arm is unreachable through this class's public API --
every later call either finds the original timer still live (early
return, left untouched) or already past its deadline (enqueue's own
upfront check rejects before #armTimeout runs again).

Verified by mutating #armTimeout to unconditionally cancel and re-arm
on every call: the retitled test then fails with a named assertion
(setTimer called 2 times, expected 1), proving it can actually tell
the two mechanisms apart. Re-ran the worker's own lease-acquisition
suite, the gateway boundary/fleet-coordinator suites, and the full
unit suite (1775 passed) since this is shared core on the worker's
lease path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MA98m7ua7qvDFZjxFaww6Z

Author: claude

src/core/wait-queue.test.ts

@@ -151,17 +151,39 @@ describe("WaitQueue", () => {
     expect(timedOut).toHaveBeenCalledWith(waiter);
   });
 
-  it("re-arms only the time actually remaining on a re-enqueue before the deadline, so the total wait never exceeds the original timeoutMs", async () => {
+  // H5 (round 3 review): this test used to claim it proved a re-enqueue "re-arms only the time
+  // actually remaining" -- but every assertion in it (the waiter's `state` at various clock
+  // ticks, and when its promise finally rejects) would come out identical whether `#armTimeout`
+  // actually cancelled the original timer and started a fresh one for the recomputed remainder,
+  // or simply left that still-live timer alone. Both mechanisms hit the same fixed `deadlineAt`.
+  // The `setTimer` spy below is what tells them apart: it asserts directly on the *mechanism* --
+  // exactly one timer is ever armed across the whole `queued -> processing -> queued` cycle, not
+  // two -- which is what `#armTimeout`'s own doc comment now documents as the only thing that
+  // can happen (its `waiter.timer !== undefined` guard makes a genuine recompute-and-re-arm
+  // unreachable through this class's public API). The externally observed timing this test also
+  // pins is real and worth keeping; it just is not, on its own, evidence of which mechanism
+  // produced it.
+  it("leaves a still-live timer alone across a queued -> processing -> queued cycle, rather than cancelling and re-arming it, so the total wait never exceeds the original timeoutMs (H5, round 3 review)", async () => {
     const { clock, queue } = createQueue();
     const waiter = createWaiter(queue, "agent", { timeoutMs: 100 });
+    const setTimerSpy = vi.spyOn(clock, "setTimer");
+    const cancelSpy = vi.spyOn(clock, "cancel");
 
     queue.enqueue(waiter);
+    expect(setTimerSpy).toHaveBeenCalledTimes(1); // the first (and, this test proves, only) arm
     clock.advance(30);
     queue.markProcessing(waiter);
     clock.advance(20); // t=50: still well before the t=100 deadline
     expect(queue.enqueue(waiter)).toBe(true); // re-queued, e.g. a stale-view NO_CAPACITY
     expect(waiter.state).toBe("queued");
 
+    // The proof this test exists for: re-enqueuing well before the deadline armed nothing new
+    // and cancelled nothing -- `#armTimeout`'s guard found the original timer still live and
+    // returned immediately. A recompute-and-re-arm implementation would have cancelled that
+    // timer and called `setTimer` a second time here, for the ~50ms actually remaining.
+    expect(setTimerSpy).toHaveBeenCalledTimes(1);
+    expect(cancelSpy).not.toHaveBeenCalled();
+
     // If this cycle had re-armed a fresh 100ms window, the waiter would still be pending here
     // (t=50 + 49ms = 99ms of its own window, or 149ms of total elapsed time either way). It
     // does not: the original deadline was t=100, and only 50ms of elapsed time remain from it.
@@ -170,6 +192,9 @@ describe("WaitQueue", () => {
     clock.advance(1); // t=100: the original deadline, reached exactly once, not once per cycle
     await expect(waiter.promise).rejects.toEqual(expect.any(QueueTimeoutError));
     expect(queue.depth).toBe(0);
+    // Still just the one timer, start to finish -- the fixed deadline was honoured by leaving it
+    // running, not by any second arm.
+    expect(setTimerSpy).toHaveBeenCalledTimes(1);
   });
 
   it("never delivers a progress push to a waiter that already settled, even the very same tick (P3, round 2 review)", async () => {

src/core/wait-queue.ts

@@ -318,6 +318,25 @@ export class WaitQueue {
    * the request is actively being handled, and this class does not know whether that attempt is
    * about to succeed. `enqueue`'s own deadline check is what catches this case at the next
    * re-queue, since `waiter.timer` is already cleared by the time this branch returns.
+   *
+   * H5 (round 3 review): the `waiter.timer !== undefined` guard above means `remainingMs` is
+   * only ever actually computed with a genuinely partial value in theory, never in practice --
+   * every real call into this method lands in one of exactly two cases. A fresh arm (`waiter.
+   * timer` was never set) always computes the *full* `timeoutMs`, since `deadlineAt` is being
+   * fixed in the same line. Every later call finds one of two things: the original timer is
+   * still counting down (`waiter.timer !== undefined`), so the guard above returns immediately
+   * and the still-live timer -- already targeting the correct fixed `deadlineAt` -- is left
+   * untouched, no second `setTimer` call, no recompute, no re-arm; or that timer already fired
+   * exactly at `deadlineAt` (clearing itself, above), in which case `enqueue`'s own upfront
+   * `clock.now() >= deadlineAt` check rejects the waiter before this method is even called
+   * again. Nothing in this class's public API cancels an armed timer without settling its
+   * waiter, so there is no third case where `waiter.timer === undefined`, a `deadlineAt` is
+   * already set, and `clock.now()` is still short of it -- the one shape that would make this
+   * arithmetic produce something other than the full budget or an already-expired one. The
+   * "remaining budget" behaviour this method's own name promises is real (a re-enqueue can never
+   * push the total wait past the original `timeoutMs`), it is just delivered by leaving the
+   * first timer alone rather than by this line ever recomputing a partial value -- see this
+   * method's own test for what that means for what a test here can and cannot prove.
    */
   #armTimeout(waiter: MutableWaiter): void {
     if (waiter.timer !== undefined || waiter.options.timeoutMs === undefined) return;