diff --git a/gitnexus/bench/impact-pdg/measure.mjs b/gitnexus/bench/impact-pdg/measure.mjs index 34b83ffe2..42cad12e7 100644 --- a/gitnexus/bench/impact-pdg/measure.mjs +++ b/gitnexus/bench/impact-pdg/measure.mjs @@ -122,23 +122,36 @@ async function analyzeAndImpact(fx, home, { pdgOn = true } = {}) { // The parent process must see the temp GITNEXUS_HOME too — LocalBackend.init() // reads the REAL registry under getGlobalDir() (no mock). A fresh backend per // fixture avoids cross-fixture pool/registry caching. - process.env.GITNEXUS_HOME = home; - const { LocalBackend } = await import( - path.join(REPO_ROOT, 'src', 'mcp', 'local', 'local-backend.ts') - ); - const backend = new LocalBackend(); - await backend.init(); + // + // FIX 8: wrap the post-analyze body in try/finally. The analyze-failure path + // above already cleans `work`; if `backend.callTool()` (or init/import) THROWS + // here, `work` would otherwise leak. On success we return `work` so the caller + // can run its own validation + cleanup; on throw we remove it before rethrow. + let succeeded = false; + try { + process.env.GITNEXUS_HOME = home; + const { LocalBackend } = await import( + path.join(REPO_ROOT, 'src', 'mcp', 'local', 'local-backend.ts') + ); + const backend = new LocalBackend(); + await backend.init(); - const results = {}; - for (const mode of MODES) { - results[mode] = await backend.callTool('impact', { - repo: work, - target: fx.gt.criterion.name, - direction: fx.gt.criterion.direction, - mode, - }); + const results = {}; + for (const mode of MODES) { + results[mode] = await backend.callTool('impact', { + repo: work, + target: fx.gt.criterion.name, + direction: fx.gt.criterion.direction, + mode, + }); + } + succeeded = true; + return { work, results }; + } finally { + // Only clean up on the throwing path — on success the caller owns `work` + // (it runs `validateFixture(fx, work, ...)` then removes it in its finally). + if (!succeeded) fs.rmSync(work, { recursive: true, force: true }); } - return { work, results }; } /** Flatten an impact result's byDepth into canonical symbol keys (the CIS). */ diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index b0b5723ef..7def81066 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -227,6 +227,13 @@ async function projectBlocksToSymbols(deps: { // `(s:Function OR s:Method)` disjunction (unsupported in the LadybugDB // Cypher subset — the established cross-label pattern, see // `enrichCandidateLabels`). `LIMIT` is a small validated int literal. + // FIX 6: do NOT swallow a query failure as `[]`. A DB error (lock / + // corruption / missing path) must NOT masquerade as a genuine no-owning- + // symbol result — that would silently inflate `unresolvedCount` and hide + // the failure. Letting it reject propagates through `Promise.all` → + // `projectBlocksToSymbols` → `_runImpactPDG` → `_impactImpl` up to the + // `impact()` structured-error catch, where it surfaces as a real error + // with a recovery suggestion (rather than a clean-looking partial radius). const rows = await exec( lbugPath, `MATCH (s:\`Function\`) @@ -238,7 +245,7 @@ async function projectBlocksToSymbols(deps: { RETURN s.id AS id, s.name AS name, 'Method' AS label, s.startLine AS startLine LIMIT 8`, { filePath, symStart }, - ).catch(() => [] as any[]); + ); if (rows.length === 0) { // No owning symbol — top-level/free-statement block or a lambda whose @@ -287,6 +294,33 @@ async function projectBlocksToSymbols(deps: { return { symbols: resolved, unresolvedCount, ambiguousCount }; } +/** + * The KTD8 parity fields a PDG impact result carries even when it short-circuits + * to an empty radius (degraded layer / no PDG body / no dependence reachability). + * + * A programmatic consumer iterating `byDepth`, reading `byDepthCounts[1]`, or + * coalescing `affected_processes`/`affected_modules` must find a well-formed + * (empty) shape on EVERY early return, not `undefined` (which would render as + * "isolated"/"no data" instead of "inconclusive"). The CLI branches on + * `pdgLayer` first so it is safe regardless, but the JSON contract must be + * uniform across all three early returns — this single source guarantees that. + */ +function emptyPdgParityFields(): { + byDepth: Record; + byDepthCounts: Record; + summary: { direct: number; processes_affected: number; modules_affected: number }; + affected_processes: unknown[]; + affected_modules: unknown[]; +} { + return { + byDepth: {}, + byDepthCounts: { 1: 0 }, + summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, + affected_processes: [], + affected_modules: [], + }; +} + /** * Assemble the consumer-safe PDG impact result (U4 / KTD8 parity matrix). * @@ -577,17 +611,37 @@ async function pdgLayerStatus(deps: { // Meta unreadable (e.g. a seeded test DB): one bounded probe confirms the // layer status is genuinely undeterminable from the DB. A missing layer is // indistinguishable from an all-linear (edge-free) one (#2188), so whether the - // probe finds a row or not the note stays inconclusive ("status unknown") — - // never the definitive no-layer wording. The probe is bounded (LIMIT 1) and - // anchored on the edge-type discriminator, never an unbounded path scan. - await deps.executeParameterized( - deps.lbugPath, - `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) WHERE r.type IN ['CDG', 'REACHING_DEF'] RETURN r.type AS type LIMIT 1`, - {}, - ); + // probe finds a row or not the state stays `'unknown'` (never the definitive + // no-layer wording). The probe is bounded (`LIMIT 1`) and anchored on the + // BasicBlock→BasicBlock partition (the `(:BasicBlock)…(:BasicBlock)` label pair + // restricts it to the sparse pdg-edge partition, never a global rel scan — the + // established `_explainImpl` anchoring pattern), and it is wrapped so a db-lock + // / missing-path throw degrades to the same `'unknown'` signal rather than + // propagating and losing it. + // + // The probe result is NOT discarded: a visible CDG/REACHING_DEF edge (with + // meta unreadable) is a weak-but-real "edges are present, but completeness is + // unprovable" signal, distinct from "no edges visible at all". Both stay + // `'unknown'` (inconclusive), but the note distinguishes them so the operator + // gets the more useful hint. + let edgesVisible = false; + try { + const rows = await deps.executeParameterized( + deps.lbugPath, + `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) WHERE r.type IN ['CDG', 'REACHING_DEF'] RETURN r.type AS type LIMIT 1`, + {}, + ); + edgesVisible = Array.isArray(rows) && rows.length > 0; + } catch { + // db-lock / missing-path / corrupt probe — fall through as not-visible, but + // keep the `'unknown'` signal (a probe failure must not lose it). + edgesVisible = false; + } return { state: 'unknown', - note: 'PDG layer status unknown — no CDG/REACHING_DEF edges visible and meta is unreadable; was this repo indexed with gitnexus analyze --pdg?', + note: edgesVisible + ? 'PDG layer status unknown — CDG/REACHING_DEF edges ARE visible but meta is unreadable, so the layer cannot be confirmed complete (a partial layer looks the same); was this repo fully indexed with gitnexus analyze --pdg?' + : 'PDG layer status unknown — no CDG/REACHING_DEF edges visible and meta is unreadable; was this repo indexed with gitnexus analyze --pdg?', }; } // AI context generation is CLI-only (gitnexus analyze) @@ -3410,6 +3464,47 @@ export class LocalBackend { }; } + /** + * Build the SAME BasicBlock seed anchor (`anchorClause` + `queryParams`) as + * `resolveBlockAnchor`'s symbol branch, but from an ALREADY-RESOLVED symbol — + * WITHOUT re-running `resolveSymbolCandidates`. + * + * Why this exists (correctness keystone): `_impactImpl` already resolves the + * target to a confident single symbol honoring the caller's + * `target_uid`/`file_path`/`kind` hints. Re-resolving by the bare `sym.name` + * inside `_runImpactPDG` would (a) RE-AMBIGUATE a globally-ambiguous name the + * caller had disambiguated (returning the "ambiguous" early payload instead of + * the PDG result), or (b) anchor the seed on a DIFFERENT same-name symbol in + * another file → a wrong-symbol blast radius. Anchoring directly from the + * resolved `{ filePath, startLine, endLine }` preserves the disambiguation. + * + * The window is byte-identical to `resolveBlockAnchor`'s symbol branch: BOTH + * span bounds are shifted `+1` (1-based BasicBlock `startLine` vs the 0-based + * symbol span — the lower `+1` excludes a neighbor's block on the line above, + * the upper `+1` keeps a guard/def/use on the final line). A symbol with no + * usable span degrades to the same file-level id-prefix filter. This is the + * resolved-symbol counterpart, NOT a second window convention. + */ + private blockAnchorForResolvedSymbol(sym: { + filePath: string; + startLine?: number; + endLine?: number; + }): { anchorClause: string; queryParams: Record } { + const idPrefix = `BasicBlock:${sym.filePath}:`; + if ( + typeof sym.startLine === 'number' && + typeof sym.endLine === 'number' && + sym.endLine >= sym.startLine + ) { + return { + anchorClause: + 'a.id STARTS WITH $idPrefix AND a.startLine >= $symStart AND a.startLine <= $symEnd', + queryParams: { idPrefix, symStart: sym.startLine + 1, symEnd: sym.endLine + 1 }, + }; + } + return { anchorClause: 'a.id STARTS WITH $idPrefix', queryParams: { idPrefix } }; + } + /** * Explain tool (#2083 M3 U6) — persisted taint-finding explanation. * WAL-aware wrapper mirroring `context`. @@ -4801,6 +4896,10 @@ export class LocalBackend { // refactor". UNKNOWN (KTD8) — never LOW (#2129/#1858 false-safe). impactedCount: 0, risk: 'UNKNOWN', + // KTD8 parity: the no-body / no-dependence PDG returns carry these, so + // the degraded return must too — a programmatic consumer iterating + // byDepth / reading byDepthCounts must not get `undefined` here. + ...emptyPdgParityFields(), }; } } @@ -5027,6 +5126,10 @@ export class LocalBackend { id: outcome.symbol.id, name: outcome.symbol.name, filePath: outcome.symbol.filePath, + // Carry the resolved span so the PDG seed anchors on THIS symbol directly, + // without re-resolving its (possibly ambiguous) name (FIX 1). + startLine: outcome.symbol.startLine, + endLine: outcome.symbol.endLine, }; const symType = outcome.resolvedLabel || outcome.symbol.type || ''; @@ -5041,8 +5144,6 @@ export class LocalBackend { direction, maxDepth, limit: Number.isFinite(params.limit) ? params.limit : 100, - offset: Number.isFinite(params.offset) ? params.offset : 0, - summaryOnly: params.summaryOnly, // KTD2 extraction-seam discipline: hand the engine its DB dependency // explicitly rather than `this.`-binding it, so the traversal (U3/U4) // can later move to a standalone `pdg-impact.ts` as a move, not a @@ -5069,23 +5170,6 @@ export class LocalBackend { }); } - /** - * PDG-backed blast radius (`mode:'pdg'`) — STUB (U1). - * - * The real engine (U3/U4) resolves the target symbol to its BasicBlocks, - * runs a direction-aware bounded BFS over the persisted `CDG` + - * `REACHING_DEF` edges (KTD4 truth table, KTD11 query constraints), then - * projects the reachable blocks back to owning symbols and assembles a - * consumer-safe result (KTD8 parity matrix). U1 ships only the param / - * validation surface, so this returns a structured "pending" payload. - * - * KTD2 extraction-seam discipline: written as a method taking its DB - * dependency as an explicit parameter (not reaching back through `this.` for - * the query path) so the traversal can later be lifted to a standalone - * `gitnexus/src/mcp/local/pdg-impact.ts` engine as a *move*, not a rewrite. - * The stub does not yet consume `executeParameterized` — the U3 anchor / - * BFS / projection code joins here. - */ /** * U3 — the PDG blast-radius TRAVERSAL (KTD2, KTD4, KTD6, KTD11). * @@ -5123,13 +5207,11 @@ export class LocalBackend { */ private async _runImpactPDG(deps: { repo: RepoHandle; - sym: { id: string; name: string; filePath: string }; + sym: { id: string; name: string; filePath: string; startLine?: number; endLine?: number }; symType: string; direction: 'upstream' | 'downstream'; maxDepth: number; limit: number; - offset: number; - summaryOnly?: boolean; executeParameterized: typeof executeParameterized; }): Promise { const { repo, sym, direction, maxDepth, executeParameterized: exec } = deps; @@ -5156,14 +5238,16 @@ export class LocalBackend { // Depth: clamp to a sane positive integer (the caller default is 3). const depthBudget = Number.isInteger(maxDepth) && maxDepth >= 1 ? maxDepth : 3; - // ── Seed: resolve the target symbol to its BasicBlocks (KTD2 reuse) ─────── - // resolveBlockAnchor maps the symbol to its block id-prefix + the corrected - // [symStart+1, symEnd+1] line window (do NOT re-derive the 1-based-block / - // 0-based-symbol offset — the helper owns it). `'impact'` toolName widening - // makes any ambiguous-anchor message name THIS tool. - const resolved = await this.resolveBlockAnchor(repo, sym.name, 'impact'); - if (resolved.early) return resolved.early; - const { anchorClause, queryParams } = resolved; + // ── Seed: anchor the target's BasicBlocks from the ALREADY-RESOLVED symbol ─ + // `_impactImpl` already resolved `sym` to a confident single match honoring + // the caller's target_uid/file_path/kind hints. Re-resolving by the bare + // `sym.name` here would RE-AMBIGUATE a disambiguated name (returning the + // "ambiguous" early payload instead of the PDG result) or anchor the seed on + // a DIFFERENT same-name symbol in another file (wrong-symbol blast radius). + // So build the seed anchor DIRECTLY from the resolved symbol's + // [startLine+1, endLine+1] window — the same window `resolveBlockAnchor`'s + // symbol branch produces, without re-running `resolveSymbolCandidates`. + const { anchorClause, queryParams } = this.blockAnchorForResolvedSymbol(sym); const seedRows = await exec( repo.lbugPath, @@ -5173,6 +5257,11 @@ export class LocalBackend { const seedBlocks: string[] = seedRows .map((r: any) => String(r.id ?? r[0] ?? '')) .filter((id: string) => id.length > 0); + // FIX 7: the seed query is `LIMIT $stepLimit`-bounded like every BFS step. + // A function with more seed blocks than `stepLimit` would silently under-seed + // (and thus under-report) — flag it so the result carries the same truncation + // signal the BFS steps do, never a silent partial seed. + const seedTruncated = seedRows.length >= stepLimit; // ── KTD6 no-body contract: distinguish "no PDG body" from "no dependence" ── // A symbol that resolves but produces ZERO anchored blocks has no CFG body @@ -5202,11 +5291,7 @@ export class LocalBackend { // KTD8 parity fields so a consumer iterating byDepth / reading the // depth counts on a no-body result still finds a well-formed (empty) // shape rather than `undefined` (which would render as "isolated"). - byDepth: {} as Record, - byDepthCounts: { 1: 0 } as Record, - summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, - affected_processes: [] as unknown[], - affected_modules: [] as unknown[], + ...emptyPdgParityFields(), unresolvedBlockCount: 0, ambiguousProjectionCount: 0, }; @@ -5223,9 +5308,11 @@ export class LocalBackend { // `truncatedByDepth`: the BFS still had a non-empty frontier when the depth // budget ran out (more reachable blocks exist past `maxDepth`). // `truncatedByLimit`: a single step's neighbour query hit the interpolated - // LIMIT, so that step's expansion is a lower bound. Either flags `truncated`. + // LIMIT, so that step's expansion is a lower bound. The SEED query is + // LIMIT-bounded too, so `seedTruncated` seeds this flag — a partial seed is + // a lower-bound expansion just like a partial step. Either flags `truncated`. let truncatedByDepth = false; - let truncatedByLimit = false; + let truncatedByLimit = seedTruncated; // The endpoint the frontier is matched on, and the endpoint collected, flip // by direction — but the SAME sense applies to BOTH edge types (KTD4). @@ -5296,11 +5383,7 @@ export class LocalBackend { ambiguousProjectionCount: 0, ...(truncated ? { truncated: true } : {}), ...(truncatedBy ? { truncatedBy } : {}), - byDepth: {} as Record, - byDepthCounts: { 1: 0 } as Record, - summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, - affected_processes: [] as unknown[], - affected_modules: [] as unknown[], + ...emptyPdgParityFields(), }; } diff --git a/gitnexus/test/integration/impact-pdg-shape.test.ts b/gitnexus/test/integration/impact-pdg-shape.test.ts index 2f6e365b8..dd899659c 100644 --- a/gitnexus/test/integration/impact-pdg-shape.test.ts +++ b/gitnexus/test/integration/impact-pdg-shape.test.ts @@ -251,6 +251,100 @@ withTestLbugDB( }); }); + // ── FIX 1 keystone: the PDG seed anchors on the ALREADY-RESOLVED symbol ─── + // The seed must NOT be re-resolved by bare `sym.name` inside `_runImpactPDG` + // (that would re-ambiguate a file_path/uid-disambiguated name, or anchor on a + // DIFFERENT same-name symbol → wrong-symbol blast radius). With two functions + // named `sameName` in different files, disambiguating by file_path/target_uid + // must (a) produce the CORRECT file's blast radius, and (b) NEVER fall into + // the callgraph `_runImpactBFS` fan-out. + describe('seed anchors on the resolved (disambiguated) symbol, not a name re-resolution', () => { + it('file_path disambiguation reaches the right file’s downstream owner (not the other same-name fn)', async () => { + const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS'); + try { + const result = await backend.callTool('impact', { + target: 'sameName', + file_path: 'src/b.ts', + direction: 'downstream', + mode: 'pdg', + }); + // Resolved cleanly (NOT the ambiguous early payload) and it is a PDG result. + expect(result.status).not.toBe('ambiguous'); + expect(result.mode).toBe('pdg'); + expect(result.error).toBeUndefined(); + // The target is the B-file `sameName`, and the blast radius reflects B's + // downstream owner `onlyB` — NEVER A's `onlyA` (the wrong-file anchor). + expect(result.target.id).toBe('func:sameB'); + expect(result.target.filePath).toBe('src/b.ts'); + const names = new Set( + Object.values(result.byDepth as Record) + .flat() + .map((i: any) => i.name), + ); + expect(names.has('onlyB')).toBe(true); + expect(names.has('onlyA')).toBe(false); + // KTD5: no callgraph engine ran under the pdg call. + expect(bfsSpy).not.toHaveBeenCalled(); + } finally { + bfsSpy.mockRestore(); + } + }); + + it('target_uid disambiguation anchors the seed on THAT uid (not a re-ambiguation)', async () => { + const bfsSpy = vi.spyOn(backend as any, '_runImpactBFS'); + try { + const result = await backend.callTool('impact', { + target: 'sameName', + target_uid: 'func:sameA', + direction: 'downstream', + mode: 'pdg', + }); + expect(result.status).not.toBe('ambiguous'); + expect(result.mode).toBe('pdg'); + expect(result.target.id).toBe('func:sameA'); + expect(result.target.filePath).toBe('src/a.ts'); + const names = new Set( + Object.values(result.byDepth as Record) + .flat() + .map((i: any) => i.name), + ); + expect(names.has('onlyA')).toBe(true); + expect(names.has('onlyB')).toBe(false); + expect(bfsSpy).not.toHaveBeenCalled(); + } finally { + bfsSpy.mockRestore(); + } + }); + }); + + // ── fnFileOf Windows-path coverage (split-from-right) ───────────────────── + // The block→owning-symbol projector (`projectBlocksToSymbols`) recovers each + // block's file path via `fnFileOf`, which must split a `BasicBlock` id FROM + // THE RIGHT so a Windows drive-letter ':' inside the path is not mistaken for + // a segment delimiter. Exercised behaviorally (fnFileOf is module-scope, not + // exported) — mirrors how pdg-query.test.ts pins `fnLineOf` with a `C:/...` id. + describe('fnFileOf recovers a Windows-style (drive-colon) path', () => { + it('projects a downstream block whose id carries a C:/ drive path to its owning symbol', async () => { + const result = await backend.callTool('impact', { + target: 'winFn', + direction: 'downstream', + mode: 'pdg', + }); + expect(result.error).toBeUndefined(); + expect(result.mode).toBe('pdg'); + const items = Object.values(result.byDepth as Record).flat(); + // The downstream Windows block `BasicBlock:C:/src/win.ts:21:0:0` owns + // `winUse` (0-based startLine 20). If `fnFileOf` split from the LEFT it + // would yield `C` (the drive letter) as the path and fail to resolve the + // owning symbol — surfacing it as unresolved instead. Correct split-from- + // right recovers `C:/src/win.ts` and resolves `winUse`. + const winUse = items.find((i: any) => i.name === 'winUse'); + expect(winUse).toBeDefined(); + expect(winUse.id).toBe('func:winUse'); + expect(winUse.filePath).toBe('C:/src/win.ts'); + }); + }); + // ── No-body symbol parity (KTD6 × KTD8) ─────────────────────────────────── describe('no-body symbol still yields a parity-shaped (non-LOW) result', () => { it('an interface (no CFG body) returns the no-body note + well-formed empty shape', async () => { @@ -329,6 +423,75 @@ withTestLbugDB( await edge('CDG', K2, T, 'T'); await edge('CDG', T, U, 'T'); + // ── Windows drive-colon path fixture (exercises fnFileOf split-from-right) ─ + // Separate file `C:/src/win.ts`, isolated from `target`'s graph so the + // existing impactedCount/byDepth assertions are untouched. `winFn` is its + // own seed target; a downstream RD block (`:21:`) owns `winUse`. + const WF = 'C:/src/win.ts'; + const winFnSeed = `BasicBlock:${WF}:11:0:0`; // winFn@0-based[10,12] ⇒ window [11,13] + const winUseBlk = `BasicBlock:${WF}:21:0:0`; // winUse@0-based[20,20] ⇒ fnLine 21 + const winNode = ( + id: string, + name: string, + startLine: number, + endLine: number, + type: 'Function' = 'Function', + ) => + adapter.executePrepared( + `CREATE (n:${type} {id: $id, name: $name, filePath: $filePath, startLine: $startLine, endLine: $endLine, isExported: true, content: 'x', description: 'win fixture'})`, + { id, name, filePath: WF, startLine, endLine }, + ); + const winBlock = (id: string, startLine: number, text: string) => + adapter.executePrepared( + `CREATE (b:BasicBlock {id: $id, filePath: $filePath, startLine: $startLine, endLine: $startLine, text: $text})`, + { id, filePath: WF, startLine, text }, + ); + await winNode('func:winFn', 'winFn', 10, 12); + await winNode('func:winUse', 'winUse', 20, 20); + await winBlock(winFnSeed, 11, 'const w = win();'); + await winBlock(winUseBlk, 21, 'useWin(w);'); + await edge('REACHING_DEF', winFnSeed, winUseBlk, 'w'); + + // ── Same-name-in-different-files fixture (FIX 1 keystone: seed must anchor + // on the file_path/uid-disambiguated symbol, NOT re-resolve by bare name) ── + // Two functions BOTH named `sameName`, in `src/a.ts` and `src/b.ts`, each + // with a DISTINCT downstream owner (`onlyA` vs `onlyB`). A correct seed + // (anchored on the already-resolved symbol's file+span) reaches only the + // chosen file's downstream block; a re-resolution by bare `sameName` would + // either re-ambiguate or anchor on the wrong file. + const AFILE = 'src/a.ts'; + const BFILE = 'src/b.ts'; + const sameSeedA = `BasicBlock:${AFILE}:11:0:0`; // sameName@A 0-based[10,10] ⇒ window [11,11] + const onlyABlk = `BasicBlock:${AFILE}:21:0:0`; // onlyA@0-based[20,20] ⇒ fnLine 21 + const sameSeedB = `BasicBlock:${BFILE}:11:0:0`; // sameName@B 0-based[10,10] ⇒ window [11,11] + const onlyBBlk = `BasicBlock:${BFILE}:21:0:0`; // onlyB@0-based[20,20] ⇒ fnLine 21 + const node2 = ( + id: string, + name: string, + filePath: string, + startLine: number, + endLine: number, + ) => + adapter.executePrepared( + `CREATE (n:Function {id: $id, name: $name, filePath: $filePath, startLine: $startLine, endLine: $endLine, isExported: true, content: 'x', description: 'samename fixture'})`, + { id, name, filePath, startLine, endLine }, + ); + const block2 = (id: string, filePath: string, startLine: number, text: string) => + adapter.executePrepared( + `CREATE (b:BasicBlock {id: $id, filePath: $filePath, startLine: $startLine, endLine: $startLine, text: $text})`, + { id, filePath, startLine, text }, + ); + await node2('func:sameA', 'sameName', AFILE, 10, 10); + await node2('func:onlyA', 'onlyA', AFILE, 20, 20); + await node2('func:sameB', 'sameName', BFILE, 10, 10); + await node2('func:onlyB', 'onlyB', BFILE, 20, 20); + await block2(sameSeedA, AFILE, 11, 'const a = mk();'); + await block2(onlyABlk, AFILE, 21, 'useA(a);'); + await block2(sameSeedB, BFILE, 11, 'const b = mk();'); + await block2(onlyBBlk, BFILE, 21, 'useB(b);'); + await edge('REACHING_DEF', sameSeedA, onlyABlk, 'a'); + await edge('REACHING_DEF', sameSeedB, onlyBBlk, 'b'); + vi.mocked(listRegisteredRepos).mockResolvedValue([ { name: 'shape-repo', diff --git a/gitnexus/test/integration/impact-pdg-traversal.test.ts b/gitnexus/test/integration/impact-pdg-traversal.test.ts index 859dde3ba..0afa96ba4 100644 --- a/gitnexus/test/integration/impact-pdg-traversal.test.ts +++ b/gitnexus/test/integration/impact-pdg-traversal.test.ts @@ -106,7 +106,7 @@ withTestLbugDB( expect(set).not.toContain(D2); }); - it('downstream CDG reaches the controlled blocks', async () => { + it('downstream CDG reaches the controlled blocks, never the controller', async () => { const result = await backend.callTool('impact', { target: 'target', direction: 'downstream', @@ -116,6 +116,15 @@ withTestLbugDB( // forward over CDG: S controls K1 controls K2. expect(set).toContain(K1); expect(set).toContain(K2); + // CDG-precision exclusion: the CONTROLLER `P` (the CDG predecessor of S) + // must NOT appear downstream — it is an *upstream* block. A CDG + // direction/precision bug (e.g. traversing the CDG edge in reverse) would + // leak P into the downstream set; pin it so such a bug fails here. + // (D1/D2 are legitimately present downstream via the REACHING_DEF edges — + // the combined CDG+RD frontier is one direction over BOTH edge types — so + // the meaningful CDG-precision exclusion is the controller, not the RD + // uses; the combined-frontier test below pins the exact {D1,D2,K1,K2} set.) + expect(set).not.toContain(P); }); it('upstream CDG reaches the controller, not the controlled blocks', async () => { diff --git a/gitnexus/test/unit/security.test.ts b/gitnexus/test/unit/security.test.ts index 37cb9f6f8..906ae305e 100644 --- a/gitnexus/test/unit/security.test.ts +++ b/gitnexus/test/unit/security.test.ts @@ -73,6 +73,10 @@ describe('VALID_RELATION_TYPES', () => { // types" sweep can't drag them in, mirroring the TAINTED/TAINT_PATH pins. expect(VALID_RELATION_TYPES.has('CDG')).toBe(false); expect(VALID_RELATION_TYPES.has('POST_DOMINATE')).toBe(false); + // REACHING_DEF is the other BasicBlock→BasicBlock PDG edge (#2086 impact + // PDG mode traverses it directly, never through impact's symbol-space BFS). + // Pinned alongside CDG/POST_DOMINATE so an allow-all sweep can't drag it in. + expect(VALID_RELATION_TYPES.has('REACHING_DEF')).toBe(false); }); });