diff --git a/gitnexus/src/core/ingestion/array-utils.ts b/gitnexus/src/core/ingestion/array-utils.ts new file mode 100644 index 000000000..33b492bd2 --- /dev/null +++ b/gitnexus/src/core/ingestion/array-utils.ts @@ -0,0 +1,11 @@ +/** + * Append all elements from `source` into `target` without using the spread + * operator. `target.push(...source)` converts every element into a function + * argument which exceeds V8's call-stack limit when `source` has more than + * ~65 000 entries (common in large monoliths with many symbols). + */ +export function safePushAll(target: T[], source: readonly T[]): void { + for (let i = 0; i < source.length; i++) { + target.push(source[i]); + } +} diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index 1e1193b68..6d455c144 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -1,4 +1,5 @@ import { createKnowledgeGraph } from '../graph/graph.js'; +import { safePushAll } from './array-utils.js'; import { processStructure } from './structure-processor.js'; import { processMarkdown } from './markdown-processor.js'; import { processCobol, isCobolFile, isJclFile } from './cobol-processor.js'; @@ -72,20 +73,6 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; const isDev = process.env.NODE_ENV === 'development'; -/** - * Append all elements from `source` into `target` without using the spread - * operator. `target.push(...source)` converts every element into a function - * argument which exceeds V8's call-stack limit when `source` has more than - * ~65 000 entries (common in large monoliths with many symbols). - * - * Exported for regression testing — see safe-push-all.test.ts. - */ -export function safePushAll(target: T[], source: readonly T[]): void { - for (let i = 0; i < source.length; i++) { - target.push(source[i]); - } -} - const EXPO_NAV_PATTERNS = [ /router\.(push|replace|navigate)\(\s*['"`]([^'"`]+)['"`]/g, /]*href=\s*['"`]([^'"`]+)['"`]/g, diff --git a/gitnexus/test/unit/safe-push-all.test.ts b/gitnexus/test/unit/safe-push-all.test.ts index 5e8ca698b..8ebe756ab 100644 --- a/gitnexus/test/unit/safe-push-all.test.ts +++ b/gitnexus/test/unit/safe-push-all.test.ts @@ -1,16 +1,21 @@ /** - * Regression test for safePushAll — the helper introduced to replace - * `array.push(...largeArray)` in the pipeline chunk-accumulation loop. + * Regression tests for the safePushAll helper and the pipeline call sites + * that must use it instead of `array.push(...largeArray)`. * * The spread operator converts every element into a V8 call-stack argument. * At ~65k+ elements a single `push(...arr)` call overflows the stack. - * This test ensures safePushAll handles that size without crashing and - * will catch any future reintroduction of the spread pattern. + * + * Two layers of defense: + * 1. Unit tests — safePushAll itself works at scale. + * 2. Static analysis — pipeline.ts contains no `.push(...` in the + * chunk-accumulation section, catching future reintroductions. */ import { describe, it, expect } from 'vitest'; -import { safePushAll } from '../../src/core/ingestion/pipeline.js'; +import { safePushAll } from '../../src/core/ingestion/array-utils.js'; +import fs from 'fs'; +import path from 'path'; -/** Size that reliably triggers the V8 stack overflow with push(...arr). */ +/** Size well above V8's ~65k call-stack argument limit. */ const OVERFLOW_SIZE = 200_000; describe('safePushAll', () => { @@ -37,12 +42,52 @@ describe('safePushAll', () => { for (let i = 0; i < OVERFLOW_SIZE; i++) source[i] = i; const target: number[] = []; - // This must not throw — push(...source) would RangeError here. safePushAll(target, source); expect(target.length).toBe(OVERFLOW_SIZE); - // Spot-check order is preserved at boundaries expect(target[0]).toBe(0); expect(target[OVERFLOW_SIZE - 1]).toBe(OVERFLOW_SIZE - 1); }); }); + +describe('pipeline.ts has no spread-push in chunk accumulation', () => { + const pipelinePath = path.resolve(__dirname, '../../src/core/ingestion/pipeline.ts'); + const source = fs.readFileSync(pipelinePath, 'utf-8'); + const lines = source.split('\n'); + + it('does not use .push(...) on deferred accumulation arrays', () => { + // These are the arrays that accumulate per-chunk worker results. + // Any .push(...) on them will overflow on large repos. + const accumulatorNames = [ + 'deferredWorkerCalls', + 'deferredWorkerHeritage', + 'deferredConstructorBindings', + 'deferredAssignments', + 'workerTypeEnvBindings', + 'allFetchCalls', + 'allExtractedRoutes', + 'allDecoratorRoutes', + 'allToolDefs', + 'allORMQueries', + ]; + + const spreadPushPattern = /\.push\(\.\.\./; + const violations: string[] = []; + + lines.forEach((line, idx) => { + if (spreadPushPattern.test(line)) { + const trimmed = line.trim(); + // Check if the push target is one of the known accumulator arrays + if (accumulatorNames.some((name) => trimmed.startsWith(`${name}.push(`))) { + violations.push(`line ${idx + 1}: ${trimmed}`); + } + } + }); + + expect( + violations, + 'Found .push(...) on chunk-accumulation arrays in pipeline.ts. ' + + 'Use safePushAll() instead to avoid V8 stack overflow on large repos.', + ).toEqual([]); + }); +});