From 1b238ea786182ecb033e67b7854e0c691ca096c3 Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Fri, 31 Jul 2026 09:00:20 +0000 Subject: [PATCH] fix(discovery-index): bound TtlCache by a max-entry cap enforced on set --- packages/discovery-index/src/cache.ts | 28 ++++++++- packages/discovery-index/src/server.ts | 8 +-- test/unit/discovery-index/cache.test.ts | 82 ++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 6 deletions(-) diff --git a/packages/discovery-index/src/cache.ts b/packages/discovery-index/src/cache.ts index 1bdd456442..53ff737d01 100644 --- a/packages/discovery-index/src/cache.ts +++ b/packages/discovery-index/src/cache.ts @@ -10,10 +10,16 @@ interface Entry { expiresAt: number; } +/** Default max-entry cap for a `TtlCache` constructed without an explicit one. */ +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,10 +32,30 @@ export class TtlCache { return entry.value; } + /** A never-before-seen key first enforces the entry cap; an existing key is overwritten in place + * without evicting anything (it isn't growing the store). */ set(key: string, value: V, ttlMs: number): void { + if (!this.store.has(key)) { + this.evictForCapacity(); + } this.store.set(key, { value, expiresAt: this.now() + Math.max(0, ttlMs) }); } + /** Drops every already-expired entry first, then evicts the oldest-inserted entry (the order a `Map` + * already preserves) until the store is back under the cap, making room for the key `set` is about to add. */ + private evictForCapacity(): void { + if (this.store.size < this.maxEntries) return; + const now = this.now(); + for (const [key, entry] of this.store) { + if (entry.expiresAt <= now) this.store.delete(key); + } + while (this.store.size >= this.maxEntries) { + const oldestKey = this.store.keys().next().value; + if (oldestKey === undefined) break; + this.store.delete(oldestKey); + } + } + delete(key: string): void { this.store.delete(key); } 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..3da8a204dc 100644 --- a/test/unit/discovery-index/cache.test.ts +++ b/test/unit/discovery-index/cache.test.ts @@ -1,5 +1,6 @@ +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"; function clock(startMs = 0) { let now = startMs; @@ -65,4 +66,83 @@ describe("discovery-index TtlCache (#7164)", () => { expect(await cache.getOrCompute("k", 100, compute)).toBe(2); expect(calls).toBe(2); }); + + describe("max-entry cap", () => { + it("evicts the oldest-inserted entry once the cap is exceeded", () => { + const cache = new TtlCache(Date.now, 2); + cache.set("a", "1", 1000); + cache.set("b", "2", 1000); + cache.set("c", "3", 1000); + expect(cache.size).toBe(2); + expect(cache.get("a")).toBeUndefined(); + expect(cache.get("c")).toBe("3"); + }); + + it("drops an already-expired entry before falling back to oldest-inserted eviction", () => { + const c = clock(); + const cache = new TtlCache(c.now, 2); + cache.set("a", "1", 100); + c.advance(101); // "a" is now expired but not yet lazily evicted + cache.set("b", "2", 1000); + cache.set("c", "3", 1000); + expect(cache.size).toBe(2); + expect(cache.get("a")).toBeUndefined(); + expect(cache.get("b")).toBe("2"); + expect(cache.get("c")).toBe("3"); + }); + + it("overwriting an existing key never evicts, even at the cap", () => { + const cache = new TtlCache(Date.now, 2); + cache.set("a", "1", 1000); + cache.set("b", "2", 1000); + cache.set("a", "1-updated", 1000); + expect(cache.size).toBe(2); + expect(cache.get("a")).toBe("1-updated"); + expect(cache.get("b")).toBe("2"); + }); + + it("falls back to DEFAULT_CACHE_MAX_ENTRIES when no cap is given", () => { + const cache = new TtlCache(); + for (let i = 0; i < DEFAULT_CACHE_MAX_ENTRIES + 1; i += 1) { + cache.set(`k${i}`, String(i), 1000); + } + expect(cache.size).toBe(DEFAULT_CACHE_MAX_ENTRIES); + }); + + it("a cap of 0 never grows the store past a single entry and doesn't hang", () => { + const cache = new TtlCache(Date.now, 0); + cache.set("a", "1", 1000); + cache.set("b", "2", 1000); + expect(cache.size).toBe(1); + expect(cache.get("b")).toBe("2"); + }); + + it("REGRESSION: a key that is never re-read must not survive past the entry cap", () => { + const cap = 3; + const cache = new TtlCache(Date.now, cap); + for (let i = 0; i < cap + 50; i += 1) { + cache.set(`scope-${i}`, i, 1000); + expect(cache.size).toBeLessThanOrEqual(cap); + } + expect(cache.size).toBe(cap); + }); + }); + + describe("server.ts wiring (#10029)", () => { + // server.ts is a process entrypoint and isn't unit-imported (see its own header comment), so assert + // the constructor wiring by reading its source text rather than importing it. + const SERVER_SOURCE = readFileSync("packages/discovery-index/src/server.ts", "utf8"); + + it("passes DEFAULT_CACHE_MAX_ENTRIES to all three TtlCache construction sites", () => { + const ttlCacheConstructions = SERVER_SOURCE.match(/new TtlCache[^)]*\([^)]*\)/g) ?? []; + expect(ttlCacheConstructions.length).toBe(3); + for (const construction of ttlCacheConstructions) { + expect(construction).toContain("DEFAULT_CACHE_MAX_ENTRIES"); + } + }); + + it("imports DEFAULT_CACHE_MAX_ENTRIES from ./cache.js", () => { + expect(SERVER_SOURCE).toMatch(/import\s*\{[^}]*DEFAULT_CACHE_MAX_ENTRIES[^}]*\}\s*from\s*"\.\/cache\.js"/); + }); + }); });