diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index d4b3ea063..c62076bda 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -7050,8 +7050,27 @@ export class LocalBackend { // Risk scoring const processCount = affectedProcesses.length; const moduleCount = affectedModules.length; - let risk = 'LOW'; - if (directCount >= 30 || processCount >= 5 || moduleCount >= 5 || impacted.length >= 200) { + 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 || @@ -7062,6 +7081,8 @@ export class LocalBackend { risk = 'HIGH'; } else if (directCount >= 5 || impacted.length >= 30) { risk = 'MEDIUM'; + } else { + risk = 'LOW'; } // Build per-depth counts (always included, even in summaryOnly mode) @@ -7090,6 +7111,16 @@ export class LocalBackend { direction, impactedCount: impacted.length, risk, + ...(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.', + } + : {}), ...epistemic, ...(!traversalComplete && { partial: true }), summary: { diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index ee7d7fbee..f33b3e864 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -455,7 +455,8 @@ 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 +- 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. +- 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) diff --git a/gitnexus/test/integration/impact-zero-caller-risk.test.ts b/gitnexus/test/integration/impact-zero-caller-risk.test.ts new file mode 100644 index 000000000..485cbe917 --- /dev/null +++ b/gitnexus/test/integration/impact-zero-caller-risk.test.ts @@ -0,0 +1,104 @@ +/** + * Integration test: an empty upstream walk is UNKNOWN risk, never LOW. + * + * `risk: LOW` asserts "safe to change". That is a claim ABOUT callers, so a + * walk that resolved NONE has nothing to base it on: the symbol is either + * genuinely unused, or reached only through a reference class the index does + * not record (a property access on a plain object, a bare-identifier read of a + * module-scope `Const` — neither mints a reference site today). Reporting LOW + * there is the false-safe signal `anyKnownRisk` already refuses to emit on the + * ambiguous-candidate path, and that #2687 removed by making an undetermined + * `impactedCount` `null` rather than `0`. + * + * Direction matters: an empty DOWNSTREAM walk says this symbol resolved no + * callees, which is not a safety verdict, so it keeps its existing risk. + */ +import { it, expect, beforeAll, vi } from 'vitest'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { withTestLbugDB } 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 SEED = [ + // No edges in either direction — the empty-walk case. + `CREATE (orphan:Function {id: 'Function:src/orphan.ts:orphanHelper', name: 'orphanHelper', filePath: 'src/orphan.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + // A resolved caller -> callee pair — the control that must stay LOW. + `CREATE (used:Function {id: 'Function:src/used.ts:usedHelper', name: 'usedHelper', filePath: 'src/used.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (caller:Function {id: 'Function:src/caller.ts:callerFn', name: 'callerFn', filePath: 'src/caller.ts', startLine: 1, endLine: 8, isExported: true, content: '', description: ''})`, + `MATCH (a:Function {id:'Function:src/caller.ts:callerFn'}), (b:Function {id:'Function:src/used.ts:usedHelper'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.9, reason:'direct', step:0}]->(b)`, +]; + +withTestLbugDB( + 'impact-zero-caller-risk', + (handle) => { + let backend: LocalBackend; + beforeAll(() => { + backend = (handle as any)._backend; + }); + + it('reports UNKNOWN, not LOW, when an upstream walk resolves no callers', async () => { + const result = await backend.callTool('impact', { + target: 'orphanHelper', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('UNKNOWN'); + }); + + it('explains the withheld verdict in riskNote', async () => { + const result = await backend.callTool('impact', { + target: 'orphanHelper', + direction: 'upstream', + }); + expect(typeof result.riskNote).toBe('string'); + // The note must say absence-of-edges is not proof of disuse; an agent + // gating its own edits reads this instead of inferring safety from 0. + expect(result.riskNote).toMatch(/not evidence/i); + }); + + it('leaves a resolved caller set at LOW with no riskNote', async () => { + const result = await backend.callTool('impact', { + target: 'usedHelper', + direction: 'upstream', + }); + expect(result.impactedCount).toBeGreaterThanOrEqual(1); + expect(result.risk).toBe('LOW'); + expect(result.riskNote).toBeUndefined(); + }); + + it('does not hedge an empty DOWNSTREAM walk — that is not a safety claim', async () => { + const result = await backend.callTool('impact', { + target: 'orphanHelper', + direction: 'downstream', + }); + expect(result.impactedCount).toBe(0); + expect(result.risk).toBe('LOW'); + expect(result.riskNote).toBeUndefined(); + }); + }, + { + 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: 3, nodes: 3, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +);