From 4fd803fb7d48372cffe2adb9f4916327ed090bbe Mon Sep 17 00:00:00 2001 From: qualiobra Date: Fri, 20 Mar 2026 11:26:58 -0400 Subject: [PATCH] fix: add TTL eviction and max-size cap to SESSION_MANAGER_CACHE The module-level SESSION_MANAGER_CACHE Map grows unboundedly because expired entries are never deleted and there is no size limit. Changes: - Lazy TTL eviction: delete expired entries in isSessionManagerCached() - Max-size cap (500): evict oldest entry when full in trackSessionManagerAccess() - LRU refresh: delete+set on existing key access to maintain insertion order - Export clearSessionManagerCache() and sessionManagerCacheSize() for testing Note: trackSessionManagerAccess() uses delete()+set() rather than just set() because Map.set() on an existing key does not refresh its iteration position (MDN). This ensures recently-accessed entries survive eviction. --- .../session-manager-cache.test.ts | 97 +++++++++++++++++++ .../session-manager-cache.ts | 44 ++++++++- 2 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 src/agents/pi-embedded-runner/session-manager-cache.test.ts diff --git a/src/agents/pi-embedded-runner/session-manager-cache.test.ts b/src/agents/pi-embedded-runner/session-manager-cache.test.ts new file mode 100644 index 00000000000..37068605a0b --- /dev/null +++ b/src/agents/pi-embedded-runner/session-manager-cache.test.ts @@ -0,0 +1,97 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// The module reads process.env and Date.now() at call time, so we +// dynamically re-import after setting up fakes / env vars. +type Mod = typeof import("./session-manager-cache.js"); + +describe("session-manager-cache", () => { + let mod: Mod; + + beforeEach(async () => { + vi.useFakeTimers(); + // Ensure caching is enabled (default TTL > 0). + delete process.env.OPENCLAW_SESSION_MANAGER_CACHE_TTL_MS; + vi.resetModules(); + mod = await import("./session-manager-cache.js"); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + delete process.env.OPENCLAW_SESSION_MANAGER_CACHE_TTL_MS; + }); + + // ── Max-size cap ────────────────────────────────────────────── + + it("enforces max-size cap of 500 entries", () => { + for (let i = 0; i < 510; i++) { + mod.trackSessionManagerAccess(`/sessions/s-${i}.json`); + } + expect(mod.sessionManagerCacheSize()).toBeLessThanOrEqual(500); + }); + + // ── LRU ordering via delete+set ─────────────────────────────── + + it("re-access refreshes LRU position — recently used entry survives eviction", () => { + // Fill cache to 499 entries (s-0 … s-498). + for (let i = 0; i < 499; i++) { + mod.trackSessionManagerAccess(`/sessions/s-${i}.json`); + } + expect(mod.sessionManagerCacheSize()).toBe(499); + + // Re-access s-0 → moves it to the end of insertion order. + mod.trackSessionManagerAccess("/sessions/s-0.json"); + expect(mod.sessionManagerCacheSize()).toBe(499); // no growth, same key + + // Add two new entries to push past 500. + mod.trackSessionManagerAccess("/sessions/s-500.json"); // size → 500 + mod.trackSessionManagerAccess("/sessions/s-501.json"); // evicts oldest (s-1) + + // s-0 was refreshed so it should still be present (500 entries, s-0 near end). + // The evicted entry should be s-1 (oldest after s-0 was refreshed). + expect(mod.sessionManagerCacheSize()).toBe(500); + + // After eviction, the refreshed entry (s-0) must survive, oldest untouched (s-1) must be evicted + expect(mod.isSessionManagerCached("/sessions/s-0.json")).toBe(true); + expect(mod.isSessionManagerCached("/sessions/s-1.json")).toBe(false); + }); + + // ── Lazy TTL eviction ───────────────────────────────────────── + + it("lazy TTL eviction removes expired entry on cache check", async () => { + // Insert an entry directly. + mod.trackSessionManagerAccess("/sessions/a.json"); + expect(mod.sessionManagerCacheSize()).toBe(1); + + // Advance time past the 45 s default TTL. + vi.advanceTimersByTime(46_000); + + // prewarmSessionFile calls isSessionManagerCached internally, which + // should now delete the stale entry and return false. The subsequent + // fs.open will fail (file doesn't exist) so the entry is NOT re-added. + await mod.prewarmSessionFile("/sessions/a.json"); + expect(mod.sessionManagerCacheSize()).toBe(0); // expired entry was evicted + }); + + // ── clearSessionManagerCache ────────────────────────────────── + + it("clearSessionManagerCache empties the cache", () => { + mod.trackSessionManagerAccess("/sessions/a.json"); + mod.trackSessionManagerAccess("/sessions/b.json"); + expect(mod.sessionManagerCacheSize()).toBe(2); + + mod.clearSessionManagerCache(); + expect(mod.sessionManagerCacheSize()).toBe(0); + }); + + // ── Disabled cache (TTL=0) ──────────────────────────────────── + + it("skips tracking when cache is disabled via TTL=0", async () => { + process.env.OPENCLAW_SESSION_MANAGER_CACHE_TTL_MS = "0"; + vi.resetModules(); + const disabled = await import("./session-manager-cache.js"); + + disabled.trackSessionManagerAccess("/sessions/a.json"); + expect(disabled.sessionManagerCacheSize()).toBe(0); + }); +}); diff --git a/src/agents/pi-embedded-runner/session-manager-cache.ts b/src/agents/pi-embedded-runner/session-manager-cache.ts index 99dd340496f..e0bc6381c95 100644 --- a/src/agents/pi-embedded-runner/session-manager-cache.ts +++ b/src/agents/pi-embedded-runner/session-manager-cache.ts @@ -9,6 +9,9 @@ type SessionManagerCacheEntry = { const SESSION_MANAGER_CACHE = new Map(); const DEFAULT_SESSION_MANAGER_TTL_MS = 45_000; // 45 seconds +// Safety-net cap. Working set is far smaller (entries live ≤45 s); +// env override is intentionally not exposed — 500 >> realistic working set. +const SESSION_MANAGER_CACHE_MAX_SIZE = 500; function getSessionManagerTtl(): number { return resolveCacheTtlMs({ @@ -25,6 +28,20 @@ export function trackSessionManagerAccess(sessionFile: string): void { if (!isSessionManagerCacheEnabled()) { return; } + // Refresh insertion order for existing keys so FIFO eviction is LRU-like. + // Map.set() on an existing key does NOT update insertion order (MDN), so + // we delete first to move it to the end. + if (SESSION_MANAGER_CACHE.has(sessionFile)) { + SESSION_MANAGER_CACHE.delete(sessionFile); + } + // Evict oldest entry when the cache is at its size limit. + // Map iteration order is insertion-order, so the first key is the oldest. + if (SESSION_MANAGER_CACHE.size >= SESSION_MANAGER_CACHE_MAX_SIZE) { + const oldest = SESSION_MANAGER_CACHE.keys().next(); + if (!oldest.done) { + SESSION_MANAGER_CACHE.delete(oldest.value); + } + } const now = Date.now(); SESSION_MANAGER_CACHE.set(sessionFile, { sessionFile, @@ -32,7 +49,11 @@ export function trackSessionManagerAccess(sessionFile: string): void { }); } -function isSessionManagerCached(sessionFile: string): boolean { +/** + * Check whether a session file is present and non-expired in the cache. + * @internal Visible for testing only. + */ +export function isSessionManagerCached(sessionFile: string): boolean { if (!isSessionManagerCacheEnabled()) { return false; } @@ -42,7 +63,11 @@ function isSessionManagerCached(sessionFile: string): boolean { } const now = Date.now(); const ttl = getSessionManagerTtl(); - return now - entry.loadedAt <= ttl; + if (now - entry.loadedAt > ttl) { + SESSION_MANAGER_CACHE.delete(sessionFile); + return false; + } + return true; } export async function prewarmSessionFile(sessionFile: string): Promise { @@ -67,3 +92,18 @@ export async function prewarmSessionFile(sessionFile: string): Promise { // File doesn't exist yet, SessionManager will create it } } + +/** + * Clear all cached entries. Exported for tests and graceful-shutdown hooks. + */ +export function clearSessionManagerCache(): void { + SESSION_MANAGER_CACHE.clear(); +} + +/** + * Return the current number of entries in the cache. + * @internal Visible for testing only. + */ +export function sessionManagerCacheSize(): number { + return SESSION_MANAGER_CACHE.size; +}