From 7c723ce7940aa59009498b2cd3876207ea505e11 Mon Sep 17 00:00:00 2001 From: Chareonwit Kunna <21arenabreakout12@gmail.com> Date: Sat, 29 Aug 2026 16:57:59 +0700 Subject: [PATCH 1/4] fix(impact): resolve repo-relative file paths via filePath (fixes #3074) (#3084) * fix(impact): resolve repo-relative file paths via filePath (fixes #3074) - resolve repo-relative paths like supabase/functions/_shared/crypto.ts via n.filePath exact + anchored ENDS WITH suffix, not just n.id/n.name - return impactedCount:null on not_found so miss cannot be read as 0/UNKNOWN safe - relax parenthesised OR-clause test to allow extra filePath terms * fix(impact): scope filePath match to File nodes (review #3084 P1) * fix(impact): make file path resolution parseable and safe * docs(pdg): align result contract fixtures with v3 * test(impact): add exact path precedence and not_found contract assertions --- gitnexus/src/mcp/local/local-backend.ts | 40 ++++++++++-- gitnexus/src/mcp/local/pdg-impact.ts | 17 ++++-- gitnexus/src/mcp/tools.ts | 2 +- ...impact-pdg-callsummary-degradation.test.ts | 6 +- gitnexus/test/unit/calltool-dispatch.test.ts | 61 ++++++++++++++++++- .../test/unit/cli-impact-pdg-format.test.ts | 4 +- .../unit/impact-pdg-compose-dedup.test.ts | 2 +- 7 files changed, 113 insertions(+), 19 deletions(-) diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index cf197c732..6451b0d9b 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -3818,7 +3818,22 @@ export class LocalBackend { } else if (isQualified) { // Parenthesised because the kind filter below is appended with AND, which // binds tighter than OR. - whereClause = `WHERE (n.id = $symName OR n.name = $symName)`; + // #3074: a repo-relative file path (e.g. "supabase/functions/_shared/crypto.ts") + // is the most natural way to name a File and is exactly what `target.filePath` + // reports, but the old clause only matched `n.id` (= "File:") or basename + // `n.name`, so the same path the graph stores never resolved. Also match the + // repo-relative `n.filePath` exactly and via an anchored suffix (segment-boundary + // "ENDS WITH $suffix" where suffix is "/"+path) so "a.ts" does not spuriously + // match "mylib/a.ts" — same anchoring used in detect_changes (#2915). + const suffix = pathSuffixOf(name); + // File-path terms must be scoped to File nodes — n.filePath is shared by + // every symbol in the file, so an unlabeled predicate would turn + // "src/actions.ts" into every symbol in that file (bot review #3084 P1). + // LadybugDB does not allow label tests in WHERE (n:File), so scope via + // id prefix — File nodes are `File:`. + whereClause = `WHERE (n.id = $symName OR n.name = $symName OR (n.id STARTS WITH $filePrefix AND (n.filePath = $symName OR n.filePath ENDS WITH $suffix)))`; + queryParams.suffix = suffix; + queryParams.filePrefix = 'File:'; } else { whereClause = `WHERE n.name = $symName`; } @@ -3909,7 +3924,7 @@ export class LocalBackend { if (rows.length === 0) return { kind: 'not_found' }; // Normalise row shape across object / tuple returns from LadybugDB. - const normalized = rows.map((r: any) => ({ + let normalized = rows.map((r: any) => ({ id: (r.id ?? r[0]) as string, name: (r.name ?? r[1]) as string, type: (r.type ?? r[2] ?? '') as string, @@ -3919,6 +3934,16 @@ export class LocalBackend { ...(include_content ? { content: (r.content ?? r[6]) as string | undefined } : {}), })); + // An exact File path wins over anchored suffix candidates. Without this, + // `lib/a.ts` and `src/lib/a.ts` both score as File candidates and turn an + // otherwise unambiguous exact target into `ambiguous` (#3084 review P2). + if (isQualified) { + const exactFiles = normalized.filter( + (candidate) => candidate.id.startsWith('File:') && candidate.filePath === name, + ); + if (exactFiles.length > 0) normalized = exactFiles; + } + // The COUNT can never legitimately be below the page it accompanies, so a // value under `normalized.length` means the count leg failed or returned an // unreadable shape. Keep the window size as the floor — reporting zero would @@ -6127,6 +6152,7 @@ export class LocalBackend { direction: params.direction, suggestion, recoverySuggestion, + undetermined: true, }); return pdgErr; } @@ -6134,7 +6160,7 @@ export class LocalBackend { error: message, target: { name: params.target }, direction: params.direction, - impactedCount: 0, + impactedCount: null, risk: 'UNKNOWN', suggestion, ...(recoverySuggestion ? { recoverySuggestion } : {}), @@ -6229,6 +6255,7 @@ export class LocalBackend { `(single-repo PDG impact). Remove them or use mode:'callgraph' for cross-repo fan-out.`, target: crossDepthTarget, direction, + undetermined: true, }); return pdgErr; } @@ -6295,12 +6322,17 @@ export class LocalBackend { error: `Target '${missing}' not found`, target: notFoundTarget, direction, + undetermined: true, }) : { error: `Target '${missing}' not found`, target: { name: target }, direction, - impactedCount: 0, + // #3074 follow-up: do not ship a normal-shaped 0/UNKNOWN blast radius + // alongside the error — it reads as a real "nothing depends on this" + // answer. Null marks UNDETERMINED (same as the ambiguous path) so a + // consumer testing `impactedCount === 0` cannot misread a miss as safe. + impactedCount: null, risk: 'UNKNOWN', }; } diff --git a/gitnexus/src/mcp/local/pdg-impact.ts b/gitnexus/src/mcp/local/pdg-impact.ts index d2de59629..4cd311be1 100644 --- a/gitnexus/src/mcp/local/pdg-impact.ts +++ b/gitnexus/src/mcp/local/pdg-impact.ts @@ -118,8 +118,10 @@ export function splitCalleeIds(raw: unknown): string[] { * Bump on any breaking change to the PDG result fields. * v2: `startLine` in the result is now 1-based display (#2380), matching the * context/query/impact tools (was 0-based). + * v3: error envelopes use `impactedCount: null` when a target cannot be resolved + * (#3074), so consumers cannot read a miss as a measured zero. */ -export const PDG_RESULT_VERSION = 2 as const; +export const PDG_RESULT_VERSION = 3 as const; /** A reachable dependence block resolved to its source statement. */ export interface PdgStatement { @@ -742,7 +744,7 @@ export interface PdgInterproceduralImpact { export interface PdgImpactBaseResult extends PdgImpactParityFields { mode: 'pdg'; /** Contract version of the mode:'pdg' impact result shape; bump on any breaking change to the PDG result fields. */ - pdgResultVersion: 2; + pdgResultVersion: 3; target: PdgImpactTarget; direction: 'upstream' | 'downstream'; impactedCount: number; @@ -815,11 +817,11 @@ export interface PdgImpactDegradedResult extends PdgImpactBaseResult { export interface PdgImpactErrorResult { mode?: 'pdg'; /** Contract version of the mode:'pdg' impact result shape; bump on any breaking change to the PDG result fields. */ - pdgResultVersion: 2; + pdgResultVersion: 3; error: string; target: PdgImpactTarget; direction: 'upstream' | 'downstream'; - impactedCount: 0; + impactedCount: number | null; risk: 'UNKNOWN'; suggestion?: string; recoverySuggestion?: string; @@ -838,6 +840,8 @@ export function makePdgImpactErrorResult(input: { mode?: 'pdg'; suggestion?: string; recoverySuggestion?: string; + /** True when analysis did not obtain a measured impact count. */ + undetermined?: boolean; }): PdgImpactErrorResult { return { ...(input.mode ? { mode: input.mode } : {}), @@ -845,7 +849,10 @@ export function makePdgImpactErrorResult(input: { error: input.error, target: input.target, direction: input.direction, - impactedCount: 0, + // #3074 follow-up + P2 review: an unmeasured PDG result must not ship a + // confident-looking 0 blast radius. null marks UNDETERMINED, so a consumer + // testing === 0 cannot misread a miss or failed query as safe. + impactedCount: input.undetermined ? null : 0, risk: 'UNKNOWN', ...(input.suggestion ? { suggestion: input.suggestion } : {}), ...(input.recoverySuggestion ? { recoverySuggestion: input.recoverySuggestion } : {}), diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index be793cbeb..53700a7a2 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -469,7 +469,7 @@ MODE (opt-in): "callgraph" (default) walks symbol→symbol edges (CALLS/IMPORTS/ STATEMENT-ANCHORED PDG SLICE: with mode:'pdg', pass "line" (1-based source line within the target symbol) to seed the dependence slice on the statement at that line and return what depends on it in affectedStatements (line + text). Inter-procedural symbols are still reported through interproceduralByDepth/pdgInterprocedural and the compatibility byDepth bucket. Without "line", pdg returns whole-symbol inter-procedural reach plus local whole-symbol PDG diagnostics. -PDG OUTPUT CONTRACT: every mode:'pdg' result (success, empty, degraded, or error) carries pdgResultVersion:2 — a stable discriminator for external consumers that bumps on any breaking change to the PDG result shape (distinct from the DB schema version). Successful PDG results include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, interproceduralByDepth/pdgInterprocedural for cross-function reach, compatibility byDepth/byDepthCounts, risk:'UNKNOWN', and a note describing the unified contract. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', pdgResultVersion:2, target metadata when the target resolves, risk:'UNKNOWN', note/remediation, and empty byDepth parity fields — never a false-safe zero. If depth and limit both bound the slice, truncatedByReasons reports both causes while truncatedBy remains scalar. Return-value-ascent coverage is published structurally at pdgEvidence.ascent — present iff the inter-procedural descent ran, including on an empty slice — with referencesScanned (DISTINCT callees scanned for a CALL_SUMMARY: a distinct-id tally, not a call-site count — two call sites to the same callee count once), returnFlowFound (whether the ascent fired anywhere in the slice), undecodableSummaryCount, examinedComplete (whether that scan covered every callee the index recorded a resolved id for on the visited blocks), incompleteReasons ('traversal-truncated' | 'callee-list-capped' | 'callee-ids-unrecorded'), and callSummaryLayerPresent. Read callSummaryLayerPresent FIRST: false ⇒ a pre-CALL_SUMMARY index, so {referencesScanned:N>0, returnFlowFound:false} is self-consistent and says nothing about the callees — the scan ran, but no layer existed in which a return-flow could be recorded (remedy: re-run gitnexus analyze --pdg). Branch on those fields; the note narrates the same facts in prose for humans and is not a stable contract. +PDG OUTPUT CONTRACT: every mode:'pdg' result (success, empty, degraded, or error) carries pdgResultVersion:3 — a stable discriminator for external consumers that bumps on any breaking change to the PDG result shape (distinct from the DB schema version). Successful PDG results include mode:'pdg', a full target envelope (id/name/type/filePath), affectedStatements, affectedStatementCount, interproceduralByDepth/pdgInterprocedural for cross-function reach, compatibility byDepth/byDepthCounts, risk:'UNKNOWN', and a note describing the unified contract. Degraded PDG results (no-layer, sub-layer-missing, unknown) keep mode:'pdg', pdgResultVersion:3, target metadata when the target resolves, risk:'UNKNOWN', note/remediation, and empty byDepth parity fields — never a false-safe zero. If depth and limit both bound the slice, truncatedByReasons reports both causes while truncatedBy remains scalar. Return-value-ascent coverage is published structurally at pdgEvidence.ascent — present iff the inter-procedural descent ran, including on an empty slice — with referencesScanned (DISTINCT callees scanned for a CALL_SUMMARY: a distinct-id tally, not a call-site count — two call sites to the same callee count once), returnFlowFound (whether the ascent fired anywhere in the slice), undecodableSummaryCount, examinedComplete (whether that scan covered every callee the index recorded a resolved id for on the visited blocks), incompleteReasons ('traversal-truncated' | 'callee-list-capped' | 'callee-ids-unrecorded'), and callSummaryLayerPresent. Read callSummaryLayerPresent FIRST: false ⇒ a pre-CALL_SUMMARY index, so {referencesScanned:N>0, returnFlowFound:false} is self-consistent and says nothing about the callees — the scan ran, but no layer existed in which a return-flow could be recorded (remedy: re-run gitnexus analyze --pdg). Branch on those fields; the note narrates the same facts in prose for humans and is not a stable contract. WHEN TO USE: Before making code changes — especially refactoring, renaming, or modifying shared code. Shows what would break. AFTER THIS: Review d=1 items (WILL BREAK). Use context() on high-risk symbols. diff --git a/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts b/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts index ee9b31c92..eb43fc926 100644 --- a/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts +++ b/gitnexus/test/integration/impact-pdg-callsummary-degradation.test.ts @@ -17,7 +17,7 @@ * "complete" result. * * This golden asserts the EXACT degraded envelope (not just non-crash): - * - the result is still mode:'pdg' with pdgResultVersion:2 (the contract + * - the result is still mode:'pdg' with pdgResultVersion:3 (the contract * discriminator); * - the intra slice is PRESENT (CALL_SUMMARY is NOT a required sub-layer — the * index is `ready`, pdgLayer is undefined, risk is UNKNOWN, epistemic is the @@ -75,14 +75,14 @@ withTestLbugDB( }); describe('CALL_SUMMARY-absent (v3 / pre-FU-C index): the ascent is silent but the user is TOLD', () => { - it('returns the EXACT degraded envelope — mode:pdg, pdgResultVersion:2, intra slice present, risk UNKNOWN', async () => { + it('returns the EXACT degraded envelope — mode:pdg, pdgResultVersion:3, intra slice present, risk UNKNOWN', async () => { const result = await slice(); // Golden envelope: the index is `ready` (CALL_SUMMARY is NOT a required // sub-layer), so this is a real traversal result — NOT a pdgLayer // degradation early-return. The intra slice ran and risk stays UNKNOWN. expect(result).toMatchObject({ mode: 'pdg', - pdgResultVersion: 2, + pdgResultVersion: 3, risk: 'UNKNOWN', epistemic: 'pdg-intra-procedural', target: { id: 'func:fnA', name: 'fnA' }, diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index e3f1892ce..f5693f090 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -1344,12 +1344,67 @@ describe('LocalBackend.callTool', () => { await backend.callTool('context', { name: 'src/a.ts:collide', kind: 'Function' }); const parenthesised = - /WHERE \(n\.id = \$symName OR n\.name = \$symName\) AND n\.id STARTS WITH \$kindPrefix/; + /WHERE \(n\.id = \$symName OR n\.name = \$symName OR \(n\.id STARTS WITH \$filePrefix AND \(n\.filePath = \$symName OR n\.filePath ENDS WITH \$suffix\)\)\) AND n\.id STARTS WITH \$kindPrefix/; const calls = resolverCalls(); expect(calls).toHaveLength(2); expect(calls.filter((c) => parenthesised.test(c.query))).toHaveLength(2); }); + it('exact File path wins over suffixed matches during qualified resolution (#3084 review P2)', async () => { + (executeParameterized as any).mockImplementation(async (_repo: string, query: string) => { + if (query.startsWith('MATCH (n)')) { + return [ + { + id: 'File:src/lib/a.ts', + name: 'a.ts', + filePath: 'src/lib/a.ts', + kind: 'File', + total_hits: 1, + }, + { + id: 'File:lib/a.ts', + name: 'a.ts', + filePath: 'lib/a.ts', + kind: 'File', + total_hits: 1, + }, + ]; + } + return [{ total: 2 }]; + }); + + const result = await backend.callTool('context', { name: 'lib/a.ts' }); + expect(result).toMatchObject({ + status: 'found', + symbol: { + filePath: 'lib/a.ts', + uid: 'File:lib/a.ts', + }, + }); + }); + + it('not_found impact queries return impactedCount null and risk UNKNOWN across modes (#3074 / #3084 review)', async () => { + (executeParameterized as any).mockImplementation(async () => []); + + const cgResult = await backend.callTool('impact', { target: 'nonexistent_target_xyz' }); + expect(cgResult).toMatchObject({ + error: "Target 'nonexistent_target_xyz' not found", + impactedCount: null, + risk: 'UNKNOWN', + }); + + const pdgResult = await backend.callTool('impact', { + target: 'nonexistent_target_xyz', + mode: 'pdg', + }); + expect(pdgResult).toMatchObject({ + error: "Target 'nonexistent_target_xyz' not found", + impactedCount: null, + risk: 'UNKNOWN', + pdgResultVersion: 3, + }); + }); + it('retries UNFILTERED when the kind hint matches no label prefix (#2787 review F5)', async () => { // `kind` is a free-form string on the tool schema. A miscased or // repo-absent kind must not turn a real name into `not_found` — the @@ -3241,7 +3296,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { expect(result.mode).toBe('pdg'); expect(result.target).toEqual({ name: 'missingSymbol' }); expect(result.direction).toBe('upstream'); - expect(result.impactedCount).toBe(0); + expect(result.impactedCount).toBeNull(); expect(result.risk).toBe('UNKNOWN'); }); @@ -3257,7 +3312,7 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { expect(result.mode).toBe('pdg'); expect(result.target).toEqual({ name: 'main' }); expect(result.direction).toBe('downstream'); - expect(result.impactedCount).toBe(0); + expect(result.impactedCount).toBeNull(); expect(result.risk).toBe('UNKNOWN'); expect(result.suggestion).toMatch(/context/); implSpy.mockRestore(); diff --git a/gitnexus/test/unit/cli-impact-pdg-format.test.ts b/gitnexus/test/unit/cli-impact-pdg-format.test.ts index cb17487a5..9e9f4870f 100644 --- a/gitnexus/test/unit/cli-impact-pdg-format.test.ts +++ b/gitnexus/test/unit/cli-impact-pdg-format.test.ts @@ -38,7 +38,7 @@ function pdgFindings(overrides: Record = {}): Record { // The PDG result family advertises a contract version (FIX #2) so external // MCP/agent consumers can version against future shape evolution. It is a // mode:'pdg'-only field — never on the default callgraph result. - expect(pdgFindings()).toMatchObject({ mode: 'pdg', pdgResultVersion: 2 }); + expect(pdgFindings()).toMatchObject({ mode: 'pdg', pdgResultVersion: 3 }); }); it('surfaces ambiguous-projection and unresolved block counts honestly', () => { diff --git a/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts b/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts index ecf1ce9e5..a3c92f4e7 100644 --- a/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts +++ b/gitnexus/test/unit/impact-pdg-compose-dedup.test.ts @@ -36,7 +36,7 @@ const local = ( impactedCount: number, ): PdgImpactSuccessResult => ({ mode: 'pdg', - pdgResultVersion: 2, + pdgResultVersion: 3, target: { id: 'T', name: 'criterion', type: 'Function', filePath: 'src/a.ts' }, direction: 'downstream', risk: 'UNKNOWN', From bf7dcf98ca6a41a4fe21f85fead193020bbf3790 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Sat, 29 Aug 2026 11:40:56 +0100 Subject: [PATCH 2/4] feat(analyze): add incremental watch mode (#3072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(analyze): add incremental watch mode * fix(watch): harden control file reads * fix(watch): contain refresh errors and bound reads * fix(watch): stream strict control file reads * fix(watch): harden refresh recovery and lifecycle * fix(watch): report ignored repository defaults * fix(analyze): preserve signal exit semantics * style(analyze): format signal exit helper * test(config): exercise descriptor growth guard * test(watch): await source event before rename * fix(watch): keep live-index retries honest and ignore analyzer writes Hold retry backoff when events merge, stop only after a live-index mutation, skip .gitnexus self-writes, and reject the remaining one-shot watch flags. Export impact-risk scoring from gitnexus-shared so consumers can share the same scale. Co-authored-by: Cursor * fix(watch): contain queue edge cases after review Preserve overflow-only refreshes, contain synchronous refresh failures, and mark successful atomic publication before later operations can fail. Co-authored-by: Cursor --------- Co-authored-by: Gergo Magyar Co-authored-by: Cursor Co-authored-by: Gergő Magyar --- .claude/skills/gitnexus-cli/SKILL.md | 6 +- README.md | 23 + .../skills/gitnexus-cli/SKILL.md | 6 +- gitnexus-shared/src/impact-risk.ts | 129 +++++ gitnexus-shared/src/index.ts | 11 + gitnexus/README.md | 26 + gitnexus/package-lock.json | 77 +-- gitnexus/package.json | 1 + gitnexus/scripts/cross-platform-shard.ts | 6 +- gitnexus/scripts/cross-platform-tests.ts | 3 +- gitnexus/skills/gitnexus-cli.md | 6 +- gitnexus/src/cli/analyze-config.ts | 21 +- gitnexus/src/cli/analyze-options.ts | 4 + gitnexus/src/cli/analyze.ts | 55 +- gitnexus/src/cli/help-i18n.ts | 2 + gitnexus/src/cli/i18n/en.ts | 2 + gitnexus/src/cli/i18n/zh-CN.ts | 2 + gitnexus/src/cli/index.ts | 11 +- gitnexus/src/cli/watch-queue.ts | 184 +++++++ gitnexus/src/cli/watch.ts | 501 ++++++++++++++++++ gitnexus/src/config/ignore-service.ts | 47 +- gitnexus/src/config/repo-control-file.ts | 117 ++++ .../ingestion/pipeline-phases/parse-impl.ts | 9 + .../core/ingestion/pipeline-phases/parse.ts | 2 + gitnexus/src/core/ingestion/pipeline.ts | 13 +- gitnexus/src/core/run-analyze.ts | 97 +++- gitnexus/src/storage/parse-cache.ts | 12 +- gitnexus/src/types/pipeline.ts | 2 + .../integration/analyze-atomic-swap.test.ts | 112 +++- gitnexus/test/integration/cli-e2e.test.ts | 192 +++++++ .../test/integration/watch-filesystem.test.ts | 230 ++++++++ gitnexus/test/unit/analyze-config.test.ts | 80 ++- .../test/unit/analyze-heap-respawn.test.ts | 9 + .../test/unit/cross-platform-shard.test.ts | 4 +- .../unit/incremental-orchestration.test.ts | 7 + .../test/unit/incremental-parse-cache.test.ts | 27 + .../test/unit/watch-failure-policy.test.ts | 29 + gitnexus/test/unit/watch-paths.test.ts | 180 +++++++ gitnexus/test/unit/watch-queue.test.ts | 348 ++++++++++++ 39 files changed, 2504 insertions(+), 89 deletions(-) create mode 100644 gitnexus-shared/src/impact-risk.ts create mode 100644 gitnexus/src/cli/watch-queue.ts create mode 100644 gitnexus/src/cli/watch.ts create mode 100644 gitnexus/src/config/repo-control-file.ts create mode 100644 gitnexus/test/integration/watch-filesystem.test.ts create mode 100644 gitnexus/test/unit/watch-failure-policy.test.ts create mode 100644 gitnexus/test/unit/watch-paths.test.ts create mode 100644 gitnexus/test/unit/watch-queue.test.ts diff --git a/.claude/skills/gitnexus-cli/SKILL.md b/.claude/skills/gitnexus-cli/SKILL.md index e993c38f8..1b1f733b6 100644 --- a/.claude/skills/gitnexus-cli/SKILL.md +++ b/.claude/skills/gitnexus-cli/SKILL.md @@ -21,6 +21,8 @@ Run from the project root. This parses all source files, builds the knowledge gr | Flag | Effect | | -------------- | ---------------------------------------------------------------- | +| `--watch` | Keep a Git repository index current with serialized refreshes | +| `--debounce ` | Watch quiet period before refresh (default: 300 ms) | | `--force` | Force full re-index even if up to date | | `--embeddings` | Enable embedding generation for semantic search (off by default) | | `--drop-embeddings` | Drop existing embeddings on rebuild. By default, an `analyze` without `--embeddings` preserves them. | @@ -28,6 +30,8 @@ Run from the project root. This parses all source files, builds the knowledge gr **When to run:** First time in a project, after major code changes, or when `gitnexus://repo/{name}/context` reports the index is stale. In Claude Code, a PostToolUse hook detects staleness after `git commit` and `git merge` and notifies the agent to run `analyze` — the hook does not run analyze itself, to avoid blocking the agent for up to 120s and risking KuzuDB corruption on timeout. +Use `node .gitnexus/run.cjs analyze --watch` for a long-lived local Git repository. It performs an initial analysis, queues scanner-admitted file changes, and retries intact failed batches with bounded backoff. Watch refreshes update only the graph: they skip AGENTS.md / CLAUDE.md injection and standard skill installation, so run a one-shot `analyze` when those generated files need updating. Watch rejects one-shot or context-output flags including `--force`, embedding flags, `--skills`, `--default-branch`, `--skip-agents-md`, `--skip-skills`, `--no-stats`, `--self-commit`, `--index-only`, and `--skip-git`. It never pulls remotes. Running MCP and `serve` processes periodically check for a published replacement and reopen it without a restart. MCP checks are throttled to once every five seconds, so a tool call before the next check can briefly use the previous index. + ### status — Check index freshness ```bash @@ -86,5 +90,5 @@ Lists all repositories registered in `~/.gitnexus/registry.json`. The MCP `list_ ## Troubleshooting - **"Not inside a git repository"**: Run from a directory inside a git repo -- **Index is stale after re-analyzing**: Restart Claude Code to reload the MCP server +- **Index is stale after re-analyzing**: Wait for the next MCP tool call to reopen the published index; this normally takes no more than five seconds - **Embeddings slow**: Omit `--embeddings` (it's off by default) or set `OPENAI_API_KEY` for faster API-based embedding diff --git a/README.md b/README.md index e89ddcdb3..5e0536a48 100644 --- a/README.md +++ b/README.md @@ -384,6 +384,7 @@ Everyday commands: ```bash gitnexus setup # Configure MCP for detected editors (one-time; -c to select) gitnexus analyze [path] # Index a repository (or update a stale index) +gitnexus analyze [path] --watch # Watch local files and serialize incremental refreshes gitnexus mcp # Start MCP server (stdio) — serves all indexed repos gitnexus serve # Start local HTTP server (multi-repo) for web UI connection gitnexus eval-server # Start lightweight evaluation HTTP tools (loopback by default) @@ -396,6 +397,28 @@ gitnexus uninstall # Preview removal of GitNexus MCP/skills/hooks You can also query the graph directly from the terminal — `gitnexus query`, `context`, `impact`, `trace`, `cypher`, `detect-changes`, and `check` mirror the MCP tools of the same names, and `gitnexus doctor` prints runtime platform capabilities. +`gitnexus analyze --watch` requires a Git repository. It runs one initial +analysis, then debounces scanner-admitted working-tree changes for 300 ms by +default and applies serialized incremental refreshes. Events arriving during a +refresh remain queued, and retryable failures retain the same batch with bounded +backoff. Invalid `.gitnexusrc` or ignore-file reloads pause ordinary refreshes +until the control file is fixed. Stop the watcher with Ctrl+C. + +Watch mode accepts `--debounce`, `--workers`, `--worker-timeout`, +`--max-file-size`, `--branch`, `--pdg`, `--name`, `--allow-duplicate-name`, and +`--verbose`. Explicit one-shot options such as `--force`, `--repair-fts`, +embedding flags, `--skills`, `--self-commit`, `--index-only`, and `--skip-git` +are rejected. Unsupported defaults from `.gitnexusrc` are ignored with a +warning rather than making an otherwise valid repository unwatchable. + +POSIX requests clone-first copy-and-swap publication when the live index has no +orphan sidecars. Windows and sidecar fallback runs update in place: failures +known to occur before writes are retried, while a failure that may have mutated +the live index stops the watcher. Watch mode does not pull remotes. Running MCP +and `serve` processes reopen a newly published index automatically; MCP observes +the replacement on its next tool call, typically within five seconds, so no +restart is required. +
Authenticated eval-server binding diff --git a/gitnexus-claude-plugin/skills/gitnexus-cli/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-cli/SKILL.md index e993c38f8..1b1f733b6 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-cli/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-cli/SKILL.md @@ -21,6 +21,8 @@ Run from the project root. This parses all source files, builds the knowledge gr | Flag | Effect | | -------------- | ---------------------------------------------------------------- | +| `--watch` | Keep a Git repository index current with serialized refreshes | +| `--debounce ` | Watch quiet period before refresh (default: 300 ms) | | `--force` | Force full re-index even if up to date | | `--embeddings` | Enable embedding generation for semantic search (off by default) | | `--drop-embeddings` | Drop existing embeddings on rebuild. By default, an `analyze` without `--embeddings` preserves them. | @@ -28,6 +30,8 @@ Run from the project root. This parses all source files, builds the knowledge gr **When to run:** First time in a project, after major code changes, or when `gitnexus://repo/{name}/context` reports the index is stale. In Claude Code, a PostToolUse hook detects staleness after `git commit` and `git merge` and notifies the agent to run `analyze` — the hook does not run analyze itself, to avoid blocking the agent for up to 120s and risking KuzuDB corruption on timeout. +Use `node .gitnexus/run.cjs analyze --watch` for a long-lived local Git repository. It performs an initial analysis, queues scanner-admitted file changes, and retries intact failed batches with bounded backoff. Watch refreshes update only the graph: they skip AGENTS.md / CLAUDE.md injection and standard skill installation, so run a one-shot `analyze` when those generated files need updating. Watch rejects one-shot or context-output flags including `--force`, embedding flags, `--skills`, `--default-branch`, `--skip-agents-md`, `--skip-skills`, `--no-stats`, `--self-commit`, `--index-only`, and `--skip-git`. It never pulls remotes. Running MCP and `serve` processes periodically check for a published replacement and reopen it without a restart. MCP checks are throttled to once every five seconds, so a tool call before the next check can briefly use the previous index. + ### status — Check index freshness ```bash @@ -86,5 +90,5 @@ Lists all repositories registered in `~/.gitnexus/registry.json`. The MCP `list_ ## Troubleshooting - **"Not inside a git repository"**: Run from a directory inside a git repo -- **Index is stale after re-analyzing**: Restart Claude Code to reload the MCP server +- **Index is stale after re-analyzing**: Wait for the next MCP tool call to reopen the published index; this normally takes no more than five seconds - **Embeddings slow**: Omit `--embeddings` (it's off by default) or set `OPENAI_API_KEY` for faster API-based embedding diff --git a/gitnexus-shared/src/impact-risk.ts b/gitnexus-shared/src/impact-risk.ts new file mode 100644 index 000000000..6c18614f6 --- /dev/null +++ b/gitnexus-shared/src/impact-risk.ts @@ -0,0 +1,129 @@ +export type ImpactRisk = 'LOW' | 'MEDIUM' | 'HIGH' | 'CRITICAL' | 'UNKNOWN'; + +export type ImpactRiskAxis = 'processes' | 'modules'; + +export type UnusedImpactRiskReason = + | 'file-nodes-have-no-process-or-community-membership' + | 'enrichment-skipped' + | 'enrichment-budget-exhausted' + | 'enrichment-query-failed'; + +export interface UnusedImpactRiskAxis { + axis: ImpactRiskAxis; + reason: UnusedImpactRiskReason; +} + +export interface ImpactRiskInput { + direction: 'upstream' | 'downstream'; + directCount: number; + processCount: number; + moduleCount: number; + impactedCount: number; + unusedAxes?: readonly UnusedImpactRiskAxis[]; +} + +export interface ImpactRiskResult { + risk: ImpactRisk; + riskSharedAxes: ImpactRisk; + riskScale: { + comparableAcrossKinds: boolean; + unusedAxes: readonly UnusedImpactRiskAxis[]; + }; +} + +function score( + input: Pick< + ImpactRiskInput, + 'direction' | 'directCount' | 'processCount' | 'moduleCount' | 'impactedCount' + >, +): ImpactRisk { + const { direction, directCount, processCount, moduleCount, impactedCount } = input; + + if (direction === 'upstream' && impactedCount === 0) return 'UNKNOWN'; + if (directCount >= 30 || processCount >= 5 || moduleCount >= 5 || impactedCount >= 200) { + return 'CRITICAL'; + } + if (directCount >= 15 || processCount >= 3 || moduleCount >= 3 || impactedCount >= 100) { + return 'HIGH'; + } + if (directCount >= 5 || impactedCount >= 30) return 'MEDIUM'; + return 'LOW'; +} + +function countsWithUnusedAxesZeroed( + input: ImpactRiskInput, +): Pick< + ImpactRiskInput, + 'direction' | 'directCount' | 'processCount' | 'moduleCount' | 'impactedCount' +> { + let processCount = input.processCount; + let moduleCount = input.moduleCount; + for (const unused of input.unusedAxes ?? []) { + if (unused.axis === 'processes') processCount = 0; + if (unused.axis === 'modules') moduleCount = 0; + } + return { + direction: input.direction, + directCount: input.directCount, + processCount, + moduleCount, + impactedCount: input.impactedCount, + }; +} + +/** Map walk outcomes to unused process/module axes so comparability matches what was sampled. */ +export function unusedAxesForImpactWalk(input: { + isFileTarget: boolean; + skipEnrichment: boolean; + maxChunks: number; + processQueryFailed: boolean; + moduleQueryFailed: boolean; + /** When 0, a zero chunk budget is not an unused-axis event — there was nothing to enrich. */ + impactedCount?: number; +}): UnusedImpactRiskAxis[] { + if (input.isFileTarget) { + return [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ]; + } + if (input.skipEnrichment) { + return [ + { axis: 'processes', reason: 'enrichment-skipped' }, + { axis: 'modules', reason: 'enrichment-skipped' }, + ]; + } + if (input.maxChunks === 0 && (input.impactedCount ?? 1) > 0) { + return [ + { axis: 'processes', reason: 'enrichment-budget-exhausted' }, + { axis: 'modules', reason: 'enrichment-budget-exhausted' }, + ]; + } + const unused: UnusedImpactRiskAxis[] = []; + if (input.processQueryFailed) { + unused.push({ axis: 'processes', reason: 'enrichment-query-failed' }); + } + if (input.moduleQueryFailed) { + unused.push({ axis: 'modules', reason: 'enrichment-query-failed' }); + } + return unused; +} + +export function scoreImpactRisk(input: ImpactRiskInput): ImpactRiskResult { + const unusedAxes = input.unusedAxes ?? []; + + return { + risk: score(countsWithUnusedAxesZeroed(input)), + riskSharedAxes: score({ ...input, processCount: 0, moduleCount: 0 }), + riskScale: { + comparableAcrossKinds: unusedAxes.length === 0, + unusedAxes, + }, + }; +} diff --git a/gitnexus-shared/src/index.ts b/gitnexus-shared/src/index.ts index 13c2eac5a..9857a60cc 100644 --- a/gitnexus-shared/src/index.ts +++ b/gitnexus-shared/src/index.ts @@ -25,6 +25,17 @@ export { } from './language-detection.js'; export type { MroStrategy } from './mro-strategy.js'; +// Impact risk scoring +export { scoreImpactRisk, unusedAxesForImpactWalk } from './impact-risk.js'; +export type { + ImpactRisk, + ImpactRiskAxis, + ImpactRiskInput, + ImpactRiskResult, + UnusedImpactRiskAxis, + UnusedImpactRiskReason, +} from './impact-risk.js'; + // Pipeline progress export type { PipelinePhase, PipelineProgress } from './pipeline.js'; diff --git a/gitnexus/README.md b/gitnexus/README.md index 8013026f2..93b2dd354 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -234,6 +234,7 @@ Your AI agent gets **17 tools** (15 per-repo + 2 group) automatically: gitnexus setup # Configure MCP for detected editors (one-time; use -c to select) gitnexus uninstall # Preview removal of GitNexus MCP/skills/hooks (add --force to apply) gitnexus analyze [path] # Index a repository (or update stale index) +gitnexus analyze [path] --watch # Watch local files and serialize incremental refreshes gitnexus analyze --repair-fts # Fast path: rebuild/verify only FTS indexes on existing index data gitnexus analyze --force # Full rebuild: re-parse + graph rebuild + FTS rebuild gitnexus analyze --embeddings # Enable embedding generation (slower, better search) @@ -282,6 +283,31 @@ gitnexus group status # Check staleness of repos in a group gitnexus group impact --target --repo # Cross-repo blast radius ``` +`gitnexus analyze --watch` requires a Git repository. It performs an initial +analysis and then debounces scanner-admitted working-tree changes for 300 ms by +default into serialized incremental refreshes. Events arriving during a run +remain queued, and retryable failures retain the same batch with bounded +backoff. Invalid `.gitnexusrc` or ignore-file reloads pause ordinary refreshes +until the control file is fixed. Watch refreshes update only the graph: they +intentionally skip AGENTS.md / CLAUDE.md injection and standard skill +installation. Run a one-shot `gitnexus analyze` when those generated files need +updating. Stop watch mode with Ctrl+C. + +Watch mode accepts `--debounce`, `--workers`, `--worker-timeout`, +`--max-file-size`, `--branch`, `--pdg`, `--name`, `--allow-duplicate-name`, and +`--verbose`. Explicit one-shot options such as `--force`, `--repair-fts`, +embedding flags, `--skills`, `--default-branch`, `--skip-agents-md`, +`--skip-skills`, `--no-stats`, `--self-commit`, `--index-only`, and `--skip-git` +are rejected. Unsupported defaults from `.gitnexusrc` are ignored with a warning. + +POSIX requests clone-first copy-and-swap publication when the live index has no +orphan sidecars. Windows and sidecar fallback runs update in place: failures +known to occur before writes are retried, while a failure that may have mutated +the live index stops the watcher. Watch mode does not pull remotes. Running MCP +and `serve` processes periodically check for a newly published index and reopen +it without a restart. MCP checks are throttled to once every five seconds, so a +tool call before the next check can briefly use the previous index. + GraphQL contract matching is opt-in in the group's `group.yaml`: ```yaml diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index 7188c7e1c..b8b4c8b32 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -14,6 +14,7 @@ "@modelcontextprotocol/sdk": "^1.0.0", "@scarf/scarf": "^1.4.0", "busboy": "^1.6.0", + "chokidar": "^4.0.3", "cli-progress": "^3.12.0", "commander": "^15.0.0", "cors": "^2.8.5", @@ -826,9 +827,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -845,9 +843,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -864,9 +859,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -883,9 +875,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -902,9 +891,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -921,9 +907,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -940,9 +923,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -959,9 +939,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -978,9 +955,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1003,9 +977,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1028,9 +999,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1053,9 +1021,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1078,9 +1043,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1103,9 +1065,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1128,9 +1087,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1153,9 +1109,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2429,6 +2382,21 @@ "url": "https://github.com/chalk/chalk-template?sponsor=1" } }, + "node_modules/chokidar": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/chokidar/-/chokidar-4.0.3.tgz", + "integrity": "sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA==", + "license": "MIT", + "dependencies": { + "readdirp": "^4.0.1" + }, + "engines": { + "node": ">= 14.16.0" + }, + "funding": { + "url": "https://paulmillr.com/funding/" + } + }, "node_modules/chownr": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/chownr/-/chownr-3.0.0.tgz", @@ -4659,6 +4627,19 @@ "rc": "cli.js" } }, + "node_modules/readdirp": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/readdirp/-/readdirp-4.1.2.tgz", + "integrity": "sha512-GDhwkLfywWL2s6vEjyhri+eXmfH6j1L7JE27WhqLeYzoh/A3DBaYGEj2H/HFZCn/kMfim73FXxEJTw06WtxQwg==", + "license": "MIT", + "engines": { + "node": ">= 14.18.0" + }, + "funding": { + "type": "individual", + "url": "https://paulmillr.com/funding/" + } + }, "node_modules/real-require": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/real-require/-/real-require-0.2.0.tgz", diff --git a/gitnexus/package.json b/gitnexus/package.json index 5fccc65d1..cf511b17b 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -60,6 +60,7 @@ "@modelcontextprotocol/sdk": "^1.0.0", "@scarf/scarf": "^1.4.0", "busboy": "^1.6.0", + "chokidar": "^4.0.3", "cli-progress": "^3.12.0", "commander": "^15.0.0", "cors": "^2.8.5", diff --git a/gitnexus/scripts/cross-platform-shard.ts b/gitnexus/scripts/cross-platform-shard.ts index 8728d8e31..2c52dd86b 100644 --- a/gitnexus/scripts/cross-platform-shard.ts +++ b/gitnexus/scripts/cross-platform-shard.ts @@ -3,7 +3,7 @@ * * WHY THIS EXISTS. `run-cross-platform.ts` used to hand vitest the whole file * list plus `--shard=i/n`, and vitest partitions by file COUNT. Runtime on this - * suite is wildly uneven — measured on the Windows runner, `cli-e2e` is 361 s + * suite is wildly uneven — measured on the Windows runner, `cli-e2e` is 621 s * and `worker-pool` 221 s, while most files are under a second — so a * count-split routinely put several of the heaviest suites on one shard. That * is #2449, and this file's sibling header has documented the symptom ("the @@ -44,7 +44,9 @@ * partition depend on the very machine load it is trying to protect against. */ export const WINDOWS_WEIGHTS_SEC: Readonly> = { - 'test/integration/cli-e2e.test.ts': 361, + // Re-measured after the analyze --watch e2e landed in #3072. The previous + // 361 s entry undercharged this suite and left shard 1 close to the watchdog. + 'test/integration/cli-e2e.test.ts': 621, 'test/integration/worker-pool.test.ts': 222, 'test/unit/incremental-vector-extension-ordering.test.ts': 87, // ESTIMATE, not a measurement (#2841): this suite drives more full diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index d38672724..3f96c5c40 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -234,7 +234,7 @@ const SPAWN_CLI = [ // Cheap: measured on the Windows runner at 448 ms, 53 ms and sub-second. An // earlier attempt to register them still turned the matrix red — not from // their own cost, but because vitest sharded by file COUNT, so inserting any - // file re-partitioned the list and happened to cluster `cli-e2e` (361 s) with + // file re-partitioned the list and happened to cluster `cli-e2e` (621 s) with // `cli-limit-e2e` (75 s) on one shard. The split is weight-aware now // (`scripts/cross-platform-shard.ts`), so a cheap file can no longer move a // heavy one. @@ -273,6 +273,7 @@ const NATIVE_ADDON_SMOKE = [ // platforms (CRLF, symlinks, permissions, temp dirs) const FILESYSTEM = [ 'test/integration/filesystem-walker.test.ts', + 'test/integration/watch-filesystem.test.ts', 'test/integration/markdown-processor-crlf.test.ts', 'test/integration/ignore-and-skip-e2e.test.ts', // Pins that the bridge pairing verdict is measured before the database is diff --git a/gitnexus/skills/gitnexus-cli.md b/gitnexus/skills/gitnexus-cli.md index e993c38f8..1b1f733b6 100644 --- a/gitnexus/skills/gitnexus-cli.md +++ b/gitnexus/skills/gitnexus-cli.md @@ -21,6 +21,8 @@ Run from the project root. This parses all source files, builds the knowledge gr | Flag | Effect | | -------------- | ---------------------------------------------------------------- | +| `--watch` | Keep a Git repository index current with serialized refreshes | +| `--debounce ` | Watch quiet period before refresh (default: 300 ms) | | `--force` | Force full re-index even if up to date | | `--embeddings` | Enable embedding generation for semantic search (off by default) | | `--drop-embeddings` | Drop existing embeddings on rebuild. By default, an `analyze` without `--embeddings` preserves them. | @@ -28,6 +30,8 @@ Run from the project root. This parses all source files, builds the knowledge gr **When to run:** First time in a project, after major code changes, or when `gitnexus://repo/{name}/context` reports the index is stale. In Claude Code, a PostToolUse hook detects staleness after `git commit` and `git merge` and notifies the agent to run `analyze` — the hook does not run analyze itself, to avoid blocking the agent for up to 120s and risking KuzuDB corruption on timeout. +Use `node .gitnexus/run.cjs analyze --watch` for a long-lived local Git repository. It performs an initial analysis, queues scanner-admitted file changes, and retries intact failed batches with bounded backoff. Watch refreshes update only the graph: they skip AGENTS.md / CLAUDE.md injection and standard skill installation, so run a one-shot `analyze` when those generated files need updating. Watch rejects one-shot or context-output flags including `--force`, embedding flags, `--skills`, `--default-branch`, `--skip-agents-md`, `--skip-skills`, `--no-stats`, `--self-commit`, `--index-only`, and `--skip-git`. It never pulls remotes. Running MCP and `serve` processes periodically check for a published replacement and reopen it without a restart. MCP checks are throttled to once every five seconds, so a tool call before the next check can briefly use the previous index. + ### status — Check index freshness ```bash @@ -86,5 +90,5 @@ Lists all repositories registered in `~/.gitnexus/registry.json`. The MCP `list_ ## Troubleshooting - **"Not inside a git repository"**: Run from a directory inside a git repo -- **Index is stale after re-analyzing**: Restart Claude Code to reload the MCP server +- **Index is stale after re-analyzing**: Wait for the next MCP tool call to reopen the published index; this normally takes no more than five seconds - **Embeddings slow**: Omit `--embeddings` (it's off by default) or set `OPENAI_API_KEY` for faster API-based embedding diff --git a/gitnexus/src/cli/analyze-config.ts b/gitnexus/src/cli/analyze-config.ts index 6e1afc7bb..64b7c1573 100644 --- a/gitnexus/src/cli/analyze-config.ts +++ b/gitnexus/src/cli/analyze-config.ts @@ -30,6 +30,7 @@ import fs from 'node:fs'; import path from 'node:path'; +import { readRepoControlFile } from '../config/repo-control-file.js'; import type { AnalyzeOptions } from './analyze-options.js'; export const GITNEXUS_RC_FILENAME = '.gitnexusrc'; @@ -370,7 +371,6 @@ const normalizeLevel = ( */ export function loadAnalyzeConfig(repoRoot: string): Partial | undefined { const filePath = path.join(repoRoot, GITNEXUS_RC_FILENAME); - let raw: string; try { raw = fs.readFileSync(filePath, 'utf-8'); @@ -379,6 +379,25 @@ export function loadAnalyzeConfig(repoRoot: string): Partial | u throw new GitNexusRcError(`Could not read ${GITNEXUS_RC_FILENAME}: ${(err as Error).message}`); } + return parseAnalyzeConfig(raw); +} + +/** Load `.gitnexusrc` through the strict bounded reader used by watch mode. */ +export async function loadAnalyzeConfigStrict( + repoRoot: string, +): Promise | undefined> { + let raw: string | null; + try { + raw = await readRepoControlFile(repoRoot, GITNEXUS_RC_FILENAME); + } catch (err) { + throw new GitNexusRcError(`Could not read ${GITNEXUS_RC_FILENAME}: ${(err as Error).message}`); + } + return raw === null ? undefined : parseAnalyzeConfig(raw); +} + +function parseAnalyzeConfig(rawInput: string): Partial { + let raw = rawInput; + // Strip a leading UTF-8 BOM: Node's 'utf-8' decode keeps it, and JSON.parse // then fails with a confusing "Unexpected token" on an otherwise-valid file // (#1996 tri-review). Only one leading BOM is stripped; in-string control diff --git a/gitnexus/src/cli/analyze-options.ts b/gitnexus/src/cli/analyze-options.ts index c3d1b3e8f..460218954 100644 --- a/gitnexus/src/cli/analyze-options.ts +++ b/gitnexus/src/cli/analyze-options.ts @@ -15,6 +15,10 @@ * import cycle. `analyze.ts` re-exports the type for existing importers. */ export interface AnalyzeOptions { + /** Keep this repository current with serialized incremental refreshes. */ + watch?: boolean; + /** Watch quiet period in milliseconds. */ + debounce?: string; force?: boolean; repairFts?: boolean; /** diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index e2208a3c8..54bf12d11 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -363,6 +363,7 @@ interface RespawnExit { stdout?: string; stderr?: string; message?: string; + forwardedSignal?: NodeJS.Signals; } const appendOutputTail = (tail: string, chunk: unknown): string => { @@ -395,17 +396,28 @@ const runRespawnedAnalyze = ( let stdout = ''; let stderr = ''; let settled = false; - const finish = (exit: RespawnExit): void => { - if (settled) return; - settled = true; - resolve(exit); - }; - + let forwardedSignal: NodeJS.Signals | undefined; const child = spawn(process.execPath, [...args], { stdio: ['inherit', 'pipe', 'pipe'], windowsHide: true, env, }); + const forwardSignal = (signal: NodeJS.Signals): void => { + forwardedSignal ??= signal; + if (child.exitCode === null && child.signalCode === null) child.kill(signal); + }; + const forwardSigint = () => forwardSignal('SIGINT'); + const forwardSigterm = () => forwardSignal('SIGTERM'); + const finish = (exit: RespawnExit): void => { + if (settled) return; + settled = true; + process.removeListener('SIGINT', forwardSigint); + process.removeListener('SIGTERM', forwardSigterm); + resolve({ ...exit, forwardedSignal }); + }; + + process.once('SIGINT', forwardSigint); + process.once('SIGTERM', forwardSigterm); child.stdout?.on('data', (chunk) => { stdout = appendOutputTail(stdout, chunk); @@ -548,7 +560,16 @@ export function parseMaxOldSpaceMb(nodeOptions: string): number | null { * tooling), not a deliberate per-run choice: warn and respawn with the * auto cap. Pre-#2649 this returned early and large repos then OOM'd on * whatever heap the environment happened to specify. */ -async function ensureHeap(): Promise { +export function forwardedSignalExitCode(signal: NodeJS.Signals, cleanTermination: boolean): number { + if (cleanTermination) return 0; + if (signal === 'SIGINT') return 130; + if (signal === 'SIGTERM') return 143; + return 1; +} + +export async function ensureHeap( + options: { cleanForwardedTermination?: boolean } = {}, +): Promise { // Explicit opt-out disables auto-sizing ENTIRELY — both the ambient-pin // override and the default v8-limit respawn — and is honored SILENTLY: // the operator already made the call, and stderr-sensitive consumers @@ -590,6 +611,13 @@ async function ensureHeap(): Promise { }; if (shouldBridgeRespawnProgressTty()) childEnv[RESPAWN_PROGRESS_ENV] = '1'; const childExit = await runRespawnedAnalyze(childArgs, childEnv); + if (childExit.forwardedSignal !== undefined) { + process.exitCode = forwardedSignalExitCode( + childExit.forwardedSignal, + options.cleanForwardedTermination === true, + ); + return true; + } if (childExit.status !== 0 || childExit.signal) { if (childProcessLikelyOom(childExit)) { cliError( @@ -740,6 +768,19 @@ export const analyzeCommandWithRunnerIdentity = async ( options?: AnalyzeOptions, ): Promise => analyzeCommand(inputPath, options, runnerIdentityAtBootstrap); +export async function analyzeOrWatchCommandWithRunnerIdentity( + runnerIdentityAtBootstrap: AnalyzerRunnerIdentity, + inputPath?: string, + options: AnalyzeOptions = {}, +): Promise { + if (options.watch) { + const { watchCommandWithRunnerIdentity } = await import('./watch.js'); + await watchCommandWithRunnerIdentity(runnerIdentityAtBootstrap, inputPath, options); + return; + } + await analyzeCommandWithRunnerIdentity(runnerIdentityAtBootstrap, inputPath, options); +} + const analyzeCommandImpl = async ( inputPath?: string, cliOptions?: AnalyzeOptions, diff --git a/gitnexus/src/cli/help-i18n.ts b/gitnexus/src/cli/help-i18n.ts index eeea3eeae..cd4724d70 100644 --- a/gitnexus/src/cli/help-i18n.ts +++ b/gitnexus/src/cli/help-i18n.ts @@ -72,6 +72,8 @@ const OPTION_DESCRIPTION_KEYS = { 'analyze|--embedding-batch-size ': 'help.option.analyze.embeddingBatchSize', 'analyze|--embedding-sub-batch-size ': 'help.option.analyze.embeddingSubBatchSize', 'analyze|--embedding-device ': 'help.option.analyze.embeddingDevice', + 'analyze|--watch': 'help.option.analyze.watch', + 'analyze|--debounce ': 'help.option.analyze.debounce', 'index|-f, --force': 'help.option.index.force', 'index|--allow-non-git': 'help.option.index.allowNonGit', 'mcp|--http': 'help.option.mcp.http', diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index c22da5211..15e1644e7 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -217,6 +217,8 @@ export const en = { 'help.option.analyze.embeddingBatchSize': 'Number of nodes per embedding batch', 'help.option.analyze.embeddingSubBatchSize': 'Number of chunks per embedding model call', 'help.option.analyze.embeddingDevice': 'Embedding device: auto, cpu, dml, cuda, or wasm', + 'help.option.analyze.watch': 'Keep the index current with serialized incremental refreshes', + 'help.option.analyze.debounce': 'Watch quiet period before refreshing (milliseconds)', 'help.option.index.force': 'Register even if index metadata is missing (stats will be empty)', 'help.option.index.allowNonGit': 'Allow registering folders that are not Git repositories', 'help.option.port': 'Port number', diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 7ef2d244b..63e290e54 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -203,6 +203,8 @@ export const zhCN = { 'help.option.analyze.embeddingBatchSize': '每个嵌入批次的节点数', 'help.option.analyze.embeddingSubBatchSize': '每次嵌入模型调用的分块数', 'help.option.analyze.embeddingDevice': '嵌入设备：auto、cpu、dml、cuda 或 wasm', + 'help.option.analyze.watch': '监视本地源文件变更并串行执行增量刷新', + 'help.option.analyze.debounce': '刷新前的静默等待时间（毫秒）', 'help.option.index.force': '即使缺少索引元数据也注册（统计为空）', 'help.option.index.allowNonGit': '允许注册非 Git 仓库文件夹', 'help.option.port': '端口号', diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index a1edf0b53..8296787af 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -57,6 +57,8 @@ let dimsEnvCaptured = false; program .command('analyze [path]') .description('Index a repository (full analysis)') + .option('--watch', 'Keep the index current with serialized incremental refreshes') + .option('--debounce ', 'Watch quiet period before refreshing (default: 300 milliseconds)') .option('-f, --force', 'Force full re-index even if up to date') .option('--repair-fts', 'Repair/rebuild search FTS indexes without full re-analysis') .option( @@ -162,6 +164,11 @@ program ) .addHelpText('after', () => t('help.analyze.environment')) .hook('preAction', (thisCommand: Command) => { + const analyzeOpts = thisCommand.opts(); + if (analyzeOpts['debounce'] !== undefined && analyzeOpts['watch'] !== true) { + process.stderr.write('\n --debounce requires --watch\n\n'); + process.exit(1); + } // ONLY GITNEXUS_EMBEDDING_DIMS must be set here: schema.ts reads it at // module-load time during the lazy import('./analyze.js') below (via the // static chain analyze.ts → run-analyze.ts → schema.ts), so deferring to @@ -169,7 +176,7 @@ program // lazily at runtime (readConfig), so analyzeCommandImpl is their sole // setter — keeping them out of this hook means they fall under the impl's // env snapshot/restore and don't leak across in-process invocations. - const dimsOpt = thisCommand.opts()['embeddingDims']; + const dimsOpt = analyzeOpts['embeddingDims']; if (dimsOpt !== undefined) { // Validate + normalize BEFORE writing the env var: schema.ts throws on a // bad value at module-load, which — on the synchronous program.parse() @@ -202,7 +209,7 @@ program createAnalyzerLbugLazyAction( () => import('../core/analyzer-identity.js'), () => import('./analyze.js'), - 'analyzeCommandWithRunnerIdentity', + 'analyzeOrWatchCommandWithRunnerIdentity', import.meta.url, ), ); diff --git a/gitnexus/src/cli/watch-queue.ts b/gitnexus/src/cli/watch-queue.ts new file mode 100644 index 000000000..3f701ef50 --- /dev/null +++ b/gitnexus/src/cli/watch-queue.ts @@ -0,0 +1,184 @@ +export type WatchRefresh = (paths: readonly string[]) => Promise; +export type WatchRefreshError = (error: unknown, paths: readonly string[]) => void; + +export const WATCH_FULL_REFRESH_PATH = '*'; + +export interface WatchRefreshQueueOptions { + readonly maxWaitMs?: number; + readonly maxPendingPaths?: number; + readonly retryBaseDelayMs?: number; + readonly retryMaxDelayMs?: number; + readonly holdEventsUntilInitialRefresh?: boolean; + readonly isPriorityPath?: (filePath: string) => boolean; +} + +/** Debounces filesystem events and guarantees that refreshes never overlap. */ +export class WatchRefreshQueue { + private readonly pending = new Set(); + private readonly idleWaiters = new Set<() => void>(); + private timer: ReturnType | undefined; + private active: Promise | undefined; + private closed = false; + private initialPending = false; + private firstPendingAt: number | undefined; + private overflowed = false; + private consecutiveFailures = 0; + private retryNotBefore: number | undefined; + + constructor( + private readonly refresh: WatchRefresh, + private readonly onError: WatchRefreshError, + private readonly debounceMs: number, + private readonly options: WatchRefreshQueueOptions = {}, + ) { + this.initialPending = options.holdEventsUntilInitialRefresh === true; + } + + enqueue(filePath: string): void { + if (this.closed) return; + this.addPendingPath(filePath); + this.firstPendingAt ??= Date.now(); + if (!this.initialPending && this.active === undefined) this.schedule(); + } + + private addPendingPath(filePath: string): void { + const maxPendingPaths = this.options.maxPendingPaths ?? 1_000; + const priority = this.options.isPriorityPath?.(filePath) === true; + if (this.pending.has(filePath)) { + // A duplicate does not increase memory use or imply that paths were dropped. + } else if (this.pending.size < maxPendingPaths) { + this.pending.add(filePath); + } else { + this.overflowed = true; + if (priority) { + const evictable = [...this.pending].find( + (pendingPath) => this.options.isPriorityPath?.(pendingPath) !== true, + ); + if (evictable !== undefined) { + this.pending.delete(evictable); + this.pending.add(filePath); + } + } + } + } + + /** Run the initial refresh while still queueing events that arrive during it. */ + async runInitial(): Promise { + if (this.closed) return; + if (this.active !== undefined) throw new Error('Watch refresh is already running'); + try { + await this.runBatch([], true); + } finally { + this.initialPending = false; + if (!this.closed && this.hasPendingWork()) this.schedule(); + else this.resolveIdleWaiters(); + } + } + + async waitForIdle(): Promise { + if (this.isIdle()) return; + await new Promise((resolve) => this.idleWaiters.add(resolve)); + } + + async close(): Promise { + this.closed = true; + if (this.timer !== undefined) clearTimeout(this.timer); + this.timer = undefined; + this.pending.clear(); + this.firstPendingAt = undefined; + this.overflowed = false; + this.consecutiveFailures = 0; + this.retryNotBefore = undefined; + // A refresh rejection is already surfaced through `onError` (or through + // runInitial). Closing from that handler can race the runBatch `finally`, + // so consume the same rejection here instead of reporting it twice. + await this.active?.catch(() => {}); + this.resolveIdleWaiters(); + } + + private schedule(retryDelayMs?: number): void { + if (this.timer !== undefined) clearTimeout(this.timer); + const maxWaitMs = this.options.maxWaitMs ?? Math.max(this.debounceMs, 2_000); + const now = Date.now(); + if (retryDelayMs !== undefined) this.retryNotBefore = now + retryDelayMs; + const elapsed = this.firstPendingAt === undefined ? 0 : now - this.firstPendingAt; + const debounced = Math.max(0, Math.min(this.debounceMs, maxWaitMs - elapsed)); + // An event arriving mid-backoff merges into the pending batch but must not + // pull the retry earlier than the deadline the backoff already committed to. + const delay = + retryDelayMs ?? + (this.retryNotBefore === undefined + ? debounced + : Math.max(debounced, this.retryNotBefore - now)); + this.timer = setTimeout(() => { + this.timer = undefined; + void this.drain(); + }, delay); + } + + private async drain(): Promise { + if (this.closed || this.active !== undefined || !this.hasPendingWork()) return; + const paths = [ + ...(this.overflowed ? [WATCH_FULL_REFRESH_PATH] : []), + ...[...this.pending].sort(), + ]; + this.pending.clear(); + this.firstPendingAt = undefined; + this.overflowed = false; + this.retryNotBefore = undefined; + await this.runBatch(paths, false); + } + + private async runBatch(paths: readonly string[], propagateError: boolean): Promise { + let work: Promise; + try { + work = this.refresh(paths); + } catch (error) { + work = Promise.reject(error); + } + this.active = work; + let retryDelayMs: number | undefined; + try { + await work; + this.consecutiveFailures = 0; + } catch (error) { + if (propagateError) throw error; + try { + await this.onError(error, paths); + } catch { + // Refresh failures are already handled here; a reporter must not + // reject the detached drain promise and become an unhandled rejection. + } + if (!this.closed) { + if (paths.includes(WATCH_FULL_REFRESH_PATH)) this.overflowed = true; + for (const filePath of paths) { + if (filePath !== WATCH_FULL_REFRESH_PATH) this.addPendingPath(filePath); + } + this.firstPendingAt = Date.now(); + this.consecutiveFailures++; + const base = this.options.retryBaseDelayMs ?? Math.max(250, this.debounceMs); + const maximum = this.options.retryMaxDelayMs ?? 30_000; + retryDelayMs = Math.min(maximum, base * 2 ** (this.consecutiveFailures - 1)); + } + } finally { + if (this.active === work) this.active = undefined; + if (!this.closed && !this.initialPending && this.hasPendingWork()) + this.schedule(retryDelayMs); + else this.resolveIdleWaiters(); + } + } + + private hasPendingWork(): boolean { + return this.overflowed || this.pending.size > 0; + } + + private isIdle(): boolean { + return this.active === undefined && this.timer === undefined && !this.hasPendingWork(); + } + + private resolveIdleWaiters(): void { + if (!this.isIdle() && !this.closed) return; + for (const resolve of this.idleWaiters) resolve(); + this.idleWaiters.clear(); + } +} diff --git a/gitnexus/src/cli/watch.ts b/gitnexus/src/cli/watch.ts new file mode 100644 index 000000000..3cbec636f --- /dev/null +++ b/gitnexus/src/cli/watch.ts @@ -0,0 +1,501 @@ +import path from 'node:path'; +import fs from 'node:fs/promises'; +import { watch, type FSWatcher } from 'chokidar'; +import { createWatchIgnorePredicate } from '../config/ignore-service.js'; +import { + analyzeFailureMayHaveMutatedLiveIndex, + runFullAnalysis, + type AnalyzeOptions as CoreAnalyzeOptions, + type AnalyzeResult, +} from '../core/run-analyze.js'; +import { getGitRoot, hasGitDir } from '../storage/git.js'; +import type { AnalyzerRunnerIdentity } from '../storage/repo-manager.js'; +import { GITNEXUS_DIR } from '../storage/repo-meta.js'; +import { + loadAnalyzeConfigStrict, + mergeAnalyzeOptions, + validateBranchName, +} from './analyze-config.js'; +import type { AnalyzeOptions } from './analyze-options.js'; +import { ensureHeap } from './analyze.js'; +import { cliError, cliInfo, cliWarn } from './cli-message.js'; +import { + WATCH_FULL_REFRESH_PATH, + WatchRefreshQueue, + type WatchRefreshError, +} from './watch-queue.js'; + +const DEFAULT_DEBOUNCE_MS = 300; +const MAX_TIMER_DELAY_MS = 2_147_483_647; +const MAX_FILE_SIZE_KB = 32 * 1024; +const TRANSIENT_WATCH_ERROR_CODES = new Set(['EACCES', 'ENOENT', 'ENOTDIR', 'EPERM']); + +export type WatchCliOptions = AnalyzeOptions; + +function posixWatchPath(filePath: string): string { + return filePath.replace(/\\/g, '/').replace(/^\.\/+/, ''); +} + +export function isRelevantWatchPath(filePath: string): boolean { + const normalized = posixWatchPath(filePath); + return ( + normalized.length > 0 && + normalized !== '.' && + !normalized.startsWith('../') && + !path.posix.isAbsolute(normalized) && + !path.win32.isAbsolute(filePath) + ); +} + +function isIgnoreControlPath(filePath: string): boolean { + const normalized = posixWatchPath(filePath); + return normalized === '.gitignore' || normalized === '.gitnexusignore'; +} + +function isConfigControlPath(filePath: string): boolean { + return posixWatchPath(filePath) === '.gitnexusrc'; +} + +function isAnalyzerOwnedWatchPath(filePath: string): boolean { + const normalized = posixWatchPath(filePath).replace(/\/+$/, ''); + return normalized === GITNEXUS_DIR || normalized.startsWith(`${GITNEXUS_DIR}/`); +} + +function repoRelativeWatchPath(repoPath: string, candidate: string): string | null { + const relative = path.relative(repoPath, candidate).replace(/\\/g, '/'); + if (!relative || relative.startsWith('../') || path.isAbsolute(relative)) return null; + return relative; +} + +export interface WatchEnvironmentBaseline { + readonly maxFileSize: string | undefined; + readonly workerTimeout: string | undefined; + readonly verbose: string | undefined; +} + +function setEnvironment(name: string, value: string | undefined): void { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; +} + +function positiveInteger( + value: string | undefined, + flag: string, + maximum?: number, +): number | undefined { + if (value === undefined) return undefined; + const parsed = Number(value); + if (!Number.isInteger(parsed) || parsed < 1) + throw new Error(`${flag} must be a positive integer`); + if (maximum !== undefined && parsed > maximum) { + throw new Error(`${flag} must not exceed ${maximum}`); + } + return parsed; +} + +export async function resolveWatchOptions( + repoPath: string, + cli: WatchCliOptions, + baseline: WatchEnvironmentBaseline, + reportIgnoredConfig: (names: readonly string[]) => void = () => {}, +): Promise { + const config = (await loadAnalyzeConfigStrict(repoPath)) ?? {}; + const merged = mergeAnalyzeOptions(cli, config); + const unsupported = [ + ['--force', cli.force], + ['--repair-fts', cli.repairFts], + ['--embeddings', cli.embeddings], + ['--drop-embeddings', cli.dropEmbeddings], + ['--skills', cli.skills], + ['--default-branch', cli.defaultBranch], + ['--skip-agents-md', cli.skipAgentsMd], + ['--skip-skills', cli.skipSkills], + ['--no-stats', cli.stats === false], + ['--self-commit', cli.selfCommit], + ['--index-only', cli.indexOnly], + ['--skip-git', cli.skipGit], + ['walCheckpointThreshold', cli.walCheckpointThreshold], + ['embeddingThreads', cli.embeddingThreads], + ['embeddingBatchSize', cli.embeddingBatchSize], + ['embeddingSubBatchSize', cli.embeddingSubBatchSize], + ['embeddingDevice', cli.embeddingDevice], + ['embeddingBaseUrl', cli.embeddingBaseUrl], + ['embeddingModel', cli.embeddingModel], + ['--embedding-auth-token', cli.embeddingAuthToken], + ['--embedding-dims', cli.embeddingDims], + ].filter(([, value]) => value !== undefined && value !== false); + if (unsupported.length > 0) { + throw new Error( + `analyze --watch does not support ${unsupported.map(([name]) => name).join(', ')}`, + ); + } + reportIgnoredConfig( + [ + ['embeddings', config.embeddings], + ['dropEmbeddings', config.dropEmbeddings], + ['defaultBranch', config.defaultBranch], + ['skipAgentsMd', config.skipAgentsMd !== undefined], + ['skipSkills', config.skipSkills !== undefined], + ['stats', config.stats !== undefined], + ['walCheckpointThreshold', config.walCheckpointThreshold], + ['embeddingThreads', config.embeddingThreads], + ['embeddingBatchSize', config.embeddingBatchSize], + ['embeddingSubBatchSize', config.embeddingSubBatchSize], + ['embeddingDevice', config.embeddingDevice], + ['embeddingBaseUrl', config.embeddingBaseUrl], + ['embeddingModel', config.embeddingModel], + ] + .filter(([, value]) => value !== undefined && value !== false) + .map(([name]) => String(name)), + ); + const branch = + merged.branch === undefined ? undefined : validateBranchName(merged.branch, '--branch'); + const workerPoolSize = positiveInteger(merged.workers, '--workers'); + const workerTimeoutSeconds = positiveInteger(merged.workerTimeout, 'workerTimeout'); + const maxFileSize = positiveInteger(merged.maxFileSize, 'maxFileSize', MAX_FILE_SIZE_KB); + + setEnvironment( + 'GITNEXUS_MAX_FILE_SIZE', + maxFileSize === undefined ? baseline.maxFileSize : String(maxFileSize), + ); + if (workerTimeoutSeconds !== undefined) { + process.env.GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS = String(workerTimeoutSeconds * 1000); + } else { + setEnvironment('GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS', baseline.workerTimeout); + } + setEnvironment('GITNEXUS_VERBOSE', merged.verbose ? '1' : baseline.verbose); + + return { + pdg: merged.pdg, + branch, + registryName: merged.name, + allowDuplicateName: merged.allowDuplicateName, + workerPoolSize, + fetchWrappers: merged.fetchWrappers, + skipAgentsMd: true, + skipSkills: true, + noStats: true, + atomicIncremental: process.platform !== 'win32', + }; +} + +function refreshSummary( + result: AnalyzeResult, + observedPaths: readonly string[], + durationMs: number, + lastSuccessfulRefreshAt: string, +): string { + const measured = result.incrementalStats; + const changed = measured?.changedFiles ?? (result.alreadyUpToDate ? 0 : observedPaths.length); + const reparsed = + measured?.reparsedFiles ?? + (typeof result.pipelineResult?.reparsedFileCount === 'number' + ? result.pipelineResult.reparsedFileCount + : 0); + const dependents = measured?.affectedDependents ?? 0; + const mode = measured?.writeMode ?? (result.alreadyUpToDate ? 'no-op' : 'full'); + return ( + `Refresh complete: ${changed} changed, ${reparsed} re-parsed, ` + + `${dependents} affected dependent(s), ${durationMs}ms, ${mode}; ` + + `last success ${lastSuccessfulRefreshAt}` + ); +} + +async function waitUntilReady(watcher: FSWatcher): Promise { + await new Promise((resolve, reject) => { + const ready = () => { + watcher.off('error', failed); + resolve(); + }; + const failed = (error: unknown) => { + watcher.off('ready', ready); + reject(error); + }; + watcher.once('ready', ready); + watcher.once('error', failed); + }); +} + +export interface WatchFileLoop { + readonly waitForIdle: () => Promise; + readonly close: () => Promise; +} + +class WatchControlReloadError extends Error { + constructor(cause: unknown) { + super(cause instanceof Error ? cause.message : String(cause), { cause }); + this.name = 'WatchControlReloadError'; + } +} + +export function shouldStopAfterWatchRefreshFailure( + error: unknown, + paths: readonly string[], +): boolean { + return ( + paths.length > 0 && + !(error instanceof WatchControlReloadError) && + analyzeFailureMayHaveMutatedLiveIndex(error) + ); +} + +/** Start the real filesystem watcher with bounded, serialized refreshes. */ +export async function startWatchFileLoop( + repoPath: string, + debounceMs: number, + refresh: (paths: readonly string[]) => Promise, + onError: WatchRefreshError, + onWatcherError: (error: unknown) => void = (error) => onError(error, []), +): Promise { + let ignorePath = await createWatchIgnorePredicate(repoPath); + let ignoreControlValid = true; + const queue = new WatchRefreshQueue( + async (paths) => { + if (paths.some(isIgnoreControlPath) || !ignoreControlValid) { + const retryingInvalidControls = !ignoreControlValid; + try { + ignorePath = await createWatchIgnorePredicate(repoPath); + ignoreControlValid = true; + watcher.add(repoPath); + } catch (error) { + ignoreControlValid = false; + throw new WatchControlReloadError( + retryingInvalidControls + ? new Error( + 'Ignore controls remain invalid; fix them before indexing more changes.', + { + cause: error, + }, + ) + : error, + ); + } + } + await refresh(paths); + }, + onError, + debounceMs, + { + maxWaitMs: Math.max(2_000, debounceMs * 10), + maxPendingPaths: 1_000, + holdEventsUntilInitialRefresh: true, + isPriorityPath: (filePath) => isIgnoreControlPath(filePath) || isConfigControlPath(filePath), + }, + ); + + const watcher: FSWatcher = watch(repoPath, { + ignoreInitial: true, + atomic: true, + followSymlinks: false, + awaitWriteFinish: { stabilityThreshold: 100, pollInterval: 20 }, + ignored: (candidate, stats) => { + const relative = repoRelativeWatchPath(repoPath, candidate); + if (relative !== null && isAnalyzerOwnedWatchPath(relative)) return true; + if (relative !== null && (isIgnoreControlPath(relative) || isConfigControlPath(relative))) { + return false; + } + return ignorePath(candidate, stats?.isDirectory() ?? false); + }, + }); + watcher.on('all', (event, changedPath) => { + if (event !== 'add' && event !== 'change' && event !== 'unlink') return; + const relative = repoRelativeWatchPath(repoPath, changedPath); + if (relative && isRelevantWatchPath(relative) && !isAnalyzerOwnedWatchPath(relative)) { + queue.enqueue(relative); + } + }); + watcher.on('error', (error) => { + // Chokidar can surface a transient EPERM on Windows while an ignored + // analyzer-owned path is replaced. Re-arm the root and force one bounded + // catch-up refresh so a missed event cannot leave the graph stale. Other + // watcher errors may mean coverage was lost and remain fatal. + if (TRANSIENT_WATCH_ERROR_CODES.has((error as NodeJS.ErrnoException).code ?? '')) { + watcher.add(repoPath); + queue.enqueue(WATCH_FULL_REFRESH_PATH); + return; + } + onWatcherError(error); + }); + + try { + await waitUntilReady(watcher); + await queue.runInitial(); + } catch (error) { + await watcher.close(); + await queue.close(); + throw error; + } + + return { + waitForIdle: () => queue.waitForIdle(), + close: async () => { + await watcher.close(); + await queue.close(); + }, + }; +} + +export async function watchCommandWithRunnerIdentity( + runnerIdentityAtBootstrap: AnalyzerRunnerIdentity, + inputPath?: string, + cliOptions: WatchCliOptions = {}, +): Promise { + if (await ensureHeap({ cleanForwardedTermination: true })) return; + + const requestedRepoPath = inputPath ? path.resolve(inputPath) : getGitRoot(process.cwd()); + if (requestedRepoPath === null || !hasGitDir(requestedRepoPath)) { + cliError(' gitnexus analyze --watch requires a Git repository.'); + process.exitCode = 1; + return; + } + const repoPath = await fs.realpath(requestedRepoPath); + const baselineEnvironment: WatchEnvironmentBaseline = { + maxFileSize: process.env.GITNEXUS_MAX_FILE_SIZE, + workerTimeout: process.env.GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS, + verbose: process.env.GITNEXUS_VERBOSE, + }; + try { + let ignoredConfigSignature: string | undefined; + const reportIgnoredConfig = (names: readonly string[]) => { + const signature = [...names].sort().join(','); + if (signature === ignoredConfigSignature) return; + ignoredConfigSignature = signature; + if (names.length > 0) { + cliWarn(`Watch mode ignores unsupported .gitnexusrc settings: ${names.join(', ')}.`); + } + }; + let debounceMs: number; + let analyzeOptions: CoreAnalyzeOptions; + try { + debounceMs = + positiveInteger( + cliOptions.debounce ?? String(DEFAULT_DEBOUNCE_MS), + '--debounce', + MAX_TIMER_DELAY_MS, + ) ?? DEFAULT_DEBOUNCE_MS; + analyzeOptions = await resolveWatchOptions( + repoPath, + cliOptions, + baselineEnvironment, + reportIgnoredConfig, + ); + } catch (error) { + cliError(` ${error instanceof Error ? error.message : String(error)}`); + process.exitCode = 1; + return; + } + + let stopWatching!: () => void; + const stopped = new Promise((resolve) => { + stopWatching = resolve; + }); + const stop = () => stopWatching(); + process.once('SIGINT', stop); + process.once('SIGTERM', stop); + try { + let loop: WatchFileLoop; + let fatalRefreshError: unknown; + let configControlValid = true; + let lastSuccessfulRefreshAt: string | undefined; + try { + loop = await startWatchFileLoop( + repoPath, + debounceMs, + async (paths) => { + if (paths.some(isConfigControlPath) || !configControlValid) { + const retryingInvalidConfig = !configControlValid; + try { + analyzeOptions = await resolveWatchOptions( + repoPath, + cliOptions, + baselineEnvironment, + reportIgnoredConfig, + ); + configControlValid = true; + } catch (error) { + configControlValid = false; + throw new WatchControlReloadError( + retryingInvalidConfig + ? new Error( + 'Configuration remains invalid; fix it before indexing more changes.', + { + cause: error, + }, + ) + : error, + ); + } + } + const startedAt = Date.now(); + const result = await runFullAnalysis( + repoPath, + analyzeOptions, + { + onProgress: () => {}, + onLog: + process.env.GITNEXUS_VERBOSE === '1' + ? (message) => cliInfo(` ${message}`) + : undefined, + }, + runnerIdentityAtBootstrap, + ); + lastSuccessfulRefreshAt = new Date().toISOString(); + if (paths.length === 0) { + cliInfo( + result.alreadyUpToDate + ? `Watching ${repoPath}; index is up to date.` + : `Watching ${repoPath}; initial index ready in ${Date.now() - startedAt}ms.`, + ); + } else { + cliInfo( + refreshSummary(result, paths, Date.now() - startedAt, lastSuccessfulRefreshAt), + ); + } + }, + (error, paths) => { + const detail = paths.length > 0 ? ` (${paths.length} queued path(s))` : ''; + if (shouldStopAfterWatchRefreshFailure(error, paths)) { + fatalRefreshError = error; + cliError( + `Refresh failed${detail}: ${error instanceof Error ? error.message : String(error)}. ` + + 'Watch mode is stopping because the live index may have been updated in place.', + ); + stopWatching(); + return; + } + const lastSuccess = lastSuccessfulRefreshAt ?? 'none yet'; + cliWarn( + `Refresh failed${detail}: ${error instanceof Error ? error.message : String(error)}. ` + + `Retry scheduled; last success ${lastSuccess}.`, + ); + }, + (error) => { + fatalRefreshError = error; + cliError( + `Watcher failed: ${error instanceof Error ? error.message : String(error)}. ` + + 'Watch mode is stopping.', + ); + stopWatching(); + }, + ); + } catch (error) { + cliError( + ` Unable to start watcher: ${error instanceof Error ? error.message : String(error)}`, + ); + process.exitCode = 1; + return; + } + + await stopped; + await loop.close(); + if (fatalRefreshError !== undefined) process.exitCode = 1; + } finally { + process.removeListener('SIGINT', stop); + process.removeListener('SIGTERM', stop); + } + } finally { + setEnvironment('GITNEXUS_MAX_FILE_SIZE', baselineEnvironment.maxFileSize); + setEnvironment('GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS', baselineEnvironment.workerTimeout); + setEnvironment('GITNEXUS_VERBOSE', baselineEnvironment.verbose); + } +} diff --git a/gitnexus/src/config/ignore-service.ts b/gitnexus/src/config/ignore-service.ts index f1b9e1150..ac2dc7704 100644 --- a/gitnexus/src/config/ignore-service.ts +++ b/gitnexus/src/config/ignore-service.ts @@ -3,6 +3,7 @@ import { existsSync } from 'fs'; import fs from 'fs/promises'; import nodePath from 'path'; import type { Path } from 'path-scurry'; +import { readRepoControlFile } from './repo-control-file.js'; import { logger } from '../core/logger.js'; import { getCoreExcludesFilePath, getGitInfoExcludePath } from '../storage/git.js'; @@ -401,6 +402,8 @@ export interface IgnoreOptions { noGitignore?: boolean; /** Skip core.excludesFile and $GIT_COMMON_DIR/info/exclude. Defaults to GITNEXUS_NO_GLOBAL_IGNORE env var. */ noGlobalIgnore?: boolean; + /** Fail repository-control reloads closed so long-lived watchers keep their prior predicate. */ + strictRepoControlFiles?: boolean; } export const loadIgnoreRules = async ( @@ -442,20 +445,56 @@ export const loadIgnoreRules = async ( for (const filename of filenames) { try { - const content = await fs.readFile(nodePath.join(repoPath, filename), 'utf-8'); + const content = options?.strictRepoControlFiles + ? await readRepoControlFile(repoPath, filename) + : await fs.readFile(nodePath.join(repoPath, filename), 'utf-8'); + if (content === null) continue; ig.add(content); hasRules = true; } catch (err: unknown) { const code = (err as NodeJS.ErrnoException).code; - if (code !== 'ENOENT') { - logger.warn(` Warning: could not read ${filename}: ${(err as Error).message}`); - } + if (!options?.strictRepoControlFiles && code === 'ENOENT') continue; + if (options?.strictRepoControlFiles) throw err; + logger.warn(` Warning: could not read ${filename}: ${(err as Error).message}`); } } return hasRules ? ig : null; }; +/** + * Build a synchronous predicate for long-lived filesystem watchers. + * + * Unlike {@link createIgnoreFilter}, callers pass ordinary absolute or + * repository-relative paths instead of path-scurry `Path` objects. The rule + * precedence deliberately mirrors the scanner: explicit negations win over + * hardcoded defaults unless a more-specific rule re-ignores the path. + */ +export const createWatchIgnorePredicate = async ( + repoPath: string, + options?: IgnoreOptions, +): Promise<(candidatePath: string, isDirectory?: boolean) => boolean> => { + const ig = await loadIgnoreRules(repoPath, { ...options, strictRepoControlFiles: true }); + const repoRoot = nodePath.resolve(repoPath); + + return (candidatePath: string, isDirectory = false): boolean => { + const absolute = nodePath.isAbsolute(candidatePath) + ? nodePath.resolve(candidatePath) + : nodePath.resolve(repoRoot, candidatePath); + const rel = nodePath.relative(repoRoot, absolute).replace(/\\/g, '/'); + if (!rel) return false; + if (rel === '..' || rel.startsWith('../') || nodePath.isAbsolute(rel)) return true; + + if (ig && hasExplicitUnignore(ig, rel) && !ig.ignores(isDirectory ? `${rel}/` : rel)) { + return false; + } + + if (ig && ig.ignores(isDirectory ? `${rel}/` : rel)) return true; + if (isDirectory && isHardcodedIgnoredDirectoryAtPath(repoRoot, absolute)) return true; + return shouldIgnorePath(rel); + }; +}; + /** * Walk ancestor segments of `rel` and check whether `.gitnexusignore` * (or `.gitignore`) contains an explicit `!pattern` negation that diff --git a/gitnexus/src/config/repo-control-file.ts b/gitnexus/src/config/repo-control-file.ts new file mode 100644 index 000000000..13e08adb0 --- /dev/null +++ b/gitnexus/src/config/repo-control-file.ts @@ -0,0 +1,117 @@ +import fs from 'node:fs'; +import * as path from 'node:path'; + +export const MAX_REPO_CONTROL_FILE_BYTES = 1024 * 1024; + +/** Read a bounded, regular control file owned by the repository root. */ +export async function readRepoControlFile( + repoRoot: string, + filename: string, +): Promise { + const requestedRoot = path.resolve(repoRoot); + const requested = path.resolve(requestedRoot, filename); + const relative = path.relative(requestedRoot, requested); + if (relative.startsWith('..') || path.isAbsolute(relative)) { + throw new Error(`${filename} resolves outside the repository root`); + } + + try { + const canonicalRoot = fs.realpathSync(requestedRoot); + const beforeOpen = fs.lstatSync(requested); + if (beforeOpen.isSymbolicLink()) throw new Error(`${filename} must not be a symbolic link`); + if (!beforeOpen.isFile()) throw new Error(`${filename} must be a regular file`); + if (beforeOpen.nlink !== 1) throw new Error(`${filename} must not be a hard link`); + if (beforeOpen.size > MAX_REPO_CONTROL_FILE_BYTES) { + throw new Error(`${filename} exceeds ${MAX_REPO_CONTROL_FILE_BYTES} bytes`); + } + return await new Promise((resolve, reject) => { + const stream = fs.createReadStream(requested, { + flags: 'r', + start: 0, + end: MAX_REPO_CONTROL_FILE_BYTES, + autoClose: true, + }); + const chunks: Buffer[] = []; + let totalBytes = 0; + let validated = false; + let settled = false; + + const finish = (value: string): void => { + if (settled) return; + settled = true; + resolve(value); + }; + const fail = (error: unknown): void => { + if (settled) return; + settled = true; + reject(error); + }; + + stream.pause(); + stream.once('open', (fd) => { + try { + const opened = fs.fstatSync(fd); + if (!opened.isFile()) throw new Error(`${filename} must be a regular file`); + if (opened.nlink !== 1) throw new Error(`${filename} must not be a hard link`); + if (opened.size > MAX_REPO_CONTROL_FILE_BYTES) { + throw new Error(`${filename} exceeds ${MAX_REPO_CONTROL_FILE_BYTES} bytes`); + } + + const entry = fs.lstatSync(requested); + if (entry.isSymbolicLink()) throw new Error(`${filename} must not be a symbolic link`); + if ( + !entry.isFile() || + entry.nlink !== 1 || + entry.dev !== opened.dev || + entry.ino !== opened.ino + ) { + throw new Error(`${filename} moved or was replaced while being opened`); + } + const canonicalFile = fs.realpathSync(requested); + const canonicalRelative = path.relative(canonicalRoot, canonicalFile); + if (canonicalRelative.startsWith('..') || path.isAbsolute(canonicalRelative)) { + throw new Error(`${filename} resolves outside the repository root`); + } + const canonical = fs.statSync(canonicalFile); + if ( + canonical.nlink !== 1 || + canonical.dev !== opened.dev || + canonical.ino !== opened.ino + ) { + throw new Error(`${filename} moved or was replaced while being opened`); + } + + validated = true; + stream.resume(); + } catch (error) { + fail(error); + stream.destroy(); + } + }); + stream.on('data', (chunk: Buffer | string) => { + const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + totalBytes += bytes.length; + if (totalBytes > MAX_REPO_CONTROL_FILE_BYTES) { + fail(new Error(`${filename} exceeds ${MAX_REPO_CONTROL_FILE_BYTES} bytes`)); + stream.destroy(); + return; + } + chunks.push(bytes); + }); + stream.once('end', () => { + if (!validated) { + fail(new Error(`${filename} could not be validated`)); + return; + } + finish(Buffer.concat(chunks, totalBytes).toString('utf8')); + }); + stream.once('error', fail); + stream.once('close', () => { + if (!settled) fail(new Error(`${filename} closed before it could be read`)); + }); + }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; + throw error; + } +} diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index cd4b8edf4..157e29027 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -477,6 +477,8 @@ export async function runChunkedParseAndResolve( * files. There is no sequential parser — the pool is the sole parse path * whenever a chunk misses the cache. */ usedWorkerPool: boolean; + /** Files dispatched to parser workers after parse-cache lookup. */ + reparsedFileCount: number; /** Worker-produced ParsedFile artifacts aggregated across chunks. * Threaded into scope-resolution as a re-extract cache so the warm- * cache analyze run can skip the dominant `extractParsedFile` cost @@ -783,6 +785,7 @@ export async function runChunkedParseAndResolve( : new Set(); let chunkCacheHits = 0; let chunkCacheMisses = 0; + let reparsedFileCount = 0; try { // U1 — bounded chunk concurrency (B1 from PR #1693 review): pre-fetch @@ -1106,6 +1109,7 @@ export async function runChunkedParseAndResolve( // Cache miss: dispatch to workers, capture the raw results, store // them under the chunk hash for the next run. chunkCacheMisses++; + reparsedFileCount += chunkFiles.length; if (durableParsedFileDir !== undefined && chunkHash !== null) { try { await prepareDurableParsedFileChunk(durableParsedFileDir, chunkHash); @@ -1622,6 +1626,11 @@ export async function runChunkedParseAndResolve( // no pool was needed: a warm all-cache-hit run replays cached worker output // without spawning workers, or there were no parseable files. usedWorkerPool: workerPool !== undefined, + // Exact number of files sent through workers on parse-cache misses. A + // changed file can invalidate its whole content-addressed chunk, so this + // is intentionally measured at dispatch time rather than inferred from + // the git/hash diff. + reparsedFileCount, // Per-file ParsedFile artifacts produced by workers' calls to // `extractParsedFile`. Consumed by scope-resolution as a re-extraction // cache: when the file's ParsedFile is here, scope-resolution skips its own diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts index 38bd4601b..8e151ed73 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts @@ -71,6 +71,8 @@ export interface ParseOutput { * is no sequential parser; the pool is the sole parse path on a cache miss. */ readonly usedWorkerPool: boolean; + /** Files actually dispatched to parser workers after parse-cache lookup. */ + readonly reparsedFileCount: number; /** * Per-file `ParsedFile` artifacts produced by workers' calls to * `extractParsedFile`. Threaded through to `scopeResolutionPhase` diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index 69858ff28..050822e87 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -370,11 +370,13 @@ export const runPipelineFromRepo = async ( } // Extract final results for the PipelineResult contract - const { totalFiles, usedWorkerPool, unavailableScopeLanguageFiles } = getPhaseOutput<{ - totalFiles: number; - usedWorkerPool: boolean; - unavailableScopeLanguageFiles: number; - }>(results, 'parse'); + const { totalFiles, usedWorkerPool, reparsedFileCount, unavailableScopeLanguageFiles } = + getPhaseOutput<{ + totalFiles: number; + usedWorkerPool: boolean; + reparsedFileCount: number; + unavailableScopeLanguageFiles: number; + }>(results, 'parse'); let communityResult: CommunitiesOutput['communityResult'] | undefined; let processResult: ProcessesOutput['processResult'] | undefined; @@ -426,6 +428,7 @@ export const runPipelineFromRepo = async ( resolutionOutcomes, undecidedSatisfaction, usedWorkerPool, + reparsedFileCount, scopeExtractionFailures, unavailableScopeLanguageFiles, pdgEmitManifest, diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index f3889d444..94d53de16 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -13,6 +13,7 @@ import { detectGraphWriteCollapse, type GraphWriteCollapseVerdict } from './inde import { PDG_EDGE_TYPES } from './lbug/pdg-emit-sink.js'; import path from 'path'; import fs from 'fs/promises'; +import { constants as fsConstants } from 'node:fs'; import { randomUUID } from 'node:crypto'; import { retryRename } from '../storage/fs-atomic.js'; import { acquireIndexLock } from '../storage/index-lock.js'; @@ -471,6 +472,29 @@ export interface AnalyzeOptions { * Process exit reclaims the handles. Long-lived callers (MCP server, tests) * leave this unset so they get a real close. See `closeLbug`. */ skipNativeCloseOnExit?: boolean; + /** + * Stage an incremental write in a copy of the live index before publishing + * it. Used by long-lived watch mode so a failed refresh leaves the previous + * graph readable. Currently supported on POSIX, where an open DB can be + * atomically renamed; Windows retains the established in-place path. + */ + atomicIncremental?: boolean; +} + +const liveIndexMutationRisks = new WeakSet(); + +function recordLiveIndexMutationRisk(error: unknown): void { + if ((typeof error === 'object' && error !== null) || typeof error === 'function') { + liveIndexMutationRisks.add(error); + } +} + +/** Whether a failed analyze may already have changed the live DB. */ +export function analyzeFailureMayHaveMutatedLiveIndex(error: unknown): boolean { + return ( + ((typeof error === 'object' && error !== null) || typeof error === 'function') && + liveIndexMutationRisks.has(error) + ); } export interface AnalyzeResult { @@ -522,6 +546,14 @@ export interface AnalyzeResult { * (The historical "primary" name is kept — it is public API surface.) */ isPrimaryBranch?: boolean; + /** Measured work performed by a successful incremental refresh. */ + incrementalStats?: { + changedFiles: number; + reparsedFiles: number; + affectedDependents: number; + deletedFiles: number; + writeMode: 'incremental' | 'full'; + }; } /** @@ -1944,12 +1976,15 @@ async function runFullAnalysisInner( process.platform === 'win32' && options.pdg !== true && process.env.GITNEXUS_ATOMIC_WINDOWS_SWAP === '1'; - // Incremental atomicity copies the whole index into the temp before mutating - // it, which negates incremental's speed premise — so it is opt-in - // (GITNEXUS_ATOMIC_INCREMENTAL=1) pending a benchmark. Full rebuilds always - // swap where the platform allows. + // Incremental atomicity stages the whole index before mutation. It remains + // opt-in for ordinary analyze runs; watch mode requests it for failure + // preservation. The copy requests a filesystem clone and records its actual + // duration, while Node falls back to a normal copy where reflinks are absent. const wantAtomicIncremental = - isIncremental && !!hashDiff && process.env.GITNEXUS_ATOMIC_INCREMENTAL === '1'; + isIncremental && + !!hashDiff && + process.platform !== 'win32' && + (options.atomicIncremental === true || process.env.GITNEXUS_ATOMIC_INCREMENTAL === '1'); // #2614 F3: the copy-then-swap stages ONLY the main lbug file, so a live index // carrying an orphan .wal/.shadow (a silently-failed prior checkpoint) would // be copied incompletely and lose that delta. Only take the atomic path when @@ -1967,6 +2002,10 @@ async function runFullAnalysisInner( // valve. Nothing between here and there reads either binding except // `initLbug(buildPath)`, which the upgrade re-runs against the staging path. let useAtomicSwap = (isFullRebuild || atomicIncremental) && (posixSwap || windowsSwapOk); + // Set only at the first operation that can mutate the live graph store. + // Pre-write failures (config, lock, parsing, metadata, importer expansion) + // remain retryable even when this platform cannot use an atomic swap. + let liveIndexMutationStarted = false; // #2658: a per-run staging name (was the fixed `lbug.new`). Even under the // single-writer lock, a unique name means a crashed run's half-built staging // file can never be mistaken for — or clobber — a live run's; the lock's @@ -1999,10 +2038,15 @@ async function runFullAnalysisInner( if (atomicIncremental) { // Stage the live index into the temp so the in-place delete/writeback // below mutates the COPY, and the end-of-run swap publishes it atomically. - // Clear any stale temp first (a crashed run), then copy the (consolidated, - // single-file) live index. Whole-file copy — hence opt-in. + // Clear any stale temp first (a crashed run), then clone/copy the + // consolidated single-file live index. await wipeLbugDbFiles(buildPath); - await fs.copyFile(lbugPath, buildPath); + const copyStartedAt = Date.now(); + await fs.copyFile(lbugPath, buildPath, fsConstants.COPYFILE_FICLONE); + log( + `atomic-incremental: staged ${lbugPath} in ${Date.now() - copyStartedAt}ms ` + + '(copy-on-write requested; filesystem fallback is allowed)', + ); } } else { // Full rebuild path: wipe DB files first. @@ -2038,7 +2082,13 @@ async function runFullAnalysisInner( // (`buildPath` = `.new`, clearing any stragglers from a crashed // run) and leaves the live index untouched until the end-of-run swap. On // Windows buildPath === lbugPath, so this is the original in-place wipe. - await wipeLbugDbFiles(buildPath); + if (buildPath === lbugPath) liveIndexMutationStarted = true; + try { + await wipeLbugDbFiles(buildPath); + } catch (error) { + if (liveIndexMutationStarted) recordLiveIndexMutationRisk(error); + throw error; + } } // Size the buffer pool to the graph just built by the pipeline (a page cache @@ -2061,7 +2111,12 @@ async function runFullAnalysisInner( // Full rebuild (POSIX) builds into the temp `buildPath`; incremental and // Windows use `buildPath === lbugPath` in place. - await initLbug(buildPath); + try { + await initLbug(buildPath); + } catch (error) { + if (liveIndexMutationStarted) recordLiveIndexMutationRisk(error); + throw error; + } // Manual WAL checkpoint driver (#1741): periodically drain the WAL // from JS so the un-retriable native auto-checkpoint almost never @@ -2086,6 +2141,7 @@ async function runFullAnalysisInner( // "escalated full write" (DB wiped, index destroyed) — tri-review // 4669518496 P1. let escalatedFullWrite = false; + let incrementalStats: AnalyzeResult['incrementalStats']; // Phase 3.5's restore scope (FIX 3 of this shipping review): on the // SURGICAL write plan this is the exact file set whose rows // deleteNodesForFiles just removed — only THOSE files' cached embedding @@ -2211,6 +2267,13 @@ async function runFullAnalysisInner( } } const importerExpansion = writableFiles.size - directlyChangedCount; + incrementalStats = { + changedFiles: hashDiff.changed.length + hashDiff.added.length + hashDiff.deleted.length, + reparsedFiles: pipelineResult.reparsedFileCount, + affectedDependents: importerExpansion, + deletedFiles: hashDiff.deleted.length, + writeMode: 'incremental', + }; await saveIncrementalDirtyState('importer-bfs', { importerExpansion, shadowSeedCount: shadowSeed.length, @@ -2572,6 +2635,7 @@ async function runFullAnalysisInner( } await walCheckpointDriver.stop(); await closeLbug(); + if (buildPath === lbugPath) liveIndexMutationStarted = true; await wipeLbugDbFiles(buildPath); await initLbug(buildPath); walCheckpointDriver = startWalCheckpointDriver(); @@ -2597,6 +2661,7 @@ async function runFullAnalysisInner( // same connection, and nothing on this branch creates or drops an index // in between — so re-reading would only weaken the one-read invariant // the snapshot type exists to enforce. + if (buildPath === lbugPath) liveIndexMutationStarted = true; await dropSearchFTSIndexes(indexCatalogRows); // 1b. Remove the write set's existing rows — batched (#2409): one // DETACH DELETE per table per 200-file chunk. The former per-file @@ -3773,6 +3838,7 @@ async function runFullAnalysisInner( : false; if (useAtomicSwap && builtDbExists) { await retryRename(buildPath, lbugPath); + liveIndexMutationStarted = true; // Clear any sidecars orphaned beside the replaced file. A cleanly-closed // prior index has none; a crashed one could, and it would be replay // poison next to the freshly published index. Best-effort. @@ -3807,6 +3873,12 @@ async function runFullAnalysisInner( ftsSkipped: !ftsReady, ftsSkipReason: ftsReady ? undefined : ftsSkipReason, isPrimaryBranch: !placement.branch, + incrementalStats: incrementalStats + ? { + ...incrementalStats, + writeMode: escalatedFullWrite ? 'full' : 'incremental', + } + : undefined, }; } catch (err) { // Ensure LadybugDB is closed even on error. Stop the driver first @@ -3845,6 +3917,11 @@ async function runFullAnalysisInner( /* swallow — orphan reclamation must never mask the real failure */ } } + if (liveIndexMutationStarted) { + // Preserve the original error identity/prototype: callers distinguish + // IndexLockTimeoutError and other domain failures with `instanceof`. + recordLiveIndexMutationRisk(err); + } throw err; } } diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 636b73486..bcbb4c189 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -917,7 +917,17 @@ export const persistParseCacheChunk = async ( createdCacheDirs.add(cacheDir); } const payload = JSON.stringify(slim, mapReplacer); - await fs.writeFile(getCacheChunkPath(cache.storagePath, chunkHash), payload, 'utf-8'); + const chunkPath = getCacheChunkPath(cache.storagePath, chunkHash); + try { + await fs.writeFile(chunkPath, payload, 'utf-8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + // Long-lived analyze --watch processes can replace the sharded cache + // directory after this process-local memo recorded it as created. + await fs.mkdir(cacheDir, { recursive: true }); + createdCacheDirs.add(cacheDir); + await fs.writeFile(chunkPath, payload, 'utf-8'); + } cache.onDiskKeys ??= new Set(); cache.onDiskKeys.add(chunkHash); cache.entries.delete(chunkHash); diff --git a/gitnexus/src/types/pipeline.ts b/gitnexus/src/types/pipeline.ts index 950cd3f31..5c11f800b 100644 --- a/gitnexus/src/types/pipeline.ts +++ b/gitnexus/src/types/pipeline.ts @@ -40,6 +40,8 @@ export interface PipelineResult { * affordance so regression suites can prove the pool engaged. */ usedWorkerPool: boolean; + /** Files actually dispatched to parser workers after parse-cache lookup. */ + reparsedFileCount: number; /** Files omitted from scope-resolution while the rest of analysis continued. */ scopeExtractionFailures: readonly string[]; /** Files scope resolution could not inspect because their parser was unavailable. */ diff --git a/gitnexus/test/integration/analyze-atomic-swap.test.ts b/gitnexus/test/integration/analyze-atomic-swap.test.ts index 8bee4aba0..f664a6bb6 100644 --- a/gitnexus/test/integration/analyze-atomic-swap.test.ts +++ b/gitnexus/test/integration/analyze-atomic-swap.test.ts @@ -22,17 +22,28 @@ type LbugAdapter = typeof import('../../src/core/lbug/lbug-adapter.js'); const ctx = vi.hoisted(() => ({ loadMock: vi.fn(), realLoad: null as LbugAdapter['loadGraphToLbug'] | null, + deleteMock: vi.fn(), + realDelete: null as LbugAdapter['deleteNodesForFiles'] | null, })); // Delegating mock: overrides only loadGraphToLbug so a rebuild can be made to // fail on demand (mirrors run-analyze-adopt-failure.test.ts). vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => { const actual = await importOriginal(); ctx.realLoad = actual.loadGraphToLbug; + ctx.realDelete = actual.deleteNodesForFiles; ctx.loadMock.mockImplementation(actual.loadGraphToLbug); - return { ...actual, loadGraphToLbug: ctx.loadMock }; + ctx.deleteMock.mockImplementation(actual.deleteNodesForFiles); + return { + ...actual, + loadGraphToLbug: ctx.loadMock, + deleteNodesForFiles: ctx.deleteMock, + }; }); -import { runFullAnalysis } from '../../src/core/run-analyze.js'; +import { + analyzeFailureMayHaveMutatedLiveIndex, + runFullAnalysis, +} from '../../src/core/run-analyze.js'; import { getStoragePaths } from '../../src/storage/repo-manager.js'; import { initLbug as poolInit, @@ -68,6 +79,10 @@ describe.skipIf(isWin)('atomic full-rebuild swap (#2)', () => { ctx.loadMock.mockImplementation((...a: Parameters) => ctx.realLoad!(...a), ); + ctx.deleteMock.mockReset(); + ctx.deleteMock.mockImplementation((...a: Parameters) => + ctx.realDelete!(...a), + ); }); afterEach(async () => { @@ -129,6 +144,29 @@ describe.skipIf(isWin)('atomic full-rebuild swap (#2)', () => { } }, 180_000); + it('marks a failure after an atomic publish as potentially live-mutating', async () => { + const { repo, cleanup } = await makeRepo(); + try { + const failure = await runFullAnalysis( + repo, + {}, + { + onProgress: (phase, percent) => { + if (phase === 'done' && percent === 100) { + throw new Error('injected post-publish failure'); + } + }, + }, + ).catch((error: unknown) => error); + + expect(failure).toMatchObject({ message: 'injected post-publish failure' }); + expect(analyzeFailureMayHaveMutatedLiveIndex(failure)).toBe(true); + await expect(fs.stat(getStoragePaths(repo).lbugPath)).resolves.toBeTruthy(); + } finally { + await cleanup(); + } + }, 180_000); + it('the read pool serves the freshly-swapped index after a rebuild (#1 + #2 end-to-end)', async () => { const { repo, cleanup } = await makeRepo(); const repoId = 'atomic-swap-e2e'; @@ -202,6 +240,76 @@ describe.skipIf(isWin)('atomic full-rebuild swap (#2)', () => { } }, 180_000); + it('keeps the live graph unchanged when atomic incremental writeback fails', async () => { + const { repo, cleanup } = await makeRepo(); + const repoId = 'atomic-incr-failure'; + try { + await runFullAnalysis(repo, {}, { onProgress: () => {} }); + const { lbugPath } = getStoragePaths(repo); + const before = await identity(lbugPath); + + await fs.writeFile( + path.join(repo, 'a.ts'), + 'export function greet(n: string) { return `hi ${n}`; }\nexport function caller() { return greet("x"); }\nexport function addedAfterRetry() { return 1; }\n', + ); + execSync('git -c user.name=t -c user.email=t@t commit -am change', { + cwd: repo, + stdio: 'pipe', + }); + + ctx.deleteMock.mockRejectedValueOnce(new Error('injected incremental write failure')); + const failure = await runFullAnalysis( + repo, + { atomicIncremental: true }, + { onProgress: () => {} }, + ).catch((error: unknown) => error); + expect(failure).toMatchObject({ message: 'injected incremental write failure' }); + expect(analyzeFailureMayHaveMutatedLiveIndex(failure)).toBe(false); + expect(await identity(lbugPath)).toBe(before); + expect(await lingeringTemp(lbugPath)).toEqual([]); + + await poolInit(repoId, lbugPath); + const beforeRetry = ( + await poolQuery(repoId, 'MATCH (f:Function) RETURN f.name AS n') + ).flatMap((row) => Object.values(row as Record).map(String)); + expect(beforeRetry).toContain('greet'); + expect(beforeRetry).not.toContain('addedAfterRetry'); + await poolClose(repoId); + + await runFullAnalysis(repo, { atomicIncremental: true }, { onProgress: () => {} }); + await poolInit(repoId, lbugPath); + const afterRetry = (await poolQuery(repoId, 'MATCH (f:Function) RETURN f.name AS n')).flatMap( + (row) => Object.values(row as Record).map(String), + ); + expect(afterRetry).toContain('addedAfterRetry'); + } finally { + await poolClose(repoId); + await cleanup(); + } + }, 180_000); + + it('marks failed in-place incremental writes as potentially live-mutating', async () => { + const { repo, cleanup } = await makeRepo(); + try { + await runFullAnalysis(repo, {}, { onProgress: () => {} }); + await fs.writeFile(path.join(repo, 'a.ts'), 'export function changed() { return 1; }\n'); + execSync('git -c user.name=t -c user.email=t@t commit -am change', { + cwd: repo, + stdio: 'pipe', + }); + + ctx.deleteMock.mockRejectedValueOnce(new Error('injected in-place failure')); + const failure = await runFullAnalysis(repo, {}, { onProgress: () => {} }).catch( + (error: unknown) => error, + ); + expect(failure).toBeInstanceOf(Error); + expect(failure).toMatchObject({ message: 'injected in-place failure' }); + expect(analyzeFailureMayHaveMutatedLiveIndex(failure)).toBe(true); + } finally { + await cleanup(); + } + }, 180_000); + it('publishes cleanly on the production close path (skipNativeCloseOnExit) (#2614 F5)', async () => { const { repo, cleanup } = await makeRepo(); try { diff --git a/gitnexus/test/integration/cli-e2e.test.ts b/gitnexus/test/integration/cli-e2e.test.ts index d44b5c4a8..ffff7dcfd 100644 --- a/gitnexus/test/integration/cli-e2e.test.ts +++ b/gitnexus/test/integration/cli-e2e.test.ts @@ -1048,6 +1048,198 @@ describe('CLI end-to-end', () => { expect(result.stdout).toMatch(/analyze|status|serve/i); }); + it('shows the analyze watch mode and its debounce controls', () => { + const result = runCliRaw(['analyze', '--help'], MINI_REPO); + expect(result.status).toBe(0); + expect(result.stdout).toContain('--watch'); + expect(result.stdout).toContain('--debounce'); + expect(result.stdout).toContain('--workers'); + }); + + it('rejects --debounce without --watch', () => { + const result = runCliRaw(['analyze', '--debounce', '25'], MINI_REPO); + expect(result.status).toBe(1); + expect(result.stderr).toContain('--debounce requires --watch'); + }); + + it('runs production analyze --watch with exact telemetry and transactional config reloads', async () => { + const repo = makeMiniRepoCopy('watch-repo', 'gn-watch-cli-'); + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-watch-cli-home-')); + try { + fs.writeFileSync( + path.join(repo, '.gitnexusrc'), + JSON.stringify({ workers: '1', maxFileSize: '1' }), + 'utf8', + ); + await new Promise((resolve, reject) => { + const child = spawn( + process.execPath, + [...CLI_SPAWN_PREFIX, 'analyze', repo, '--watch', '--debounce', '25', '--workers', '1'], + { + cwd: repo, + stdio: ['ignore', 'pipe', 'pipe'], + env: cliEnv({ GITNEXUS_HOME: home }), + }, + ); + let stdout = ''; + let stderr = ''; + let transcript = ''; + let baselineNodes: number | undefined; + let stage = 'ready'; + let stageOffset = 0; + let settled = false; + const timer = setTimeout(() => { + if (settled) return; + settled = true; + child.kill('SIGTERM'); + reject(new Error(`watch CLI timed out\nstdout:\n${stdout}\nstderr:\n${stderr}`)); + }, 480_000); + + const advance = (nextStage: string, action: () => void) => { + stage = nextStage; + stageOffset = transcript.length; + setTimeout(action, 200); + }; + + const writeLargeSource = (fileName: string, functionName: string) => { + fs.writeFileSync( + path.join(repo, fileName), + `const padding = '${'x'.repeat(1_500)}';\n` + + `export function ${functionName}(): number { return padding.length; }\n`, + 'utf8', + ); + }; + + const handleOutput = () => { + const output = transcript.slice(stageOffset); + if (stage === 'ready' && /Watching .*index (?:is up to date|ready)/.test(output)) { + const meta = JSON.parse( + fs.readFileSync(path.join(repo, '.gitnexus', 'gitnexus.json'), 'utf8'), + ); + baselineNodes = meta.stats.nodes; + advance('proof', () => { + fs.writeFileSync( + path.join(repo, 'watch-proof.ts'), + 'export function watchProof(): number { return 1; }\n', + 'utf8', + ); + }); + return; + } + if (stage === 'proof' && /Refresh complete: 1 changed, 8 re-parsed,/.test(output)) { + const meta = JSON.parse( + fs.readFileSync(path.join(repo, '.gitnexus', 'gitnexus.json'), 'utf8'), + ); + expect(meta.stats.nodes).toBeGreaterThan(baselineNodes!); + advance('first-large-file', () => + writeLargeSource('oversized-before.ts', 'skippedByLimit'), + ); + return; + } + if ( + stage === 'first-large-file' && + output.includes('Skipped 1 large files (>1KB)') && + output.includes('- oversized-before.ts') && + /Refresh complete: 0 changed,/.test(output) + ) { + advance('invalid-config', () => { + fs.writeFileSync( + path.join(repo, '.gitnexusrc'), + JSON.stringify({ workers: '1', maxFileSize: '0' }), + 'utf8', + ); + }); + return; + } + if ( + stage === 'invalid-config' && + /Refresh failed.*maxFileSize must be a positive integer/.test(output) + ) { + advance('second-large-file', () => + writeLargeSource('oversized-after-invalid.ts', 'stillSkipped'), + ); + return; + } + if ( + stage === 'second-large-file' && + /Refresh failed.*Configuration remains invalid/.test(output) + ) { + advance('recovered-config', () => { + fs.writeFileSync( + path.join(repo, '.gitnexusrc'), + JSON.stringify({ workers: '1', maxFileSize: '4096' }), + 'utf8', + ); + }); + return; + } + if ( + stage === 'recovered-config' && + /Refresh complete: [2-9][0-9]* changed, [1-9][0-9]* re-parsed,/.test(output) + ) { + stage = 'stopping'; + setTimeout(() => child.kill('SIGTERM'), 100); + } + }; + child.stderr.on('data', (chunk: Buffer) => { + const text = chunk.toString(); + stderr += text; + transcript += text; + handleOutput(); + }); + child.stdout.on('data', (chunk: Buffer) => { + const text = chunk.toString(); + stdout += text; + transcript += text; + handleOutput(); + }); + child.once('error', (error) => { + if (settled) return; + settled = true; + clearTimeout(timer); + reject(error); + }); + child.once('close', (code, signal) => { + if (settled) return; + settled = true; + clearTimeout(timer); + const expectedWindowsTermination = + process.platform === 'win32' && code === null && signal === 'SIGTERM'; + if (code !== 0 && !expectedWindowsTermination) { + reject( + new Error( + `watch CLI exited ${code ?? signal}\nstdout:\n${stdout}\nstderr:\n${stderr}`, + ), + ); + return; + } + expect(stage).toBe('stopping'); + expect(transcript).toContain('Refresh complete: 1 changed, 8 re-parsed,'); + resolve(); + }); + }); + + for (const [symbol, file] of [ + ['watchProof', 'watch-proof.ts'], + ['skippedByLimit', 'oversized-before.ts'], + ['stillSkipped', 'oversized-after-invalid.ts'], + ]) { + const result = runCliWithEnv( + ['context', symbol, '--file', file], + repo, + { GITNEXUS_HOME: home }, + 30_000, + ); + expect(result.status).toBe(0); + expect(result.stdout).toContain(symbol); + expect(result.stdout).toContain(file); + } + } finally { + cleanupTempDirSync(path.dirname(repo)); + cleanupTempDirSync(home); + } + }, 540_000); + it('fails with unknown command', () => { const result = runCliRaw(['nonexistent'], MINI_REPO); diff --git a/gitnexus/test/integration/watch-filesystem.test.ts b/gitnexus/test/integration/watch-filesystem.test.ts new file mode 100644 index 000000000..84fa4f8c3 --- /dev/null +++ b/gitnexus/test/integration/watch-filesystem.test.ts @@ -0,0 +1,230 @@ +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { startWatchFileLoop, type WatchFileLoop } from '../../src/cli/watch.js'; +import { cleanupTempDir } from '../helpers/test-db.js'; + +const tempDirs: string[] = []; +const loops: WatchFileLoop[] = []; + +async function waitFor(predicate: () => boolean, timeoutMs = 5_000): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() >= deadline) throw new Error('timed out waiting for watcher event'); + await new Promise((resolve) => setTimeout(resolve, 25)); + } +} + +async function makeRepo(): Promise { + const repo = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-watch-fs-')); + tempDirs.push(repo); + execFileSync('git', ['init', '-q'], { cwd: repo }); + return repo; +} + +afterEach(async () => { + await Promise.all(loops.splice(0).map((loop) => loop.close())); + await Promise.all(tempDirs.splice(0).map((dir) => cleanupTempDir(dir))); +}); + +describe('watch filesystem integration', () => { + it('fails startup and closes the watcher when the initial analysis fails', async () => { + const repo = await makeRepo(); + const onError = vi.fn(); + + await expect( + startWatchFileLoop( + repo, + 25, + async () => { + throw new Error('initial analysis failed'); + }, + onError, + ), + ).rejects.toThrow('initial analysis failed'); + expect(onError).not.toHaveBeenCalled(); + }); + + it('never enqueues analyzer-owned .gitnexus writes created by the initial refresh', async () => { + const repo = await makeRepo(); + const batches: string[][] = []; + const loop = await startWatchFileLoop( + repo, + 25, + async (paths) => { + batches.push([...paths]); + if (paths.length === 0) { + await fs.mkdir(path.join(repo, '.gitnexus'), { recursive: true }); + await fs.writeFile(path.join(repo, '.gitnexus', 'gitnexus.json'), '{}\n', 'utf8'); + await fs.writeFile(path.join(repo, '.gitnexus', 'lbug'), 'index bytes', 'utf8'); + } + }, + (error) => { + throw error; + }, + ); + loops.push(loop); + + await new Promise((resolve) => setTimeout(resolve, 200)); + await loop.waitForIdle(); + + expect(batches).toEqual([[]]); + }); + + it('coalesces indexed add/change/rename/delete events and stops cleanly', async () => { + const repo = await makeRepo(); + const batches: string[][] = []; + const loop = await startWatchFileLoop( + repo, + 30, + async (paths) => batches.push([...paths]), + (error) => { + throw error; + }, + ); + loops.push(loop); + expect(batches).toEqual([[]]); + + await fs.writeFile(path.join(repo, 'README.md'), '# One', 'utf8'); + await fs.writeFile(path.join(repo, 'src.ts'), 'export const one = 1;', 'utf8'); + await fs.writeFile(path.join(repo, 'src.ts'), 'export const one = 2;', 'utf8'); + await waitFor(() => batches.flat().includes('README.md') && batches.flat().includes('src.ts')); + + await fs.rename(path.join(repo, 'src.ts'), path.join(repo, 'renamed.ts')); + await waitFor(() => batches.flat().includes('renamed.ts')); + await fs.rm(path.join(repo, 'renamed.ts')); + await waitFor(() => batches.flat().filter((entry) => entry === 'renamed.ts').length >= 2); + + expect(batches.flat()).toEqual(expect.arrayContaining(['README.md', 'src.ts', 'renamed.ts'])); + await loop.close(); + loops.pop(); + const countAfterClose = batches.length; + await fs.writeFile(path.join(repo, 'after-close.ts'), 'export {};', 'utf8'); + await new Promise((resolve) => setTimeout(resolve, 150)); + expect(batches).toHaveLength(countAfterClose); + }); + + it('queues edits during refresh, recovers after failure, and ignores external symlinks', async () => { + const repo = await makeRepo(); + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-watch-outside-')); + tempDirs.push(outside); + await fs.symlink( + outside, + path.join(repo, 'external'), + process.platform === 'win32' ? 'junction' : 'dir', + ); + const successful: string[][] = []; + const errors: string[][] = []; + let failNext = false; + let releaseRefresh: (() => void) | undefined; + const loop = await startWatchFileLoop( + repo, + 25, + async (paths) => { + if (failNext) { + failNext = false; + throw new Error('injected refresh failure'); + } + successful.push([...paths]); + if (paths.includes('first.ts')) { + await new Promise((resolve) => { + releaseRefresh = resolve; + }); + } + }, + (_error, paths) => errors.push([...paths]), + ); + loops.push(loop); + + await fs.writeFile(path.join(repo, 'first.ts'), 'export const first = 1;', 'utf8'); + await waitFor(() => releaseRefresh !== undefined); + await fs.writeFile(path.join(repo, 'during.ts'), 'export const during = 1;', 'utf8'); + releaseRefresh!(); + await waitFor(() => successful.flat().includes('during.ts')); + + failNext = true; + await fs.writeFile(path.join(repo, 'fails.ts'), 'export const fail = 1;', 'utf8'); + await waitFor(() => errors.length === 1); + await waitFor(() => successful.flat().includes('fails.ts')); + await fs.writeFile(path.join(repo, 'retry.ts'), 'export const retry = 1;', 'utf8'); + await waitFor(() => successful.flat().includes('retry.ts')); + + const beforeExternal = successful.length + errors.length; + await fs.writeFile(path.join(outside, 'outside.ts'), 'export const outside = 1;', 'utf8'); + await new Promise((resolve) => setTimeout(resolve, 200)); + expect(successful.length + errors.length).toBe(beforeExternal); + }); + + it('reloads gitignore rules before processing subsequent file events', async () => { + const repo = await makeRepo(); + await fs.writeFile(path.join(repo, '.gitignore'), 'blocked.ts\n', 'utf8'); + const batches: string[][] = []; + const loop = await startWatchFileLoop( + repo, + 25, + async (paths) => batches.push([...paths]), + (error) => { + throw error; + }, + ); + loops.push(loop); + + await fs.writeFile(path.join(repo, 'blocked.ts'), 'export const blocked = 1;', 'utf8'); + await new Promise((resolve) => setTimeout(resolve, 200)); + expect(batches.flat()).not.toContain('blocked.ts'); + + await fs.writeFile(path.join(repo, '.gitignore'), '', 'utf8'); + await waitFor(() => batches.flat().includes('.gitignore')); + await fs.writeFile(path.join(repo, 'blocked.ts'), 'export const blocked = 2;', 'utf8'); + await waitFor(() => batches.flat().includes('blocked.ts')); + }); + + it('keeps the last valid ignore predicate after an oversized reload and later recovers', async () => { + const repo = await makeRepo(); + await fs.writeFile(path.join(repo, '.gitignore'), 'blocked.ts\n', 'utf8'); + const batches: string[][] = []; + const errors: string[][] = []; + const loop = await startWatchFileLoop( + repo, + 25, + async (paths) => batches.push([...paths]), + (_error, paths) => errors.push([...paths]), + ); + loops.push(loop); + + await fs.writeFile(path.join(repo, '.gitignore'), 'x'.repeat(1024 * 1024 + 1), 'utf8'); + await waitFor(() => errors.flat().includes('.gitignore')); + await waitFor(() => errors.length >= 2); + await fs.writeFile(path.join(repo, 'other.ts'), 'export const other = 1;', 'utf8'); + await waitFor(() => errors.flat().includes('other.ts')); + expect(batches.flat()).not.toContain('other.ts'); + await fs.writeFile(path.join(repo, 'blocked.ts'), 'export const blocked = 1;', 'utf8'); + await new Promise((resolve) => setTimeout(resolve, 200)); + expect(batches.flat()).not.toContain('blocked.ts'); + + await fs.writeFile(path.join(repo, '.gitignore'), '', 'utf8'); + await waitFor(() => batches.flat().includes('.gitignore')); + await fs.writeFile(path.join(repo, 'blocked.ts'), 'export const blocked = 2;', 'utf8'); + await waitFor(() => batches.flat().includes('blocked.ts')); + }); + + it('observes root control files even when gitignore excludes them', async () => { + const repo = await makeRepo(); + await fs.writeFile(path.join(repo, '.gitignore'), '.gitnexusrc\n', 'utf8'); + const batches: string[][] = []; + const loop = await startWatchFileLoop( + repo, + 25, + async (paths) => batches.push([...paths]), + (error) => { + throw error; + }, + ); + loops.push(loop); + + await fs.writeFile(path.join(repo, '.gitnexusrc'), '{}\n', 'utf8'); + await waitFor(() => batches.flat().includes('.gitnexusrc')); + }); +}); diff --git a/gitnexus/test/unit/analyze-config.test.ts b/gitnexus/test/unit/analyze-config.test.ts index 988c9cb87..60d4c545f 100644 --- a/gitnexus/test/unit/analyze-config.test.ts +++ b/gitnexus/test/unit/analyze-config.test.ts @@ -1,9 +1,12 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fsSync from 'node:fs'; import fs from 'fs/promises'; import path from 'path'; import os from 'os'; import { loadAnalyzeConfig, + loadAnalyzeConfigStrict, mergeAnalyzeOptions, resolveDefaultBranch, validateBranchName, @@ -13,6 +16,10 @@ import { DEFAULT_BRANCH_FALLBACK, } from '../../src/cli/analyze-config.js'; import type { AnalyzeOptions } from '../../src/cli/analyze.js'; +import { + MAX_REPO_CONTROL_FILE_BYTES, + readRepoControlFile, +} from '../../src/config/repo-control-file.js'; describe('analyze-config (.gitnexusrc support, #243)', () => { let dir: string; @@ -34,6 +41,77 @@ describe('analyze-config (.gitnexusrc support, #243)', () => { expect(loadAnalyzeConfig(dir)).toBeUndefined(); }); + it('rejects an oversized repository config before parsing', async () => { + await writeRc(' '.repeat(MAX_REPO_CONTROL_FILE_BYTES + 1)); + await expect(loadAnalyzeConfigStrict(dir)).rejects.toThrow(/exceeds/); + }); + + it('keeps the read bounded if a control file grows after its size check', async () => { + await writeRc('{}'); + const fstatSync = fsSync.fstatSync; + const stat = vi.spyOn(fsSync, 'fstatSync').mockImplementation((fd) => { + const opened = fstatSync(fd); + Object.defineProperty(opened, 'size', { value: MAX_REPO_CONTROL_FILE_BYTES + 1 }); + return opened; + }); + + try { + await expect(readRepoControlFile(dir, GITNEXUS_RC_FILENAME)).rejects.toThrow(/exceeds/); + expect(stat).toHaveBeenCalledOnce(); + } finally { + stat.mockRestore(); + } + }); + + it('rejects a hardlinked repository config', async () => { + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-rc-hardlink-')); + try { + const target = path.join(outside, 'config.json'); + await fs.writeFile(target, JSON.stringify({ workers: '8' })); + await fs.link(target, path.join(dir, GITNEXUS_RC_FILENAME)); + await expect(loadAnalyzeConfigStrict(dir)).rejects.toThrow(/hard link/); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32')( + 'rejects a FIFO before opening it for reading', + async () => { + const fifo = path.join(dir, GITNEXUS_RC_FILENAME); + execFileSync('mkfifo', [fifo]); + let timeout: ReturnType | undefined; + + try { + await expect( + Promise.race([ + readRepoControlFile(dir, GITNEXUS_RC_FILENAME), + new Promise((_resolve, reject) => { + timeout = setTimeout(() => reject(new Error('FIFO read did not fail promptly')), 500); + }), + ]), + ).rejects.toThrow(/regular file/); + } finally { + if (timeout !== undefined) clearTimeout(timeout); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects a final-file symlink for repository config', + async () => { + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-rc-outside-')); + try { + const target = path.join(outside, 'config.json'); + await fs.writeFile(target, JSON.stringify({ workers: '8' })); + await fs.symlink(target, path.join(dir, GITNEXUS_RC_FILENAME), 'file'); + await expect(loadAnalyzeConfigStrict(dir)).rejects.toThrow(/symbolic link/); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }, + ); + it('throws an actionable error on invalid JSON, naming the file', async () => { await writeRc('{ not valid json '); expect(() => loadAnalyzeConfig(dir)).toThrow(GitNexusRcError); diff --git a/gitnexus/test/unit/analyze-heap-respawn.test.ts b/gitnexus/test/unit/analyze-heap-respawn.test.ts index 53b684f9b..7176fb46d 100644 --- a/gitnexus/test/unit/analyze-heap-respawn.test.ts +++ b/gitnexus/test/unit/analyze-heap-respawn.test.ts @@ -220,6 +220,15 @@ describe('analyzeCommand heap respawn', () => { expect(parseMaxOldSpaceMb('--max-old-space-size --other-flag')).toBeNull(); }); + it('preserves conventional signal exits for analyze but treats watch shutdown as clean', async () => { + const { forwardedSignalExitCode } = await import('../../src/cli/analyze.js'); + expect(forwardedSignalExitCode('SIGINT', false)).toBe(130); + expect(forwardedSignalExitCode('SIGTERM', false)).toBe(143); + expect(forwardedSignalExitCode('SIGINT', true)).toBe(0); + expect(forwardedSignalExitCode('SIGTERM', true)).toBe(0); + expect(forwardedSignalExitCode('SIGABRT', false)).toBe(1); + }); + it('GITNEXUS_MEMORY=off also disables the default (unpinned) respawn (#2649 review)', async () => { delete process.env.NODE_OPTIONS; process.env.GITNEXUS_MEMORY = 'off'; diff --git a/gitnexus/test/unit/cross-platform-shard.test.ts b/gitnexus/test/unit/cross-platform-shard.test.ts index da88151f9..d5e83552e 100644 --- a/gitnexus/test/unit/cross-platform-shard.test.ts +++ b/gitnexus/test/unit/cross-platform-shard.test.ts @@ -3,7 +3,7 @@ * * The regression this guards is specific and was expensive: three CHEAP files * were registered in `SPAWN_CLI`, vitest re-partitioned the list by file COUNT, - * and the reshuffle clustered `cli-e2e` (361 s on Windows) with `cli-limit-e2e` + * and the reshuffle clustered `cli-e2e` (now 621 s on Windows) with `cli-limit-e2e` * (75 s) and `analyze-heap-oom-e2e` (23 s) on one shard, which then blew the * 20-minute watchdog. The added files cost nothing; the COUNT-split did it. * @@ -49,7 +49,7 @@ describe('cross-platform shard partition', () => { }); it('never puts the two heaviest suites on the same shard', () => { - // The exact shape of the outage: cli-e2e and worker-pool are 361 s and + // The exact shape of the outage: cli-e2e and worker-pool are 621 s and // 222 s, so together they are most of a shard's budget before anything else // is scheduled. const shards = allShards(ALL_CROSS_PLATFORM, SHARD_TOTAL); diff --git a/gitnexus/test/unit/incremental-orchestration.test.ts b/gitnexus/test/unit/incremental-orchestration.test.ts index ff315faa6..eb792b701 100644 --- a/gitnexus/test/unit/incremental-orchestration.test.ts +++ b/gitnexus/test/unit/incremental-orchestration.test.ts @@ -842,6 +842,13 @@ describe('runFullAnalysis — incremental orchestration', () => { { onProgress: () => {} }, ); expect(incremental.alreadyUpToDate).toBeUndefined(); + expect(incremental.incrementalStats).toMatchObject({ + changedFiles: 1, + affectedDependents: 2, + deletedFiles: 0, + writeMode: 'incremental', + }); + expect(incremental.incrementalStats?.reparsedFiles).toBe(7); expect( querySpy.mock.calls.some( ([query]) => diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index b476f9743..8097c8ca5 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -748,4 +748,31 @@ describe('loadParseCache / saveParseCache (round-trip)', () => { await rm(dir, { recursive: true, force: true }); } }); + + it('recreates a memoized shard directory after a long-lived process replaces it', async () => { + const dir = await mkdtemp(path.join(tmpdir(), 'gnx-pc-')); + try { + const firstKey = 'd'.repeat(64); + const secondKey = 'e'.repeat(64); + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set([firstKey]), + storagePath: dir, + onDiskKeys: new Set(), + }; + + await persistParseCacheChunk(cache, firstKey, [minimalResult({ fileCount: 1 })]); + await rm(path.join(dir, 'parse-cache'), { recursive: true, force: true }); + + cache.usedKeys = new Set([secondKey]); + await persistParseCacheChunk(cache, secondKey, [minimalResult({ fileCount: 2 })]); + await saveParseCache(dir, cache); + + const loaded = await loadParseCache(dir); + expect((await loadParseCacheChunk(loaded, secondKey))?.[0]?.fileCount).toBe(2); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); }); diff --git a/gitnexus/test/unit/watch-failure-policy.test.ts b/gitnexus/test/unit/watch-failure-policy.test.ts new file mode 100644 index 000000000..aa32f0454 --- /dev/null +++ b/gitnexus/test/unit/watch-failure-policy.test.ts @@ -0,0 +1,29 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const analyzeFailureMayHaveMutatedLiveIndex = vi.hoisted(() => vi.fn()); + +vi.mock('../../src/core/run-analyze.js', () => ({ + analyzeFailureMayHaveMutatedLiveIndex, + runFullAnalysis: vi.fn(), +})); + +import { shouldStopAfterWatchRefreshFailure } from '../../src/cli/watch.js'; + +describe('watch refresh failure policy', () => { + beforeEach(() => analyzeFailureMayHaveMutatedLiveIndex.mockReset()); + + it('retries a queued pre-write failure even when incremental writes are in-place', () => { + const error = new Error('failed before live graph mutation'); + analyzeFailureMayHaveMutatedLiveIndex.mockReturnValue(false); + + expect(shouldStopAfterWatchRefreshFailure(error, ['src/a.ts'])).toBe(false); + }); + + it('stops only when a queued failure may have mutated the live graph', () => { + const error = new Error('failed during live graph mutation'); + analyzeFailureMayHaveMutatedLiveIndex.mockReturnValue(true); + + expect(shouldStopAfterWatchRefreshFailure(error, ['src/a.ts'])).toBe(true); + expect(shouldStopAfterWatchRefreshFailure(error, [])).toBe(false); + }); +}); diff --git a/gitnexus/test/unit/watch-paths.test.ts b/gitnexus/test/unit/watch-paths.test.ts new file mode 100644 index 000000000..8accbb5ee --- /dev/null +++ b/gitnexus/test/unit/watch-paths.test.ts @@ -0,0 +1,180 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { createWatchIgnorePredicate } from '../../src/config/ignore-service.js'; +import { isRelevantWatchPath, resolveWatchOptions } from '../../src/cli/watch.js'; +import * as git from '../../src/storage/git.js'; + +vi.mock('../../src/storage/git.js', () => ({ + getCoreExcludesFilePath: vi.fn(), + getGitInfoExcludePath: vi.fn(), +})); + +let repoPath: string; + +beforeEach(async () => { + repoPath = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-watch-')); + vi.mocked(git.getCoreExcludesFilePath).mockReturnValue(null); + vi.mocked(git.getGitInfoExcludePath).mockReturnValue(null); +}); + +afterEach(async () => { + await fs.rm(repoPath, { recursive: true, force: true }); +}); + +describe('watch path selection', () => { + it('accepts every scanner-admitted file instead of maintaining a second allow-list', () => { + expect(isRelevantWatchPath('src/service.ts')).toBe(true); + expect(isRelevantWatchPath('server/app.py')).toBe(true); + expect(isRelevantWatchPath('backend/project.csproj')).toBe(true); + expect(isRelevantWatchPath('.gitnexusrc')).toBe(true); + expect(isRelevantWatchPath('README.md')).toBe(true); + expect(isRelevantWatchPath('docs/guide.mdx')).toBe(true); + expect(isRelevantWatchPath('config/application-prod.yml')).toBe(true); + expect(isRelevantWatchPath('src/main/resources/application.properties')).toBe(true); + expect(isRelevantWatchPath('templates/page.html')).toBe(true); + expect(isRelevantWatchPath('templates/page.htm')).toBe(true); + expect(isRelevantWatchPath('views/page.ejs')).toBe(true); + expect(isRelevantWatchPath('views/page.hbs')).toBe(true); + expect(isRelevantWatchPath('views/page.blade.php')).toBe(true); + expect( + isRelevantWatchPath( + 'src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports', + ), + ).toBe(true); + expect(isRelevantWatchPath('src/main/resources/META-INF/spring.factories')).toBe(true); + expect(isRelevantWatchPath('tsconfig.base.json')).toBe(true); + expect(isRelevantWatchPath('packages/api/tsconfig.build.json')).toBe(true); + expect(isRelevantWatchPath('schema.sql')).toBe(true); + expect(isRelevantWatchPath('Dockerfile')).toBe(true); + expect(isRelevantWatchPath('assets/logo.png')).toBe(true); + expect(isRelevantWatchPath('../outside.ts')).toBe(false); + expect(isRelevantWatchPath('C:\\outside.ts')).toBe(false); + }); + + it('honors hardcoded, gitignore, and explicit-unignore rules', async () => { + await fs.writeFile( + path.join(repoPath, '.gitignore'), + ['generated/*', '!generated/', '!generated/keep.ts'].join('\n'), + ); + const ignored = await createWatchIgnorePredicate(repoPath); + + expect(ignored(path.join(repoPath, 'node_modules', 'pkg', 'index.ts'))).toBe(true); + expect(ignored(path.join(repoPath, 'generated'), true)).toBe(false); + expect(ignored(path.join(repoPath, 'generated', 'drop.ts'))).toBe(true); + expect(ignored(path.join(repoPath, 'generated', 'keep.ts'))).toBe(false); + expect(ignored(path.join(repoPath, 'src', 'keep.ts'))).toBe(false); + expect(ignored(path.resolve(repoPath, '..', 'outside.ts'))).toBe(true); + }); + + it('does not partially mutate environment state when a reloaded config is invalid', async () => { + const names = [ + 'GITNEXUS_MAX_FILE_SIZE', + 'GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS', + 'GITNEXUS_VERBOSE', + ] as const; + const original = Object.fromEntries(names.map((name) => [name, process.env[name]])); + try { + await fs.writeFile( + path.join(repoPath, '.gitnexusrc'), + JSON.stringify({ maxFileSize: '2048', workerTimeout: '90', workers: '2' }), + ); + const baseline = { maxFileSize: '512', workerTimeout: '30000', verbose: undefined }; + await resolveWatchOptions(repoPath, {}, baseline); + expect(process.env.GITNEXUS_MAX_FILE_SIZE).toBe('2048'); + expect(process.env.GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS).toBe('90000'); + + await fs.writeFile( + path.join(repoPath, '.gitnexusrc'), + JSON.stringify({ maxFileSize: '4096', workerTimeout: '120', workers: '0' }), + ); + await expect(resolveWatchOptions(repoPath, {}, baseline)).rejects.toThrow( + '--workers must be a positive integer', + ); + expect(process.env.GITNEXUS_MAX_FILE_SIZE).toBe('2048'); + expect(process.env.GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS).toBe('90000'); + } finally { + for (const name of names) { + const value = original[name]; + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + } + }); + + it('ignores unsupported repository defaults but rejects explicit unsupported CLI flags', async () => { + await fs.writeFile( + path.join(repoPath, '.gitnexusrc'), + JSON.stringify({ + embeddings: true, + defaultBranch: 'develop', + skipAgentsMd: false, + skipSkills: false, + stats: true, + }), + ); + const ignored: string[][] = []; + await expect( + resolveWatchOptions( + repoPath, + {}, + { + maxFileSize: undefined, + workerTimeout: undefined, + verbose: undefined, + }, + (names) => ignored.push([...names]), + ), + ).resolves.toMatchObject({ skipAgentsMd: true, skipSkills: true }); + expect(ignored).toEqual([ + ['embeddings', 'defaultBranch', 'skipAgentsMd', 'skipSkills', 'stats'], + ]); + + const unsupportedCliOptions: Array<[Parameters[1], string]> = [ + [{ embeddings: true }, '--embeddings'], + [{ defaultBranch: 'develop' }, '--default-branch'], + [{ skipAgentsMd: true }, '--skip-agents-md'], + [{ skipSkills: true }, '--skip-skills'], + [{ stats: false }, '--no-stats'], + ]; + for (const [options, flag] of unsupportedCliOptions) { + await expect( + resolveWatchOptions(repoPath, options, { + maxFileSize: undefined, + workerTimeout: undefined, + verbose: undefined, + }), + ).rejects.toThrow(`analyze --watch does not support ${flag}`); + } + }); + + it('rejects a watch file-size threshold above the parser ceiling', async () => { + await expect( + resolveWatchOptions( + repoPath, + { maxFileSize: '32769' }, + { + maxFileSize: undefined, + workerTimeout: undefined, + verbose: undefined, + }, + ), + ).rejects.toThrow('maxFileSize must not exceed 32768'); + }); + + it.skipIf(process.platform === 'win32')( + 'rejects repository ignore files that are final-file symlinks', + async () => { + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-watch-outside-')); + try { + const target = path.join(outside, 'ignore'); + await fs.writeFile(target, 'secret.ts\n'); + await fs.symlink(target, path.join(repoPath, '.gitignore'), 'file'); + await expect(createWatchIgnorePredicate(repoPath)).rejects.toThrow(/symbolic link/); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }, + ); +}); diff --git a/gitnexus/test/unit/watch-queue.test.ts b/gitnexus/test/unit/watch-queue.test.ts new file mode 100644 index 000000000..260008a96 --- /dev/null +++ b/gitnexus/test/unit/watch-queue.test.ts @@ -0,0 +1,348 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { WATCH_FULL_REFRESH_PATH, WatchRefreshQueue } from '../../src/cli/watch-queue.js'; + +afterEach(() => vi.useRealTimers()); + +describe('WatchRefreshQueue', () => { + it('propagates an initial refresh failure without reporting it as retryable', async () => { + const onError = vi.fn(); + const queue = new WatchRefreshQueue( + async () => { + throw new Error('initial analyze failed'); + }, + onError, + 10, + ); + + await expect(queue.runInitial()).rejects.toThrow('initial analyze failed'); + expect(onError).not.toHaveBeenCalled(); + await queue.close(); + }); + + it('debounces and deduplicates rapid writes', async () => { + vi.useFakeTimers(); + const batches: readonly string[][] = []; + const mutable = batches as string[][]; + const queue = new WatchRefreshQueue( + async (paths) => mutable.push([...paths]), + () => {}, + 100, + ); + + queue.enqueue('src/a.ts'); + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(50); + queue.enqueue('src/b.ts'); + await vi.advanceTimersByTimeAsync(99); + expect(batches).toEqual([]); + await vi.advanceTimersByTimeAsync(1); + await queue.waitForIdle(); + + expect(batches).toEqual([['src/a.ts', 'src/b.ts']]); + }); + + it('queues edits made during a refresh and never overlaps writers', async () => { + vi.useFakeTimers(); + let releaseFirst!: () => void; + let active = 0; + let peak = 0; + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => { + active++; + peak = Math.max(peak, active); + batches.push([...paths]); + if (batches.length === 1) await new Promise((resolve) => (releaseFirst = resolve)); + active--; + }, + () => {}, + 100, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(100); + queue.enqueue('src/b.ts'); + queue.enqueue('src/c.ts'); + releaseFirst(); + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(100); + await queue.waitForIdle(); + + expect(peak).toBe(1); + expect(batches).toEqual([['src/a.ts'], ['src/b.ts', 'src/c.ts']]); + }); + + it('retries a failed refresh without dropping its batch', async () => { + vi.useFakeTimers(); + const errors: string[][] = []; + const successful: string[][] = []; + let attempts = 0; + const queue = new WatchRefreshQueue( + async (paths) => { + attempts++; + if (attempts === 1) throw new Error('failed'); + successful.push([...paths]); + }, + (_error, paths) => errors.push([...paths]), + 10, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + expect(errors).toEqual([['src/a.ts']]); + expect(successful).toEqual([]); + await vi.advanceTimersByTimeAsync(250); + await queue.waitForIdle(); + + expect(errors).toEqual([['src/a.ts']]); + expect(successful).toEqual([['src/a.ts']]); + }); + + it('parks pre-ready events until the initial refresh completes', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => batches.push([...paths]), + () => {}, + 50, + { holdEventsUntilInitialRefresh: true }, + ); + + queue.enqueue('src/during-walk.ts'); + await vi.advanceTimersByTimeAsync(80); + expect(batches).toEqual([]); + + await queue.runInitial(); + expect(batches).toEqual([[]]); + await vi.advanceTimersByTimeAsync(50); + await queue.waitForIdle(); + expect(batches).toEqual([[], ['src/during-walk.ts']]); + }); + + it('backs off repeated failures instead of spinning at the debounce interval', async () => { + vi.useFakeTimers(); + let attempts = 0; + const queue = new WatchRefreshQueue( + async () => { + attempts++; + if (attempts < 4) throw new Error('still unavailable'); + }, + () => {}, + 10, + { retryBaseDelayMs: 100, retryMaxDelayMs: 400 }, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + expect(attempts).toBe(1); + await vi.advanceTimersByTimeAsync(99); + expect(attempts).toBe(1); + await vi.advanceTimersByTimeAsync(1); + expect(attempts).toBe(2); + await vi.advanceTimersByTimeAsync(199); + expect(attempts).toBe(2); + await vi.advanceTimersByTimeAsync(1); + expect(attempts).toBe(3); + await vi.advanceTimersByTimeAsync(399); + expect(attempts).toBe(3); + await vi.advanceTimersByTimeAsync(1); + await queue.waitForIdle(); + expect(attempts).toBe(4); + }); + + it('merges an event during retry backoff without shortening the retry delay', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + let attempts = 0; + const queue = new WatchRefreshQueue( + async (paths) => { + attempts++; + if (attempts === 1) throw new Error('failed'); + batches.push([...paths]); + }, + () => {}, + 10, + { retryBaseDelayMs: 1_000 }, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + expect(attempts).toBe(1); + + await vi.advanceTimersByTimeAsync(100); + queue.enqueue('src/b.ts'); + await vi.advanceTimersByTimeAsync(899); + expect(attempts).toBe(1); + + await vi.advanceTimersByTimeAsync(1); + await queue.waitForIdle(); + + expect(attempts).toBe(2); + expect(batches).toEqual([['src/a.ts', 'src/b.ts']]); + }); + + it('contains a throwing error reporter for a detached refresh', async () => { + vi.useFakeTimers(); + const queue = new WatchRefreshQueue( + async () => { + throw new Error('refresh failed'); + }, + async () => { + throw new Error('reporting failed'); + }, + 10, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + + await queue.close(); + await expect(queue.waitForIdle()).resolves.toBeUndefined(); + }); + + it('contains a synchronously throwing refresh and retries its batch', async () => { + vi.useFakeTimers(); + const errors: string[][] = []; + const successful: string[][] = []; + let attempts = 0; + const queue = new WatchRefreshQueue( + (paths) => { + attempts++; + if (attempts === 1) throw new Error('synchronous refresh failure'); + successful.push([...paths]); + return Promise.resolve(); + }, + (_error, paths) => errors.push([...paths]), + 10, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + expect(errors).toEqual([['src/a.ts']]); + await vi.advanceTimersByTimeAsync(250); + await queue.waitForIdle(); + + expect(attempts).toBe(2); + expect(successful).toEqual([['src/a.ts']]); + }); + + it('closes cleanly when a refresh failure triggers shutdown', async () => { + vi.useFakeTimers(); + let closePromise: Promise | undefined; + const queue = new WatchRefreshQueue( + async () => { + throw new Error('stop after failure'); + }, + () => { + closePromise = queue.close(); + }, + 10, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + + expect(closePromise).toBeDefined(); + await expect(closePromise).resolves.toBeUndefined(); + }); + + it('flushes by max wait even when writes never become quiet', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => batches.push([...paths]), + () => {}, + 100, + { maxWaitMs: 250 }, + ); + + queue.enqueue('src/0.ts'); + await vi.advanceTimersByTimeAsync(90); + queue.enqueue('src/1.ts'); + await vi.advanceTimersByTimeAsync(90); + queue.enqueue('src/2.ts'); + await vi.advanceTimersByTimeAsync(70); + await queue.waitForIdle(); + + expect(batches).toEqual([['src/0.ts', 'src/1.ts', 'src/2.ts']]); + }); + + it('bounds high-cardinality paths while retaining priority control files', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => batches.push([...paths]), + () => {}, + 10, + { + maxPendingPaths: 2, + isPriorityPath: (filePath) => filePath === '.gitnexusrc', + }, + ); + + for (let index = 0; index < 20; index++) queue.enqueue(`src/${index}.ts`); + queue.enqueue('.gitnexusrc'); + await vi.advanceTimersByTimeAsync(10); + await queue.waitForIdle(); + + expect(batches).toEqual([[WATCH_FULL_REFRESH_PATH, '.gitnexusrc', 'src/1.ts']]); + }); + + it('bounds a flood of distinct priority paths', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => batches.push([...paths]), + () => {}, + 10, + { + maxPendingPaths: 2, + isPriorityPath: (filePath) => filePath.endsWith('/.gitignore'), + }, + ); + + for (let index = 0; index < 20; index++) queue.enqueue(`packages/${index}/.gitignore`); + await vi.advanceTimersByTimeAsync(10); + await queue.waitForIdle(); + + expect(batches).toEqual([ + [WATCH_FULL_REFRESH_PATH, 'packages/0/.gitignore', 'packages/1/.gitignore'], + ]); + }); + + it('does not report overflow for a duplicate at capacity', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => batches.push([...paths]), + () => {}, + 10, + { maxPendingPaths: 2 }, + ); + + queue.enqueue('src/a.ts'); + queue.enqueue('src/b.ts'); + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + await queue.waitForIdle(); + + expect(batches).toEqual([['src/a.ts', 'src/b.ts']]); + }); + + it('runs a full refresh when the pending-path limit is zero', async () => { + vi.useFakeTimers(); + const batches: string[][] = []; + const queue = new WatchRefreshQueue( + async (paths) => batches.push([...paths]), + () => {}, + 10, + { maxPendingPaths: 0 }, + ); + + queue.enqueue('src/a.ts'); + await vi.advanceTimersByTimeAsync(10); + await queue.waitForIdle(); + + expect(batches).toEqual([[WATCH_FULL_REFRESH_PATH]]); + }); +}); From b7850e669546a79d66e8ab1da85a67e6f64f1fe9 Mon Sep 17 00:00:00 2001 From: guyua9 Date: Sat, 29 Aug 2026 19:33:22 +0800 Subject: [PATCH 3/4] fix(cursor): preserve quoted shell search patterns (#2938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cursor): preserve quoted shell search patterns * fix: preserve backslashes in quoted shell patterns * fix(cursor): parse attached regexp options * fix(cursor): parse attached regexp options * fix(cursor): honor rg end-of-options marker * fix(cursor): scan repeated regexp options (#2938) Keep parsing after short explicit patterns so later eligible regexps are selected without mistaking path operands for search terms. Co-authored-by: Cursor * fix(cursor): harden shell search pattern parsing (#2938) Keep unquoted Windows backslashes so rg.exe paths still parse, skip pattern-file operands, and treat grep -r as recursive rather than a valued replace flag. Co-authored-by: Cursor --------- Co-authored-by: luyua9 Co-authored-by: Gergő Magyar Co-authored-by: Gergo Magyar Co-authored-by: Cursor --- .../hooks/gitnexus-hook.cjs | 221 +++++++++++++++++- gitnexus/test/unit/cursor-hook.test.ts | 103 +++++--- 2 files changed, 284 insertions(+), 40 deletions(-) diff --git a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs index e68aca1de..564384f83 100644 --- a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs +++ b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs @@ -85,40 +85,241 @@ function findGitNexusDir(startDir) { return null; } +function tokenizeShellWords(command) { + const tokens = []; + let current = ''; + let quote = null; + let escaped = false; + let hasToken = false; + + for (let index = 0; index < command.length; index += 1) { + const char = command[index]; + if (escaped) { + current += char; + escaped = false; + hasToken = true; + continue; + } + + if (quote === "'") { + if (char === "'") quote = null; + else current += char; + hasToken = true; + continue; + } + + if (quote === '"') { + if (char === '"') { + quote = null; + } else if (char === '\\') { + const next = command[index + 1]; + if (next === '$' || next === '`' || next === '"' || next === '\\') { + escaped = true; + } else { + current += '\\'; + } + } else { + current += char; + } + hasToken = true; + continue; + } + + if (char === '\\') { + const next = command[index + 1]; + if (next === undefined || /\s/.test(next) || next === "'" || next === '"' || next === '\\') { + escaped = true; + } else { + current += '\\' + next; + index += 1; + } + hasToken = true; + } else if (char === "'" || char === '"') { + quote = char; + hasToken = true; + } else if (/\s/.test(char)) { + if (hasToken) tokens.push(current); + current = ''; + hasToken = false; + } else if (char === ';' || char === '|' || char === '&') { + if (hasToken) tokens.push(current); + current = ''; + hasToken = false; + const next = command[index + 1]; + if ((char === '|' || char === '&') && next === char) { + tokens.push(char + char); + index += 1; + } else { + tokens.push(char); + } + } else { + current += char; + hasToken = true; + } + } + + if (escaped) current += '\\'; + if (hasToken) tokens.push(current); + return tokens; +} + function parseRgGrepPattern(cmd) { - const tokens = cmd.split(/\s+/); + const tokens = tokenizeShellWords(cmd); let foundCmd = false; let skipNext = false; + let skipNextAsPattern = false; + let endOfOptions = false; + let explicitPatternSeen = false; + let patternFileSeen = false; const flagsWithValues = new Set([ '-e', '-f', + '--file', '-m', + '--max-count', '-A', '-B', '-C', '-g', '--glob', + '--iglob', '-t', '--type', '--include', '--exclude', + '--encoding', + '--path', ]); + const rgValueFlags = new Set(['-r', '--replace']); + const patternFlags = new Set(['-e', '--regexp']); + const connectors = new Set(['&&', '||', ';', '|', '&']); + const wrappers = new Set([ + 'npx', + 'bunx', + 'pnpm', + 'yarn', + 'npm', + 'sudo', + 'env', + 'command', + 'time', + 'nice', + 'xargs', + 'dlx', + 'exec', + 'run', + 'git', + ]); + const wrapperFlagsWithValues = new Set([ + '--package', + '-p', + '--call', + '--prefix', + '--shell', + '--filter', + '--workspace', + '--dir', + '--cwd', + ]); + const basename = (token) => + token + .split(/[\\/]/) + .pop() + ?.replace(/\.(exe|cmd|bat)$/i, ''); + let previousToken; + let seenWrapper = false; + let searchCommand = null; for (const token of tokens) { if (skipNext) { skipNext = false; + if (skipNextAsPattern) { + skipNextAsPattern = false; + if (token.length >= 3) return token; + } + previousToken = token; continue; } if (!foundCmd) { - if (/\brg$|\bgrep$/.test(token)) foundCmd = true; + if (connectors.has(token)) { + seenWrapper = false; + previousToken = token; + continue; + } + const commandName = basename(token); + if (wrappers.has(commandName)) { + seenWrapper = true; + previousToken = token; + continue; + } + if (seenWrapper && token.startsWith('-')) { + const flagName = token.split('=', 1)[0]; + if (!token.includes('=') && wrapperFlagsWithValues.has(flagName)) skipNext = true; + previousToken = token; + continue; + } + if (seenWrapper && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) { + previousToken = token; + continue; + } + const atCommandPosition = + previousToken === undefined || + connectors.has(previousToken) || + wrappers.has(basename(previousToken)) || + seenWrapper; + if (atCommandPosition && (commandName === 'rg' || commandName === 'grep')) { + foundCmd = true; + searchCommand = commandName; + } else if (seenWrapper) { + seenWrapper = false; + } + previousToken = token; + continue; + } + previousToken = token; + if (endOfOptions) { + if (explicitPatternSeen || patternFileSeen) continue; + return token.length >= 3 ? token : null; + } + if (token === '--') { + endOfOptions = true; continue; } if (token.startsWith('-')) { - if (flagsWithValues.has(token)) skipNext = true; + if (token === '-f' || token === '--file') { + skipNext = true; + patternFileSeen = true; + continue; + } + if (token.startsWith('--file=')) { + patternFileSeen = true; + continue; + } + if (token.startsWith('--regexp=')) { + explicitPatternSeen = true; + const value = token.slice('--regexp='.length); + if (value.length >= 3) return value; + continue; + } + const attachedPattern = token.match(/^-e(.+)$/); + if (attachedPattern) { + explicitPatternSeen = true; + if (attachedPattern[1].length >= 3) return attachedPattern[1]; + continue; + } + if ( + flagsWithValues.has(token) || + patternFlags.has(token) || + (searchCommand === 'rg' && rgValueFlags.has(token)) + ) { + skipNext = true; + skipNextAsPattern = patternFlags.has(token); + if (skipNextAsPattern) explicitPatternSeen = true; + } continue; } - const cleaned = token.replace(/['"]/g, ''); - return cleaned.length >= 3 ? cleaned : null; + if (explicitPatternSeen || patternFileSeen) continue; + return token.length >= 3 ? token : null; } return null; } @@ -179,12 +380,6 @@ function extractPattern(toolName, toolInput) { if (t === 'shell') { const cmd = toolInput.command || ''; if (!/\brg\b|\bgrep\b/.test(cmd)) return null; - // NOTE: parseRgGrepPattern uses split(/\s+/) and cannot handle shell - // quoting. `rg "User Service" src/` returns "User" (the first token - // after the rg/grep arg, with surrounding quotes stripped) — the - // multi-word pattern is intentionally not reconstructed since BM25 is - // already token-tolerant. Quoted single tokens (`rg "validateUser"`) - // work fine. return parseRgGrepPattern(cmd); } @@ -282,4 +477,6 @@ function main() { } } -main(); +if (require.main === module) main(); + +module.exports = { parseRgGrepPattern, tokenizeShellWords }; diff --git a/gitnexus/test/unit/cursor-hook.test.ts b/gitnexus/test/unit/cursor-hook.test.ts index 54b5c1f84..dc9bc423b 100644 --- a/gitnexus/test/unit/cursor-hook.test.ts +++ b/gitnexus/test/unit/cursor-hook.test.ts @@ -19,6 +19,7 @@ */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawnSync } from 'child_process'; +import { createRequire } from 'module'; import fs from 'fs'; import path from 'path'; import os from 'os'; @@ -55,6 +56,12 @@ const CURSOR_HOOKS_JSON = path.resolve( 'hooks.json', ); +const require = createRequire(import.meta.url); +const { parseRgGrepPattern, tokenizeShellWords } = require(CURSOR_HOOK) as { + parseRgGrepPattern: (command: string) => string | null; + tokenizeShellWords: (command: string) => string[]; +}; + // ─── Cursor-specific output parser ────────────────────────────────── // Cursor postToolUse output shape: { "additional_context": "..." } @@ -549,37 +556,77 @@ describe('Cursor hook concurrency guard (integration)', () => { }); }); -// ─── Documented contract behavior (extractPattern via the live hook) ─ +// ─── Shell pattern parsing ────────────────────────────────────────── -describe('Shell quoted-pattern parser limitations (documented)', () => { - // The Shell parser cannot reconstruct shell quoting. These tests pin the - // current behavior so a future "fix" doesn't silently change extraction - // — and so users diagnosing a noisy/missed pattern can find the behavior - // documented in tests. - // - // We can't observe the extracted pattern directly without an indexed - // repo, but we *can* confirm the hook reaches the augment-call path - // (vs. early-exiting) by checking exit status + clean stdout for cases - // where parseRgGrepPattern would yield a >=3-char token. - - it('quoted multi-word `rg "User Service"` extracts the first word only', () => { - const result = runHook(CURSOR_HOOK, { - tool_name: 'Shell', - tool_input: { command: 'rg "User Service" src/' }, - cwd: tmpDir, // no .gitnexus → exits early after extract - }); - expect(result.status).toBe(0); - expect(result.stdout.trim()).toBe(''); +describe('Shell quoted-pattern parser', () => { + it.each([ + ['rg "User Service" src/', 'User Service'], + ["grep 'error boundary' -- src/", 'error boundary'], + ['rg User\\ Service src/', 'User Service'], + [String.raw`rg "C:\Users" src/`, String.raw`C:\Users`], + ['rg -e "User Service" src/', 'User Service'], + ['rg --regexp=UserService src/', 'UserService'], + ['grep -eUserService src/', 'UserService'], + ['rg -e x -e LongPattern src/', 'LongPattern'], + ['rg -ex -eLongPattern src/', 'LongPattern'], + ['rg --regexp=x --regexp=LongPattern src/', 'LongPattern'], + ['/usr/bin/rg -- "User Service" src/', 'User Service'], + ['rg -- -error src/', '-error'], + [String.raw`C:\Users\me\bin\rg.exe UserService src/`, 'UserService'], + ['rg.exe "validateUser" src/', 'validateUser'], + ['grep.cmd -e LongPattern src/', 'LongPattern'], + ['cd grep && rg LongPattern src/', 'LongPattern'], + ['npx rg "User Service" src/', 'User Service'], + ['npx --yes rg UserService src/', 'UserService'], + ['npx --package rg grep LongPattern src/', 'LongPattern'], + ['rg UserService; echo done', 'UserService'], + ['rg UserService&& echo done', 'UserService'], + ['rg --max-count 100 UserService src/', 'UserService'], + ['grep -r UserService src/', 'UserService'], + ['rg --replace x UserService src/', 'UserService'], + ['git grep UserService src/', 'UserService'], + ])('extracts %j from %j', (command, expected) => { + expect(parseRgGrepPattern(command)).toBe(expected); }); - it('single-token quoted `rg "validateUser"` works as expected', () => { - const result = runHook(CURSOR_HOOK, { - tool_name: 'Shell', - tool_input: { command: 'rg "validateUser"' }, - cwd: tmpDir, - }); - expect(result.status).toBe(0); - expect(result.stdout.trim()).toBe(''); + it.each([ + ['rg --regexp= src/'], + ['rg --regexp="" src/'], + ['rg -e x -- LongPattern src/'], + ['rg -f patterns.txt src/'], + ['rg --file=patterns.txt src/'], + ['rg -eab src/'], + ['sudo echo rg UserService src/'], + ])('extracts no pattern from %j', (command) => { + expect(parseRgGrepPattern(command)).toBeNull(); + }); + + it('does not treat a path after a short explicit pattern as the pattern', () => { + expect(parseRgGrepPattern('rg -e x src/')).toBeNull(); + }); + + it('keeps single-token quoted patterns intact', () => { + expect(parseRgGrepPattern('rg "validateUser"')).toBe('validateUser'); + }); + + it('keeps backslashes in unquoted Windows paths but honours escaped spaces', () => { + expect(tokenizeShellWords(String.raw`C:\foo\bar`)).toEqual([String.raw`C:\foo\bar`]); + expect(tokenizeShellWords('User\\ Service')).toEqual(['User Service']); + expect(tokenizeShellWords('trailing\\')).toEqual(['trailing\\']); + }); + + it('splits unquoted shell operators from adjacent arguments', () => { + expect(tokenizeShellWords('rg UserService; echo done')).toEqual([ + 'rg', + 'UserService', + ';', + 'echo', + 'done', + ]); + expect(tokenizeShellWords("rg 'UserService; echo done'")).toEqual([ + 'rg', + 'UserService; echo done', + ]); }); }); From 54f97c86c7d938e7a7bbf3652645561b66dc38cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 29 Aug 2026 12:58:15 +0100 Subject: [PATCH 4/4] fix(impact): make File risk comparable via shared axes (#3075) (#3082) * docs(plans): add impact file risk plan Capture the evidence, constraints, and verification path for fixing incomparable File and symbol impact risk. Co-authored-by: Cursor * refactor(impact): centralize risk scoring Keep the existing thresholds in one shared scorer and expose a common-axis comparison for targets with unavailable enrichment axes. Co-authored-by: Cursor * fix(impact): expose incomparable file risk scale Mark File impact results when process and module axes are unavailable, and provide a common-axis score for honest cross-kind comparisons. Co-authored-by: Cursor * fix(impact): explain cross-kind risk comparisons Surface the common-axis score in CLI and agent guidance while reusing the shared threshold ladder in the web impact tool. Co-authored-by: Cursor * fix(impact): fail closed when enrichment is incomplete Preserve proved HIGH/CRITICAL process counts, treat failed queries as UNKNOWN, and surface riskScale metadata on MCP, group, CLI, and Graph-RAG File walks. Co-authored-by: Cursor --------- Co-authored-by: Gergo Magyar Co-authored-by: Cursor --- .../skills/gitnexus-impact-analysis/SKILL.md | 9 + AGENTS.md | 6 +- CLAUDE.md | 6 +- ...26-08-28-gitnexus-plan-impact-file-risk.md | 312 +++++++++++++++++ .../skills/gitnexus-impact-analysis/SKILL.md | 9 + .../skills/gitnexus-impact-analysis/SKILL.md | 9 + gitnexus-shared/src/impact-risk.ts | 66 ++-- gitnexus-web/src/core/llm/tools.ts | 111 ++++-- gitnexus-web/test/unit/impact-tool.test.ts | 209 +++++++++++ gitnexus/skills/gitnexus-impact-analysis.md | 9 + gitnexus/src/cli/ai-context.ts | 8 +- gitnexus/src/cli/eval-server.ts | 30 +- gitnexus/src/core/group/cross-impact.ts | 36 +- gitnexus/src/core/group/types.ts | 14 +- gitnexus/src/mcp/local/local-backend.ts | 127 ++++--- gitnexus/src/mcp/tools.ts | 6 +- .../impact-file-risk-scale.test.ts | 143 ++++++++ .../impact-zero-caller-risk.test.ts | 7 + .../ai-context-unknown-risk-policy.test.ts | 3 + gitnexus/test/unit/ai-context.test.ts | 10 + .../test/unit/cli-impact-pdg-format.test.ts | 7 +- gitnexus/test/unit/eval-formatters.test.ts | 46 ++- .../group/cross-impact-fanout-cap.test.ts | 26 ++ gitnexus/test/unit/group/cross-impact.test.ts | 29 ++ gitnexus/test/unit/group/types.test.ts | 17 + .../unit/impact-batching-grouping.test.ts | 327 +++++++++++++++++- gitnexus/test/unit/impact-risk.test.ts | 237 +++++++++++++ .../test/unit/shipped-skills-sync.test.ts | 19 + gitnexus/test/unit/tools.test.ts | 11 + 29 files changed, 1725 insertions(+), 124 deletions(-) create mode 100644 docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md create mode 100644 gitnexus-web/test/unit/impact-tool.test.ts create mode 100644 gitnexus/test/integration/impact-file-risk-scale.test.ts create mode 100644 gitnexus/test/unit/impact-risk.test.ts diff --git a/.claude/skills/gitnexus-impact-analysis/SKILL.md b/.claude/skills/gitnexus-impact-analysis/SKILL.md index 4fb73f3e6..85d90c90d 100644 --- a/.claude/skills/gitnexus-impact-analysis/SKILL.md +++ b/.claude/skills/gitnexus-impact-analysis/SKILL.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/AGENTS.md b/AGENTS.md index c83a2e909..05be52af2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -119,10 +119,10 @@ This project is indexed by GitNexus as **GitNexus** (248612 symbols, 565510 rela - **MUST run impact analysis before editing.** Use `impact({target: "symbolName", direction: "upstream"})` (MCP) or `node .gitnexus/run.cjs impact "symbolName" --direction upstream --repo .` (CLI fallback); report callers, processes, and risk. Never substitute grep for graph analysis. For unified PDG impact, add `mode: "pdg"` with optional `line: ` — it returns statement-level `affectedStatements` over CDG + REACHING_DEF and inter-procedural symbols in `interproceduralByDepth`/`byDepth`; no-layer/degraded PDG results are UNKNOWN-risk notes (`--pdg` layer). CLI equivalent: `node .gitnexus/run.cjs impact "symbolName" --direction upstream --mode pdg --line --repo .`. - **MUST analyze graph changes before committing.** Use `detect_changes({scope: "all"})` (MCP) or `node .gitnexus/run.cjs detect-changes --scope all --repo .` (CLI fallback). `partial: true` or `truncated: true` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: `detect_changes({scope: "compare", base_ref: "main"})` or `node .gitnexus/run.cjs detect-changes --scope compare --base-ref "main" --repo .`. -- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- MUST warn on HIGH/CRITICAL `risk` pre-edit; never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat `risk: UNKNOWN` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). `impact` pairs `UNKNOWN` with a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- 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"})`. +- Explore with `query({search_query: "concept"})` for process-grouped flows. +- Use `context({name: "symbolName"})` for callers, callees, and flows. - For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). - 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. diff --git a/CLAUDE.md b/CLAUDE.md index 55b84d583..10681d819 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,10 +70,10 @@ This project is indexed by GitNexus as **GitNexus** (248612 symbols, 565510 rela - **MUST run impact analysis before editing.** Use `impact({target: "symbolName", direction: "upstream"})` (MCP) or `node .gitnexus/run.cjs impact "symbolName" --direction upstream --repo .` (CLI fallback); report callers, processes, and risk. Never substitute grep for graph analysis. For unified PDG impact, add `mode: "pdg"` with optional `line: ` — it returns statement-level `affectedStatements` over CDG + REACHING_DEF and inter-procedural symbols in `interproceduralByDepth`/`byDepth`; no-layer/degraded PDG results are UNKNOWN-risk notes (`--pdg` layer). CLI equivalent: `node .gitnexus/run.cjs impact "symbolName" --direction upstream --mode pdg --line --repo .`. - **MUST analyze graph changes before committing.** Use `detect_changes({scope: "all"})` (MCP) or `node .gitnexus/run.cjs detect-changes --scope all --repo .` (CLI fallback). `partial: true` or `truncated: true` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: `detect_changes({scope: "compare", base_ref: "main"})` or `node .gitnexus/run.cjs detect-changes --scope compare --base-ref "main" --repo .`. -- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- MUST warn on HIGH/CRITICAL `risk` pre-edit; never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat `risk: UNKNOWN` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). `impact` pairs `UNKNOWN` with a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- 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"})`. +- Explore with `query({search_query: "concept"})` for process-grouped flows. +- Use `context({name: "symbolName"})` for callers, callees, and flows. - For security review, `explain({target: "fileOrSymbol"})` lists taint findings (source→sink flows; needs `analyze --pdg`). - 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. diff --git a/docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md b/docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md new file mode 100644 index 000000000..7e14f2174 --- /dev/null +++ b/docs/plans/2026-08-28-gitnexus-plan-impact-file-risk.md @@ -0,0 +1,312 @@ +# GitNexus Engineering Plan + +> Task: Fix #3075 — File `impact` risk is not comparable to Function/Method risk. +> Evidence verified at commit `6bff33d14cbfe1e7b4f04bca51507e9f64ef579c` (`feat/kotlin-const-resolver`); GitNexus index 129 commits behind, refresh skipped: full-repo `--index-only --pdg` rebuild is impractical this session. Scorer and schema claims are `[verified]` from source; live inversion numbers are `[graph]` on the stale index. + +## 1. Objective + +Make File vs symbol `impact.risk` honest for consumers: either they can tell the scales differ, or they can compare on a shared two-axis score. Do **not** DEFINES-bridge processes/modules onto File targets (issue reporter sampled 8/10 one-importer files jumping to HIGH/CRITICAL). Do **not** retune Function HIGH/CRITICAL thresholds (agent warn-before-edit). + +Acceptance: + +- A File with a wider blast radius than a Function in the same file no longer looks “safer” when a consumer only reads `risk`, **or** the result states that `risk` is not comparable across kinds and offers `riskSharedAxes` for comparison. +- File targets still cannot trip HIGH/CRITICAL via `processes_affected` / `modules_affected` unless those axes become real in the index (they are not today). +- Existing Function/Method labels under the current four-axis ladder stay the same for the same inputs. +- MCP `riskNote` remains UNKNOWN-only (`tools.ts` contract). + +## 2. Current Behaviour + +Callgraph `impact` ends in `LocalBackend._runImpactBFS` (`gitnexus/src/mcp/local/local-backend.ts`). After BFS it enriches impacted ids with `STEP_IN_PROCESS` and `MEMBER_OF`, then scores: + +```7720:7738:gitnexus/src/mcp/local/local-backend.ts + } else if ( + directCount >= 30 || + processCount >= 5 || + moduleCount >= 5 || + impacted.length >= 200 + ) { + risk = 'CRITICAL'; + } else if ( + directCount >= 15 || + processCount >= 3 || + moduleCount >= 3 || + impacted.length >= 100 + ) { + risk = 'HIGH'; + } else if (directCount >= 5 || impacted.length >= 30) { + risk = 'MEDIUM'; + } else { + risk = 'LOW'; + } +``` + +Empty upstream → `UNKNOWN` + `riskNote`. Downstream empty stays LOW. `skipEnrichment` (ambiguous probes) already scores on direct+total only. PDG mode forces `risk: UNKNOWN` (`composeUnifiedPdgImpactResult`) — out of scope. + +File BFS walk is mostly File←IMPORTS File. Enrichment queries those File ids. Processes are CALLS traces (`process-processor.ts`); communities admit only Function/Class/Method/Interface (`isCommunitySymbol` in `community-processor.ts:412-416`). File is not in that set. `enrichCandidateLabels` UNION also **omits File**, so File `target.type` is often `""`; detect File via `id` prefix `File:`. + +Web Graph RAG (`gitnexus-web/src/core/llm/tools.ts` ~1331–1346) duplicates the same ladder. + +## 3. Relevant Architecture + +| Layer | Role | +|---|---| +| Index | File never sources `STEP_IN_PROCESS` / `MEMBER_OF` by construction | +| MCP `_runImpactBFS` | Blast radius + four-axis `risk` | +| Ambiguous probes | `skipEnrichment` → 2-axis `risk` already | +| `mergeRisk` | Group overlay; monotone in crossings; does not know target kind | +| CLI `formatImpactResult` | Prints counts; **does not print `risk`** on the resolved callgraph path; JSON `impactCommand` still ships `risk` | +| `ai-context.ts` / `tools.ts` | Agent contract: warn on HIGH/CRITICAL; `riskNote` UNKNOWN-only | +| Web LLM `impact` | Same formula, prose `RISK:` line | + +Modules: Local (MCP), Cli (format/docs), Group (`mergeRisk`), gitnexus-web LLM tools. Shared package `gitnexus-shared` is already a dependency of both CLI and web. + +## 4. GitNexus Findings + +- Primary: `_runImpactBFS` — d=1 `[graph]` `impact(target:_runImpactBFS, maxDepth:1, includeTests:true)`: `_impactImpl`, `impactByUid`. Production chain `[verified]`: `impact` → `_impactImpl` → `_runImpactBFS`; `impactByUid` skips per-symbol process lists but **not** aggregation (`skipPerSymbolEnrichment` only). +- `LocalBackend.impact` d=1 `[graph]` `context`: `callTool`. +- Duplicate scorer `[verified]` grep: `gitnexus-web/src/core/llm/tools.ts`. +- `mergeRisk` `[verified]` callers in `src/`: only `runGroupImpact` (`cross-impact.ts:907`). Graph d=1 listed a test File (`impact-pdg-shape.test.ts`) and missed `runGroupImpact` — trust source. +- Schema `[verified]`: `isCommunitySymbol` excludes File; `schema.ts` documents MEMBER_OF as Function/Class/Method/Interface only. +- Live inversion `[graph]` stale index, `impact summaryOnly` on GitNexus: + +| target | kind | impacted | direct | processes | modules | risk | +|---|---|---|---|---|---|---| +| `lbug-config.ts` | File | 54 | 12 | 0 | 0 | MEDIUM | +| `openLbugConnection` | Function | 16 | 9 | 3 | 2 | HIGH | +| `local-backend.ts` | File | 12 | 10 | 0 | 0 | MEDIUM | +| `refreshRepos` | Method | 50 | 5 | 4 | 7 | CRITICAL | + +- Clusters/processes resources `[graph]`: Local/Cli/Group sit in the impact path; process traces are function-stepped, not File-stepped. +- Related tests `[verified]`: `test/unit/impact-pagination.test.ts` (CRITICAL from `direct=400`); `test/integration/impact-zero-caller-risk.test.ts` (`withTestLbugDB` seed — pattern to extend); `test/unit/eval-formatters.test.ts` (`formatImpactResult`); group `mergeRisk` tests. + +## 5. Statement-Level PDG Findings + +PDG unavailable (`pdg_query` on `_runImpactBFS`: “no PDG layer”). Recommend `node .gitnexus/run.cjs analyze --index-only --pdg` before any future statement-slice work. Control flow of the scorer is a straight if/else after enrichment; no hidden guards. `skipEnrichment` is the only branch that structurally zeros process/module counts besides File ids. + +## 6. Proposed Changes + +### 6.1 Extract `scoreImpactRisk` — `gitnexus-shared/src/impact-risk.ts` (new) + +- **Responsibility:** Pure function: `{ direction, directCount, processCount, moduleCount, impactedCount, unusedAxes }` → `{ risk, riskSharedAxes, riskScale }`. +- **Behaviour:** Existing UNKNOWN/CRITICAL/HIGH/MEDIUM/LOW thresholds unchanged when `unusedAxes` is empty. `riskSharedAxes` always scores as if `processCount=0` and `moduleCount=0` (UNKNOWN rule still applies). `riskScale.comparableAcrossKinds` is false iff `unusedAxes` is non-empty. `riskScale.unusedAxes` lists `{ axis, reason }`. +- **Constraints:** Zero deps. Export from `gitnexus-shared/src/index.ts`. Do not put MCP types here. +- **File detection:** caller passes unused axes; helper does not parse UIDs. + +### 6.2 Wire MCP — `_runImpactBFS` in `local-backend.ts` + +- After computing `processCount`/`moduleCount`, set `unusedAxes`: + - target `id` starts with `File:` **or** `symType === 'File'` → processes + modules, reason `file-nodes-have-no-process-or-community-membership`; + - `skipEnrichment` → same axes, reason `enrichment-skipped` (ambiguous probes). +- Replace inline ladder with `scoreImpactRisk`. +- Spread `riskScale` and `riskSharedAxes` on the result next to `risk`. Do **not** set `riskNote` for File. +- Ambiguous candidate summaries: forward the new fields (probes already skip enrichment). +- `target.type` for File: if still `""`, prefer `'File'` when `id` starts with `File:` (display-only; helps CLI). + +### 6.3 Web duplicate — `gitnexus-web/src/core/llm/tools.ts` + +- Import `scoreImpactRisk` from `gitnexus-shared`. Print `RISK:` from `risk`; if `!comparableAcrossKinds`, one extra line: not comparable to Function risk; shared-axes label is `riskSharedAxes`. + +### 6.4 Agent/MCP contract copy + +- `gitnexus/src/mcp/tools.ts` impact description: document `riskScale` / `riskSharedAxes`; keep `riskNote` UNKNOWN-only; say File `risk` is not comparable to symbol `risk`. +- `gitnexus/src/cli/ai-context.ts`: HIGH/CRITICAL warning still applies; add: do not rank a File `MEDIUM` below a contained Function `HIGH` without `riskSharedAxes`. +- `formatImpactResult`: on resolved callgraph results with `risk`, print `Risk: {risk}` and, when incomparable, `Shared-axes risk: {riskSharedAxes} (File/process axes unused)`. + +### 6.5 Explicitly not changing + +- DEFINES-bridge, community/process indexers, `mergeRisk` formula, PDG `UNKNOWN`, `detectChanges` `risk_level`, Function thresholds. + +## 7. Implementation Sequence + +1. Add `gitnexus-shared` helper + unit table (issue-shaped inputs + UNKNOWN + skipEnrichment). Shared package tests if present; otherwise `gitnexus/test/unit/impact-risk.test.ts` importing the helper. +2. Switch `_runImpactBFS` + candidate probe payload. Tree still coherent: old `risk` values identical for Function fixtures. +3. Integration seed in `impact-zero-caller-risk.test.ts` **or** new `impact-file-risk-scale.test.ts`: File with ≥5 File IMPORTS (MEDIUM on direct) vs Function with 3 process-member callers (HIGH); assert File `riskScale.comparableAcrossKinds === false`, Function true, File `riskSharedAxes === risk`, Function `riskSharedAxes` is LOW/MEDIUM while `risk` is HIGH. +4. CLI formatter + `eval-formatters.test.ts`. +5. `tools.ts` + `ai-context.ts` wording. +6. Web import + a unit assertion on the printed RISK block if a test already covers that tool. +7. `npx tsc --noEmit` in `gitnexus/` and `gitnexus-web/`; `cd gitnexus && npm run test:unit -- test/unit/impact-risk.test.ts test/unit/eval-formatters.test.ts`; integration file from step 3. + +## 8. Test Strategy + +| File | Scenarios | +|---|---| +| `gitnexus/test/unit/impact-risk.test.ts` (new) | Issue table: File(25,13,0,0)→MEDIUM; Function(15,2,4,2)→HIGH; shared-axes File MEDIUM vs Function LOW; empty upstream UNKNOWN; downstream empty LOW; skipEnrichment unused axes; CRITICAL via direct≥30 still works with unused process axes | +| `gitnexus/test/integration/impact-file-risk-scale.test.ts` (new) | `withTestLbugDB` seed: `File:src/crypto.ts` ← 13 File IMPORTS, no File STEP_IN_PROCESS; `getEncryptionKey` with 2 CALLS from functions that have STEP_IN_PROCESS to 4 distinct Process nodes — reproduce inversion; assert new fields | +| `gitnexus/test/integration/impact-zero-caller-risk.test.ts` | Unchanged UNKNOWN/`riskNote`; candidates may grow `riskScale` — assert still present only when UNKNOWN for `riskNote` | +| `gitnexus/test/unit/impact-pagination.test.ts` | Hub CRITICAL unchanged | +| `gitnexus/test/unit/eval-formatters.test.ts` | Resolved result prints Risk + shared-axes line for File-shaped `riskScale` | +| Web | Only if an existing Graph RAG impact test snapshots `RISK:` | + +Commands (exist in `gitnexus/package.json`): `npm run test:unit`, `npm test` (full vitest), `npx tsc --noEmit`. Web: `npm test`, `npx tsc -b --noEmit`. Integration needs `pretest:integration` / `npm run test:integration` (runs `scripts/build.js`). + +## 9. Risk and Impact Analysis + +Direct dependents of `_runImpactBFS` `[graph]`: `_impactImpl`, `impactByUid`. `_impactImpl` is the only d=1 of `impact` besides the method’s own class. Any JSON consumer of `impact` (MCP, CLI `output(result)`, group local leg) sees additive fields — compatible if they ignore unknowns. + +- **HIGH workflow:** Function HIGH/CRITICAL unchanged. File still cannot reach HIGH via processes; a File with `direct≥15` or `total≥100` still can. Agents that compare File MEDIUM vs Function HIGH must start using `riskSharedAxes` or `riskScale`. +- **Ambiguous `maxRisk`:** probes skip enrichment, so File vs Function candidates are already 2-axis there — inversion is weaker on that path. +- **Group `mergeRisk`:** still compares incomparable File local `risk` to crossing count. Do not retune this PR; if a group File target is common, follow-up. +- **Web:** browser bundle picks up `gitnexus-shared` export — confirm `gitnexus-shared` build/exports include the new file. +- **Performance:** none (pure arithmetic after existing enrichment). +- **Ladybug empty labels:** File detection must not rely on `symType` alone. + +## 10. Files Expected to Change + +| File | Symbols | Reason | +|---|---|---| +| `gitnexus-shared/src/impact-risk.ts` | `scoreImpactRisk` | New shared scorer | +| `gitnexus-shared/src/index.ts` | exports | Public helper | +| `gitnexus/src/mcp/local/local-backend.ts` | `_runImpactBFS`, ambiguous candidate map | Wire scorer + File unused axes | +| `gitnexus/src/mcp/tools.ts` | `impact` description | Contract | +| `gitnexus/src/cli/ai-context.ts` | generated Always Do | Agent warning | +| `gitnexus/src/cli/eval-server.ts` | `formatImpactResult` | Print scale | +| `gitnexus-web/src/core/llm/tools.ts` | web `impact` | Same formula | +| `gitnexus/test/unit/impact-risk.test.ts` | — | Table tests | +| `gitnexus/test/integration/impact-file-risk-scale.test.ts` | — | Seeded inversion | +| `gitnexus/test/unit/eval-formatters.test.ts` | `formatImpactResult` | Formatter | + +## 11. Reusable Implementation Context + +```yaml +implementation_context: + task_summary: "Fix #3075: File impact.risk is a 2-axis score silently labelled on a 4-axis scale. Extract scoreImpactRisk; mark File/skipEnrichment axes unused; add riskScale + riskSharedAxes; do not DEFINES-bridge or retune Function thresholds." + acceptance_criteria: + - "File vs Function comparison is either labelled incomparable (riskScale) or done via riskSharedAxes" + - "Function/Method risk for identical four-axis inputs unchanged" + - "riskNote still UNKNOWN-only" + - "Integration seed reproduces crypto.ts-style inversion and asserts the new fields" + primary_symbols: + - symbol: "_runImpactBFS" + file: "gitnexus/src/mcp/local/local-backend.ts" + lines: "6991-7888" + role: "BFS + enrichment + inline risk ladder (replace ladder only)" + - symbol: "scoreImpactRisk" + file: "gitnexus-shared/src/impact-risk.ts" + lines: "new" + role: "Pure scorer + shared-axes + riskScale" + - symbol: "formatImpactResult" + file: "gitnexus/src/cli/eval-server.ts" + lines: "305-641" + role: "Human/LLM text surface for impact JSON" + related_symbols: + - symbol: "_impactImpl" + relationship: "CALLS" + relevance: "Resolves target, PDG vs callgraph, ambiguous skipEnrichment probes" + - symbol: "impactByUid" + relationship: "CALLS" + relevance: "Group fan-out; keep skipPerSymbolEnrichment; still run aggregation" + - symbol: "mergeRisk" + relationship: "consumes risk string" + relevance: "Do not change this PR" + - symbol: "isCommunitySymbol" + relationship: "index gate" + relevance: "Why File modules_affected is always 0" + - symbol: "composeUnifiedPdgImpactResult" + relationship: "separate path" + relevance: "PDG risk stays UNKNOWN" + execution_path: + - "impact / callTool → _impactImpl (resolve symbol, File id prefix File:)" + - "_runImpactBFS: IMPORTS-heavy walk for File; CALLS walk for Function" + - "Enrich STEP_IN_PROCESS / MEMBER_OF on impacted ids (empty for File ids)" + - "scoreImpactRisk with unusedAxes for File or skipEnrichment" + - "JSON to MCP/CLI; formatImpactResult for eval text; web LLM tools parallel path" + pdg_constraints: + - description: "No PDG layer on the planning index; scorer is post-enrichment arithmetic" + affected_statements: [] + implementation_consequence: "Do not wait on PDG; do not change pdg impact risk" + architectural_patterns: + - pattern: "Additive optional JSON fields on impact (riskNote, epistemic, partial)" + example_location: "gitnexus/src/mcp/local/local-backend.ts _runImpactBFS base object ~7754" + usage_guidance: "Add riskScale/riskSharedAxes the same way; never overload riskNote" + - pattern: "withTestLbugDB CREATE seed for impact contract" + example_location: "gitnexus/test/integration/impact-zero-caller-risk.test.ts" + usage_guidance: "Seed File IMPORTS + Function CALLS + Process membership separately" + files_to_modify: + - file: "gitnexus-shared/src/impact-risk.ts" + symbols: ["scoreImpactRisk"] + intended_change: "new pure scorer" + - file: "gitnexus-shared/src/index.ts" + symbols: [] + intended_change: "re-export" + - file: "gitnexus/src/mcp/local/local-backend.ts" + symbols: ["_runImpactBFS"] + intended_change: "unusedAxes + helper; File type display" + - file: "gitnexus/src/mcp/tools.ts" + symbols: [] + intended_change: "document fields" + - file: "gitnexus/src/cli/ai-context.ts" + symbols: [] + intended_change: "agent comparability note" + - file: "gitnexus/src/cli/eval-server.ts" + symbols: ["formatImpactResult"] + intended_change: "print risk + shared-axes when incomparable" + - file: "gitnexus-web/src/core/llm/tools.ts" + symbols: [] + intended_change: "import helper; extra prose line" + tests: + - file: "gitnexus/test/unit/impact-risk.test.ts" + scenarios: + - "File(25,13,0,0)+unused process/module → risk MEDIUM, comparableAcrossKinds false, riskSharedAxes MEDIUM" + - "Function(15,2,4,2) → HIGH, riskSharedAxes LOW (direct 2, total 15)" + - "upstream impactedCount 0 → UNKNOWN both fields" + - "direct 400 → CRITICAL even with unused process axes" + - file: "gitnexus/test/integration/impact-file-risk-scale.test.ts" + scenarios: + - "Seed File crypto.ts with 13 File importers vs getEncryptionKey with process-rich callers → inversion on risk, File incomparable, Function comparable" + - file: "gitnexus/test/unit/eval-formatters.test.ts" + scenarios: + - "formatImpactResult includes Shared-axes risk when riskScale.comparableAcrossKinds is false" + verification_commands: + - "cd gitnexus && npx tsc --noEmit" + - "cd gitnexus && npm run test:unit -- test/unit/impact-risk.test.ts test/unit/eval-formatters.test.ts test/unit/impact-pagination.test.ts" + - "cd gitnexus && npm run test:integration -- test/integration/impact-file-risk-scale.test.ts test/integration/impact-zero-caller-risk.test.ts" + - "cd gitnexus-web && npx tsc -b --noEmit" + risks: + - "Consumers that only read risk still see the inversion unless they adopt riskScale/riskSharedAxes — that is the chosen (explicit-scale) fix" + - "File type often empty; must key unusedAxes off File: id prefix" + - "gitnexus-shared export must reach the web bundle" + assumptions: + - "WHAT: File nodes never gain STEP_IN_PROCESS/MEMBER_OF without an indexer change. HOW: keep isCommunitySymbol and process traces as-is; tests seed File with zero such edges" + - "WHAT: Additive JSON fields are backward compatible. HOW: existing tests that exact-match the full impact object may need to allow extra keys — grep expect(res).toEqual on impact results before landing" + - "WHAT: HEAD 6bff33d is the pin; scorer line numbers ~7720. HOW: re-read the ladder if that hunk moved" + open_questions: + - "Whether GroupImpactResult should copy riskScale from local File targets (deferred unless tests already snapshot the full group object)" + avoid: + - "Do not DEFINES-bridge File→symbol processes/modules" + - "Do not lower Function process/module HIGH/CRITICAL thresholds" + - "Do not reuse riskNote for File incomparability" + - "Do not change PDG impact risk or detectChanges risk_level" + - "Do not treat labels(n)[0] or empty target.type as proof the node is not a File" + - "Do not repeat full repository discovery" +``` + +## 12. Assumptions and Open Questions + +**Assumptions** + +- Indexer will not start attaching File→Process/Community in this change (`isCommunitySymbol` stays). `[verified]` source; `[assumed]` future indexers. +- Ignoring unknown JSON keys is safe for MCP clients; any `toEqual` goldens in-repo must be updated. `[assumed]` — grep during implement. +- Stale-index inversion (`lbug-config.ts` vs `openLbugConnection`) is illustrative; the integration seed is the regression lock. `[graph]` vs `[verified]` seed. + +**Open questions** + +- Group `mergeRisk` + File local risk: copy `riskScale` onto `GroupImpactResult`? Default **no** unless a test breaks. +- Class/Interface STEP_IN_PROCESS sparsity: out of scope (#3075 is File). +- Printing `risk` on CLI formatted output is new (JSON already has it). Keep the extra lines short. + +**Deferred** + +- Recalibrated File-only HIGH thresholds. +- Indexing File community membership. +- DEFINES-bridge after a threshold RFC. +- Related #2975 (docs vs scorer wording) except as touched by `tools.ts`. + +## 13. Definition of Done + +- [ ] `scoreImpactRisk` is the only callgraph ladder in MCP and web. +- [ ] File (and skipEnrichment) results include `riskScale.comparableAcrossKinds === false` and `riskSharedAxes`. +- [ ] Function four-axis HIGH/CRITICAL cases in unit tests still pass with the same labels. +- [ ] Integration seed proves wider File blast + lower `risk` than a contained Function, and `riskSharedAxes` orders them without pretending processes existed on the File. +- [ ] `riskNote` still absent unless `risk === 'UNKNOWN'`. +- [ ] `tools.ts` + `ai-context.ts` state that File `risk` is not comparable to symbol `risk`. +- [ ] `cd gitnexus && npx tsc --noEmit` and the named unit/integration commands pass; web typecheck passes. diff --git a/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md index 4fb73f3e6..85d90c90d 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-impact-analysis/SKILL.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md b/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md index 4fb73f3e6..85d90c90d 100644 --- a/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md +++ b/gitnexus-cursor-integration/skills/gitnexus-impact-analysis/SKILL.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/gitnexus-shared/src/impact-risk.ts b/gitnexus-shared/src/impact-risk.ts index 6c18614f6..413d02f76 100644 --- a/gitnexus-shared/src/impact-risk.ts +++ b/gitnexus-shared/src/impact-risk.ts @@ -6,6 +6,7 @@ export type UnusedImpactRiskReason = | 'file-nodes-have-no-process-or-community-membership' | 'enrichment-skipped' | 'enrichment-budget-exhausted' + | 'enrichment-truncated' | 'enrichment-query-failed'; export interface UnusedImpactRiskAxis { @@ -50,7 +51,20 @@ function score( return 'LOW'; } -function countsWithUnusedAxesZeroed( +const UNMEASURED_REASONS: ReadonlySet = new Set([ + 'file-nodes-have-no-process-or-community-membership', + 'enrichment-skipped', + 'enrichment-budget-exhausted', +]); + +function unusedPair(reason: UnusedImpactRiskReason): UnusedImpactRiskAxis[] { + return [ + { axis: 'processes', reason }, + { axis: 'modules', reason }, + ]; +} + +function countsWithUnmeasuredAxesZeroed( input: ImpactRiskInput, ): Pick< ImpactRiskInput, @@ -59,6 +73,7 @@ function countsWithUnusedAxesZeroed( let processCount = input.processCount; let moduleCount = input.moduleCount; for (const unused of input.unusedAxes ?? []) { + if (!UNMEASURED_REASONS.has(unused.reason)) continue; if (unused.axis === 'processes') processCount = 0; if (unused.axis === 'modules') moduleCount = 0; } @@ -79,33 +94,23 @@ export function unusedAxesForImpactWalk(input: { processQueryFailed: boolean; moduleQueryFailed: boolean; /** When 0, a zero chunk budget is not an unused-axis event — there was nothing to enrich. */ - impactedCount?: number; + impactedCount: number; + /** True when process/module queries ran on a strict subset of impacted symbols. */ + enrichmentTruncated?: boolean; }): UnusedImpactRiskAxis[] { if (input.isFileTarget) { - return [ - { - axis: 'processes', - reason: 'file-nodes-have-no-process-or-community-membership', - }, - { - axis: 'modules', - reason: 'file-nodes-have-no-process-or-community-membership', - }, - ]; + return unusedPair('file-nodes-have-no-process-or-community-membership'); } if (input.skipEnrichment) { - return [ - { axis: 'processes', reason: 'enrichment-skipped' }, - { axis: 'modules', reason: 'enrichment-skipped' }, - ]; + return unusedPair('enrichment-skipped'); } - if (input.maxChunks === 0 && (input.impactedCount ?? 1) > 0) { - return [ - { axis: 'processes', reason: 'enrichment-budget-exhausted' }, - { axis: 'modules', reason: 'enrichment-budget-exhausted' }, - ]; + if (input.maxChunks === 0 && input.impactedCount > 0) { + return unusedPair('enrichment-budget-exhausted'); } const unused: UnusedImpactRiskAxis[] = []; + if (input.enrichmentTruncated) { + unused.push(...unusedPair('enrichment-truncated')); + } if (input.processQueryFailed) { unused.push({ axis: 'processes', reason: 'enrichment-query-failed' }); } @@ -115,11 +120,28 @@ export function unusedAxesForImpactWalk(input: { return unused; } +const INCOMPLETE_SAMPLE_REASONS: ReadonlySet = new Set([ + 'enrichment-query-failed', + 'enrichment-truncated', +]); + export function scoreImpactRisk(input: ImpactRiskInput): ImpactRiskResult { const unusedAxes = input.unusedAxes ?? []; + const observedRisk = score(countsWithUnmeasuredAxesZeroed(input)); + const incompleteSample = unusedAxes.some((unused) => + INCOMPLETE_SAMPLE_REASONS.has(unused.reason), + ); + // Failed queries and truncated samples make observed process/module counts + // lower bounds. Preserve any HIGH/CRITICAL warning already proved by those + // counts, but never emit a confident LOW/MEDIUM edit gate from an incomplete + // enrichment pass. + const risk = + incompleteSample && (observedRisk === 'LOW' || observedRisk === 'MEDIUM') + ? 'UNKNOWN' + : observedRisk; return { - risk: score(countsWithUnusedAxesZeroed(input)), + risk, riskSharedAxes: score({ ...input, processCount: 0, moduleCount: 0 }), riskScale: { comparableAcrossKinds: unusedAxes.length === 0, diff --git a/gitnexus-web/src/core/llm/tools.ts b/gitnexus-web/src/core/llm/tools.ts index a702e7db8..407018678 100644 --- a/gitnexus-web/src/core/llm/tools.ts +++ b/gitnexus-web/src/core/llm/tools.ts @@ -13,7 +13,7 @@ import { tool } from '@langchain/core/tools'; import { z } from 'zod'; -import { NODE_TABLES, REL_TYPES } from 'gitnexus-shared'; +import { NODE_TABLES, REL_TYPES, scoreImpactRisk, unusedAxesForImpactWalk } from 'gitnexus-shared'; import type { EnrichedSearchResult, GrepResult } from '../../services/backend-client'; /** @@ -1275,6 +1275,9 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, stepCount: number | null; }> = []; let affectedClusters: Array<{ label: string; hits: number; impact: string }> = []; + let processQueryFailed = false; + let clusterQueryFailed = false; + let clusterClassificationFailed = false; if (trimmedIds.length > 0) { const processQuery = ` @@ -1302,9 +1305,23 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, : ''; const [processRes, clusterRes, directClusterRes] = await Promise.all([ - executeQuery(processQuery), - executeQuery(clusterQuery), - directClusterQuery ? executeQuery(directClusterQuery) : Promise.resolve([]), + executeQuery(processQuery).catch((err) => { + processQueryFailed = true; + if (import.meta.env.DEV) console.warn('Impact process enrichment failed:', err); + return []; + }), + executeQuery(clusterQuery).catch((err) => { + clusterQueryFailed = true; + if (import.meta.env.DEV) console.warn('Impact cluster enrichment failed:', err); + return []; + }), + directClusterQuery + ? executeQuery(directClusterQuery).catch((err) => { + clusterClassificationFailed = true; + if (import.meta.env.DEV) console.warn('Impact cluster enrichment failed:', err); + return []; + }) + : Promise.resolve([]), ]); const directClusterSet = new Set(); @@ -1323,7 +1340,11 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, affectedClusters = clusterRes.map((row: any) => { const label = Array.isArray(row) ? row[0] : row.label; const hits = Array.isArray(row) ? row[1] : row.hits; - const impact = directClusterSet.has(label) ? 'direct' : 'indirect'; + const impact = clusterClassificationFailed + ? 'classification-unavailable' + : directClusterSet.has(label) + ? 'direct' + : 'indirect'; return { label, hits, impact }; }); } @@ -1331,19 +1352,25 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, const directCount = depth1.length; const processCount = affectedProcesses.length; const clusterCount = affectedClusters.length; - let risk = 'LOW'; - if (directCount >= 30 || processCount >= 5 || clusterCount >= 5 || totalAffected >= 200) { - risk = 'CRITICAL'; - } else if ( - directCount >= 15 || - processCount >= 3 || - clusterCount >= 3 || - totalAffected >= 100 - ) { - risk = 'HIGH'; - } else if (directCount >= 5 || totalAffected >= 30) { - risk = 'MEDIUM'; - } + const enrichmentCapped = allNodeIds.length > maxIdsForContext; + const unusedAxes = unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed, + moduleQueryFailed: clusterQueryFailed, + impactedCount: totalAffected, + enrichmentTruncated: enrichmentCapped, + }); + const scored = scoreImpactRisk({ + direction, + directCount, + processCount, + moduleCount: clusterCount, + impactedCount: totalAffected, + unusedAxes, + }); + const { risk, riskSharedAxes, riskScale } = scored; // ===== COMPACT TABULAR OUTPUT ===== const lines: string[] = [ @@ -1351,22 +1378,42 @@ MATCH (n:Function {id: emb.nodeId}) RETURN n`, `Confidence: High ${confidenceBuckets.high} | Medium ${confidenceBuckets.medium} | Low ${confidenceBuckets.low}`, ``, `AFFECTED PROCESSES:`, - ...(affectedProcesses.length > 0 - ? affectedProcesses.map( - (p) => - `- ${p.label} - BROKEN at step ${p.minStep ?? '?'} (${p.hits} symbols, ${p.stepCount ?? '?'} steps)`, - ) - : ['- None found']), + ...(processQueryFailed + ? ['- Unavailable (enrichment query failed)'] + : affectedProcesses.length > 0 + ? affectedProcesses.map( + (p) => + `- ${p.label} - BROKEN at step ${p.minStep ?? '?'} (${p.hits} symbols, ${p.stepCount ?? '?'} steps)`, + ) + : ['- None found']), ``, `AFFECTED CLUSTERS:`, - ...(affectedClusters.length > 0 - ? affectedClusters.map((c) => `- ${c.label} (${c.impact}, ${c.hits} symbols)`) - : ['- None found']), + ...(clusterQueryFailed + ? ['- Unavailable (enrichment query failed)'] + : affectedClusters.length > 0 + ? affectedClusters.map((c) => `- ${c.label} (${c.impact}, ${c.hits} symbols)`) + : ['- None found']), ``, - `RISK: ${risk}`, + `RISK: ${risk} (edit gate — warn on HIGH/CRITICAL)`, + `Shared-axes: ${riskSharedAxes} (File vs symbol compare only; do not waive a HIGH risk warning)`, + `Note: this Graph-RAG surface expands File targets to in-file symbols before enrichment, so process/cluster axes are comparable here when enrichment succeeds. MCP File impact does not.`, + ...(riskScale.comparableAcrossKinds + ? [] + : [ + `Note: process/module axes were unused (${riskScale.unusedAxes.map((a) => a.reason).join(', ')}).`, + ]), + ...(risk === 'UNKNOWN' && (processQueryFailed || clusterQueryFailed) + ? ['Note: risk is unresolved because enrichment failed; retry before editing.'] + : []), + ...(enrichmentCapped + ? [`Note: process/cluster enrichment is partial (first ${maxIdsForContext} symbols).`] + : []), + ...(clusterClassificationFailed + ? ['Note: direct/indirect cluster classification is unavailable.'] + : []), `- Direct callers: ${directCount}`, - `- Processes affected: ${processCount}`, - `- Clusters affected: ${clusterCount}`, + `- Processes affected: ${processQueryFailed ? 'unavailable' : processCount}`, + `- Clusters affected: ${clusterQueryFailed ? 'unavailable' : clusterCount}`, ``, ]; @@ -1472,7 +1519,9 @@ relationTypes filter (optional): Additional output sections: - Affected processes (with step impact) - Affected clusters (direct/indirect) -- Risk summary (based on direct callers, processes, clusters)`, +- RISK is the edit gate: warn before edits on HIGH/CRITICAL; UNKNOWN requires retry or corroboration +- Shared-axes risk compares File and symbol targets using direct/total counts only; it never waives the RISK gate +- riskScale notes unavailable process/module axes. This Graph-RAG tool expands File targets to in-file symbols; MCP File impact does not`, schema: z.object({ target: z.string().describe('Name of the function, class, or file to analyze'), direction: z diff --git a/gitnexus-web/test/unit/impact-tool.test.ts b/gitnexus-web/test/unit/impact-tool.test.ts new file mode 100644 index 000000000..f04817ed1 --- /dev/null +++ b/gitnexus-web/test/unit/impact-tool.test.ts @@ -0,0 +1,209 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createGraphRAGTools, type GraphRAGBackend } from '../../src/core/llm/tools'; + +const noOpBackend: GraphRAGBackend = { + executeQuery: async () => [], + search: async () => [], + grep: async () => [], + readFile: async () => '', +}; + +function impactTool(backend: GraphRAGBackend) { + return createGraphRAGTools(backend).find((candidate) => candidate.name === 'impact')!; +} + +describe('Graph-RAG impact risk contract', () => { + it('advertises the edit gate, shared axes, and MCP File difference', () => { + const description = impactTool(noOpBackend).description; + expect(description).toContain('RISK is the edit gate'); + expect(description).toContain('Shared-axes risk'); + expect(description).toContain('riskScale'); + expect(description).toContain('MCP File impact does not'); + }); + + it('renders failed enrichment as unavailable and fails the risk gate closed', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + if (query.includes('MATCH (affected)-[r:CodeRelation]->(target)')) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + startLine: 4, + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) throw new Error('process query failed'); + if (query.includes('MEMBER_OF')) return []; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('AFFECTED PROCESSES:\n- Unavailable (enrichment query failed)'); + expect(output).not.toContain('AFFECTED PROCESSES:\n- None found'); + expect(output).toContain('RISK: UNKNOWN'); + expect(output).toContain('risk is unresolved because enrichment failed'); + expect(output).toContain('- Processes affected: unavailable'); + }); + + it('preserves proved CRITICAL risk when the cluster query fails', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + if (query.includes('MATCH (affected)-[r:CodeRelation]->(target)')) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) { + return Array.from({ length: 5 }, (_, index) => ({ + label: `process-${index}`, + hits: 1, + minStep: index + 1, + stepCount: 5, + })); + } + if (query.includes('MEMBER_OF') && query.includes('COUNT(DISTINCT s.id)')) { + throw new Error('cluster query failed'); + } + if (query.includes('MEMBER_OF')) return []; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('RISK: CRITICAL'); + expect(output).toContain('AFFECTED CLUSTERS:\n- Unavailable (enrichment query failed)'); + expect(output).toContain('- Processes affected: 5'); + expect(output).toContain('- Clusters affected: unavailable'); + }); + + it('does not invent direct/indirect cluster classification after its query fails', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + if (query.includes('MATCH (affected)-[r:CodeRelation]->(target)')) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) return []; + if (query.includes('MEMBER_OF') && query.includes('RETURN DISTINCT')) { + throw new Error('classification query failed'); + } + if (query.includes('MEMBER_OF')) return [{ label: 'Core', hits: 1 }]; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('- Core (classification-unavailable, 1 symbols)'); + expect(output).toContain('direct/indirect cluster classification is unavailable'); + expect(output).not.toContain('process/module axes were unused'); + }); + + it('treats successful File expansion as comparable because enrichment runs on member symbols', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("n.filePath CONTAINS 'src/target.ts'")) { + return [{ id: 'file-id', nodeType: 'File', filePath: 'src/target.ts' }]; + } + if (query.includes("callee.filePath = 'src/target.ts'")) { + return [ + { + id: 'caller-id', + name: 'caller', + nodeType: 'Function', + filePath: 'src/caller.ts', + edgeType: 'CALLS', + confidence: 1, + }, + ]; + } + if (query.includes('STEP_IN_PROCESS')) { + return [{ label: 'Build', hits: 1, minStep: 1, stepCount: 1 }]; + } + if (query.includes('MEMBER_OF') && query.includes('RETURN DISTINCT')) { + return [{ label: 'Core' }]; + } + if (query.includes('MEMBER_OF')) return [{ label: 'Core', hits: 1 }]; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'src/target.ts', + direction: 'upstream', + maxDepth: 1, + }); + + expect(output).toContain('process/cluster axes are comparable here when enrichment succeeds'); + expect(output).toContain('- Processes affected: 1'); + expect(output).toContain('- Clusters affected: 1'); + expect(output).not.toContain('process/module axes were unused'); + }); + + it('surfaces the 500-symbol enrichment cap as partial', async () => { + const executeQuery = vi.fn(async (query: string) => { + if (query.includes("WHERE n.name = 'target'")) { + return [{ id: 'target-id', nodeType: 'Function', filePath: 'src/target.ts' }]; + } + const depth = query.includes('3 AS depth') ? 3 : query.includes('2 AS depth') ? 2 : 1; + if (query.includes('CodeRelation') && query.includes(` ${depth} AS depth`)) { + return Array.from({ length: 200 }, (_, index) => ({ + id: `d${depth}-${index}`, + name: `node-${depth}-${index}`, + nodeType: 'Function', + filePath: `src/d${depth}-${index}.ts`, + edgeType: 'CALLS', + confidence: 1, + })); + } + if (query.includes('STEP_IN_PROCESS') || query.includes('MEMBER_OF')) return []; + return []; + }); + + const output = await impactTool({ ...noOpBackend, executeQuery }).invoke({ + target: 'target', + direction: 'upstream', + maxDepth: 3, + }); + + expect(output).toContain('process/cluster enrichment is partial (first 500 symbols)'); + expect(output).toContain('enrichment-truncated'); + expect(output).not.toContain('enrichment-budget-exhausted'); + }); +}); diff --git a/gitnexus/skills/gitnexus-impact-analysis.md b/gitnexus/skills/gitnexus-impact-analysis.md index 4fb73f3e6..85d90c90d 100644 --- a/gitnexus/skills/gitnexus-impact-analysis.md +++ b/gitnexus/skills/gitnexus-impact-analysis.md @@ -93,6 +93,15 @@ dispatch, cross-language calls), so few-callers ⇒ LOW does **not** apply. The result carries a `riskNote` saying so. Confirm with a text search before treating the symbol as safe to change or delete. +`risk` is the edit gate: warn on HIGH/CRITICAL and stop on UNKNOWN until the +uncertainty is resolved. Within single-repo mode, compare File and symbol +targets with local `riskSharedAxes` (direct/total only). Within group mode, +compare only group results: their `riskSharedAxes` overlays resolved +cross-repo crossings on that local value. Never use either field to waive the +edit gate. Check `riskScale.unusedAxes` before comparing kinds: MCP File walks +omit process/module axes, while web Graph-RAG expands File targets to in-file +symbols before enrichment. + ## Tools **impact** — the primary tool for symbol blast radius. If MCP is unavailable, use `node .gitnexus/run.cjs impact --direction upstream --repo .` instead: diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 258aeb46c..cb7b42c60 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -218,16 +218,16 @@ This project is indexed by GitNexus as **${projectName}**${noStats ? '' : ` (${s ## Always Do -- **MUST run impact analysis before editing.** Use \`impact({target: "symbolName", direction: "upstream"})\` (MCP) or \`${runner} impact "symbolName" --direction upstream --repo .\` (CLI fallback); report callers, processes, and risk. Never substitute grep for graph analysis.${ +- **MUST run impact before editing.** Use \`impact({target: "symbolName", direction: "upstream"})\` or \`${runner} impact "symbolName" --direction upstream --repo .\`; report callers, processes, and risk. Never substitute grep for graph analysis.${ hasPdg ? ` For unified PDG impact, add \`mode: "pdg"\` with optional \`line: \` — it returns statement-level \`affectedStatements\` over CDG + REACHING_DEF and inter-procedural symbols in \`interproceduralByDepth\`/\`byDepth\`; no-layer/degraded PDG results are UNKNOWN-risk notes (\`--pdg\` layer). CLI equivalent: \`${runner} impact "symbolName" --direction upstream --mode pdg --line --repo .\`.` : '' } - **MUST analyze graph changes before committing.** Use \`detect_changes({scope: "all"})\` (MCP) or \`${runner} detect-changes --scope all --repo .\` (CLI fallback). \`partial: true\` or \`truncated: true\` is not a clean check — a zero means unseen, not unaffected; re-run it. For regression review: \`detect_changes({scope: "compare", base_ref: ${JSON.stringify(markdownSafeBranch(defaultBranch))}})\` or \`${runner} detect-changes --scope compare --base-ref ${JSON.stringify(markdownSafeBranch(defaultBranch))} --repo .\`. -- **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. +- MUST warn on HIGH/CRITICAL \`risk\` pre-edit; never use \`riskSharedAxes\` to waive a HIGH/CRITICAL \`risk\` warning. Compare File/symbol: MCP File omits axes; Graph-RAG expands File. - **MUST treat \`risk: UNKNOWN\` as unresolved, not as low.** An empty caller set is not evidence the symbol is unused — it can also mean the callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls). \`impact\` pairs \`UNKNOWN\` with a \`riskNote\` saying so. Confirm with a text search before treating the symbol as safe to change or delete; do not proceed on the strength of a zero. -- 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"})\`. +- Explore with \`query({search_query: "concept"})\` for process-grouped flows. +- Use \`context({name: "symbolName"})\` for callers, callees, and flows. - 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.` diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index caf19bc03..b2f656c39 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -302,6 +302,20 @@ function formatTruncationSuffix(result: { return label ? ` (by ${label})` : ''; } +function pushCallgraphRiskLines(lines: string[], result: any): void { + if (result.risk) { + lines.push(`Risk: ${result.risk}`); + } + if (result.riskNote) { + lines.push(String(result.riskNote)); + } + if (result.riskScale?.comparableAcrossKinds === false && result.riskSharedAxes) { + lines.push( + `Shared-axes risk: ${result.riskSharedAxes} (process/module axes are unavailable — compare File vs symbol only; do not use this to waive a HIGH/CRITICAL risk warning)`, + ); + } +} + export function formatImpactResult(result: any): string { if (result.error) { const suggestion = result.suggestion ? `\nSuggestion: ${result.suggestion}` : ''; @@ -567,14 +581,21 @@ export function formatImpactResult(result: any): string { // #1858 — "isolated" is a confident claim. If an interface / indirection // boundary is on the path, the true count is a lower bound, not zero; // callers binding via DI / dynamic dispatch were not traced. Say so instead. + const lines: string[] = []; if (result.epistemic === 'lower-bound') { - const lines = [ + lines.push( `${target?.name || '?'}: no direct ${direction} dependencies traced, but this is a LOWER BOUND — unresolved indirection on the path (actual impact may be higher):`, - ]; + ); for (const b of result.boundaries || []) lines.push(` • ${b}`); - return lines.join('\n'); + } else if (direction === 'upstream') { + lines.push( + `${target?.name || '?'}: No ${direction} callers resolved. This is not evidence the symbol is unused or isolated.`, + ); + } else { + lines.push(`${target?.name || '?'}: No ${direction} dependencies found.`); } - return `${target?.name || '?'}: No ${direction} dependencies found. This symbol appears isolated.`; + pushCallgraphRiskLines(lines, result); + return lines.join('\n'); } const lines: string[] = []; @@ -594,6 +615,7 @@ export function formatImpactResult(result: any): string { ); for (const b of result.boundaries || []) lines.push(` • ${b}`); } + pushCallgraphRiskLines(lines, result); lines.push(''); const depthLabels: Record = { diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts index 19ddd6fee..485b1ee8c 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -5,6 +5,7 @@ import fsp from 'node:fs/promises'; import path from 'node:path'; +import type { ImpactRisk } from 'gitnexus-shared'; import type { BridgeHandle, BridgeMeta, @@ -381,7 +382,17 @@ function extractProcessNames(impact: unknown): string[] { // permanently that a PDG `risk:'UNKNOWN'` never coalesces to a confident `LOW`. // No behavior change — `'UNKNOWN'` was already handled correctly at the // `(localRisk === 'LOW' || localRisk === 'UNKNOWN')` branch below. -export function mergeRisk(localRisk: string, cross: CrossRepoImpact[]): string { +function asImpactRisk(value: unknown, fallback: ImpactRisk = 'LOW'): ImpactRisk { + return value === 'LOW' || + value === 'MEDIUM' || + value === 'HIGH' || + value === 'CRITICAL' || + value === 'UNKNOWN' + ? value + : fallback; +} + +export function mergeRisk(localRisk: ImpactRisk, cross: CrossRepoImpact[]): ImpactRisk { const traversed = cross.filter((c) => c.fanout_status !== 'not_attempted'); const highConf = traversed.some((c) => c.contract.confidence >= 0.85); if (localRisk === 'CRITICAL') return 'CRITICAL'; @@ -391,6 +402,22 @@ export function mergeRisk(localRisk: string, cross: CrossRepoImpact[]): string { return localRisk; } +function liftLocalRiskMeta( + local: unknown, + cross: CrossRepoImpact[], +): Pick { + const { riskSharedAxes, riskScale } = local as { + riskSharedAxes?: unknown; + riskScale?: GroupImpactResult['riskScale']; + }; + return { + ...(riskSharedAxes !== undefined + ? { riskSharedAxes: mergeRisk(asImpactRisk(riskSharedAxes), cross) } + : {}), + ...(riskScale !== undefined ? { riskScale } : {}), + }; +} + /** * Is this bridge's metadata unable to say where its contents came from? * @@ -601,6 +628,7 @@ export async function runGroupImpact( cross_repo_hits: 0, }, risk: 'UNKNOWN', + ...liftLocalRiskMeta(local, []), timeoutMs, crossDepthWarning, }; @@ -656,7 +684,8 @@ export async function runGroupImpact( modules_affected: s.modules_affected ?? 0, cross_repo_hits: 0, }, - risk: String((local as { risk?: string }).risk ?? 'LOW'), + risk: asImpactRisk((local as { risk?: unknown }).risk), + ...liftLocalRiskMeta(local, []), timeoutMs, crossDepthWarning, }; @@ -826,7 +855,7 @@ export async function runGroupImpact( } const localSum = (local as { summary?: Record })?.summary || {}; - const localRisk = String((local as { risk?: string }).risk ?? 'LOW'); + const localRisk = asImpactRisk((local as { risk?: unknown }).risk); const localPartial = Boolean((local as { partial?: boolean }).partial); // The bridge's own incompleteness, in the shared vocabulary, read through // what this query DECLARED. The fan-out above already drops every neighbour @@ -905,6 +934,7 @@ export async function runGroupImpact( cross_repo_hits: cross.length, }, risk: mergeRisk(localRisk, cross), + ...liftLocalRiskMeta(local, cross), timeoutMs, crossDepthWarning, }; diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index beeb6f053..023629506 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -1,3 +1,5 @@ +import type { ImpactRisk, ImpactRiskResult } from 'gitnexus-shared'; + export type ContractType = | 'http' | 'graphql' @@ -194,7 +196,17 @@ export interface GroupImpactResult { modules_affected: number; cross_repo_hits: number; }; - risk: string; + risk: ImpactRisk; + /** + * Two-axis (direct + total) risk from the local leg, then `mergeRisk` with + * crossings — compare File vs symbol here, not via top-level `risk`. + */ + riskSharedAxes?: ImpactRisk; + /** + * Local-leg scale metadata (File / skipped enrichment). Crossings do not + * invent process/module membership for File nodes. + */ + riskScale?: ImpactRiskResult['riskScale']; /** * `'lower-bound'` when the fan-out was cut short, so `risk` is a FLOOR, not a * verdict. Same vocabulary as single-repo `impact`'s `epistemic` field. diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 6451b0d9b..544f64c12 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -9,6 +9,7 @@ import fs from 'fs/promises'; import path from 'path'; import { createHash } from 'crypto'; +import { scoreImpactRisk, unusedAxesForImpactWalk, type ImpactRiskResult } from 'gitnexus-shared'; import { initLbug, executeQuery, @@ -6412,6 +6413,8 @@ export class LocalBackend { let summary: { impactedCount: number; risk: string; + riskSharedAxes?: string; + riskScale?: ImpactRiskResult['riskScale']; riskNote?: string; summary?: { direct: number }; } | null = null; @@ -6451,6 +6454,10 @@ export class LocalBackend { score: Number(c.score.toFixed(2)), impactedCount: summary?.impactedCount ?? 0, risk: summary?.risk ?? 'UNKNOWN', + ...(summary?.riskSharedAxes !== undefined + ? { riskSharedAxes: summary.riskSharedAxes } + : {}), + ...(summary?.riskScale !== undefined ? { riskScale: summary.riskScale } : {}), direct: summary?.summary?.direct ?? 0, ...(summary?.riskNote !== undefined ? { riskNote: summary.riskNote } : {}), // Carry the explanation with the verdict. The single-symbol path @@ -7504,6 +7511,10 @@ export class LocalBackend { const parsedMaxChunks = rawMaxChunks ? Number(rawMaxChunks) : Number.NaN; const MAX_CHUNKS = Number.isInteger(parsedMaxChunks) && parsedMaxChunks >= 0 ? parsedMaxChunks : 10; + let processQueryFailed = false; + let moduleQueryFailed = false; + let enrichmentDegraded = false; + let moduleClassificationFailed = false; // `skipEnrichment` (ambiguous #2129 per-candidate probes) bypasses the // process/module aggregation passes entirely — those probes need only the @@ -7554,7 +7565,12 @@ export class LocalBackend { ORDER BY pId `, { ids }, - ).catch(() => []); + ).catch((err) => { + processQueryFailed = true; + enrichmentDegraded = true; + logQueryError('impact:process-chunk', err); + return []; + }); for (const row of rows) { const pId = row.pId ?? row[0]; @@ -7605,6 +7621,8 @@ export class LocalBackend { ep.earliest_broken_step = Math.min(ep.earliest_broken_step, minStep ?? Infinity); } } catch (e) { + processQueryFailed = true; + enrichmentDegraded = true; logQueryError('impact:process-chunk', e); } } @@ -7624,7 +7642,11 @@ export class LocalBackend { RETURN p.id AS pid, MIN(r.step) AS minStep `, { pIds, ids: allImpactedIds }, - ).catch(() => []); + ).catch((err) => { + enrichmentDegraded = true; + logQueryError('impact:process-chunk-backfill', err); + return []; + }); for (const mr of missingRows) { const pid = mr.pid ?? mr[0]; @@ -7638,6 +7660,7 @@ export class LocalBackend { } } } catch (e) { + enrichmentDegraded = true; logQueryError('impact:process-chunk-backfill', e); } } @@ -7698,7 +7721,12 @@ export class LocalBackend { LIMIT 20 `, { ids: idsChunk }, - ).catch(() => []); + ).catch((err) => { + moduleQueryFailed = true; + enrichmentDegraded = true; + logQueryError('impact:module-chunk', err); + return []; + }); for (const r of rows) { const name = r.name ?? r[0] ?? null; @@ -7707,6 +7735,8 @@ export class LocalBackend { moduleHitsMap.set(name, (moduleHitsMap.get(name) || 0) + hits); } } catch (e) { + moduleQueryFailed = true; + enrichmentDegraded = true; logQueryError('impact:module-chunk', e); } }; @@ -7732,12 +7762,19 @@ export class LocalBackend { RETURN DISTINCT c.heuristicLabel AS name `, { ids: idsChunk }, - ).catch(() => []); + ).catch((err) => { + enrichmentDegraded = true; + moduleClassificationFailed = true; + logQueryError('impact:direct-module-chunk', err); + return []; + }); for (const r of rows) { const name = r.name ?? r[0] ?? null; if (name) directModuleSet.add(name); } } catch (e) { + enrichmentDegraded = true; + moduleClassificationFailed = true; logQueryError('impact:direct-module-chunk', e); } }; @@ -7762,7 +7799,11 @@ export class LocalBackend { return { name, hits, - impact: directModuleNameSet.has(name) ? 'direct' : 'indirect', + impact: moduleClassificationFailed + ? 'classification-unavailable' + : directModuleNameSet.has(name) + ? 'direct' + : 'indirect', }; }); } @@ -7770,40 +7811,25 @@ export class LocalBackend { // Risk scoring const processCount = affectedProcesses.length; const moduleCount = affectedModules.length; - let risk: string; - if (direction === 'upstream' && impacted.length === 0) { - // An upstream walk that resolved NO callers cannot support `LOW`. "Safe - // to change" is a claim ABOUT callers, and this walk found none to reason - // about: the symbol may be genuinely unused, or reached only through a - // reference class this index does not record — a property access on a - // plain object, or a bare-identifier read of a module-scope `Const`, - // neither of which mints a reference site today. Seeding `LOW` from an - // empty result is the same false-safe signal `anyKnownRisk` refuses to - // emit on the ambiguous-candidate path, and that #2687 removed by making - // an undetermined `impactedCount` `null` instead of `0`. - // - // Downstream is deliberately untouched: an empty downstream walk reports - // that this symbol resolved no callees, which is not a safety verdict. - risk = 'UNKNOWN'; - } else if ( - directCount >= 30 || - processCount >= 5 || - moduleCount >= 5 || - impacted.length >= 200 - ) { - risk = 'CRITICAL'; - } else if ( - directCount >= 15 || - processCount >= 3 || - moduleCount >= 3 || - impacted.length >= 100 - ) { - risk = 'HIGH'; - } else if (directCount >= 5 || impacted.length >= 30) { - risk = 'MEDIUM'; - } else { - risk = 'LOW'; - } + const isFileTarget = symType === 'File' || String(symId).startsWith('File:'); + const unusedAxes = unusedAxesForImpactWalk({ + isFileTarget, + skipEnrichment, + maxChunks: MAX_CHUNKS, + processQueryFailed, + moduleQueryFailed, + impactedCount: impacted.length, + enrichmentTruncated: + !skipEnrichment && MAX_CHUNKS > 0 && impacted.length > MAX_CHUNKS * CHUNK_SIZE, + }); + const { risk, riskSharedAxes, riskScale } = scoreImpactRisk({ + direction, + directCount, + processCount, + moduleCount, + impactedCount: impacted.length, + unusedAxes, + }); // Build per-depth counts (always included, even in summaryOnly mode) const byDepthCounts: Record = {}; @@ -7823,7 +7849,7 @@ export class LocalBackend { target: { id: symId, name: sym.name || sym[1], - type: symType, + type: isFileTarget ? symType || 'File' : symType, filePath: sym.filePath || sym[2], ...(beanMetadata ? { bean: beanMetadata } : {}), ...(aopMetadata ? { aop: aopMetadata } : {}), @@ -7831,18 +7857,27 @@ export class LocalBackend { direction, impactedCount: impacted.length, risk, + riskSharedAxes, + riskScale, ...(risk === 'UNKNOWN' ? { riskNote: - 'No callers resolved. Absence of edges is not evidence the symbol is unused: ' + - 'a caller reaching it through a reference class this index does not record — ' + - 'plain-object property access, a bare-identifier read of a module-scope const — ' + - 'produces no edge to find. Confirm with a text search before treating the ' + - 'change as safe.', + processQueryFailed || moduleQueryFailed + ? 'Risk is unresolved because process/module enrichment failed. Observed counts ' + + 'are lower bounds; retry impact before treating the change as safe.' + : unusedAxes.some((axis) => axis.reason === 'enrichment-truncated') + ? 'Risk is unresolved because process/module enrichment was truncated. Observed ' + + 'counts are lower bounds; retry with a higher IMPACT_MAX_CHUNKS before ' + + 'treating the change as safe.' + : 'No callers resolved. Absence of edges is not evidence the symbol is unused: ' + + 'a caller reaching it through a reference class this index does not record — ' + + 'plain-object property access, a bare-identifier read of a module-scope const — ' + + 'produces no edge to find. Confirm with a text search before treating the ' + + 'change as safe.', } : {}), ...epistemic, - ...(!traversalComplete && { partial: true }), + ...((!traversalComplete || enrichmentDegraded) && { partial: true }), summary: { direct: directCount, processes_affected: processCount, diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 53700a7a2..4244a95bf 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -475,11 +475,13 @@ WHEN TO USE: Before making code changes — especially refactoring, renaming, or AFTER THIS: Review d=1 items (WILL BREAK). Use context() on high-risk symbols. Output includes: -- risk: LOW / MEDIUM / HIGH / CRITICAL / UNKNOWN. An upstream walk that resolved ZERO callers reports UNKNOWN, never LOW, and carries riskNote: "safe to change" is a claim about callers and there were none to reason about, so the symbol is either genuinely unused OR reached only through a reference class the index does not record (plain-object property access, a bare-identifier read of a module-scope const). Confirm with a text search before acting on it. Downstream walks are unaffected — an empty downstream result reports resolved callees, not safety. +- risk: LOW / MEDIUM / HIGH / CRITICAL / UNKNOWN. This is the HIGH/CRITICAL edit-gate field. File targets lack process/community membership, so their risk is not directly comparable with symbol risk; use riskSharedAxes to compare the direct/total axes common to both. Group-mode (\`repo: "@…"\`) results lift the same fields to the top-level envelope. The web Graph-RAG impact tool expands File targets to in-file symbols before enrichment, so process/cluster axes remain comparable there. An upstream walk that resolved ZERO callers reports UNKNOWN, never LOW, and carries riskNote: "safe to change" is a claim about callers and there were none to reason about, so the symbol is either genuinely unused OR reached only through a reference class the index does not record (plain-object property access, a bare-identifier read of a module-scope const). Confirm with a text search before acting on it. Downstream walks are unaffected — an empty downstream result reports resolved callees, not safety. +- riskSharedAxes: single-repo risk computed only from direct and total impact. Group mode then applies the cross-repo crossing overlay to that local value. Suitable for comparing File and symbol targets within the same mode. Never substitute it for \`risk\` when deciding whether to warn before edits. +- riskScale: { comparableAcrossKinds, unusedAxes } — names process/module axes that were structurally unavailable, skipped, budget-exhausted (\`IMPACT_MAX_CHUNKS=0\`), truncated (sampled a subset of impacted symbols), or failed at query time. Failed-query and truncated-sample counts are lower bounds: known HIGH/CRITICAL warnings survive, otherwise risk is UNKNOWN. Group impact copies this metadata from the local leg. - riskNote: string — present only when risk is UNKNOWN; states why the verdict is withheld. - summary: direct callers, processes affected, modules affected - affected_processes: which execution flows break and at which step -- affected_modules: which functional areas are hit (direct vs indirect) +- affected_modules: which functional areas are hit (direct vs indirect; classification-unavailable when that secondary query fails) - byDepth: affected symbols grouped by traversal depth (paginated by limit/offset; omitted when summaryOnly:true — use byDepthCounts for totals per depth, pagination object when truncated). Each item includes a processes:[{id,label,processType,step}] field listing the execution flows that symbol participates in. Empty when the symbol has no process membership. Can ALSO be empty when partial:true is set — either the process-aggregation pass hit its cap before detecting affected processes, or per-symbol enrichment was capped on a very large page. When partial:true, do NOT treat processes:[] as proof of no participation; cross-check the top-level affected_processes list. - epistemic: 'exact' | 'lower-bound' — whether impactedCount is the whole story. 'lower-bound' means the walk provably missed callers, so the count is a floor. Absent only on skipped probes (ambiguous-candidate lists, group fan-out). - boundaries: string[] — one plain-language sentence per reason the count is short. Prose for humans; branch on causes instead. diff --git a/gitnexus/test/integration/impact-file-risk-scale.test.ts b/gitnexus/test/integration/impact-file-risk-scale.test.ts new file mode 100644 index 000000000..16b2e6a7f --- /dev/null +++ b/gitnexus/test/integration/impact-file-risk-scale.test.ts @@ -0,0 +1,143 @@ +import { beforeAll, expect, it, vi } from 'vitest'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { withTestLbugDB, type IndexedDBHandle } from '../helpers/test-indexed-db.js'; + +vi.mock('../../src/storage/repo-manager.js', () => ({ + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), +})); + +const fileImporters = Array.from( + { length: 13 }, + (_, index) => + `CREATE (f:File {id: 'File:src/importer-${index}.ts', name: 'importer-${index}.ts', filePath: 'src/importer-${index}.ts', content: ''})`, +); +const fileImportEdges = Array.from( + { length: 13 }, + (_, index) => + `MATCH (a:File {id:'File:src/importer-${index}.ts'}), (b:File {id:'File:src/crypto.ts'}) CREATE (a)-[:CodeRelation {type:'IMPORTS', confidence:1.0, reason:'import', step:0}]->(b)`, +); +const processNodes = Array.from( + { length: 4 }, + (_, index) => + `CREATE (p:Process {id: 'proc-${index}', label: 'Flow ${index}', heuristicLabel: 'Flow ${index}', processType: 'cross_community', stepCount: 3, communities: [], entryPointId: 'Function:src/entry-${index}.ts:entry${index}', terminalId: 'Function:src/crypto.ts:getEncryptionKey'})`, +); +const processEntryPoints = Array.from( + { length: 4 }, + (_, index) => + `CREATE (ep:Function {id: 'Function:src/entry-${index}.ts:entry${index}', name: 'entry${index}', filePath: 'src/entry-${index}.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, +); +const processEdges = Array.from( + { length: 4 }, + (_, index) => + `MATCH (a:Function {id:'Function:src/caller-${index % 2}.ts:caller${index % 2}'}), (p:Process {id:'proc-${index}'}) CREATE (a)-[:CodeRelation {type:'STEP_IN_PROCESS', confidence:1.0, reason:'trace-detection', step:1}]->(p)`, +); + +const SEED = [ + `CREATE (f:File {id: 'File:src/crypto.ts', name: 'crypto.ts', filePath: 'src/crypto.ts', content: ''})`, + ...fileImporters, + ...fileImportEdges, + `CREATE (fn:Function {id: 'Function:src/crypto.ts:getEncryptionKey', name: 'getEncryptionKey', filePath: 'src/crypto.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (c0:Function {id: 'Function:src/caller-0.ts:caller0', name: 'caller0', filePath: 'src/caller-0.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (c1:Function {id: 'Function:src/caller-1.ts:caller1', name: 'caller1', filePath: 'src/caller-1.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `MATCH (a:Function {id:'Function:src/caller-0.ts:caller0'}), (b:Function {id:'Function:src/crypto.ts:getEncryptionKey'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:1.0, reason:'direct', step:0}]->(b)`, + `MATCH (a:Function {id:'Function:src/caller-1.ts:caller1'}), (b:Function {id:'Function:src/crypto.ts:getEncryptionKey'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:1.0, reason:'direct', step:0}]->(b)`, + ...processEntryPoints, + ...processNodes, + ...processEdges, +]; + +type BackendHandle = IndexedDBHandle & { _backend?: LocalBackend }; + +withTestLbugDB( + 'impact-file-risk-scale', + (handle) => { + let backend: LocalBackend; + beforeAll(() => { + const ext = handle as BackendHandle; + if (!ext._backend) throw new Error('LocalBackend not initialized'); + backend = ext._backend; + }); + + it('marks the wider File score incomparable with the process-rich Function score', async () => { + const file = await backend.callTool('impact', { + target: 'crypto.ts', + kind: 'File', + direction: 'upstream', + }); + const fn = await backend.callTool('impact', { + target: 'getEncryptionKey', + kind: 'Function', + direction: 'upstream', + }); + + expect(file.impactedCount).toBe(13); + expect(file.risk).toBe('MEDIUM'); + expect(file.riskSharedAxes).toBe('MEDIUM'); + expect(file.target.type).toBe('File'); + expect(file.riskScale).toEqual({ + comparableAcrossKinds: false, + unusedAxes: [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ], + }); + expect(file.riskNote).toBeUndefined(); + + expect(fn.impactedCount).toBe(2); + expect(fn.risk).toBe('HIGH'); + expect(fn.riskSharedAxes).toBe('LOW'); + expect(fn.summary.processes_affected).toBe(4); + expect(fn.riskScale).toEqual({ + comparableAcrossKinds: true, + unusedAxes: [], + }); + expect(fn.riskNote).toBeUndefined(); + }); + + it('marks downstream File risk incomparable on the same seed', async () => { + const file = await backend.callTool('impact', { + target: 'crypto.ts', + kind: 'File', + direction: 'downstream', + }); + expect(file.target.type).toBe('File'); + expect(file.riskScale.comparableAcrossKinds).toBe(false); + expect(file.riskScale.unusedAxes).toEqual( + expect.arrayContaining([ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ]), + ); + }); + }, + { + seed: SEED, + poolAdapter: true, + afterSetup: async (handle) => { + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'test-repo', + path: '/test/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + stats: { files: 14, nodes: 21, communities: 0, processes: 4 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as BackendHandle)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/impact-zero-caller-risk.test.ts b/gitnexus/test/integration/impact-zero-caller-risk.test.ts index 3438ffc46..ad6653916 100644 --- a/gitnexus/test/integration/impact-zero-caller-risk.test.ts +++ b/gitnexus/test/integration/impact-zero-caller-risk.test.ts @@ -71,6 +71,8 @@ withTestLbugDB( expect(result).not.toHaveProperty('error'); expect(result.impactedCount).toBe(0); expect(result.risk).toBe('UNKNOWN'); + expect(result.riskScale.comparableAcrossKinds).toBe(true); + expect(result.riskNote).toBeDefined(); }); // The ambiguous fan-out narrows candidates into a fresh object, and that @@ -93,6 +95,11 @@ withTestLbugDB( expect(c.risk).toBe('UNKNOWN'); expect(typeof c.riskNote).toBe('string'); expect(c.riskNote).toMatch(/not evidence/i); + expect( + (c as { riskScale?: { unusedAxes?: { reason: string }[] } }).riskScale?.unusedAxes, + ).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-skipped' })]), + ); } }); diff --git a/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts b/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts index b906a1975..37f13de8b 100644 --- a/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts +++ b/gitnexus/test/unit/ai-context-unknown-risk-policy.test.ts @@ -30,6 +30,9 @@ describe('generateGitNexusContent keeps the risk: UNKNOWN policy unconditional ( const content = generateGitNexusContent('UnknownRiskProject', stats, { hasPdg }); expect(content).toContain('MUST treat `risk: UNKNOWN` as unresolved, not as low.'); + expect(content).toContain( + 'never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning', + ); expect(content).toContain( 'callers are not resolvable by the index (plain-object property access, dynamic dispatch, cross-language calls)', ); diff --git a/gitnexus/test/unit/ai-context.test.ts b/gitnexus/test/unit/ai-context.test.ts index d8cf4b208..1e2611df8 100644 --- a/gitnexus/test/unit/ai-context.test.ts +++ b/gitnexus/test/unit/ai-context.test.ts @@ -225,6 +225,16 @@ describe('generateAIContextFiles', () => { expect(withoutPdg).toContain('explain('); }); + it('documents the MCP and Graph-RAG File-risk scale difference', () => { + const content = generateGitNexusContent('RiskScaleProject', { + nodes: 50, + edges: 100, + processes: 5, + }); + expect(content).toContain('MCP File omits axes'); + expect(content).toContain('Graph-RAG expands File'); + }); + it('emits MD060-compatible compact tables in generated docs (#2709)', () => { const content = generateGitNexusContent('MarkdownProject', { nodes: 50, diff --git a/gitnexus/test/unit/cli-impact-pdg-format.test.ts b/gitnexus/test/unit/cli-impact-pdg-format.test.ts index 9e9f4870f..e04f524af 100644 --- a/gitnexus/test/unit/cli-impact-pdg-format.test.ts +++ b/gitnexus/test/unit/cli-impact-pdg-format.test.ts @@ -498,9 +498,10 @@ describe('formatImpactResult — callgraph rendering is UNCHANGED (regression gu }, }; - it('renders the callgraph result with the exact pre-U5 text (byte-identical)', () => { + it('renders the callgraph result with Risk on the callgraph contract (byte-identical for U5+risk)', () => { const expected = [ 'Blast radius for Function computeTotal (upstream): 2 symbol(s) depends on this (will break if changed)', + 'Risk: MEDIUM', '', 'd=1: WILL BREAK (direct) (1)', ' Function callerA → src/a.ts [CALLS]', @@ -532,14 +533,14 @@ describe('formatImpactResult — callgraph rendering is UNCHANGED (regression gu expect(out).not.toContain('PDG-dependent symbols'); }); - it('renders the callgraph isolated / zero case unchanged', () => { + it('renders the callgraph isolated / zero case with risk, without claiming isolation', () => { const out = formatImpactResult({ target: { name: 'lonely' }, direction: 'downstream', impactedCount: 0, risk: 'LOW', }); - expect(out).toBe('lonely: No downstream dependencies found. This symbol appears isolated.'); + expect(out).toBe('lonely: No downstream dependencies found.\nRisk: LOW'); }); it('renders the callgraph lower-bound (DI/dynamic-dispatch) copy unchanged', () => { diff --git a/gitnexus/test/unit/eval-formatters.test.ts b/gitnexus/test/unit/eval-formatters.test.ts index c0349e80d..cdd12b4fd 100644 --- a/gitnexus/test/unit/eval-formatters.test.ts +++ b/gitnexus/test/unit/eval-formatters.test.ts @@ -409,6 +409,35 @@ describe('formatImpactResult', () => { expect(result).toContain('caller2'); }); + it('prints the shared-axes comparison when a target has unavailable risk axes', () => { + const result = formatImpactResult({ + target: { kind: 'File', name: 'crypto.ts' }, + direction: 'upstream', + impactedCount: 13, + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale: { + comparableAcrossKinds: false, + unusedAxes: [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ], + }, + byDepthCounts: { 1: 13 }, + }); + + expect(result).toContain('Risk: MEDIUM'); + expect(result).toContain('Shared-axes risk: MEDIUM'); + expect(result).toContain('process/module axes are unavailable'); + expect(result).toContain('do not use this to waive a HIGH/CRITICAL risk warning'); + }); + it('handles zero impact', () => { const result = formatImpactResult({ target: { name: 'foo' }, @@ -416,7 +445,22 @@ describe('formatImpactResult', () => { impactedCount: 0, byDepth: {}, }); - expect(result).toContain('No upstream dependencies'); + expect(result).toContain('No upstream callers resolved'); + expect(result).not.toContain('appears isolated'); + }); + + it('prints UNKNOWN and riskNote for an empty upstream walk', () => { + const result = formatImpactResult({ + target: { name: 'foo' }, + direction: 'upstream', + impactedCount: 0, + risk: 'UNKNOWN', + riskNote: 'safe to change is a claim about callers and there were none to reason about', + byDepth: {}, + }); + expect(result).toContain('Risk: UNKNOWN'); + expect(result).toContain('safe to change is a claim about callers'); + expect(result).not.toContain('appears isolated'); }); it('formats impact by depth', () => { diff --git a/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts b/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts index aac1ba794..58b11451c 100644 --- a/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts +++ b/gitnexus/test/unit/group/cross-impact-fanout-cap.test.ts @@ -175,6 +175,32 @@ describe('group impact fan-out is bounded by a count, not by the clock (#2787)', expect(result).not.toHaveProperty('riskEpistemic'); }); + it('applies crossing risk to shared axes while preserving local scale metadata', async () => { + bridgeRows.value = [crossingRow(0, 0.9)]; + const riskScale = { + comparableAcrossKinds: true, + unusedAxes: [], + } as const; + const port = makePort({ + impact: vi.fn(async () => ({ + target: { id: 'Function:src/api.ts:publish', filePath: 'src/api.ts' }, + byDepth: { 1: [{ id: 'u1', filePath: 'src/a.ts' }] }, + summary: { direct: 1, processes_affected: 4, modules_affected: 0 }, + risk: 'HIGH', + riskSharedAxes: 'LOW', + riskScale, + })) as GroupToolPort['impact'], + }); + + const result = await run(port); + + expect(result).toMatchObject({ + risk: 'HIGH', + riskSharedAxes: 'HIGH', + riskScale, + }); + }); + it('marks risk as a floor when a crossing is dropped, and does NOT clamp the value down', async () => { // Three crossings, one neighbour unresolvable. Two traverse → HIGH (the // >=0.85-confidence gate). Had the third traversed, `traversed.length >= 3` diff --git a/gitnexus/test/unit/group/cross-impact.test.ts b/gitnexus/test/unit/group/cross-impact.test.ts index 9aac679a0..bceaa9195 100644 --- a/gitnexus/test/unit/group/cross-impact.test.ts +++ b/gitnexus/test/unit/group/cross-impact.test.ts @@ -371,6 +371,35 @@ describe('cross-impact', () => { expect(r).not.toHaveProperty('truncationReason'); }); + it('lifts local riskSharedAxes and riskScale when there are no symbol uids to fan out', async () => { + const riskScale = { + comparableAcrossKinds: false, + unusedAxes: [ + { + axis: 'processes' as const, + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + ], + }; + const r = await runLocalOnlyImpact(async () => ({ + byDepth: {}, + summary: { direct: 13, processes_affected: 0, modules_affected: 0 }, + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale, + })); + expect(r).toMatchObject({ + truncated: false, + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale, + }); + }); + it('test_runGroupImpact_bridge_schema_mismatch_returns_error', async () => { const { tmpDir, groupDir, cleanup } = tmpGroup(); vi.stubEnv('GITNEXUS_HOME', tmpDir); diff --git a/gitnexus/test/unit/group/types.test.ts b/gitnexus/test/unit/group/types.test.ts index 94b0b556c..50eddf921 100644 --- a/gitnexus/test/unit/group/types.test.ts +++ b/gitnexus/test/unit/group/types.test.ts @@ -6,6 +6,7 @@ import type { CrossLink, ContractRegistry, GroupManifestLink, + GroupImpactResult, MatchType, } from '../../../src/core/group/types.js'; @@ -132,4 +133,20 @@ describe('Group types', () => { }; expect(l.contract).toBe('/x'); }); + + it('uses the shared closed union for unused impact-axis reasons', () => { + type UnusedAxis = NonNullable['unusedAxes'][number]; + const valid = { + axis: 'processes', + reason: 'enrichment-query-failed', + } satisfies UnusedAxis; + const invalid = { + axis: 'processes', + // @ts-expect-error unknown reasons must not widen the shared contract + reason: 'not-a-real-reason', + } satisfies UnusedAxis; + + expect(valid.reason).toBe('enrichment-query-failed'); + expect(invalid.reason).toBe('not-a-real-reason'); + }); }); diff --git a/gitnexus/test/unit/impact-batching-grouping.test.ts b/gitnexus/test/unit/impact-batching-grouping.test.ts index 8224cd1d3..e068870f9 100644 --- a/gitnexus/test/unit/impact-batching-grouping.test.ts +++ b/gitnexus/test/unit/impact-batching-grouping.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; // Mock the lbug-adapter module before importing LocalBackend so the class // uses the mocked implementations of executeQuery / executeParameterized. @@ -38,6 +38,10 @@ describe('impact: batching and grouping', () => { vi.clearAllMocks(); }); + afterEach(() => { + delete process.env.IMPACT_MAX_CHUNKS; + }); + it('batches 250 IDs into 3 chunked STEP_IN_PROCESS queries', async () => { // Prepare backend and a fake repo handle const backend = new LocalBackend(); @@ -320,11 +324,328 @@ describe('impact: batching and grouping', () => { expect(Array.isArray(res.affected_modules)).toBe(true); const modNames = res.affected_modules.map((m: any) => m.name); expect(modNames).toContain('ModuleA'); + expect(res.riskScale.comparableAcrossKinds).toBe(false); + expect(res.riskScale.unusedAxes).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-truncated' })]), + ); // Cleanup env delete process.env.IMPACT_MAX_CHUNKS; }); + it('marks IMPACT_MAX_CHUNKS=0 as unused process/module axes', async () => { + process.env.IMPACT_MAX_CHUNKS = '0'; + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-zero-budget', + name: 'repo-zero-budget', + repoPath: '/tmp/repo-zero-budget', + storagePath: '/tmp/repo-zero-budget/.gitnexus', + lbugPath: '/tmp/repo-zero-budget/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symX', name: 'TargetX', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetX', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.riskScale.comparableAcrossKinds).toBe(false); + expect(res.riskScale.unusedAxes).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-budget-exhausted' })]), + ); + delete process.env.IMPACT_MAX_CHUNKS; + }); + + it('marks swallowed process enrichment failures as unused process axes', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-enrich-fail', + name: 'repo-enrich-fail', + repoPath: '/tmp/repo-enrich-fail', + storagePath: '/tmp/repo-enrich-fail/.gitnexus', + lbugPath: '/tmp/repo-enrich-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('STEP_IN_PROCESS')) { + throw new Error('process chunk failed'); + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symFail', name: 'TargetFail', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetFail', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.riskScale.unusedAxes).toEqual( + expect.arrayContaining([ + expect.objectContaining({ axis: 'processes', reason: 'enrichment-query-failed' }), + ]), + ); + expect(res.risk).toBe('UNKNOWN'); + expect(res.partial).toBe(true); + expect(res.riskNote).toContain('enrichment failed'); + }); + + it('marks module query failure without discarding a successful process axis', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-module-fail', + name: 'repo-module-fail', + repoPath: '/tmp/repo-module-fail', + storagePath: '/tmp/repo-module-fail/.gitnexus', + lbugPath: '/tmp/repo-module-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('MEMBER_OF')) throw new Error('module chunk failed'); + if (query.includes('STEP_IN_PROCESS') && query.includes('COUNT(DISTINCT s.id)')) { + return [ + { + pId: 'p1', + entryPointId: 'ep1', + epName: 'main', + epType: 'Function', + hits: 1, + minStep: 1, + }, + ]; + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symModule', name: 'TargetModule', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetModule', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_processes).toHaveLength(1); + expect(res.riskScale.unusedAxes).toEqual([ + { axis: 'modules', reason: 'enrichment-query-failed' }, + ]); + expect(res.risk).toBe('UNKNOWN'); + expect(res.partial).toBe(true); + }); + + it('keeps process risk measured when only minStep backfill fails', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-backfill-fail', + name: 'repo-backfill-fail', + repoPath: '/tmp/repo-backfill-fail', + storagePath: '/tmp/repo-backfill-fail/.gitnexus', + lbugPath: '/tmp/repo-backfill-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('MIN(r.step) AS minStep') && !query.includes('COUNT(DISTINCT s.id)')) { + throw new Error('minStep backfill failed'); + } + if (query.includes('STEP_IN_PROCESS') && query.includes('COUNT(DISTINCT s.id)')) { + return [ + { + pId: 'p1', + entryPointId: 'ep1', + epName: 'main', + epType: 'Function', + hits: 1, + minStep: null, + }, + ]; + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + if (query.includes('MEMBER_OF')) return []; + return [{ id: 'symBackfill', name: 'TargetBackfill', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetBackfill', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_processes).toHaveLength(1); + expect(res.riskScale.unusedAxes).toEqual([]); + expect(res.risk).toBe('LOW'); + expect(res.partial).toBe(true); + }); + + it('keeps observed process warnings when a later enrichment chunk fails', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-later-process-fail', + name: 'repo-later-process-fail', + repoPath: '/tmp/repo-later-process-fail', + storagePath: '/tmp/repo-later-process-fail/.gitnexus', + lbugPath: '/tmp/repo-later-process-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + let processChunk = 0; + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('STEP_IN_PROCESS') && query.includes('COUNT(DISTINCT s.id)')) { + processChunk += 1; + if (processChunk === 2) throw new Error('later process chunk failed'); + return Array.from({ length: 5 }, (_, index) => ({ + pId: `p${index}`, + entryPointId: `ep${index}`, + epName: `process-${index}`, + epType: 'Function', + hits: 1, + minStep: 1, + })); + } + if (query.includes('r.type IN') && !query.includes('STEP_IN_PROCESS')) { + return Array.from({ length: 150 }, (_, index) => ({ + id: `node-${index}`, + name: `n${index}`, + filePath: `file-${index}.js`, + relType: 'CALLS', + confidence: null, + })); + } + if (query.includes('MEMBER_OF')) return []; + return [{ id: 'symLaterFail', name: 'TargetLaterFail', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetLaterFail', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_processes).toHaveLength(5); + expect(res.risk).toBe('CRITICAL'); + expect(res.riskScale.unusedAxes).toContainEqual({ + axis: 'processes', + reason: 'enrichment-query-failed', + }); + expect(res.partial).toBe(true); + }); + + it('does not invent direct/indirect module classification after backfill failure', async () => { + const backend = new LocalBackend(); + const repoHandle = { + id: 'repo-module-classification-fail', + name: 'repo-module-classification-fail', + repoPath: '/tmp/repo-module-classification-fail', + storagePath: '/tmp/repo-module-classification-fail/.gitnexus', + lbugPath: '/tmp/repo-module-classification-fail/.gitnexus/lbug', + indexedAt: 'now', + lastCommit: 'c', + stats: {}, + } as any; + (backend as any).repos.set(repoHandle.id, repoHandle); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + executeQueryMock.mockImplementation(async () => []); + executeParameterizedMock.mockImplementation(async (...args: any[]) => { + const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); + if (query.includes('MEMBER_OF') && query.includes('RETURN DISTINCT c.heuristicLabel')) { + throw new Error('module classification failed'); + } + if (query.includes('MEMBER_OF')) return [{ name: 'ModuleA', hits: 1 }]; + if (query.includes('STEP_IN_PROCESS')) return []; + if (query.includes('r.type IN')) { + return [ + { + id: 'node-1', + name: 'n1', + filePath: 'file-1.js', + relType: 'CALLS', + confidence: null, + }, + ]; + } + return [{ id: 'symClassify', name: 'TargetClassify', filePath: 'f' }]; + }); + + const res = await (backend as any)._impactImpl(repoHandle, { + target: 'TargetClassify', + direction: 'downstream', + maxDepth: 1, + } as any); + expect(res.affected_modules).toEqual([ + expect.objectContaining({ name: 'ModuleA', impact: 'classification-unavailable' }), + ]); + expect(res.riskScale.unusedAxes).toEqual([]); + expect(res.partial).toBe(true); + }); + it('caps implicit object-callable expansion and reports partial impact', async () => { const backend = new LocalBackend(); const repoHandle = { @@ -384,6 +705,10 @@ describe('impact: batching and grouping', () => { expect(traversalCall?.[2]?.frontierIds).toEqual(['owner']); expect(result.byDepth['1']).toHaveLength(5000); expect(result.partial).toBe(true); + expect(result.riskScale.comparableAcrossKinds).toBe(false); + expect(result.riskScale.unusedAxes).toEqual( + expect.arrayContaining([expect.objectContaining({ reason: 'enrichment-skipped' })]), + ); }); it('marks object impact partial when callable seeding fails', async () => { diff --git a/gitnexus/test/unit/impact-risk.test.ts b/gitnexus/test/unit/impact-risk.test.ts new file mode 100644 index 000000000..bd1b2661e --- /dev/null +++ b/gitnexus/test/unit/impact-risk.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from 'vitest'; +import { + scoreImpactRisk, + unusedAxesForImpactWalk, + type ImpactRiskInput, + type UnusedImpactRiskAxis, +} from 'gitnexus-shared'; + +const fileUnusedAxes: readonly UnusedImpactRiskAxis[] = [ + { + axis: 'processes', + reason: 'file-nodes-have-no-process-or-community-membership', + }, + { + axis: 'modules', + reason: 'file-nodes-have-no-process-or-community-membership', + }, +]; + +const base: ImpactRiskInput = { + direction: 'upstream', + directCount: 0, + processCount: 0, + moduleCount: 0, + impactedCount: 1, +}; + +describe('scoreImpactRisk', () => { + it('makes the issue #3075 File and Function scales explicit', () => { + const file = scoreImpactRisk({ + ...base, + directCount: 13, + impactedCount: 25, + unusedAxes: fileUnusedAxes, + }); + const fn = scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 4, + moduleCount: 2, + impactedCount: 15, + }); + + expect(file).toEqual({ + risk: 'MEDIUM', + riskSharedAxes: 'MEDIUM', + riskScale: { + comparableAcrossKinds: false, + unusedAxes: fileUnusedAxes, + }, + }); + expect(fn).toEqual({ + risk: 'HIGH', + riskSharedAxes: 'LOW', + riskScale: { + comparableAcrossKinds: true, + unusedAxes: [], + }, + }); + }); + + it('preserves UNKNOWN only for an empty upstream walk', () => { + expect(scoreImpactRisk({ ...base, impactedCount: 0 }).risk).toBe('UNKNOWN'); + expect(scoreImpactRisk({ ...base, direction: 'downstream', impactedCount: 0 }).risk).toBe( + 'LOW', + ); + }); + + it('marks skipped enrichment as a non-comparable scale', () => { + const skippedAxes: readonly UnusedImpactRiskAxis[] = [ + { axis: 'processes', reason: 'enrichment-skipped' }, + { axis: 'modules', reason: 'enrichment-skipped' }, + ]; + + expect(scoreImpactRisk({ ...base, unusedAxes: skippedAxes }).riskScale).toEqual({ + comparableAcrossKinds: false, + unusedAxes: skippedAxes, + }); + }); + + it('preserves direct and total thresholds when enrichment axes are unused', () => { + expect( + scoreImpactRisk({ + ...base, + directCount: 30, + impactedCount: 30, + unusedAxes: fileUnusedAxes, + }).risk, + ).toBe('CRITICAL'); + expect( + scoreImpactRisk({ + ...base, + directCount: 15, + impactedCount: 15, + unusedAxes: fileUnusedAxes, + }).risk, + ).toBe('HIGH'); + expect( + scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 10, + moduleCount: 10, + unusedAxes: fileUnusedAxes, + }).risk, + ).toBe('LOW'); + }); + + it('zeros unused process/module counts on the primary risk ladder', () => { + const skippedAxes: readonly UnusedImpactRiskAxis[] = [ + { axis: 'processes', reason: 'enrichment-skipped' }, + { axis: 'modules', reason: 'enrichment-skipped' }, + ]; + const scored = scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 4, + moduleCount: 4, + impactedCount: 15, + unusedAxes: skippedAxes, + }); + expect(scored.risk).toBe('LOW'); + expect(scored.riskSharedAxes).toBe('LOW'); + }); + + it('preserves a warning already proved before a later enrichment failure', () => { + const scored = scoreImpactRisk({ + ...base, + directCount: 2, + processCount: 5, + impactedCount: 5, + unusedAxes: [{ axis: 'processes', reason: 'enrichment-query-failed' }], + }); + + expect(scored.risk).toBe('CRITICAL'); + expect(scored.riskSharedAxes).toBe('LOW'); + expect(scored.riskScale.comparableAcrossKinds).toBe(false); + }); + + it('fails closed when query failure leaves only a LOW or MEDIUM observed score', () => { + for (const directCount of [2, 6]) { + const scored = scoreImpactRisk({ + ...base, + direction: 'downstream', + directCount, + processCount: 0, + impactedCount: directCount, + unusedAxes: [{ axis: 'processes', reason: 'enrichment-query-failed' }], + }); + expect(scored.risk).toBe('UNKNOWN'); + } + }); + + it('fails closed when a truncated sample leaves only a LOW or MEDIUM observed score', () => { + const truncatedAxes: readonly UnusedImpactRiskAxis[] = [ + { axis: 'processes', reason: 'enrichment-truncated' }, + { axis: 'modules', reason: 'enrichment-truncated' }, + ]; + const scored = scoreImpactRisk({ + ...base, + direction: 'downstream', + directCount: 2, + processCount: 1, + moduleCount: 1, + impactedCount: 8, + unusedAxes: truncatedAxes, + }); + expect(scored.risk).toBe('UNKNOWN'); + expect(scored.riskScale.comparableAcrossKinds).toBe(false); + }); +}); + +describe('unusedAxesForImpactWalk', () => { + it('marks File, skipEnrichment, zero budget, and query failure distinctly', () => { + expect( + unusedAxesForImpactWalk({ + isFileTarget: true, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed: true, + moduleQueryFailed: true, + impactedCount: 1, + }).every((a) => a.reason === 'file-nodes-have-no-process-or-community-membership'), + ).toBe(true); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: true, + maxChunks: 0, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 1, + }).map((a) => a.reason), + ).toEqual(['enrichment-skipped', 'enrichment-skipped']); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 0, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 1, + }).map((a) => a.reason), + ).toEqual(['enrichment-budget-exhausted', 'enrichment-budget-exhausted']); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed: true, + moduleQueryFailed: false, + impactedCount: 1, + }), + ).toEqual([{ axis: 'processes', reason: 'enrichment-query-failed' }]); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 0, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 0, + }), + ).toEqual([]); + expect( + unusedAxesForImpactWalk({ + isFileTarget: false, + skipEnrichment: false, + maxChunks: 10, + processQueryFailed: false, + moduleQueryFailed: false, + impactedCount: 501, + enrichmentTruncated: true, + }).map((a) => a.reason), + ).toEqual(['enrichment-truncated', 'enrichment-truncated']); + }); +}); diff --git a/gitnexus/test/unit/shipped-skills-sync.test.ts b/gitnexus/test/unit/shipped-skills-sync.test.ts index c82e45321..984037e4c 100644 --- a/gitnexus/test/unit/shipped-skills-sync.test.ts +++ b/gitnexus/test/unit/shipped-skills-sync.test.ts @@ -170,6 +170,21 @@ describe('intended standard-skill improvements stay in every applicable copy', ( } }); + it('keeps the cross-surface risk-scale guidance in every impact-analysis copy', () => { + const required = [ + '`riskSharedAxes`', + 'MCP File walks', + 'web Graph-RAG expands File targets', + 'Within single-repo mode', + 'Within group mode', + 'overlays resolved', + ]; + for (const file of standardSkillCopies('gitnexus-impact-analysis')) { + const content = fs.readFileSync(file, 'utf-8'); + for (const fragment of required) expect(content).toContain(fragment); + } + }); + // Same shape as the UNKNOWN guard above, for the other half of the verdict: // `detect_changes` can come back SHORT — `partial` when a batched graph query // failed, `truncated` when the changed-symbol listing hit its cap — and both @@ -331,6 +346,10 @@ describe('root AGENTS.md / CLAUDE.md managed block keeps the risk: UNKNOWN polic const REQUIRED_FRAGMENTS = [ 'MUST treat `risk: UNKNOWN` as unresolved, not as low.', 'never read `UNKNOWN` as an all-clear', + 'never use `riskSharedAxes` to waive a HIGH/CRITICAL `risk` warning', + 'Compare File/symbol', + 'MCP File omits axes', + 'Graph-RAG expands File', ]; it.each(['AGENTS.md', 'CLAUDE.md'])('%s managed block documents the policy', (file) => { diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index d9a2890d9..700407f78 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -193,6 +193,17 @@ describe('GITNEXUS_TOOLS', () => { expect(impactTool.description).toContain('truncatedBy'); }); + it('documents riskSharedAxes as a compare aid, not the edit gate', () => { + const impactTool = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!; + expect(impactTool.description).toContain('riskSharedAxes'); + expect(impactTool.description).toContain('Never substitute it for `risk`'); + expect(impactTool.description).toContain('IMPACT_MAX_CHUNKS=0'); + expect(impactTool.description).toContain('sampled a subset of impacted symbols'); + expect(impactTool.description).toContain('Graph-RAG'); + expect(impactTool.description).toContain('cross-repo crossing overlay'); + expect(impactTool.description).toContain('known HIGH/CRITICAL warnings survive'); + }); + it.each(['query', 'context', 'impact'])( '%s advertises an optional positive maxTokens budget', (name) => {