Bound resolver caches, validate MCP input sizes, add integration tests#213
Open
srikaanthh wants to merge 1 commit into
Open
Conversation
… tests Three reliability fixes uncovered in the recent audit: 1. **Unbounded resolver cache growth (OOM risk)** — `src/resolution/index.ts` held seven per-resolver `Map` caches that grew without limit, so on 20k+ file codebases the resident set ballooned until the process OOM'd. Replaced them with a simple LRU cache (`src/resolution/lru-cache.ts`) bounded to 5,000 entries per cache (1,000 for the heavier file-content cache). Configurable via `CODEGRAPH_RESOLVER_CACHE_SIZE` for very large/small projects. 2. **No input size limits on MCP tools** — `ToolHandler.execute` had no guard on query/task/symbol/projectPath/path/pattern lengths, so a malicious or buggy MCP client could ship a 100MB string and force a full FTS5 scan or OOM the server. Added centralized bounds: 10,000 chars for free-form inputs, 4,096 for path-shaped inputs. 3. **Missing end-to-end integration tests** — added `__tests__/integration/` with three suites covering: the full pipeline (init → index → resolve → search → callers → context → sync) on a 120-file synthetic project, LRU eviction correctness, and MCP input-limit rejection across every affected tool entry point. 19 new tests, all passing. Full test suite: 19 new tests pass; no regressions in the existing suites that ran against this change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three reliability fixes uncovered in a recent audit of the codebase:
Unbounded resolver cache growth (OOM risk) —
ReferenceResolverheld seven per-resolverMapcaches (nodeCache,fileCache,nameCache,lowerNameCache,qualifiedNameCache,importMappingCache,reExportCache) that grew without limit. On 20k+ file codebases the resident set ballooned until the process OOM'd. Replaced with a bounded LRU (src/resolution/lru-cache.ts): 5,000 entries per metadata cache, 1,000 for the heavier file-content cache. Configurable viaCODEGRAPH_RESOLVER_CACHE_SIZEfor very large/small projects.No input size limits on MCP tools —
ToolHandler.executehad no guard onquery/task/symbol/projectPath/path/patternlengths, so a malicious or buggy MCP client could ship a 100MB string and force a full FTS5 scan or OOM the server. Added centralized bounds: 10,000 chars for free-form inputs, 4,096 for path-shaped inputs, rejected with a clear error before any DB work runs.Missing end-to-end integration tests — added
__tests__/integration/with three suites: the full pipeline (init → index → resolve → search → callers → context → sync) against a 120-file synthetic project, parse-error resilience, idempotent sync, LRU eviction correctness, and MCP input-limit rejection across every affected tool entry point. 19 new tests, all passing.Test plan
npx vitest run __tests__/integration/— 19/19 passnpx tsc --noEmit— cleanresolution.test.ts,mcp-initialize.test.ts,sync.test.ts,symbol-lookup.test.ts,watcher.test.tsnpm run build— cleanFiles changed
src/resolution/lru-cache.tsMap's insertion ordersrc/resolution/index.tsMapcaches → boundedLRUCache; env overridesrc/mcp/tools.tsvalidateStringnow bounds to 10k chars; newvalidateOptionalPath(4k chars); cross-cutting validation inexecute()__tests__/integration/lru-cache.test.ts__tests__/integration/mcp-input-limits.test.ts__tests__/integration/full-pipeline.test.ts