diff --git a/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts b/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts index ccb44b037..b62b6ff20 100644 --- a/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts +++ b/gitnexus/src/core/ingestion/utils/callable-flow-captures.ts @@ -216,7 +216,15 @@ export function synthesizeCallableFlowCaptures( const out: CaptureMatch[] = []; for (const assignment of assignments) { - emitAssignmentFact(assignment, knownCallableNames, valueBindings, options, out); + for (const source of valueAlternatives(assignment.source)) { + emitAssignmentFact( + { ...assignment, source }, + knownCallableNames, + valueBindings, + options, + out, + ); + } } for (const fn of functions) emitFormalFacts(fn, options, out); for (const node of nodes) { @@ -587,6 +595,34 @@ function assignmentParts( return [{ container: node, destination, source }]; } +/** Operators whose result is one of their operands, not a computed value. */ +const VALUE_SELECTING_OPERATORS = new Set(['??', '||', 'or', '?:']); + +/** + * The operands a value-selecting expression can evaluate to (#3354): + * `a ?? b`, `a || b`, `a or b`, and `c ? a : b` each yield one of their + * branches, so each branch flows into the destination. Anything else is its + * own single alternative, which leaves every other source shape untouched. + */ +function valueAlternatives(node: SyntaxNode): SyntaxNode[] { + let inner = node; + while (inner.type.includes('parenthesized') && inner.namedChildCount === 1) { + inner = inner.namedChild(0)!; + } + const consequence = inner.childForFieldName('consequence'); + const alternative = inner.childForFieldName('alternative'); + if (consequence !== null && alternative !== null && inner.childForFieldName('condition')) { + return [...valueAlternatives(consequence), ...valueAlternatives(alternative)]; + } + const left = inner.childForFieldName('left'); + const right = inner.childForFieldName('right'); + const operator = inner.childForFieldName('operator')?.type; + if (left !== null && right !== null && operator && VALUE_SELECTING_OPERATORS.has(operator)) { + return [...valueAlternatives(left), ...valueAlternatives(right)]; + } + return [node]; +} + function emitAssignmentFact( assignment: AssignmentParts, knownCallableNames: ReadonlySet, diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 31720337f..061c3c70d 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -4431,7 +4431,10 @@ export class LocalBackend { } | { kind: 'not_found' } > { - const { uid, name, include_content } = query; + const { name, include_content } = query; + // A blank uid is an omitted optional field (strict adapters send " "/""), + // not a lookup key — fall through to the name instead of `not_found`. + const uid = query.uid?.trim(); const selectClause = `n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine${include_content ? ', n.content AS content' : ''}`; // Direct UID — zero-ambiguity path. @@ -7194,7 +7197,7 @@ export class LocalBackend { ); if (outcome.kind === 'not_found') { - const missing = params.target_uid ?? target; + const missing = params.target_uid?.trim() || target; // not_found = no resolved symbol, so the envelope keeps the partial-but- // typed target (typed PdgImpactTarget — there is no id/type/filePath yet). const notFoundTarget: PdgImpactTarget = { name: target }; @@ -8017,6 +8020,14 @@ export class LocalBackend { const confidenceFilter = safeMinConfidence > 0 ? ' AND r.confidence >= $minConfidence' : ''; const symId = sym.id || sym[0]; + // #3354: a walk with no anchor id cannot say anything about THIS symbol, + // yet it still ships a normal-looking `exact` result with the target's + // name echoed back. Throw so every caller's catch reports UNKNOWN instead. + if (!symId) { + throw new Error( + `Impact target '${sym.name || sym[1] || '?'}' resolved without a node id; refusing to report a blast radius`, + ); + } // #1858 — kick off the epistemic boundary probe concurrently with the BFS. // It depends only on symId/symType/symName (all known now) and touches no diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 7e91cee4c..e7a6645c6 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -778,7 +778,11 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // v104 (#3339 review): TS/JS pair-HOC queries now name object-pair // `mutation(withAuth(arrow))` handlers. Warm caches replay the pre-fix // capture set (anonymous arrows, no Function name), so both stores re-extract. -const SCHEMA_BUMP = 104; +// v112 (#3354): callable-value flow now follows each branch of `a ?? f`, +// `a || f`, and `c ? f : g`. Warm caches replay the pre-fix flow facts, which +// have no flow for those assignments, so both stores re-extract. 105-111 are +// claimed by open PR #3326 (Elixir). +const SCHEMA_BUMP = 112; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/index.ts b/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/index.ts new file mode 100644 index 000000000..be47f8e38 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/index.ts @@ -0,0 +1,23 @@ +import { runSweep, runAlias, runOr, runThen, runElse } from './sweep'; + +type Handler = (env: unknown) => Promise; + +export async function aliasOnly(env: unknown) { + const run = runAlias; + await run(env); +} + +export async function nullish(env: { __sweep?: Handler }) { + const sweep = env.__sweep ?? runSweep; + await sweep(env); +} + +export async function logicalOr(env: { override?: Handler }) { + const run = env.override || runOr; + await run(env); +} + +export async function ternary(env: unknown, fast: boolean) { + const run = fast ? runThen : runElse; + await run(env); +} diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/sweep/index.ts b/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/sweep/index.ts new file mode 100644 index 000000000..10b6950ca --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/sweep/index.ts @@ -0,0 +1,5 @@ +export async function runSweep(env: unknown): Promise {} +export async function runAlias(env: unknown): Promise {} +export async function runOr(env: unknown): Promise {} +export async function runThen(env: unknown): Promise {} +export async function runElse(env: unknown): Promise {} diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/worker.ts b/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/worker.ts new file mode 100644 index 000000000..8e51fe4a3 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-callable-alternatives/worker.ts @@ -0,0 +1,10 @@ +import { runSweep } from './sweep'; + +// The reporter's shape: a Cloudflare worker's default-export object whose +// `scheduled` handler lets tests inject a replacement sweep. +export default { + async scheduled(_c: unknown, env: { __sweep?: typeof runSweep }) { + const sweep = env.__sweep ?? runSweep; + await sweep(env); + }, +}; diff --git a/gitnexus/test/integration/resolvers/typescript-callable-alternatives.test.ts b/gitnexus/test/integration/resolvers/typescript-callable-alternatives.test.ts new file mode 100644 index 000000000..2cc102e9a --- /dev/null +++ b/gitnexus/test/integration/resolvers/typescript-callable-alternatives.test.ts @@ -0,0 +1,53 @@ +/** + * TypeScript: a callable chosen by a value-selecting expression (#3354). + * + * `const sweep = env.__sweep ?? runSweep; await sweep(env)` is how the + * reporter's Cloudflare worker made its sweep injectable in tests. The + * callable-value flow only accepted a single designator on the right-hand + * side, so the `??` produced no flow, `scheduled` never showed up as a caller + * of `runSweep`, and `impact` answered with one caller fewer while still + * claiming `epistemic: "exact"`. Each branch of `??`, `||`, and `?:` can be + * the value that is later invoked, so each branch is a flow into the binding. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import path from 'path'; +import { + FIXTURES, + getRelationships, + edgeSet, + runPipelineFromRepo, + type PipelineResult, +} from './helpers.js'; + +describe('TypeScript callable chosen by ?? / || / ?:', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'typescript-callable-alternatives'), + () => {}, + ); + }, 60000); + + const calls = () => edgeSet(getRelationships(result, 'CALLS')); + + it('control: a plain alias reaches its callee', () => { + expect(calls()).toContain('aliasOnly → runAlias'); + }); + + it('`a ?? fn` reaches fn', () => { + expect(calls()).toContain('nullish → runSweep'); + }); + + it('`a ?? fn` inside an object-literal method reaches fn (worker `scheduled`)', () => { + expect(calls()).toContain('scheduled → runSweep'); + }); + + it('`a || fn` reaches fn', () => { + expect(calls()).toContain('logicalOr → runOr'); + }); + + it('`c ? f : g` reaches both branches', () => { + expect(calls()).toEqual(expect.arrayContaining(['ternary → runThen', 'ternary → runElse'])); + }); +}); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 8e688c45d..b42cae8e3 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -622,6 +622,38 @@ describe('LocalBackend.callTool', () => { }); }); + it('reports UNKNOWN instead of a blast radius when the target resolves without a node id (#3354)', async () => { + // Every query returns the same id-less row: the resolver picks it as the + // single match, and the frontier query would answer for no symbol at all. + (executeParameterized as any).mockResolvedValue([{ name: 'runSweep', type: 'Function' }]); + + const result = await backend.callTool('impact', { target: 'runSweep', direction: 'upstream' }); + + expect(result).toMatchObject({ + target: { name: 'runSweep' }, + impactedCount: null, + risk: 'UNKNOWN', + }); + expect(result.error).toMatch(/without a node id/); + expect(result).not.toHaveProperty('byDepthCounts'); + }); + + it('treats a whitespace target_uid as omitted and resolves the name (#3354)', async () => { + (executeParameterized as any).mockResolvedValue([]); + + const result = await backend.callTool('impact', { + target: 'validate', + target_uid: ' ', + direction: 'upstream', + }); + + // Name resolution ran (no rows → not found by NAME), not a lookup of uid ' '. + expect(result.error).toBe("Target 'validate' not found"); + const boundParams = (executeParameterized as any).mock.calls.map((c: unknown[]) => c[2]); + expect(boundParams).not.toContainEqual(expect.objectContaining({ uid: expect.anything() })); + expect(boundParams).toContainEqual(expect.objectContaining({ symName: 'validate' })); + }); + it('normalizes impact aliases before @group forwarding', async () => { resolveAtMemberMock.mockResolvedValue({ ok: true, repoPath: '/tmp/test-project' }); const groupImpactSpy = vi diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 879af7a38..55549332e 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -288,8 +288,10 @@ describe('PARSE_CACHE_VERSION', () => { // Moved 103 -> 104 for #3339 review: pair-HOC queries name // `mutation(withAuth(arrow))` object-pair handlers. Warm caches replay // anonymous arrows, so both stores re-extract. - it('pins SCHEMA_BUMP to 104 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(104); + // Moved 104 -> 112 for #3354: callable-value flow follows `??`/`||`/`?:` + // branches. 105-111 are claimed by open PR #3326. + it('pins SCHEMA_BUMP to 112 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(112); expect(PARSE_CACHE_BUCKET_COUNT).toBe(128); // The PREVIOUS version must fail the reuse gate, not merely differ from the // current one — a hardcoded number outside the conflict hunk rebases cleanly @@ -298,6 +300,7 @@ describe('PARSE_CACHE_VERSION', () => { for (const taken of [ 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, + 104, 105, 106, 107, 108, 109, 110, 111, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); }