diff --git a/.claude/skills/gitnexus/gitnexus-guide/SKILL.md b/.claude/skills/gitnexus/gitnexus-guide/SKILL.md index a70279222..7f90f4e6d 100644 --- a/.claude/skills/gitnexus/gitnexus-guide/SKILL.md +++ b/.claude/skills/gitnexus/gitnexus-guide/SKILL.md @@ -36,10 +36,11 @@ For any task involving code understanding, debugging, impact analysis, or refact | `context` | 360-degree symbol view — categorized refs, processes it participates in | | `impact` | Symbol blast radius — what breaks at depth 1/2/3 with confidence | | `detect_changes` | Git-diff impact — what do your current changes affect | -| `check` | Check graph invariants such as circular imports | | `rename` | Multi-file coordinated rename with confidence-tagged edits | | `cypher` | Raw graph queries (read `gitnexus://repo/{name}/schema` first) | | `explain` | Persisted taint findings — source→sink data flows (needs `analyze --pdg`) | +| `pdg_query` | Control/data dependence — what gates X (CDG) / where Y flows (REACHING_DEF); needs `analyze --pdg` | +| `check` | Check graph invariants such as circular imports | | `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | ### Paginating `list_repos` @@ -83,6 +84,15 @@ Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). A repo indexed without `--pdg` returns a clear "no taint layer" note. Caveats: findings are intra-procedural only — cross-function, closure/callback, property/field, and implicit flows are not modeled, so the absence of a finding is **not** proof of safety. `SANITIZES` (sanitizer-kill) edges are queryable via `cypher`. +### Control & data dependence (`pdg_query`) + +`pdg_query` reads the control/data-dependence layers `gitnexus analyze --pdg` records (CDG + REACHING_DEF, basic-block granular) — the control/data analog of `explain`. It is **always anchored** (a `target` file path or symbol, resolved like `context`) and has two modes: + +- `pdg_query { mode: "controls", target: "..." }` — CDG: "under what condition does X run?". Each edge is a controlling predicate block → dependent block with the branch sense (`'T'`/`'F'`) in `reason`; an edge into an early `return`/`throw` is flagged `guard: true` (guard-clause discovery — the sense depends on the predicate, so don't filter guards by a fixed label). +- `pdg_query { mode: "flows", target: "...", variable?: "..." }` — REACHING_DEF def→use edges within the function; pass `variable` to trace one binding. + +A repo indexed without `--pdg` returns a "no PDG layer" note (or "status unknown" when the layer can't be confirmed). Intra-procedural only — cross-function flow is taint's domain (`explain`). The raw CDG/REACHING_DEF edges are also queryable via `cypher`. See the `gitnexus-pdg-query` skill for the full query surface. + ## Resources Reference Lightweight reads (~100-500 tokens) for navigation: diff --git a/.claude/skills/gitnexus/gitnexus-pdg-query/SKILL.md b/.claude/skills/gitnexus/gitnexus-pdg-query/SKILL.md new file mode 100644 index 000000000..f2fcd7d3b --- /dev/null +++ b/.claude/skills/gitnexus/gitnexus-pdg-query/SKILL.md @@ -0,0 +1,89 @@ +--- +name: gitnexus-pdg-query +description: "Use when querying or extending GitNexus's PDG control/data-dependence surface (the `pdg_query` MCP tool, CDG/REACHING_DEF edges), or reasoning about \"what controls X\" / \"where does Y flow\" / guard clauses. Examples: \"what guards this statement?\", \"trace this variable within the function\", \"why is the pdg_query result empty?\", \"add a CDG query\"." +--- + +# PDG query surface with GitNexus + +Expert knowledge for the `pdg_query` MCP tool and the control/data-dependence +edges it reads — the opt-in `--pdg` program-dependence layers. Read this before +touching `gitnexus/src/mcp/local/local-backend.ts` (`_pdgQueryImpl`) or the +`pdg_query` tool def, or when explaining a `pdg_query` result. + +## When to Use + +- "Under what condition does this statement run?" (guarding predicates). +- "Where does this variable flow inside the function?" (def→use). +- Guard-clause discovery (early-return guards — subsumes the #559 heuristic). +- Extending or reviewing `pdg_query` / the CDG / REACHING_DEF read path. +- Debugging an empty or surprising `pdg_query` result. + +## The layered substrate (build order) + +`pdg_query` runs **on** the same graph taint runs on. Each layer is opt-in +behind `--pdg`; a default `analyze` run records none of them (byte-identical). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L5 CDG Ferrante control dependence (post-dominators) (M5 #2085) +``` + +All three are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table +(keyed by the `type` property). There is **no** `Function → BasicBlock` edge. + +## The two modes + +- `pdg_query({ mode: 'controls', target })` — CDG. For the anchored function, + each edge: controlling predicate block → dependent block + branch sense in + `label` (`'T'` = predicate's true/taken arm, `'F'` = false/fall-through). An + edge into an early-return/throw block is flagged `guard: true`. +- `pdg_query({ mode: 'flows', target, variable? })` — REACHING_DEF def→use + edges; `variable` filters to one binding. + +`target` is **required** — a file path or a symbol/function name (resolved like +`context()`). There is no anchorless mode (see below). + +## The corrected guard-clause Cypher + +The RFC #567 §2 form (`[:CDG {label:'F'}]`) does **not** run as written. Edges +are values of the single `CodeRelation` table's `type` property, and the branch +sense is in `reason`, NOT a `label` column: + +```cypher +MATCH (pred:BasicBlock)-[r:CodeRelation {type: 'CDG'}]->(dep:BasicBlock) +WHERE dep.text STARTS WITH 'return' OR dep.text STARTS WITH 'throw' +RETURN pred.startLine, r.reason AS branch, dep.startLine, dep.text +``` + +`r.reason` is the sense the predicate took to reach the early exit. For +`if (!ok) return;` the return rides the predicate's **true** arm (`'T'`) and the +protected body rides the **false** arm (`'F'`) — polarity depends on the guard, +so don't hard-code one sense. + +## Gotchas (the load-bearing ones) + +- **Always anchored + LIMIT-bounded.** LadybugDB has no rel-property index, so + an unanchored `[:CDG*]`/`[:REACHING_DEF*]` path scan is unbounded. `pdg_query` + requires `target` and bounds the page; raw `cypher` callers must anchor on a + file id-prefix or symbol span themselves. +- **BasicBlock↔symbol join is reconstructed.** No `Function→BasicBlock` edge: + the block is matched by its id-prefix (`BasicBlock:::…`) + plus `startLine` within the symbol's span. BasicBlock `startLine` is **1-based** + while the symbol node's `startLine`/`endLine` are **0-based**, so **both** bounds + are shifted `+1` (`[symStart+1, symEnd+1]`): the upper `+1` keeps a guard/def/use + on the function's **final line**, the lower `+1` excludes an adjacent function's + block on the line directly **above**. Same-line / nested functions anchor coarsely. +- **No PDG layer ⇒ a note, not an error.** If the repo wasn't indexed with + `--pdg` the tool returns `{ results: [], note: "no PDG layer …" }` (cheap meta + probe on `RepoMeta.pdg.maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`). +- **CDG labels are binary in M5/M6.** Every `switch`-case arm is `'T'`; per-case + conditions are not yet distinguished. +- **Intra-procedural only.** Cross-function flow is taint's domain (`explain`). + +## Mirror, don't fork + +`_pdgQueryImpl` is the front half of `_explainImpl` (WAL wrapper, meta no-layer +probe, limit validation, `resolveSymbolCandidates` anchoring) with CDG/ +REACHING_DEF instead of TAINTED — and none of taint's path-codec / interproc +`TAINT_PATH` machinery. Reuse those shared helpers; do not re-implement them. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b3319f172..4aa854a88 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -41,6 +41,8 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). | `route_map` | API route → handler → consumer mappings | | `tool_map` | MCP/RPC tool definitions and handlers | | `shape_check` | Response shape vs consumer property access mismatches | +| `explain` | Persisted taint findings (source→sink data flows) — needs `analyze --pdg` | +| `pdg_query` | Control/data dependence — CDG (`mode: controls`) / REACHING_DEF (`mode: flows`) — needs `analyze --pdg` | | `group_list` | List repo groups or details for one group | | `group_sync` | Rebuild group Contract Registry (`contracts.json`) and bridge graph | @@ -204,9 +206,17 @@ Language-agnostic scope-resolution resolver. This is the resolution path for eve Orchestrator: `runScopeResolution(input, provider)` in `scope-resolution/pipeline/run.ts`. Pipeline phase: `scopeResolutionPhase` in `scope-resolution/pipeline/phase.ts` — iterates the registered `SCOPE_RESOLVERS` over the worker-serialized `ParsedFile`s. (Per-language `emitScopeCaptures` hooks may reuse a cached Tree via the orchestrator's `treeCache`, but in worker-pool runs that cache is empty — Trees can't cross MessageChannels — so they consume the pre-extracted `ParsedFile` instead; § Performance notes.) -### Optional CFG/PDG emission (`--pdg`, #2081 M1) +### Optional CFG/PDG emission (`--pdg`, #2081–#2086) -On a `--pdg` run, the parse worker builds a per-function control-flow graph from the tree-sitter AST (`LanguageProvider.cfgVisitor`; TypeScript/JavaScript in M1) and serializes it onto `ParsedFile.cfgSideChannel` as plain data. Scope-resolution then emits `BasicBlock` nodes + `CFG` edges from that side-channel **inside Phase 4 of `runScopeResolution`, while the disk-backed ParsedFile store is still live** — the only window where the worker-built CFGs are loaded (the store is cleared right after the phase returns). A standalone post-`mro` phase would read an empty store, so the CFG emit deliberately lives in-phase, mirroring the `applyCaptureSideChannel` pattern. The opt-in is off by default (graph byte-identical), folded into the parse-cache key (a pdg-off warm cache is never reused on a `--pdg` run), and bounded by a per-function edge cap that logs any dropped edges. Edge *kind* (`seq`/`cond-true`/`loop-back`/…) rides in the `CFG` relationship's `reason` (CFG is a single `CodeRelation` type, not one type per kind). See `core/ingestion/cfg/`. +On a `--pdg` run the parse worker builds a per-function control-flow graph from the tree-sitter AST (`LanguageProvider.cfgVisitor`; TypeScript/JavaScript today) and serializes it onto `ParsedFile.cfgSideChannel` as plain data. Scope-resolution then emits the program-dependence layers from that side-channel **inside Phase 4 of `runScopeResolution`, while the disk-backed ParsedFile store is still live** — the only window where the worker-built CFGs are loaded (the store is cleared right after the phase returns). A standalone post-`mro` phase would read an empty store, so the emit deliberately lives in-phase, mirroring the `applyCaptureSideChannel` pattern. The opt-in is off by default (graph byte-identical), folded into the parse-cache key (a pdg-off warm cache is never reused on a `--pdg` run), and each layer is bounded by a per-function edge cap that logs any dropped edges. All layers are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table, keyed by `type`; there is **no** `Function → BasicBlock` edge — the symbol↔block join is reconstructed from the BasicBlock id prefix + line span. The layers build on each other: + +- **M1 — CFG** (#2081): `BasicBlock` nodes + `CFG` edges. Edge *kind* (`seq`/`cond-true`/`loop-back`/…) rides the `reason` column (CFG is one `CodeRelation` type, not one per kind). +- **M2 — REACHING_DEF** (#2082): GEN/KILL def→use data dependence from a pure fixpoint solver; the variable name rides `reason`. +- **M3/M4 — TAINTED / SANITIZES / TAINT_PATH** (#2083–#2084): intra- and inter-procedural taint (source→sink) — the `explain` tool's data. +- **M5 — CDG** (#2085): Ferrante control dependence over a Cooper–Harvey–Kennedy post-dominator tree (the EXIT-rooted reverse CFG); branch sense (`'T'`/`'F'`) rides `reason`. A CFG whose EXIT is unreachable from some block is skipped for CDG (post-dominance would be unsound) while its CFG/REACHING_DEF layers are kept. +- **M6 — read surface** (#2086): the `pdg_query` MCP tool answers "what gates X?" (CDG, `mode: controls`) and "where does Y flow?" (REACHING_DEF, `mode: flows`); `explain` is the taint consumer. Both are always anchored + `LIMIT`-bounded (LadybugDB has no rel-property index) and share one `resolveBlockAnchor` helper. These PDG edge types are deliberately kept out of the default `VALID_RELATION_TYPES` / web schema. + +See `core/ingestion/cfg/` (emit + the pure CFG / post-dominator / control-dependence / reaching-defs / taint passes) and `mcp/local/local-backend.ts` (`_pdgQueryImpl`, `_explainImpl`, the shared `resolveBlockAnchor`). ### `ScopeResolver` contract @@ -383,6 +393,8 @@ Defined in `lbug/schema.ts`. Separate node tables per type, single `CodeRelation **Relation types** (`CodeRelation.type`): CONTAINS, DEFINES, CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, ACCESSES, METHOD_OVERRIDES, METHOD_IMPLEMENTS, MEMBER_OF, STEP_IN_PROCESS, HANDLES_ROUTE, FETCHES, HANDLES_TOOL, ENTRY_POINT_OF. +**Optional `--pdg` additions** (off by default, opt-in via `gitnexus analyze --pdg`; see _Optional CFG/PDG emission_ above): a `BasicBlock` node table, plus the PDG relation types `CFG`, `REACHING_DEF`, `CDG`, `TAINTED`, `SANITIZES`, and `TAINT_PATH` on the same `CodeRelation` table. These are deliberately kept out of the default `VALID_RELATION_TYPES` / web graph schema — query them via `cypher`, `explain`, or `pdg_query`. + ## Embeddings and search **Embeddings** (`src/core/embeddings/`): Snowflake arctic-embed-xs (384D). Embeddable: File, Function, Class, Method, Interface. Incremental via SHA1 content hash. Separate `Embedding` table. diff --git a/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md index a71429f32..7f90f4e6d 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md @@ -39,6 +39,8 @@ For any task involving code understanding, debugging, impact analysis, or refact | `rename` | Multi-file coordinated rename with confidence-tagged edits | | `cypher` | Raw graph queries (read `gitnexus://repo/{name}/schema` first) | | `explain` | Persisted taint findings — source→sink data flows (needs `analyze --pdg`) | +| `pdg_query` | Control/data dependence — what gates X (CDG) / where Y flows (REACHING_DEF); needs `analyze --pdg` | +| `check` | Check graph invariants such as circular imports | | `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | ### Paginating `list_repos` @@ -82,6 +84,15 @@ Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). A repo indexed without `--pdg` returns a clear "no taint layer" note. Caveats: findings are intra-procedural only — cross-function, closure/callback, property/field, and implicit flows are not modeled, so the absence of a finding is **not** proof of safety. `SANITIZES` (sanitizer-kill) edges are queryable via `cypher`. +### Control & data dependence (`pdg_query`) + +`pdg_query` reads the control/data-dependence layers `gitnexus analyze --pdg` records (CDG + REACHING_DEF, basic-block granular) — the control/data analog of `explain`. It is **always anchored** (a `target` file path or symbol, resolved like `context`) and has two modes: + +- `pdg_query { mode: "controls", target: "..." }` — CDG: "under what condition does X run?". Each edge is a controlling predicate block → dependent block with the branch sense (`'T'`/`'F'`) in `reason`; an edge into an early `return`/`throw` is flagged `guard: true` (guard-clause discovery — the sense depends on the predicate, so don't filter guards by a fixed label). +- `pdg_query { mode: "flows", target: "...", variable?: "..." }` — REACHING_DEF def→use edges within the function; pass `variable` to trace one binding. + +A repo indexed without `--pdg` returns a "no PDG layer" note (or "status unknown" when the layer can't be confirmed). Intra-procedural only — cross-function flow is taint's domain (`explain`). The raw CDG/REACHING_DEF edges are also queryable via `cypher`. See the `gitnexus-pdg-query` skill for the full query surface. + ## Resources Reference Lightweight reads (~100-500 tokens) for navigation: diff --git a/gitnexus-claude-plugin/skills/gitnexus-pdg-query/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-pdg-query/SKILL.md new file mode 100644 index 000000000..f2fcd7d3b --- /dev/null +++ b/gitnexus-claude-plugin/skills/gitnexus-pdg-query/SKILL.md @@ -0,0 +1,89 @@ +--- +name: gitnexus-pdg-query +description: "Use when querying or extending GitNexus's PDG control/data-dependence surface (the `pdg_query` MCP tool, CDG/REACHING_DEF edges), or reasoning about \"what controls X\" / \"where does Y flow\" / guard clauses. Examples: \"what guards this statement?\", \"trace this variable within the function\", \"why is the pdg_query result empty?\", \"add a CDG query\"." +--- + +# PDG query surface with GitNexus + +Expert knowledge for the `pdg_query` MCP tool and the control/data-dependence +edges it reads — the opt-in `--pdg` program-dependence layers. Read this before +touching `gitnexus/src/mcp/local/local-backend.ts` (`_pdgQueryImpl`) or the +`pdg_query` tool def, or when explaining a `pdg_query` result. + +## When to Use + +- "Under what condition does this statement run?" (guarding predicates). +- "Where does this variable flow inside the function?" (def→use). +- Guard-clause discovery (early-return guards — subsumes the #559 heuristic). +- Extending or reviewing `pdg_query` / the CDG / REACHING_DEF read path. +- Debugging an empty or surprising `pdg_query` result. + +## The layered substrate (build order) + +`pdg_query` runs **on** the same graph taint runs on. Each layer is opt-in +behind `--pdg`; a default `analyze` run records none of them (byte-identical). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L5 CDG Ferrante control dependence (post-dominators) (M5 #2085) +``` + +All three are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table +(keyed by the `type` property). There is **no** `Function → BasicBlock` edge. + +## The two modes + +- `pdg_query({ mode: 'controls', target })` — CDG. For the anchored function, + each edge: controlling predicate block → dependent block + branch sense in + `label` (`'T'` = predicate's true/taken arm, `'F'` = false/fall-through). An + edge into an early-return/throw block is flagged `guard: true`. +- `pdg_query({ mode: 'flows', target, variable? })` — REACHING_DEF def→use + edges; `variable` filters to one binding. + +`target` is **required** — a file path or a symbol/function name (resolved like +`context()`). There is no anchorless mode (see below). + +## The corrected guard-clause Cypher + +The RFC #567 §2 form (`[:CDG {label:'F'}]`) does **not** run as written. Edges +are values of the single `CodeRelation` table's `type` property, and the branch +sense is in `reason`, NOT a `label` column: + +```cypher +MATCH (pred:BasicBlock)-[r:CodeRelation {type: 'CDG'}]->(dep:BasicBlock) +WHERE dep.text STARTS WITH 'return' OR dep.text STARTS WITH 'throw' +RETURN pred.startLine, r.reason AS branch, dep.startLine, dep.text +``` + +`r.reason` is the sense the predicate took to reach the early exit. For +`if (!ok) return;` the return rides the predicate's **true** arm (`'T'`) and the +protected body rides the **false** arm (`'F'`) — polarity depends on the guard, +so don't hard-code one sense. + +## Gotchas (the load-bearing ones) + +- **Always anchored + LIMIT-bounded.** LadybugDB has no rel-property index, so + an unanchored `[:CDG*]`/`[:REACHING_DEF*]` path scan is unbounded. `pdg_query` + requires `target` and bounds the page; raw `cypher` callers must anchor on a + file id-prefix or symbol span themselves. +- **BasicBlock↔symbol join is reconstructed.** No `Function→BasicBlock` edge: + the block is matched by its id-prefix (`BasicBlock:::…`) + plus `startLine` within the symbol's span. BasicBlock `startLine` is **1-based** + while the symbol node's `startLine`/`endLine` are **0-based**, so **both** bounds + are shifted `+1` (`[symStart+1, symEnd+1]`): the upper `+1` keeps a guard/def/use + on the function's **final line**, the lower `+1` excludes an adjacent function's + block on the line directly **above**. Same-line / nested functions anchor coarsely. +- **No PDG layer ⇒ a note, not an error.** If the repo wasn't indexed with + `--pdg` the tool returns `{ results: [], note: "no PDG layer …" }` (cheap meta + probe on `RepoMeta.pdg.maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`). +- **CDG labels are binary in M5/M6.** Every `switch`-case arm is `'T'`; per-case + conditions are not yet distinguished. +- **Intra-procedural only.** Cross-function flow is taint's domain (`explain`). + +## Mirror, don't fork + +`_pdgQueryImpl` is the front half of `_explainImpl` (WAL wrapper, meta no-layer +probe, limit validation, `resolveSymbolCandidates` anchoring) with CDG/ +REACHING_DEF instead of TAINTED — and none of taint's path-codec / interproc +`TAINT_PATH` machinery. Reuse those shared helpers; do not re-implement them. diff --git a/gitnexus-shared/src/graph/types.ts b/gitnexus-shared/src/graph/types.ts index 86abc9eba..085c27d03 100644 --- a/gitnexus-shared/src/graph/types.ts +++ b/gitnexus-shared/src/graph/types.ts @@ -157,7 +157,21 @@ export type RelationshipType = | 'SANITIZES' /** Materialized source→sink taint path. Working name — final name/representation * is confirmed when M3/M4 emits it; no persisted edge exists before then. */ - | 'TAINT_PATH'; + | 'TAINT_PATH' + /** Control-dependence edge (PDG, issue #2085 M5): block `dependent` (target) + * executes only because the branch at block `controller` (source) took a + * given side. The branch sense (`'T'` | `'F'`) rides the relation's existing + * `reason` column — mirroring how `CFG` stores its edge kind there — since + * the single `CodeRelation` table has no dedicated label column. */ + | 'CDG' + /** Debug-only post-dominator-tree edge (#2085 M5): a block → its immediate + * post-dominator, emitted behind the `GITNEXUS_PDG_EMIT_POST_DOMINATE` env + * flag for inspection. Never emitted in a normal `--pdg` run. Note: as a + * member of this exported union it is a forward-compatibility commitment — + * removing it later is a breaking schema change — and it is deliberately + * excluded from `VALID_RELATION_TYPES` so it never enters impact-style + * symbol-space traversal (same posture as the taint substrate edges). */ + | 'POST_DOMINATE'; export interface GraphNode { id: string; diff --git a/gitnexus-shared/src/lbug/schema-constants.ts b/gitnexus-shared/src/lbug/schema-constants.ts index d022ba5c4..875f74d2e 100644 --- a/gitnexus-shared/src/lbug/schema-constants.ts +++ b/gitnexus-shared/src/lbug/schema-constants.ts @@ -77,6 +77,12 @@ export const REL_TYPES = [ 'TAINTED', 'SANITIZES', 'TAINT_PATH', + // Control dependence (PDG, issue #2085 M5) — CDG carries its 'T'|'F' branch + // label in the relation's `reason` column; POST_DOMINATE is debug-only + // (behind GITNEXUS_PDG_EMIT_POST_DOMINATE). Both are BasicBlock→BasicBlock, + // reusing the existing FROM BasicBlock TO BasicBlock pair in RELATION_SCHEMA. + 'CDG', + 'POST_DOMINATE', ] as const; export type RelType = (typeof REL_TYPES)[number]; diff --git a/gitnexus/skills/gitnexus-guide.md b/gitnexus/skills/gitnexus-guide.md index a71429f32..7f90f4e6d 100644 --- a/gitnexus/skills/gitnexus-guide.md +++ b/gitnexus/skills/gitnexus-guide.md @@ -39,6 +39,8 @@ For any task involving code understanding, debugging, impact analysis, or refact | `rename` | Multi-file coordinated rename with confidence-tagged edits | | `cypher` | Raw graph queries (read `gitnexus://repo/{name}/schema` first) | | `explain` | Persisted taint findings — source→sink data flows (needs `analyze --pdg`) | +| `pdg_query` | Control/data dependence — what gates X (CDG) / where Y flows (REACHING_DEF); needs `analyze --pdg` | +| `check` | Check graph invariants such as circular imports | | `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | ### Paginating `list_repos` @@ -82,6 +84,15 @@ Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). A repo indexed without `--pdg` returns a clear "no taint layer" note. Caveats: findings are intra-procedural only — cross-function, closure/callback, property/field, and implicit flows are not modeled, so the absence of a finding is **not** proof of safety. `SANITIZES` (sanitizer-kill) edges are queryable via `cypher`. +### Control & data dependence (`pdg_query`) + +`pdg_query` reads the control/data-dependence layers `gitnexus analyze --pdg` records (CDG + REACHING_DEF, basic-block granular) — the control/data analog of `explain`. It is **always anchored** (a `target` file path or symbol, resolved like `context`) and has two modes: + +- `pdg_query { mode: "controls", target: "..." }` — CDG: "under what condition does X run?". Each edge is a controlling predicate block → dependent block with the branch sense (`'T'`/`'F'`) in `reason`; an edge into an early `return`/`throw` is flagged `guard: true` (guard-clause discovery — the sense depends on the predicate, so don't filter guards by a fixed label). +- `pdg_query { mode: "flows", target: "...", variable?: "..." }` — REACHING_DEF def→use edges within the function; pass `variable` to trace one binding. + +A repo indexed without `--pdg` returns a "no PDG layer" note (or "status unknown" when the layer can't be confirmed). Intra-procedural only — cross-function flow is taint's domain (`explain`). The raw CDG/REACHING_DEF edges are also queryable via `cypher`. See the `gitnexus-pdg-query` skill for the full query surface. + ## Resources Reference Lightweight reads (~100-500 tokens) for navigation: diff --git a/gitnexus/skills/gitnexus-pdg-query.md b/gitnexus/skills/gitnexus-pdg-query.md new file mode 100644 index 000000000..f2fcd7d3b --- /dev/null +++ b/gitnexus/skills/gitnexus-pdg-query.md @@ -0,0 +1,89 @@ +--- +name: gitnexus-pdg-query +description: "Use when querying or extending GitNexus's PDG control/data-dependence surface (the `pdg_query` MCP tool, CDG/REACHING_DEF edges), or reasoning about \"what controls X\" / \"where does Y flow\" / guard clauses. Examples: \"what guards this statement?\", \"trace this variable within the function\", \"why is the pdg_query result empty?\", \"add a CDG query\"." +--- + +# PDG query surface with GitNexus + +Expert knowledge for the `pdg_query` MCP tool and the control/data-dependence +edges it reads — the opt-in `--pdg` program-dependence layers. Read this before +touching `gitnexus/src/mcp/local/local-backend.ts` (`_pdgQueryImpl`) or the +`pdg_query` tool def, or when explaining a `pdg_query` result. + +## When to Use + +- "Under what condition does this statement run?" (guarding predicates). +- "Where does this variable flow inside the function?" (def→use). +- Guard-clause discovery (early-return guards — subsumes the #559 heuristic). +- Extending or reviewing `pdg_query` / the CDG / REACHING_DEF read path. +- Debugging an empty or surprising `pdg_query` result. + +## The layered substrate (build order) + +`pdg_query` runs **on** the same graph taint runs on. Each layer is opt-in +behind `--pdg`; a default `analyze` run records none of them (byte-identical). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L5 CDG Ferrante control dependence (post-dominators) (M5 #2085) +``` + +All three are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table +(keyed by the `type` property). There is **no** `Function → BasicBlock` edge. + +## The two modes + +- `pdg_query({ mode: 'controls', target })` — CDG. For the anchored function, + each edge: controlling predicate block → dependent block + branch sense in + `label` (`'T'` = predicate's true/taken arm, `'F'` = false/fall-through). An + edge into an early-return/throw block is flagged `guard: true`. +- `pdg_query({ mode: 'flows', target, variable? })` — REACHING_DEF def→use + edges; `variable` filters to one binding. + +`target` is **required** — a file path or a symbol/function name (resolved like +`context()`). There is no anchorless mode (see below). + +## The corrected guard-clause Cypher + +The RFC #567 §2 form (`[:CDG {label:'F'}]`) does **not** run as written. Edges +are values of the single `CodeRelation` table's `type` property, and the branch +sense is in `reason`, NOT a `label` column: + +```cypher +MATCH (pred:BasicBlock)-[r:CodeRelation {type: 'CDG'}]->(dep:BasicBlock) +WHERE dep.text STARTS WITH 'return' OR dep.text STARTS WITH 'throw' +RETURN pred.startLine, r.reason AS branch, dep.startLine, dep.text +``` + +`r.reason` is the sense the predicate took to reach the early exit. For +`if (!ok) return;` the return rides the predicate's **true** arm (`'T'`) and the +protected body rides the **false** arm (`'F'`) — polarity depends on the guard, +so don't hard-code one sense. + +## Gotchas (the load-bearing ones) + +- **Always anchored + LIMIT-bounded.** LadybugDB has no rel-property index, so + an unanchored `[:CDG*]`/`[:REACHING_DEF*]` path scan is unbounded. `pdg_query` + requires `target` and bounds the page; raw `cypher` callers must anchor on a + file id-prefix or symbol span themselves. +- **BasicBlock↔symbol join is reconstructed.** No `Function→BasicBlock` edge: + the block is matched by its id-prefix (`BasicBlock:::…`) + plus `startLine` within the symbol's span. BasicBlock `startLine` is **1-based** + while the symbol node's `startLine`/`endLine` are **0-based**, so **both** bounds + are shifted `+1` (`[symStart+1, symEnd+1]`): the upper `+1` keeps a guard/def/use + on the function's **final line**, the lower `+1` excludes an adjacent function's + block on the line directly **above**. Same-line / nested functions anchor coarsely. +- **No PDG layer ⇒ a note, not an error.** If the repo wasn't indexed with + `--pdg` the tool returns `{ results: [], note: "no PDG layer …" }` (cheap meta + probe on `RepoMeta.pdg.maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`). +- **CDG labels are binary in M5/M6.** Every `switch`-case arm is `'T'`; per-case + conditions are not yet distinguished. +- **Intra-procedural only.** Cross-function flow is taint's domain (`explain`). + +## Mirror, don't fork + +`_pdgQueryImpl` is the front half of `_explainImpl` (WAL wrapper, meta no-layer +probe, limit validation, `resolveSymbolCandidates` anchoring) with CDG/ +REACHING_DEF instead of TAINTED — and none of taint's path-codec / interproc +`TAINT_PATH` machinery. Reuse those shared helpers; do not re-implement them. diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 6700c020b..8cbfe2b6d 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -35,6 +35,12 @@ export interface AIContextOptions { * plain caller that omits it gets "main", preserving prior behavior. */ defaultBranch?: string; + /** + * Whether the index was built with `--pdg` (#2086 M6). Gates the `pdg_query` + * line in the generated block — without the PDG layer the tool only returns a + * "no PDG layer" note, so advertising it on a non-`--pdg` index is noise. + */ + hasPdg?: boolean; } const GITNEXUS_START_MARKER = ''; @@ -105,26 +111,45 @@ export function markdownSafeBranch(branch: string): string { return branch.replace(/`/g, ''); } +/** Options for {@link generateGitNexusContent} (collapsed from positional + * params, #2188 review — six `undefined`s to reach `hasPdg` was the smell). */ +export interface GitNexusContentOptions { + generatedSkills?: GeneratedSkillInfo[]; + groupNames?: string[]; + noStats?: boolean; + skipSkills?: boolean; + /** Project-relative path to the runner `gitnexus analyze` drops next to the + * index (#1945). Referenced by docs so a single CLI-neutral command resolves + * the available runner (global `gitnexus` → `pnpm dlx` → `npx`) at call time. */ + runnerPath?: string; + /** Default branch for the regression-compare example (#243). Configurable so + * projects on `develop`/`master`/etc. don't get `base_ref: "main"` rewritten + * back over their fix on every analyze. The value is embedded inside a + * Markdown inline-code span: validateBranchName rejects backticks upstream, + * and `markdownSafeBranch` strips any remaining backtick here as defense in + * depth, so JSON.stringify's quote/escape handling is sufficient and the + * branch cannot break out of the span (#1996 tri-review P1). */ + defaultBranch?: string; + /** Whether the index was built with `--pdg` (#2086 M6). Gates the pdg_query + * line below — false (default) omits it, so a non-pdg index doesn't advertise + * a tool that only returns a "no PDG layer" note. */ + hasPdg?: boolean; +} + export function generateGitNexusContent( projectName: string, stats: RepoStats, - generatedSkills?: GeneratedSkillInfo[], - groupNames?: string[], - noStats?: boolean, - skipSkills?: boolean, - // Project-relative path to the runner `gitnexus analyze` drops next to the - // index (#1945). Referenced by docs so a single CLI-neutral command resolves - // the available runner (global `gitnexus` → `pnpm dlx` → `npx`) at call time. - runnerPath: string = '.gitnexus/run.cjs', - // Default branch for the regression-compare example (#243). Configurable so - // projects on `develop`/`master`/etc. don't get `base_ref: "main"` rewritten - // back over their fix on every analyze. The value is embedded inside a - // Markdown inline-code span: validateBranchName rejects backticks upstream, - // and `markdownSafeBranch` strips any remaining backtick here as defense in - // depth, so JSON.stringify's quote/escape handling is sufficient and the - // branch cannot break out of the span (#1996 tri-review P1). - defaultBranch: string = 'main', + opts: GitNexusContentOptions = {}, ): string { + const { + generatedSkills, + groupNames, + noStats, + skipSkills, + runnerPath = '.gitnexus/run.cjs', + defaultBranch = 'main', + hasPdg = false, + } = opts; const generatedRows = generatedSkills && generatedSkills.length > 0 ? generatedSkills @@ -179,7 +204,11 @@ This project is indexed by GitNexus as **${projectName}**${noStats ? '' : ` (${s - **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. - When exploring unfamiliar code, use \`query({search_query: "concept"})\` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. - When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use \`context({name: "symbolName"})\`. -- For security review, \`explain({target: "fileOrSymbol"})\` lists taint findings (source→sink flows; needs \`analyze --pdg\`). +- For security review, \`explain({target: "fileOrSymbol"})\` lists taint findings (source→sink flows; needs \`analyze --pdg\`).${ + hasPdg + ? `\n- For control/data dependence, \`pdg_query({mode: "controls", target: "fileOrSymbol"})\` answers "under what condition does X run?" (CDG, incl. guard clauses) and \`pdg_query({mode: "flows", target, variable})\` traces "where does variable Y flow?" (REACHING_DEF). \`--pdg\` layer.` + : '' + } ## Never Do @@ -447,16 +476,15 @@ export async function generateAIContextFiles( logger.warn(`Could not write GitNexus runner to ${runnerPath}: ${String(err)}`); } - const content = generateGitNexusContent( - projectName, - stats, + const content = generateGitNexusContent(projectName, stats, { generatedSkills, groupNames, - options?.noStats, - options?.skipSkills, + noStats: options?.noStats, + skipSkills: options?.skipSkills, runnerPath, - options?.defaultBranch ?? 'main', - ); + defaultBranch: options?.defaultBranch ?? 'main', + hasPdg: options?.hasPdg ?? false, + }); const createdFiles: string[] = []; if (!options?.skipAgentsMd) { diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index da768c689..0cf17ab13 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -1403,6 +1403,7 @@ const analyzeCommandImpl = async ( // Mirror runFullAnalysis `noStats` bridge (#1477) — same expression; // exercised on the `--skills` path by analyze-no-stats-bridge.test.ts. noStats: options.stats === false, + hasPdg: options.pdg === true, }, ); } diff --git a/gitnexus/src/core/ingestion/cfg/control-dependence.ts b/gitnexus/src/core/ingestion/cfg/control-dependence.ts new file mode 100644 index 000000000..033075327 --- /dev/null +++ b/gitnexus/src/core/ingestion/cfg/control-dependence.ts @@ -0,0 +1,172 @@ +/** + * Control dependence (#2085 M5 U3) — Ferrante, Ottenstein & Warren §3.1.1 over + * the post-dominator tree. A block `dependent` is control-dependent on a branch + * block `controller` when `controller` decides whether `dependent` executes: + * formally, there is a CFG edge `controller → B` such that `dependent` + * post-dominates `B` but does NOT strictly post-dominate `controller`. + * + * Construction (§3.1.1): for each CFG edge `(A, B)` where `B` does NOT + * post-dominate `A`, walk UP the post-dom tree from `B` to (but not including) + * `ipdom(A)`; every block on that path is control-dependent on `A`. The branch + * SENSE of the edge ('T' | 'F') becomes the edge label (KTD4 / KTD3 — it rides + * the persisted relation's `reason` column). + * + * PURE AND DETERMINISTIC (mirrors post-dominators.ts / reaching-defs.ts): no + * graph, no logger, importable outside the worker; output is deduped per + * (controller, dependent, label) and sorted, so snapshot tests and + * content-derived edge ids are stable. The loop header legitimately appears as + * control-dependent on ITSELF (`controller === dependent`) — the loop predicate + * gates its own re-execution; this is standard PDG behavior, not a bug. + */ +import { + computePostDominators, + postDominates, + NO_IPDOM, + type PostDomTree, +} from './post-dominators.js'; +import type { CfgEdgeKind, FunctionCfg } from './types.js'; + +export type CdgLabel = 'T' | 'F'; + +export interface ControlDepEdge { + /** The branch block whose outcome controls `dependentBlock`. */ + readonly controllerBlock: number; + /** The block that executes only because `controllerBlock` took `label`. */ + readonly dependentBlock: number; + /** Branch sense of the controlling CFG edge — see {@link branchSense}. */ + readonly label: CdgLabel; +} + +export interface ControlDepResult { + /** Deduped, sorted (controller, dependent, label) control-dependence edges. */ + readonly edges: readonly ControlDepEdge[]; + /** + * True when the `maxEdges` ceiling was reached; `edges` is then a + * deterministic prefix (CFG-edge iteration order, sorted), never a silent + * drop. Mirrors {@link computeReachingDefs}'s `truncated`. + */ + readonly truncated: boolean; +} + +/** + * Per-controller branch-arm senses, derived from the controller block's OUTGOING + * edge kinds. The CFG edge kind alone cannot name a branch sense: the M1 visitor + * emits an explicit `cond-true`/`cond-false` only for a `then`/`else` arm, but a + * condition's FALL-THROUGH false arm (no-`else`, or a guard's `if (!ok) return;`) + * is wired as `seq`, and an `if` ending a loop body falls through as `loop-back` + * — while a `do/while` bottom-test's TRUE arm is also a `loop-back`. So `seq` + * and `loop-back` are genuinely ambiguous in isolation (issue #2188 F1). + * + * The fix reads the sense from the CONTROLLER's structure: a 2-way branch emits + * exactly one explicitly-sensed arm (`cond-true`/`switch-case` ⇒ true, or + * `cond-false` ⇒ false), and its other (ambiguous) arm is the COMPLEMENT. This + * map records which explicit senses each block emits so {@link labelFor} can + * resolve an ambiguous edge against its sibling. + */ +interface ArmSenses { + hasTrueArm: boolean; // emits a cond-true or switch-case edge + hasFalseArm: boolean; // emits a cond-false edge +} + +function buildArmSenses(cfg: FunctionCfg): ArmSenses[] { + const n = cfg.blocks.length; + const senses: ArmSenses[] = Array.from({ length: n }, () => ({ + hasTrueArm: false, + hasFalseArm: false, + })); + for (const e of cfg.edges) { + if (e.from < 0 || e.from >= n) continue; + if (e.kind === 'cond-true' || e.kind === 'switch-case') senses[e.from].hasTrueArm = true; + else if (e.kind === 'cond-false') senses[e.from].hasFalseArm = true; + } + return senses; +} + +/** + * The CDG label ('T'|'F') for a control-dependence edge, given the controlling + * block's arm senses. An explicitly-sensed edge is taken at face value; an + * ambiguous fall-through edge (`seq`/`loop-back`/`fallthrough`/jump) is the + * COMPLEMENT of the controller's explicit sibling arm. Per-case `switch` value + * labels are deferred to #2086 — every `switch-case` is 'T' in M5. + */ +function labelFor(kind: CfgEdgeKind, controller: ArmSenses): CdgLabel { + if (kind === 'cond-true' || kind === 'switch-case') return 'T'; + if (kind === 'cond-false') return 'F'; + // Ambiguous structural kind: take the complement of the controller's explicit + // arm. A block with a true arm reaches here via its false fall-through; a + // do/while bottom-test (false arm = cond-false) reaches here via its true + // loop-back. With neither explicit arm (a degenerate / exit-unreachable + // region — see #2188 F2, where the dependence itself is unsound) the sense is + // indeterminate; default 'F' since fall-through is the common case. + if (controller.hasTrueArm) return 'F'; + if (controller.hasFalseArm) return 'T'; + return 'F'; +} + +/** + * Compute control-dependence edges for one function's CFG. `postDom` may be + * supplied to reuse an already-built tree; otherwise it is computed. See the + * module doc for the purity/determinism contract. + */ +export function computeControlDependence( + cfg: FunctionCfg, + postDom?: PostDomTree, + // Heap-safety ceiling on materialized edges, mirroring computeReachingDefs' + // `maxFacts` (#2188 review): the pre-dedup walk is O(edges × post-dom depth), + // so bound it before it can spike. `0` ⇒ unbounded. On overflow `edges` is a + // deterministic prefix and `truncated` is set — never a silent drop. + maxEdges: number = 0, +): ControlDepResult { + const tree = postDom ?? computePostDominators(cfg); + const { ipdom } = tree; + const n = cfg.blocks.length; + const armSenses = buildArmSenses(cfg); + const cap = maxEdges > 0 ? maxEdges : Infinity; + + const out: ControlDepEdge[] = []; + const seen = new Set(); + let truncated = false; + + scan: for (const e of cfg.edges) { + const a = e.from; + const b = e.to; + if (a < 0 || a >= n || b < 0 || b >= n) continue; + // No control dependence when B post-dominates A — every path leaving A + // through this edge still reaches B, so A does not decide B's execution. + // This guard is exactly AC2: a dependence exists IFF post-dominance fails. + if (postDominates(tree, b, a)) continue; + + // Sense is read from the CONTROLLER's arms, not this edge's kind alone — + // seq/loop-back fall-through false arms would otherwise mislabel as 'T' + // (#2188 F1). + const label = labelFor(e.kind, armSenses[a]); + const stop = ipdom[a]; // walk up to ipdom(A), EXCLUSIVE (NO_IPDOM ⇒ to root) + let cur = b; + let steps = 0; + // `steps <= n` is defensive — the ipdom chain is a finite tree. + while (cur !== NO_IPDOM && cur !== stop && steps <= n) { + const key = `${a}:${cur}:${label}`; + if (!seen.has(key)) { + // Check BEFORE pushing so `truncated` means a genuine overflow (a new + // unique edge had to be dropped), not merely "reached the ceiling" — + // exactly `cap` edges is a full, non-truncated result. + if (out.length >= cap) { + truncated = true; + break scan; + } + seen.add(key); + out.push({ controllerBlock: a, dependentBlock: cur, label }); + } + cur = ipdom[cur]; + steps += 1; + } + } + + out.sort( + (x, y) => + x.controllerBlock - y.controllerBlock || + x.dependentBlock - y.dependentBlock || + (x.label < y.label ? -1 : x.label > y.label ? 1 : 0), + ); + return { edges: out, truncated }; +} diff --git a/gitnexus/src/core/ingestion/cfg/emit.ts b/gitnexus/src/core/ingestion/cfg/emit.ts index dc682d5b0..246baa1d4 100644 --- a/gitnexus/src/core/ingestion/cfg/emit.ts +++ b/gitnexus/src/core/ingestion/cfg/emit.ts @@ -21,6 +21,12 @@ import type { KnowledgeGraph } from '../../graph/types.js'; import { generateId } from '../../../lib/utils.js'; import { computeReachingDefs } from './reaching-defs.js'; +import { computeControlDependence } from './control-dependence.js'; +import { + computePostDominators, + isExitReachableFromAllBlocks, + NO_IPDOM, +} from './post-dominators.js'; import type { BindingEntry, FunctionCfg } from './types.js'; /** @@ -41,6 +47,38 @@ export const DEFAULT_MAX_CFG_EDGES_PER_FUNCTION = 5000; */ export const DEFAULT_PDG_MAX_REACHING_DEF_EDGES_PER_FUNCTION = 4000; +/** + * Default per-function CDG edge cap (#2085 M5). CDG edge count is bounded by + * (blocks × control-nesting-depth) — comparable to the CFG edge count — so it + * reuses the CFG default of 5000. Counts DEDUPED (controller, dependent, label) + * edges (the pure {@link computeControlDependence} already dedups). `0` ⇒ + * unlimited; `undefined` ⇒ this default. Folded into the `RepoMeta.pdg` stamp + * (U5) so introducing CDG forces a full writeback for pre-CDG `--pdg` indexes. + */ +export const DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION = 5000; + +/** + * Heap-safety ceiling on {@link computeControlDependence}'s pre-dedup + * materialization (#2188 review). The walk is O(edges × post-dom depth), and its + * `out` IS the deduped-edge quantity the per-function cap trims — so, UNLIKE + * REACHING_DEF's facts ceiling, this is deliberately NOT derived from the + * runtime edge cap (doing so would pre-truncate the very set the cap reports on, + * losing the exact dropped count). A fixed, generous multiple of the default + * edge cap: far above any real function — a catastrophe backstop only. When hit, + * the per-function cap reporting plus the `truncated` flag keep it observable + * (never a silent drop). + */ +export const DEFAULT_PDG_MAX_CDG_MATERIALIZATION_PER_FUNCTION = + 8 * DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION; + +/** + * Env flag that additionally emits diagnostic `POST_DOMINATE` edges + * (block → its immediate post-dominator) alongside CDG (#2085 M5 KTD8). Off in + * every normal `--pdg` run — these are for inspecting the post-dom tree, not a + * queryable product surface. Accepts `1`/`true` (case-insensitive). + */ +export const POST_DOMINATE_DEBUG_ENV = 'GITNEXUS_PDG_EMIT_POST_DOMINATE'; + /** * Fact-materialization headroom over the edge cap (#2082 M2 U3/F3): facts are * O(defs×uses) BY SPEC in merge-heavy code, and the edge cap alone bounds the @@ -417,3 +455,153 @@ export function emitFileReachingDefs( return result; } + +export interface CdgEmitResult { + /** Deduped (controller, dependent, label) CDG edges persisted. */ + edges: number; + /** CDG edges dropped by the per-function edge cap. */ + droppedEdges: number; + /** Functions that hit the CDG edge cap. */ + cappedFunctions: number; + /** Diagnostic POST_DOMINATE edges emitted (0 unless the debug env is set). */ + postDominateEdges: number; + /** + * Functions skipped because EXIT was not reachable from every entry-reachable + * block — post-dominance would be unsound (#2188 review). CFG/REACHING_DEF for + * those functions are kept; only their CDG projection is omitted. + */ + skippedUnsoundFunctions: number; +} + +/** Whether the POST_DOMINATE debug env flag is enabled (`1`/`true`). */ +const postDominateDebugEnabled = (): boolean => { + const v = process.env[POST_DOMINATE_DEBUG_ENV]; + return v === '1' || v?.toLowerCase() === 'true'; +}; + +/** + * Compute control dependence per function and persist the bounded CDG + * projection (#2085 M5 U4). Mirrors {@link emitFileReachingDefs}: the pure + * {@link computeControlDependence} already dedups to (controller, dependent, + * label), so the per-function cap applies to deduped edges and overflow logs + * one unconditional `onWarn` naming the dropped count — no silent truncation + * (R6/R7). The branch label ('T'|'F') rides the `reason` column (KTD3), + * mirroring how CFG stores its edge kind. + * + * When {@link POST_DOMINATE_DEBUG_ENV} is set, also emits diagnostic + * `POST_DOMINATE` edges (block → its immediate post-dominator). These are NOT + * capped or counted against the CDG budget — they exist only for inspecting the + * post-dom tree and never appear in a normal run. + */ +export function emitFileCdg( + graph: KnowledgeGraph, + cfgs: readonly FunctionCfg[], + maxEdgesPerFunction: number = DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION, + onWarn?: (message: string) => void, +): CdgEmitResult { + const result: CdgEmitResult = { + edges: 0, + droppedEdges: 0, + cappedFunctions: 0, + postDominateEdges: 0, + skippedUnsoundFunctions: 0, + }; + const cap = maxEdgesPerFunction > 0 ? maxEdgesPerFunction : Infinity; + const emitPostDom = postDominateDebugEnabled(); + + for (const cfg of cfgs) { + const { filePath, functionStartLine, functionStartColumn } = cfg; + // Sound post-dominance requires EXIT reachable from every entry-reachable + // block (#2188 review). A CFG that violates it — a future visitor's + // multi-terminal / non-terminating shape — would yield a CDG that both + // drops real and invents spurious dependences, so skip CDG for it. CFG and + // REACHING_DEF (emitted elsewhere, independent of post-dominance) are kept. + if (!isExitReachableFromAllBlocks(cfg)) { + result.skippedUnsoundFunctions++; + onWarn?.( + `[cdg] ${filePath}:${functionStartLine}: EXIT not reachable from all ` + + `blocks — CDG skipped for this function (CFG/REACHING_DEF unaffected)`, + ); + continue; + } + // Compute the post-dom tree once and feed it to the control-dependence + // pass (avoids recomputing it) and to the optional POST_DOMINATE emit. + const tree = computePostDominators(cfg); + // Bound the pre-dedup materialization (heap parity with REACHING_DEF). The + // fixed ceiling is a catastrophe backstop; the per-function edge cap below + // remains the reporting authority. A ceiling hit is surfaced, not silent. + const { edges: cdgEdges, truncated } = computeControlDependence( + cfg, + tree, + DEFAULT_PDG_MAX_CDG_MATERIALIZATION_PER_FUNCTION, + ); + if (truncated) { + onWarn?.( + `[cdg] ${filePath}:${functionStartLine}: control-dependence materialization ` + + `ceiling (${DEFAULT_PDG_MAX_CDG_MATERIALIZATION_PER_FUNCTION}) reached — ` + + `edge counts for this function are a floor`, + ); + } + + let emittedForFn = 0; + for (const edge of cdgEdges) { + if (emittedForFn >= cap) { + const dropped = cdgEdges.length - emittedForFn; + result.droppedEdges += dropped; + result.cappedFunctions++; + onWarn?.( + `[cdg] ${filePath}:${functionStartLine}: per-function CDG edge cap ` + + `(${maxEdgesPerFunction}) reached — dropped ${dropped} of ${cdgEdges.length} edges`, + ); + break; + } + const sourceId = basicBlockId( + filePath, + functionStartLine, + functionStartColumn, + edge.controllerBlock, + ); + const targetId = basicBlockId( + filePath, + functionStartLine, + functionStartColumn, + edge.dependentBlock, + ); + graph.addRelationship({ + id: generateId( + 'CDG', + `${filePath}:${functionStartLine}:${functionStartColumn}:` + + `${edge.controllerBlock}->${edge.dependentBlock}:${edge.label}`, + ), + type: 'CDG', + sourceId, + targetId, + confidence: 1.0, + reason: edge.label, // 'T' | 'F' — queryable, mirrors CFG's kind-in-reason + }); + result.edges++; + emittedForFn++; + } + + if (emitPostDom) { + for (let b = 0; b < tree.ipdom.length; b++) { + const ip = tree.ipdom[b]; + if (ip === NO_IPDOM) continue; + graph.addRelationship({ + id: generateId( + 'POST_DOMINATE', + `${filePath}:${functionStartLine}:${functionStartColumn}:${b}->${ip}`, + ), + type: 'POST_DOMINATE', + sourceId: basicBlockId(filePath, functionStartLine, functionStartColumn, b), + targetId: basicBlockId(filePath, functionStartLine, functionStartColumn, ip), + confidence: 1.0, + reason: '', + }); + result.postDominateEdges++; + } + } + } + + return result; +} diff --git a/gitnexus/src/core/ingestion/cfg/post-dominators.ts b/gitnexus/src/core/ingestion/cfg/post-dominators.ts new file mode 100644 index 000000000..336a9e303 --- /dev/null +++ b/gitnexus/src/core/ingestion/cfg/post-dominators.ts @@ -0,0 +1,218 @@ +/** + * Post-dominators (#2085 M5 U2) — the immediate-post-dominator tree of one + * function's CFG, the substrate the Ferrante control-dependence pass walks. + * + * A block `p` post-dominates a block `b` iff every path from `b` to the + * function EXIT passes through `p`. Post-dominators are exactly the DOMINATORS + * of the REVERSE CFG rooted at EXIT, so this is the Cooper–Harvey–Kennedy + * "A Simple, Fast Dominance Algorithm" run over reversed edges. KTD2 of the M5 + * plan picks CHK over Lengauer–Tarjan: per-function CFGs are small and + * line-capped, CHK is near-linear in practice, and its iterative shape matches + * the reaching-defs fixpoint already in this module. + * + * PURE AND DETERMINISTIC (load-bearing, mirrors reaching-defs.ts): no graph, no + * logger, importable outside the worker; predecessors/successors are sorted and + * iteration is reverse-postorder so the `ipdom` array is identical across runs + * (snapshot tests and content-derived edge ids depend on it). + * + * The single-EXIT invariant the M1 TS visitor preserves (visitors/typescript.ts) + * makes EXIT the unique reverse-CFG root. Blocks that cannot reach EXIT in the + * forward CFG (an exit-less infinite loop) are not reverse-reachable from it and + * have NO post-dominator: their `ipdom` is {@link NO_IPDOM}. The control- + * dependence pass treats "no post-dominator" as "does not post-dominate" (KTD5). + * + * NOTE (issue #2188 F2): this is NOT a fully sound over-approximation. Inside a + * region where NO block reaches EXIT, every `ipdom` is `NO_IPDOM`, so the + * Ferrante walk degenerates to one edge per control point — it can both DROP a + * real control dependence and INVENT a spurious one. This does not arise for the + * current TS visitor (every loop is given a structural `header → loopExit` + * `cond-false` edge, so EXIT stays reverse-reachable), but it is unsound for + * hand-built CFGs and any future language visitor lacking that exit edge. + * Nontermination-sensitive post-dominance (a virtual root over the + * non-terminating SCCs) would be the correct treatment — tracked for follow-up. + */ +import type { FunctionCfg } from './types.js'; + +/** + * Sentinel `ipdom` value: the block has no immediate post-dominator. True for + * the EXIT block itself (the reverse-CFG root) and for any block that cannot + * reach EXIT. Chosen as -1 so the {@link postDominates} climb terminates + * naturally instead of self-looping on the root. + */ +export const NO_IPDOM = -1; + +export interface PostDomTree { + /** + * `ipdom[b]` = the index of `b`'s immediate post-dominator, or + * {@link NO_IPDOM} when `b` has none (EXIT, or a block that cannot reach EXIT). + */ + readonly ipdom: readonly number[]; +} + +/** + * Compute the immediate-post-dominator tree for one function's CFG. See the + * module doc for the purity/determinism contract and EXIT-root assumptions. + */ +export function computePostDominators(cfg: FunctionCfg): PostDomTree { + const n = cfg.blocks.length; + const exit = cfg.exitIndex; + if (n === 0 || exit < 0 || exit >= n) { + return { ipdom: new Array(n).fill(NO_IPDOM) }; + } + + // Forward adjacency (sorted for deterministic intersect order). The reverse + // CFG, on which we compute dominators, flips these: a node's reverse-CFG + // successors are its CFG predecessors, and its reverse-CFG predecessors + // (the "preds" CHK intersects over) are its CFG successors. + const cfgPreds: number[][] = Array.from({ length: n }, () => []); + const cfgSuccs: number[][] = Array.from({ length: n }, () => []); + for (const e of cfg.edges) { + if (e.from < 0 || e.from >= n || e.to < 0 || e.to >= n) continue; + cfgSuccs[e.from].push(e.to); + cfgPreds[e.to].push(e.from); + } + for (const l of cfgPreds) l.sort((a, b) => a - b); + for (const l of cfgSuccs) l.sort((a, b) => a - b); + + // Postorder of the reverse CFG from EXIT (traversing CFG-predecessor edges). + // Iterative DFS with an explicit phase stack; children pushed in sorted order + // for determinism. postNum is the CHK comparison key: higher = closer to root. + const postNum = new Array(n).fill(-1); + const postorder: number[] = []; + const visited = new Array(n).fill(false); + const stack: { node: number; childIdx: number }[] = [{ node: exit, childIdx: 0 }]; + visited[exit] = true; + while (stack.length) { + const top = stack[stack.length - 1]; + const revSuccs = cfgPreds[top.node]; // reverse-CFG successors + if (top.childIdx < revSuccs.length) { + const next = revSuccs[top.childIdx]; + top.childIdx += 1; + if (!visited[next]) { + visited[next] = true; + stack.push({ node: next, childIdx: 0 }); + } + } else { + postNum[top.node] = postorder.length; + postorder.push(top.node); + stack.pop(); + } + } + const rpo = [...postorder].reverse(); + + // CHK fixpoint. ipdom[exit] = exit DURING computation (the root dominates + // itself, so the intersect climb has a common terminus); it is reset to + // NO_IPDOM before returning so callers' climbs terminate at the root. + const ipdom = new Array(n).fill(NO_IPDOM); + ipdom[exit] = exit; + + const intersect = (a: number, b: number): number => { + let f1 = a; + let f2 = b; + while (f1 !== f2) { + while (postNum[f1] < postNum[f2]) f1 = ipdom[f1]; + while (postNum[f2] < postNum[f1]) f2 = ipdom[f2]; + } + return f1; + }; + + let changed = true; + while (changed) { + changed = false; + for (const b of rpo) { + if (b === exit) continue; + // CHK "predecessors in the reverse CFG" = this block's CFG successors. + // Fold only those already processed (ipdom assigned); RPO guarantees at + // least one for every block reverse-reachable from EXIT. + let newIpdom = NO_IPDOM; + for (const s of cfgSuccs[b]) { + if (ipdom[s] !== NO_IPDOM) { + newIpdom = newIpdom === NO_IPDOM ? s : intersect(s, newIpdom); + } + } + if (newIpdom !== NO_IPDOM && ipdom[b] !== newIpdom) { + ipdom[b] = newIpdom; + changed = true; + } + } + } + + ipdom[exit] = NO_IPDOM; // root: no post-dominator above it + return { ipdom }; +} + +/** + * Does block `p` post-dominate block `b`? Climbs the post-dom tree from `b` + * toward EXIT and tests membership of `p`. Reflexive: a block post-dominates + * itself. A block with no post-dominator (EXIT, or one that cannot reach EXIT) + * is post-dominated only by itself. The step guard is purely defensive — the + * `ipdom` chain is a tree and always terminates at {@link NO_IPDOM}. + */ +export function postDominates(tree: PostDomTree, p: number, b: number): boolean { + const { ipdom } = tree; + const n = ipdom.length; + if (p < 0 || b < 0 || p >= n || b >= n) return false; + let cur = b; + let steps = 0; + while (cur !== NO_IPDOM && steps <= n) { + if (cur === p) return true; + cur = ipdom[cur]; + steps += 1; + } + return false; +} + +/** + * Precondition for SOUND post-dominance (#2188 review): EXIT must be reachable + * (forward) from every block that is itself reachable from ENTRY. When it + * fails — an entry-reachable region that cannot reach EXIT, e.g. a + * non-terminating loop or a multi-terminal CFG a future language visitor might + * emit — the EXIT-rooted reverse walk degenerates (every such block gets + * {@link NO_IPDOM}), which both DROPS real control dependences and INVENTS + * spurious ones (the unsoundness documented in the module header). Consumers + * ({@link emitFileCdg}) check this and skip CDG for the function rather than + * persist an unsound projection — CFG and REACHING_DEF, which do not depend on + * post-dominance, are unaffected. + * + * The current TS visitor always satisfies this (every loop is given a + * structural `header → loopExit` edge, keeping EXIT reverse-reachable), so this + * is a guard for future visitors and hand-built CFGs, not a behavior change + * today. Pure and O(V+E). + */ +export function isExitReachableFromAllBlocks(cfg: FunctionCfg): boolean { + const n = cfg.blocks.length; + if (n === 0) return true; + const { entryIndex, exitIndex } = cfg; + if (entryIndex < 0 || entryIndex >= n || exitIndex < 0 || exitIndex >= n) return false; + + const succ: number[][] = Array.from({ length: n }, () => []); + const pred: number[][] = Array.from({ length: n }, () => []); + for (const e of cfg.edges) { + if (e.from < 0 || e.from >= n || e.to < 0 || e.to >= n) continue; + succ[e.from].push(e.to); + pred[e.to].push(e.from); + } + + const reach = (start: number, adj: readonly number[][]): Uint8Array => { + const seen = new Uint8Array(n); + const stack = [start]; + seen[start] = 1; + while (stack.length > 0) { + const b = stack.pop() as number; + for (const next of adj[b]) { + if (!seen[next]) { + seen[next] = 1; + stack.push(next); + } + } + } + return seen; + }; + + const fromEntry = reach(entryIndex, succ); // forward-reachable from ENTRY + const canReachExit = reach(exitIndex, pred); // can reach EXIT (reverse from EXIT) + for (let i = 0; i < n; i++) { + if (fromEntry[i] && !canReachExit[i]) return false; + } + return true; +} diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index 269724e0d..334c022ac 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -83,6 +83,15 @@ export interface PipelineOptions { * programmatic / server path only, like the M1 caps. */ pdgMaxReachingDefEdgesPerFunction?: number; + /** + * Per-function CDG (control-dependence) edge cap for the scope-resolution + * emit step (#2085 M5). `undefined` ⇒ `DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION` + * (5000); `0` ⇒ no cap (unlimited). Emit-time-only — NOT folded into the + * parse-cache chunk key; recorded resolved in `RepoMeta.pdg` so introducing + * CDG (an absent stamp key) forces a full writeback for pre-CDG `--pdg` + * indexes. No CLI flag — programmatic / server path only. + */ + pdgMaxCdgEdgesPerFunction?: number; /** * Per-function taint findings cap for the scope-resolution taint pass * (#2083 M3). `undefined` ⇒ `DEFAULT_PDG_MAX_TAINT_FINDINGS_PER_FUNCTION` diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts index 6b748dd97..6bf86dc84 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts @@ -375,6 +375,7 @@ export const scopeResolutionPhase: PipelinePhase = { pdg: ctx.options?.pdg === true, pdgMaxEdgesPerFunction: ctx.options?.pdgMaxEdgesPerFunction, pdgMaxReachingDefEdgesPerFunction: ctx.options?.pdgMaxReachingDefEdgesPerFunction, + pdgMaxCdgEdgesPerFunction: ctx.options?.pdgMaxCdgEdgesPerFunction, pdgMaxTaintFindingsPerFunction: ctx.options?.pdgMaxTaintFindingsPerFunction, pdgMaxTaintHops: ctx.options?.pdgMaxTaintHops, recordResolutionOutcome: (outcome) => { diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index faa29b17c..733f76be3 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -37,10 +37,12 @@ import { buildGraphNodeLookup } from '../graph-bridge/node-lookup.js'; import { emitFileCfgs, emitFileReachingDefs, + emitFileCdg, isEmitSafeCfg, DEFAULT_MAX_CFG_EDGES_PER_FUNCTION, DEFAULT_PDG_MAX_REACHING_DEF_EDGES_PER_FUNCTION, DEFAULT_PDG_MAX_REACHING_DEF_FACTS_PER_FUNCTION, + DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION, REACHING_DEF_FACTS_PER_EDGE_CAP, } from '../../cfg/emit.js'; import { @@ -289,6 +291,9 @@ interface RunScopeResolutionInput { /** Per-function REACHING_DEF edge cap (#2082 M2). `undefined` ⇒ * {@link DEFAULT_PDG_MAX_REACHING_DEF_EDGES_PER_FUNCTION}; `0` ⇒ no cap. */ readonly pdgMaxReachingDefEdgesPerFunction?: number; + /** Per-function CDG (control-dependence) edge cap (#2085 M5). `undefined` ⇒ + * {@link DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION}; `0` ⇒ no cap. */ + readonly pdgMaxCdgEdgesPerFunction?: number; /** Per-function taint findings cap (#2083 M3, consumed by the U4 taint * emit step in the pdg window). `undefined` ⇒ * `DEFAULT_PDG_MAX_TAINT_FINDINGS_PER_FUNCTION` (200); `0` ⇒ no cap. */ @@ -771,6 +776,8 @@ export function runScopeResolution( let rdDropped = 0; let rdFacts = 0; let rdTruncated = 0; + let cdgEdges = 0; + let cdgDropped = 0; // ── M3 taint setup (#2083 U4) ──────────────────────────────────────── // Explicit model-registration seam (idempotent, cheap) — the registry // stays empty on non-pdg runs, preserving default-run parity. The @@ -877,6 +884,22 @@ export function runScopeResolution( rdFacts += rd.facts; rdTruncated += rd.truncatedFunctions; + // M5 (#2085 U5): control dependence over the SAME validated CFGs. + // Independent of taint — runs for every `--pdg` language (post-dom + + // Ferrante are language-agnostic, no source/sink model needed). Pure + // compute; the bounded (controller, dependent, label) projection is + // persisted and its time folds into the `pdg=` PROF segment next to RD. + const tCdg = PROF ? performance.now() : 0; + const cdg = emitFileCdg( + graph, + wellFormed, + input.pdgMaxCdgEdgesPerFunction ?? DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION, + (message) => logger.warn(message), // unconditional — R6, no silent truncation + ); + if (PROF) pdgMs += performance.now() - tCdg; + cdgEdges += cdg.edges; + cdgDropped += cdg.droppedEdges; + // M3 (#2083 U4): taint over the SAME validated CFGs, inside the SAME // per-file try (a taint throw costs this file's taint layer only — // its CFG/REACHING_DEF edges above are already in the graph). Skipped @@ -950,6 +973,8 @@ export function runScopeResolution( `; ${rdEdges} REACHING_DEF edges (${rdFacts} facts)` + (rdDropped > 0 ? `, ${rdDropped} REACHING_DEF edges dropped (per-function cap)` : '') + (rdTruncated > 0 ? `, ${rdTruncated} function(s) hit the fact limit` : '') + + `; ${cdgEdges} CDG edges` + + (cdgDropped > 0 ? `, ${cdgDropped} CDG edges dropped (per-function cap)` : '') + // M3 volume telemetry — only for languages with a registered model. (taintSpec !== undefined ? `; taint: ${taintTotals.findings} TAINTED, ${taintTotals.kills} SANITIZES ` + diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 8de20925b..7910dde60 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -49,6 +49,7 @@ import { DEFAULT_PDG_MAX_FUNCTION_LINES } from './ingestion/cfg/collect.js'; import { DEFAULT_MAX_CFG_EDGES_PER_FUNCTION, DEFAULT_PDG_MAX_REACHING_DEF_EDGES_PER_FUNCTION, + DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION, } from './ingestion/cfg/emit.js'; import { DEFAULT_PDG_MAX_TAINT_FINDINGS_PER_FUNCTION, @@ -152,6 +153,10 @@ export interface AnalyzeOptions { /** Per-function REACHING_DEF edge cap (#2082 M2). Forwarded to * `PipelineOptions.pdgMaxReachingDefEdgesPerFunction`. */ pdgMaxReachingDefEdgesPerFunction?: number; + /** Per-function CDG edge cap (#2085 M5). Forwarded to + * `PipelineOptions.pdgMaxCdgEdgesPerFunction`. No CLI flag or rc key — + * programmatic / server path only, like the other pdg caps. */ + pdgMaxCdgEdgesPerFunction?: number; /** Per-function taint findings cap (#2083 M3). Forwarded to * `PipelineOptions.pdgMaxTaintFindingsPerFunction`. No CLI flag or rc key * (KTD8) — programmatic / server path only, like the other pdg caps. */ @@ -371,6 +376,7 @@ type PdgOptions = Pick< | 'pdgMaxFunctionLines' | 'pdgMaxEdgesPerFunction' | 'pdgMaxReachingDefEdgesPerFunction' + | 'pdgMaxCdgEdgesPerFunction' | 'pdgMaxTaintFindingsPerFunction' | 'pdgMaxTaintHops' | 'pdgMaxInterprocFindings' @@ -386,6 +392,12 @@ export const resolvePdgConfig = (options: PdgOptions): RepoMeta['pdg'] => maxReachingDefEdgesPerFunction: options.pdgMaxReachingDefEdgesPerFunction ?? DEFAULT_PDG_MAX_REACHING_DEF_EDGES_PER_FUNCTION, + // #2085 M5: control-dependence cap. Absent on any pre-M5 (M2/M3/M4-era) + // stamp → the key-union pdgModeMismatch trips the first CDG-aware run + // over an existing `--pdg` index and forces the full writeback that + // materialises CDG edges for every file without `--force`. + maxCdgEdgesPerFunction: + options.pdgMaxCdgEdgesPerFunction ?? DEFAULT_PDG_MAX_CDG_EDGES_PER_FUNCTION, // #2083 M3: taint caps + model identity. The key-union comparator in // pdgModeMismatch picks these up structurally — an M2-era stamp lacks // all three, so the first M3 run over an M2 `--pdg` index trips a full @@ -802,6 +814,7 @@ export async function runFullAnalysis( pdgMaxFunctionLines: options.pdgMaxFunctionLines, pdgMaxEdgesPerFunction: options.pdgMaxEdgesPerFunction, pdgMaxReachingDefEdgesPerFunction: options.pdgMaxReachingDefEdgesPerFunction, + pdgMaxCdgEdgesPerFunction: options.pdgMaxCdgEdgesPerFunction, pdgMaxTaintFindingsPerFunction: options.pdgMaxTaintFindingsPerFunction, pdgMaxTaintHops: options.pdgMaxTaintHops, pdgMaxInterprocFindings: options.pdgMaxInterprocFindings, @@ -1423,6 +1436,7 @@ export async function runFullAnalysis( skipSkills: options.skipSkills, noStats: options.noStats, defaultBranch: options.defaultBranch, + hasPdg: options.pdg === true, }, ); } catch { diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index d50bd67ac..6526a4bef 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -59,6 +59,8 @@ import { LIST_REPOS_MAX_LIMIT, EXPLAIN_DEFAULT_LIMIT, EXPLAIN_MAX_LIMIT, + PDG_QUERY_DEFAULT_LIMIT, + PDG_QUERY_MAX_LIMIT, } from '../tools.js'; import { findImportCycles } from '../../core/graph/import-cycles.js'; import { decodeTaintPath } from '../../core/ingestion/taint/path-codec.js'; @@ -1293,6 +1295,8 @@ export class LocalBackend { return this.context(repo, params); case 'explain': return this.explain(repo, params); + case 'pdg_query': + return this.pdgQuery(repo, params); case 'impact': return this.impact(repo, params); case 'detect_changes': @@ -2794,6 +2798,103 @@ export class LocalBackend { }; } + /** + * Resolve a `target` (file path OR symbol/function name) into a BasicBlock + * SOURCE-block anchor, shared by `explain` (TAINTED) and `pdg_query` + * (CDG/REACHING_DEF) — both reconstruct the symbol↔block join the same way + * (there is no Function→BasicBlock edge). #2188 review: extracted from two + * near-identical copies that had DRIFTED — `_explainImpl` used a 0-based, + * un-widened span window that dropped a function's final-line block and could + * leak a neighbor's line-above block; this single resolver applies the correct + * `[symStart+1, symEnd+1]` window (1-based BasicBlock startLine vs 0-based + * symbol span) to BOTH callers. + * + * Returns a BARE `anchorClause` (no leading `AND`) so each caller composes its + * own `WHERE`; `early` carries the not-found/ambiguous payload (caller returns + * it verbatim). `target` / symbol names flow only through `queryParams` bind + * params — never interpolated into Cypher. + */ + private async resolveBlockAnchor( + repo: RepoHandle, + target: string, + toolName: 'explain' | 'pdg_query', + ): Promise<{ + anchorClause: string; + queryParams: Record; + anchor: { file: string; symbol?: string; startLine?: number; endLine?: number }; + early?: Record; + }> { + if (looksLikeFilePath(target)) { + return { + anchorClause: + '(a.id STARTS WITH $idPrefix OR a.filePath = $targetPath OR a.filePath ENDS WITH $targetSuffix)', + queryParams: { + idPrefix: `BasicBlock:${target}:`, + targetPath: target, + targetSuffix: `/${target}`, + }, + anchor: { file: target }, + }; + } + const outcome = await this.resolveSymbolCandidates(repo, { name: target }, {}); + if (outcome.kind === 'not_found') { + return { + anchorClause: '', + queryParams: {}, + anchor: { file: '' }, + early: { error: `Symbol '${target}' not found` }, + }; + } + if (outcome.kind === 'ambiguous') { + return { + anchorClause: '', + queryParams: {}, + anchor: { file: '' }, + early: { + status: 'ambiguous', + message: `Found ${outcome.candidates.length} symbols matching '${target}'. Re-call ${toolName} with the file path, or disambiguate via context() first.`, + candidates: outcome.candidates.map((c) => ({ + uid: c.id, + name: c.name, + kind: c.type, + filePath: c.filePath, + line: c.startLine, + score: Number(c.score.toFixed(2)), + })), + }, + }; + } + const sym = outcome.symbol; + const idPrefix = `BasicBlock:${sym.filePath}:`; + if ( + typeof sym.startLine === 'number' && + typeof sym.endLine === 'number' && + sym.endLine >= sym.startLine + ) { + // BasicBlock startLine is 1-based; the symbol span is 0-based. Shift BOTH + // bounds +1 so the window is the function's true block span: the lower +1 + // excludes a neighbor's block on the line directly above, the upper +1 + // keeps a guard/def/use on the final line (#2188 review). + 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 }, + anchor: { + file: sym.filePath, + symbol: sym.name, + startLine: sym.startLine, + endLine: sym.endLine, + }, + }; + } + // No usable span — degrade to the file-level filter (documented). + return { + anchorClause: 'a.id STARTS WITH $idPrefix', + queryParams: { idPrefix }, + anchor: { file: sym.filePath, symbol: sym.name }, + }; + } + /** * Explain tool (#2083 M3 U6) — persisted taint-finding explanation. * WAL-aware wrapper mirroring `context`. @@ -2876,64 +2977,9 @@ export class LocalBackend { // Resolve the optional anchor into a WHERE clause on the SOURCE block. const target = typeof params.target === 'string' ? params.target.trim() : ''; let anchorClause = ''; - const queryParams: Record = {}; + let queryParams: Record = {}; let anchor: { file: string; symbol?: string; startLine?: number; endLine?: number } | undefined; - // Build the anchor as a file filter (used only when `target` is path-ish). - const buildFileAnchor = (): void => { - // Exact path via the BasicBlock id-prefix template, OR a - // path-separator-aligned suffix so partial paths work like context()'s - // file_path hint ("vuln.ts" ⇒ "src/vuln.ts", never "devuln.ts"). - anchorClause = - 'AND (a.id STARTS WITH $idPrefix OR a.filePath = $targetPath OR a.filePath ENDS WITH $targetSuffix)'; - queryParams.idPrefix = `BasicBlock:${target}:`; - queryParams.targetPath = target; - queryParams.targetSuffix = `/${target}`; - anchor = { file: target as string }; - }; - - // Resolve `target` as a symbol into the anchor. Returns an early-return - // payload (not_found / ambiguous) or undefined on success. - const resolveSymbolAnchor = async (): Promise | undefined> => { - const outcome = await this.resolveSymbolCandidates(repo, { name: target as string }, {}); - if (outcome.kind === 'not_found') { - return { error: `Symbol '${target}' not found` }; - } - if (outcome.kind === 'ambiguous') { - return { - status: 'ambiguous', - message: `Found ${outcome.candidates.length} symbols matching '${target}'. Re-call explain with the file path, or disambiguate via context() first.`, - candidates: outcome.candidates.map((c) => ({ - uid: c.id, - name: c.name, - kind: c.type, - filePath: c.filePath, - line: c.startLine, - score: Number(c.score.toFixed(2)), - })), - }; - } - const sym = outcome.symbol; - queryParams.idPrefix = `BasicBlock:${sym.filePath}:`; - anchor = { file: sym.filePath, symbol: sym.name }; - if ( - typeof sym.startLine === 'number' && - typeof sym.endLine === 'number' && - sym.endLine >= sym.startLine - ) { - anchorClause = - 'AND a.id STARTS WITH $idPrefix AND a.startLine >= $symStart AND a.startLine <= $symEnd'; - queryParams.symStart = sym.startLine; - queryParams.symEnd = sym.endLine; - anchor.startLine = sym.startLine; - anchor.endLine = sym.endLine; - } else { - // No usable span — degrade to the file-level filter (documented). - anchorClause = 'AND a.id STARTS WITH $idPrefix'; - } - return undefined; - }; - // Bounded by construction: the BasicBlock→BasicBlock partition holds only // the sparse pdg layers, TAINTED rows are per-function-capped at analyze // time, and the page is LIMIT-bounded (the limit is a validated integer — @@ -2941,7 +2987,7 @@ export class LocalBackend { const runAnchoredQuery = async (): Promise<{ rows: unknown[]; totalFindings: number }> => { const matchClause = ` MATCH (a:BasicBlock)-[r:CodeRelation]->(b:BasicBlock) - WHERE r.type = 'TAINTED' ${anchorClause}`; + WHERE r.type = 'TAINTED'${anchorClause ? ` AND ${anchorClause}` : ''}`; const [qRows, countRows] = await Promise.all([ executeParameterized( repo.lbugPath, @@ -2966,14 +3012,14 @@ export class LocalBackend { }; if (target) { - if (looksLikeFilePath(target)) { - buildFileAnchor(); - } else { - // A bare or dotted symbol name (`UserController.create`) — resolve as a - // symbol rather than silently file-anchoring to an empty result. - const early = await resolveSymbolAnchor(); - if (early) return early; - } + // Shared symbol↔block anchor resolver (#2188): file id-prefix OR symbol + // span, with the corrected [symStart+1, symEnd+1] window. A bare/dotted + // symbol name resolves as a symbol rather than silently file-anchoring. + const resolved = await this.resolveBlockAnchor(repo, target, 'explain'); + if (resolved.early) return resolved.early; + anchorClause = resolved.anchorClause; + queryParams = resolved.queryParams; + anchor = resolved.anchor; } const { rows, totalFindings } = await runAnchoredQuery(); @@ -3147,6 +3193,203 @@ export class LocalBackend { }; } + private async pdgQuery( + repo: RepoHandle, + params: { mode?: string; target?: string; variable?: string; limit?: number }, + ): Promise { + try { + return await this._pdgQueryImpl(repo, params); + } catch (err: any) { + const msg = (err instanceof Error ? err.message : String(err)) || 'pdg_query failed'; + if (isWalCorruptionError(err)) { + return { error: msg, recoverySuggestion: WAL_RECOVERY_SUGGESTION }; + } + throw err; + } + } + + /** + * Query the persisted PDG (#2086 M6) — the control/data-dependence analog of + * `explain`. `controls` reads CDG ("under what condition does X run?", branch + * sense 'T'|'F' in `reason`); `flows` reads REACHING_DEF (def→use, variable + * name in `reason`). Intra-procedural, basic-block granular. + * + * Bounded by construction: the BasicBlock→BasicBlock partition holds only the + * sparse, per-function-capped pdg layers, the query is anchored to one file/ + * symbol, and the page is LIMIT-bounded (validated integer, interpolated + * because LadybugDB does not parameterize LIMIT). LadybugDB has no rel- + * property index, so the anchor IS the bound — there is no anchorless mode. + * + * Symbol↔block join: there is no Function→BasicBlock edge; the SOURCE block + * (`a` — controller for CDG, def for REACHING_DEF) is filtered by the + * BasicBlock id-prefix (`basicBlockId` template) plus its `startLine` within + * the symbol's span. BasicBlock `startLine` is 1-based while symbol-node + * `startLine`/`endLine` are 0-based, so BOTH bounds are shifted +1 + * (`[symStart+1, symEnd+1]`) onto the block basis: the upper +1 keeps a + * guard/def/use on the function's final line, and the lower +1 excludes an + * adjacent function's block on the line directly above (#2188 review). Both + * endpoints share the function (intra-procedural), so filtering the source + * endpoint suffices. + */ + private async _pdgQueryImpl( + repo: RepoHandle, + params: { mode?: string; target?: string; variable?: string; limit?: number } = {}, + ): Promise { + await this.ensureInitialized(repo); + + // Mode validation — the JSON-schema enum is advisory for MCP clients, so + // the backend enforces it (an unhandled mode would otherwise fall through). + const mode = params.mode; + if (mode !== 'controls' && mode !== 'flows') { + return { + error: `Invalid "mode": expected "controls" or "flows", got ${JSON.stringify(params.mode)}.`, + }; + } + + const rawLimit = params.limit ?? PDG_QUERY_DEFAULT_LIMIT; + if (!Number.isInteger(rawLimit) || rawLimit < 1 || rawLimit > PDG_QUERY_MAX_LIMIT) { + return { + error: `Invalid "limit": expected an integer in [1, ${PDG_QUERY_MAX_LIMIT}], got ${JSON.stringify(params.limit)}.`, + }; + } + const limit = rawLimit; + + // PDG queries are always anchored (no rel-property index ⇒ an unanchored + // basic-block path scan is unbounded). `target` is required. + const target = typeof params.target === 'string' ? params.target.trim() : ''; + if (!target) { + return { + error: + 'pdg_query requires a "target" (a file path or symbol/function name) — PDG queries are always anchored.', + }; + } + + const edgeType = mode === 'controls' ? 'CDG' : 'REACHING_DEF'; + // Definitive: the meta stamp says this layer was never recorded. + const NO_PDG_NOTE = `no PDG layer — run gitnexus analyze --pdg to record ${edgeType} edges for this repo`; + // Inconclusive: meta is unreadable AND a global probe found zero rows of this + // edge type — but a genuinely edge-free layer (all-linear functions) looks + // identical to a missing one, so don't assert absence (#2188 review). + const PDG_LAYER_UNKNOWN_NOTE = `no ${edgeType} edges found for this target; PDG layer status unknown — was this repo indexed with gitnexus analyze --pdg?`; + + // Cheap meta probe: the layer exists iff the pdg stamp carries the + // mode-relevant cap (maxCdgEdgesPerFunction for CDG, maxReachingDef… + // for REACHING_DEF). Absent ⇒ the no-layer hint without a DB scan. + let pdgStamped: boolean | undefined; + try { + const meta = await loadMeta(path.dirname(repo.lbugPath)); + if (meta) { + pdgStamped = + mode === 'controls' + ? meta.pdg?.maxCdgEdgesPerFunction !== undefined + : meta.pdg?.maxReachingDefEdgesPerFunction !== undefined; + } + } catch { + /* meta unreadable — decide from the DB below */ + } + if (pdgStamped === false) { + return { mode, results: [], total: 0, note: NO_PDG_NOTE }; + } + + // Resolve the anchor on the SOURCE block via the shared resolver also used + // by explain (#2188): file id-prefix OR symbol span on the corrected + // [symStart+1, symEnd+1] window. `target` is required, so the early cases + // (not-found/ambiguous) return here and `anchor`/`anchorClause` are always + // set below (anchor stays non-optional — no `| undefined` — #2188 CodeQL). + const resolved = await this.resolveBlockAnchor(repo, target, 'pdg_query'); + if (resolved.early) return resolved.early; + const { anchorClause, anchor } = resolved; + const queryParams = resolved.queryParams; + + // Optional variable filter (flows mode) — REACHING_DEF stores the variable + // name in `reason`. + let reasonClause = ''; + if (mode === 'flows' && typeof params.variable === 'string' && params.variable.trim()) { + reasonClause = ' AND r.reason = $variable'; + queryParams.variable = params.variable.trim(); + } + + // edgeType is a hardcoded per-mode literal (never user input); `target` / + // `variable` flow only through bind params (no Cypher interpolation). + const matchClause = ` + MATCH (a:BasicBlock)-[r:CodeRelation]->(b:BasicBlock) + WHERE r.type = '${edgeType}' AND ${anchorClause}${reasonClause}`; + const [rows, countRows] = await Promise.all([ + executeParameterized( + repo.lbugPath, + `${matchClause} + RETURN a.id AS srcId, a.startLine AS srcLine, b.startLine AS dstLine, b.text AS dstText, r.reason AS reason + ORDER BY srcId, dstLine, reason + LIMIT ${limit}`, + queryParams, + ), + executeParameterized( + repo.lbugPath, + `${matchClause}\n RETURN COUNT(*) AS total`, + queryParams, + ), + ]); + const total = Number((countRows[0] as any)?.total ?? (countRows[0] as any)?.[0] ?? 0); + + // Unreadable meta + anchored miss: one bounded probe distinguishes "no rows + // for this anchor" from "no rows of this edge type at all". With meta + // unreadable we cannot tell a missing layer from an edge-free one, so the + // note is the inconclusive "status unknown" form, not the definitive + // NO_PDG_NOTE (which is reserved for the meta-stamped absence above). + if (total === 0 && pdgStamped === undefined) { + const probe = await executeParameterized( + repo.lbugPath, + `MATCH (:BasicBlock)-[r:CodeRelation]->(:BasicBlock) WHERE r.type = '${edgeType}' RETURN r.reason AS reason LIMIT 1`, + {}, + ); + if (probe.length === 0) return { mode, results: [], total: 0, note: PDG_LAYER_UNKNOWN_NOTE }; + } + + // basicBlockId = `BasicBlock::::` — split + // from the RIGHT (filePath may contain ':'). + const fnLineOf = (id: string): number => { + const parts = id.split(':'); + return Number(parts[parts.length - 3]); + }; + + const results = + mode === 'controls' + ? rows.map((r: any) => { + const fnLine = fnLineOf(String(r.srcId ?? r[0] ?? '')); + const dstText = String(r.dstText ?? r[3] ?? ''); + // A CDG edge into an early-exit block is a guard clause (subsumes + // #559): the controller predicate gates the dependent via `label`. + const isGuardExit = /^\s*(return|throw|continue|break)\b/.test(dstText); + return { + ...(Number.isInteger(fnLine) ? { functionLine: fnLine } : {}), + controller: { line: (r.srcLine ?? r[1]) as number | undefined }, + dependent: { line: (r.dstLine ?? r[2]) as number | undefined, text: dstText }, + label: String(r.reason ?? r[4] ?? ''), + ...(isGuardExit ? { guard: true } : {}), + }; + }) + : rows.map((r: any) => { + const fnLine = fnLineOf(String(r.srcId ?? r[0] ?? '')); + return { + ...(Number.isInteger(fnLine) ? { functionLine: fnLine } : {}), + variable: String(r.reason ?? r[4] ?? ''), + def: { line: (r.srcLine ?? r[1]) as number | undefined }, + use: { + line: (r.dstLine ?? r[2]) as number | undefined, + text: String(r.dstText ?? r[3] ?? ''), + }, + }; + }); + + return { + mode, + anchor, + results, + total, + ...(total > results.length ? { truncated: true } : {}), + }; + } + /** * Legacy explore — kept for backwards compatibility with resources.ts. * Routes cluster/process types to direct graph queries. diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index ec37c196c..5bf81a772 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -472,6 +472,12 @@ relationships: - MEMBER_OF: Symbol belongs to community - STEP_IN_PROCESS: Symbol is step N in process +pdg_layers: "Recorded ONLY when indexed with 'gitnexus analyze --pdg'. Intra-procedural, basic-block granular; both endpoints are BasicBlock nodes. Prefer the pdg_query tool over raw Cypher." + - BasicBlock: "Basic-block node. Columns: id, filePath, startLine, endLine, text. id = 'BasicBlock::::'." + - CFG: "Control-flow edge BasicBlock->BasicBlock. Edge kind (seq/cond-true/cond-false/loop-back/...) is in reason." + - CDG: "Control-DEPENDENCE edge BasicBlock->BasicBlock — the source predicate gates the target's execution. Branch sense 'T'|'F' in reason. Query via pdg_query mode:'controls'." + - REACHING_DEF: "Data-dependence (def->use) edge BasicBlock->BasicBlock. Source-level variable name is in reason. Query via pdg_query mode:'flows'." + relationship_table: "All relationships use a single CodeRelation table with a 'type' property. Properties: type (STRING), confidence (DOUBLE), reason (STRING), step (INT32)" example_queries: @@ -489,6 +495,11 @@ example_queries: WHERE p.heuristicLabel = "LoginFlow" RETURN s.name, r.step ORDER BY r.step + + guard_clauses (--pdg only; prefer pdg_query mode:'controls'): | + MATCH (pred:BasicBlock)-[r:CodeRelation {type: 'CDG'}]->(dep:BasicBlock) + WHERE dep.text STARTS WITH 'return' OR dep.text STARTS WITH 'throw' + RETURN pred.startLine, r.reason AS branch, dep.startLine, dep.text `; } diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index ec3d76e04..e2ffe10a7 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -72,6 +72,11 @@ export const LIST_REPOS_MAX_LIMIT = 200; export const EXPLAIN_DEFAULT_LIMIT = 50; export const EXPLAIN_MAX_LIMIT = 200; +// pdg_query result-page bounds (#2086 M6). Mirror the EXPLAIN_* limits — the +// no-rel-index path means every page must be anchored + LIMIT-bounded. +export const PDG_QUERY_DEFAULT_LIMIT = 50; +export const PDG_QUERY_MAX_LIMIT = 200; + export const GITNEXUS_TOOLS: ToolDefinition[] = [ { name: 'list_repos', @@ -226,7 +231,8 @@ TIPS: - All relationships use single CodeRelation table — filter with {type: 'CALLS'} etc. - Community = auto-detected functional area (Leiden algorithm). Properties: heuristicLabel, cohesion, symbolCount, keywords, description, enrichedBy - Process = execution flow trace from entry point to terminal. Properties: heuristicLabel, processType, stepCount, communities, entryPointId, terminalId -- Use heuristicLabel (not label) for human-readable community/process names`, +- Use heuristicLabel (not label) for human-readable community/process names +- PDG layers (only when indexed with \`--pdg\`): BasicBlock nodes + CFG / CDG (control dependence, branch sense 'T'|'F' in reason) / REACHING_DEF (def→use, variable in reason) edges, all BasicBlock→BasicBlock. Prefer the \`pdg_query\` tool — it anchors + bounds these for you (raw \`[:CDG*]\`/\`[:REACHING_DEF*]\` path scans are unindexed and unbounded).`, annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', @@ -582,6 +588,58 @@ Findings are deliberately NOT part of impact()'s traversal or the web schema — required: [], }, }, + { + name: 'pdg_query', + description: `Query the persisted Program Dependence Graph recorded by \`gitnexus analyze --pdg\` — control dependence (CDG) and data dependence (REACHING_DEF) at basic-block granularity. The control/data analog of \`explain\` (which is the taint consumer). + +MODES: +- \`controls\` — "under what condition does X run?". Returns, for the anchored function, each control-dependence edge: the controlling predicate block, the dependent block, and the branch sense ('T' = the predicate's true/taken arm, 'F' = its false/fall-through arm). An edge into an early return/throw block is flagged \`guard: true\` (subsumes the #559 guard heuristic); the branch sense of a guard depends on its predicate — \`if (!ok) return;\` rides the 'T' arm — so don't filter guards by a fixed label. +- \`flows\` — "where does variable Y flow?". Returns REACHING_DEF def→use edges for the anchored function; pass \`variable\` to filter to one binding. + +WHEN TO USE: comprehension ("what guards this statement?"), data-flow tracing within a function, guard-clause discovery. Requires \`gitnexus analyze --pdg\`; without that layer the tool returns a clear "no PDG layer" note, not an error. + +ANCHORING (required): \`target\` is a file path or a symbol/function name (resolved like context()). PDG queries are ALWAYS anchored — there is no whole-repo enumeration (an unanchored basic-block path scan is unbounded; LadybugDB has no rel-property index). A symbol target is line-range granular; an ambiguous name returns ranked candidates, unknown returns not-found. + +CONTRACT CAVEATS: +- CDG labels are binary 'T'/'F' in M5/M6; per-case \`switch\` arm conditions are not yet distinguished (every case dispatch is 'T'). +- Granularity is basic-block, reconstructed to the function via the BasicBlock id + line span (no Function→BasicBlock edge); deeply same-line-packed functions may anchor coarsely. +- Control/data dependence is intra-procedural (per function). Cross-function flow is taint's domain (\`explain\`). +- These edges are deliberately NOT part of impact()'s traversal — \`pdg_query\` is the dedicated consumer; raw edges are also queryable via \`cypher\`.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, + inputSchema: { + type: 'object', + properties: { + mode: { + type: 'string', + enum: ['controls', 'flows'], + description: + "'controls' = control dependence (CDG: what condition gates X); 'flows' = data dependence (REACHING_DEF: where variable Y flows).", + }, + target: { + type: 'string', + description: + 'Required anchor: a file path (e.g. "src/handlers/run.ts" — suffix match accepted) or a symbol/function name (resolved like context()).', + }, + variable: { + type: 'string', + description: + 'Optional (flows mode only): restrict REACHING_DEF results to this source-level variable name.', + }, + limit: { + type: 'integer', + description: `Max edges returned (default: ${PDG_QUERY_DEFAULT_LIMIT}, max: ${PDG_QUERY_MAX_LIMIT}). "total" reports the full matched count; "truncated" is set when the page is smaller.`, + default: PDG_QUERY_DEFAULT_LIMIT, + minimum: 1, + maximum: PDG_QUERY_MAX_LIMIT, + }, + repo: { + type: 'string', + description: 'Repository name or path. Omit if only one repo is indexed.', + }, + }, + required: ['mode', 'target'], + }, + }, { name: 'route_map', description: `Show API route mappings: which components/hooks fetch which API endpoints, and which handler files serve them. @@ -717,6 +775,7 @@ const BRANCH_SCOPED_TOOLS = new Set([ 'context', 'detect_changes', 'explain', + 'pdg_query', 'check', 'impact', 'rename', diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 56714e5be..b03db9fc5 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -158,6 +158,14 @@ export interface RepoMeta { * type for that reason; resolved (always present) on every M2+ write. */ maxReachingDefEdgesPerFunction?: number; + /** + * Emit-side per-function CDG (control-dependence) edge cap, resolved + * (0 = unlimited; #2085 M5). ABSENT on any pre-M5 stamp — that absence is + * what trips `pdgModeMismatch` on the first CDG-aware run and forces the + * full writeback that materialises CDG edges. Optional for that upgrade + * reason; resolved (always present) on every M5+ write. + */ + maxCdgEdgesPerFunction?: number; /** * Per-function taint findings cap, resolved (0 = unlimited; #2083 M3). * ABSENT on an M1/M2-era stamp — like `maxReachingDefEdgesPerFunction`, diff --git a/gitnexus/test/integration/basicblock-roundtrip.test.ts b/gitnexus/test/integration/basicblock-roundtrip.test.ts index 88f9a60c9..ef6cfc86f 100644 --- a/gitnexus/test/integration/basicblock-roundtrip.test.ts +++ b/gitnexus/test/integration/basicblock-roundtrip.test.ts @@ -4,11 +4,13 @@ * * Exercises the real csv-generator → loadGraphToLbug → COPY → query path: * - a BasicBlock node (id/filePath/startLine/endLine/text) round-trips - * - one edge of each new type (CFG/REACHING_DEF/TAINTED/SANITIZES/TAINT_PATH) - * between two BasicBlocks round-trips (asserts the new FROM/TO DDL pair + - * REL_TYPES load through bulk COPY) + * - one edge of each new type (CFG/REACHING_DEF/TAINTED/SANITIZES/TAINT_PATH, + * plus CDG/POST_DOMINATE from #2085 M5) between two BasicBlocks round-trips + * (asserts the new FROM/TO DDL pair + REL_TYPES load through bulk COPY) * - REACHING_DEF carries its `variable` in the existing `reason` column * (M0/S1 storage decision) and a variable-filtered query returns it + * - CDG carries its branch label ('T'|'F') in the same `reason` column + * (#2085 M5) and a label-filtered query returns it * - the DDL (BASICBLOCK_SCHEMA wired into NODE_SCHEMA_QUERIES) loads on a * fresh DB — if BASICBLOCK_SCHEMA were not in SCHEMA_QUERIES, initLbug would * never create the table and these COPYs would fail (F1 guard, end-to-end) @@ -27,7 +29,15 @@ let dbPath: string; const BB1 = 'BasicBlock:src/a.ts:0'; const BB2 = 'BasicBlock:src/a.ts:1'; -const NEW_EDGE_TYPES = ['CFG', 'REACHING_DEF', 'TAINTED', 'SANITIZES', 'TAINT_PATH'] as const; +const NEW_EDGE_TYPES = [ + 'CFG', + 'REACHING_DEF', + 'TAINTED', + 'SANITIZES', + 'TAINT_PATH', + 'CDG', + 'POST_DOMINATE', +] as const; beforeAll(async () => { tmpBase = path.join(os.tmpdir(), `gitnexus-bb-roundtrip-${Date.now()}-${process.pid}`); @@ -65,7 +75,7 @@ beforeAll(async () => { sourceId: BB1, targetId: BB2, type, - reason: type === 'REACHING_DEF' ? 'x' : `${type.toLowerCase()}-edge`, + reason: type === 'REACHING_DEF' ? 'x' : type === 'CDG' ? 'T' : `${type.toLowerCase()}-edge`, })), ); @@ -151,4 +161,14 @@ describe('BasicBlock + taint/PDG edge round-trip (#2080)', () => { expect(rows[0].from).toBe(BB1); expect(rows[0].to).toBe(BB2); }); + + it('CDG carries its branch label in reason and is queryable by it (#2085 M5)', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const rows = await adapter.executeQuery( + "MATCH (a:BasicBlock)-[r:CodeRelation {type: 'CDG', reason: 'T'}]->(b:BasicBlock) RETURN a.id AS from, b.id AS to", + ); + expect(rows).toHaveLength(1); + expect(rows[0].from).toBe(BB1); + expect(rows[0].to).toBe(BB2); + }); }); diff --git a/gitnexus/test/integration/cfg/__snapshots__/cdg-snapshot.test.ts.snap b/gitnexus/test/integration/cfg/__snapshots__/cdg-snapshot.test.ts.snap new file mode 100644 index 000000000..c7b07fdc3 --- /dev/null +++ b/gitnexus/test/integration/cfg/__snapshots__/cdg-snapshot.test.ts.snap @@ -0,0 +1,93 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`AC1 — CDG snapshot on the M1 fixture > matches the committed control-dependence set for every fixture function 1`] = ` +[ + { + "cdg": [], + "startLine": 9, + }, + { + "cdg": [ + "2->3:T", + "2->4:F", + ], + "startLine": 14, + }, + { + "cdg": [ + "2->3:T", + "2->4:F", + "4->5:T", + "4->6:F", + ], + "startLine": 23, + }, + { + "cdg": [ + "2->2:T", + "2->4:T", + ], + "startLine": 34, + }, + { + "cdg": [ + "2->2:T", + "2->4:T", + "2->5:T", + ], + "startLine": 41, + }, + { + "cdg": [ + "2->2:T", + "2->4:T", + ], + "startLine": 48, + }, + { + "cdg": [ + "2->4:T", + "2->5:T", + "2->6:T", + "2->7:T", + "2->8:T", + ], + "startLine": 55, + }, + { + "cdg": [ + "5->3:F", + "5->4:F", + ], + "startLine": 69, + }, + { + "cdg": [ + "2->3:T", + "2->4:F", + ], + "startLine": 80, + }, + { + "cdg": [ + "2->2:T", + "2->4:T", + "4->5:T", + "4->6:F", + ], + "startLine": 87, + }, + { + "cdg": [ + "3->7:F", + "4->5:T", + "4->6:F", + ], + "startLine": 102, + }, + { + "cdg": [], + "startLine": 115, + }, +] +`; diff --git a/gitnexus/test/integration/cfg/cdg-snapshot.test.ts b/gitnexus/test/integration/cfg/cdg-snapshot.test.ts new file mode 100644 index 000000000..1b8a594eb --- /dev/null +++ b/gitnexus/test/integration/cfg/cdg-snapshot.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest'; +import fs from 'fs'; +import path from 'path'; +import Parser from 'tree-sitter'; +import TypeScript from 'tree-sitter-typescript'; +import { collectFunctionCfgs } from '../../../src/core/ingestion/cfg/collect.js'; +import { computeControlDependence } from '../../../src/core/ingestion/cfg/control-dependence.js'; +import { computePostDominators } from '../../../src/core/ingestion/cfg/post-dominators.js'; +import { getProvider } from '../../../src/core/ingestion/languages/index.js'; +import { SupportedLanguages } from '../../../src/config/supported-languages.js'; +import type { FunctionCfg } from '../../../src/core/ingestion/cfg/types.js'; + +// #2085 M5 AC1 — a committed snapshot of the CDG edge set on the shared M1 +// fixture (the same `ten-functions.ts` the REACHING_DEF snapshot uses). The +// serialization is deterministic — sorted `controller->dependent:label` +// strings — so any post-dominator / Ferrante behavior change shows as a +// reviewable snapshot diff, never silent drift. + +const FIXTURES = path.join(__dirname, 'fixtures'); + +function cfgsOfFile(file: string): readonly FunctionCfg[] { + const visitor = getProvider(SupportedLanguages.TypeScript).cfgVisitor; + if (!visitor) throw new Error('no cfgVisitor'); + const source = fs.readFileSync(path.join(FIXTURES, file), 'utf8'); + const parser = new Parser(); + parser.setLanguage(TypeScript.typescript); + return collectFunctionCfgs(parser.parse(source).rootNode, visitor, file).cfgs; +} + +/** Deterministic rendering: startLine + sorted controller->dependent:label. */ +function serialize(cfg: FunctionCfg): Record { + const { edges } = computeControlDependence(cfg); + return { + startLine: cfg.functionStartLine, + cdg: edges.map((e) => `${e.controllerBlock}->${e.dependentBlock}:${e.label}`), + }; +} + +describe('AC1 — CDG snapshot on the M1 fixture', () => { + it('matches the committed control-dependence set for every fixture function', () => { + const cfgs = cfgsOfFile('ten-functions.ts'); + expect(cfgs).toHaveLength(12); + expect(cfgs.map(serialize)).toMatchSnapshot(); + }); + + it('every CDG edge references in-range blocks with a valid T/F label (AC2 sanity)', () => { + for (const cfg of cfgsOfFile('ten-functions.ts')) { + const tree = computePostDominators(cfg); + for (const e of computeControlDependence(cfg, tree).edges) { + expect(e.controllerBlock).toBeGreaterThanOrEqual(0); + expect(e.controllerBlock).toBeLessThan(cfg.blocks.length); + expect(e.dependentBlock).toBeGreaterThanOrEqual(0); + expect(e.dependentBlock).toBeLessThan(cfg.blocks.length); + expect(['T', 'F']).toContain(e.label); + } + } + }); +}); diff --git a/gitnexus/test/integration/cfg/cfg-emit.test.ts b/gitnexus/test/integration/cfg/cfg-emit.test.ts index 98efe908a..84459e0aa 100644 --- a/gitnexus/test/integration/cfg/cfg-emit.test.ts +++ b/gitnexus/test/integration/cfg/cfg-emit.test.ts @@ -2,10 +2,20 @@ import { describe, it, expect, vi } from 'vitest'; import Parser from 'tree-sitter'; import TypeScript from 'tree-sitter-typescript'; import { collectFunctionCfgs } from '../../../src/core/ingestion/cfg/collect.js'; -import { emitFileCfgs, emitFileReachingDefs } from '../../../src/core/ingestion/cfg/emit.js'; +import { + emitFileCfgs, + emitFileReachingDefs, + emitFileCdg, + POST_DOMINATE_DEBUG_ENV, +} from '../../../src/core/ingestion/cfg/emit.js'; import { getProvider } from '../../../src/core/ingestion/languages/index.js'; import { SupportedLanguages } from '../../../src/config/supported-languages.js'; -import type { CfgVisitor, FunctionCfg } from '../../../src/core/ingestion/cfg/types.js'; +import type { + BasicBlockData, + CfgEdgeData, + CfgVisitor, + FunctionCfg, +} from '../../../src/core/ingestion/cfg/types.js'; import type { SyntaxNode } from '../../../src/core/ingestion/utils/ast-helpers.js'; import type { KnowledgeGraph } from '../../../src/core/graph/types.js'; @@ -338,3 +348,156 @@ describe('U4 (#2082 M2) — emitFileReachingDefs', () => { expect(rels.slice(firstIds.length).map((e) => e.id)).toEqual(firstIds); }); }); + +describe('U4 (#2085 M5) — emitFileCdg', () => { + it('emits CDG edges between BasicBlocks with the branch label in reason', () => { + // if/else diamond → both arms control-dependent on the branch (T and F) + const cfgs = cfgsOf( + `function f(x: number) { if (x) { a(); } else { b(); } c(); }`, + 'src/cdg.ts', + ); + const { graph, rels } = recordingGraph(); + const r = emitFileCdg(graph, cfgs); + + expect(r.edges).toBe(rels.length); + expect(rels.length).toBeGreaterThan(0); + for (const e of rels) { + expect(e.type).toBe('CDG'); + expect(e.sourceId).toMatch(/^BasicBlock:src\/cdg\.ts:\d+:\d+:\d+$/); + expect(e.targetId).toMatch(/^BasicBlock:src\/cdg\.ts:\d+:\d+:\d+$/); + expect(['T', 'F']).toContain(e.reason); // label rides reason (KTD3) + } + const labels = new Set(rels.map((e) => e.reason)); + expect(labels.has('T')).toBe(true); + expect(labels.has('F')).toBe(true); + expect(r.postDominateEdges).toBe(0); // debug env off + }); + + it('a straight-line function has no control dependence', () => { + const cfgs = cfgsOf(`function f() { a(); b(); c(); }`, 'lin.ts'); + const { graph, rels } = recordingGraph(); + const r = emitFileCdg(graph, cfgs); + expect(rels).toHaveLength(0); + expect(r.edges).toBe(0); + }); + + it('deduped edge ids are unique and deterministic across runs', () => { + const cfgs = cfgsOf( + `function f(x: number, y: number) { if (x) { if (y) { a(); } else { b(); } } else { c(); } }`, + 'det.ts', + ); + const first = recordingGraph(); + emitFileCdg(first.graph, cfgs); + const ids = first.rels.map((e) => e.id); + expect(new Set(ids).size).toBe(ids.length); // no id collisions + const second = recordingGraph(); + emitFileCdg(second.graph, cfgs); + expect(second.rels.map((e) => e.id)).toEqual(ids); // deterministic + }); + + it('per-function edge cap stops at the cap, records the drop, and warns (R6)', () => { + const cfgs = cfgsOf( + `function f(x: number, y: number) { if (x) { if (y) { a(); } else { b(); } } else { c(); } }`, + 'cap.ts', + ); + const full = recordingGraph(); + const total = emitFileCdg(full.graph, cfgs).edges; + expect(total).toBeGreaterThan(1); + + const { graph, rels } = recordingGraph(); + const onWarn = vi.fn(); + const r = emitFileCdg(graph, cfgs, 1, onWarn); + expect(rels.length).toBe(1); // emitted exactly the cap + expect(r.droppedEdges).toBe(total - 1); + expect(r.cappedFunctions).toBe(1); + expect(onWarn).toHaveBeenCalledTimes(1); + expect(onWarn.mock.calls[0][0]).toContain('CDG edge cap'); + }); + + it('cap of 0 means unlimited (no warning)', () => { + const cfgs = cfgsOf(`function f(x: number) { if (x) { a(); } else { b(); } }`, 'u.ts'); + const { graph, rels } = recordingGraph(); + const onWarn = vi.fn(); + const r = emitFileCdg(graph, cfgs, 0, onWarn); + expect(rels.length).toBe(r.edges); + expect(r.droppedEdges).toBe(0); + expect(onWarn).not.toHaveBeenCalled(); + }); + + it('skips CDG for a CFG whose EXIT is unreachable from all blocks (#2188 unsound guard)', () => { + // Hand-built exit-less loop: 0=entry → 1 ⇄ 2 spin forever; 3=exit is + // disconnected. Post-dominance would be unsound there, so CDG is skipped — + // while a normal sibling function in the same batch still emits CDG. + const blocks: BasicBlockData[] = [0, 1, 2, 3].map((i) => ({ + index: i, + startLine: i + 1, + endLine: i + 1, + text: '', + kind: i === 0 ? 'entry' : i === 3 ? 'exit' : 'normal', + })); + const edges: CfgEdgeData[] = [ + { from: 0, to: 1, kind: 'seq' }, + { from: 1, to: 2, kind: 'seq' }, + { from: 2, to: 1, kind: 'seq' }, + ]; + const unsound: FunctionCfg = { + filePath: 'spin.ts', + functionStartLine: 1, + functionStartColumn: 0, + entryIndex: 0, + exitIndex: 3, + blocks, + edges, + }; + const sound = cfgsOf( + `function f(x: number) { if (x) { a(); } else { b(); } c(); }`, + 'sound.ts', + )[0]; + + const { graph, rels } = recordingGraph(); + const onWarn = vi.fn(); + const r = emitFileCdg(graph, [unsound, sound], 0, onWarn); + + expect(r.skippedUnsoundFunctions).toBe(1); + // No CDG edge originates from the unsound function... + expect(rels.some((e) => e.sourceId.startsWith('BasicBlock:spin.ts:'))).toBe(false); + // ...but the sound sibling still emitted CDG normally. + expect(rels.length).toBeGreaterThan(0); + expect(rels.every((e) => e.sourceId.startsWith('BasicBlock:sound.ts:'))).toBe(true); + expect(r.edges).toBe(rels.length); + expect(onWarn).toHaveBeenCalledTimes(1); + expect(onWarn.mock.calls[0][0]).toContain('EXIT not reachable'); + }); + + it('emits POST_DOMINATE debug edges only when the env flag is set (KTD8)', () => { + const cfgs = cfgsOf(`function f(x: number) { if (x) { a(); } else { b(); } c(); }`, 'pd.ts'); + + // flag unset → no POST_DOMINATE edges + const off = recordingGraph(); + const rOff = emitFileCdg(off.graph, cfgs); + expect(off.rels.some((e) => e.type === 'POST_DOMINATE')).toBe(false); + expect(rOff.postDominateEdges).toBe(0); + + // flag set → POST_DOMINATE edges appear (not counted against CDG cap) + const prev = process.env[POST_DOMINATE_DEBUG_ENV]; + process.env[POST_DOMINATE_DEBUG_ENV] = '1'; + try { + const on = recordingGraph(); + const rOn = emitFileCdg(on.graph, cfgs); + const pd = on.rels.filter((e) => e.type === 'POST_DOMINATE'); + expect(pd.length).toBeGreaterThan(0); + expect(rOn.postDominateEdges).toBe(pd.length); + // CDG edge count is unchanged by the debug flag + expect(rOn.edges).toBe(rOff.edges); + + // the case-insensitive 'true' OR-branch of postDominateDebugEnabled + process.env[POST_DOMINATE_DEBUG_ENV] = 'TRUE'; + const onTrue = recordingGraph(); + const rOnTrue = emitFileCdg(onTrue.graph, cfgs); + expect(rOnTrue.postDominateEdges).toBeGreaterThan(0); + } finally { + if (prev === undefined) delete process.env[POST_DOMINATE_DEBUG_ENV]; + else process.env[POST_DOMINATE_DEBUG_ENV] = prev; + } + }); +}); diff --git a/gitnexus/test/integration/cfg/fixtures/pdg-repo/guards.ts b/gitnexus/test/integration/cfg/fixtures/pdg-repo/guards.ts new file mode 100644 index 000000000..161332d6e --- /dev/null +++ b/gitnexus/test/integration/cfg/fixtures/pdg-repo/guards.ts @@ -0,0 +1,24 @@ +// Guard-clause + data-flow fixture for the pdg_query integration test (#2086). +// Kept free of taint sources/sinks so it adds no TAINTED findings to the shared +// pdg-repo fixture (taint-explain / pipeline-pdg assert on taint dynamically). + +export function guarded(ok: boolean, x: number): number { + // `if (!ok) return` — the early return is control-dependent on the guard + // predicate (the #559 guard-clause shape); the post-guard body is + // control-dependent on the complementary arm. + if (!ok) { + return -1; + } + const y = x * 2; + const z = y + 1; + return z; +} + +export function loopFlow(items: number[]): number { + // A loop-carried accumulator — exercises REACHING_DEF (def→use of `sum`). + let sum = 0; + for (const it of items) { + sum = sum + it; + } + return sum; +} diff --git a/gitnexus/test/integration/cfg/pipeline-pdg.test.ts b/gitnexus/test/integration/cfg/pipeline-pdg.test.ts index d5d8415b3..2177c761a 100644 --- a/gitnexus/test/integration/cfg/pipeline-pdg.test.ts +++ b/gitnexus/test/integration/cfg/pipeline-pdg.test.ts @@ -21,6 +21,7 @@ function counts(result: PipelineResult): { reachingDefs: number; tainted: number; sanitizes: number; + cdg: number; } { let basicBlocks = 0; result.graph.forEachNode((n) => { @@ -30,13 +31,15 @@ function counts(result: PipelineResult): { let reachingDefs = 0; let tainted = 0; let sanitizes = 0; + let cdg = 0; for (const rel of result.graph.iterRelationships()) { if (rel.type === 'CFG') cfgEdges++; if (rel.type === 'REACHING_DEF') reachingDefs++; if (rel.type === 'TAINTED') tainted++; if (rel.type === 'SANITIZES') sanitizes++; + if (rel.type === 'CDG') cdg++; } - return { basicBlocks, cfgEdges, reachingDefs, tainted, sanitizes }; + return { basicBlocks, cfgEdges, reachingDefs, tainted, sanitizes, cdg }; } const tmpDirs: string[] = []; @@ -138,13 +141,49 @@ describe('U7 — end-to-end --pdg pipeline', () => { expect(sawVulnFlow).toBe(true); // the req.body → exec flow, via `cmd` }, 60000); - it('with --pdg off (default): emits zero BasicBlock nodes and zero CFG edges', async () => { + // M5 (#2085 U6): control dependence rides the same `--pdg` gate. AC3 — the + // CDG edges make "under what condition does block X run?" answerable: each + // edge's source is the controlling branch block and its `reason` is the + // 'T'|'F' sense. (The dedicated `pdg_query` MCP tool is #2086; here the raw + // graph carries the answer.) + it('with --pdg on: emits CDG edges (controller→dependent, T/F label) — AC3 answerability', async () => { + const result = await runPipelineFromRepo(freshRepo(), () => {}, { pdg: true }); + const { cdg } = counts(result); + expect(cdg).toBeGreaterThan(0); + + const blockIds = new Set(); + result.graph.forEachNode((n) => { + if (n.label === 'BasicBlock') blockIds.add(n.id); + }); + + // "What controls block X?" — index CDG edges by dependent block. Every CDG + // edge connects two persisted BasicBlocks and carries a T/F label. + const controllersOf = new Map(); + for (const rel of result.graph.iterRelationships()) { + if (rel.type !== 'CDG') continue; + expect(blockIds.has(rel.sourceId)).toBe(true); + expect(blockIds.has(rel.targetId)).toBe(true); + expect(['T', 'F']).toContain(rel.reason); + const list = controllersOf.get(rel.targetId) ?? []; + list.push({ controller: rel.sourceId, label: rel.reason }); + controllersOf.set(rel.targetId, list); + } + // At least one block has its controlling branch + condition recoverable — + // the query "under what condition does this block run?" is answerable. + expect(controllersOf.size).toBeGreaterThan(0); + for (const [, controls] of controllersOf) { + expect(controls.length).toBeGreaterThan(0); + } + }, 60000); + + it('with --pdg off (default): emits zero BasicBlock nodes and zero CFG/CDG edges', async () => { const result = await runPipelineFromRepo(freshRepo(), () => {}); - const { basicBlocks, cfgEdges, reachingDefs, tainted, sanitizes } = counts(result); + const { basicBlocks, cfgEdges, reachingDefs, tainted, sanitizes, cdg } = counts(result); expect(basicBlocks).toBe(0); expect(cfgEdges).toBe(0); expect(reachingDefs).toBe(0); expect(tainted).toBe(0); expect(sanitizes).toBe(0); + expect(cdg).toBe(0); }, 60000); }); diff --git a/gitnexus/test/integration/pdg-query.test.ts b/gitnexus/test/integration/pdg-query.test.ts new file mode 100644 index 000000000..74bf232d0 --- /dev/null +++ b/gitnexus/test/integration/pdg-query.test.ts @@ -0,0 +1,485 @@ +/** + * Integration Tests: MCP `pdg_query` tool (#2086 M6) + * + * End-to-end against a REAL LadybugDB: the pdg-repo fixture is indexed by the + * real pipeline with `--pdg` (workers — requires `node scripts/build.js`), the + * resulting BasicBlock nodes + CDG/REACHING_DEF edges and the fixture's + * Function symbols are persisted into the test DB, and `pdg_query` is exercised + * through the full `callTool` dispatch: + * + * - controls mode: "under what condition does X run?" (CDG), incl. the + * guard-clause subset (early-return block, #559 subsumption / R1) + * - flows mode: "where does variable Y flow?" (REACHING_DEF def→use) / R2 + * - symbol + file anchoring; required-target / invalid-mode / bad-limit errors + * - a repo WITHOUT the pdg layer → the "no PDG layer" note, not an error + * + * Seeding via the real emit output (not hand-written rows) pins the format + * compatibility between the M5/M2 write path and the M6 read path — the + * BasicBlock id template + the 'T'/'F' / variable `reason` semantics. + */ +import { describe, it, expect, beforeAll, vi } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos, loadMeta } from '../../src/storage/repo-manager.js'; +import { withTestLbugDB } from '../helpers/test-indexed-db.js'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), + // No meta.json for the seeded test DB — pdg_query's meta probe degrades to + // the row-existence probe (the seeded-DB reality, like taint-explain). + loadMeta: vi.fn().mockResolvedValue(null), + }; +}); + +const FIXTURE = path.join(__dirname, 'cfg', 'fixtures', 'pdg-repo'); + +// ─── Block 1: a --pdg index with real CDG + REACHING_DEF edges ─────── + +withTestLbugDB( + 'pdg-query', + (handle) => { + describe('pdg_query against a --pdg index', () => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as typeof handle & { _backend?: LocalBackend }; + if (!ext._backend) throw new Error('LocalBackend not initialized in afterSetup'); + backend = ext._backend; + }); + + it('controls mode answers "what controls X" and flags the guard clause (R1)', async () => { + const result = await backend.callTool('pdg_query', { mode: 'controls', target: 'guarded' }); + expect(result).not.toHaveProperty('error'); + expect(result.mode).toBe('controls'); + expect(result.anchor.symbol).toBe('guarded'); + expect(result.results.length).toBeGreaterThan(0); + // every edge has a 'T'/'F' branch label + for (const e of result.results) expect(['T', 'F']).toContain(e.label); + // the early `return -1` is control-dependent on the guard predicate → + // flagged guard:true (the #559 guard-clause subsumption) + const guardEdge = result.results.find((e: any) => e.guard === true); + expect(guardEdge, 'a guard-clause edge into an early-exit block').toBeDefined(); + expect(guardEdge.dependent.text).toMatch(/return/); + }); + + it('flows mode answers "where does variable Y flow" (R2)', async () => { + const result = await backend.callTool('pdg_query', { + mode: 'flows', + target: 'loopFlow', + variable: 'sum', + }); + expect(result).not.toHaveProperty('error'); + expect(result.mode).toBe('flows'); + expect(result.results.length).toBeGreaterThan(0); + for (const e of result.results) expect(e.variable).toBe('sum'); + }); + + it('flows mode without a variable filter returns all def→use edges for the anchor', async () => { + const result = await backend.callTool('pdg_query', { mode: 'flows', target: 'loopFlow' }); + expect(result).not.toHaveProperty('error'); + expect(result.results.length).toBeGreaterThan(0); + expect(result.results.some((e: any) => e.variable === 'sum')).toBe(true); + }); + + it('controls mode anchors by file path too', async () => { + const result = await backend.callTool('pdg_query', { + mode: 'controls', + target: 'guards.ts', + }); + expect(result).not.toHaveProperty('error'); + expect(result.results.length).toBeGreaterThan(0); + }); + + it('rejects a missing target (PDG queries are always anchored)', async () => { + const result = await backend.callTool('pdg_query', { mode: 'controls' }); + expect(result).toHaveProperty('error'); + expect(result.error).toMatch(/target/i); + }); + + it('rejects an invalid mode', async () => { + const result = await backend.callTool('pdg_query', { mode: 'slice', target: 'guarded' }); + expect(result).toHaveProperty('error'); + expect(result.error).toMatch(/mode/i); + }); + + it('rejects an out-of-bounds limit', async () => { + for (const limit of [0, -1, 1.5, 10_000, NaN]) { + const result = await backend.callTool('pdg_query', { + mode: 'controls', + target: 'guarded', + limit, + }); + expect(result).toHaveProperty('error'); + expect(result.error).toMatch(/limit/i); + } + }); + + it('an unknown symbol target mirrors context() not-found semantics', async () => { + const result = await backend.callTool('pdg_query', { + mode: 'controls', + target: 'nonexistentPdgFn999', + }); + expect(result).toHaveProperty('error'); + expect(result.error).toMatch(/not found/i); + }); + + it('a call with no arguments returns a clean validation error, not a crash (#2188)', async () => { + // An MCP client may send {"name":"pdg_query"} with no `arguments` field; + // the dispatch then hands `params: undefined` to the impl. It must + // default to {} and surface the mode-validation error, not a TypeError. + const result = await backend.callTool('pdg_query'); + expect(result).toHaveProperty('error'); + expect(result.error).toMatch(/mode/i); + }); + }); + }, + { + poolAdapter: true, + afterSetup: async (handle) => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-pdgq-')); + try { + fs.cpSync(FIXTURE, repoDir, { recursive: true }); + const pipelineResult = await runPipelineFromRepo(repoDir, () => {}, { pdg: true }); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const nodes: Array<{ label: string; props: Record }> = []; + pipelineResult.graph.forEachNode((n) => { + if (n.label === 'BasicBlock') { + nodes.push({ + label: 'BasicBlock', + props: { + id: n.id, + filePath: n.properties.filePath ?? '', + startLine: n.properties.startLine ?? 0, + endLine: n.properties.endLine ?? 0, + text: n.properties.text ?? '', + }, + }); + } else if (n.label === 'Function') { + nodes.push({ + label: 'Function', + props: { + id: n.id, + name: n.properties.name ?? '', + filePath: n.properties.filePath ?? '', + startLine: n.properties.startLine ?? 0, + endLine: n.properties.endLine ?? 0, + }, + }); + } + }); + for (const node of nodes) { + const assignments = Object.keys(node.props) + .map((k) => `${k}: $${k}`) + .join(', '); + await adapter.executePrepared( + `CREATE (n:${node.label} {${assignments}})`, + node.props as Record, + ); + } + let pdgEdges = 0; + for (const rel of pipelineResult.graph.iterRelationships()) { + if (rel.type !== 'CDG' && rel.type !== 'REACHING_DEF') continue; + await adapter.executePrepared( + `MATCH (a:BasicBlock {id: $src}), (b:BasicBlock {id: $dst}) + CREATE (a)-[:CodeRelation {type: '${rel.type}', confidence: $confidence, reason: $reason, step: 0}]->(b)`, + { + src: rel.sourceId, + dst: rel.targetId, + confidence: rel.confidence ?? 1.0, + reason: rel.reason ?? '', + }, + ); + pdgEdges++; + } + if (pdgEdges === 0) { + throw new Error('fixture produced no CDG/REACHING_DEF edges — pdg emit regressed?'); + } + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'pdg-repo', + path: '/pdg/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + stats: { files: 4, nodes: 4, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); + +// ─── Block 2: a repo indexed WITHOUT --pdg ─────────────────────────── + +withTestLbugDB( + 'pdg-query-nopdg', + (handle) => { + describe('pdg_query without a PDG layer', () => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as typeof handle & { _backend?: LocalBackend }; + if (!ext._backend) throw new Error('LocalBackend not initialized in afterSetup'); + backend = ext._backend; + }); + + it('controls returns the status-unknown note when meta is unreadable + probe empty (#2188)', async () => { + // Meta is mocked unreadable (null) and the seed has no CDG rows. A + // missing layer is indistinguishable from an edge-free one here, so the + // note is inconclusive ("status unknown"), not the definitive absence. + const result = await backend.callTool('pdg_query', { mode: 'controls', target: 'plainFn' }); + expect(result).not.toHaveProperty('error'); + expect(result.results).toEqual([]); + expect(result.note).toMatch(/status unknown/i); + expect(result.note).not.toMatch(/no PDG layer/i); + expect(result.note).toContain('--pdg'); + }); + + it('flows returns the status-unknown note too when meta is unreadable', async () => { + const result = await backend.callTool('pdg_query', { mode: 'flows', target: 'plain.ts' }); + expect(result).not.toHaveProperty('error'); + expect(result.results).toEqual([]); + expect(result.note).toMatch(/status unknown/i); + }); + + it('a readable meta without a pdg stamp short-circuits to the DEFINITIVE no-layer note', async () => { + // Meta is readable but carries no CDG cap ⇒ the layer truly was never + // recorded; this path keeps the definitive "no PDG layer" wording. + vi.mocked(loadMeta).mockResolvedValueOnce({} as any); + const result = await backend.callTool('pdg_query', { mode: 'controls', target: 'plainFn' }); + expect(result.results).toEqual([]); + expect(result.note).toMatch(/no PDG layer/i); + }); + }); + }, + { + seed: [ + `CREATE (fn:Function {id: 'func:plainFn', name: 'plainFn', filePath: 'src/plain.ts', startLine: 1, endLine: 5, isExported: true, content: 'function plainFn() {}', description: 'no pdg layer here'})`, + ], + poolAdapter: true, + afterSetup: async (handle) => { + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'plain-repo', + path: '/plain/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'def456', + stats: { files: 1, nodes: 1, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); + +// ─── Block 3: symbol-anchor line-base off-by-one (#2188 review) ────── +// +// Hand-seeded with controlled line numbers (no parser dependency): `targetFn` +// occupies 0-based symbol lines 10–14, and a neighbor function sits directly +// above it with its last block on 1-based line 10 — the line right above +// targetFn's declaration (1-based line 11). BasicBlock startLine is 1-based +// while the symbol span is 0-based, so the anchor window must be [11,15] (both +// bounds shifted +1). The pre-fix window [10,15] (lower bound left 0-based) +// over-includes the neighbor's line-10 block. This pins the lower-bound +1. + +withTestLbugDB( + 'pdg-query-adjacency', + (handle) => { + describe('pdg_query symbol anchoring (#2188 lower-bound off-by-one)', () => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as typeof handle & { _backend?: LocalBackend }; + if (!ext._backend) throw new Error('LocalBackend not initialized in afterSetup'); + backend = ext._backend; + }); + + it('excludes a neighbor function block on the line directly above the target', async () => { + const result = await backend.callTool('pdg_query', { + mode: 'controls', + target: 'targetFn', + }); + expect(result).not.toHaveProperty('error'); + // Only targetFn's own control edge — the neighbor's line-10 edge is out + // of the [11,15] window after the lower-bound +1 fix. + expect(result.results).toHaveLength(1); + expect(result.results[0].dependent.text).toMatch(/doThing/); + expect(result.results[0].functionLine).toBe(11); + expect(result.results.some((e: any) => /aboveDep/.test(e.dependent.text))).toBe(false); + }); + }); + }, + { + poolAdapter: true, + afterSetup: async (handle) => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const nodeStmts = [ + `CREATE (fn:Function {id: 'func:targetFn', name: 'targetFn', filePath: 'src/adj.ts', startLine: 10, endLine: 14, isExported: true, content: 'function targetFn(x) {}', description: 'adjacency regression'})`, + // targetFn's blocks (fnStartLine segment '11', 1-based startLines 12/13) + `CREATE (b:BasicBlock {id: 'BasicBlock:src/adj.ts:11:0:0', filePath: 'src/adj.ts', startLine: 12, endLine: 12, text: 'if (x)'})`, + `CREATE (b:BasicBlock {id: 'BasicBlock:src/adj.ts:11:0:1', filePath: 'src/adj.ts', startLine: 13, endLine: 13, text: 'doThing();'})`, + // neighbor function's blocks (fnStartLine segment '9', 1-based startLine 10) + `CREATE (b:BasicBlock {id: 'BasicBlock:src/adj.ts:9:0:0', filePath: 'src/adj.ts', startLine: 10, endLine: 10, text: 'if (above)'})`, + `CREATE (b:BasicBlock {id: 'BasicBlock:src/adj.ts:9:0:1', filePath: 'src/adj.ts', startLine: 10, endLine: 10, text: 'aboveDep();'})`, + ]; + for (const s of nodeStmts) await adapter.executePrepared(s, {}); + const cdgEdge = (src: string, dst: string) => + adapter.executePrepared( + `MATCH (a:BasicBlock {id: $src}), (b:BasicBlock {id: $dst}) + CREATE (a)-[:CodeRelation {type: 'CDG', confidence: 1.0, reason: 'T', step: 0}]->(b)`, + { src, dst }, + ); + await cdgEdge('BasicBlock:src/adj.ts:11:0:0', 'BasicBlock:src/adj.ts:11:0:1'); + await cdgEdge('BasicBlock:src/adj.ts:9:0:0', 'BasicBlock:src/adj.ts:9:0:1'); + + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'adj-repo', + path: '/adj/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'adj789', + stats: { files: 1, nodes: 5, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); + +// ─── Block 4: coverage gaps — ambiguous, truncated, Windows-':' path (#2188) ── +// +// Hand-seeded edge cases the M6 review flagged as untested. + +withTestLbugDB( + 'pdg-query-gaps', + (handle) => { + describe('pdg_query coverage gaps (#2188)', () => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as typeof handle & { _backend?: LocalBackend }; + if (!ext._backend) throw new Error('LocalBackend not initialized in afterSetup'); + backend = ext._backend; + }); + + it('an ambiguous symbol name returns ranked candidates, not a guess', async () => { + const result = await backend.callTool('pdg_query', { mode: 'controls', target: 'dupFn' }); + expect(result.status).toBe('ambiguous'); + expect(Array.isArray(result.candidates)).toBe(true); + expect(result.candidates.length).toBeGreaterThanOrEqual(2); + for (const c of result.candidates) { + expect(c).toHaveProperty('uid'); + expect(c.name).toBe('dupFn'); + expect(c).toHaveProperty('filePath'); + expect(typeof c.score).toBe('number'); + } + }); + + it('paginates: results capped at limit, total reports the full count, truncated set', async () => { + const result = await backend.callTool('pdg_query', { + mode: 'controls', + target: 'busyFn', + limit: 2, + }); + expect(result).not.toHaveProperty('error'); + expect(result.results).toHaveLength(2); + expect(result.total).toBe(3); + expect(result.truncated).toBe(true); + }); + + it('does not set truncated when the page holds every match', async () => { + const result = await backend.callTool('pdg_query', { + mode: 'controls', + target: 'busyFn', + limit: 50, + }); + expect(result.results).toHaveLength(3); + expect(result.total).toBe(3); + expect(result).not.toHaveProperty('truncated'); + }); + + it("decodes functionLine for a Windows-style filePath containing ':' (split-from-right)", async () => { + const result = await backend.callTool('pdg_query', { mode: 'controls', target: 'winFn' }); + expect(result).not.toHaveProperty('error'); + expect(result.results.length).toBeGreaterThan(0); + // id = BasicBlock:C:/src/win.ts:6:0:0 ⇒ fnLine segment '6' despite the + // ':' in the drive letter (fnLineOf splits from the right). + expect(result.results[0].functionLine).toBe(6); + }); + }); + }, + { + poolAdapter: true, + afterSetup: async (handle) => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const fn = (id: string, name: string, filePath: string, startLine: number, endLine: number) => + adapter.executePrepared( + `CREATE (fn:Function {id: $id, name: $name, filePath: $filePath, startLine: $startLine, endLine: $endLine, isExported: true, content: 'x', description: 'gap fixture'})`, + { id, name, filePath, startLine, endLine }, + ); + const block = (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 }, + ); + const cdg = (src: string, dst: string) => + adapter.executePrepared( + `MATCH (a:BasicBlock {id: $src}), (b:BasicBlock {id: $dst}) + CREATE (a)-[:CodeRelation {type: 'CDG', confidence: 1.0, reason: 'T', step: 0}]->(b)`, + { src, dst }, + ); + + // (1) Ambiguous: two functions sharing a name in different files. + await fn('func:dupFn@a', 'dupFn', 'a.ts', 1, 3); + await fn('func:dupFn@b', 'dupFn', 'b.ts', 1, 3); + + // (2) Truncated: busyFn (0-based 10–20 ⇒ window [11,21]); one controller + // block (line 12) with three CDG dependents. + await fn('func:busyFn', 'busyFn', 'busy.ts', 10, 20); + await block('BasicBlock:busy.ts:11:0:0', 'busy.ts', 12, 'if (x)'); + await block('BasicBlock:busy.ts:11:0:1', 'busy.ts', 13, 'a();'); + await block('BasicBlock:busy.ts:11:0:2', 'busy.ts', 14, 'b();'); + await block('BasicBlock:busy.ts:11:0:3', 'busy.ts', 15, 'c();'); + await cdg('BasicBlock:busy.ts:11:0:0', 'BasicBlock:busy.ts:11:0:1'); + await cdg('BasicBlock:busy.ts:11:0:0', 'BasicBlock:busy.ts:11:0:2'); + await cdg('BasicBlock:busy.ts:11:0:0', 'BasicBlock:busy.ts:11:0:3'); + + // (3) Windows-style path with a ':' (drive letter) inside the block id. + await fn('func:winFn', 'winFn', 'C:/src/win.ts', 5, 8); + await block('BasicBlock:C:/src/win.ts:6:0:0', 'C:/src/win.ts', 7, 'if (y)'); + await block('BasicBlock:C:/src/win.ts:6:0:1', 'C:/src/win.ts', 7, 'd();'); + await cdg('BasicBlock:C:/src/win.ts:6:0:0', 'BasicBlock:C:/src/win.ts:6:0:1'); + + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'gaps-repo', + path: '/gaps/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'gap001', + stats: { files: 4, nodes: 12, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/taint-explain.test.ts b/gitnexus/test/integration/taint-explain.test.ts index 8d999ccfc..7f500679b 100644 --- a/gitnexus/test/integration/taint-explain.test.ts +++ b/gitnexus/test/integration/taint-explain.test.ts @@ -479,3 +479,84 @@ withTestLbugDB( }, }, ); + +// ─── Block 4: symbol-anchor window correctness (#2188 _explainImpl off-by-one) ── +// +// Hand-seeded with controlled line numbers (no parser dependency). `tailFn` +// occupies 0-based symbol lines 10–14, so its BasicBlocks land on 1-based lines +// 11–15 and the correct anchor window is [symStart+1, symEnd+1] = [11,15]. The +// pre-fix _explainImpl used [symStart, symEnd] = [10,14], which both DROPPED a +// taint source on the function's final line (1-based 15) and LEAKED a neighbor's +// block on the line directly above (1-based 10). One query proves both bounds — +// and FAILS on the pre-fix window (it would return the line-10 neighbor instead). + +withTestLbugDB( + 'taint-explain-anchor-window', + (handle) => { + describe('explain symbol anchoring (#2188 [symStart+1, symEnd+1] window)', () => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as typeof handle & { _backend?: LocalBackend }; + if (!ext._backend) throw new Error('LocalBackend not initialized'); + backend = ext._backend; + }); + + it('includes the final-line taint source and excludes the neighbor-above block', async () => { + const result = (await backend.callTool('explain', { target: 'tailFn' })) as { + findings: Array<{ source?: { line?: number } }>; + error?: string; + }; + expect(result).not.toHaveProperty('error'); + // Only tailFn's own final-line (1-based 15) taint source survives; the + // neighbor's line-10 block is below the [11,15] window (lower-bound +1). + expect(result.findings).toHaveLength(1); + expect(result.findings[0].source?.line).toBe(15); + expect(result.findings.some((f) => f.source?.line === 10)).toBe(false); + }); + }); + }, + { + poolAdapter: true, + afterSetup: async (handle) => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + // tailFn: 0-based span 10–14 ⇒ 1-based blocks on 11–15, window [11,15]. + await adapter.executePrepared( + `CREATE (fn:Function {id: 'func:tailFn', name: 'tailFn', filePath: 'anchor.ts', startLine: 10, endLine: 14, isExported: true, content: 'function tailFn() {}', description: 'anchor-window regression'})`, + {}, + ); + const block = (id: string, startLine: number, text: string) => + adapter.executePrepared( + `CREATE (b:BasicBlock {id: $id, filePath: 'anchor.ts', startLine: $startLine, endLine: $startLine, text: $text})`, + { id, startLine, text }, + ); + // tailFn's source/sink on its FINAL line (1-based 15 = endLine 14 + 1). + await block('BasicBlock:anchor.ts:11:0:5', 15, 'const x = req.body;'); + await block('BasicBlock:anchor.ts:11:0:6', 15, 'exec(x);'); + // a neighbor function's block on the line directly ABOVE tailFn (1-based 10). + await block('BasicBlock:anchor.ts:9:0:0', 10, 'const y = other();'); + await block('BasicBlock:anchor.ts:9:0:1', 10, 'use(y);'); + const tainted = (src: string, dst: string, reason: string) => + adapter.executePrepared( + `MATCH (a:BasicBlock {id: $src}), (b:BasicBlock {id: $dst}) + CREATE (a)-[:CodeRelation {type: 'TAINTED', confidence: 1.0, reason: $reason, step: 0}]->(b)`, + { src, dst, reason }, + ); + await tainted('BasicBlock:anchor.ts:11:0:5', 'BasicBlock:anchor.ts:11:0:6', 'tail'); + await tainted('BasicBlock:anchor.ts:9:0:0', 'BasicBlock:anchor.ts:9:0:1', 'neighbor'); + + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'anchor-repo', + path: '/anchor/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'aw0001', + stats: { files: 1, nodes: 5, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/unit/ai-context.test.ts b/gitnexus/test/unit/ai-context.test.ts index 747bcec1a..30a9441c1 100644 --- a/gitnexus/test/unit/ai-context.test.ts +++ b/gitnexus/test/unit/ai-context.test.ts @@ -138,8 +138,7 @@ describe('generateAIContextFiles', () => { const content = generateGitNexusContent( 'TestProject', { nodes: 50, edges: 100, processes: 5 }, - undefined, - ['TeamGroup'], + { groupNames: ['TeamGroup'] }, ); expect(content).toContain('## Cross-Repo Groups'); expect(content).toContain('node .gitnexus/run.cjs group list'); @@ -150,6 +149,20 @@ describe('generateAIContextFiles', () => { expect(content).not.toMatch(/npx gitnexus group/); }); + it('gates the pdg_query line on hasPdg (#2086 M6 — no existing taint gate to mirror)', () => { + const stats = { nodes: 50, edges: 100, processes: 5 }; + // hasPdg=true → the pdg_query line is present. + const withPdg = generateGitNexusContent('PdgProject', stats, { hasPdg: true }); + expect(withPdg).toContain('pdg_query'); + expect(withPdg).toContain('under what condition does X run'); + // hasPdg omitted (default false) → no pdg_query line; a non-pdg index must + // not advertise a tool that only returns a "no PDG layer" note. + const withoutPdg = generateGitNexusContent('PlainProject', stats); + expect(withoutPdg).not.toContain('pdg_query'); + // the unconditional explain line stays regardless of the pdg flag. + expect(withoutPdg).toContain('explain('); + }); + it('degrades gracefully when the runner copy fails (#1945)', async () => { // A read-only/full-disk storage dir must not abort generation. The copy is // best-effort + logged; the generated docs still carry the inline bootstrap @@ -893,16 +906,7 @@ Indexed as **placeholder** (1 symbols, 1 relationships, 1 execution flows). Cust it('generated regression-compare example uses the configured default branch (#243)', () => { const stats = { nodes: 50, edges: 100, processes: 5 }; - const develop = generateGitNexusContent( - 'P', - stats, - undefined, - undefined, - undefined, - undefined, - undefined, - 'develop', - ); + const develop = generateGitNexusContent('P', stats, { defaultBranch: 'develop' }); expect(develop).toContain('base_ref: "develop"'); expect(develop).not.toContain('base_ref: "main"'); }); @@ -930,32 +934,14 @@ Indexed as **placeholder** (1 symbols, 1 relationships, 1 execution flows). Cust it('JSON-escapes a markdown/quote-bearing branch so it cannot break the code span (#243)', () => { // A branch name with a double-quote must be JSON-escaped, not concatenated // raw, so it stays inside the inline code span. - const content = generateGitNexusContent( - 'P', - { nodes: 1 }, - undefined, - undefined, - undefined, - undefined, - undefined, - 'we"ird', - ); + const content = generateGitNexusContent('P', { nodes: 1 }, { defaultBranch: 'we"ird' }); expect(content).toContain('base_ref: "we\\"ird"'); }); it('a backtick branch cannot break the generated Markdown code span (#1996 P1)', () => { // The branch is embedded inside a backtick inline-code span; a stray // backtick would close it early. markdownSafeBranch strips it at the sink. - const content = generateGitNexusContent( - 'P', - { nodes: 1 }, - undefined, - undefined, - undefined, - undefined, - undefined, - 'main`evil', - ); + const content = generateGitNexusContent('P', { nodes: 1 }, { defaultBranch: 'main`evil' }); const line = content.split('\n').find((l) => l.includes('base_ref'))!; // Even backtick count ⇒ every span is balanced (the regression line opens // and closes exactly one). diff --git a/gitnexus/test/unit/analyze-no-stats-bridge.test.ts b/gitnexus/test/unit/analyze-no-stats-bridge.test.ts index 22e58a061..629bfac02 100644 --- a/gitnexus/test/unit/analyze-no-stats-bridge.test.ts +++ b/gitnexus/test/unit/analyze-no-stats-bridge.test.ts @@ -168,6 +168,8 @@ describe('analyzeCommand commander → runFullAnalysis noStats bridge (#1477)', // #243: resolved default branch threaded into the --skills regen path. defaultBranch: 'main', noStats: true, + // #2086 M6: the --pdg gate is threaded too; false here (no --pdg flag). + hasPdg: false, }); } finally { exitSpy.mockRestore(); diff --git a/gitnexus/test/unit/cfg/control-dependence.test.ts b/gitnexus/test/unit/cfg/control-dependence.test.ts new file mode 100644 index 000000000..e307fb69b --- /dev/null +++ b/gitnexus/test/unit/cfg/control-dependence.test.ts @@ -0,0 +1,413 @@ +import { describe, it, expect } from 'vitest'; +import { + computeControlDependence, + type ControlDepEdge, + type CdgLabel, +} from '../../../src/core/ingestion/cfg/control-dependence.js'; +import { + computePostDominators, + postDominates, +} from '../../../src/core/ingestion/cfg/post-dominators.js'; +import type { + BasicBlockData, + CfgEdgeData, + CfgEdgeKind, + FunctionCfg, +} from '../../../src/core/ingestion/cfg/types.js'; +import Parser from 'tree-sitter'; +import TypeScript from 'tree-sitter-typescript'; +import { collectFunctionCfgs } from '../../../src/core/ingestion/cfg/collect.js'; +import { getProvider } from '../../../src/core/ingestion/languages/index.js'; +import { SupportedLanguages } from '../../../src/config/supported-languages.js'; + +// U3 (#2085 M5) — Ferrante §3.1.1 control dependence over the post-dom tree. +// Hand-built CFG literals plus real-parser regression tests. The labelled +// expected edge sets ARE the spec; the property test (AC2) cross-checks the +// tree-walk against a reference that computes post-dominance INDEPENDENTLY (by +// node-removal reachability, sharing NO code with post-dominators.ts), so a +// post-dominator *direction* bug cannot pass both (#2188 F4). + +// ── hand-built CFG helper (edges carry a kind so labels can be asserted) ───── + +function mkCfg( + blockCount: number, + edges: [number, number, CfgEdgeKind][], + opts: { entry?: number; exit?: number } = {}, +): FunctionCfg { + const entry = opts.entry ?? 0; + const exit = opts.exit ?? blockCount - 1; + const blocks: BasicBlockData[] = Array.from({ length: blockCount }, (_, i) => ({ + index: i, + startLine: i + 1, + endLine: i + 1, + text: '', + kind: i === entry ? 'entry' : i === exit ? 'exit' : 'normal', + })); + const cfgEdges: CfgEdgeData[] = edges.map(([from, to, kind]) => ({ from, to, kind })); + return { + filePath: 't.ts', + functionStartLine: 1, + functionStartColumn: 0, + entryIndex: entry, + exitIndex: exit, + blocks, + edges: cfgEdges, + }; +} + +const ser = (e: ControlDepEdge): string => `${e.controllerBlock}->${e.dependentBlock}:${e.label}`; +const serAll = (edges: readonly ControlDepEdge[]): string[] => edges.map(ser); + +/** + * Build a successor adjacency list for a CFG (in-range edges only). + */ +function succsOf(cfg: FunctionCfg): number[][] { + const n = cfg.blocks.length; + const succs: number[][] = Array.from({ length: n }, () => []); + for (const e of cfg.edges) + if (e.from >= 0 && e.from < n && e.to >= 0 && e.to < n) succs[e.from].push(e.to); + return succs; +} + +/** + * INDEPENDENT post-dominance via node-removal reachability — shares NO code with + * post-dominators.ts (that is the whole point: it must catch a CHK *direction* + * bug that a shared-substrate reference would mirror, #2188 F4). `p` + * post-dominates `b` iff every path from `b` to EXIT passes through `p`: + * reflexive (`p === b`), else true exactly when EXIT is unreachable from `b` + * once `p` is removed (AND `b` can reach EXIT at all). Defined only for the + * exit-reachable fixtures used below — the exit-unreachable case is the known + * unsound region (#2188 F2) and is deliberately excluded from the AC2 set. + */ +function independentPostDom(cfg: FunctionCfg, succs: number[][], p: number, b: number): boolean { + if (p === b) return true; + const exit = cfg.exitIndex; + const reach = (avoid: number): boolean => { + if (b === avoid) return false; + const seen = new Set([b]); + const stack = [b]; + while (stack.length) { + const x = stack.pop()!; + if (x === exit) return true; + for (const y of succs[x]) { + if (y === avoid || seen.has(y)) continue; + seen.add(y); + stack.push(y); + } + } + return false; + }; + // p post-dominates b ⇔ b reaches EXIT, but cannot reach it with p removed. + return reach(-1) && !reach(p); +} + +/** + * Reference control-dependence pairs from the Ferrante definition, using the + * INDEPENDENT post-dominance above: N is control-dependent on A iff some CFG + * edge A→B has N post-dominating B while N does NOT strictly post-dominate A. + * Label-agnostic (distinct "A->N" pairs) — the pure definition has no sense. + */ +function referencePairs(cfg: FunctionCfg): Set { + const n = cfg.blocks.length; + const succs = succsOf(cfg); + const pd = (x: number, y: number): boolean => independentPostDom(cfg, succs, x, y); + const pairs = new Set(); + for (let a = 0; a < n; a++) { + for (const b of succs[a]) { + if (pd(b, a)) continue; // edge is not a control point + for (let nn = 0; nn < n; nn++) { + const nPostDomB = pd(nn, b); + const nStrictlyPostDomA = nn !== a && pd(nn, a); + if (nPostDomB && !nStrictlyPostDomA) pairs.add(`${a}->${nn}`); + } + } + } + return pairs; +} + +describe('computeControlDependence — Ferrante §3.1.1', () => { + it('diamond: each arm is control-dependent on the branch with its own T/F label', () => { + // 0(branch) → 1(then, T), 2(else, F); 1,2 → 3(join) → 4(exit) + const cfg = mkCfg(5, [ + [0, 1, 'cond-true'], + [0, 2, 'cond-false'], + [1, 3, 'seq'], + [2, 3, 'seq'], + [3, 4, 'seq'], + ]); + const { edges } = computeControlDependence(cfg); + expect(serAll(edges).sort()).toEqual(['0->1:T', '0->2:F']); + // the join (3) post-dominates the branch, so it depends on nothing + expect(edges.some((e) => e.dependentBlock === 3)).toBe(false); + }); + + it('guard clause: the post-guard body is control-dependent on the guard (the #559/#2086 case)', () => { + // function f(x){ if(!ok(x)) return; use(x); } + // 0(entry) → 1(guard); 1 → 2(return, T) , 1 → 3(use, F); 2,3 → 4(exit) + const cfg = mkCfg(5, [ + [0, 1, 'seq'], + [1, 2, 'cond-true'], // !ok(x) → return + [1, 3, 'cond-false'], // else → use(x) + [2, 4, 'return'], + [3, 4, 'seq'], + ]); + const { edges } = computeControlDependence(cfg); + // use(x) (block 3) runs only when the guard condition is false → label 'F' + expect(serAll(edges).sort()).toEqual(['1->2:T', '1->3:F']); + }); + + it('straight-line function (no branches) has no control dependence', () => { + const cfg = mkCfg(3, [ + [0, 1, 'seq'], + [1, 2, 'seq'], + ]); + expect(computeControlDependence(cfg).edges).toEqual([]); + }); + + it('while loop: the body depends on the header, and the header is control-dependent on itself', () => { + // 0(entry) → 1(header); 1 → 2(body, T) , 1 → 3(exit, F); 2 → 1 (back-edge) + const cfg = mkCfg(4, [ + [0, 1, 'seq'], + [1, 2, 'cond-true'], + [2, 1, 'loop-back'], + [1, 3, 'cond-false'], + ]); + const { edges } = computeControlDependence(cfg); + // body(2) control-dep on header(1); header(1) control-dep on itself (the + // loop predicate gates its own re-execution — standard PDG behavior). + expect(serAll(edges).sort()).toEqual(['1->1:T', '1->2:T']); + }); + + it('switch: every case body is control-dependent on the dispatch (all T in M5)', () => { + // 0(entry) → 1(dispatch); 1 → 2,3,4 (cases); 2,3,4 → 5(exit) + const cfg = mkCfg(6, [ + [0, 1, 'seq'], + [1, 2, 'switch-case'], + [1, 3, 'switch-case'], + [1, 4, 'switch-case'], + [2, 5, 'break'], + [3, 5, 'break'], + [4, 5, 'break'], + ]); + const { edges } = computeControlDependence(cfg); + expect(serAll(edges).sort()).toEqual(['1->2:T', '1->3:T', '1->4:T']); + }); + + it('exit-less loop (KTD5): terminates and stays in-range, but the result is KNOWN-UNSOUND (#2188 F2)', () => { + // No block can reach EXIT (block 3), so every ipdom is NO_IPDOM and the + // Ferrante walk degenerates to one edge per control point. The termination / + // in-range invariants MUST hold (the walk hits NO_IPDOM immediately). The + // emitted dependence SET, however, is NOT a sound over-approximation: it both + // drops real dependences and invents spurious ones in exit-unreachable + // regions (#2188 F2). This test pins the degenerate output to document that + // behavior, NOT to bless it; the labels here are likewise indeterminate + // (no controller carries an explicit cond-true/cond-false arm, so the + // fall-through complement resolves to 'F'). The current TS visitor never + // produces such a region (every loop gets a structural header→loopExit edge). + const cfg = mkCfg(4, [ + [0, 1, 'seq'], + [1, 2, 'seq'], + [2, 1, 'loop-back'], + ]); + const { edges } = computeControlDependence(cfg); + expect(serAll(edges).sort()).toEqual(['0->1:F', '1->2:F', '2->1:F']); + for (const e of edges) { + expect(e.controllerBlock).toBeGreaterThanOrEqual(0); + expect(e.controllerBlock).toBeLessThan(cfg.blocks.length); + expect(e.dependentBlock).toBeGreaterThanOrEqual(0); + expect(e.dependentBlock).toBeLessThan(cfg.blocks.length); + } + }); + + it('is deterministic (stable sorted order across runs)', () => { + const make = (): FunctionCfg => + mkCfg(5, [ + [0, 1, 'cond-true'], + [0, 2, 'cond-false'], + [1, 3, 'seq'], + [2, 3, 'seq'], + [3, 4, 'seq'], + ]); + expect(serAll(computeControlDependence(make()).edges)).toEqual( + serAll(computeControlDependence(make()).edges), + ); + }); + + describe('AC2 — a control dependence exists iff post-dominance fails for the branch', () => { + const fixtures: Record = { + diamond: mkCfg(5, [ + [0, 1, 'cond-true'], + [0, 2, 'cond-false'], + [1, 3, 'seq'], + [2, 3, 'seq'], + [3, 4, 'seq'], + ]), + guard: mkCfg(5, [ + [0, 1, 'seq'], + [1, 2, 'cond-true'], + [1, 3, 'cond-false'], + [2, 4, 'return'], + [3, 4, 'seq'], + ]), + loop: mkCfg(4, [ + [0, 1, 'seq'], + [1, 2, 'cond-true'], + [2, 1, 'loop-back'], + [1, 3, 'cond-false'], + ]), + // NOTE: the exit-unreachable case is deliberately NOT an AC2 fixture — its + // dependence set is unsound (#2188 F2), so asserting walk == independent + // reference would (correctly) fail. It has its own characterization test + // above that documents the degenerate behavior. + // nested if: outer branch (0) → inner branch (1) or outer-else (5); + // inner branch → 2/3 → inner join (4); 4 and 5 → outer join (6, exit). + nestedIf: mkCfg( + 7, + [ + [0, 1, 'cond-true'], + [0, 5, 'cond-false'], + [1, 2, 'cond-true'], + [1, 3, 'cond-false'], + [2, 4, 'seq'], + [3, 4, 'seq'], + [4, 6, 'seq'], + [5, 6, 'seq'], + ], + { entry: 0, exit: 6 }, + ), + switchStmt: mkCfg(6, [ + [0, 1, 'seq'], + [1, 2, 'switch-case'], + [1, 3, 'switch-case'], + [1, 4, 'switch-case'], + [2, 5, 'break'], + [3, 5, 'break'], + [4, 5, 'break'], + ]), + }; + + it.each(Object.keys(fixtures))( + '%s: tree-walk pair set equals the brute-force reference', + (name) => { + const cfg = fixtures[name]; + const { edges } = computeControlDependence(cfg); + const walkPairs = new Set(edges.map((e) => `${e.controllerBlock}->${e.dependentBlock}`)); + expect(walkPairs).toEqual(referencePairs(cfg)); + }, + ); + + it.each(Object.keys(fixtures))( + '%s: for every CFG edge, it yields a dependent IFF the target does not post-dominate the source', + (name) => { + const cfg = fixtures[name]; + const tree = computePostDominators(cfg); + const { edges } = computeControlDependence(cfg); + for (const e of cfg.edges) { + const failsPostDom = !postDominates(tree, e.to, e.from); + // does THIS edge's source appear as a controller with at least one + // dependent reachable from its target? Equivalent statement of AC2: + // post-dominance failing for (from→to) ⇔ `from` is a control point. + const fromIsControlPoint = edges.some((c) => c.controllerBlock === e.from); + if (failsPostDom) { + expect( + fromIsControlPoint, + `${name}: edge ${e.from}->${e.to} should make ${e.from} a control point`, + ).toBe(true); + } + // and a self-post-dominating edge (to post-dominates from) can never + // be the SOLE reason a block is a control point: if from has only + // post-dominating successors it controls nothing. + } + }, + ); + }); +}); + +describe('computeControlDependence — maxEdges materialization ceiling (#2188)', () => { + // A switch dispatch yields three deduped CDG edges (1->2/3/4, all 'T'). + const switchCfg = (): FunctionCfg => + mkCfg(6, [ + [0, 1, 'seq'], + [1, 2, 'switch-case'], + [1, 3, 'switch-case'], + [1, 4, 'switch-case'], + [2, 5, 'break'], + [3, 5, 'break'], + [4, 5, 'break'], + ]); + + it('stops at the ceiling and reports truncated (deterministic prefix)', () => { + const r = computeControlDependence(switchCfg(), undefined, 2); + expect(r.truncated).toBe(true); + expect(r.edges).toHaveLength(2); + // the prefix is still sorted/deduped, a valid subset of the full result + for (const e of r.edges) expect(['T', 'F']).toContain(e.label); + }); + + it('maxEdges of 0 means unbounded (full result, not truncated)', () => { + const r = computeControlDependence(switchCfg(), undefined, 0); + expect(r.truncated).toBe(false); + expect(serAll(r.edges).sort()).toEqual(['1->2:T', '1->3:T', '1->4:T']); + }); + + it('a ceiling at/above the true count is not truncated', () => { + const r = computeControlDependence(switchCfg(), undefined, 3); + expect(r.truncated).toBe(false); + expect(r.edges).toHaveLength(3); + }); +}); + +describe('#2188 F1 — branch-label correctness on the REAL TS visitor (regression)', () => { + // The label is the AC3 "under what condition does X run?" answer. The bug: + // branchSense inferred it from the edge KIND alone, but the M1 visitor wires a + // condition's fall-through FALSE arm as `seq`/`loop-back` (not `cond-false`), + // so guard clauses / loop break got 'T' instead of 'F'. These tests run the + // REAL parser+visitor (the hand-built tests above used a fictional `cond-false` + // edge and could not catch the regression). + const tsVisitor = getProvider(SupportedLanguages.TypeScript).cfgVisitor; + const parser = new Parser(); + if (tsVisitor) parser.setLanguage(TypeScript.typescript); + + function cdgOf(code: string): { cfg: FunctionCfg; edges: readonly ControlDepEdge[] } { + if (!tsVisitor) throw new Error('no cfgVisitor'); + const cfgs = collectFunctionCfgs(parser.parse(code).rootNode, tsVisitor, 't.ts').cfgs; + expect(cfgs.length).toBe(1); + return { cfg: cfgs[0], edges: computeControlDependence(cfgs[0]).edges }; + } + const labelOf = ( + edges: readonly ControlDepEdge[], + controller: number, + dependent: number, + ): CdgLabel | undefined => + edges.find((e) => e.controllerBlock === controller && e.dependentBlock === dependent)?.label; + + it("guard clause: post-guard body runs on the guard's FALSE (seq) arm → 'F'", () => { + const { cfg, edges } = cdgOf(`function f(x){ if (!ok(x)) return; use(x); }`); + const guard = cfg.blocks.find((b) => b.text.includes('ok(x)'))!; + const use = cfg.blocks.find((b) => b.text.includes('use(x)'))!; + expect(labelOf(edges, guard.index, use.index)).toBe('F'); + }); + + it("do/while: body runs on the bottom-test's TRUE (loop-back) arm → 'T'", () => { + const { cfg, edges } = cdgOf(`function f(){ do { body(); } while (c()); }`); + const test = cfg.blocks.find((b) => b.text.includes('c()'))!; + const body = cfg.blocks.find((b) => b.text.includes('body()'))!; + expect(labelOf(edges, test.index, body.index)).toBe('T'); + }); + + it("while+break: post-break tail runs on the if's FALSE (seq) arm → 'F'", () => { + const { cfg, edges } = cdgOf(`function f(o,i){ while (o) { if (i) break; tail(); } }`); + const ifCond = cfg.blocks.find((b) => b.text === '(i)')!; + const tail = cfg.blocks.find((b) => b.text.includes('tail()'))!; + expect(labelOf(edges, ifCond.index, tail.index)).toBe('F'); + }); + + it("if/else still labels both arms correctly (no regression) → then 'T', else 'F'", () => { + const { cfg, edges } = cdgOf(`function f(x){ if (x) { a(); } else { b(); } }`); + const cond = cfg.blocks.find((b) => b.text === '(x)')!; + const thenB = cfg.blocks.find((b) => b.text.includes('a()'))!; + const elseB = cfg.blocks.find((b) => b.text.includes('b()'))!; + expect(labelOf(edges, cond.index, thenB.index)).toBe('T'); + expect(labelOf(edges, cond.index, elseB.index)).toBe('F'); + }); +}); diff --git a/gitnexus/test/unit/cfg/post-dominators.test.ts b/gitnexus/test/unit/cfg/post-dominators.test.ts new file mode 100644 index 000000000..abc257b41 --- /dev/null +++ b/gitnexus/test/unit/cfg/post-dominators.test.ts @@ -0,0 +1,230 @@ +import { describe, it, expect } from 'vitest'; +import { + computePostDominators, + isExitReachableFromAllBlocks, + postDominates, +} from '../../../src/core/ingestion/cfg/post-dominators.js'; +import type { + BasicBlockData, + CfgEdgeData, + FunctionCfg, +} from '../../../src/core/ingestion/cfg/types.js'; + +// U2 (#2085 M5) — post-dominators on the EXIT-rooted reverse CFG. Pinned on +// hand-built FunctionCfg literals with zero tree-sitter dependency, mirroring +// reaching-defs.test.ts / cfg-builder.test.ts. Post-dominance has crisp, +// well-known expected outputs per topology, so the expected ipdom values ARE +// the spec for the Cooper–Harvey–Kennedy iterative dominators implementation. + +// ── hand-built CFG helper ─────────────────────────────────────────────────── + +function mkCfg( + blockCount: number, + edges: [number, number][], + opts: { entry?: number; exit?: number } = {}, +): FunctionCfg { + const entry = opts.entry ?? 0; + const exit = opts.exit ?? blockCount - 1; + const blocks: BasicBlockData[] = Array.from({ length: blockCount }, (_, i) => ({ + index: i, + startLine: i + 1, + endLine: i + 1, + text: '', + kind: i === entry ? 'entry' : i === exit ? 'exit' : 'normal', + })); + const cfgEdges: CfgEdgeData[] = edges.map(([from, to]) => ({ from, to, kind: 'seq' })); + return { + filePath: 't.ts', + functionStartLine: 1, + functionStartColumn: 0, + entryIndex: entry, + exitIndex: exit, + blocks, + edges: cfgEdges, + }; +} + +const NONE = -1; + +describe('computePostDominators — ipdom on the reverse CFG', () => { + it('linear chain: each block is post-dominated by its successor', () => { + // 0(entry) → 1 → 2 → 3(exit) + const cfg = mkCfg(4, [ + [0, 1], + [1, 2], + [2, 3], + ]); + const tree = computePostDominators(cfg); + expect(tree.ipdom[3]).toBe(NONE); // exit (root) has no post-dominator above it + expect(tree.ipdom[2]).toBe(3); + expect(tree.ipdom[1]).toBe(2); + expect(tree.ipdom[0]).toBe(1); + + expect(postDominates(tree, 3, 0)).toBe(true); + expect(postDominates(tree, 2, 0)).toBe(true); + expect(postDominates(tree, 0, 2)).toBe(false); + expect(postDominates(tree, 1, 1)).toBe(true); // reflexive + }); + + it('diamond (if/else with join): the join post-dominates the branch, arms do not', () => { + // 0(branch) → 1(then), 2(else); 1,2 → 3(join) → 4(exit) + const cfg = mkCfg(5, [ + [0, 1], + [0, 2], + [1, 3], + [2, 3], + [3, 4], + ]); + const tree = computePostDominators(cfg); + expect(tree.ipdom[4]).toBe(NONE); + expect(tree.ipdom[3]).toBe(4); + expect(tree.ipdom[1]).toBe(3); + expect(tree.ipdom[2]).toBe(3); + expect(tree.ipdom[0]).toBe(3); // join post-dominates the branch, not an arm + + expect(postDominates(tree, 3, 0)).toBe(true); + expect(postDominates(tree, 1, 0)).toBe(false); // `then` does NOT post-dominate the branch + expect(postDominates(tree, 2, 0)).toBe(false); + expect(postDominates(tree, 3, 1)).toBe(true); + }); + + it('while loop: the header post-dominates the body; back-edge does not break the tree', () => { + // 0(entry) → 1(header) → 2(body) → 1; header → 3(exit) + const cfg = mkCfg(4, [ + [0, 1], + [1, 2], + [2, 1], + [1, 3], + ]); + const tree = computePostDominators(cfg); + expect(tree.ipdom[3]).toBe(NONE); + expect(tree.ipdom[1]).toBe(3); + expect(tree.ipdom[2]).toBe(1); // every path from body to exit goes through the header + expect(tree.ipdom[0]).toBe(1); + + expect(postDominates(tree, 1, 2)).toBe(true); + expect(postDominates(tree, 3, 2)).toBe(true); + expect(postDominates(tree, 2, 1)).toBe(false); + }); + + it('multiple returns collapsing to a single EXIT: exit post-dominates everything', () => { + // 0(entry) → 1(cond) → 2(return), 3(return); 2,3 → 4(exit) + const cfg = mkCfg(5, [ + [0, 1], + [1, 2], + [1, 3], + [2, 4], + [3, 4], + ]); + const tree = computePostDominators(cfg); + expect(tree.ipdom[4]).toBe(NONE); + expect(tree.ipdom[1]).toBe(4); + expect(tree.ipdom[2]).toBe(4); + expect(tree.ipdom[3]).toBe(4); + expect(tree.ipdom[0]).toBe(1); + for (const b of [0, 1, 2, 3]) expect(postDominates(tree, 4, b)).toBe(true); + }); + + it('exit-less infinite loop: blocks that cannot reach EXIT have no post-dominator (KTD5)', () => { + // 0(entry) → 1 → 2 → 1 (no edge ever reaches exit block 3) + const cfg = mkCfg(4, [ + [0, 1], + [1, 2], + [2, 1], + ]); + const tree = computePostDominators(cfg); + expect(tree.ipdom[3]).toBe(NONE); // exit itself + expect(tree.ipdom[0]).toBe(NONE); // cannot reach exit + expect(tree.ipdom[1]).toBe(NONE); + expect(tree.ipdom[2]).toBe(NONE); + // No post-dominator means only reflexive post-dominance, and the climb must + // terminate (no infinite loop) even on the cycle. + expect(postDominates(tree, 3, 0)).toBe(false); + expect(postDominates(tree, 0, 0)).toBe(true); + expect(postDominates(tree, 1, 2)).toBe(false); + }); + + it('trivial single-block function (entry === exit) does not crash', () => { + const cfg = mkCfg(1, [], { entry: 0, exit: 0 }); + const tree = computePostDominators(cfg); + expect(tree.ipdom[0]).toBe(NONE); + expect(postDominates(tree, 0, 0)).toBe(true); + }); + + it('is deterministic across runs', () => { + const make = (): FunctionCfg => + mkCfg(5, [ + [0, 1], + [0, 2], + [1, 3], + [2, 3], + [3, 4], + ]); + const a = computePostDominators(make()); + const b = computePostDominators(make()); + expect(a.ipdom).toEqual(b.ipdom); + }); +}); + +// ── post-dominance soundness precondition (#2188 review) ──────────────────── +// EXIT must be reachable (forward) from every block reachable from ENTRY, else +// the EXIT-rooted reverse walk degenerates and CDG is unsound. The current TS +// visitor always satisfies this; the guard protects future / hand-built CFGs. +describe('isExitReachableFromAllBlocks', () => { + it('holds for a normal single-EXIT diamond (every block reaches EXIT)', () => { + const cfg = mkCfg(5, [ + [0, 1], + [0, 2], + [1, 3], + [2, 3], + [3, 4], + ]); + expect(isExitReachableFromAllBlocks(cfg)).toBe(true); + }); + + it('holds for a loop whose header has a structural edge to EXIT', () => { + // 0=entry → 1=header; header → 2=body → back to header; header → 3=exit. + const cfg = mkCfg(4, [ + [0, 1], + [1, 2], + [2, 1], + [1, 3], + ]); + expect(isExitReachableFromAllBlocks(cfg)).toBe(true); + }); + + it('fails when an entry-reachable region cannot reach EXIT (exit-less loop)', () => { + // 0=entry → 1; 1↔2 spin forever with no edge to 3=exit. EXIT is unreachable + // from the {1,2} region → post-dominance would be unsound there. + const cfg = mkCfg(4, [ + [0, 1], + [1, 2], + [2, 1], + ]); + expect(isExitReachableFromAllBlocks(cfg)).toBe(false); + }); + + it('fails for the review counterexample (A→B, A→L, B→X→L→A; EXIT disconnected)', () => { + // Indices: 0=A(entry), 1=B, 2=X, 3=L, 4=EXIT (disconnected). The A/B/X/L + // cycle never reaches EXIT, so the precondition must reject it. + const cfg = mkCfg(5, [ + [0, 1], + [0, 3], + [1, 2], + [2, 3], + [3, 0], + ]); + expect(isExitReachableFromAllBlocks(cfg)).toBe(false); + }); + + it('ignores blocks unreachable from ENTRY (they need not reach EXIT)', () => { + // 0=entry → 1=exit directly; 2 is an island unreachable from entry. The + // island does not violate the precondition (it is never analyzed). + const cfg = mkCfg(3, [[0, 1]], { entry: 0, exit: 1 }); + expect(isExitReachableFromAllBlocks(cfg)).toBe(true); + }); + + it('holds for the single-block CFG (entry === exit)', () => { + expect(isExitReachableFromAllBlocks(mkCfg(1, [], { entry: 0, exit: 0 }))).toBe(true); + }); +}); diff --git a/gitnexus/test/unit/pdg-mode-flip.test.ts b/gitnexus/test/unit/pdg-mode-flip.test.ts index 572007a1c..5e8dbe2f9 100644 --- a/gitnexus/test/unit/pdg-mode-flip.test.ts +++ b/gitnexus/test/unit/pdg-mode-flip.test.ts @@ -140,6 +140,42 @@ describe('pdgModeMismatch — M3→M4 interproc-cap stamp upgrade (#2084 review }); }); +describe('pdgModeMismatch — pre-M5→M5 CDG-cap stamp upgrade (#2085 M5, pure)', () => { + it('resolvePdgConfig stamps the resolved CDG cap', async () => { + const { resolvePdgConfig } = await import('../../src/core/run-analyze.js'); + const stamp = resolvePdgConfig({ pdg: true }); + expect(stamp?.maxCdgEdgesPerFunction).toBe(5000); + }); + + it('a pre-M5 stamp (no CDG key) mismatches a CDG-aware request — upgrade forces full writeback', async () => { + const { pdgModeMismatch } = await import('../../src/core/run-analyze.js'); + // What an M4-era run wrote: every cap through the interproc set + model + // digest, but NO maxCdgEdgesPerFunction. The key-union comparator sees + // 5000 !== undefined and trips the full writeback that materialises CDG + // edges for every file without --force. + const m4Stamp = { + maxFunctionLines: 2000, + maxEdgesPerFunction: 5000, + maxReachingDefEdgesPerFunction: 4000, + maxTaintFindingsPerFunction: 200, + maxTaintHops: 32, + maxInterprocFindings: 2000, + maxInterprocHops: 32, + maxInterprocEdges: 1000, + taintModelVersion, + }; + expect(pdgModeMismatch(m4Stamp, { pdg: true })).toBe(true); + }); + + it('a CDG cap change alone trips the mismatch', async () => { + const { pdgModeMismatch, resolvePdgConfig } = await import('../../src/core/run-analyze.js'); + const stamp = resolvePdgConfig({ pdg: true }); + expect(pdgModeMismatch(stamp, { pdg: true, pdgMaxCdgEdgesPerFunction: 10 })).toBe(true); + // explicit default ≡ default (resolution before comparison) + expect(pdgModeMismatch(stamp, { pdg: true, pdgMaxCdgEdgesPerFunction: 5000 })).toBe(false); + }); +}); + describe('detect_changes BasicBlock exclusion (#2082 U7)', () => { it('the symbol-overlap id-prefix filter excludes exactly the BasicBlock rows', async () => { const repo = await setupMiniRepo(); @@ -215,6 +251,7 @@ describe('runFullAnalysis — pdg-mode flip (#2099 F1)', () => { maxFunctionLines: 2000, maxEdgesPerFunction: 5000, maxReachingDefEdgesPerFunction: 4000, + maxCdgEdgesPerFunction: 5000, maxTaintFindingsPerFunction: 200, maxTaintHops: 32, maxInterprocFindings: 2000, @@ -270,6 +307,7 @@ describe('runFullAnalysis — pdg-mode flip (#2099 F1)', () => { maxFunctionLines: 2000, maxEdgesPerFunction: 1, maxReachingDefEdgesPerFunction: 4000, + maxCdgEdgesPerFunction: 5000, maxTaintFindingsPerFunction: 200, maxTaintHops: 32, maxInterprocFindings: 2000, diff --git a/gitnexus/test/unit/run-analyze.test.ts b/gitnexus/test/unit/run-analyze.test.ts index e1f442321..d89114360 100644 --- a/gitnexus/test/unit/run-analyze.test.ts +++ b/gitnexus/test/unit/run-analyze.test.ts @@ -340,6 +340,7 @@ describe('pdgModeMismatch / resolvePdgConfig (#2099 F1)', () => { maxFunctionLines: 2000, maxEdgesPerFunction: 5000, maxReachingDefEdgesPerFunction: 4000, + maxCdgEdgesPerFunction: 5000, maxTaintFindingsPerFunction: 200, maxTaintHops: 32, maxInterprocFindings: 2000, @@ -365,6 +366,7 @@ describe('pdgModeMismatch / resolvePdgConfig (#2099 F1)', () => { pdgMaxFunctionLines: 0, pdgMaxEdgesPerFunction: 0, pdgMaxReachingDefEdgesPerFunction: 0, + pdgMaxCdgEdgesPerFunction: 0, pdgMaxTaintFindingsPerFunction: 0, pdgMaxTaintHops: 0, pdgMaxInterprocFindings: 0, @@ -375,6 +377,7 @@ describe('pdgModeMismatch / resolvePdgConfig (#2099 F1)', () => { maxFunctionLines: 0, maxEdgesPerFunction: 0, maxReachingDefEdgesPerFunction: 0, + maxCdgEdgesPerFunction: 0, maxTaintFindingsPerFunction: 0, maxTaintHops: 0, maxInterprocFindings: 0, diff --git a/gitnexus/test/unit/schema.test.ts b/gitnexus/test/unit/schema.test.ts index 33f18ed01..0dd55c4f2 100644 --- a/gitnexus/test/unit/schema.test.ts +++ b/gitnexus/test/unit/schema.test.ts @@ -101,6 +101,12 @@ describe('LadybugDB Schema', () => { expect(REL_TYPES).toContain(t); } }); + + it('includes the control-dependence edge types (issue #2085 M5)', () => { + for (const t of ['CDG', 'POST_DOMINATE']) { + expect(REL_TYPES).toContain(t); + } + }); }); describe('node schema DDL', () => { diff --git a/gitnexus/test/unit/security.test.ts b/gitnexus/test/unit/security.test.ts index 5f1934068..37cb9f6f8 100644 --- a/gitnexus/test/unit/security.test.ts +++ b/gitnexus/test/unit/security.test.ts @@ -65,6 +65,15 @@ describe('VALID_RELATION_TYPES', () => { expect(VALID_RELATION_TYPES.has('TAINT_PATH')).toBe(false); expect(VALID_RELATION_TYPES.size).toBe(16); }); + + it('CDG control-dependence edge types stay OUT of the impact allow-list (#2085 M5)', () => { + // CDG and POST_DOMINATE are BasicBlock→BasicBlock (block space), like the + // taint substrate — they must not enter impact()'s symbol-space BFS. Pinned + // explicitly (not just via the size==16 guard) so a future "add all emitted + // 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); + }); }); // ─── Valid node labels ─────────────────────────────────────────────── diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 156cc7a20..0cd25fb70 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -21,8 +21,8 @@ const MUTATING_TOOLS = new Set(['rename', 'group_sync']); const OPEN_WORLD_READ_ONLY_TOOLS = new Set(['query']); describe('GITNEXUS_TOOLS', () => { - it('exports all tools (8 base + 1 explain + 3 route/tool/shape + 1 api_impact + 2 group)', () => { - expect(GITNEXUS_TOOLS).toHaveLength(15); + it('exports all tools (8 base + 1 explain + 1 pdg_query + 3 route/tool/shape + 1 api_impact + 2 group)', () => { + expect(GITNEXUS_TOOLS).toHaveLength(16); }); it('contains all expected tool names', () => { @@ -38,6 +38,7 @@ describe('GITNEXUS_TOOLS', () => { 'rename', 'impact', 'explain', + 'pdg_query', 'api_impact', ]), );