From 50aa4be3b2c2c9a1561fc44878e5d8f87b99d16e Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Thu, 8 Oct 2026 21:53:55 +0100 Subject: [PATCH] fix(routes): prefer production handlers and preserve app.all (#3505) --- .../bench/receiver-resolution/baseline.json | 12 +- gitnexus/src/core/ingestion/call-processor.ts | 66 ++-- .../ingestion/pipeline-phases/parse-impl.ts | 4 + .../core/ingestion/pipeline-phases/parse.ts | 2 + .../core/ingestion/pipeline-phases/routes.ts | 15 +- .../ingestion/route-extractors/route-path.ts | 14 +- .../core/ingestion/workers/parse-worker.ts | 64 +++- gitnexus/src/storage/parse-cache.ts | 9 +- .../express-route-priority/spec/stubs.ts | 10 + .../express-route-priority/src/server.ts | 24 ++ .../resolvers/express-routes.test.ts | 173 +++++++++ .../test/unit/incremental-parse-cache.test.ts | 11 +- .../test/unit/route-source-priority.test.ts | 354 ++++++++++++++++++ 13 files changed, 698 insertions(+), 60 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/express-route-priority/spec/stubs.ts create mode 100644 gitnexus/test/fixtures/lang-resolution/express-route-priority/src/server.ts create mode 100644 gitnexus/test/unit/route-source-priority.test.ts diff --git a/gitnexus/bench/receiver-resolution/baseline.json b/gitnexus/bench/receiver-resolution/baseline.json index b234edc46..f40307044 100644 --- a/gitnexus/bench/receiver-resolution/baseline.json +++ b/gitnexus/bench/receiver-resolution/baseline.json @@ -199,19 +199,19 @@ } }, "countArm": { - "callDrops": 124, - "totalDropsAllKinds": 170, + "callDrops": 132, + "totalDropsAllKinds": 178, "bySiteKind": { - "call": 124, + "call": 132, "read": 27, "write": 19 }, "callDropsByExtension": { ".java": 49, ".py": 16, + ".ts": 15, ".zig": 11, ".cs": 8, - ".ts": 7, ".cpp": 7, ".tsx": 6, ".go": 5, @@ -224,14 +224,14 @@ }, "callDropsByShape": { "chain-field": 60, - "chain-call": 27, + "chain-call": 35, "no-chain": 23, "<>": 11, "chain-mixed": 2, "chain-unwrap": 1 }, "callDropsByOrigin": { - "in-program": 54, + "in-program": 62, "external": 44, "unknown": 26 } diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index 8a3eef1cd..161b55c0e 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -25,12 +25,14 @@ import type { ExtractedDecoratorRoute } from './workers/parse-worker.js'; import type { LanguageProvider } from './language-provider.js'; import { normalizeFetchURL, routeMatches } from './route-extractors/nextjs.js'; import { + isTestRouteFile, normalizeExtractedRoutePath, normalizeRouteMethod, routeNodeKey, } from './route-extractors/route-path.js'; import { extractReturnTypeName } from './type-extractors/shared.js'; import { DATA_ROUTE_TABLE_SOURCE } from './route-extractors/data-route-table.js'; +import { reconcileDispatchGuardRoutes } from './route-extractors/dispatch-guard.js'; import { toZeroBasedLine } from './utils/line-base.js'; const MAX_EXPORTS_PER_FILE = 500; @@ -356,12 +358,12 @@ export const processRoutesFromExtracted = async ( * `handlerName` (the decorated method, captured at extraction); resolve it * directly in the route's own file. * - * First-writer-wins per route identity, matching the routes phase's dedup (it - * keeps the first route registered for a `(method, url)` key and counts the rest - * as duplicates). The first route to claim a key reserves it **even when its - * handler is unresolvable**, so a later same-key route can never stamp its - * handler onto the first route's Route node (the routes phase made that first - * route the node-winner). Keying is `routeNodeKey(method, url)` (#2289): a + * Production declarations take precedence over test declarations; equal-priority + * candidates remain first-writer-wins. The selected declaration reserves its key **even when its + * handler is unresolvable**, so a losing route can never donate its handler. + * When supplied, `selectedRoutes` receives the admitted winners for the routes + * phase, keeping node provenance and handler provenance identical. + * Keying is `routeNodeKey(method, url)` (#2289): a * same-URL multi-verb pair (`GET /x` + `POST /x`) resolves two handlers, one per * node; method-less / wildcard routes key by URL alone, byte-identical to the * pre-#2289 behavior. Routes whose handler cannot be *uniquely* resolved (no @@ -374,12 +376,11 @@ export function resolveRouteHandlerSymbols( extractedRoutes: readonly ExtractedRoute[], decoratorRoutes: readonly ExtractedDecoratorRoute[], routeContext?: RouteHandlerResolutionContext, + selectedRoutes?: Set, ): Map { const out = new Map(); - // Route identities already claimed by an earlier route (resolved or not). - // Mirrors the routes phase `addRoute` first-writer-wins so the handler we - // stamp always belongs to the route that actually won the Route node. - const claimed = new Set(); + const claimed = new Map(); + const admittedDecoratorRoutes = reconcileDispatchGuardRoutes(decoratorRoutes); // Resolve a single same-file symbol by name. Exactly one match → its nodeId. // Zero or many matches → undefined (fail-open). tRPC same-name handlers @@ -480,24 +481,26 @@ export function resolveRouteHandlerSymbols( : uniqueById(model.methods.lookupAllByOwner(owner.nodeId, member))?.nodeId; }; - const claim = ( - routePath: string | null, - prefix: string | null, - httpMethod: string | null | undefined, - symbolId: string | undefined, - ) => { + const claim = (route: ExtractedRoute | ExtractedDecoratorRoute, symbolId: string | undefined) => { // An empty path is a valid, pathless mapping and normalizes to either `/` // or its class/router prefix. Only null means the extractor had no route. - if (routePath === null) return; - const url = normalizeExtractedRoutePath(routePath, prefix); - const key = routeNodeKey(normalizeRouteMethod(httpMethod), url); - if (claimed.has(key)) return; // first-writer-wins: later same-key routes can't override - claimed.add(key); + if (route.routePath === null) return; + const url = normalizeExtractedRoutePath(route.routePath, route.prefix ?? null); + const key = routeNodeKey(normalizeRouteMethod(route.httpMethod), url); + const previous = claimed.get(key); + if (previous && (!isTestRouteFile(previous.filePath) || isTestRouteFile(route.filePath))) + return; + if (previous) selectedRoutes?.delete(previous); + claimed.set(key, route); + selectedRoutes?.add(route); + out.delete(key); if (symbolId) out.set(key, symbolId); }; // Laravel framework routes — controller class + method name. for (const route of extractedRoutes) { + // Match the routes phase's admission rule for extracted declarations. + if (!route.routePath) continue; let methodId: string | undefined; if (route.controllerName && route.methodName) { let controllerDef: SymbolDefinition | undefined; @@ -516,7 +519,7 @@ export function resolveRouteHandlerSymbols( routeContext?.nodeStartLine, )?.nodeId; } - claim(route.routePath, route.prefix ?? null, route.httpMethod, methodId); + claim(route, methodId); } const dataHandlerByRoute = new Map(); @@ -524,11 +527,16 @@ export function resolveRouteHandlerSymbols( string, { handlers: Set; hasUnresolved: boolean } >(); - for (const dr of decoratorRoutes) { + const dataIdentity = (dr: ExtractedDecoratorRoute): string => { + const url = normalizeExtractedRoutePath(dr.routePath, dr.prefix ?? null); + return `${Number(isTestRouteFile(dr.filePath))}:${routeNodeKey(normalizeRouteMethod(dr.httpMethod), url)}`; + }; + for (const dr of admittedDecoratorRoutes) { if (dr.source !== DATA_ROUTE_TABLE_SOURCE || !dr.handlerName || !dr.routePath) continue; const handlerId = resolveDataRouteHandler(dr.filePath, dr.handlerName); - const url = normalizeExtractedRoutePath(dr.routePath, dr.prefix ?? null); - const key = routeNodeKey(normalizeRouteMethod(dr.httpMethod), url); + // Conflicting test handlers must not invalidate a production table. + // Ambiguity is still rejected within each priority tier. + const key = dataIdentity(dr); const state = dataHandlersByIdentity.get(key) ?? { handlers: new Set(), hasUnresolved: false, @@ -563,18 +571,16 @@ export function resolveRouteHandlerSymbols( // handlers. Data tables additionally suppress an identity when duplicate // entries resolve to different handlers: recording either one would invent a // single-winner dispatch that the loop does not prove. - for (const dr of decoratorRoutes) { + for (const dr of admittedDecoratorRoutes) { const handlerId = decoratorHandlerId(dr); // An unproven data-table entry never becomes a Route node, so it must not // reserve the identity and suppress a later, valid framework declaration. if (dr.source === DATA_ROUTE_TABLE_SOURCE && handlerId === undefined) continue; if (dr.source === DATA_ROUTE_TABLE_SOURCE && dr.routePath) { - const url = normalizeExtractedRoutePath(dr.routePath, dr.prefix ?? null); - const key = routeNodeKey(normalizeRouteMethod(dr.httpMethod), url); - const state = dataHandlersByIdentity.get(key); + const state = dataHandlersByIdentity.get(dataIdentity(dr)); if (state === undefined || state.hasUnresolved || state.handlers.size !== 1) continue; } - claim(dr.routePath, dr.prefix ?? null, dr.httpMethod, handlerId); + claim(dr, handlerId); } return out; diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index 600ee3b91..cc72a04d5 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -458,6 +458,7 @@ export async function runChunkedParseAndResolve( allFetchWrapperDefs: FetchWrapperDef[]; allExtractedRoutes: ExtractedRoute[]; allDecoratorRoutes: ExtractedDecoratorRoute[]; + selectedRoutes: ReadonlySet; allToolDefs: ExtractedToolDef[]; allORMQueries: ExtractedORMQuery[]; bindingAccumulator: BindingAccumulator; @@ -1932,6 +1933,7 @@ export async function runChunkedParseAndResolve( ), }; }); + const selectedRoutes = new Set(); const routeHandlerSymbols = resolveRouteHandlerSymbols( model, allExtractedRoutes, @@ -1947,6 +1949,7 @@ export async function runChunkedParseAndResolve( return typeof n?.properties.startLine === 'number' ? n.properties.startLine : undefined; }, }, + selectedRoutes, ); return { exportedTypeMap, @@ -1954,6 +1957,7 @@ export async function runChunkedParseAndResolve( allFetchWrapperDefs, allExtractedRoutes, allDecoratorRoutes, + selectedRoutes, allToolDefs, allORMQueries, bindingAccumulator, diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts index 29c62777b..0c4946afd 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts @@ -50,6 +50,8 @@ export interface ParseOutput { readonly allFetchWrapperDefs: readonly FetchWrapperDef[]; readonly allExtractedRoutes: readonly ExtractedRoute[]; readonly allDecoratorRoutes: readonly ExtractedDecoratorRoute[]; + /** Exact admitted declarations that own Route nodes and handler attribution. */ + readonly selectedRoutes: ReadonlySet; readonly allToolDefs: readonly ExtractedToolDef[]; readonly allORMQueries: readonly ExtractedORMQuery[]; /** Route URL → resolved handler symbol UID (Part 2, #2138). Consumed by the diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index cc1c48488..0d526bb5c 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -180,6 +180,7 @@ export const routesPhase: PipelinePhase = { allExtractedRoutes, allDecoratorRoutes, routeHandlerSymbols, + selectedRoutes, } = getPhaseOutput(deps, 'parse'); // Local copy — routes phase must not mutate upstream ParseOutput @@ -295,11 +296,14 @@ export const routesPhase: PipelinePhase = { for (const route of allExtractedRoutes) { if (!route.routePath) continue; const routeUrl = normalizeExtractedRoutePath(route.routePath, route.prefix); - addRoute(routeUrl, { - filePath: route.filePath, - source: 'framework-route', - method: normalizeRouteMethod(route.httpMethod), - }); + if (!selectedRoutes || selectedRoutes.has(route)) { + addRoute(routeUrl, { + filePath: route.filePath, + source: 'framework-route', + method: normalizeRouteMethod(route.httpMethod), + }); + } + // Losing declarations may still provide distinct named aliases. if (route.routeName && !namedRouteRegistry.has(route.routeName)) { namedRouteRegistry.set(route.routeName, routeUrl); } @@ -309,6 +313,7 @@ export const routesPhase: PipelinePhase = { // idiom, which no single file can reconcile. Framework routes are untouched; // their verb-less form is a declaration, not a weaker observation. for (const dr of reconcileDispatchGuardRoutes(allDecoratorRoutes)) { + if (selectedRoutes && !selectedRoutes.has(dr)) continue; const url = normalizeExtractedRoutePath(dr.routePath, dr.prefix ?? null); const method = normalizeRouteMethod(dr.httpMethod); const routeKey = routeNodeKey(method, url); diff --git a/gitnexus/src/core/ingestion/route-extractors/route-path.ts b/gitnexus/src/core/ingestion/route-extractors/route-path.ts index 2aa55f846..f2844de2d 100644 --- a/gitnexus/src/core/ingestion/route-extractors/route-path.ts +++ b/gitnexus/src/core/ingestion/route-extractors/route-path.ts @@ -1,12 +1,14 @@ +import { isTestFilePath } from '../utils/test-file-path.js'; + /** - * Shared route-path normalization. + * Shared route-path normalization, route identity, and test-file classification. * * Extracted from the routes phase so both the routes phase (which creates the * `Route` graph node, keyed by `(method, url)` via `routeNodeKey` — #2289) and * the parse phase (which resolves each route's handler symbol and needs the * SAME key to associate the resolved id back to the route) can compute an - * identical route identity without a phase-to-phase import cycle. Pure string - * logic, no dependencies. + * identical route identity without a phase-to-phase import cycle. Test-route + * classification reuses the shared test-file path classifier. */ /** @@ -66,3 +68,9 @@ export function normalizeRouteMethod(raw: string | null | undefined): string | u export function routeNodeKey(method: string | undefined, url: string): string { return method && method !== '*' ? `${method} ${url}` : url; } + +/** Keep test registrations available, but let production files win duplicate route identities. */ +export function isTestRouteFile(filePath: string): boolean { + const normalized = filePath.replace(/\\/g, '/'); + return isTestFilePath(normalized) || /(^|\/)e2e(\/|$)/i.test(normalized); +} diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 18b65eba1..bbefe5e2b 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -124,7 +124,11 @@ import { type SyntaxNode, } from '../utils/ast-helpers.js'; import { isPositionQualifiedLocalLabel } from '../utils/callable-labels.js'; -import { extractCallArgTypes, type MixedChainStep } from '../utils/call-analysis.js'; +import { + countCallArguments, + extractCallArgTypes, + type MixedChainStep, +} from '../utils/call-analysis.js'; import { buildTypeEnv } from '../type-env.js'; import type { ConstructorBinding } from '../type-env.js'; import { detectFrameworkFromAST } from '../framework-detection.js'; @@ -2042,6 +2046,10 @@ const processFileGroup = ( // HTTP client calls like axios.get('/api/users') that match the same pattern // as Express route registrations. const callNode = captureMap['express_route']; + // route(path) returns a builder; verb registrations need a handler. + // Count arguments without comments, which are also named AST children. + const minimumArguments = method === 'route' ? 1 : 2; + if ((countCallArguments(callNode) ?? 0) < minimumArguments) continue; const funcNode = callNode.childForFieldName?.('function') ?? callNode.children?.[0]; // Walk through nested member_expressions and call_expressions to // reach the innermost receiver identifier. Handles chains like: @@ -2079,17 +2087,49 @@ const processFileGroup = ( continue; } - const httpMethod = - method === 'all' || method === 'use' || method === 'route' - ? 'GET' - : method.toUpperCase(); - result.decoratorRoutes.push({ - filePath: file.path, - routePath, - httpMethod, - decoratorName: `express.${method}`, - lineNumber: captureMap['express_route'].startPosition.row + lineOffset, - }); + const registrations: { method: string; node: SyntaxNode }[] = []; + if (method === 'route') { + // A builder alone registers nothing. Follow only direct chained + // verb calls, each of which must supply a non-comment handler arg. + let builderNode = callNode; + while (builderNode.parent?.type === 'member_expression') { + const member = builderNode.parent; + const registration = member.parent; + const verb = member.childForFieldName('property')?.text; + if ( + member.childForFieldName('object')?.id !== builderNode.id || + registration?.type !== 'call_expression' || + registration.childForFieldName('function')?.id !== member.id || + !verb || + !EXPRESS_ROUTE_METHODS.has(verb) || + verb === 'route' || + verb === 'use' + ) { + break; + } + if ((countCallArguments(registration) ?? 0) >= 1) { + registrations.push({ method: verb, node: registration }); + } + builderNode = registration; + } + } else { + registrations.push({ method, node: callNode }); + } + for (const registration of registrations) { + const httpMethod = + registration.method === 'all' + ? '*' + : registration.method === 'use' + ? 'GET' + : registration.method.toUpperCase(); + result.decoratorRoutes.push({ + filePath: file.path, + routePath, + httpMethod, + decoratorName: `express.${registration.method}`, + lineNumber: registration.node.startPosition.row + lineOffset, + }); + } } continue; } diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index b91dc6089..5f534cffe 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -838,7 +838,14 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // v131 (#3504): Python namespace imports retain explicit alias syntax. Warm // v130 ParsedFiles lack this fact and can bind a root-spelled alias to the // package root instead of the imported module. -const SCHEMA_BUMP = 131; +// v132 (#3487, #3505): merge the parallel Express route capture changes. +// Require semantic handler arguments, retain route(path) builders, and preserve +// app.all as method-agnostic. Both branches used v131 for different captures, +// so invalidate either branch's cache before reusing the combined schema. +// v133 (#3505): route(path) builders emit only chained verb registrations +// with semantic handler arguments. Warm v132 caches can retain phantom GET +// routes for bare builders and lose the actual methods of chained handlers. +const SCHEMA_BUMP = 133; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/lang-resolution/express-route-priority/spec/stubs.ts b/gitnexus/test/fixtures/lang-resolution/express-route-priority/spec/stubs.ts new file mode 100644 index 000000000..f7a273d60 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/express-route-priority/spec/stubs.ts @@ -0,0 +1,10 @@ +function fakeInfo() { + return 'fake info'; +} +function fakeQuery() { + return 'fake query'; +} + +app.get('/api/info', fakeInfo); +app.post('/api/query', fakeQuery); +app.post('/test-only', fakeQuery); diff --git a/gitnexus/test/fixtures/lang-resolution/express-route-priority/src/server.ts b/gitnexus/test/fixtures/lang-resolution/express-route-priority/src/server.ts new file mode 100644 index 000000000..00a197f1f --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/express-route-priority/src/server.ts @@ -0,0 +1,24 @@ +function realInfo() { return 'info'; } +function realQuery() { return 'query'; } +function mcp() { return 'mcp'; } + +app.get('/api/info', realInfo); +app.post('/api/query', realQuery); +app.all('/api/mcp', mcp); +app.route('/api/chained').get(realInfo); +app.route('/api/chained-post').post(realQuery); +app.route('/api/chained-multi').get(realInfo).post(realQuery); +app.route('/api/chained-all').all(mcp); +app.route('/ghost-builder'); +app.route('/ghost-builder-empty').get(); +app.route('/ghost-builder-block-comment').post(/* handler pending */); +app.route('/ghost-builder-line-comment').all( + // handler pending +); + +const counts = new Map(); +counts.get('/ghost'); +counts.get('/ghost-block-comment' /* cached key */); +counts.get( + '/ghost-line-comment', // cached key +); diff --git a/gitnexus/test/integration/resolvers/express-routes.test.ts b/gitnexus/test/integration/resolvers/express-routes.test.ts index 2ab5999be..f4edca7a1 100644 --- a/gitnexus/test/integration/resolvers/express-routes.test.ts +++ b/gitnexus/test/integration/resolvers/express-routes.test.ts @@ -1,5 +1,18 @@ import { describe, it, expect, beforeAll } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; import path from 'path'; +import { + loadParseCache, + PARSE_CACHE_VERSION, + pruneCache, + saveParseCache, + type ParseCache, +} from '../../../src/storage/parse-cache.js'; +import { + getDurableParsedFileDir, + pruneAndSaveDurableParsedFileStore, +} from '../../../src/storage/parsedfile-store.js'; import { FIXTURES, getRelationships, @@ -62,3 +75,163 @@ describe('Express/Hono route detection', () => { expect(healthEdge!.sourceFilePath).toContain('server.ts'); }); }); + +describe('Express route identity and source priority', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'express-route-priority'), () => {}); + }, 60000); + + it('keeps production handlers when test stubs register the same route', () => { + const handled = getRelationships(result, 'HANDLES_ROUTE'); + for (const route of ['/api/info', '/api/query']) { + const sources = handled + .filter((edge) => edge.target === route) + .map((edge) => edge.sourceFilePath); + expect(sources).toContain('src/server.ts'); + expect(sources).not.toContain('spec/stubs.ts'); + } + }); + + it('models app.all as method-agnostic, not GET', () => { + const mcpRoutes = getNodesByLabelFull(result, 'Route').filter( + (node) => node.name === '/api/mcp', + ); + expect(mcpRoutes).toHaveLength(1); + expect(mcpRoutes[0].properties.method).toBe('*'); + }); + + it.each(['/ghost', '/ghost-block-comment', '/ghost-line-comment'])( + 'does not classify the one-argument Map.get lookup %s as a route', + (route) => { + expect(getNodesByLabel(result, 'Route')).not.toContain(route); + }, + ); + + it.each([ + ['/api/chained', ['GET']], + ['/api/chained-post', ['POST']], + ['/api/chained-multi', ['GET', 'POST']], + ['/api/chained-all', ['*']], + ])('uses the registered verbs for route builder %s', (route, methods) => { + const chainedRoutes = getNodesByLabelFull(result, 'Route').filter( + (node) => node.name === route, + ); + expect(chainedRoutes.map((node) => node.properties.method).sort()).toEqual(methods); + const handled = getRelationships(result, 'HANDLES_ROUTE').filter( + (edge) => edge.target === route, + ); + expect(handled).toHaveLength(methods.length); + expect(handled.every((edge) => edge.sourceFilePath === 'src/server.ts')).toBe(true); + }); + + it.each([ + '/ghost-builder', + '/ghost-builder-empty', + '/ghost-builder-block-comment', + '/ghost-builder-line-comment', + ])('does not invent a route for builder %s without a handler', (route) => { + expect(getNodesByLabel(result, 'Route')).not.toContain(route); + expect(getRelationships(result, 'HANDLES_ROUTE').some((edge) => edge.target === route)).toBe( + false, + ); + }); + + it('still indexes a test-only route when there is no production collision', () => { + expect(getNodesByLabel(result, 'Route')).toContain('/test-only'); + }); + + it('preserves route ownership through serialized warm and mixed parse-cache replay', async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-express-route-cache-')); + const repoDir = path.join(tempDir, 'repo'); + const storageDir = path.join(tempDir, 'storage'); + try { + fs.cpSync(path.join(FIXTURES, 'express-route-priority'), repoDir, { recursive: true }); + const cold: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + storagePath: storageDir, + onDiskKeys: new Set(), + }; + const coldResult = await runPipelineFromRepo(repoDir, () => {}, { + parseCache: cold, + workerPoolSize: 1, + }); + expect(coldResult.usedWorkerPool).toBe(true); + expect(coldResult.reparsedFileCount).toBe(2); + expect(coldResult.parseCacheHitFileCount).toBe(0); + + pruneCache(cold, cold.usedKeys); + const savedKeys = await saveParseCache(storageDir, cold); + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + new Set(savedKeys), + ); + const warm = await loadParseCache(storageDir); + expect(warm).not.toBeNull(); + const warmResult = await runPipelineFromRepo(repoDir, () => {}, { + parseCache: warm ?? undefined, + workerPoolSize: 1, + }); + expect(warmResult.usedWorkerPool).toBe(false); + expect(warmResult.reparsedFileCount).toBe(0); + expect(warmResult.parseCacheHitFileCount).toBe(2); + + // Keep the earlier test-file chunk cached while the production route + // definitions are parsed again. Comments do not change route semantics. + fs.appendFileSync(path.join(repoDir, 'src/server.ts'), '\n// unchanged endpoints\n'); + const mixed = await loadParseCache(storageDir); + expect(mixed).not.toBeNull(); + const mixedResult = await runPipelineFromRepo(repoDir, () => {}, { + parseCache: mixed ?? undefined, + workerPoolSize: 1, + }); + expect(mixedResult.usedWorkerPool).toBe(true); + expect(mixedResult.reparsedFileCount).toBe(1); + expect(mixedResult.parseCacheHitFileCount).toBe(1); + + const project = (pipeline: PipelineResult) => ({ + routes: getNodesByLabelFull(pipeline, 'Route') + .map((route) => ({ + method: route.properties.method, + path: route.name, + filePath: route.properties.filePath, + })) + .sort( + (a, b) => + a.path.localeCompare(b.path) || String(a.method).localeCompare(String(b.method)), + ), + handled: getRelationships(pipeline, 'HANDLES_ROUTE') + .map((edge) => ({ + method: pipeline.graph.getNode(edge.rel.targetId)?.properties.method, + path: edge.target, + filePath: edge.sourceFilePath, + })) + .sort( + (a, b) => + a.path.localeCompare(b.path) || String(a.method).localeCompare(String(b.method)), + ), + }); + const expected = [ + { method: 'GET', path: '/api/chained', filePath: 'src/server.ts' }, + { method: '*', path: '/api/chained-all', filePath: 'src/server.ts' }, + { method: 'GET', path: '/api/chained-multi', filePath: 'src/server.ts' }, + { method: 'POST', path: '/api/chained-multi', filePath: 'src/server.ts' }, + { method: 'POST', path: '/api/chained-post', filePath: 'src/server.ts' }, + { method: 'GET', path: '/api/info', filePath: 'src/server.ts' }, + { method: '*', path: '/api/mcp', filePath: 'src/server.ts' }, + { method: 'POST', path: '/api/query', filePath: 'src/server.ts' }, + { method: 'POST', path: '/test-only', filePath: 'spec/stubs.ts' }, + ]; + const coldProjection = project(coldResult); + expect(coldProjection).toEqual({ routes: expected, handled: expected }); + expect(project(warmResult)).toEqual(coldProjection); + expect(project(mixedResult)).toEqual(coldProjection); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }, 120_000); +}); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index ea4489a4b..3eb9faf63 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -303,8 +303,13 @@ describe('PARSE_CACHE_VERSION', () => { // Moved 125 -> 126 for #3446: SDK positional tool definitions and attribution. // Moved 126 -> 127 for #3450: reject destructured SDK registration-method writes. // Moved 127 -> 128 for #3450: recognize SDK namespace imports. - it('pins SCHEMA_BUMP to 131 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354, #3371, #2965, #3390, #3398, #3396, #3394, #3399, #3414, #3408, #3402, #3446, #3450, #3499, #3502, #3504)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(131); + // Moved 128 -> 130 on main for #3499/#3502 declaration-binding corrections. + // Moved 128 -> 130 in parallel for #3487/#3505 Express route corrections. + // Moved 130 -> 131 to invalidate both incompatible branch cache schemas. + // Moved 131 -> 132 to combine #3504 alias facts with Express captures. + // Moved 132 -> 133 for #3505: only chained handler verbs register builder routes. + it('pins SCHEMA_BUMP to 133 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354, #3371, #2965, #3390, #3398, #3396, #3394, #3399, #3414, #3408, #3402, #3446, #3450, #3499, #3502, #3487, #3505, #3504)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(133); 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 @@ -314,7 +319,7 @@ describe('PARSE_CACHE_VERSION', () => { 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, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, - 123, 124, 125, 126, 127, 128, 129, 130, + 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); } diff --git a/gitnexus/test/unit/route-source-priority.test.ts b/gitnexus/test/unit/route-source-priority.test.ts new file mode 100644 index 000000000..028a26947 --- /dev/null +++ b/gitnexus/test/unit/route-source-priority.test.ts @@ -0,0 +1,354 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { createSemanticModel } from '../../src/core/ingestion/model/index.js'; +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; +import { resolveRouteHandlerSymbols } from '../../src/core/ingestion/call-processor.js'; +import { routesPhase } from '../../src/core/ingestion/pipeline-phases/routes.js'; +import type { ParseOutput } from '../../src/core/ingestion/pipeline-phases/parse.js'; +import type { ExtractedRoute } from '../../src/core/ingestion/route-extractors/laravel.js'; +import type { ExtractedDecoratorRoute } from '../../src/core/ingestion/workers/parse-worker.js'; +import { DATA_ROUTE_TABLE_SOURCE } from '../../src/core/ingestion/route-extractors/data-route-table.js'; +import { DISPATCH_GUARD_SOURCE } from '../../src/core/ingestion/route-extractors/dispatch-guard.js'; +import { + isTestRouteFile, + routeNodeKey, +} from '../../src/core/ingestion/route-extractors/route-path.js'; +import { generateId } from '../../src/lib/utils.js'; + +type Route = ExtractedRoute | ExtractedDecoratorRoute; +const PROD = 'src/server.ts'; +const TEST = 'test/stubs.ts'; +const KEY = routeNodeKey('GET', '/api/orders'); +const handlerId = (filePath: string, name: string) => `Function:${filePath}:${name}`; + +function extracted(filePath: string, handler = 'handle'): ExtractedRoute { + return { + filePath, + httpMethod: 'GET', + routePath: '/api/orders', + routeName: null, + controllerName: null, + methodName: handler, + middleware: [], + prefix: null, + lineNumber: 1, + }; +} + +function decorator(filePath: string, handler = 'handle'): ExtractedDecoratorRoute { + return { + filePath, + httpMethod: 'GET', + routePath: '/api/orders', + decoratorName: 'express.get', + handlerName: handler, + lineNumber: 1, + }; +} + +function table(filePath: string, handler = 'handle'): ExtractedDecoratorRoute { + return { ...decorator(filePath, handler), source: DATA_ROUTE_TABLE_SOURCE }; +} + +async function resolveAndEmit( + extractedRoutes: ExtractedRoute[], + decoratorRoutes: ExtractedDecoratorRoute[], + definitions: Array<[filePath: string, name: string]>, + additionalFiles: Record = {}, +) { + const model = createSemanticModel(); + const graph = createKnowledgeGraph(); + for (const [filePath, name] of definitions) { + const id = handlerId(filePath, name); + model.symbols.add(filePath, name, id, 'Function'); + graph.addNode({ + id, + label: 'Function', + properties: { name, filePath, startLine: 0, endLine: 0 }, + }); + } + const selected = new Set(); + const symbols = resolveRouteHandlerSymbols( + model, + extractedRoutes, + decoratorRoutes, + undefined, + selected, + ); + const allPaths = [ + ...new Set([ + ...[...extractedRoutes, ...decoratorRoutes].map((r) => r.filePath), + ...Object.keys(additionalFiles), + ]), + ]; + const repoPath = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-route-priority-')); + try { + for (const filePath of allPaths) { + await fs.mkdir(path.dirname(path.join(repoPath, filePath)), { recursive: true }); + await fs.writeFile( + path.join(repoPath, filePath), + additionalFiles[filePath] ?? 'export function handle() {}\n', + ); + } + // Pass the raw declarations with the admitted references, as the parse + // phase does: losing declarations may still carry named route aliases. + const output = { + allPaths, + allFetchCalls: [], + allFetchWrapperDefs: [], + allExtractedRoutes: extractedRoutes, + allDecoratorRoutes: decoratorRoutes, + selectedRoutes: selected, + routeHandlerSymbols: symbols, + } as unknown as ParseOutput; + await routesPhase.execute( + { repoPath, graph, onProgress: () => {}, pipelineStart: Date.now() }, + new Map([['parse', { phaseName: 'parse', output, durationMs: 0 }]]), + ); + } finally { + await fs.rm(repoPath, { recursive: true, force: true }); + } + return { + graph, + symbols, + selected, + route: (key = KEY) => graph.getNode(generateId('Route', key)), + sources: (key = KEY) => + graph.relationships + .filter((r) => r.type === 'HANDLES_ROUTE' && r.targetId === generateId('Route', key)) + .map((r) => r.sourceId) + .sort(), + }; +} + +describe('route declaration priority through resolver and routes phase', () => { + it('keeps named aliases from losing declarations for both Blade consumers', async () => { + const first = { ...extracted(PROD, 'first'), routeName: 'orders.primary' }; + const alias = { ...extracted(PROD, 'second'), routeName: 'orders.alias' }; + const primaryView = 'resources/views/orders/primary.blade.php'; + const aliasView = 'resources/views/orders/alias.blade.php'; + const result = await resolveAndEmit( + [first, alias], + [], + [ + [PROD, 'first'], + [PROD, 'second'], + ], + { + [primaryView]: `Orders`, + [aliasView]: `Orders alias`, + }, + ); + expect([...result.selected]).toEqual([first]); + expect(result.selected.has(alias)).toBe(false); + expect(result.route()?.properties.handlerSymbolId).toBe(handlerId(PROD, 'first')); + const fetches = result.graph.relationships.filter((r) => r.type === 'FETCHES'); + expect(fetches).toHaveLength(2); + expect(fetches.map((r) => r.sourceId).sort()).toEqual( + [generateId('File', primaryView), generateId('File', aliasView)].sort(), + ); + expect(fetches.map((r) => r.targetId)).toEqual([ + generateId('Route', KEY), + generateId('Route', KEY), + ]); + }); + + it.each(['test-extracted', 'test-decorator'] as const)( + 'production wins across route collections with %s', + async (kind) => { + const prod = kind === 'test-extracted' ? decorator(PROD) : extracted(PROD); + const stub = kind === 'test-extracted' ? extracted(TEST) : decorator(TEST); + const result = await resolveAndEmit( + [kind === 'test-extracted' ? (stub as ExtractedRoute) : (prod as ExtractedRoute)], + [ + kind === 'test-extracted' + ? (prod as ExtractedDecoratorRoute) + : (stub as ExtractedDecoratorRoute), + ], + [ + [PROD, 'handle'], + [TEST, 'handle'], + ], + ); + expect([...result.selected]).toEqual([prod]); + expect(result.selected.has(prod)).toBe(true); + expect(result.selected.has(stub)).toBe(false); + expect(result.route()?.properties.filePath).toBe(PROD); + expect(result.route()?.properties.handlerSymbolId).toBe(handlerId(PROD, 'handle')); + expect(result.sources()).toEqual( + [generateId('File', PROD), handlerId(PROD, 'handle')].sort(), + ); + }, + ); + + it('compares normalized prefixes and methods, retaining a different verb', async () => { + const stub = { + ...extracted(TEST), + httpMethod: ' get ', + routePath: '/orders/', + prefix: '/api/', + }; + const prod = { ...decorator(PROD), routePath: '/api//orders/' }; + const post = { ...decorator(TEST, 'post'), httpMethod: 'POST' }; + const result = await resolveAndEmit( + [stub], + [prod, post], + [ + [TEST, 'handle'], + [TEST, 'post'], + [PROD, 'handle'], + ], + ); + expect([...result.selected]).toEqual([prod, post]); + expect(result.route()?.properties.filePath).toBe(PROD); + expect(result.symbols.get(KEY)).toBe(handlerId(PROD, 'handle')); + expect(result.route(routeNodeKey('POST', '/api/orders'))?.properties.handlerSymbolId).toBe( + handlerId(TEST, 'post'), + ); + }); + + it.each(['test-first', 'production-first'] as const)( + 'an unresolved production handler cannot borrow a test handler (%s)', + async (order) => { + const prod = decorator(PROD, 'missing'); + const stub = decorator(TEST); + const routes = order === 'test-first' ? [stub, prod] : [prod, stub]; + const result = await resolveAndEmit([], routes, [[TEST, 'handle']]); + expect([...result.selected]).toEqual([prod]); + expect(result.symbols.has(KEY)).toBe(false); + expect(result.route()?.properties.filePath).toBe(PROD); + expect(result.route()?.properties).not.toHaveProperty('handlerSymbolId'); + expect(result.sources()).toEqual([generateId('File', PROD)]); + }, + ); + + it.each([PROD, TEST])( + 'keeps same-tier first writer and test-only declarations (%s)', + async (file) => { + const first = decorator(file, 'first'); + const second = decorator(file, 'second'); + const result = await resolveAndEmit( + [], + [first, second], + [ + [file, 'first'], + [file, 'second'], + ], + ); + expect([...result.selected]).toEqual([first]); + expect(result.route()?.properties.filePath).toBe(file); + expect(result.route()?.properties.handlerSymbolId).toBe(handlerId(file, 'first')); + }, + ); + + it.each(['conflicting', 'unresolved'] as const)( + 'test data tables cannot invalidate a production table (%s)', + async (kind) => { + const prod = table(PROD); + const stub = table(TEST, kind === 'conflicting' ? 'fake' : 'missing'); + const result = await resolveAndEmit( + [], + [stub, prod], + [ + [PROD, 'handle'], + [TEST, 'fake'], + ], + ); + expect([...result.selected]).toEqual([prod]); + expect(result.route()?.properties.filePath).toBe(PROD); + expect(result.route()?.properties.handlerSymbolId).toBe(handlerId(PROD, 'handle')); + }, + ); + + it.each(['conflicting', 'unresolved'] as const)( + 'suppresses ambiguous same-tier tables (%s)', + async (kind) => { + const first = table(PROD); + const second = table(PROD, kind === 'conflicting' ? 'other' : 'missing'); + const result = await resolveAndEmit( + [], + [first, second], + [ + [PROD, 'handle'], + [PROD, 'other'], + ], + ); + expect(result.selected.size).toBe(0); + expect(result.symbols.size).toBe(0); + expect(result.route()).toBeUndefined(); + }, + ); + + it('falls back from an unproven production table to the actual test declaration', async () => { + const prod = table(PROD, 'missing'); + const stub = decorator(TEST); + const result = await resolveAndEmit([], [prod, stub], [[TEST, 'handle']]); + expect([...result.selected]).toEqual([stub]); + expect(result.route()?.properties.filePath).toBe(TEST); + expect(result.route()?.properties.handlerSymbolId).toBe(handlerId(TEST, 'handle')); + expect(result.sources()).toEqual([generateId('File', TEST), handlerId(TEST, 'handle')].sort()); + }); + + it('a discarded methodless dispatch guard cannot donate the app.all handler', async () => { + const methodless = { + ...decorator(PROD, 'stale'), + httpMethod: '', + source: DISPATCH_GUARD_SOURCE, + }; + const verbed = { ...decorator(PROD, 'get'), source: DISPATCH_GUARD_SOURCE }; + const wildcard = { ...decorator(PROD, 'all'), httpMethod: '*', decoratorName: 'express.all' }; + const result = await resolveAndEmit( + [], + [methodless, verbed, wildcard], + [ + [PROD, 'stale'], + [PROD, 'get'], + [PROD, 'all'], + ], + ); + expect(result.selected.has(methodless)).toBe(false); + expect(result.selected.has(wildcard)).toBe(true); + expect(result.route('/api/orders')?.properties.method).toBe('*'); + expect(result.route('/api/orders')?.properties.handlerSymbolId).toBe(handlerId(PROD, 'all')); + expect(result.sources('/api/orders')).toEqual( + [generateId('File', PROD), handlerId(PROD, 'all')].sort(), + ); + expect(result.route()?.properties.handlerSymbolId).toBe(handlerId(PROD, 'get')); + }); +}); + +describe('route priority shares test-file classification', () => { + it.each([ + 'spec/routes.ts', + 'testing/routes.ts', + '__mocks__/routes.ts', + '__tests__/routes.ts', + 'routes.spec.ts', + 'routes.test.js', + 'routes_test.go', + 'routes_test.py', + 'routes_test.dart', + 'routes_spec.rb', + 'RoutesTest.php', + 'RoutesTests.cs', + 'RoutesTests.swift', + 'e2e/routes.ts', + 'E2E/routes.ts', + 'src\\testing\\routes.ts', + 'src\\e2e\\routes.ts', + ])('recognizes test registrations at %s', (file) => { + expect(isTestRouteFile(file)).toBe(true); + }); + + it.each([ + 'src/routes.ts', + 'src/contest.ts', + 'src/Contest.swift', + 'src/Latest.php', + 'src/e2e-utils/routes.ts', + ])('keeps production registrations at %s', (file) => { + expect(isTestRouteFile(file)).toBe(false); + }); +});