From cca2806e57c272ed919b1cde224d3d2a23d2f668 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 24 Jul 2026 16:29:47 -0700 Subject: [PATCH] test(tool-gateway): make idle-down slot test deterministic via injected clock (#10226) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server has a gateway layer that coordinates tool execution and runtime slots > - The idle-down test for the local stdio fixture slot was relying on real wall-clock timing > - On slower runners, that made the test nondeterministic because the slot could be reaped before the presence assertion ran > - This pull request switches the test to use the existing injectable clock seam so time only advances when the test says it should > - The benefit is that the idle-down behavior stays covered while the test becomes deterministic and no longer flakes under load ## Linked Issues or Issue Description This PR fixes a flaky gateway test in the server test suite. The `tool-gateway` idle-down scenario was asserting slot presence while also depending on a very short real-time idle TTL and a later sleep-based reap. On loaded runners, the intervening work could exceed the TTL, which caused the slot to disappear early and the assertion to see an empty list. The fix keeps the production code path unchanged and drives the test from the supervisor's existing injectable clock. The test now holds time steady through the presence check, then advances the clock past the idle deadline to trigger the reap deterministically. The original behavioral assertions stay intact: slot reuse, counter increments, metadata, and stop status still get verified. ## What Changed - Replaced the real-time idle-down wait in the `tool-gateway` test with the runtime supervisor's injectable clock seam. - Kept the existing assertions for slot reuse, slot identity, counters, metadata, and stop behavior. - Removed the test's dependency on wall-clock timing so the idle-down path is deterministic under load. ## Verification - Targeted server typecheck passed with `tsc --noEmit`. - `tool-gateway.test.ts` passed in full: 49/49. - The targeted idle-down scenario passed 50/50 in a tight loop with 0 failures after the clock injection change. ## Risks - Low risk: this is a test-only change and does not modify production gateway logic. - The test now exercises the idle-down logic through a controlled clock rather than real elapsed time, which is the point of the fix but does slightly reduce wall-clock realism in the test itself. ## Model Used OpenAI Codex (GPT-5), tool-using coding agent; context window not surfaced in the workspace. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Harold Kim Co-authored-by: Paperclip --- server/src/__tests__/tool-gateway.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/tool-gateway.test.ts b/server/src/__tests__/tool-gateway.test.ts index 7ed7d47ed8..5a7d4cc0a3 100644 --- a/server/src/__tests__/tool-gateway.test.ts +++ b/server/src/__tests__/tool-gateway.test.ts @@ -3517,7 +3517,15 @@ rl.on("line", (line) => { "mcp-stdio-fixture:increment_counter", "mcp-stdio-fixture:runtime_status", ]); - const gateway = createTestToolGatewayService(db, { runtimeSupervisor: { idleTtlMs: 25 } }); + // Drive idle-down off an injected clock so the assertions below do not + // depend on real wall-clock elapsing under 25ms (the source of the flake). + // The supervisor computes idleDeadlineAt = now() + idleTtlMs and reaps lazily + // on every listRuntimeSlots call, so a fixed clock keeps the slot alive until + // we deliberately advance past the TTL. + let clockMs = Date.now(); + const gateway = createTestToolGatewayService(db, { + runtimeSupervisor: { idleTtlMs: 25, now: () => new Date(clockMs) }, + }); const session = await gateway.createSession({ companyId: company.id, agentId: agent.id, @@ -3540,6 +3548,7 @@ rl.on("line", (line) => { expect(firstData).toMatchObject({ lazyStarted: true, reusedRuntimeSlot: false, counter: 1 }); expect(secondData).toMatchObject({ lazyStarted: false, reusedRuntimeSlot: true, counter: 1 }); expect(secondData.slotId).toBe(firstData.slotId); + // Clock has not advanced past the deadline, so the slot is deterministically present. await expect(gateway.listRuntimeSlots(company.id)).resolves.toHaveLength(1); const [idleSlot] = await db.select().from(toolRuntimeSlots).where(eq(toolRuntimeSlots.companyId, company.id)); expect(idleSlot).toMatchObject({ @@ -3554,7 +3563,8 @@ rl.on("line", (line) => { resourceLimits: expect.objectContaining({ memoryCeilingSupported: expect.any(Boolean) }), }); - await new Promise((resolve) => setTimeout(resolve, 35)); + // Advance the injected clock past the idle TTL to deterministically reap the slot. + clockMs += 35; await expect(gateway.listRuntimeSlots(company.id)).resolves.toEqual([]); const [stoppedSlot] = await db.select().from(toolRuntimeSlots).where(eq(toolRuntimeSlots.id, idleSlot.id)); expect(stoppedSlot).toMatchObject({