From fc9e8270819ba2391f90c1f473bab44bca30a1ba Mon Sep 17 00:00:00 2001 From: lesseradmin Date: Mon, 7 Sep 2026 18:26:59 -0500 Subject: [PATCH] fix(cobol): prefer copybook dirs so COPY EXTERNAL does not hit vendor decoys COPY of an out-of-repo member first-won any same-named .cpy, so vendor/EXTERNAL.cpy became a live cobol-copy IMPORTS edge. Share one resolver between census and the regex processor: prefer copybooks/cpy/copy plus the importer dir when present, else fail-open. Drop COBOL KNOWN_GAPS. Fixes #2967 --- .../src/core/ingestion/cobol-processor.ts | 35 ++---- .../ingestion/languages/cobol/copy-target.ts | 113 ++++++++++++++++++ .../languages/cobol/scope-resolver.ts | 72 +---------- .../unit/cobol-copy-external-imports.test.ts | 78 ++++++++++++ .../cobol-import-target-parity.test.ts | 46 ++++++- .../external-import-conformance.test.ts | 7 +- 6 files changed, 255 insertions(+), 96 deletions(-) create mode 100644 gitnexus/src/core/ingestion/languages/cobol/copy-target.ts create mode 100644 gitnexus/test/unit/cobol-copy-external-imports.test.ts diff --git a/gitnexus/src/core/ingestion/cobol-processor.ts b/gitnexus/src/core/ingestion/cobol-processor.ts index 8963ea893..3fac5072d 100644 --- a/gitnexus/src/core/ingestion/cobol-processor.ts +++ b/gitnexus/src/core/ingestion/cobol-processor.ts @@ -6,8 +6,8 @@ * does its own extraction, and writes directly to the graph. * * Pipeline: - * 1. Separate programs from copybooks - * 2. Build copybook map (name -> content) + * 1. Separate programs from copybooks and JCL + * 2. Index file content by path for COPY expansion * 3. For each program: expand COPY statements, then run regex extraction * 4. Map CobolRegexResults to graph nodes and relationships * 5. Optionally process JCL files for job-step cross-references @@ -25,6 +25,7 @@ import { } from './cobol/cobol-preprocessor.js'; import { expandCopies } from './cobol/cobol-copy-expander.js'; import { processJclFiles } from './cobol/jcl-processor.js'; +import { resolveCobolCopyTarget } from './languages/cobol/copy-target.js'; import { logger } from '../logger.js'; @@ -123,38 +124,23 @@ export const processCobol = ( // ── 1. Separate programs, copybooks, and JCL ─────────────────────── const programs: CobolFile[] = []; - const copybooks: CobolFile[] = []; const jclFiles: CobolFile[] = []; for (const file of files) { const ext = path.extname(file.path).toLowerCase(); if (JCL_EXTENSIONS.has(ext)) { jclFiles.push(file); - } else if (isCopybook(file.path)) { - copybooks.push(file); - } else if (COBOL_EXTENSIONS.has(ext)) { + } else if (COBOL_EXTENSIONS.has(ext) && !isCopybook(file.path)) { programs.push(file); } } - // ── 2. Build copybook map (uppercase name -> content) ────────────── - const copybookMap = new Map(); - for (const cb of copybooks) { - const name = path.basename(cb.path, path.extname(cb.path)).toUpperCase(); - copybookMap.set(name, { content: cb.content, path: cb.path }); - } - - // Build reverse lookup: path -> content for O(1) readCopy + // Path → content for every COBOL/JCL file we ingested. Resolution picks + // the path (#2967 / census lockstep); this map only supplies the body. const copybookByPath = new Map(); - for (const [, entry] of copybookMap) { - copybookByPath.set(entry.path, entry.content); + for (const f of files) { + copybookByPath.set(f.path, f.content); } - - // Resolve and read callbacks for expandCopies - const resolveCopy = (name: string): string | null => { - const entry = copybookMap.get(name.toUpperCase()); - return entry ? entry.path : null; - }; // Memoize preprocessed copybook content for the duration of this // processCobol call. A single copybook is COPYed by many programs (and at // many COPY sites within a program); without this cache @@ -196,6 +182,11 @@ export const processCobol = ( // Preprocess: clean patch markers const cleaned = preprocessCobolSource(file.content); + // Per-program so COPY EXTERNAL in src/PROG.cbl cannot land on + // vendor/EXTERNAL.cpy when a copybooks/ dir is present (#2967). + const resolveCopy = (name: string): string | null => + resolveCobolCopyTarget(name, file.path, allPathSet); + // Expand COPY statements const { expandedContent, copyResolutions } = expandCopies( cleaned, diff --git a/gitnexus/src/core/ingestion/languages/cobol/copy-target.ts b/gitnexus/src/core/ingestion/languages/cobol/copy-target.ts new file mode 100644 index 000000000..8fd90e841 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/cobol/copy-target.ts @@ -0,0 +1,113 @@ +/** + * Shared COBOL `COPY` target resolution (#2967 / #2908). + * + * Both the census (`cobolScopeResolver.resolveImportTarget`) and the live + * regex processor (`resolveCopy` → `cobol-copy` IMPORTS) must answer the + * same path for the same `(name, fromFile, allFilePaths)`. Greening only + * the census would leave analyze fabricating an IMPORTS edge onto a vendor + * decoy the compiler would never have searched. + * + * Well-known copybook directory segments (`copybooks`, `COPYBOOKS`, `cpy`, + * `copy`) plus the importer's own directory are the preferred class. When + * the file set contains at least one of those well-known segments, a `COPY` + * name resolves only inside that class (first in Set-iteration order). + * `vendor/EXTERNAL.cpy` therefore misses while `copybooks/CUSTREC.cpy` hits. + * + * When the file set has no such directory, fail-open to today's two-tier + * first-wins basename index — shops whose copybooks *are* the tree have + * nothing to prefer. The importer directory is *not* enough on its own to + * leave fail-open (`{vendor/EXTERNAL.cpy, src/PROG.cbl}` must still answer + * `vendor/EXTERNAL.cpy`). + * + * The two-tier index is still memoized on the `allFilePaths` Set identity + * (#2908). Preferred-class filtering happens at pick time from the stored + * per-name arrays, so a pass still scans the set once. + */ +import path from 'node:path'; +import { perFileSet } from '../../import-resolvers/per-file-set.js'; + +const COPYBOOK_EXTENSIONS = new Set(['.cpy', '.copybook']); +const COBOL_SOURCE_EXTENSIONS = new Set(['.cbl', '.cob', '.cobol']); + +/** Path-component names that mark a conventional copybook directory. */ +const PREFERRED_DIR_NAMES = new Set(['copybooks', 'COPYBOOKS', 'cpy', 'copy']); + +interface CobolCopyIndex { + /** `.cpy` / `.copybook` files — tier 1, Set-iteration order per stem. */ + readonly copybooks: ReadonlyMap; + /** `.cbl` / `.cob` / `.cobol` files — tier 2, Set-iteration order per stem. */ + readonly sources: ReadonlyMap; + /** True iff any file path has a well-known copybook directory segment. */ + readonly hasPreferredDir: boolean; +} + +function pathHasPreferredDir(fp: string): boolean { + const parts = fp.split('/'); + for (let i = 0; i < parts.length - 1; i++) { + if (PREFERRED_DIR_NAMES.has(parts[i])) return true; + } + return false; +} + +function pushUnique(map: Map, key: string, fp: string): void { + const list = map.get(key); + if (list === undefined) { + map.set(key, [fp]); + return; + } + list.push(fp); +} + +const getCobolCopyIndex = perFileSet((allFilePaths: ReadonlySet): CobolCopyIndex => { + const copybooks = new Map(); + const sources = new Map(); + let hasPreferredDir = false; + for (const fp of allFilePaths) { + if (!hasPreferredDir && pathHasPreferredDir(fp)) hasPreferredDir = true; + const ext = path.extname(fp).toLowerCase(); + const tier = COPYBOOK_EXTENSIONS.has(ext) + ? copybooks + : COBOL_SOURCE_EXTENSIONS.has(ext) + ? sources + : undefined; + if (tier === undefined) continue; + const basename = path.basename(fp, ext).toUpperCase(); + pushUnique(tier, basename, fp); + } + return { copybooks, sources, hasPreferredDir }; +}); + +function pickCopyPath( + paths: readonly string[] | undefined, + fromFile: string, + hasPreferredDir: boolean, +): string | null { + if (paths === undefined || paths.length === 0) return null; + if (!hasPreferredDir) return paths[0] ?? null; + const fromDir = path.dirname(fromFile); + for (const fp of paths) { + if (pathHasPreferredDir(fp) || path.dirname(fp) === fromDir) return fp; + } + return null; +} + +/** + * Resolve a COBOL `COPY` member name against the workspace file set. + * + * `fromFile` is the importing program (census) or the program currently + * being expanded (processor). It is used only to include the importer's + * directory in the preferred class when a well-known copybook dir exists. + */ +export function resolveCobolCopyTarget( + targetRaw: string, + fromFile: string, + allFilePaths: ReadonlySet, +): string | null { + const upper = targetRaw.toUpperCase(); + const index = getCobolCopyIndex(allFilePaths); + return ( + pickCopyPath(index.copybooks.get(upper), fromFile, index.hasPreferredDir) ?? + pickCopyPath(index.sources.get(upper), fromFile, index.hasPreferredDir) ?? + null + ); +} diff --git a/gitnexus/src/core/ingestion/languages/cobol/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/cobol/scope-resolver.ts index cfea8f39e..3d9e0eea6 100644 --- a/gitnexus/src/core/ingestion/languages/cobol/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/cobol/scope-resolver.ts @@ -9,74 +9,12 @@ * Reference: `languages/python/scope-resolver.ts`. */ -import path from 'node:path'; import type { ParsedFile } from 'gitnexus-shared'; import { SupportedLanguages } from 'gitnexus-shared'; -import { perFileSet } from '../../import-resolvers/per-file-set.js'; import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers.js'; import type { ScopeResolver } from '../../scope-resolution/contract/scope-resolver.js'; import { cobolProvider } from '../cobol.js'; - -// Copybook file extensions for COPY name resolution -const COPYBOOK_EXTENSIONS = new Set(['.cpy', '.copybook']); -// COBOL source files, searched only after every copybook has missed. -const COBOL_SOURCE_EXTENSIONS = new Set(['.cbl', '.cob', '.cobol']); - -/** - * Uppercased-basename → first file carrying it, one map PER TIER, memoized on - * the `allFilePaths` Set identity (#2908). - * - * `resolveImportTarget` used to run two full workspace scans per `COPY` — one - * for the copybook tier, one for the source tier — each calling `path.extname` - * + `path.basename` + `toUpperCase` on every entry. A `COPY` of a member that - * lives outside the repo (the common case: vendor and system copybooks) missed - * in both, so both scans always ran to completion, making resolution - * O(copies × files). The orchestrator passes the SAME Set to every import in a - * pass (`pipeline/run.ts` builds it once), so a `WeakMap` keyed on that Set - * turns the scans into one build per run. - * - * Two tiers rather than one map is the tie-break, not a stylistic choice: a - * `.cpy`/`.copybook` hit beats a `.cbl`/`.cob`/`.cobol` hit even when the source - * file comes FIRST in Set-iteration order, which is exactly what collapsing the - * tiers into a single first-wins map would silently discard. Within a tier the - * first file in Set-iteration order wins, mirroring the `return` on first match - * in the scans this replaces. - * - * The per-file key is derived with the same `path.extname(fp).toLowerCase()` → - * `path.basename(fp, ext)` → `toUpperCase()` sequence the scans used, including - * its quirk: `path.basename` strips the suffix only on an exact, case-sensitive - * match, so `Foo.CPY` indexes under `FOO.CPY` rather than `FOO`. Node's `path` - * stays in the loop for the same reason — on POSIX it does not treat `\` as a - * separator, and hand-rolled slicing on `/` would start resolving backslash - * paths the scans never resolved. - */ -interface CobolCopyIndex { - /** `.cpy` / `.copybook` files — tier 1. */ - readonly copybooks: ReadonlyMap; - /** `.cbl` / `.cob` / `.cobol` files — tier 2. */ - readonly sources: ReadonlyMap; -} - -const getCobolCopyIndex = perFileSet((allFilePaths: ReadonlySet): CobolCopyIndex => { - const copybooks = new Map(); - const sources = new Map(); - // One pass builds both tiers: the two scans walked the same files and - // classified each by the same extension test. - for (const fp of allFilePaths) { - const ext = path.extname(fp).toLowerCase(); - const tier = COPYBOOK_EXTENSIONS.has(ext) - ? copybooks - : COBOL_SOURCE_EXTENSIONS.has(ext) - ? sources - : undefined; - if (tier === undefined) continue; - const basename = path.basename(fp, ext).toUpperCase(); - // First in Set-iteration order wins, as the scans' first-match `return` did. - if (!tier.has(basename)) tier.set(basename, fp); - } - - return { copybooks, sources }; -}); +import { resolveCobolCopyTarget } from './copy-target.js'; const cobolScopeResolver: ScopeResolver = { language: SupportedLanguages.Cobol, @@ -84,11 +22,9 @@ const cobolScopeResolver: ScopeResolver = { importEdgeReason: 'cobol-scope: copy', // ── Resolve COPY bookname to file path ───────────────────────────── - resolveImportTarget: (targetRaw, _fromFile, allFilePaths) => { - const upper = targetRaw.toUpperCase(); - const index = getCobolCopyIndex(allFilePaths); - // Copybooks first, then COBOL sources — the tier order IS the tie-break. - return index.copybooks.get(upper) ?? index.sources.get(upper) ?? null; + // Shared with the regex processor's resolveCopy (#2967 lockstep). + resolveImportTarget: (targetRaw, fromFile, allFilePaths) => { + return resolveCobolCopyTarget(targetRaw, fromFile, allFilePaths); }, // COBOL has no binding-merge rules beyond the default (local-first-then-imports). diff --git a/gitnexus/test/unit/cobol-copy-external-imports.test.ts b/gitnexus/test/unit/cobol-copy-external-imports.test.ts new file mode 100644 index 000000000..8a7cfb2e1 --- /dev/null +++ b/gitnexus/test/unit/cobol-copy-external-imports.test.ts @@ -0,0 +1,78 @@ +/** + * Live regex-processor pin for COBOL COPY EXTERNAL (#2967). + * + * Census `resolveImportTarget` greening is not enough: `cobol-processor` + * emits `cobol-copy` IMPORTS from `expandCopies` + `resolveCopy`. A vendor + * decoy that the compiler would never search must not get that edge when a + * well-known copybook directory is present; fail-open without one. + */ +import { describe, expect, it } from 'vitest'; + +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; +import { processCobol } from '../../src/core/ingestion/cobol-processor.js'; +import { generateId } from '../../src/lib/utils.js'; + +const PROG = ` IDENTIFICATION DIVISION. + PROGRAM-ID. PROG. + DATA DIVISION. + WORKING-STORAGE SECTION. + COPY EXTERNAL. + COPY CUSTREC. +`; + +const CUSTREC = ` 01 WS-CUSTOMER-DATA. + 05 WS-CUST-CODE PIC X(10). +`; + +const EXTERNAL = ` 01 WS-VENDOR-DECOY PIC X(8). +`; + +function seedFiles( + graph: ReturnType, + files: ReadonlyArray<{ path: string; content: string }>, +): void { + for (const f of files) { + graph.addNode({ + id: generateId('File', f.path), + label: 'File', + properties: { name: f.path, filePath: f.path }, + }); + } +} + +function cobolCopyTargets(graph: ReturnType): string[] { + return graph.relationships + .filter((r) => r.type === 'IMPORTS' && r.reason === 'cobol-copy') + .map((r) => r.targetId) + .sort(); +} + +describe('COBOL processor COPY EXTERNAL does not fabricate vendor IMPORTS (#2967)', () => { + it('emits no cobol-copy IMPORTS onto vendor/EXTERNAL.cpy when copybooks/ is present', () => { + const files = [ + { path: 'copybooks/CUSTREC.cpy', content: CUSTREC }, + { path: 'vendor/EXTERNAL.cpy', content: EXTERNAL }, + { path: 'src/PROG.cbl', content: PROG }, + ]; + const graph = createKnowledgeGraph(); + seedFiles(graph, files); + processCobol(graph, files, new Set(files.map((f) => f.path))); + + const targets = cobolCopyTargets(graph); + expect(targets).toContain(generateId('File', 'copybooks/CUSTREC.cpy')); + expect(targets).not.toContain(generateId('File', 'vendor/EXTERNAL.cpy')); + }); + + it('fail-open: without a copybook dir, COPY EXTERNAL still emits onto vendor/EXTERNAL.cpy', () => { + const files = [ + { path: 'vendor/EXTERNAL.cpy', content: EXTERNAL }, + { path: 'src/PROG.cbl', content: PROG }, + ]; + const graph = createKnowledgeGraph(); + seedFiles(graph, files); + processCobol(graph, files, new Set(files.map((f) => f.path))); + + const targets = cobolCopyTargets(graph); + expect(targets).toContain(generateId('File', 'vendor/EXTERNAL.cpy')); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/cobol-import-target-parity.test.ts b/gitnexus/test/unit/scope-resolution/cobol-import-target-parity.test.ts index b46859c68..186d7efc7 100644 --- a/gitnexus/test/unit/scope-resolution/cobol-import-target-parity.test.ts +++ b/gitnexus/test/unit/scope-resolution/cobol-import-target-parity.test.ts @@ -119,13 +119,16 @@ const STEMS = ['CUSTREC', 'custrec', 'AcctRec', 'PAYROLL', 'BOOK', 'COMMON', 'TA */ const EXTS = ['.cpy', '.copybook', '.CPY', '.cbl', '.cob', '.cobol', '.CBL', '.txt', '']; -function corpus(seed: number, fileCount: number): Set { +/** Directories with no well-known copybook segment — fail-open parity only. */ +const FAIL_OPEN_DIRS = ['', 'src', 'jcl/proclib', 'win\\dir']; + +function corpusFromDirs(seed: number, fileCount: number, dirs: readonly string[]): Set { const files = new Set(); for (let i = 0; i < fileCount; i++) { const a = mix(seed * 7919 + i); const b = mix(a ^ 0x9e3779b9); const c = mix(b ^ 0x85ebca6b); - const dir = DIRS[a % DIRS.length]; + const dir = dirs[a % dirs.length]; const stem = STEMS[b % STEMS.length]; const rel = `${stem}${EXTS[c % EXTS.length]}`; files.add(dir === '' ? rel : `${dir}/${rel}`); @@ -137,6 +140,10 @@ function corpus(seed: number, fileCount: number): Set { return files; } +function corpus(seed: number, fileCount: number): Set { + return corpusFromDirs(seed, fileCount, DIRS); +} + /** * `COPY` operands as they appear in source, plus the spellings that reach the * corpus's awkward files. Lower-case entries are what breaks if the target @@ -166,11 +173,31 @@ const TARGETS = [ const REPOS = 40; +function hasPreferredCopyDir(files: ReadonlySet): boolean { + for (const fp of files) { + const parts = fp.split('/'); + for (let i = 0; i < parts.length - 1; i++) { + if ( + parts[i] === 'copybooks' || + parts[i] === 'COPYBOOKS' || + parts[i] === 'cpy' || + parts[i] === 'copy' + ) { + return true; + } + } + } + return false; +} + describe('COBOL COPY-target index hoist — output parity with the pre-change scans (#2908)', () => { - it('agrees with the verbatim pre-change resolver over the generated corpus', () => { + it('agrees with the verbatim pre-change resolver over fail-open corpora (no well-known copybook dir)', () => { let checked = 0; for (let repo = 0; repo < REPOS; repo++) { - const files = corpus(repo, 6 + (repo % 25)); + const files = corpusFromDirs(repo, 6 + (repo % 25), FAIL_OPEN_DIRS); + expect(hasPreferredCopyDir(files), `repo=${repo} accidentally grew a copybook dir`).toBe( + false, + ); for (const target of TARGETS) { expect(resolve(target, files), `cobol "${target}" repo=${repo}`).toEqual( legacyResolveCobolImportTarget(target, files), @@ -181,6 +208,17 @@ describe('COBOL COPY-target index hoist — output parity with the pre-change sc expect(checked).toBe(REPOS * TARGETS.length); }); + it('COPY EXTERNAL misses vendor/EXTERNAL.cpy when a copybooks/ dir is present (#2967)', () => { + const files = new Set(['copybooks/CUSTREC.cpy', 'vendor/EXTERNAL.cpy', 'src/PROG.cbl']); + expect(resolve('EXTERNAL', files)).toBeNull(); + expect(resolve('CUSTREC', files)).toBe('copybooks/CUSTREC.cpy'); + }); + + it('fail-open: without a copybook dir, COPY EXTERNAL still basename-matches (#2967)', () => { + const files = new Set(['vendor/EXTERNAL.cpy', 'src/PROG.cbl']); + expect(resolve('EXTERNAL', files)).toBe('vendor/EXTERNAL.cpy'); + }); + it('the corpus actually resolves things (the parity arm is not vacuous)', () => { // A corpus that resolved nothing would make the arm above pass on // `null === null` forever. Measured on this corpus: 390 hits. diff --git a/gitnexus/test/unit/scope-resolution/external-import-conformance.test.ts b/gitnexus/test/unit/scope-resolution/external-import-conformance.test.ts index dca6f187d..4f1219308 100644 --- a/gitnexus/test/unit/scope-resolution/external-import-conformance.test.ts +++ b/gitnexus/test/unit/scope-resolution/external-import-conformance.test.ts @@ -358,7 +358,11 @@ const CASES: ReadonlyMap = new Map([ fromFile: 'src/PROG.cbl', resolutionConfig: undefined, external: 'EXTERNAL', - decoy: 'vendor/EXTERNAL.cpy', + // After the copybook-dir preference, vendor/EXTERNAL.cpy is intentionally + // unreachable (that is the #2967 fix). The reachable decoy is the in-repo + // copybook; vendor/EXTERNAL.cpy stays in `files` so EXTERNAL→[] is not a + // vacuous miss of an empty workspace. + decoy: 'copybooks/CUSTREC.cpy', reachesDecoy: 'CUSTREC', }, ], @@ -394,7 +398,6 @@ const KNOWN_GAPS: ReadonlyMap = new Map `Sources/Foundation/Thing.swift`'], [SupportedLanguages.C, '`stdio.h` -> `src/stdio.h`'], [SupportedLanguages.CPlusPlus, '`cstdio.h` -> `src/cstdio.h`'], - [SupportedLanguages.Cobol, '`EXTERNAL` -> `vendor/EXTERNAL.cpy`'], ]); /**