fix(discovery-index): bound TtlCache by a max-entry cap enforced on set

Without a cap, a key that is never re-read (e.g. a scope no caller repeats,
or a soft-claim that's never released) stays resident in the long-lived
module-level caches for the whole process lifetime. set() now drops
already-expired entries first, then evicts the oldest-inserted entry until
the store is back at or under the cap.

Author: bitfathers94

packages/discovery-index/src/cache.ts

@@ -3,17 +3,24 @@
 // (src/selfhost/redis-cache.ts) live in the main Cloudflare Worker package and aren't importable from a
 // separate npm workspace package — there's no in-repo precedent for a plain in-memory keyed-with-expiry
 // cache, so this is net new, kept deliberately small (lazy expiry check on read, no background sweep;
-// entries this service caches — GitHub issue metadata — are cheap to recompute on a stale miss).
+// entries this service caches — GitHub issue metadata — are cheap to recompute on a stale miss). A
+// constructor-supplied max-entry cap, enforced in `set`, bounds retention for keys that are never re-read.
 
 interface Entry<V> {
   value: V;
   expiresAt: number;
 }
 
+/** Default max-entry cap for a TtlCache with no explicit cap passed to its constructor. */
+export const DEFAULT_CACHE_MAX_ENTRIES = 5_000;
+
 export class TtlCache<V> {
   private readonly store = new Map<string, Entry<V>>();
 
-  constructor(private readonly now: () => number = Date.now) {}
+  constructor(
+    private readonly now: () => number = Date.now,
+    private readonly maxEntries: number = DEFAULT_CACHE_MAX_ENTRIES,
+  ) {}
 
   /** Returns the cached value, or undefined if absent or expired (an expired entry is evicted on read). */
   get(key: string): V | undefined {
@@ -26,7 +33,24 @@ export class TtlCache<V> {
     return entry.value;
   }
 
+  /** Bounds the store to `maxEntries` before inserting a NEW key: first drops every already-expired entry,
+   *  then (if still at cap) evicts the oldest-inserted entries -- the Map's own insertion order -- until
+   *  there's room. Overwriting an already-present key never grows the store, so it skips this entirely. */
+  private evictForCapacity(key: string): void {
+    if (this.store.has(key) || this.store.size < this.maxEntries) return;
+    const now = this.now();
+    for (const [storedKey, entry] of this.store) {
+      if (entry.expiresAt <= now) this.store.delete(storedKey);
+    }
+    while (this.store.size >= this.maxEntries) {
+      const oldestKey = this.store.keys().next().value;
+      if (oldestKey === undefined) break;
+      this.store.delete(oldestKey);
+    }
+  }
+
   set(key: string, value: V, ttlMs: number): void {
+    this.evictForCapacity(key);
     this.store.set(key, { value, expiresAt: this.now() + Math.max(0, ttlMs) });
   }
 

packages/discovery-index/src/server.ts

@@ -7,7 +7,7 @@
 import { serve } from "@hono/node-server";
 import type { AiPolicyVerdict, DiscoveryIndexCandidate } from "@loopover/engine";
 import { createApp } from "./app.js";
-import { TtlCache } from "./cache.js";
+import { DEFAULT_CACHE_MAX_ENTRIES, TtlCache } from "./cache.js";
 import { DEFAULT_CACHE_TTL_MS } from "./discovery-query.js";
 import { GitHubClient } from "./github-client.js";
 import { captureUnhandledPostHogError, flushDiscoveryIndexPostHog, initDiscoveryIndexPostHog, resolvePostHogEnvironment, shutdownDiscoveryIndexPostHog } from "./posthog.js";
@@ -27,10 +27,10 @@ const softClaimTtlMs =
 
 const app = createApp({
   github: new GitHubClient({ token: githubToken }),
-  resultCache: new TtlCache<DiscoveryIndexCandidate[]>(),
-  policyCache: new TtlCache<AiPolicyVerdict>(),
+  resultCache: new TtlCache<DiscoveryIndexCandidate[]>(Date.now, DEFAULT_CACHE_MAX_ENTRIES),
+  policyCache: new TtlCache<AiPolicyVerdict>(Date.now, DEFAULT_CACHE_MAX_ENTRIES),
   cacheTtlMs,
-  softClaimStore: new SoftClaimStore(new TtlCache(), softClaimTtlMs),
+  softClaimStore: new SoftClaimStore(new TtlCache(Date.now, DEFAULT_CACHE_MAX_ENTRIES), softClaimTtlMs),
   githubConfigured: githubToken.trim().length > 0,
 });
 

test/unit/discovery-index/cache.test.ts

@@ -1,5 +1,8 @@
+import { readFileSync } from "node:fs";
 import { describe, expect, it } from "vitest";
-import { TtlCache } from "../../../packages/discovery-index/src/cache";
+import { DEFAULT_CACHE_MAX_ENTRIES, TtlCache } from "../../../packages/discovery-index/src/cache";
+
+const SERVER_SOURCE = readFileSync("packages/discovery-index/src/server.ts", "utf8");
 
 function clock(startMs = 0) {
   let now = startMs;
@@ -65,4 +68,80 @@ describe("discovery-index TtlCache (#7164)", () => {
     expect(await cache.getOrCompute("k", 100, compute)).toBe(2);
     expect(calls).toBe(2);
   });
+
+  describe("max-entry cap", () => {
+    it("exports a positive default cap constant", () => {
+      expect(Number.isInteger(DEFAULT_CACHE_MAX_ENTRIES)).toBe(true);
+      expect(DEFAULT_CACHE_MAX_ENTRIES).toBeGreaterThan(0);
+    });
+
+    it("with a cap of 2, a third distinct key evicts the oldest and stays at size 2", () => {
+      const cache = new TtlCache<string>(Date.now, 2);
+      cache.set("a", "1", 60_000);
+      cache.set("b", "2", 60_000);
+      cache.set("c", "3", 60_000);
+      expect(cache.size).toBe(2);
+      expect(cache.get("a")).toBeUndefined();
+      expect(cache.get("c")).toBe("3");
+    });
+
+    it("under the cap, set does not evict anything", () => {
+      const cache = new TtlCache<string>(Date.now, 2);
+      cache.set("a", "1", 60_000);
+      expect(cache.size).toBe(1);
+      expect(cache.get("a")).toBe("1");
+    });
+
+    it("evicts already-expired entries before falling back to oldest-inserted eviction", () => {
+      const c = clock();
+      const cache = new TtlCache<string>(c.now, 2);
+      cache.set("a", "1", 100); // will be expired
+      c.advance(101);
+      cache.set("b", "2", 60_000); // live
+      cache.set("c", "3", 60_000); // expired-drop of "a" makes room, "b" survives
+      expect(cache.size).toBe(2);
+      expect(cache.get("a")).toBeUndefined();
+      expect(cache.get("b")).toBe("2");
+      expect(cache.get("c")).toBe("3");
+    });
+
+    it("REGRESSION: a key that is never re-read must not survive past the entry cap", () => {
+      const cap = 10;
+      const cache = new TtlCache<number>(Date.now, cap);
+      for (let i = 0; i < cap + 50; i++) {
+        cache.set(`key-${i}`, i, 60_000);
+        expect(cache.size).toBeLessThanOrEqual(cap);
+      }
+      expect(cache.size).toBe(cap);
+    });
+
+    it("overwriting an already-present key at the cap does not evict any other entry", () => {
+      const cache = new TtlCache<string>(Date.now, 2);
+      cache.set("a", "1", 60_000);
+      cache.set("b", "2", 60_000);
+      cache.set("b", "2-updated", 60_000);
+      expect(cache.size).toBe(2);
+      expect(cache.get("a")).toBe("1");
+      expect(cache.get("b")).toBe("2-updated");
+    });
+
+    it("falls back to the default cap when no cap is passed to the constructor", () => {
+      const cache = new TtlCache<number>();
+      for (let i = 0; i < DEFAULT_CACHE_MAX_ENTRIES + 5; i++) {
+        cache.set(`key-${i}`, i, 60_000);
+      }
+      expect(cache.size).toBe(DEFAULT_CACHE_MAX_ENTRIES);
+    });
+  });
+
+  describe("server.ts wiring (#7164)", () => {
+    it("passes an explicit cap to all three long-lived cache instances", () => {
+      const explicitCapSites = [...SERVER_SOURCE.matchAll(/new TtlCache(?:<[^>]*>)?\([^)]*DEFAULT_CACHE_MAX_ENTRIES[^)]*\)/g)];
+      expect(explicitCapSites).toHaveLength(3);
+    });
+
+    it("imports the cap constant from cache.ts", () => {
+      expect(SERVER_SOURCE).toMatch(/import\s*\{[^}]*DEFAULT_CACHE_MAX_ENTRIES[^}]*\}\s*from\s*"\.\/cache\.js"/);
+    });
+  });
 });