fix(mcp-sessions): require a minimum idle age before LRU eviction at capacity (#467)

src/config.test.ts

@@ -263,6 +263,43 @@ assert.throws(
 assert.equal(loadConfig(baseEnv).mcpSessionIdleTimeoutMs, 30 * 60 * 1000);
 assert.equal(loadConfig(baseEnv).mcpSessionMaxSessions, 256);
 assert.equal(loadConfig(baseEnv).mcpCutoverBuildReadyRoot, undefined);
+assert.equal(loadConfig(baseEnv).mcpSessionLruMinIdleMs, 60_000);
+assert.equal(
+  loadConfig({ ...baseEnv, DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: "120000" }).mcpSessionLruMinIdleMs,
+  120_000,
+);
+assert.equal(
+  loadConfig({
+    ...baseEnv,
+    DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS: "60000",
+    DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: "60000",
+  }).mcpSessionLruMinIdleMs,
+  60_000,
+);
+// A short idle timeout clamps the default instead of making it invalid.
+assert.equal(
+  loadConfig({ ...baseEnv, DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS: "30000" }).mcpSessionLruMinIdleMs,
+  30_000,
+);
+for (const bad of ["0", "-5", "1.5", "abc"]) {
+  assert.throws(
+    () => loadConfig({ ...baseEnv, DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: bad }),
+    new RegExp(`Invalid DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: ${bad}`),
+  );
+}
+assert.throws(
+  () =>
+    loadConfig({
+      ...baseEnv,
+      DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS: "30000",
+      DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: "30001",
+    }),
+  /Invalid DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: 30001/,
+);
+assert.throws(
+  () => loadConfig({ ...baseEnv, DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: "1800001" }),
+  /Invalid DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS: 1800001/,
+);
 assert.equal(
   loadConfig({ ...baseEnv, DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS: "30000" }).mcpSessionIdleTimeoutMs,
   30000,

src/config.ts

@@ -18,6 +18,7 @@ const DEFAULT_OAUTH_REFRESH_TOKEN_TTL_SECONDS = 30 * 24 * 60 * 60;
 const DEFAULT_ARTIFACT_MAX_FILE_BYTES = 100 * 1024 * 1024;
 const DEFAULT_MCP_SESSION_IDLE_TIMEOUT_MS = 30 * 60 * 1000;
 const DEFAULT_MCP_SESSION_MAX_SESSIONS = 256;
+const DEFAULT_MCP_SESSION_LRU_MIN_IDLE_MS = 60_000;
 const DEFAULT_CHAT_SWARM_MAX_WORKERS = 16;
 const DEFAULT_CHAT_SWARM_QUEUE_LIMIT = 1000;
 const DEFAULT_CHAT_SWARM_RESULT_MAX_CHARS = 256 * 1024;
@@ -52,6 +53,7 @@ export interface ServerConfig {
   codexBin?: string;
   mcpSessionIdleTimeoutMs: number;
   mcpSessionMaxSessions: number;
+  mcpSessionLruMinIdleMs: number;
   chatSwarmEnabled: boolean;
   chatSwarmMaxWorkers: number;
   chatSwarmQueueLimit: number;
@@ -387,6 +389,19 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig {
     new URL(publicBaseUrl).hostname,
     ...(files.config.allowedHosts ?? []),
   ];
+  const mcpSessionIdleTimeoutMs = parsePositiveInteger(
+    env.DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS ?? numberConfigValue(files.config.mcpSessionIdleTimeoutMs),
+    DEFAULT_MCP_SESSION_IDLE_TIMEOUT_MS,
+    "DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS",
+  );
+  // The default is clamped so a short idle timeout never makes the default invalid;
+  // an explicit value above the idle timeout is rejected.
+  const mcpSessionLruMinIdleMs = parsePositiveInteger(
+    env.DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS ?? numberConfigValue(files.config.mcpSessionLruMinIdleMs),
+    Math.min(DEFAULT_MCP_SESSION_LRU_MIN_IDLE_MS, mcpSessionIdleTimeoutMs),
+    "DEVSPACE_MCP_SESSION_LRU_MIN_IDLE_MS",
+    mcpSessionIdleTimeoutMs,
+  );
 
   return {
     host,
@@ -430,11 +445,8 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig {
     agentExecutionBackend: parseAgentExecutionBackend(env.DEVSPACE_AGENT_EXECUTION_BACKEND),
     codexGoalsEnabled: parseBoolean(env.DEVSPACE_CODEX_GOALS),
     codexBin: env.DEVSPACE_CODEX_BIN?.trim() || undefined,
-    mcpSessionIdleTimeoutMs: parsePositiveInteger(
-      env.DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS ?? numberConfigValue(files.config.mcpSessionIdleTimeoutMs),
-      DEFAULT_MCP_SESSION_IDLE_TIMEOUT_MS,
-      "DEVSPACE_MCP_SESSION_IDLE_TIMEOUT_MS",
-    ),
+    mcpSessionIdleTimeoutMs,
+    mcpSessionLruMinIdleMs,
     mcpSessionMaxSessions: parsePositiveInteger(
       env.DEVSPACE_MCP_SESSION_MAX_SESSIONS ?? numberConfigValue(files.config.mcpSessionMaxSessions),
       DEFAULT_MCP_SESSION_MAX_SESSIONS,

src/mcp-sessions.test.ts

@@ -164,6 +164,7 @@ const boundedOverflow = createTransport();
 assert.deepEqual(boundedRegistry.register("overflow", boundedOverflow), {
   accepted: false,
   reason: "capacity_exhausted",
+  rejectionCause: "all_in_flight",
 });
 assert.equal(boundedRegistry.size, 3);
 assert.equal(boundedRegistry.get("oldest"), oldest);
@@ -175,6 +176,7 @@ const boundedOverflowTwo = createTransport();
 assert.deepEqual(boundedRegistry.register("overflow-two", boundedOverflowTwo), {
   accepted: false,
   reason: "capacity_exhausted",
+  rejectionCause: "all_in_flight",
 });
 assert.equal(boundedRegistry.get("oldest"), oldest);
 assert.equal(boundedRegistry.get("newest"), newest);
@@ -242,7 +244,11 @@ assert.equal(lifecycleRegistry.size, 2);
 assert.equal(lifecycleRegistry.beginRequest("lifecycle-third"), true);
 const rejectedTransport = createTransport();
 const rejectedRegistration = lifecycleRegistry.register("lifecycle-rejected", rejectedTransport);
-assert.deepEqual(rejectedRegistration, { accepted: false, reason: "capacity_exhausted" });
+assert.deepEqual(rejectedRegistration, {
+  accepted: false,
+  reason: "capacity_exhausted",
+  rejectionCause: "all_in_flight",
+});
 assert.equal(lifecycleRegistry.size, 2);
 assert.equal(lifecycleRegistry.get("lifecycle-rejected"), undefined);
 await Promise.resolve();
@@ -476,11 +482,12 @@ const preflightRegistry = new McpSessionRegistry<FakeTransport>({
   now: () => preflightNow,
   maxSessions: 1,
   idleTimeoutMs: 100,
+  lruMinIdleMs: 40,
 });
 let preflightNow = 0;
 preflightRegistry.register("preflight-resident", createTransport());
 preflightNow = 50;
-// A young idle resident is LRU-evictable, so the pre-check agrees with register().
+// A resident idle past lruMinIdleMs is LRU-evictable, so the pre-check agrees with register().
 assert.equal(preflightRegistry.canAcceptRegistration(), true);
 assert.equal(preflightRegistry.beginRequest("preflight-resident"), true);
 assert.equal(preflightRegistry.admitRegistration(), false);
@@ -489,6 +496,7 @@ const postPreflightTransport = createTransport();
 assert.deepEqual(preflightRegistry.register("post-preflight", postPreflightTransport), {
   accepted: false,
   reason: "capacity_exhausted",
+  rejectionCause: "all_in_flight",
 });
 await Promise.resolve();
 assert.equal(preflightRegistry.metrics().capacityRejections, 2);
@@ -570,6 +578,7 @@ assert.equal(closingRegistry.acknowledgeToolsList("closing", currentSnapshot), f
     now: () => lruNow,
     maxSessions: 256,
     idleTimeoutMs: 1_800_000,
+    lruMinIdleMs: 500,
     onDispose: (sessionId, _transport, reason) => lruDisposals.push([sessionId, reason]),
   });
   const lruTransports: FakeTransport[] = [];
@@ -608,6 +617,7 @@ assert.equal(closingRegistry.acknowledgeToolsList("closing", currentSnapshot), f
   assert.deepEqual(lruRegistry.register("lru-rejected", lruRejected), {
     accepted: false,
     reason: "capacity_exhausted",
+    rejectionCause: "all_in_flight",
   });
   assert.equal(lruRegistry.metrics().capacityRejections, 2);
   assert.equal(lruRegistry.metrics().lruEvictions, 1);
@@ -632,3 +642,100 @@ assert.equal(closingRegistry.acknowledgeToolsList("closing", currentSnapshot), f
   assert.equal(idleRegistry.metrics().capacityEvictions, 1);
   assert.equal(idleRegistry.metrics().lruEvictions, 0);
 }
+
+// Issue #467: LRU eviction at capacity requires a minimum idle age.
+{
+  const fillRegistry = (lruMinIdleMs: number, clock: { now: number }) => {
+    const registry = new McpSessionRegistry<FakeTransport>({
+      now: () => clock.now,
+      maxSessions: 256,
+      idleTimeoutMs: 1_800_000,
+      lruMinIdleMs,
+    });
+    const transports: FakeTransport[] = [];
+    for (let i = 0; i < 256; i += 1) {
+      const transport = createTransport();
+      transports.push(transport);
+      assert.equal(registry.register(`min-${i}`, transport).accepted, true);
+    }
+    return { registry, transports };
+  };
+
+  // (a) Young sessions stay protected.
+  {
+    const clock = { now: 0 };
+    const { registry, transports } = fillRegistry(60_000, clock);
+    clock.now = 10_000;
+    assert.equal(registry.canAcceptRegistration(), false);
+    assert.equal(registry.admitRegistration(), false);
+    assert.equal(registry.capacityRejectionCause(), "no_session_idle_min");
+    assert.equal(registry.metrics().capacityRejections, 1);
+    const rejected = createTransport();
+    assert.deepEqual(registry.register("min-new", rejected), {
+      accepted: false,
+      reason: "capacity_exhausted",
+      rejectionCause: "no_session_idle_min",
+    });
+    const metrics = registry.metrics();
+    assert.equal(metrics.capacityRejections, 2);
+    assert.equal(metrics.lruEvictions, 0);
+    assert.equal(metrics.capacityEvictions, 0);
+    assert.equal(registry.size, 256);
+    assert.equal(transports.every((t) => t.closeCalls === 0), true);
+  }
+
+  // (b) Sessions idle past the minimum are evicted LRU-first, no rejection.
+  {
+    const clock = { now: 0 };
+    const { registry, transports } = fillRegistry(60_000, clock);
+    clock.now = 90_000;
+    assert.equal(registry.admitRegistration(), true);
+    assert.deepEqual(registry.register("min-new", createTransport()), { accepted: true, evicted: 1 });
+    await Promise.resolve();
+    const metrics = registry.metrics();
+    assert.equal(metrics.lruEvictions, 1);
+    assert.equal(metrics.capacityRejections, 0);
+    assert.equal(transports[0].closeCalls, 1);
+    assert.equal(registry.get("min-0"), undefined);
+  }
+
+  // Boundary: exactly lruMinIdleMs idle is evictable; one tick younger is not.
+  {
+    const clock = { now: 0 };
+    const { registry } = fillRegistry(60_000, clock);
+    clock.now = 59_999;
+    assert.equal(registry.canAcceptRegistration(), false);
+    clock.now = 60_000;
+    assert.equal(registry.canAcceptRegistration(), true);
+  }
+
+  // (c) All in flight is reported distinctly from the young-session case.
+  {
+    const clock = { now: 0 };
+    const { registry } = fillRegistry(60_000, clock);
+    clock.now = 90_000;
+    for (let i = 0; i < 256; i += 1) assert.equal(registry.beginRequest(`min-${i}`), true);
+    clock.now = 200_000;
+    assert.equal(registry.admitRegistration(), false);
+    assert.equal(registry.capacityRejectionCause(), "all_in_flight");
+    assert.deepEqual(registry.register("min-new", createTransport()), {
+      accepted: false,
+      reason: "capacity_exhausted",
+      rejectionCause: "all_in_flight",
+    });
+    assert.equal(registry.metrics().lruEvictions, 0);
+  }
+
+  // Mixed: one in-flight old session plus young idle sessions is still "no_session_idle_min".
+  {
+    const clock = { now: 0 };
+    const { registry } = fillRegistry(60_000, clock);
+    clock.now = 100_000;
+    assert.equal(registry.beginRequest("min-0"), true);
+    // beginRequest refreshes min-0; all others are now old enough, so touch them young again.
+    for (let i = 1; i < 256; i += 1) assert.equal(registry.get(`min-${i}`) !== undefined, true);
+    clock.now = 110_000;
+    assert.equal(registry.capacityRejectionCause(), "no_session_idle_min");
+    assert.equal(registry.canAcceptRegistration(), false);
+  }
+}

src/mcp-sessions.ts

@@ -12,7 +12,17 @@ export type McpSessionDisposalReason = "capacity_eviction" | "capacity_lru" | "i
 
 export type McpSessionRegistrationResult =
   | { accepted: true; evicted: number }
-  | { accepted: false; reason: "capacity_exhausted" | "duplicate_session" };
+  | { accepted: false; reason: "duplicate_session" }
+  | { accepted: false; reason: "capacity_exhausted"; rejectionCause: McpSessionCapacityRejectionCause };
+
+/**
+ * Why a registration at capacity could not evict anything: every resident
+ * session is in flight or pending close, or some are evictable but none has
+ * been idle for at least lruMinIdleMs.
+ */
+export type McpSessionCapacityRejectionCause = "all_in_flight" | "no_session_idle_min";
+
+const DEFAULT_LRU_MIN_IDLE_MS = 60_000;
 
 interface McpSessionEntry<TTransport> {
   transport: TTransport;
@@ -27,6 +37,8 @@ export interface McpSessionRegistryOptions<TTransport extends ClosableMcpTranspo
   now?: () => number;
   maxSessions?: number;
   idleTimeoutMs?: number;
+  /** Minimum idle age before LRU eviction at capacity. Defaults to min(60s, idleTimeoutMs). */
+  lruMinIdleMs?: number;
   onDispose?: (sessionId: string, transport: TTransport, reason: McpSessionDisposalReason) => void;
 }
 
@@ -59,6 +71,7 @@ export class McpSessionRegistry<TTransport extends ClosableMcpTransport> {
   private readonly now: () => number;
   private readonly maxSessions?: number;
   private readonly idleTimeoutMs?: number;
+  private readonly lruMinIdleMs: number;
   private readonly onDispose?: McpSessionRegistryOptions<TTransport>["onDispose"];
   private highWaterActiveSessions = 0;
   private registrations = 0;
@@ -74,6 +87,8 @@ export class McpSessionRegistry<TTransport extends ClosableMcpTransport> {
     this.now = options.now ?? Date.now;
     this.maxSessions = options.maxSessions;
     this.idleTimeoutMs = options.idleTimeoutMs;
+    this.lruMinIdleMs = options.lruMinIdleMs
+      ?? Math.min(DEFAULT_LRU_MIN_IDLE_MS, options.idleTimeoutMs ?? DEFAULT_LRU_MIN_IDLE_MS);
     this.onDispose = options.onDispose;
   }
 
@@ -86,6 +101,14 @@ export class McpSessionRegistry<TTransport extends ClosableMcpTransport> {
     return this.hasEvictableSession();
   }
 
+  /** Why a registration at capacity would be rejected right now (call only when at capacity). */
+  capacityRejectionCause(): McpSessionCapacityRejectionCause {
+    for (const entry of this.sessions.values()) {
+      if (entry.inFlight === 0 && !entry.pendingClose) return "no_session_idle_min";
+    }
+    return "all_in_flight";
+  }
+
   /**
    * Advisory admission check for callers that must reject before allocating a
    * transport. A failed check is one counted capacity rejection; a later
@@ -113,7 +136,7 @@ export class McpSessionRegistry<TTransport extends ClosableMcpTransport> {
     if (this.maxSessions !== undefined && this.sessions.size >= this.maxSessions) {
       this.capacityRejections += 1;
       void this.closeUnregisteredTransport(transport);
-      return { accepted: false, reason: "capacity_exhausted" };
+      return { accepted: false, reason: "capacity_exhausted", rejectionCause: this.capacityRejectionCause() };
     }
     this.sessions.set(sessionId, {
       transport,
@@ -217,15 +240,18 @@ export class McpSessionRegistry<TTransport extends ClosableMcpTransport> {
 
   /**
    * Last-resort eviction at capacity: the least-recently-active session with no
-   * in-flight work, regardless of age. Runs only after idle eviction.
+   * in-flight work that has been idle for at least lruMinIdleMs. Runs only after
+   * idle eviction; younger sessions stay protected.
    */
   private evictLruToLimit(): Array<{ sessionId: string; transport: TTransport }> {
     const evicted: Array<{ sessionId: string; transport: TTransport }> = [];
+    const lruCutoff = this.now() - this.lruMinIdleMs;
     while (this.sessions.size >= this.maxSessions!) {
       let lruId: string | undefined;
       let lruActivity = Number.POSITIVE_INFINITY;
       for (const [sessionId, entry] of this.sessions) {
         if (entry.inFlight > 0 || entry.pendingClose) continue;
+        if (entry.lastActivityAt > lruCutoff) continue;
         if (entry.lastActivityAt < lruActivity) {
           lruActivity = entry.lastActivityAt;
           lruId = sessionId;
@@ -241,8 +267,9 @@ export class McpSessionRegistry<TTransport extends ClosableMcpTransport> {
   }
 
   private hasEvictableSession(): boolean {
+    const lruCutoff = this.now() - this.lruMinIdleMs;
     for (const entry of this.sessions.values()) {
-      if (entry.inFlight === 0 && !entry.pendingClose) return true;
+      if (entry.inFlight === 0 && !entry.pendingClose && entry.lastActivityAt <= lruCutoff) return true;
     }
     return false;
   }