diff --git a/packages/discovery-index/src/cache.ts b/packages/discovery-index/src/cache.ts index 1bdd456442..af63fe752e 100644 --- a/packages/discovery-index/src/cache.ts +++ b/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 { 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 { private readonly store = new Map>(); - 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 { 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) }); } diff --git a/packages/discovery-index/src/server.ts b/packages/discovery-index/src/server.ts index cea8ebb671..7f1b459ab7 100644 --- a/packages/discovery-index/src/server.ts +++ b/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(), - policyCache: new TtlCache(), + resultCache: new TtlCache(Date.now, DEFAULT_CACHE_MAX_ENTRIES), + policyCache: new TtlCache(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, }); diff --git a/test/unit/discovery-index/cache.test.ts b/test/unit/discovery-index/cache.test.ts index 764c748523..cbe9bebc82 100644 --- a/test/unit/discovery-index/cache.test.ts +++ b/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(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(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(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(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(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(); + 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"/); + }); + }); });