From cf66c2148f870a9892f07718197a1f670abbe4fd Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Tue, 22 Sep 2026 18:12:36 +0300 Subject: [PATCH] fix(extraction): a file over the size limit is never read (#1910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The size gate ran after `readFile`, so a committed video or blob fixture was decoded in full — 3.4 GB of RSS for a 400 MB file, `Invalid string length` past ~512 MB — only to be stored as skipped. It was then read again by every pass that scans files by name: the framework detectors' `readFile` and the resolver's cached reader, three full passes on the reporter's fixtures. Stat first, everywhere a source file is read by path: the batch reader and `extractFile` store an oversize file with a size stamp in place of its content, change detection hashes the same stamp (so a same-size rewrite of a file nothing is indexed from is not a change, while crossing the limit in either direction is), and both by-name readers return null above the limit. The limit moves to `src/file-limits.ts` so the three sites share one number. strace on a sparse 400 MB `.ts`: 153 603 reads before, 0 after; indexing it next to a real file costs no measurable RSS. Stacked on #1914, which shares the batch-reader hunk. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + __tests__/oversize-file-not-read.test.ts | 79 ++++++++++++++++++++++++ src/extraction/index.ts | 57 ++++++++++++----- src/file-limits.ts | 6 ++ src/resolution/index.ts | 9 +++ 5 files changed, 138 insertions(+), 14 deletions(-) create mode 100644 __tests__/oversize-file-not-read.test.ts create mode 100644 src/file-limits.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f2f2965f..82e7e4c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -169,6 +169,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Trails are plain JSON, one file per trail, under `.codegraph/ui/trails/` — already ignored by git, so they stay yours by default. **Export** hands you the file if you'd rather commit one for the team. This is the only thing the viewer writes: it still never indexes, never changes your graph, and never touches a line of your code. Start it with `codegraph ui --read-only` and it won't write even that — saved trails can still be opened, just not saved or deleted. ### Fixes +- A file over the size limit is no longer read before it is skipped: committed video and blob fixtures used to be decoded in full — a 400 MB fixture cost 3.4 GB of memory — only to be discarded, and the same file was read again by every resolution pass. Its size stamp now stands in for its content, during indexing and when checking for changes. (#1910) - Extension installation and interrupted-operation recovery now honor `CODEGRAPH_DIR` without creating or modifying the default data directory. diff --git a/__tests__/oversize-file-not-read.test.ts b/__tests__/oversize-file-not-read.test.ts new file mode 100644 index 000000000..246e2b445 --- /dev/null +++ b/__tests__/oversize-file-not-read.test.ts @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { oversizeStamp, hashContent } from '../src/extraction'; + +/** + * A file over the size limit is stored as skipped without ever being read + * (#1910): committed video/blob fixtures used to be decoded in full — and + * hashed — only to be discarded, costing their size in RSS per file. + */ +describe('oversize files are stat-gated, never read (#1910)', () => { + let dir: string; + afterEach(() => { if (dir) fs.rmSync(dir, { recursive: true, force: true }); }); + + const big = (bytes: number, fill = 0x41) => Buffer.alloc(bytes, fill); + + it('indexes the neighbours, records the oversize file as skipped with a size-stamp hash, and does not decode it', async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-oversize-')); + fs.writeFileSync(path.join(dir, 'app.ts'), 'export function alpha() { return beta(); }\nexport function beta() { return 1; }\n'); + // 1 MB + 1: over the limit; invalid UTF-8 (0xFF) so any decode would be visible as replacement chars. + fs.writeFileSync(path.join(dir, 'blob.ts'), big(1024 * 1024 + 1, 0xff)); + const cg = await CodeGraph.init(dir, { index: true }); + try { + expect(cg.getNodesByKind('function').map(n => n.name).sort()).toEqual(['alpha', 'beta']); + const rec = cg.getFiles().find(f => f.path === 'blob.ts'); + expect(rec).toBeDefined(); + expect(rec!.contentHash).toBe(hashContent(oversizeStamp(1024 * 1024 + 1))); + // Nothing from the blob reached the graph. + expect(cg.getNodesInFile('blob.ts').filter(n => n.kind !== 'file')).toEqual([]); + // Change detection agrees with what was stored: nothing pending. + expect(cg.getChangedFiles()).toEqual({ added: [], modified: [], removed: [] }); + // A same-size rewrite is not a change (nothing about it is indexed)... + fs.writeFileSync(path.join(dir, 'blob.ts'), big(1024 * 1024 + 1, 0xfe)); + expect(cg.getChangedFiles().modified).toEqual([]); + // ...crossing the limit is: the file becomes ordinary source. + fs.writeFileSync(path.join(dir, 'blob.ts'), 'export const gamma = 3;\n'); + expect(cg.getChangedFiles().modified).toEqual(['blob.ts']); + await cg.sync(); + expect(cg.getNodesInFile('blob.ts').some(n => n.name === 'gamma')).toBe(true); + // ...and growing back over it is a change too, stored as the stamp again. + fs.writeFileSync(path.join(dir, 'blob.ts'), big(2 * 1024 * 1024, 0xff)); + expect(cg.getChangedFiles().modified).toEqual(['blob.ts']); + await cg.sync(); + expect(cg.getFiles().find(f => f.path === 'blob.ts')!.contentHash).toBe(hashContent(oversizeStamp(2 * 1024 * 1024))); + expect(cg.getNodesInFile('blob.ts').some(n => n.name === 'gamma')).toBe(false); + } finally { + cg.close(); + } + }); + + it('a 400 MB sparse fixture indexes in well under a second and without growing the heap by its size', async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-oversize-big-')); + fs.writeFileSync(path.join(dir, 'ok.ts'), 'export const one = 1;\n'); + // Sparse: occupies no disk, but stat() reports 400 MB — a read would decode all of it. + const fd = fs.openSync(path.join(dir, 'huge.ts'), 'w'); + fs.ftruncateSync(fd, 400 * 1024 * 1024); + fs.closeSync(fd); + // Warm the engine on a sibling project first, so the grammar and worker + // start-up cost is not mistaken for the file being read. + const warm = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-oversize-warm-')); + fs.writeFileSync(path.join(warm, 'w.ts'), 'export const w = 1;\n'); + (await CodeGraph.init(warm, { index: true })).close(); + fs.rmSync(warm, { recursive: true, force: true }); + const before = process.memoryUsage().rss; + const t0 = Date.now(); + const cg = await CodeGraph.init(dir, { index: true }); + try { + expect(Date.now() - t0).toBeLessThan(5000); + // Reading 400 MB would show as at least that much RSS; the stamp shows as none. + expect(process.memoryUsage().rss - before).toBeLessThan(100 * 1024 * 1024); + expect(cg.getFiles().map(f => f.path).sort()).toEqual(['huge.ts', 'ok.ts']); + expect(cg.getChangedFiles()).toEqual({ added: [], modified: [], removed: [] }); + } finally { + cg.close(); + } + }); +}); diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 57cd6378c..f59014500 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -27,6 +27,7 @@ import { ParseWorkerPool, resolveParsePoolSize, resolveParseTimeoutMs } from './ import { StoreWriter, StoreBundle, finalizeStoreBundle } from './store-writer'; import { materializeKernelResult } from './kernel'; import { detectGeneratedFile } from './generated-detection'; +import { MAX_SOURCE_FILE_SIZE_BYTES } from '../file-limits'; import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLanguage, initGrammars, loadGrammarsForLanguages, readGrammarWasmBytes, isMpegTransportStream, hasMpegTsExtension, MPEG_TS_SNIFF_BYTES } from './grammars'; import { loadExtensionOverrides, loadIncludeIgnoredPatterns, loadExcludePatterns, loadIncludePatterns, PROJECT_CONFIG_FILENAME } from '../project-config'; import { isCodeGraphDataDir } from '../directory'; @@ -153,11 +154,28 @@ export function hashContent(content: string): string { } /** - * Skip files larger than this (bytes). Generated bundles, minified JS, and - * vendored blobs blow the WASM heap and the worker-recycle budget for no useful - * symbols. 1 MB covers essentially all hand-written source. + * What stands in for the content of a file over MAX_SOURCE_FILE_SIZE_BYTES. Such a file is + * never parsed, so its bytes are never needed — reading them only to hash and + * discard cost multi-GB RSS spikes on committed video/blob fixtures and could + * fail outright with `Invalid string length` (#1910). The stamp is a function + * of size alone: change detection compares it to the stored hash, so a + * same-size rewrite of an oversize file is not a change (nothing about it is + * indexed), while crossing the limit in either direction is. */ -const MAX_FILE_SIZE = 1024 * 1024; +export function oversizeStamp(size: number): string { + return `codegraph:oversize:${size}`; +} + +/** + * Read a file for hashing/indexing: the whole text when it is under the size + * limit, the size stamp when it is over — the caller never decodes an oversize + * file. `stats` is what the caller already has; without it the file is stat'ed. + */ +function readSourceOrStamp(fullPath: string, stats?: fs.Stats): { content: string; stats: fs.Stats } { + const st = stats ?? fs.statSync(fullPath); + if (st.size > MAX_SOURCE_FILE_SIZE_BYTES) return { content: oversizeStamp(st.size), stats: st }; + return { content: fs.readFileSync(fullPath, 'utf-8'), stats: st }; +} /** * Directory names that are dependency, build, cache, or tooling output across the @@ -1870,6 +1888,9 @@ export class ExtractionOrchestrator { const full = validatePathWithinRoot(rootDir, relativePath); if (!full) return null; try { + // Framework detectors scan source by name; a file over the size + // limit was never indexed and must not be decoded here either (#1910). + if (fs.statSync(full).size > MAX_SOURCE_FILE_SIZE_BYTES) return null; return fs.readFileSync(full, 'utf-8'); } catch { return null; @@ -2304,13 +2325,19 @@ export class ExtractionOrchestrator { // stream (#1910) is recognised from its head here, at no extra I/O, // and never decoded or parsed. The scan already drops these; this // guards the paths that hand files in by name (sync, watcher). + // Stat first: a file over the size limit is stored as skipped + // without ever being read or decoded (#1910), so ten oversize + // fixtures in one I/O batch no longer cost their size in RSS. + const stats = await fsp.stat(fullPath); + if (stats.size > MAX_SOURCE_FILE_SIZE_BYTES) { + return { filePath: fp, content: oversizeStamp(stats.size), stats, error: null as Error | null }; + } const bytes = await fsp.readFile(fullPath); if (hasMpegTsExtension(fp) && isMpegTransportStream(bytes.subarray(0, MPEG_TS_SNIFF_BYTES))) { logDebug('Skipping MPEG transport stream named .ts — not TypeScript', { filePath: fp }); return { filePath: fp, content: null as string | null, stats: null as fs.Stats | null, error: null as Error | null, skipped: true }; } const content = bytes.toString('utf-8'); - const stats = await fsp.stat(fullPath); return { filePath: fp, content, stats, error: null as Error | null }; } catch (err) { return { filePath: fp, content: null as string | null, stats: null as fs.Stats | null, error: err as Error }; @@ -2343,18 +2370,18 @@ export class ExtractionOrchestrator { continue; } - // Honour MAX_FILE_SIZE. Without this check, vendored generated + // Honour MAX_SOURCE_FILE_SIZE_BYTES. Without this check, vendored generated // headers, minified bundles, and other multi-MB files get indexed, // wasting WASM heap and the worker recycle budget on inputs with no // useful symbols. The single-file extractFile path already enforces // this; the bulk path used to silently skip the check. - if (stats.size > MAX_FILE_SIZE) { + if (stats.size > MAX_SOURCE_FILE_SIZE_BYTES) { await storeResult(filePath, content, stats, { nodes: [], edges: [], unresolvedReferences: [], errors: [{ - message: `File exceeds max size (${stats.size} > ${MAX_FILE_SIZE})`, + message: `File exceeds max size (${stats.size} > ${MAX_SOURCE_FILE_SIZE_BYTES})`, filePath, severity: 'warning', code: 'size_exceeded', @@ -2638,7 +2665,8 @@ export class ExtractionOrchestrator { let stats: fs.Stats; try { stats = await fsp.stat(fullPath); - content = await fsp.readFile(fullPath, 'utf-8'); + // An oversize file is stored as skipped; its bytes are never needed (#1910). + content = stats.size > MAX_SOURCE_FILE_SIZE_BYTES ? oversizeStamp(stats.size) : await fsp.readFile(fullPath, 'utf-8'); } catch (error) { return { nodes: [], @@ -2690,14 +2718,14 @@ export class ExtractionOrchestrator { const language = detectLanguage(relativePath, content, loadExtensionOverrides(this.rootDir)); // Check file size - if (stats.size > MAX_FILE_SIZE) { + if (stats.size > MAX_SOURCE_FILE_SIZE_BYTES) { const result: ExtractionResult = { nodes: [], edges: [], unresolvedReferences: [], errors: [ { - message: `File exceeds max size (${stats.size} > ${MAX_FILE_SIZE})`, + message: `File exceeds max size (${stats.size} > ${MAX_SOURCE_FILE_SIZE_BYTES})`, filePath: relativePath, severity: 'warning', code: 'size_exceeded', @@ -3256,9 +3284,10 @@ export class ExtractionOrchestrator { } // New, or size/mtime changed — read + hash to confirm a real content change. + // (An oversize file hashes as its size stamp, unread — #1910.) let content: string; try { - content = fs.readFileSync(fullPath, 'utf-8'); + content = readSourceOrStamp(fullPath).content; } catch (error) { logDebug('Skipping unreadable file during sync', { filePath, error: String(error) }); failedFilePaths.push(filePath); @@ -3423,7 +3452,7 @@ export class ExtractionOrchestrator { continue; } let content: string; - try { content = fs.readFileSync(fullPath, 'utf-8'); } + try { content = readSourceOrStamp(fullPath).content; } catch (error) { logDebug('Skipping unreadable file while detecting changes', { filePath, error: String(error) }); continue; @@ -3461,7 +3490,7 @@ export class ExtractionOrchestrator { const fullPath = path.join(this.rootDir, filePath); let content: string; try { - content = fs.readFileSync(fullPath, 'utf-8'); + content = readSourceOrStamp(fullPath).content; } catch (error) { logDebug('Skipping unreadable file while detecting changes', { filePath, error: String(error) }); continue; diff --git a/src/file-limits.ts b/src/file-limits.ts new file mode 100644 index 000000000..4bb4d6da1 --- /dev/null +++ b/src/file-limits.ts @@ -0,0 +1,6 @@ +/** + * Largest source file CodeGraph will parse or read during resolution. Generated + * bundles, minified sources, and dependency archives above this limit provide no + * useful symbols; 1 MB covers essentially all hand-written source. + */ +export const MAX_SOURCE_FILE_SIZE_BYTES = 1024 * 1024; diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 34516fd3d..8b40410f2 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -5,6 +5,7 @@ */ import * as fs from 'fs'; +import { MAX_SOURCE_FILE_SIZE_BYTES } from '../file-limits'; import * as path from 'path'; import { Language, Node, UnresolvedReference, Edge } from '../types'; import { QueryBuilder } from '../db/queries'; @@ -422,6 +423,14 @@ export class ReferenceResolver { } const fullPath = path.join(this.projectRoot, filePath); try { + // Import resolvers may follow package metadata to an archive (`file:*.har`, + // for example). Reject anything extraction would not accept before UTF-8 + // decoding can multiply a large binary blob into gigabytes of V8 heap. + const stats = fs.statSync(fullPath); + if (!stats.isFile() || stats.size > MAX_SOURCE_FILE_SIZE_BYTES) { + this.fileCache.set(filePath, null); + return null; + } const content = fs.readFileSync(fullPath, 'utf-8'); this.fileCache.set(filePath, content); return content;