fix(agents): do not refresh lastUsedAt on MCP lease release (#91124)

openperf · clawsweeper[bot] · Takhoffman · web-flow · commit f2530de8320e · 2026-06-08T02:08:28.000Z
Summary: - The PR removes release-time `lastUsedAt` refresh from session MCP runtime lease cleanup and adds regression tests for idle eviction after a lease expires while active. - PR surface: Source 0, Tests +74. Total +74 across 2 files. - Reproducibility: yes. from source inspection: current main refreshes `lastUsedAt` in the release callback, a ... timestamp after active leases drop to zero. I did not execute the focused Vitest in this read-only review. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(agents): do not refresh lastUsedAt on MCP lease release Validation: - ClawSweeper review passed for head c914478. - Required merge gates passed before the squash merge. Prepared head SHA: c914478 Review: #91124 (comment) Co-authored-by: openperf <16864032@qq.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
diff --git a/src/agents/agent-bundle-mcp-runtime.test.ts b/src/agents/agent-bundle-mcp-runtime.test.ts
@@ -7,11 +7,12 @@ import { afterEach, describe, expect, it, vi } from "vitest";
 import { createBundleMcpJsonSchemaValidator } from "./agent-bundle-mcp-runtime.js";
 import { cleanupBundleMcpHarness } from "./agent-bundle-mcp-test-harness.js";
 import {
-  testing,
+  createSessionMcpRuntime,
   getOrCreateSessionMcpRuntime,
   materializeBundleMcpToolsForRun,
   retireSessionMcpRuntime,
   retireSessionMcpRuntimeForSessionKey,
+  testing,
 } from "./agent-bundle-mcp-tools.js";
 import type { SessionMcpRuntime } from "./agent-bundle-mcp-types.js";
 import { writeExecutable } from "./bundle-mcp-shared.test-harness.js";
@@ -1591,7 +1592,6 @@ process.on("SIGINT", shutdown);`,
           activeLeases += 1;
           return () => {
             activeLeases -= 1;
-            lastUsedAt = now;
           };
         },
         dispose: async () => {
@@ -1626,6 +1626,65 @@ process.on("SIGINT", shutdown);`,
     expect(manager.resolveSessionId("agent:test:session-idle")).toBeUndefined();
   });
 
+  it("evicts immediately after release when TTL already elapsed during active lease", async () => {
+    let now = 1_000;
+    const disposed: string[] = [];
+    const createRuntime: RuntimeFactory = (params) => {
+      let lastUsedAt = now;
+      let activeLeases = 0;
+      return {
+        ...makeRuntime([{ toolName: "bundle_probe", description: "Bundle MCP probe" }]),
+        sessionId: params.sessionId,
+        sessionKey: params.sessionKey,
+        workspaceDir: params.workspaceDir,
+        configFingerprint: params.configFingerprint ?? "fingerprint",
+        get lastUsedAt() {
+          return lastUsedAt;
+        },
+        get activeLeases() {
+          return activeLeases;
+        },
+        markUsed: () => {
+          lastUsedAt = now;
+        },
+        acquireLease: () => {
+          activeLeases += 1;
+          return () => {
+            activeLeases -= 1;
+          };
+        },
+        dispose: async () => {
+          disposed.push(params.sessionId);
+        },
+      };
+    };
+    const manager = testing.createSessionMcpRuntimeManager({
+      createRuntime,
+      now: () => now,
+      enableIdleSweepTimer: false,
+    });
+
+    const runtime = await manager.getOrCreate({
+      sessionId: "session-idle-post-release",
+      sessionKey: "agent:test:session-idle-post-release",
+      workspaceDir: "/workspace",
+      cfg: { mcp: { servers: {}, sessionIdleTtlMs: 50 } },
+    });
+    const releaseLease = runtime.acquireLease?.();
+
+    // TTL elapses while the lease is still held, so sweep skips active runtimes.
+    now += 60;
+    await expect(manager.sweepIdleRuntimes()).resolves.toBe(0);
+
+    // Release must not reset lastUsedAt; the runtime is evictable on the very
+    // next sweep without waiting another full TTL.
+    releaseLease?.();
+    await expect(manager.sweepIdleRuntimes()).resolves.toBe(1);
+
+    expect(disposed).toEqual(["session-idle-post-release"]);
+    expect(manager.listSessionIds()).toStrictEqual([]);
+  });
+
   it("keeps idle runtime eviction disabled when the TTL is zero", async () => {
     let now = 1_000;
     const disposed: string[] = [];
@@ -1655,6 +1714,21 @@ process.on("SIGINT", shutdown);`,
     expect(manager.listSessionIds()).toEqual(["session-no-ttl"]);
     expect(disposed).toStrictEqual([]);
   });
+
+  it("production createSessionMcpRuntime acquireLease release does not refresh lastUsedAt", () => {
+    const runtime = createSessionMcpRuntime({
+      sessionId: "session-lease-timestamp-check",
+      workspaceDir: "/workspace",
+      cfg: { mcp: { servers: {} } },
+    });
+    const lastUsedBefore = runtime.lastUsedAt;
+    if (!runtime.acquireLease) {
+      throw new Error("Expected production session MCP runtime to expose acquireLease");
+    }
+    const release = runtime.acquireLease();
+    release();
+    expect(runtime.lastUsedAt).toBe(lastUsedBefore);
+  });
 });
 
 describe("disposeSession timeout", () => {
diff --git a/src/agents/agent-bundle-mcp-runtime.ts b/src/agents/agent-bundle-mcp-runtime.ts
@@ -765,7 +765,7 @@ export function createSessionMcpRuntime(params: {
         }
         released = true;
         activeLeases = Math.max(0, activeLeases - 1);
-        lastUsedAt = Date.now();
+        // Release is not use: refreshing lastUsedAt here defeats the idle-sweep TTL.
       };
     },
     getCatalog,

Original file line number	Diff line number	Diff line change
`@@ -765,7 +765,7 @@ export function createSessionMcpRuntime(params: {`
`765`	`765`	`}`
`766`	`766`	`released = true;`
`767`	`767`	`activeLeases = Math.max(0, activeLeases - 1);`
`768`		`- lastUsedAt = Date.now();`
	`768`	`+ // Release is not use: refreshing lastUsedAt here defeats the idle-sweep TTL.`
`769`	`769`	`};`
`770`	`770`	`},`
`771`	`771`	`getCatalog,`