From 9f1f86f926cf2fd0c0d54466da0f58ddfbabb872 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 5 Jun 2026 13:15:47 +0000 Subject: [PATCH] fix(ingestion): wire C static-linkage side-channel + ADL O(1) collect + tri-review cleanups (#1983) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the worker-pool-only refactor, from a tri-review of the parse path. - C static-linkage side-channel (P1): cProvider had no collect/applyCaptureSideChannel, so on the now-sole worker path C `static` file-local marks were lost across the worker boundary -> false cross-file CALLS edges + over-broad #include wildcard visibility on every C analysis (the Linux kernel is C). Mirror the C++/Kotlin wiring: serialize `staticNames` per file onto ParsedFile.captureSideChannel and restore it on the main thread (no re-parse). + a worker-path regression test (the existing c-static-isolation fixture passed vacuously — its collision resolves via #include before the global free-call fallback ever consults static-linkage). - captureSideChannel `kind` discriminant: add `kind:'cpp'`/`kind:'c'` tags + guards (Kotlin already had one) now that C/C++/Kotlin share the single generic field. - Perf: collectCppAdlSideChannel scanned the whole argInfoBySite/noAdlSites maps per file (O(F^2) per sub-batch, ~100M parseSiteKey calls at kernel scale). Add per-filePath lockstep indexes -> O(1) collect; serialized snapshot byte-identical. - Cleanups: inline the one-line processParsingWithWorkers wrapper into processParsing; drop the always-empty WorkerExtractedData.calls/assignments/constructorBindings fields; remove the voided astCache param from processParsing; refresh stale "sequential fallback" JSDoc. Validation: tsc + build clean; cpp 297/297, c 8/8 (incl. the new worker-path static-linkage guard), typescript + parsedfile-store green; cpp ADL benchmark stays linear. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/core/ingestion/languages/c-cpp.ts | 10 ++ .../languages/c/capture-side-channel.ts | 80 +++++++++++++++ .../src/core/ingestion/languages/c/index.ts | 5 + .../ingestion/languages/c/scope-resolver.ts | 18 ++++ .../ingestion/languages/c/static-linkage.ts | 12 +++ .../src/core/ingestion/languages/cpp/adl.ts | 86 ++++++++++++++-- .../languages/cpp/capture-side-channel.ts | 15 ++- .../src/core/ingestion/parsing-processor.ts | 49 +--------- .../core/ingestion/workers/parse-worker.ts | 6 +- .../src/core/ingestion/workers/worker-pool.ts | 11 ++- .../c-static-linkage-worker/caller.c | 16 +++ .../c-static-linkage-worker/lib.c | 7 ++ .../c-static-linkage-worker/lib.h | 7 ++ .../c-static-linkage-worker/local.c | 13 +++ gitnexus/test/helpers/worker-parse.ts | 2 - gitnexus/test/integration/resolvers/c.test.ts | 59 +++++++++++ .../c-static-linkage-side-channel.test.ts | 97 +++++++++++++++++++ gitnexus/test/unit/parsedfile-store.test.ts | 24 +++++ .../test/unit/parsing-worker-fallback.test.ts | 4 - 19 files changed, 452 insertions(+), 69 deletions(-) create mode 100644 gitnexus/src/core/ingestion/languages/c/capture-side-channel.ts create mode 100644 gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/caller.c create mode 100644 gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.c create mode 100644 gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.h create mode 100644 gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/local.c create mode 100644 gitnexus/test/unit/c-static-linkage-side-channel.test.ts diff --git a/gitnexus/src/core/ingestion/languages/c-cpp.ts b/gitnexus/src/core/ingestion/languages/c-cpp.ts index ac8d4d30d..84523fc70 100644 --- a/gitnexus/src/core/ingestion/languages/c-cpp.ts +++ b/gitnexus/src/core/ingestion/languages/c-cpp.ts @@ -53,6 +53,7 @@ import { cBindingScopeFor, cImportOwningScope, cReceiverBinding, + collectCStaticLinkageSideChannel, } from './c/index.js'; import { emitCppScopeCaptures, @@ -396,6 +397,15 @@ export const cProvider = defineLanguage({ // ── RFC #909 Ring 3: scope-based resolution hooks (RFC §5) ────────── emitScopeCaptures: emitCScopeCaptures, + // Worker-side: snapshot the module-level `static`-linkage marks + // `emitCScopeCaptures` just populated for this file (`markStaticName` → + // `staticNames`) into plain data on `ParsedFile.captureSideChannel`, so the + // main thread can restore them via `applyCaptureSideChannel` WITHOUT a + // re-parse (#1983 — the worker is the sole parse path). Without this, C + // `static` functions look non-file-local on the main thread and leak into + // cross-file global free-call resolution / wildcard imports. See + // `c/capture-side-channel.ts`. + collectCaptureSideChannel: collectCStaticLinkageSideChannel, interpretImport: interpretCImport, interpretTypeBinding: interpretCTypeBinding, bindingScopeFor: cBindingScopeFor, diff --git a/gitnexus/src/core/ingestion/languages/c/capture-side-channel.ts b/gitnexus/src/core/ingestion/languages/c/capture-side-channel.ts new file mode 100644 index 000000000..2f619e5f3 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/c/capture-side-channel.ts @@ -0,0 +1,80 @@ +/** + * C capture-time side-channel serialization (#1983). + * + * `emitCScopeCaptures` populates one MODULE-LEVEL, per-file map as a side + * effect that is NOT part of the returned `ParsedFile`'s scopes/defs: + * + * - `staticNames` (static-linkage.ts) — the simple names of functions + * declared with `static` storage class (file-local / translation-unit + * linkage in C), recorded via `markStaticName` from the + * `@declaration.name` capture when the function node has a `static` + * storage-class specifier. + * + * On the worker path that map is filled in the WORKER process and lost across + * the worker→main MessageChannel (and the disk-backed parsedfile-store), + * because scope-resolution reuses the serialized `ParsedFile` and SKIPS the + * main-thread re-extraction (the #1983 fix that avoids a main-thread + * tree-sitter re-parse / OOM on huge repos — e.g. the Linux kernel). The main + * thread then reads the map empty in `isStaticName` (consulted by + * `isFileLocalDef` in `c/scope-resolver.ts` and by `expandCWildcardNames` in + * static-linkage.ts) — so file-local `static` functions become eligible for + * cross-file global free-call resolution (false CALLS edges) and `#include` + * wildcard imports over-expose them. + * + * This module snapshots the per-file slice of that map into a plain, + * JSON-serializable object (carried on `ParsedFile.captureSideChannel`) and + * restores it on the main thread WITHOUT any parse. It mirrors the C++ pattern + * in `cpp/capture-side-channel.ts` and the Kotlin pattern in + * `kotlin/capture-side-channel.ts`. + * + * The single generic `ParsedFile.captureSideChannel` field is shared with C++ + * and Kotlin, which is safe because each file is one language (a `.c` file uses + * the C provider). The payload is self-describing (`{ kind: 'c', staticNames }`) + * so `applyCStaticLinkageSideChannel` only restores C state and ignores a + * foreign-shaped snapshot. + */ + +import type { ParsedFile } from 'gitnexus-shared'; +import { getStaticNamesForFile, markStaticName } from './static-linkage.js'; + +/** + * Plain JSON-serializable snapshot of the per-file C capture-time + * side-channel. Carried opaquely on `ParsedFile.captureSideChannel`. The + * `kind` tag makes the payload self-describing so `apply` can distinguish a C + * snapshot from another language's (C++ and Kotlin share the same field). + */ +export interface CCaptureSideChannel { + readonly kind: 'c'; + /** Simple names of `static` (file-local linkage) functions in this file. */ + readonly staticNames: readonly string[]; +} + +/** + * `LanguageProvider.collectCaptureSideChannel` implementation for C. + * Returns `undefined` when this file recorded no static names at all, so the + * produced `ParsedFile` carries the field only when there's data to ship. + */ +export function collectCStaticLinkageSideChannel( + filePath: string, +): CCaptureSideChannel | undefined { + const staticNames = getStaticNamesForFile(filePath); + if (staticNames.length === 0) return undefined; + return { kind: 'c', staticNames }; +} + +/** + * `ScopeResolver.applyCaptureSideChannel` implementation for C. Reads the + * worker-serialized snapshot from `parsed.captureSideChannel` and re-populates + * the module-level static-linkage map via `markStaticName`. Tolerant of + * `undefined` (file carried no data) and of an unexpected / foreign shape + * (defensive — the `kind` tag guards against restoring a non-C payload). + * Does NO tree-sitter parse. + */ +export function applyCStaticLinkageSideChannel(parsed: ParsedFile): void { + const data = parsed.captureSideChannel as CCaptureSideChannel | undefined; + if (data === undefined || data === null || typeof data !== 'object') return; + if (data.kind !== 'c' || !Array.isArray(data.staticNames)) return; + for (const name of data.staticNames) { + markStaticName(parsed.filePath, name); + } +} diff --git a/gitnexus/src/core/ingestion/languages/c/index.ts b/gitnexus/src/core/ingestion/languages/c/index.ts index c6900ecba..8ebc2c98e 100644 --- a/gitnexus/src/core/ingestion/languages/c/index.ts +++ b/gitnexus/src/core/ingestion/languages/c/index.ts @@ -13,4 +13,9 @@ export { isStaticName, clearStaticNames, expandCWildcardNames, + getStaticNamesForFile, } from './static-linkage.js'; +export { + collectCStaticLinkageSideChannel, + applyCStaticLinkageSideChannel, +} from './capture-side-channel.js'; diff --git a/gitnexus/src/core/ingestion/languages/c/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/c/scope-resolver.ts index 90d54e072..d2e0e17f7 100644 --- a/gitnexus/src/core/ingestion/languages/c/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/c/scope-resolver.ts @@ -7,6 +7,7 @@ import { cProvider } from '../c-cpp.js'; import { cArityCompatibility, cMergeBindings, resolveCImportTarget } from './index.js'; import { scanHeaderFiles } from './header-scan.js'; import { expandCWildcardNames, isStaticName, clearStaticNames } from './static-linkage.js'; +import { applyCStaticLinkageSideChannel } from './capture-side-channel.js'; /** * C `ScopeResolver` registered in `SCOPE_RESOLVERS` and consumed by @@ -31,6 +32,23 @@ export const cScopeResolver: ScopeResolver = { return scanHeaderFiles(repoPath); }, + // Worker-boundary restore (see `ScopeResolver.applyCaptureSideChannel`). + // `emitCScopeCaptures` records per-file `static`-linkage names + // (`markStaticName` → `staticNames`) as a SIDE EFFECT — that state is NOT + // serialized onto the returned ParsedFile's scopes/defs. On the worker path + // those marks are populated in the worker process and lost across the + // MessageChannel / disk store; the main thread reuses the serialized + // ParsedFile and skips `extractParsedFile`, so `isStaticName` (read by + // `isFileLocalDef` and `expandCWildcardNames`) sees an empty map and C + // `static` functions leak into cross-file global free-call resolution + // (false CALLS edges) and `#include` wildcard imports. The worker stashed a + // plain-data snapshot on `parsed.captureSideChannel` via + // `cProvider.collectCaptureSideChannel`; this restores it into the module + // map WITHOUT any tree-sitter re-parse (the #1983 fix). The + // freshly-extracted leg never calls this — its marks were just populated in + // this process. Runs BEFORE `populateOwners`. + applyCaptureSideChannel: applyCStaticLinkageSideChannel, + resolveImportTarget: (targetRaw, fromFile, allFilePaths, resolutionConfig) => { // Augment allFilePaths with .h files discovered via loadResolutionConfig // since the phase only passes .c files to the C resolver but #include diff --git a/gitnexus/src/core/ingestion/languages/c/static-linkage.ts b/gitnexus/src/core/ingestion/languages/c/static-linkage.ts index 5cfd166b1..df02c3312 100644 --- a/gitnexus/src/core/ingestion/languages/c/static-linkage.ts +++ b/gitnexus/src/core/ingestion/languages/c/static-linkage.ts @@ -29,6 +29,18 @@ export function isStaticName(filePath: string, name: string): boolean { return staticNames.get(filePath)?.has(name) ?? false; } +/** + * Return the `static` (file-local) names recorded for the given file as a + * plain array (empty when none). Used to snapshot the per-file slice of the + * module-level `staticNames` map into `ParsedFile.captureSideChannel` so it + * survives the worker→main boundary (#1983 — the worker is the sole parse + * path). See `c/capture-side-channel.ts`. + */ +export function getStaticNamesForFile(filePath: string): string[] { + const names = staticNames.get(filePath); + return names === undefined ? [] : [...names]; +} + /** Clear tracked static names (for testing). */ export function clearStaticNames(): void { staticNames.clear(); diff --git a/gitnexus/src/core/ingestion/languages/cpp/adl.ts b/gitnexus/src/core/ingestion/languages/cpp/adl.ts index 67ced2016..7a1bd9f66 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/adl.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/adl.ts @@ -108,6 +108,38 @@ const argInfoBySite = new Map(); const noAdlSites = new Set(); const classToNamespaceQualifiedName = new Map(); +/** + * Per-`filePath` index of the site keys this file contributed to + * `argInfoBySite` / `noAdlSites`, kept in **strict lockstep** with those two + * maps (#1983 perf). Without it, `collectCppAdlSideChannel(filePath)` had to + * scan the ENTIRE module-level maps (every site of every file the worker + * parsed in the current sub-batch) and `parseSiteKey` each entry just to pick + * out one file's slice — O(F²) per sub-batch (~100M `parseSiteKey` calls + * across the Linux kernel). These indexes turn collect into + * O(entries-for-this-file). + * + * Lockstep invariant: a key is pushed here at most once, exactly when it is + * first inserted into the corresponding map, and both indexes are cleared + * wherever `argInfoBySite` / `noAdlSites` are cleared (`clearCppAdlState` and + * the per-file restore in `applyCppAdlSideChannel`). The "first insert only" + * guard mirrors the maps' own de-dup (`Map.set` / `Set.add` are idempotent on + * the key), so iterating an index yields each of this file's keys exactly once + * — byte-identical to the old filtered full scan. + */ +const argInfoSiteKeysByFile = new Map(); +const noAdlSiteKeysByFile = new Map(); + +/** Push `key` into the per-file index `idx[filePath]` (creating the bucket on + * first use). Callers guard against duplicate keys so each key appears once. */ +function pushFileSiteKey(idx: Map, filePath: string, key: string): void { + let keys = idx.get(filePath); + if (keys === undefined) { + keys = []; + idx.set(filePath, keys); + } + keys.push(key); +} + /** * ADL candidate index — built **once** per pipeline run from * `(scopes, parsedFiles)` and reused by every call site. @@ -370,13 +402,20 @@ export function markCppAdlSiteArgs( col: number, args: readonly CppAdlArgInfo[], ): void { - argInfoBySite.set(siteKey(filePath, line, col), args); + const key = siteKey(filePath, line, col); + // Lockstep with `argInfoSiteKeysByFile`: index the key only on first insert + // (a re-mark overwrites the value but must NOT duplicate the index entry). + if (!argInfoBySite.has(key)) pushFileSiteKey(argInfoSiteKeysByFile, filePath, key); + argInfoBySite.set(key, args); } /** Mark a call site as ADL-suppressed (function child wrapped in * `parenthesized_expression`, e.g. `(f)(s)`). */ export function markCppAdlSiteNoAdl(filePath: string, line: number, col: number): void { - noAdlSites.add(siteKey(filePath, line, col)); + const key = siteKey(filePath, line, col); + // Lockstep with `noAdlSiteKeysByFile`: index the key only on first insert. + if (!noAdlSites.has(key)) pushFileSiteKey(noAdlSiteKeysByFile, filePath, key); + noAdlSites.add(key); } /** @@ -402,32 +441,54 @@ function parseSiteKey(key: string): { filePath: string; line: number; col: numbe return { filePath: m[1], line: Number(m[2]), col: Number(m[3]) }; } -/** Snapshot this file's ADL capture state for the worker→main side-channel. */ +/** + * Snapshot this file's ADL capture state for the worker→main side-channel. + * + * Uses the per-file `argInfoSiteKeysByFile` / `noAdlSiteKeysByFile` indexes to + * touch only THIS file's entries — O(entries-for-this-file) — instead of the + * old O(all-entries) full scan over `argInfoBySite` / `noAdlSites` (#1983). + * The output order, and therefore the serialized JSON shape, is byte-identical + * to the old filtered scan: the index records keys in the same insertion order + * the maps' own iteration would have yielded for this file, and each key is + * indexed exactly once (mark guards on first insert), so the same per-file + * subsequence is produced. + * + * `parseSiteKey` is still used to recover `line:col` from each key, but now + * only for this file's keys (a bounded handful), never for the whole batch. + */ export function collectCppAdlSideChannel(filePath: string): CppAdlSideChannel { const args: [number, number, readonly CppAdlArgInfo[]][] = []; - for (const [key, value] of argInfoBySite) { + for (const key of argInfoSiteKeysByFile.get(filePath) ?? []) { + const value = argInfoBySite.get(key); const parsed = parseSiteKey(key); - if (parsed !== undefined && parsed.filePath === filePath) { + if (value !== undefined && parsed !== undefined) { args.push([parsed.line, parsed.col, value]); } } const noAdl: [number, number][] = []; - for (const key of noAdlSites) { + for (const key of noAdlSiteKeysByFile.get(filePath) ?? []) { const parsed = parseSiteKey(key); - if (parsed !== undefined && parsed.filePath === filePath) { + if (parsed !== undefined) { noAdl.push([parsed.line, parsed.col]); } } return { argInfoBySite: args, noAdlSites: noAdl }; } -/** Restore this file's ADL capture state from the side-channel (no parse). */ +/** Restore this file's ADL capture state from the side-channel (no parse). + * Keeps the per-file site-key indexes in lockstep with `argInfoBySite` / + * `noAdlSites` (first-insert-only) so a later `collectCppAdlSideChannel` on + * the same process would still produce a correct, duplicate-free snapshot. */ export function applyCppAdlSideChannel(filePath: string, data: CppAdlSideChannel): void { for (const [line, col, value] of data.argInfoBySite) { - argInfoBySite.set(siteKey(filePath, line, col), value); + const key = siteKey(filePath, line, col); + if (!argInfoBySite.has(key)) pushFileSiteKey(argInfoSiteKeysByFile, filePath, key); + argInfoBySite.set(key, value); } for (const [line, col] of data.noAdlSites) { - noAdlSites.add(siteKey(filePath, line, col)); + const key = siteKey(filePath, line, col); + if (!noAdlSites.has(key)) pushFileSiteKey(noAdlSiteKeysByFile, filePath, key); + noAdlSites.add(key); } } @@ -437,6 +498,11 @@ export function applyCppAdlSideChannel(filePath: string, data: CppAdlSideChannel export function clearCppAdlState(): void { argInfoBySite.clear(); noAdlSites.clear(); + // Lockstep: the per-file site-key indexes mirror argInfoBySite/noAdlSites and + // MUST be cleared together — a stale index would resurrect a prior pass's + // (or prior file's, after a re-key) keys into the next snapshot. + argInfoSiteKeysByFile.clear(); + noAdlSiteKeysByFile.clear(); classToNamespaceQualifiedName.clear(); adlIndex = undefined; adlIndexSource = undefined; diff --git a/gitnexus/src/core/ingestion/languages/cpp/capture-side-channel.ts b/gitnexus/src/core/ingestion/languages/cpp/capture-side-channel.ts index d8b1d32da..74f432b69 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/capture-side-channel.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/capture-side-channel.ts @@ -48,6 +48,15 @@ import { * slice for one file. Carried opaquely on `ParsedFile.captureSideChannel`. */ export interface CppCaptureSideChannel { + /** + * Discriminant tag — the single generic `ParsedFile.captureSideChannel` + * field is shared with C (`{ kind: 'c' }`) and Kotlin (`{ kind: 'kotlin' }`). + * `applyCppCaptureSideChannel` checks this first so a foreign-language + * payload reaching the C++ apply (or vice-versa) is cleanly ignored. In + * practice apply only runs for the matching provider (one language per file), + * but the tag makes it robust and consistent with the C/Kotlin snapshots. + */ + readonly kind: 'cpp'; readonly adl: CppAdlSideChannel; /** Inline-namespace source-range keys recorded for this file. */ readonly inlineNamespaceRanges: readonly string[]; @@ -76,7 +85,7 @@ export function collectCppCaptureSideChannel(filePath: string): CppCaptureSideCh twoPhase.dependentPackBaseClasses.length === 0; if (isEmpty) return undefined; - return { adl, inlineNamespaceRanges, fileLocal, twoPhase }; + return { kind: 'cpp', adl, inlineNamespaceRanges, fileLocal, twoPhase }; } /** @@ -89,6 +98,10 @@ export function collectCppCaptureSideChannel(filePath: string): CppCaptureSideCh export function applyCppCaptureSideChannel(parsed: ParsedFile): void { const data = parsed.captureSideChannel as CppCaptureSideChannel | undefined; if (data === undefined || data === null || typeof data !== 'object') return; + // Discriminant guard — the generic `captureSideChannel` field is shared + // with C (`{ kind: 'c' }`) and Kotlin (`{ kind: 'kotlin' }`); cleanly + // ignore a non-C++ payload rather than mis-applying it. + if (data.kind !== 'cpp') return; if (data.adl !== undefined) applyCppAdlSideChannel(parsed.filePath, data.adl); if (data.inlineNamespaceRanges !== undefined) { applyCppInlineNamespaceSideChannel(parsed.filePath, data.inlineNamespaceRanges); diff --git a/gitnexus/src/core/ingestion/parsing-processor.ts b/gitnexus/src/core/ingestion/parsing-processor.ts index a08684bdc..3c7717d77 100644 --- a/gitnexus/src/core/ingestion/parsing-processor.ts +++ b/gitnexus/src/core/ingestion/parsing-processor.ts @@ -1,7 +1,6 @@ import type { NodeLabel } from 'gitnexus-shared'; import { KnowledgeGraph } from '../graph/types.js'; import type { SymbolTableWriter } from './model/index.js'; -import { ASTCache } from './ast-cache.js'; import { getLanguageFromFilename } from 'gitnexus-shared'; import { accumulateExportedTypesFromParsedNode, type ExportedTypeMap } from './call-processor.js'; @@ -12,13 +11,10 @@ import { logger } from '../logger.js'; import type { ParseWorkerResult, ParseWorkerInput, - ExtractedCall, - ExtractedAssignment, ExtractedRoute, ExtractedFetchCall, ExtractedDecoratorRoute, ExtractedToolDef, - FileConstructorBindings, FileScopeBindings, ExtractedORMQuery, FetchWrapperDef, @@ -32,8 +28,6 @@ import type { export type FileProgressCallback = (current: number, total: number, filePath: string) => void; export interface WorkerExtractedData { - calls: ExtractedCall[]; - assignments: ExtractedAssignment[]; routes: ExtractedRoute[]; fetchCalls: ExtractedFetchCall[]; fetchWrapperDefs: FetchWrapperDef[]; @@ -43,7 +37,6 @@ export interface WorkerExtractedData { routerModuleAliases: ExtractedRouterModuleAlias[]; toolDefs: ExtractedToolDef[]; ormQueries: ExtractedORMQuery[]; - constructorBindings: FileConstructorBindings[]; fileScopeBindings: FileScopeBindings[]; /** * Per-file `ParsedFile` artifacts from the new scope-based resolution @@ -63,7 +56,7 @@ export interface WorkerExtractedData { * Merge a list of `ParseWorkerResult`s into the running graph + symbol * table state and produce the chunk-aggregated `WorkerExtractedData`. * - * Extracted from `processParsingWithWorkers` so the same merge logic can + * Split out from the worker-parse path so the same merge logic can * be applied to both freshly-parsed worker output AND cached worker * output replayed during incremental analyze. Idempotent on the * accumulator fields (push-only); idempotent on graph if the caller @@ -132,8 +125,6 @@ export const mergeChunkResults = ( } return { - calls: [], - assignments: [], routes: allRoutes, fetchCalls: allFetchCalls, fetchWrapperDefs: allFetchWrapperDefs, @@ -143,7 +134,6 @@ export const mergeChunkResults = ( routerModuleAliases: allRouterModuleAliases, toolDefs: allToolDefs, ormQueries: allORMQueries, - constructorBindings: [], fileScopeBindings: fileScopeBindingsByFile, parsedFiles: allParsedFiles, }; @@ -152,7 +142,7 @@ export const mergeChunkResults = ( /** * Dispatch a chunk's files to the worker pool and return the RAW per-worker * results, WITHOUT merging them into the graph. Split out from - * {@link processParsingWithWorkers} so the parse loop can overlap one chunk's + * {@link processParsing} so the parse loop can overlap one chunk's * merge (main-thread, via {@link mergeChunkResults}) with the NEXT chunk's * worker parse — the merge is the only remaining serial main-thread step once * ParsedFile serialization moved into the workers (#worker-idle pipelining). @@ -203,28 +193,6 @@ export const dispatchChunkParse = async ( return chunkResults; }; -const processParsingWithWorkers = async ( - graph: KnowledgeGraph, - files: { path: string; content: string }[], - symbolTable: SymbolTableWriter, - astCache: ASTCache, - workerPool: WorkerPool, - onFileProgress?: FileProgressCallback, - /** - * When provided, populated with the raw worker results before merging. - * Used by the incremental-indexing parse cache to capture the per-chunk - * worker output for caching across runs. The mutation happens in-place - * so the caller (parse-impl) can keep a reference. See - * `gitnexus/src/storage/parse-cache.ts`. - */ - outRawResults?: ParseWorkerResult[], - exportedTypeMap?: ExportedTypeMap, -): Promise => { - void astCache; // worker path parses in worker threads; astCache is sequential-only - const chunkResults = await dispatchChunkParse(files, workerPool, onFileProgress, outRawResults); - return mergeChunkResults(graph, symbolTable, chunkResults, exportedTypeMap); -}; - // ============================================================================ // Public API // ============================================================================ @@ -241,7 +209,6 @@ export const processParsing = async ( graph: KnowledgeGraph, files: { path: string; content: string }[], symbolTable: SymbolTableWriter, - astCache: ASTCache, workerPool: WorkerPool, onFileProgress?: FileProgressCallback, /** @@ -267,16 +234,8 @@ export const processParsing = async ( // per-chunk warn below; the chunk-cache write-guard in parse-impl.ts keeps the // chunk uncached so the next analyze retries with a fresh pool), and a full // pool failure propagates `WorkerPoolDispatchError` so the run errors out. - const data = await processParsingWithWorkers( - graph, - files, - symbolTable, - astCache, - workerPool, - reportProgress, - outRawResults, - exportedTypeMap, - ); + const chunkResults = await dispatchChunkParse(files, workerPool, reportProgress, outRawResults); + const data = mergeChunkResults(graph, symbolTable, chunkResults, exportedTypeMap); // Session-scoped quarantine (worker-pool resilience Layer 3): surface any // files this pool has decided are unsafe for workers so the operator can see // what was skipped. The pool already filtered them out of dispatch; we only diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 74ebfbafb..bd01ce228 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -1172,8 +1172,10 @@ const processFileGroup = ( ); if (parsedFile !== undefined) { // Capture-time side-channel (#1983): `extractParsedFile` just ran the - // provider's `emitScopeCaptures`, which (for C++) populated module-level - // maps as a SIDE EFFECT that is NOT on `parsedFile`'s scopes/defs. Snapshot + // provider's `emitScopeCaptures`, which (for C++ ADL/namespace marks, + // C `static`-linkage names, and Kotlin companion scopes) populated + // module-level maps as a SIDE EFFECT that is NOT on `parsedFile`'s + // scopes/defs. Snapshot // that per-file state as plain data onto `ParsedFile.captureSideChannel` // so the main thread can restore it (via `ScopeResolver.applyCaptureSideChannel`) // WITHOUT a re-parse, after this ParsedFile crosses the worker boundary / diff --git a/gitnexus/src/core/ingestion/workers/worker-pool.ts b/gitnexus/src/core/ingestion/workers/worker-pool.ts index d36d54e90..b1b0c1f7e 100644 --- a/gitnexus/src/core/ingestion/workers/worker-pool.ts +++ b/gitnexus/src/core/ingestion/workers/worker-pool.ts @@ -102,9 +102,10 @@ export interface WorkerPool { * * Files in {@link WorkerPool.getQuarantinedPaths} are filtered out before * dispatch — they have already caused a worker death this pool lifetime and - * are not safe to re-attempt in workers. The caller is responsible for - * routing them (e.g. to sequential fallback); inspect the quarantine - * snapshot before and after each dispatch. + * are not safe to re-attempt in workers. They are dropped from the run (the + * sequential fallback that once re-parsed them was removed); inspect the + * quarantine snapshot before and after each dispatch to surface skipped files + * in diagnostics. */ dispatch( items: TInput[], @@ -192,8 +193,8 @@ export interface WorkerPoolOptions { * Hard ceiling on total wall time the pool will spend retrying / splitting * any single job. Combined with `timeoutBackoffFactor`, this prevents * exponentially-growing retry waits from accumulating into multi-hour - * stalls before the pool finally surfaces the bad file to sequential - * fallback. Default 5x `subBatchIdleTimeoutMs`. + * stalls before the pool finally quarantines the bad file and proceeds + * without it. Default 5x `subBatchIdleTimeoutMs`. */ maxCumulativeTimeoutMs?: number; /** diff --git a/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/caller.c b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/caller.c new file mode 100644 index 000000000..224dd83d7 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/caller.c @@ -0,0 +1,16 @@ +/* caller.c — calls `compute()`. + * + * It does NOT `#include "local.c"` (and could not — `local.c`'s `compute` is + * `static`, so it has file-local linkage and is invisible cross-file). The + * only legitimate cross-file `compute` target is `lib.c`'s free function, + * reached via the global free-call fallback (C `#include` brings in all + * non-static symbols). + * + * The regression guarded here: on the worker-only parse path (#1983), the + * `static`-linkage map populated in the worker is lost across the boundary, + * so without the capture side-channel `local.c`'s `static compute` looks + * non-file-local on the main thread and the global fallback emits a FALSE + * `caller_entry -> compute@local.c` CALLS edge. */ +int caller_entry(void) { + return compute(7); +} diff --git a/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.c b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.c new file mode 100644 index 000000000..ad1d9f90e --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.c @@ -0,0 +1,7 @@ +/* lib.c — a non-static (externally visible) free function `compute`. + * Declared in lib.h so callers can `#include` it and resolve to THIS one. */ +#include "lib.h" + +int compute(int x) { + return x * 2; +} diff --git a/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.h b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.h new file mode 100644 index 000000000..e997488f5 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/lib.h @@ -0,0 +1,7 @@ +/* lib.h — public header for lib.c */ +#ifndef LIB_H +#define LIB_H + +int compute(int x); + +#endif diff --git a/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/local.c b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/local.c new file mode 100644 index 000000000..889e01ee0 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/c-static-linkage-worker/local.c @@ -0,0 +1,13 @@ +/* local.c — a file-local `static` function `compute`. + * + * `static` gives C translation-unit (file-local) linkage: `compute` is NOT + * visible to any other translation unit. Its simple name collides with a + * free function of the same name defined in `lib.c`, so the cross-file + * resolver must NOT mistake one for the other. */ +static int compute(int x) { + return x + 1; +} + +int local_entry(void) { + return compute(41); +} diff --git a/gitnexus/test/helpers/worker-parse.ts b/gitnexus/test/helpers/worker-parse.ts index c8afdca99..a42f7e4d6 100644 --- a/gitnexus/test/helpers/worker-parse.ts +++ b/gitnexus/test/helpers/worker-parse.ts @@ -20,7 +20,6 @@ import { processParsing, type WorkerExtractedData, } from '../../src/core/ingestion/parsing-processor.js'; -import { createASTCache } from '../../src/core/ingestion/ast-cache.js'; import { createSemanticModel } from '../../src/core/ingestion/model/semantic-model.js'; import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; import type { KnowledgeGraph } from '../../src/core/graph/types.js'; @@ -66,7 +65,6 @@ export const parseFilesWithWorkers = async ( graph, files, model.symbols, - createASTCache(Math.max(files.length, 1)), pool, undefined, opts.outRawResults, diff --git a/gitnexus/test/integration/resolvers/c.test.ts b/gitnexus/test/integration/resolvers/c.test.ts index af2919bb0..26dc92f49 100644 --- a/gitnexus/test/integration/resolvers/c.test.ts +++ b/gitnexus/test/integration/resolvers/c.test.ts @@ -106,3 +106,62 @@ describe('C static function isolation', () => { } }); }); + +// --------------------------------------------------------------------------- +// C static-linkage survives the worker→main boundary (#1983 / worker-only path) +// +// The worker pool is now the SOLE parse path. C `static`-linkage is tracked in +// a module-level map populated by `emitCScopeCaptures` INSIDE the worker +// (`markStaticName`). Without the capture side-channel, that map is lost across +// the MessageChannel and the main-thread `isStaticName` reads empty, so a +// file-local `static` function becomes eligible for cross-file global free-call +// resolution — a FALSE CALLS edge. +// +// This fixture isolates that leak (which `c-static-isolation` above does NOT +// exercise — there the colliding name resolves via `#include` before the global +// fallback runs). Here `caller.c` calls `compute()` with no include of +// `local.c`; the only legitimate cross-file target is `lib.c`'s free `compute`. +// `local.c`'s `static compute` MUST stay file-local. +// +// Without the c-cpp.ts `collectCaptureSideChannel` + c/scope-resolver.ts +// `applyCaptureSideChannel` wiring, this test fails: `caller_entry` resolves to +// `compute@local.c`. +// --------------------------------------------------------------------------- + +describe('C static-linkage survives the worker→main boundary (#1983)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'c-static-linkage-worker'), () => {}); + }, 60000); + + it('parses via the worker pool (the path the side-channel covers)', () => { + expect(result.usedWorkerPool).toBe(true); + }); + + it('does NOT emit a cross-file CALLS edge to the file-local static compute', () => { + const calls = getRelationships(result, 'CALLS'); + + // The caller's `compute()` must resolve to lib.c's free function, never to + // local.c's `static` (file-local) one. + const callerToCompute = calls.filter( + (r) => r.source === 'caller_entry' && r.target === 'compute', + ); + // The free `compute` in lib.c is the only valid cross-file target. + expect(callerToCompute.length).toBeGreaterThan(0); + for (const edge of callerToCompute) { + expect(edge.targetFilePath).toContain('lib.c'); + expect(edge.targetFilePath).not.toContain('local.c'); + } + + // Belt-and-suspenders: NO edge anywhere may reach local.c's static compute + // from outside local.c (the intra-file local_entry → compute call is fine). + const leakedToStatic = calls.filter( + (r) => + r.target === 'compute' && + r.targetFilePath.includes('local.c') && + !r.sourceFilePath.includes('local.c'), + ); + expect(leakedToStatic).toEqual([]); + }); +}); diff --git a/gitnexus/test/unit/c-static-linkage-side-channel.test.ts b/gitnexus/test/unit/c-static-linkage-side-channel.test.ts new file mode 100644 index 000000000..8be72f9f8 --- /dev/null +++ b/gitnexus/test/unit/c-static-linkage-side-channel.test.ts @@ -0,0 +1,97 @@ +/** + * Unit tests for the C `static`-linkage capture side-channel (#1983). + * + * The worker pool is the sole parse path. C `static`-linkage is recorded in a + * module-level `staticNames` map populated by `emitCScopeCaptures` INSIDE the + * worker (`markStaticName`). That map is NOT serialized onto the returned + * `ParsedFile`, so on the worker→main boundary it must travel as plain data on + * `ParsedFile.captureSideChannel` (collect on the worker, apply on the main + * thread). These tests pin that round-trip contract directly, mirroring the + * C++ (`cpp/capture-side-channel.ts`) and Kotlin patterns: + * + * 1. Collect snapshots the per-file `staticNames` slice into a self- + * describing `{ kind: 'c', staticNames }` payload; returns `undefined` + * when the file recorded no statics (so the field ships only when needed). + * 2. Apply re-populates the module map on a "fresh" process (modelled by a + * `clearStaticNames()` between collect and apply) WITHOUT any parse, so + * `isStaticName` reads true again. + * 3. The `kind` discriminant guards apply against a foreign-language payload + * (the generic `captureSideChannel` field is shared with C++/Kotlin). + */ + +import { beforeEach, describe, expect, it } from 'vitest'; +import type { ParsedFile } from 'gitnexus-shared'; +import { + applyCStaticLinkageSideChannel, + collectCStaticLinkageSideChannel, +} from '../../src/core/ingestion/languages/c/capture-side-channel.js'; +import { + clearStaticNames, + isStaticName, + markStaticName, +} from '../../src/core/ingestion/languages/c/static-linkage.js'; + +function makeParsed(filePath: string, captureSideChannel: unknown): ParsedFile { + return { filePath, captureSideChannel } as unknown as ParsedFile; +} + +describe('C static-linkage capture side-channel round-trip (#1983)', () => { + beforeEach(() => { + clearStaticNames(); + }); + + it('collects a self-describing { kind: "c", staticNames } snapshot', () => { + markStaticName('local.c', 'compute'); + markStaticName('local.c', 'helper'); + + const snapshot = collectCStaticLinkageSideChannel('local.c'); + expect(snapshot).toBeDefined(); + expect(snapshot!.kind).toBe('c'); + // Order-independent: the set may serialize in any order. + expect([...snapshot!.staticNames].sort()).toEqual(['compute', 'helper']); + }); + + it('returns undefined for a file with no static names (field ships only when needed)', () => { + expect(collectCStaticLinkageSideChannel('public-only.c')).toBeUndefined(); + }); + + it('apply re-populates the module map on a fresh process (no parse)', () => { + markStaticName('local.c', 'compute'); + const snapshot = collectCStaticLinkageSideChannel('local.c'); + + // Model the worker→main boundary: the main thread starts with an empty map + // (the worker's marks never crossed the MessageChannel directly). + clearStaticNames(); + expect(isStaticName('local.c', 'compute')).toBe(false); + + applyCStaticLinkageSideChannel(makeParsed('local.c', snapshot)); + expect(isStaticName('local.c', 'compute')).toBe(true); + }); + + it('apply ignores undefined / null / non-object payloads (no throw)', () => { + expect(() => applyCStaticLinkageSideChannel(makeParsed('x.c', undefined))).not.toThrow(); + expect(() => applyCStaticLinkageSideChannel(makeParsed('x.c', null))).not.toThrow(); + expect(() => applyCStaticLinkageSideChannel(makeParsed('x.c', 42))).not.toThrow(); + expect(isStaticName('x.c', 'anything')).toBe(false); + }); + + it('apply ignores a foreign-language payload via the kind discriminant', () => { + // A Kotlin-shaped snapshot must NOT be restored as C static names. + const kotlinPayload = { kind: 'kotlin', companionScopes: ['scope:Logger.companion'] }; + applyCStaticLinkageSideChannel(makeParsed('App.kt', kotlinPayload)); + expect(isStaticName('App.kt', 'companion')).toBe(false); + expect(isStaticName('App.kt', 'scope:Logger.companion')).toBe(false); + }); + + it('a JSON round-trip of the snapshot still applies cleanly (disk-store fidelity)', () => { + markStaticName('mod.c', 'priv_a'); + markStaticName('mod.c', 'priv_b'); + const snapshot = collectCStaticLinkageSideChannel('mod.c'); + const throughJson = JSON.parse(JSON.stringify(snapshot)) as unknown; + + clearStaticNames(); + applyCStaticLinkageSideChannel(makeParsed('mod.c', throughJson)); + expect(isStaticName('mod.c', 'priv_a')).toBe(true); + expect(isStaticName('mod.c', 'priv_b')).toBe(true); + }); +}); diff --git a/gitnexus/test/unit/parsedfile-store.test.ts b/gitnexus/test/unit/parsedfile-store.test.ts index 87f074aff..f64bbd022 100644 --- a/gitnexus/test/unit/parsedfile-store.test.ts +++ b/gitnexus/test/unit/parsedfile-store.test.ts @@ -218,6 +218,30 @@ describe('parsedfile-store', () => { } }); + // #1983 (C): the C provider carries a self-describing static-linkage side- + // channel `{ kind: 'c', staticNames: string[] }` (the file-local `static` + // function names the worker recorded). It shares the single generic + // `captureSideChannel` field with C++/Kotlin, so confirm the plain-data shape + // survives the JSON store round-trip too — without it, `static` functions + // leak into cross-file resolution on the worker-only parse path. + it('round-trips a C ParsedFile.captureSideChannel through the store', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'pfstore-')); + try { + const sideChannel = { kind: 'c', staticNames: ['compute', 'helper'] }; + const pf = { + ...(makeParsedFile('local.c') as unknown as Record), + captureSideChannel: sideChannel, + } as unknown as ParsedFile; + + persistParsedFileShardSync(dir, 'w1-0', [pf]); + const loaded = await loadParsedFilesForPaths(dir, new Set(['local.c'])); + const got = loaded.get('local.c')!; + expect((got as { captureSideChannel?: unknown }).captureSideChannel).toEqual(sideChannel); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + it('persistParsedFileShardSync writes no shard and no directory for empty input', async () => { const dir = await mkdtemp(path.join(tmpdir(), 'pfstore-')); try { diff --git a/gitnexus/test/unit/parsing-worker-fallback.test.ts b/gitnexus/test/unit/parsing-worker-fallback.test.ts index 91c9b8a4c..e88f8b4aa 100644 --- a/gitnexus/test/unit/parsing-worker-fallback.test.ts +++ b/gitnexus/test/unit/parsing-worker-fallback.test.ts @@ -26,7 +26,6 @@ * next analyze with a fresh pool gets another chance. */ import { describe, expect, it, vi } from 'vitest'; -import { createASTCache } from '../../src/core/ingestion/ast-cache.js'; import { processParsing } from '../../src/core/ingestion/parsing-processor.js'; import type { WorkerPool } from '../../src/core/ingestion/workers/worker-pool.js'; import { WorkerPoolDispatchError } from '../../src/core/ingestion/workers/worker-pool.js'; @@ -49,7 +48,6 @@ describe('processParsing — worker-pool error propagation (U20)', () => { graph, [{ path: 'src/a.ts', content: 'export function a() { return 1; }\n' }], createSymbolTable(), - createASTCache(), workerPool, () => {}, ), @@ -81,7 +79,6 @@ describe('processParsing — worker-pool error propagation (U20)', () => { { path: 'src/a.ts', content: 'export function a() { return 1; }\n' }, ], createSymbolTable(), - createASTCache(), workerPool, () => {}, ); @@ -129,7 +126,6 @@ describe('processParsing — worker-pool error propagation (U20)', () => { { path: 'src/a.ts', content: 'export function a() { return 1; }\n' }, ], createSymbolTable(), - createASTCache(), workerPool, (_current, _total, detail) => { progressDetails.push(detail);