From e2e81d45361d2c583f1318d1221864fa421a1409 Mon Sep 17 00:00:00 2001 From: Vibe Code Date: Sat, 8 Aug 2026 01:32:16 +0200 Subject: [PATCH] fluentcart-mcp: stop the cancellation test racing the clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'removes active listeners on timeout and service shutdown' slept a real 20ms against a 10ms timeout and then asserted the entry had expired. True on an idle machine; false whenever a loaded runner stretched the gap. It failed the Node 26 lane and passed the Node 24 lane in the same CI run, which is as clear a statement of wall-clock dependence as a test can make. Now on fake timers, matching the tombstone test directly above it, with the clock advanced by exactly the timeout. Expiry becomes a fact rather than a margin. Mutation-checked, because a fake-timer test is an easy way to write one that passes regardless: advancing 0 instead of 10 reproduces the original failure exactly — expected true to be false — so the assertion still tests expiry. Restored, then run five times: 19/19 each time. The close() in the new finally block is a no-op, since close() returns early on this.closed and the test closes mid-body on purpose. The three remaining real-timer sleeps in the suite are a different shape: they sit inside a loader or handler to simulate latency, so a slower runner lengthens the simulated work rather than invalidating an assertion. Left alone. --- .../tests/transport/http-cancellation.test.ts | 66 +++++++++++-------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/fluentcart-mcp/tests/transport/http-cancellation.test.ts b/fluentcart-mcp/tests/transport/http-cancellation.test.ts index 23345c8f..397740dc 100644 --- a/fluentcart-mcp/tests/transport/http-cancellation.test.ts +++ b/fluentcart-mcp/tests/transport/http-cancellation.test.ts @@ -198,6 +198,13 @@ describe('legacy HTTP cancellation routing', () => { }) it('removes active listeners on timeout and service shutdown', async () => { + // Fake timers, like the tombstone test above. This previously slept a + // real 20ms against a 10ms timeout and asserted the entry had expired — + // true on an idle machine, false whenever a loaded runner stretched the + // gap, which is why it failed on one CI lane and passed on the other in + // the same run. Advancing the clock makes the expiry a fact rather than + // a margin. + vi.useFakeTimers() const registry = new RequestCancellationRegistry({ horizonMs: 10, timeoutMs: 10, @@ -205,33 +212,40 @@ describe('legacy HTTP cancellation routing', () => { const principal: TransportPrincipal = { kind: 'anonymous-loopback', id: 'loopback' } let aborts = 0 let releases = 0 - registry.register( - principal, - 'expired', - () => { - aborts += 1 - }, - () => { - releases += 1 - }, - ) - await new Promise((resolve) => setTimeout(resolve, 20)) - expect(registry.cancel(principal, 'expired')).toBe(false) + try { + registry.register( + principal, + 'expired', + () => { + aborts += 1 + }, + () => { + releases += 1 + }, + ) + await vi.advanceTimersByTimeAsync(10) + expect(registry.cancel(principal, 'expired')).toBe(false) - registry.register( - principal, - 'shutdown', - () => { - aborts += 1 - }, - () => { - releases += 1 - }, - ) - registry.close() - expect(registry.cancel(principal, 'shutdown')).toBe(false) - expect(aborts).toBe(0) - expect(releases).toBe(2) + registry.register( + principal, + 'shutdown', + () => { + aborts += 1 + }, + () => { + releases += 1 + }, + ) + registry.close() + expect(registry.cancel(principal, 'shutdown')).toBe(false) + expect(aborts).toBe(0) + expect(releases).toBe(2) + } finally { + // close() short-circuits on this.closed, so the mid-test close above + // makes this a no-op rather than a second round of releases. + registry.close() + vi.useRealTimers() + } }) it('disables routing on principal or request-id saturation without evicting safety state', () => {