diff --git a/gitnexus/src/cli/analyze-config.ts b/gitnexus/src/cli/analyze-config.ts index 8fbe40feb1..31df69cd3f 100644 --- a/gitnexus/src/cli/analyze-config.ts +++ b/gitnexus/src/cli/analyze-config.ts @@ -56,6 +56,7 @@ type ValueKind = | 'boolean' | 'boolean-negate' | 'string' + | 'string-array' | 'numeric-string' | 'embeddings' | 'branch'; @@ -99,6 +100,12 @@ const KEY_SPECS: Record = { embeddingBatchSize: { target: 'embeddingBatchSize', kind: 'numeric-string' }, embeddingSubBatchSize: { target: 'embeddingSubBatchSize', kind: 'numeric-string' }, embeddingDevice: { target: 'embeddingDevice', kind: 'string' }, + // #1589/#1852 residual — extra fetch-wrapper function names to treat as HTTP + // consumers. The auto-detector only flags functions that call the bare global + // `fetch()`; a wrapper built on axios / a custom client, or named outside the + // built-in convention set, is otherwise invisible to route_map consumers. + // Listing it here adds it to the cross-file consumer scan. + fetchWrappers: { target: 'fetchWrappers', kind: 'string-array' }, }; /** Top-level container key for the nested form; not itself an `AnalyzeOptions` field. */ @@ -230,6 +237,41 @@ const normalizeValue = (kind: ValueKind, value: unknown, key: string): unknown = } return trimmed; } + case 'string-array': { + // Generic shared validator — `source` already names the config key, so + // messages here stay key-agnostic (no fetch-wrapper coupling in the + // shared normalizer; #1589/#1852 review F7). + if (!Array.isArray(value)) { + throw new GitNexusRcError(`${source} must be an array of strings.`); + } + const names: string[] = []; + for (const item of value) { + if (typeof item !== 'string') { + throw new GitNexusRcError(`${source} entries must all be strings.`); + } + const trimmed = item.trim(); + if (!trimmed) { + throw new GitNexusRcError(`${source} entries must not be empty.`); + } + assertNoHiddenChars(trimmed, source); + // Values may be interpolated into a RegExp downstream. Restrict to + // identifier / member-access shapes so a config value can never smuggle + // regex metacharacters into a consumer. + if (!/^[A-Za-z_$][A-Za-z0-9_$.]*$/.test(trimmed)) { + throw new GitNexusRcError( + `${source} entry "${trimmed}" must be an identifier or member name ` + + `(letters, digits, _, $, . — e.g. "client.get").`, + ); + } + names.push(trimmed); + } + if (names.length === 0) { + throw new GitNexusRcError(`${source} must list at least one string.`); + } + // De-duplicate and cap to a sane bound so a pathological config cannot + // blow up the consumer scan's alternation. + return Array.from(new Set(names)).slice(0, 100); + } case 'numeric-string': { // Mirror Commander's contract: these options reach the existing CLI // validation as strings. Accept a JSON number or a string; normalize to a diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 2ecf8abdfb..e44778785b 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -663,6 +663,14 @@ export interface AnalyzeOptions { embeddingBatchSize?: string; embeddingSubBatchSize?: string; embeddingDevice?: string; + /** + * Extra fetch-wrapper function names to treat as HTTP consumers (#1589/#1852 + * residual). Supplied via `.gitnexusrc` `fetchWrappers: [...]`. Threaded into + * the routes phase, where the cross-file consumer scan unions them with the + * auto-detected `fetch()` wrappers so a custom/axios-based wrapper named + * outside the built-in convention still produces `route_map` consumers. + */ + fetchWrappers?: string[]; } /** @@ -1137,6 +1145,9 @@ const analyzeCommandImpl = async ( // GITNEXUS_WORKER_POOL_SIZE env mutation. `undefined` defers to the // env / auto-formula fallback inside the pipeline. workerPoolSize, + // Extra fetch-wrapper names from `.gitnexusrc` (#1589/#1852 residual); + // forwarded to the routes phase consumer scan. + fetchWrappers: options.fetchWrappers, }, { onProgress: (_phase, percent, message) => { diff --git a/gitnexus/src/cli/eval-server.ts b/gitnexus/src/cli/eval-server.ts index b2dc85dc07..a3a058264e 100644 --- a/gitnexus/src/cli/eval-server.ts +++ b/gitnexus/src/cli/eval-server.ts @@ -189,7 +189,47 @@ export function formatImpactResult(result: any): string { const byDepth = result.byDepth || {}; const total = result.impactedCount || 0; + // #2129 — an ambiguous bare name must not print the "isolated / safe to + // refactor" headline. Surface the per-candidate blast radius + the maximum, + // mirroring formatContextResult, so the real impact under whichever symbol the + // caller meant is visible on the text surface, not just in the JSON. + if (result.status === 'ambiguous') { + // #2129 review F11 — report the FULL match count (`totalCandidates`), not the + // truncated `candidates[]` length; note when the candidate list is capped. + const shown = result.candidates?.length ?? 0; + const total = result.totalCandidates ?? shown; + const countPhrase = total > shown ? `${total} symbols (showing ${shown})` : `${total} symbols`; + const lines = [ + `${target?.name || '?'}: AMBIGUOUS — ${countPhrase} share this name. ` + + `Max blast radius ${result.maxImpactedCount ?? 0} (${result.maxRisk ?? 'UNKNOWN'} risk). ` + + `Disambiguate with --uid for one authoritative result:`, + ]; + for (const c of result.candidates || []) { + lines.push( + ` ${c.kind} ${c.name} → ${c.filePath}:${c.line || '?'} ` + + `[${c.impactedCount ?? 0} ${direction}, risk ${c.risk ?? 'UNKNOWN'}] (uid: ${c.uid})`, + ); + } + // #2129 review F1 — a failed per-candidate probe makes the max a lower bound. + if (result.partialProbe) { + lines.push( + ' ⚠️ One or more candidate probes failed — max blast radius / risk are lower bounds.', + ); + } + return lines.join('\n'); + } + if (total === 0) { + // #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. + if (result.epistemic === 'lower-bound') { + const lines = [ + `${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'); + } return `${target?.name || '?'}: No ${direction} dependencies found. This symbol appears isolated.`; } @@ -202,6 +242,14 @@ export function formatImpactResult(result: any): string { if (result.partial) { lines.push('⚠️ Partial results — graph traversal was interrupted. Deeper impacts may exist.'); } + // #1858 — an interface / indirection boundary on the path makes this a lower + // bound; surface it so the count is not read as exhaustive. + if (result.epistemic === 'lower-bound') { + lines.push( + '⚠️ Lower bound — unresolved indirection on the path (callers binding via DI / dynamic dispatch are not traced; actual impact may be higher):', + ); + for (const b of result.boundaries || []) lines.push(` • ${b}`); + } lines.push(''); const depthLabels: Record = { diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index 8c0a67ac5a..561590e054 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -367,25 +367,77 @@ export const routesPhase: PipelinePhase = { // scan JS/TS consumer files for calls to those wrapper functions with // URL-like string arguments and add them to allFetchCalls so // processNextjsFetchRoutes can create FETCHES edges. - if (allFetchWrapperDefs && allFetchWrapperDefs.length > 0 && routeRegistry.size > 0) { - const wrapperNames = new Set(allFetchWrapperDefs.map((d) => d.functionName)); + // Wrapper names come from two sources: functions the parse phase + // auto-detected as calling the bare global `fetch()`, plus any names the + // user declared in `.gitnexusrc` `fetchWrappers` (#1589/#1852 residual). + // Config names let an axios/custom-client wrapper — or one named outside the + // built-in convention — still produce route_map consumers; without them it + // silently falls back to `consumers: []`. Configured names alone are enough + // to run the scan even when nothing was auto-detected. + // Configured names are already validated/trimmed/de-duped/capped by + // analyze-config.ts — trusted as-is (#1589/#1852 review F9, dropped the + // redundant re-trim/re-filter). The single filter below guards only the + // auto-detected `functionName`s, which have no shape guarantee. + const configuredWrappers = ctx.options?.fetchWrappers ?? []; + const wrapperNames = new Set( + [...(allFetchWrapperDefs ?? []).map((d) => d.functionName), ...configuredWrappers].filter( + (n): n is string => typeof n === 'string' && n.trim().length > 0, + ), + ); + if (wrapperNames.size > 0 && routeRegistry.size > 0) { const jsFiles = allPaths.filter((p) => /\.[jt]sx?$/.test(p)); - if (jsFiles.length > 0 && wrapperNames.size > 0) { - const jsContents = await readFileContents(ctx.repoPath, jsFiles); - for (const [filePath, content] of jsContents) { - for (const name of wrapperNames) { - const regex = new RegExp( - `\\b${escapeRegex(name)}\\s*\\(\\s*['"\`](/[^'"\`\\s)]+)['"\`]`, - 'g', - ); - let match; - while ((match = regex.exec(content)) !== null) { - allFetchCalls.push({ - filePath, - fetchURL: match[1], - lineNumber: content.substring(0, match.index).split('\n').length, - }); + if (jsFiles.length > 0) { + // Reuse contents already read for handler extraction; only read the + // remainder (mirrors the Expo block above). Avoids a second full read of + // files we already have in memory. + const unreadJsFiles = jsFiles.filter((p) => !handlerContents?.has(p)); + const extraContents = + unreadJsFiles.length > 0 + ? await readFileContents(ctx.repoPath, unreadJsFiles) + : new Map(); + // One alternation regex over every wrapper name per file — O(files), not + // O(files × wrappers) (#1852 review F3). Names are escaped and grouped + // non-capturing so capture group 1 stays the URL. The left boundary is a + // negative lookbehind, not `\b`: a bare configured name like `get` must + // match the free call `get('/x')` but NOT a member access `client.get(` + // (a `.get(` on an unrelated object), and `apiFetch` must not match + // `myApiFetch`. Member-style wrappers are configured with the dot + // (`client.get`), where the `.` is part of the pattern. The `u` flag + + // Unicode property classes make the boundary cover non-ASCII identifier + // characters too — ASCII `\w` would let `caféget('/x')` match `get` + // (#1852 review F10). + const alternation = [...wrapperNames].map(escapeRegex).join('|'); + const wrapperCallRegex = new RegExp( + `(? { + wrapperCallRegex.lastIndex = 0; + // 1-based line number via a running newline counter: matches arrive in + // ascending index, so accumulate newlines incrementally instead of + // re-allocating `content.substring(0, match.index).split('\n')` on + // every match (#1852 review F12). Output is identical. + let line = 1; + let scanned = 0; + let match; + while ((match = wrapperCallRegex.exec(content)) !== null) { + for (; scanned < match.index; scanned++) { + if (content.charCodeAt(scanned) === 10 /* '\n' */) line++; } + allFetchCalls.push({ + filePath, + fetchURL: match[1], + lineNumber: line, + }); + } + }; + for (const [filePath, content] of extraContents) scanContent(filePath, content); + // Also scan already-read JS/TS handler files (a handler can itself + // consume another route through a wrapper). + if (handlerContents) { + for (const p of jsFiles) { + const cached = handlerContents.get(p); + if (cached !== undefined) scanContent(p, cached); } } } diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index 0e610e6474..2b1709a35b 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -129,6 +129,15 @@ export interface PipelineOptions { * `process.env` state across invocations. When undefined, the env var decides. */ keepLocalValueSymbols?: boolean; + /** + * Extra fetch-wrapper function names to treat as HTTP consumers, threaded + * from `.gitnexusrc` `fetchWrappers` via `AnalyzeOptions` (#1589/#1852 + * residual). The routes phase unions these with the auto-detected `fetch()` + * wrappers when scanning for `route_map` consumers, so a wrapper named outside + * the built-in convention (or built on axios / a custom client) is still + * traced. Empty/undefined leaves behavior unchanged. + */ + fetchWrappers?: readonly string[]; } // ── Phase registry ───────────────────────────────────────────────────────── diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 9bc66b1699..ced261b2b6 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -159,6 +159,13 @@ export interface AnalyzeOptions { * removed); `undefined` defers to the env / auto-formula fallback. */ workerPoolSize?: number; + /** + * Extra fetch-wrapper function names to treat as HTTP consumers, forwarded to + * `PipelineOptions.fetchWrappers` (#1589/#1852 residual). Sourced from the CLI + * `.gitnexusrc` `fetchWrappers` list. `undefined`/empty leaves the route + * consumer scan unchanged. + */ + fetchWrappers?: string[]; } export interface AnalyzeResult { @@ -641,6 +648,7 @@ export async function runFullAnalysis( { parseCache, workerPoolSize: options.workerPoolSize, + fetchWrappers: options.fetchWrappers, }, ); diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index ebe7cf2848..2abfdfbc9f 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -128,6 +128,13 @@ export const VALID_RELATION_TYPES = new Set([ 'OVERRIDES', // Legacy alias — dual-read for pre-rename indexes 'METHOD_IMPLEMENTS', 'ACCESSES', + // Emitted by emit-references.ts / scope-resolution/graph-bridge/edges.ts and + // already part of the default impact relTypes + context() incoming queries. + // It was missing from this allowlist, so `impact({relationTypes:['USES']})` + // silently filtered to [] and fell back to the full default traversal + // (#2129/#1858 review F5). No IMPACT_RELATION_CONFIDENCE floor → 0.5 fallback, + // matching the FETCHES / WRAPS / HANDLES_ROUTE precedent below. + 'USES', 'HANDLES_ROUTE', 'FETCHES', 'HANDLES_TOOL', @@ -135,6 +142,20 @@ export const VALID_RELATION_TYPES = new Set([ 'WRAPS', ]); +/** + * Relation types the #1858 epistemic-boundary probe keys on. Kept as + * module-level `readonly` arrays (not Sets) because computeEpistemicBoundary + * binds them as Cypher query params (`r.type IN $heritage` / `IN $types`). + * The heritage set is exactly the IMPACT_RELATION_CONFIDENCE 0.85 tier — + * "statically verifiable, but the concrete binding past it is not". + */ +export const EPISTEMIC_HERITAGE_RELATION_TYPES: readonly string[] = [ + 'IMPLEMENTS', + 'METHOD_IMPLEMENTS', + 'EXTENDS', +]; +export const EPISTEMIC_CONSUMER_RELATION_TYPES: readonly string[] = ['CALLS', 'USES', 'ACCESSES']; + /** * Per-relation-type confidence floor for impact analysis. * @@ -2565,6 +2586,30 @@ export class LocalBackend { const symKind = isClassLike ? resolvedLabel || 'Class' : sym.type || sym[2]; const isMethodLike = symKind === 'Method' || symKind === 'Function' || symKind === 'Constructor'; + + // #1858 review F2 — start the epistemic boundary probe here (right after + // `symKind` is known) so it runs CONCURRENTLY with the methodMetadata fetch + // below, mirroring how _runImpactBFS overlaps it with the BFS. It is awaited + // at result assembly. (It cannot start earlier — `symKind` is only computed + // on this line, after the incoming/outgoing round-trips.) + // + // #1858 review F3 — pass an interface-preserving type, NOT `symKind`. + // `symKind` collapses a single-resolved Interface to 'Class' (resolvedLabel + // is '' on the single-candidate path), which would skip computeEpistemicBoundary's + // `symType === 'Interface'` self-boundary branch and under-report a leaf + // interface as 'exact'. `enrichCandidateLabels` runs BEFORE the single-candidate + // early return and patches `sym.type` from '' to 'Interface' (LadybugDB returns + // '' for labels()[0] on Interface/Class), so `sym.type` is the reliable signal + // here — mirroring impact()'s `resolvedLabel || symbol.type` derivation. Do not + // "fix" enrichment ordering; F3 depends on enrichment-before-early-return. + const epistemicSymType = (resolvedLabel || sym.type || symKind || '') as string; + const epistemicPromise = this.computeEpistemicBoundary( + repo, + symId, + epistemicSymType, + (sym.name || sym[1]) as string, + ); + let methodMetadata: Record | undefined; if (isMethodLike) { try { @@ -2597,6 +2642,13 @@ export class LocalBackend { } } + // #1858 — same epistemic boundary signal as impact(): when this symbol sits + // behind an interface / indirection boundary, callers binding via DI or + // dynamic dispatch are not reflected in `incoming`, so the view is a lower + // bound. Additive; never suppresses a field. Resolved from the probe started + // above (concurrent with methodMetadata). + const epistemic = await epistemicPromise; + return { status: 'found', symbol: { @@ -2609,6 +2661,7 @@ export class LocalBackend { ...(include_content && (sym.content || sym[6]) ? { content: sym.content || sym[6] } : {}), ...(methodMetadata ? { methodMetadata } : {}), }, + ...epistemic, incoming: categorize(incomingRows), outgoing: categorize(outgoingRows), ...(typedPropertyRows.length > 0 @@ -3261,21 +3314,121 @@ export class LocalBackend { } if (outcome.kind === 'ambiguous') { + // #2129 — a bare name that collides with several symbols must NOT report a + // bare `impactedCount: 0`. The real blast radius lives under whichever + // candidate the caller meant; a flat zero here is precisely the silent + // under-report the "run impact before editing" workflow exists to prevent + // (the dropped caller calls a *different* same-name node, so it never shows + // up against the one the resolver happened to pick). Run a bounded, + // summary-only BFS per candidate so each one's true count + risk is + // visible, and surface the maximum at the top level so the headline can + // never read as "safe to refactor". Candidates arrive sorted by score. + const AMBIGUOUS_MAX_CANDIDATES = 6; + const probed = outcome.candidates.slice(0, AMBIGUOUS_MAX_CANDIDATES); + // `partialProbe` is intentionally a SECOND incompleteness flag, distinct + // from the traversal-interrupted `partial` flag used elsewhere: it means + // one or more per-candidate probes threw, so maxRisk / maxImpactedCount + // are lower bounds over the probes that succeeded (a failed candidate must + // not be masked by a benign sibling success). + let probeFailed = false; + const candidateSummaries = await Promise.all( + probed.map(async (c) => { + const cType = c.type || ''; + const cRelTypes = + (cType === 'Class' || cType === 'Interface') && + !hasExplicitRelationTypes && + !relationTypes.includes('ACCESSES') + ? [...relationTypes, 'ACCESSES'] + : relationTypes; + // #1858/#2129 review F8 — name the shape the probe summary is read + // through (`_runImpactBFS` returns `Promise`, so this is the + // narrowing cast) so a future rename of those fields fails tsc instead + // of silently zeroing candidate counts. + let summary: { + impactedCount: number; + risk: string; + summary?: { direct: number }; + } | null = null; + try { + summary = await this._runImpactBFS( + repo, + { id: c.id, name: c.name, filePath: c.filePath }, + cType, + direction, + { + maxDepth, + relationTypes: cRelTypes, + includeTests, + minConfidence, + summaryOnly: true, + skipEpistemic: true, + skipEnrichment: true, + }, + ); + } catch (e) { + probeFailed = true; + logQueryError('impact:ambiguous-candidate', e); + } + return { + uid: c.id, + name: c.name, + kind: c.type, + filePath: c.filePath, + line: c.startLine, + score: Number(c.score.toFixed(2)), + impactedCount: summary?.impactedCount ?? 0, + risk: summary?.risk ?? 'UNKNOWN', + direct: summary?.summary?.direct ?? 0, + }; + }), + ); + + // Rank by blast radius so the most-impactful interpretation is first, and + // hoist the maximum count/risk to the top level so the response cannot be + // misread as "no impact". + candidateSummaries.sort((a, b) => b.impactedCount - a.impactedCount); + const maxImpactedCount = candidateSummaries.reduce((m, c) => Math.max(m, c.impactedCount), 0); + const RISK_ORDER = ['LOW', 'MEDIUM', 'HIGH', 'CRITICAL']; + // If EVERY candidate probe failed (all 'UNKNOWN' — e.g. pool exhaustion + // under the fan-out), the worst real risk is genuinely unknown, not LOW. + // Reporting LOW here would re-introduce the false-safe signal. Only fall to + // the LOW seed when at least one candidate produced a real risk. + const anyKnownRisk = candidateSummaries.some((c) => RISK_ORDER.includes(c.risk)); + const maxRisk = anyKnownRisk + ? candidateSummaries.reduce( + (worst, c) => (RISK_ORDER.indexOf(c.risk) > RISK_ORDER.indexOf(worst) ? c.risk : worst), + 'LOW', + ) + : 'UNKNOWN'; + const truncated = outcome.candidates.length > probed.length; + return { status: 'ambiguous', - message: `Found ${outcome.candidates.length} symbols matching '${target}'. Use target_uid, file_path, or kind to disambiguate.`, + message: + `Found ${outcome.candidates.length} symbols matching '${target}'` + + (truncated + ? ` (showing ${candidateSummaries.length} of ${outcome.candidates.length})` + : '') + + `. Blast radius differs per candidate (max ${maxImpactedCount} impacted at risk ${maxRisk}). ` + + `Disambiguate with target_uid (or file_path/kind) for a single authoritative result.`, target: { name: target }, direction, + // Full match count — `candidates[]` is truncated to AMBIGUOUS_MAX_CANDIDATES, + // so consumers (CLI formatter) need this to report "N of M" honestly (#2129 + // review F11; the CLI previously read the truncated array length). + totalCandidates: outcome.candidates.length, + // `impactedCount` stays 0 and `risk` stays UNKNOWN — there is no single + // resolved symbol, and UNKNOWN must NOT read as "safe to refactor". The + // real blast radius is surfaced per-candidate plus `maxImpactedCount` / + // `maxRisk` so a real caller can never hide behind the ambiguous zero + // (#2129). impactedCount: 0, risk: 'UNKNOWN', - candidates: outcome.candidates.map((c) => ({ - uid: c.id, - name: c.name, - kind: c.type, - filePath: c.filePath, - line: c.startLine, - score: Number(c.score.toFixed(2)), - })), + maxImpactedCount, + maxRisk, + ...(probeFailed ? { partialProbe: true } : {}), + ...(truncated && { candidatesTruncated: true }), + candidates: candidateSummaries, }; } @@ -3304,6 +3457,122 @@ export class LocalBackend { }); } + /** + * #1858 — epistemic lower-bound detection. + * + * impact()/context() traverse only edges materialized in the graph. When the + * queried symbol sits on an interface / abstract boundary, callers that bind + * to the interface via DI, a container, or dynamic dispatch — rather than + * naming the concrete symbol — are not traced. The reported count is then a + * lower bound, not an exact figure. Instead of returning a confident count + * that silently omits those callers, annotate the result with + * `epistemic: 'lower-bound'` plus a human-readable boundary note. A fully + * resolved leaf with no indirection stays `epistemic: 'exact'`. + * + * Aligns with the numeric confidence model rather than the long-deleted + * TIER_CONFIDENCE enum: the heritage/indirection edges this keys on + * (IMPLEMENTS / METHOD_IMPLEMENTS / EXTENDS) carry the 0.85 + * `IMPACT_RELATION_CONFIDENCE` floor — "statically verifiable, but the + * concrete binding past it is not". + * + * Never throws: on query error it returns 'exact', so it can only add signal, + * never suppress a result. + */ + private async computeEpistemicBoundary( + repo: RepoHandle, + symId: string, + symType: string, + symName: string, + ): Promise<{ epistemic: 'exact' | 'lower-bound'; boundaries?: string[] }> { + const HERITAGE_TYPES = EPISTEMIC_HERITAGE_RELATION_TYPES; + const CONSUMER_TYPES = EPISTEMIC_CONSUMER_RELATION_TYPES; + try { + // Discover the interface / abstract supertypes on the target's boundary. + // If the target is itself an interface, it is its own boundary node. + const boundary = new Map(); + if (symType === 'Interface') { + boundary.set(symId, { name: symName || '', label: 'Interface' }); + } + const ifaceRows = await executeParameterized( + repo.lbugPath, + `MATCH (x)-[r:CodeRelation]->(iface) + WHERE x.id = $symId AND r.type IN $heritage + RETURN DISTINCT iface.id AS id, iface.name AS name, labels(iface)[0] AS label + LIMIT 25`, + { symId, heritage: HERITAGE_TYPES }, + ).catch(() => []); + for (const r of ifaceRows) { + const id = (r.id ?? r[0]) as string; + if (id && !boundary.has(id)) { + boundary.set(id, { + name: (r.name ?? r[1] ?? '') as string, + label: (r.label ?? r[2] ?? 'Interface') as string, + }); + } + } + if (boundary.size === 0) return { epistemic: 'exact' }; + + const ifaceIds = Array.from(boundary.keys()); + // Count per interface id with scalar equality. A parameterized + // `iface.id IN $ids` combined with `COUNT(DISTINCT ...)` + implicit + // group-by returns no rows under the LadybugDB cypher subset, so query + // each boundary node individually (boundary is small — capped at 25). + const countByType = async (types: readonly string[]): Promise> => { + const m = new Map(); + await Promise.all( + ifaceIds.map(async (ifaceId) => { + const rows = await executeParameterized( + repo.lbugPath, + `MATCH (other)-[r:CodeRelation]->(iface) + WHERE iface.id = $ifaceId AND r.type IN $types + RETURN COUNT(DISTINCT other.id) AS cnt`, + { ifaceId, types }, + ).catch(() => []); + const cnt = + rows.length > 0 ? Number((rows[0] as any).cnt ?? (rows[0] as any)[0] ?? 0) : 0; + m.set(ifaceId, cnt); + }), + ); + return m; + }; + const [implCounts, consumerCounts] = await Promise.all([ + countByType(HERITAGE_TYPES), + countByType(CONSUMER_TYPES), + ]); + + const boundaries: string[] = []; + for (const [id, info] of boundary) { + const impls = implCounts.get(id) ?? 0; + const consumers = consumerCounts.get(id) ?? 0; + // Flag only a genuine indirection risk: an interface that is actually + // consumed (callers bind to it) or that has multiple implementations + // (runtime dispatch is ambiguous). A concrete type implementing an + // interface nothing references is fully traced → stays exact. + if (consumers >= 1 || impls >= 2) { + const label = (info.label || 'Interface').toLowerCase(); + const name = info.name || '(unnamed)'; + const article = /^[aeiou]/.test(label) ? 'an' : 'a'; + const parts: string[] = []; + if (impls >= 1) + parts.push(`${impls} ${impls === 1 ? 'implementation' : 'implementations'}`); + if (consumers >= 1) + parts.push( + `${consumers} interface-level ${consumers === 1 ? 'consumer' : 'consumers'}`, + ); + boundaries.push( + `${name} is ${article} ${label} with ${parts.join(' and ')}; callers that bind via the ${label} ` + + `(e.g. a DI container or dynamic dispatch) are not traced to the concrete symbol — ` + + `actual impact may be higher.`, + ); + } + } + if (boundaries.length === 0) return { epistemic: 'exact' }; + return { epistemic: 'lower-bound', boundaries }; + } catch { + return { epistemic: 'exact' }; + } + } + /** * Shared BFS traversal for impact analysis (name-resolved or UID-resolved symbol). */ @@ -3320,11 +3589,28 @@ export class LocalBackend { limit?: number; offset?: number; summaryOnly?: boolean; + // Enrichment/annotation suppression flags (#1858/#2129 review F6). Each + // suppresses a distinct sub-phase; they compose, and the real call sites are: + // - full impact()/context(): none set. + // - group cross-repo fan-out (impactByUid): skipPerSymbolEnrichment + + // skipEpistemic — the fan-out consumes only byDepth. + // - ambiguous #2129 per-candidate probe: skipEpistemic + skipEnrichment — + // needs only count + a count-based risk. + // skipPerSymbolEnrichment: drop the post-pagination per-symbol + // STEP_IN_PROCESS pass (keeps byDepth). + // skipEpistemic: skip the #1858 interface/indirection boundary probe. + // skipEnrichment: skip the process/module aggregation passes entirely; + // risk then derives from directCount/total only. NOTE: this also makes + // skipPerSymbolEnrichment a no-op (affectedProcesses stays empty), which + // is why the ambiguous probe sets only the two flags above. skipPerSymbolEnrichment?: boolean; + skipEpistemic?: boolean; + skipEnrichment?: boolean; }, ): Promise { const { maxDepth, relationTypes, includeTests, minConfidence } = opts; const skipPerSymbolEnrichment = opts.skipPerSymbolEnrichment ?? false; + const skipEnrichment = opts.skipEnrichment ?? false; const hasExplicitLimit = typeof opts.limit === 'number' && Number.isFinite(opts.limit); const paginationLimit = hasExplicitLimit ? Math.max(1, Math.min(Math.trunc(opts.limit!), 10000)) @@ -3344,6 +3630,22 @@ export class LocalBackend { const symId = sym.id || sym[0]; + // #1858 — kick off the epistemic boundary probe concurrently with the BFS. + // It depends only on symId/symType/symName (all known now) and touches no + // shared state, so its extra round-trip overlaps the traversal instead of + // adding to the serial path. `skipEpistemic` (ambiguous #2129 candidate + // probes, group fan-out) resolves to no field, preserving prior behavior. + // #1858/#2129 review F8 — the skip case adds no field, so `epistemic` is + // optional here (the union's `{}` subtype). computeEpistemicBoundary's own + // return keeps `epistemic` REQUIRED — only this promise widens to the skip + // subtype. + const epistemicPromise: Promise<{ + epistemic?: 'exact' | 'lower-bound'; + boundaries?: string[]; + }> = opts.skipEpistemic + ? Promise.resolve({}) + : this.computeEpistemicBoundary(repo, symId, symType, (sym.name || sym[1]) as string); + const impacted: any[] = []; const visited = new Set([symId]); let frontier = [symId]; @@ -3509,7 +3811,13 @@ export class LocalBackend { // Configurable via env IMPACT_MAX_CHUNKS, default 10 => max items = 1000 const MAX_CHUNKS = parseInt(process.env.IMPACT_MAX_CHUNKS || '10', 10); - if (impacted.length > 0) { + // `skipEnrichment` (ambiguous #2129 per-candidate probes) bypasses the + // process/module aggregation passes entirely — those probes need only the + // count + a count-based risk, so paying the bounded-but-real enrichment cost + // ~6× per ambiguous call is wasted. risk then derives from directCount / + // total only (processCount/moduleCount stay 0), an acceptable approximation + // for a disambiguation aid. + if (impacted.length > 0 && !skipEnrichment) { // ── Process enrichment: batched chunking (bounded by MAX_CHUNKS) ─ // Uses merged Cypher query (WITH + OPTIONAL MATCH) to fetch // process + entry point info in 1 round-trip per chunk. Converted to @@ -3776,6 +4084,10 @@ export class LocalBackend { byDepthCounts[Number(depth)] = items.length; } + // #1858 — await the epistemic boundary probe kicked off alongside the BFS + // above. Additive: leaves impactedCount and every existing field untouched. + const epistemic = await epistemicPromise; + const base = { target: { id: symId, @@ -3786,6 +4098,7 @@ export class LocalBackend { direction, impactedCount: impacted.length, risk, + ...epistemic, ...(!traversalComplete && { partial: true }), summary: { direct: directCount, @@ -3998,6 +4311,10 @@ export class LocalBackend { includeTests: opts.includeTests, minConfidence: opts.minConfidence, skipPerSymbolEnrichment: true, + // Group cross-repo fan-out consumes only byDepth (cross-impact.ts), not + // the #1858 epistemic/boundaries fields — computing them per neighbor is + // dead work on the highest-volume path, so suppress them here too. + skipEpistemic: true, }); } catch { return null; diff --git a/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/app/api/things/route.ts b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/app/api/things/route.ts new file mode 100644 index 0000000000..a75578a44a --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/app/api/things/route.ts @@ -0,0 +1,6 @@ +import { NextResponse } from 'next/server'; + +export async function GET() { + const things = [{ id: 1, name: 'Widget' }]; + return NextResponse.json(things); +} diff --git a/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/lib/http.ts b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/lib/http.ts new file mode 100644 index 0000000000..9d27dda222 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/lib/http.ts @@ -0,0 +1,11 @@ +import axios from 'axios'; + +const API_BASE = process.env.API_BASE || ''; + +// A custom HTTP wrapper built on axios — it never calls the bare global +// `fetch()`, so the parse-phase auto-detector cannot flag it. Listing +// "doRequest" in `.gitnexusrc` `fetchWrappers` is what lets the routes phase +// trace its consumers (#1589/#1852 residual). +export async function doRequest(path: string, opts?: Record) { + return axios.get(`${API_BASE}${path}`, opts); +} diff --git a/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/pages/Accented.tsx b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/pages/Accented.tsx new file mode 100644 index 0000000000..6b7d9140f6 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/pages/Accented.tsx @@ -0,0 +1,14 @@ +// `cafédoRequest` is a DIFFERENT function whose name ends in the configured +// wrapper `doRequest`, preceded by a non-ASCII letter. The consumer scan's +// left boundary must treat `é` as an identifier character (Unicode-aware) and +// NOT match `doRequest` here — otherwise this produces a spurious FETCHES edge +// to /api/things (#1852 review F10). +declare function cafédoRequest(path: string): Promise; + +export default function Accented() { + const load = async () => { + const res = await cafédoRequest('/api/things'); + return res; + }; + return null; +} diff --git a/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/pages/ThingsList.tsx b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/pages/ThingsList.tsx new file mode 100644 index 0000000000..e7ea40bf95 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/configurable-fetch-wrapper/pages/ThingsList.tsx @@ -0,0 +1,9 @@ +import { doRequest } from '../lib/http'; + +export default function ThingsList() { + const loadThings = async () => { + const res = await doRequest('/api/things'); + return res.data; + }; + return null; +} diff --git a/gitnexus/test/integration/impact-ambiguous-blast-radius.test.ts b/gitnexus/test/integration/impact-ambiguous-blast-radius.test.ts new file mode 100644 index 0000000000..0e5ee96677 --- /dev/null +++ b/gitnexus/test/integration/impact-ambiguous-blast-radius.test.ts @@ -0,0 +1,123 @@ +/** + * Integration test: impact() ambiguous-resolution blast radius (#2129) + * + * Reproduces the issue's graph shape: a small helper name (`classifyCard`) + * exists in two files. The "real" one is called by `syncContent` (+ another + * caller); a coincidental same-name helper elsewhere is called by `renderCard`. + * + * Before fix: impact("classifyCard", upstream) resolves the ambiguous bare name + * to `impactedCount: 0` with a flat candidate list — the real caller + * (`syncContent`) is silently dropped because it calls the *other* same-name + * node. After fix: the ambiguous response runs a bounded summary-only BFS per + * candidate, surfacing each one's true count + the maximum, so no real caller + * hides behind a bare zero. The BFS / edge storage are unchanged — disambiguation + * by uid still returns the exact caller. + */ +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 SYNC_LOGIC_ID = 'Function:src/sync-logic.ts:classifyCard'; +const UI_HELPERS_ID = 'Function:src/ui-helpers.ts:classifyCard'; + +const SEED = [ + // Two distinct functions named `classifyCard` in different files. + `CREATE (cc1:Function {id: '${SYNC_LOGIC_ID}', name: 'classifyCard', filePath: 'src/sync-logic.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (cc2:Function {id: '${UI_HELPERS_ID}', name: 'classifyCard', filePath: 'src/ui-helpers.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + + // Real callers of the sync-logic classifyCard (the blast radius that was lost). + `CREATE (sc:Function {id: 'Function:src/actions.ts:syncContent', name: 'syncContent', filePath: 'src/actions.ts', startLine: 10, endLine: 120, isExported: true, content: '', description: ''})`, + `CREATE (ss:Function {id: 'Function:src/actions.ts:scheduleSync', name: 'scheduleSync', filePath: 'src/actions.ts', startLine: 130, endLine: 160, isExported: true, content: '', description: ''})`, + // Caller of the coincidental ui-helpers classifyCard. + `CREATE (rc:Function {id: 'Function:src/ui-helpers.ts:renderCard', name: 'renderCard', filePath: 'src/ui-helpers.ts', startLine: 20, endLine: 40, isExported: true, content: '', description: ''})`, + + `MATCH (a:Function {id:'Function:src/actions.ts:syncContent'}), (b:Function {id:'${SYNC_LOGIC_ID}'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.85, reason:'direct', step:0}]->(b)`, + `MATCH (a:Function {id:'Function:src/actions.ts:scheduleSync'}), (b:Function {id:'${SYNC_LOGIC_ID}'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.85, reason:'direct', step:0}]->(b)`, + `MATCH (a:Function {id:'Function:src/ui-helpers.ts:renderCard'}), (b:Function {id:'${UI_HELPERS_ID}'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.85, reason:'direct', step:0}]->(b)`, +]; + +withTestLbugDB( + 'impact-ambiguous-blast-radius', + (handle) => { + let backend: LocalBackend; + beforeAll(() => { + backend = (handle as any)._backend; + }); + + it('surfaces per-candidate blast radius instead of a bare impactedCount:0', async () => { + const result = await backend.callTool('impact', { + target: 'classifyCard', + direction: 'upstream', + }); + + expect(result.status).toBe('ambiguous'); + expect(Array.isArray(result.candidates)).toBe(true); + expect(result.candidates).toHaveLength(2); + + // The fix: the maximum real blast radius is hoisted to the top level so + // the response can never be misread as "safe to refactor". + expect(result.maxImpactedCount).toBeGreaterThanOrEqual(2); + + // Each candidate carries its own true count — the dropped caller is no + // longer hidden behind the ambiguous zero. + const syncLogic = result.candidates.find((c: any) => + String(c.filePath).includes('sync-logic'), + ); + const uiHelpers = result.candidates.find((c: any) => + String(c.filePath).includes('ui-helpers'), + ); + expect(syncLogic).toBeDefined(); + expect(uiHelpers).toBeDefined(); + expect(syncLogic.impactedCount).toBeGreaterThanOrEqual(2); + expect(uiHelpers.impactedCount).toBeGreaterThanOrEqual(1); + + // Candidates are ranked by blast radius (most-impactful interpretation + // first) so the dangerous one leads. + expect(result.candidates[0].impactedCount).toBeGreaterThanOrEqual( + result.candidates[1].impactedCount, + ); + }); + + it('disambiguation by uid returns the exact dropped caller (BFS unchanged)', async () => { + const result = await backend.callTool('impact', { + target: 'classifyCard', + target_uid: SYNC_LOGIC_ID, + direction: 'upstream', + }); + + expect(result.status).not.toBe('ambiguous'); + expect(result.impactedCount).toBeGreaterThanOrEqual(2); + const names = Object.values(result.byDepth as Record) + .flat() + .map((d: any) => d.name); + expect(names).toContain('syncContent'); + expect(names).toContain('scheduleSync'); + }); + }, + { + 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: 5, nodes: 6, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/impact-epistemic-lower-bound.test.ts b/gitnexus/test/integration/impact-epistemic-lower-bound.test.ts new file mode 100644 index 0000000000..425364c497 --- /dev/null +++ b/gitnexus/test/integration/impact-epistemic-lower-bound.test.ts @@ -0,0 +1,123 @@ +/** + * Integration test: epistemic lower-bound flag (#1858) + * + * When a symbol sits behind an interface / indirection boundary, callers that + * bind via a DI container or dynamic dispatch are not traced to the concrete + * symbol — so impact()/context() report a *lower bound*, not an exact figure. + * Instead of a silent confident zero, the result is annotated + * `epistemic: 'lower-bound'` with a human-readable boundary note. A fully + * resolved leaf with no indirection stays `epistemic: 'exact'`. + * + * Graph shape (the canonical Symfony/DI case from #1858 / the #1589 comment): + * SignupController --CALLS--> Logger (interface) + * EmailLogger --IMPLEMENTS--> Logger + * FileLogger --IMPLEMENTS--> Logger + * The controller binds to the *interface*; the concrete impl is wired by the + * container, so impact("EmailLogger", upstream) finds no direct caller — but + * must flag that the true blast radius is higher. + */ +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 = [ + // Interface + two implementations + an interface-level consumer. + `CREATE (iface:Interface {id: 'Interface:src/Logger.ts:Logger', name: 'Logger', filePath: 'src/Logger.ts', startLine: 1, endLine: 5, isExported: true, content: '', description: ''})`, + `CREATE (email:Class {id: 'Class:src/EmailLogger.ts:EmailLogger', name: 'EmailLogger', filePath: 'src/EmailLogger.ts', startLine: 1, endLine: 20, isExported: true, content: '', description: ''})`, + `CREATE (file:Class {id: 'Class:src/FileLogger.ts:FileLogger', name: 'FileLogger', filePath: 'src/FileLogger.ts', startLine: 1, endLine: 20, isExported: true, content: '', description: ''})`, + `CREATE (ctrl:Class {id: 'Class:src/SignupController.ts:SignupController', name: 'SignupController', filePath: 'src/SignupController.ts', startLine: 1, endLine: 30, isExported: true, content: '', description: ''})`, + + `MATCH (a:Class {id:'Class:src/EmailLogger.ts:EmailLogger'}), (b:Interface {id:'Interface:src/Logger.ts:Logger'}) CREATE (a)-[:CodeRelation {type:'IMPLEMENTS', confidence:0.85, reason:'implements', step:0}]->(b)`, + `MATCH (a:Class {id:'Class:src/FileLogger.ts:FileLogger'}), (b:Interface {id:'Interface:src/Logger.ts:Logger'}) CREATE (a)-[:CodeRelation {type:'IMPLEMENTS', confidence:0.85, reason:'implements', step:0}]->(b)`, + `MATCH (a:Class {id:'Class:src/SignupController.ts:SignupController'}), (b:Interface {id:'Interface:src/Logger.ts:Logger'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.85, reason:'interface-call', step:0}]->(b)`, + + // A fully-resolved leaf with no indirection — must stay `exact`. + `CREATE (leaf:Function {id: 'Function:src/util.ts:formatDate', name: 'formatDate', filePath: 'src/util.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + `CREATE (caller:Function {id: 'Function:src/page.ts:renderHeader', name: 'renderHeader', filePath: 'src/page.ts', startLine: 1, endLine: 10, isExported: true, content: '', description: ''})`, + `MATCH (a:Function {id:'Function:src/page.ts:renderHeader'}), (b:Function {id:'Function:src/util.ts:formatDate'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:0.9, reason:'direct', step:0}]->(b)`, +]; + +withTestLbugDB( + 'impact-epistemic-lower-bound', + (handle) => { + let backend: LocalBackend; + beforeAll(() => { + backend = (handle as any)._backend; + }); + + it('flags impact() on a concrete impl behind an interface as lower-bound', async () => { + const result = await backend.callTool('impact', { + target: 'EmailLogger', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.epistemic).toBe('lower-bound'); + expect(Array.isArray(result.boundaries)).toBe(true); + expect(result.boundaries.join(' ')).toContain('Logger'); + }); + + it('flags impact() on the interface itself as lower-bound', async () => { + const result = await backend.callTool('impact', { + target: 'Logger', + direction: 'upstream', + }); + expect(result.epistemic).toBe('lower-bound'); + }); + + it('keeps a fully-resolved leaf exact (no false boundary)', async () => { + const result = await backend.callTool('impact', { + target: 'formatDate', + direction: 'upstream', + }); + expect(result.epistemic).toBe('exact'); + expect(result.boundaries).toBeUndefined(); + // The real caller is still reported — the flag is additive, not lossy. + expect(result.impactedCount).toBeGreaterThanOrEqual(1); + }); + + it('context() carries the same epistemic signal', async () => { + const result = await backend.callTool('context', { + name: 'EmailLogger', + file_path: 'src/EmailLogger.ts', + }); + expect(result.status).toBe('found'); + expect(result.epistemic).toBe('lower-bound'); + }); + + it('context() on a leaf interface itself is lower-bound (#1858 review F3)', async () => { + // Logger is a leaf interface — it implements/extends nothing, so the only + // boundary signal is computeEpistemicBoundary's symType==='Interface' + // self-branch. Before the F3 fix, context() collapsed symKind to 'Class' + // and this returned 'exact'. + const result = await backend.callTool('context', { name: 'Logger' }); + expect(result.status).toBe('found'); + expect(result.epistemic).toBe('lower-bound'); + }); + }, + { + 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: 6, nodes: 6, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as any)._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/resolvers/configurable-fetch-wrapper.test.ts b/gitnexus/test/integration/resolvers/configurable-fetch-wrapper.test.ts new file mode 100644 index 0000000000..e9730e7eb9 --- /dev/null +++ b/gitnexus/test/integration/resolvers/configurable-fetch-wrapper.test.ts @@ -0,0 +1,54 @@ +/** + * Integration test: configurable fetch wrappers (#1589/#1852 residual) + * + * The parse-phase auto-detector only flags functions that call the bare global + * `fetch()`. A wrapper built on axios / a custom client — like `doRequest` in + * this fixture — is invisible to it, so `route_map` silently reports + * `consumers: []` (exactly the "named outside convention" hole #1858 calls out). + * + * Declaring the wrapper name in `.gitnexusrc` `fetchWrappers` (threaded here via + * PipelineOptions) lets the routes-phase consumer scan trace it, producing the + * FETCHES edge. The control run (no config) proves the gap; the configured run + * proves the fix. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import path from 'path'; +import { FIXTURES, getRelationships, runPipelineFromRepo, type PipelineResult } from './helpers.js'; + +const FIXTURE = path.join(FIXTURES, 'configurable-fetch-wrapper'); + +describe('Configurable fetch wrapper consumer extraction', () => { + let withConfig: PipelineResult; + let withoutConfig: PipelineResult; + + beforeAll(async () => { + withoutConfig = await runPipelineFromRepo(FIXTURE, () => {}); + withConfig = await runPipelineFromRepo(FIXTURE, () => {}, { + fetchWrappers: ['doRequest'], + }); + }, 60000); + + it('does NOT trace an axios-based wrapper without configuration (the gap)', () => { + const edges = getRelationships(withoutConfig, 'FETCHES'); + const thingsEdge = edges.find((e) => e.target === '/api/things'); + expect(thingsEdge).toBeUndefined(); + }); + + it('traces the configured wrapper as a route consumer', () => { + const edges = getRelationships(withConfig, 'FETCHES'); + const thingsEdge = edges.find( + (e) => e.sourceFilePath.includes('ThingsList') && e.target === '/api/things', + ); + expect(thingsEdge).toBeDefined(); + }); + + it('does NOT match a configured bare name inside a longer non-ASCII identifier (#1852 review F10)', () => { + // Accented.tsx calls `cafédoRequest('/api/things')` — `doRequest` preceded by + // the non-ASCII letter `é`. The Unicode-aware lookbehind must reject it. + const edges = getRelationships(withConfig, 'FETCHES'); + const spurious = edges.find( + (e) => e.sourceFilePath.includes('Accented') && e.target === '/api/things', + ); + expect(spurious).toBeUndefined(); + }); +}); diff --git a/gitnexus/test/unit/analyze-config.test.ts b/gitnexus/test/unit/analyze-config.test.ts index 07bbdbea18..0c48628cf4 100644 --- a/gitnexus/test/unit/analyze-config.test.ts +++ b/gitnexus/test/unit/analyze-config.test.ts @@ -137,6 +137,28 @@ describe('analyze-config (.gitnexusrc support, #243)', () => { expect(() => loadAnalyzeConfig(dir)).toThrow(/control or hidden/); }); + // ── fetchWrappers (#1589/#1852 residual) ─────────────────────────── + + it('normalizes a fetchWrappers string array (de-duped)', async () => { + await writeRc(JSON.stringify({ fetchWrappers: ['doRequest', 'apiClient.get', 'doRequest'] })); + expect(loadAnalyzeConfig(dir)).toEqual({ fetchWrappers: ['doRequest', 'apiClient.get'] }); + }); + + it('rejects a non-array fetchWrappers value', async () => { + await writeRc(JSON.stringify({ fetchWrappers: 'doRequest' })); + expect(() => loadAnalyzeConfig(dir)).toThrow(/must be an array of strings/); + }); + + it('rejects a fetchWrappers entry with regex / non-identifier characters', async () => { + await writeRc(JSON.stringify({ fetchWrappers: ['do(.*)Request'] })); + expect(() => loadAnalyzeConfig(dir)).toThrow(/must be an identifier or member name/); + }); + + it('rejects an empty fetchWrappers array', async () => { + await writeRc(JSON.stringify({ fetchWrappers: [] })); + expect(() => loadAnalyzeConfig(dir)).toThrow(/at least one string/); + }); + // ── validateBranchName ───────────────────────────────────────────── it('validateBranchName trims and accepts normal branch names', () => { diff --git a/gitnexus/test/unit/eval-formatters.test.ts b/gitnexus/test/unit/eval-formatters.test.ts index 1ef5c950b2..4dab8c634d 100644 --- a/gitnexus/test/unit/eval-formatters.test.ts +++ b/gitnexus/test/unit/eval-formatters.test.ts @@ -213,6 +213,162 @@ describe('formatImpactResult', () => { expect(formatImpactResult({ error: 'bad request' })).toContain('Error: bad request'); }); + it('surfaces per-candidate blast radius for an ambiguous result, never the "isolated" headline (#2129)', () => { + const result = formatImpactResult({ + status: 'ambiguous', + target: { name: 'classifyCard' }, + direction: 'upstream', + impactedCount: 0, + risk: 'UNKNOWN', + maxImpactedCount: 3, + maxRisk: 'MEDIUM', + candidates: [ + { + uid: 'Function:src/sync-logic.ts:classifyCard', + name: 'classifyCard', + kind: 'Function', + filePath: 'src/sync-logic.ts', + line: 1, + impactedCount: 3, + risk: 'MEDIUM', + }, + { + uid: 'Function:src/ui-helpers.ts:classifyCard', + name: 'classifyCard', + kind: 'Function', + filePath: 'src/ui-helpers.ts', + line: 1, + impactedCount: 1, + risk: 'LOW', + }, + ], + }); + // Must NOT print the false-safe "isolated" headline. + expect(result).not.toContain('isolated'); + expect(result).toContain('AMBIGUOUS'); + expect(result).toContain('Max blast radius 3'); + // Both candidates + their real counts are visible. + expect(result).toContain('src/sync-logic.ts'); + expect(result).toContain('[3 upstream'); + expect(result).toContain('--uid'); + // No probe failed here → no lower-bound warning. + expect(result).not.toContain('candidate probes failed'); + }); + + it('warns that the max is a lower bound when a candidate probe failed (#2129 review F1)', () => { + const result = formatImpactResult({ + status: 'ambiguous', + target: { name: 'classifyCard' }, + direction: 'upstream', + impactedCount: 0, + risk: 'UNKNOWN', + maxImpactedCount: 2, + maxRisk: 'LOW', + partialProbe: true, + candidates: [ + { + uid: 'A', + name: 'classifyCard', + kind: 'Function', + filePath: 'src/a.ts', + line: 1, + impactedCount: 2, + risk: 'LOW', + }, + { + uid: 'B', + name: 'classifyCard', + kind: 'Function', + filePath: 'src/b.ts', + line: 1, + impactedCount: 0, + risk: 'UNKNOWN', + }, + ], + }); + expect(result).not.toContain('isolated'); + expect(result).toContain('candidate probes failed'); + expect(result).toContain('lower bound'); + // The honest max is still shown. + expect(result).toContain('Max blast radius 2'); + }); + + it('reports the full match count when the candidate list is truncated (#2129 review F11)', () => { + const result = formatImpactResult({ + status: 'ambiguous', + target: { name: 'handle' }, + direction: 'upstream', + impactedCount: 0, + risk: 'UNKNOWN', + maxImpactedCount: 5, + maxRisk: 'HIGH', + totalCandidates: 9, + candidatesTruncated: true, + candidates: Array.from({ length: 6 }, (_, i) => ({ + uid: `U${i}`, + name: 'handle', + kind: 'Function', + filePath: `src/h${i}.ts`, + line: 1, + impactedCount: i, + risk: 'LOW', + })), + }); + // Full count (9), not the truncated array length (6). + expect(result).toContain('9 symbols'); + expect(result).toContain('showing 6'); + }); + + it('shows a plain count when the candidate list is not truncated', () => { + const result = formatImpactResult({ + status: 'ambiguous', + target: { name: 'foo' }, + direction: 'upstream', + impactedCount: 0, + risk: 'UNKNOWN', + maxImpactedCount: 1, + maxRisk: 'LOW', + totalCandidates: 2, + candidates: [ + { + uid: 'A', + name: 'foo', + kind: 'Function', + filePath: 'src/a.ts', + line: 1, + impactedCount: 1, + risk: 'LOW', + }, + { + uid: 'B', + name: 'foo', + kind: 'Function', + filePath: 'src/b.ts', + line: 1, + impactedCount: 0, + risk: 'LOW', + }, + ], + }); + expect(result).toContain('2 symbols'); + expect(result).not.toContain('showing'); + }); + + it('surfaces the lower-bound boundary note when epistemic is lower-bound (#1858)', () => { + const result = formatImpactResult({ + target: { kind: 'Class', name: 'EmailLogger' }, + direction: 'upstream', + impactedCount: 0, + risk: 'LOW', + epistemic: 'lower-bound', + boundaries: ['Logger is an interface with 2 implementations; callers bind via DI.'], + byDepth: {}, + }); + expect(result).not.toContain('isolated'); + expect(result.toLowerCase()).toContain('lower bound'); + expect(result).toContain('Logger is an interface'); + }); + it('returns error with suggestion when provided', () => { const result = formatImpactResult({ error: 'Impact analysis failed', diff --git a/gitnexus/test/unit/impact-pagination.test.ts b/gitnexus/test/unit/impact-pagination.test.ts index 163159240e..f1a942f328 100644 --- a/gitnexus/test/unit/impact-pagination.test.ts +++ b/gitnexus/test/unit/impact-pagination.test.ts @@ -56,6 +56,11 @@ function setupMultiDepthHub(d1Count: number, d2Count: number) { const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); if (query.includes('STEP_IN_PROCESS')) return []; if (query.includes('MEMBER_OF')) return []; + // The #1858 epistemic-boundary probe (computeEpistemicBoundary) runs + // concurrently with the BFS and also matches `r.type IN`, but targets the + // `iface` alias. Return empty so it stays `epistemic: 'exact'` and does not + // consume a depth slot from the frontier counter below. + if (query.includes('iface')) return []; if (query.includes('r.type IN')) { depth++; const count = depth === 1 ? d1Count : depth === 2 ? d2Count : 0; @@ -82,6 +87,9 @@ function setupHubSymbol(count: number) { const query = typeof args[1] === 'string' ? args[1] : String(args[0] ?? ''); if (query.includes('STEP_IN_PROCESS')) return []; if (query.includes('MEMBER_OF')) return []; + // See setupMultiDepthHub — keep the #1858 epistemic probe from matching the + // `r.type IN` caller branch below. + if (query.includes('iface')) return []; if (query.includes('r.type IN')) { const res: any[] = []; for (let i = 0; i < count; i++) { diff --git a/gitnexus/test/unit/security.test.ts b/gitnexus/test/unit/security.test.ts index 44a2f33e70..0474fbf220 100644 --- a/gitnexus/test/unit/security.test.ts +++ b/gitnexus/test/unit/security.test.ts @@ -17,7 +17,7 @@ import { describe('VALID_RELATION_TYPES', () => { it('contains all expected relation types', () => { - expect(VALID_RELATION_TYPES.size).toBe(15); + expect(VALID_RELATION_TYPES.size).toBe(16); for (const t of [ 'CALLS', 'IMPORTS', @@ -29,6 +29,9 @@ describe('VALID_RELATION_TYPES', () => { 'OVERRIDES', 'METHOD_IMPLEMENTS', 'ACCESSES', + // USES is an emitted edge type (emit-references.ts) used in the default + // impact relTypes + context queries; added to the allowlist in F5. + 'USES', 'HANDLES_ROUTE', 'FETCHES', 'HANDLES_TOOL', @@ -41,7 +44,6 @@ describe('VALID_RELATION_TYPES', () => { it('rejects invalid relation types', () => { expect(VALID_RELATION_TYPES.has('CONTAINS')).toBe(false); - expect(VALID_RELATION_TYPES.has('USES')).toBe(false); expect(VALID_RELATION_TYPES.has('calls')).toBe(false); // case-sensitive expect(VALID_RELATION_TYPES.has('DROP_TABLE')).toBe(false); });