diff --git a/package.json b/package.json index 7dcba6936f6..ed90b960922 100644 --- a/package.json +++ b/package.json @@ -26,15 +26,15 @@ "debug": "cross-env DEBUG=1 node --inspect-brk scripts/start.js", "generate": "node scripts/generate-git-commit-info.js", "generate:settings-schema": "node --import tsx/esm scripts/generate-settings-schema.ts", - "build": "node scripts/build.js", + "build": "cross-env NODE_OPTIONS=\"--max-old-space-size=3072\" node scripts/build.js", "build-and-start": "npm run build && npm run start", "build:vscode": "node scripts/build_vscode_companion.js", "build:all": "npm run build && npm run build:sandbox && npm run build:vscode", "build:packages": "npm run build --workspaces", "build:sandbox": "node scripts/build_sandbox.js", "bundle": "npm run generate && node esbuild.config.js && node scripts/copy_bundle_assets.js", - "test": "npm run test --workspaces --if-present --parallel", - "test:ci": "npm run test:ci --workspaces --if-present --parallel && npm run test:scripts", + "test": "cross-env NODE_OPTIONS=\"--max-old-space-size=3072\" npm run test --workspaces --if-present --parallel", + "test:ci": "cross-env NODE_OPTIONS=\"--max-old-space-size=3072\" npm run test:ci --workspaces --if-present --parallel && npm run test:scripts", "test:scripts": "vitest run --config ./scripts/tests/vitest.config.ts", "test:e2e": "cross-env VERBOSE=true KEEP_OUTPUT=true npm run test:integration:sandbox:none", "test:integration:all": "npm run test:integration:sandbox:none && npm run test:integration:sandbox:docker && npm run test:integration:sandbox:podman", diff --git a/packages/core/src/services/fileReadCache.test.ts b/packages/core/src/services/fileReadCache.test.ts index 491e4d8923a..42ef827e1c3 100644 --- a/packages/core/src/services/fileReadCache.test.ts +++ b/packages/core/src/services/fileReadCache.test.ts @@ -505,4 +505,67 @@ describe('FileReadCache', () => { expect(cache.check(fs.statSync(file)).state).toBe('stale'); }); }); + + describe('eviction', () => { + it('evicts the oldest entry when the cache exceeds MAX_ENTRIES', () => { + // Fill cache to capacity (MAX_ENTRIES = 4096). + const cache = new FileReadCache(); + for (let i = 0; i < 4096; i++) { + cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), { + full: true, + cacheable: true, + }); + } + expect(cache.size()).toBe(4096); + + // The 4097th write triggers eviction of the oldest (ino=0). + cache.recordWrite('/x/file-new.ts', makeStats({ ino: 4096 })); + expect(cache.size()).toBeLessThanOrEqual(4096); + expect(cache.check(makeStats({ ino: 0 })).state).toBe('unknown'); + expect(cache.check(makeStats({ ino: 4096 })).state).toBe('fresh'); + }); + + it('keeps size at MAX_ENTRIES after multiple overflows', () => { + const cache = new FileReadCache(); + // Add MAX_ENTRIES + 100 distinct inodes. + for (let i = 0; i < 4196; i++) { + cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), { + full: true, + cacheable: true, + }); + } + expect(cache.size()).toBeLessThanOrEqual(4096); + }); + + it('should have bumped entries survive eviction', () => { + const cache = new FileReadCache(); + // Fill to capacity. + for (let i = 0; i < 4096; i++) { + cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), { + full: true, + cacheable: true, + }); + } + + // Frequently update ino=0 — after bump lands this moves it to the + // back of the eviction queue. + for (let i = 0; i < 10; i++) { + cache.recordRead('/x/file-0.ts', makeStats({ ino: 0 }), { + full: true, + cacheable: true, + }); + } + + // Add 50 new entries — they push the *least* recently bumped out. + for (let i = 4096; i < 4146; i++) { + cache.recordRead(`/x/file-${i}.ts`, makeStats({ ino: i }), { + full: true, + cacheable: true, + }); + } + + expect(cache.size()).toBeLessThanOrEqual(4096); + expect(cache.check(makeStats({ ino: 0 })).state).not.toBe('unknown'); + }); + }); }); diff --git a/packages/core/src/services/fileReadCache.ts b/packages/core/src/services/fileReadCache.ts index 4bac8fb19c5..b3f44adbdb3 100644 --- a/packages/core/src/services/fileReadCache.ts +++ b/packages/core/src/services/fileReadCache.ts @@ -115,6 +115,7 @@ export type FileReadCheckResult = export class FileReadCache { private readonly byInode = new Map(); + private static readonly MAX_ENTRIES = 4096; /** Build the canonical key for a file from its Stats. */ static inodeKey(stats: Stats): string { @@ -261,11 +262,22 @@ export class FileReadCache { const key = FileReadCache.inodeKey(stats); const existing = this.byInode.get(key); if (existing) { + // Bump: move existing entry to the end of the FIFO queue so that + // frequently-updated entries survive eviction. + this.byInode.delete(key); existing.realPath = absPath; existing.mtimeMs = stats.mtimeMs; existing.sizeBytes = stats.size; + this.byInode.set(key, existing); return existing; } + // Evict oldest entry when cache exceeds MAX_ENTRIES (FIFO) + if (this.byInode.size >= FileReadCache.MAX_ENTRIES) { + const oldestKey = this.byInode.keys().next().value; + if (oldestKey) { + this.byInode.delete(oldestKey); + } + } const entry: FileReadEntry = { inodeKey: key, realPath: absPath, diff --git a/packages/core/src/utils/filesearch/crawlCache.test.ts b/packages/core/src/utils/filesearch/crawlCache.test.ts index 80809dab16f..f6c840b7f66 100644 --- a/packages/core/src/utils/filesearch/crawlCache.test.ts +++ b/packages/core/src/utils/filesearch/crawlCache.test.ts @@ -5,7 +5,13 @@ */ import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; -import { getCacheKey, read, write, clear } from './crawlCache.js'; +import { + getCacheKey, + read, + write, + clear, + MAX_CACHE_ENTRIES, +} from './crawlCache.js'; describe('CrawlCache', () => { describe('getCacheKey', () => { @@ -125,5 +131,86 @@ describe('CrawlCache', () => { await vi.advanceTimersByTimeAsync(2001); expect(read(key)).toBeUndefined(); }); + + it('should enforce MAX_TOTAL_PATHS when updating an existing key with a large array', () => { + // Helper to create an array of given length + const makePaths = (n: number) => + Array.from({ length: n }, (_, i) => `path/${i}`); + + // Write key_A with a moderate number of paths + const keyA = 'project-a'; + const keyB = 'project-b'; + const keyC = 'project-c'; + + write(keyA, makePaths(1000), 60000); + write(keyB, makePaths(20000), 60000); + write(keyC, makePaths(20000), 60000); + + // Now update key_A with 60000 paths — this alone exceeds MAX_TOTAL_PATHS (50000) + // Before the fix, !crawlCache.has(keyA) was false, so eviction was skipped. + // After the fix, keyB and keyC should be evicted to make room. + write(keyA, makePaths(60000), 60000); + + // Verify keyA's data is stored + expect(read(keyA)).toBeDefined(); + expect(read(keyA)!.length).toBe(60000); + + // After eviction, keyB and keyC must be gone because keyA with 60000 paths + // exceeds the MAX_TOTAL_PATHS limit (50000), triggering eviction of others. + expect(read(keyB)).toBeUndefined(); + expect(read(keyC)).toBeUndefined(); + }); + + it('should bump existing key to end of FIFO queue on update', () => { + // When MAX_CACHE_ENTRIES is reached, the oldest (first inserted) entry is evicted. + // Updating an existing key should move it to the end, preventing its eviction. + + // Fill cache to capacity with distinct keys + for (let i = 0; i < MAX_CACHE_ENTRIES; i++) { + write(`key-${i}`, [`path`], 60000); + } + // key-0 is the oldest entry + expect(read('key-0')).toBeDefined(); + + // Update key-0 — it should bump to the end of the queue + write('key-0', [`updated-path`], 60000); + + // Now insert one more key, which should trigger eviction of the OLDEST entry. + // After bump, key-1 is the oldest, not key-0. + write(`key-new`, [`new-path`], 60000); + + // key-0 should survive because it was bumped to the end + expect(read('key-0')).toBeDefined(); + expect(read('key-0')![0]).toBe('updated-path'); + + // key-1 should be evicted (it became the oldest after key-0 was bumped) + expect(read('key-1')).toBeUndefined(); + }); + + it('should evict other entries when a new key exceeds MAX_TOTAL_PATHS', () => { + // Exact scenario from reviewer comment #2: a new key whose array + // alone exceeds MAX_TOTAL_PATHS should trigger eviction of others. + const makePaths = (n: number) => + Array.from({ length: n }, (_, i) => `path/${i}`); + + const keyA = 'project-a'; + const keyB = 'project-b'; + const keyC = 'project-c'; + + // Pre-populate with moderate entries + write(keyA, makePaths(10000), 60000); + write(keyB, makePaths(10000), 60000); + write(keyC, makePaths(10000), 60000); + + // New key with 60000 paths — exceeds MAX_TOTAL_PATHS (50000) alone + write('project-new', makePaths(60000), 60000); + + // New key should be stored, others evicted to make room + expect(read('project-new')).toBeDefined(); + expect(read('project-new')!.length).toBe(60000); + expect(read(keyA)).toBeUndefined(); + expect(read(keyB)).toBeUndefined(); + expect(read(keyC)).toBeUndefined(); + }); }); }); diff --git a/packages/core/src/utils/filesearch/crawlCache.ts b/packages/core/src/utils/filesearch/crawlCache.ts index 6d159481345..497b05c266d 100644 --- a/packages/core/src/utils/filesearch/crawlCache.ts +++ b/packages/core/src/utils/filesearch/crawlCache.ts @@ -9,6 +9,10 @@ import crypto from 'node:crypto'; const crawlCache = new Map(); const cacheTimers = new Map(); +// Limits to prevent heap exhaustion when many projects are crawled concurrently +export const MAX_CACHE_ENTRIES = 256; // max distinct project roots cached +export const MAX_TOTAL_PATHS = 50_000; // max total paths across all entries + /** * Generates a unique cache key based on the project directory and the content * of ignore files. This ensures that the cache is invalidated if the project @@ -36,12 +40,23 @@ export const getCacheKey = ( /** * Reads cached data from the in-memory cache. + * Bumps the entry to the end of the FIFO queue on hit so that + * frequently-read crawl results survive eviction by auxiliary crawls. * Returns undefined if the key is not found. */ -export const read = (key: string): string[] | undefined => crawlCache.get(key); +export const read = (key: string): string[] | undefined => { + const result = crawlCache.get(key); + if (result !== undefined) { + crawlCache.delete(key); + crawlCache.set(key, result); + } + return result; +}; /** * Writes data to the in-memory cache and sets a timer to evict it after the TTL. + * Enforces MAX_CACHE_ENTRIES (LRU by insertion order) and MAX_TOTAL_PATHS to + * prevent heap exhaustion when many large projects are crawled. */ export const write = (key: string, results: string[], ttlMs: number): void => { // Clear any existing timer for this key to prevent premature deletion @@ -49,6 +64,55 @@ export const write = (key: string, results: string[], ttlMs: number): void => { clearTimeout(cacheTimers.get(key)!); } + // Evict oldest entries when cache exceeds entry limit (FIFO / insertion-order). + // Guard: updating an existing key doesn't increase entry count, so skip eviction. + while (crawlCache.size >= MAX_CACHE_ENTRIES && !crawlCache.has(key)) { + const oldestKey = crawlCache.keys().next().value; + if (oldestKey) { + crawlCache.delete(oldestKey); + if (cacheTimers.has(oldestKey)) { + clearTimeout(cacheTimers.get(oldestKey)!); + cacheTimers.delete(oldestKey); + } + } + } + + // Evict largest entries when total path count exceeds limit. + // Calculate totalPaths excluding the key being updated to avoid counting old value. + let totalPaths = 0; + for (const [k, entry] of crawlCache) { + if (k !== key) { + totalPaths += entry.length; + } + } + while (totalPaths + results.length > MAX_TOTAL_PATHS && crawlCache.size > 0) { + // Find and remove the entry with the most paths (never evict the current key) + let largestKey: string | undefined; + let largestSize = 0; + for (const [k, v] of crawlCache) { + if (k === key) continue; + if (v.length > largestSize) { + largestSize = v.length; + largestKey = k; + } + } + if (largestKey) { + totalPaths -= crawlCache.get(largestKey)!.length; + crawlCache.delete(largestKey); + if (cacheTimers.has(largestKey)) { + clearTimeout(cacheTimers.get(largestKey)!); + cacheTimers.delete(largestKey); + } + } else { + break; + } + } + + // Bump existing key to end of FIFO queue (mirror fileReadCache.upsert behavior) + if (crawlCache.has(key)) { + crawlCache.delete(key); + } + // Store the new data crawlCache.set(key, results);