diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1308f424b..a503eb17f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -212,6 +212,13 @@ They follow explicit ESM/CommonJS imports and exports through the configured `router` is not evidence; type-only imports, shadowed bindings, conflicting writes, and unproven factories do not establish a receiver. Directly proven Hono verb registrations remain supported, without applying Express mounting semantics. +A registration whose receiver, literal path or mounted router is a parameter of a +named function (`mountSSEProgress(app, '/p', jobs)` → `app.get(routePath, …)`) +resolves through each call site of that function, once per site and in the +helper's file. This is one hop only: a site argument that is itself a parameter, +an unproven argument, or a reassigned parameter establishes nothing, and a helper +with no recorded call site is read as written, so an unknown mount path still +suppresses its router's standalone paths. A standalone router retains its local paths. A known dynamic mount suppresses that fallback, while independently proven static mounts remain available. Cycles diff --git a/gitnexus/bench/tool-accuracy/expectations.ts b/gitnexus/bench/tool-accuracy/expectations.ts index 7f7a27db8..265a6b071 100644 --- a/gitnexus/bench/tool-accuracy/expectations.ts +++ b/gitnexus/bench/tool-accuracy/expectations.ts @@ -117,7 +117,9 @@ export const EXPECTATIONS = [ id: 'routes.helper-registration', issue: 3487, title: 'Literal route forwarded through a local helper', - expected: ['GET /api/progress src/server/api.ts'], + // Like every Express route, it belongs to the file of its `app.get` call, + // which is also where its handler and response keys are. + expected: ['GET /api/progress src/server/progress.ts'], }, { id: 'api.template-wrapper', diff --git a/gitnexus/bench/tool-accuracy/known-gaps.json b/gitnexus/bench/tool-accuracy/known-gaps.json index 930b3c5df..9202c5627 100644 --- a/gitnexus/bench/tool-accuracy/known-gaps.json +++ b/gitnexus/bench/tool-accuracy/known-gaps.json @@ -2,12 +2,6 @@ "schemaVersion": 1, "baselineSourceSha": "dd096e690c06e5d54ab3595c63b83f15e04b6191", "gaps": [ - { - "id": "routes.helper-registration", - "issue": 3487, - "reason": "A literal route forwarded through a local helper is not extracted (#3487).", - "baselineActual": [] - }, { "id": "explain.guard", "issue": 3490, diff --git a/gitnexus/src/core/ingestion/route-extractors/framework-route-facts.ts b/gitnexus/src/core/ingestion/route-extractors/framework-route-facts.ts index 7876cecdb..74d626e1f 100644 --- a/gitnexus/src/core/ingestion/route-extractors/framework-route-facts.ts +++ b/gitnexus/src/core/ingestion/route-extractors/framework-route-facts.ts @@ -7,6 +7,20 @@ import type { FrameworkExpression, FrameworkRouteFacts } from './framework-route type Node = Parser.SyntaxNode; const UNKNOWN: FrameworkExpression = { kind: 'unknown' }; const HANDLER: FrameworkExpression = { kind: 'handler' }; +/** Expressions that can evaluate to a framework allocation passed into a helper. */ +const CARRIER_KINDS = new Set([ + 'binding', + 'call', + 'member', + 'import', + 'array', +]); +const FUNCTION_TYPES = [ + 'arrow_function', + 'function_expression', + 'function_declaration', + 'generator_function_declaration', +]; const METHODS = new Set(['get', 'post', 'put', 'delete', 'patch', 'all', 'route', 'use']); const children = (node: Node | null | undefined): Node[] => node?.namedChildren.filter((child) => child.type !== 'comment') ?? []; @@ -41,6 +55,7 @@ export function extractFrameworkRouteFacts( invalidBindings: [], mutations: [], calls: [], + invocations: [], }; const scopes = buildScopeTree(parsedFile.scopes); const positions = buildPositionIndex(parsedFile.scopes); @@ -53,6 +68,11 @@ export function extractFrameworkRouteFacts( importsAtPosition.set(key, imports); } const expressionCache = new Map(); + const importedNames = new Set( + parsedFile.parsedImports.flatMap((imported) => + 'localName' in imported ? [imported.localName] : [], + ), + ); const bindingKey = (scope: string, name: string): string => `${scope}\0${name}`; const ownership = (node: Node, name = node.text, atUse = false) => { const scope = positions.atPosition( @@ -100,6 +120,74 @@ export function extractFrameworkRouteFacts( return owner?.scope ? bindingKey(owner.scope.id, name) : undefined; }; const isGlobal = (node: Node, name: string) => ownership(node, name)?.status === 'absent'; + /** Whether a scope between `target` and `fn` declares the name `target` writes. */ + const shadowedBetween = (target: Node, fn: Node): boolean => { + const declares = (node: Node | null | undefined) => + !!node && + (node.type === 'identifier' + ? node.text === target.text + : node.descendantsOfType('identifier').some((id) => id.text === target.text)); + for (let scope = target.parent; scope && scope.id !== fn.id; scope = scope.parent) { + const ownsVar = FUNCTION_TYPES.includes(scope.parent?.type ?? ''); + if ( + children(scope).some( + (statement) => + (statement.type === 'lexical_declaration' || + (ownsVar && statement.type === 'variable_declaration')) && + children(statement).some((d) => declares(d.childForFieldName('name'))), + ) || + (FUNCTION_TYPES.includes(scope.type) && + (declares(scope.childForFieldName('parameter')) || + declares(scope.childForFieldName('parameters')))) || + (scope.type === 'catch_clause' && declares(scope.childForFieldName('parameter'))) + ) + return true; + } + return false; + }; + const functionValue = (fn: Node): FrameworkExpression => { + const single = fn.childForFieldName('parameter'); + const params = single ? [single] : children(fn.childForFieldName('parameters')); + // A parameter written anywhere in the body, nested closures included, is + // not the argument any more; fail closed rather than track which write runs. + // A write to a same-named binding a nested scope declares is not to it. + const written = new Set( + fn + .descendantsOfType([ + 'assignment_expression', + 'augmented_assignment_expression', + 'update_expression', + ]) + .map((write) => write.childForFieldName('left') ?? write.childForFieldName('argument')) + // A destructuring target (`[app] = [other]`) writes each name it binds; + // a member target (`app.locals = …`) writes a property, not the binding. + .flatMap((target) => + !target || ['member_expression', 'subscript_expression'].includes(target.type) + ? [] + : target.type === 'identifier' + ? [target] + : target.descendantsOfType(['identifier', 'shorthand_property_identifier_pattern']), + ) + .filter((target) => !shadowedBetween(target, fn)) + .map((target) => target.text), + ); + return { + kind: 'handler', + fn: `${filePath}:${fn.startIndex}`, + params: params + // TypeScript's `this: T` declares the receiver, not an argument position. + .filter((param) => param.childForFieldName('pattern')?.type !== 'this') + .map((param) => { + let name: Node | null = param; + if (param.type === 'required_parameter' || param.type === 'optional_parameter') + name = param.childForFieldName('pattern'); + if (name?.type === 'assignment_pattern') name = name.childForFieldName('left'); + return ( + (name?.type === 'identifier' && !written.has(name.text) && bindingId(name)) || null + ); + }), + }; + }; const expression = (node: Node | null | undefined, depth = 0): FrameworkExpression => { if (!node || depth > 64 || node.hasError) return UNKNOWN; @@ -123,14 +211,7 @@ export function extractFrameworkRouteFacts( ].includes(node.type) ) { value = expression(children(node)[0], depth + 1); - } else if ( - [ - 'arrow_function', - 'function_expression', - 'function_declaration', - 'generator_function_declaration', - ].includes(node.type) - ) { + } else if (FUNCTION_TYPES.includes(node.type)) { value = HANDLER; } else if (node.type === 'member_expression') { const name = node.childForFieldName('property'); @@ -225,13 +306,20 @@ export function extractFrameworkRouteFacts( } } else if (node.type === 'variable_declarator') { const name = node.childForFieldName('name'); - if (name) bind(name, expression(node.childForFieldName('value'))); + const value = node.childForFieldName('value'); + // Only a named function can be called by name, so only it carries the + // identity and parameters call-site forwarding needs. + if (name) + bind( + name, + value && FUNCTION_TYPES.includes(value.type) ? functionValue(value) : expression(value), + ); } else if ( node.type === 'function_declaration' || node.type === 'generator_function_declaration' ) { const name = node.childForFieldName('name'); - if (name) bind(name, HANDLER); + if (name) bind(name, functionValue(node)); } else if (node.type === 'export_statement') { const sourceNode = node.childForFieldName('source'); const source = sourceNode && plainString(sourceNode); @@ -249,13 +337,15 @@ export function extractFrameworkRouteFacts( ); } } else if (node.children.some((child) => child.type === 'default')) { + const declaration = + node.childForFieldName('value') ?? + node.childForFieldName('declaration') ?? + children(node)[0]; exported( 'default', - expression( - node.childForFieldName('value') ?? - node.childForFieldName('declaration') ?? - children(node)[0], - ), + declaration && FUNCTION_TYPES.includes(declaration.type) + ? functionValue(declaration) + : expression(declaration), ); } else if (source) { const namespace = children(node).find((child) => child.type === 'namespace_export'); @@ -382,6 +472,26 @@ export function extractFrameworkRouteFacts( lineNumber: node.startPosition.row + lineOffset, ...(handler?.type === 'identifier' ? { handlerName: handler.text } : {}), }); + } else if ( + callee?.type === 'identifier' || + (callee?.type === 'member_expression' && + importedNames.has(callee.childForFieldName('object')?.text ?? '')) + ) { + // A helper such as `mountRoutes(app, '/p')` may register on its + // parameters. Only a plain call or a namespace/default import member can + // reach a named function; arguments after a spread have no position. + const arguments_ = args(node); + const spread = arguments_.findIndex((argument) => argument.type === 'spread_element'); + const positional = spread < 0 ? arguments_ : arguments_.slice(0, spread); + // Keep only arguments that can carry an allocation or a literal path; + // a call passing neither cannot forward anything a resolver could use. + const carried = positional.map((argument) => { + const value = expression(argument); + return CARRIER_KINDS.has(value.kind) ? value : UNKNOWN; + }); + const paths = positional.map((argument) => literalPaths(argument)); + if (carried.some((value) => value !== UNKNOWN) || paths.some((path) => path !== null)) + facts.invocations.push({ callee: expression(callee), arguments: carried, paths }); } } } @@ -392,5 +502,41 @@ export function extractFrameworkRouteFacts( if (cjsDefaultAssignments <= 1) facts.exports.push(...cjsExports); } facts.invalidBindings = [...invalid]; + // A callee bound in this file to anything but a named function, an import or + // an alias (a callback parameter such as `next(err)`, a local object) can + // never reach a function registering on its parameters. + // An argument bound to a parameter or a function cannot be an allocation (a + // site argument that is itself a parameter never resolves), so it is dropped, + // and so is a call left with nothing to forward. + const valueKinds = new Map>(); + for (const { id, value } of facts.bindings) + valueKinds.set(id, (valueKinds.get(id) ?? new Set()).add(value.kind)); + const callable = (id: string) => + ['handler', 'import', 'binding', 'member'].some((kind) => + valueKinds.get(id)?.has(kind as FrameworkExpression['kind']), + ); + const allocatable = (value: FrameworkExpression): boolean => { + switch (value.kind) { + case 'binding': + return [...(valueKinds.get(value.id) ?? [])].some( + (kind) => kind !== 'handler' && kind !== 'unknown', + ); + case 'member': + return allocatable(value.object); + case 'call': + return allocatable(value.callee); + case 'array': + return value.elements.some(allocatable); + default: + return value.kind === 'import'; + } + }; + facts.invocations = facts.invocations.flatMap((invocation) => { + if (invocation.callee.kind === 'binding' && !callable(invocation.callee.id)) return []; + const carried = invocation.arguments.map((value) => (allocatable(value) ? value : UNKNOWN)); + return carried.some((value) => value !== UNKNOWN) || invocation.paths.some(Boolean) + ? [{ ...invocation, arguments: carried }] + : []; + }); return facts; } diff --git a/gitnexus/src/core/ingestion/route-extractors/framework-route-resolver.ts b/gitnexus/src/core/ingestion/route-extractors/framework-route-resolver.ts index 2a9372dd1..14bfbf4ca 100644 --- a/gitnexus/src/core/ingestion/route-extractors/framework-route-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/framework-route-resolver.ts @@ -17,7 +17,7 @@ type Value = | { kind: 'instance'; allocation: FrameworkAllocation } | { kind: 'builder'; receiverId: string; paths: string[] } | { kind: 'array'; elements: (Value | undefined)[] } - | { kind: 'handler' }; + | { kind: 'handler'; fn?: string }; type ExportResult = { found: boolean; value?: Value }; const keyOf = (value: Value): string => { switch (value.kind) { @@ -32,7 +32,7 @@ const keyOf = (value: Value): string => { case 'array': return `array:${value.elements.map((element) => element && keyOf(element)).join(',')}`; case 'handler': - return 'handler'; + return `handler:${value.fn ?? ''}`; } }; @@ -175,7 +175,7 @@ export function resolveFrameworkRouteFacts( case 'unknown': return undefined; case 'handler': - return { kind: 'handler' }; + return { kind: 'handler', ...(expression.fn ? { fn: expression.fn } : {}) }; case 'array': return { kind: 'array', @@ -301,84 +301,195 @@ export function resolveFrameworkRouteFacts( } cache.clear(); allocations.clear(); + + // One call-site hop (#3487): a registration whose receiver, path or mounted + // child is a parameter of a named function takes that argument from each + // call of the function. A site argument that is itself a parameter does not + // resolve, so forwarding never chains, and a reassigned parameter is null. + const parameterOf = new Map(); + /** The names each function is bound or exported under, for the call-site filter below. */ + const fnNames = new Map>(); + for (const file of files) + for (const { name, value } of [ + ...file.bindings.map(({ id, value }) => ({ + name: id.slice(id.lastIndexOf('\0') + 1), + value, + })), + ...file.exports, + ]) + if (value.kind === 'handler' && value.fn) { + value.params?.forEach((param, index) => { + if (param && !invalidBindings.has(param)) + parameterOf.set(param, { fn: value.fn!, index }); + }); + fnNames.set(value.fn, (fnNames.get(value.fn) ?? new Set()).add(name)); + } + // `export { mountRoutes as setup }` exports the binding: callers know `setup`. + for (const file of files) + for (const { name, value } of file.exports) { + const bound = value.kind === 'binding' ? bindings.get(value.id)?.values : undefined; + const fn = bound?.length === 1 && bound[0].kind === 'handler' ? bound[0].fn : undefined; + if (fn) fnNames.set(fn, (fnNames.get(fn) ?? new Set()).add(name)); + } + const parameter = (expression: FrameworkExpression | undefined) => + expression?.kind === 'binding' ? parameterOf.get(expression.id) : undefined; + // Only calls that can reach a function registering on its parameters are + // evaluated: resolving every helper call in a large repo is the cost cliff. + const forwardingNames = new Set(); + for (const file of files) + for (const call of file.calls) + for (const expression of [call.receiver, ...call.arguments]) { + const fn = parameter(expression)?.fn; + for (const name of (fn && fnNames.get(fn)) ?? []) forwardingNames.add(name); + } + /** Local and imported names a callee may be known by (`import { a as b }` gives both). */ + const calleeNames = (callee: FrameworkExpression): string[] => + callee.kind === 'binding' + ? [ + callee.id.slice(callee.id.lastIndexOf('\0') + 1), + ...(bindings.get(callee.id)?.values ?? []).flatMap((value) => + value.kind === 'import' ? [value.name] : [], + ), + ] + : callee.kind === 'import' || callee.kind === 'member' + ? [callee.name] + : []; + const sitesOf = new Map< + string, + { filePath: string; arguments: FrameworkExpression[]; paths: (string[] | null)[] }[] + >(); + for (const file of files) + for (const invocation of forwardingNames.size ? file.invocations : []) { + if (!calleeNames(invocation.callee).some((name) => forwardingNames.has(name))) continue; + const callee = evaluate(invocation.callee, file.filePath); + if (callee?.kind !== 'handler' || !callee.fn) continue; + const sites = sitesOf.get(callee.fn) ?? []; + sites.push({ filePath: file.filePath, ...invocation }); + sitesOf.set(callee.fn, sites); + } + interface Target { + receiver: Value | undefined; + paths: string[] | null; + /** Evaluates an argument of the call, through the site for a forwarded parameter. */ + argument: (expression: FrameworkExpression) => Value | undefined; + } + /** Receiver, paths and arguments per call site, or undefined when nothing is forwarded. */ + const forwardedTargets = (call: FrameworkCallFact, filePath: string): Target[] | undefined => { + const pathParam = call.paths ? undefined : parameter(call.arguments[0]); + // The receiver, the path or any argument (a mounted child) may be forwarded; + // all of them must come from the same function. + const fns = new Set( + [call.receiver, ...call.arguments].flatMap((expression) => parameter(expression)?.fn ?? []), + ); + const [fn] = fns; + const sites = fn ? sitesOf.get(fn) : undefined; + // No call site proves an argument: evaluate the call as written, which keeps + // an unknown mount path suppressing its child router (no invented root). + if (!fn || !sites || fns.size > 1) return undefined; + return sites.map((site) => { + const fromSite = (expression: FrameworkExpression | undefined) => { + const forwarded = parameter(expression); + return forwarded?.fn === fn + ? evaluate(site.arguments[forwarded.index] ?? { kind: 'unknown' }, site.filePath) + : expression && evaluate(expression, filePath); + }; + return { + receiver: fromSite(call.receiver), + paths: call.paths ?? (pathParam ? (site.paths[pathParam.index] ?? null) : null), + argument: fromSite, + }; + }); + }; + const result: ResolvedFrameworkRoutes = { allocations: [], registrations: [], mounts: [] }; for (const file of files) { for (const binding of file.bindings) evaluate({ kind: 'binding', id: binding.id }, file.filePath); - for (const call of file.calls) { - const receiver = evaluate(call.receiver, file.filePath); - if (!receiver) { - result.mounts.push(...(uncertainMounts.get(call) ?? [])); - continue; - } - if ( - call.method === 'use' && - receiver.kind === 'instance' && - receiver.allocation.kind !== 'hono' - ) { - const values = call.arguments.flatMap((argument) => - flatten(evaluate(argument, file.filePath)), - ); - // An invalidated argument can hide a known child even when its parent - // still resolves. Keep that incoming edge without asserting a path. - if (values.some((value) => value === undefined)) - result.mounts.push(...(uncertainMounts.get(call) ?? [])); - const first = values[0]; - const prefixes = - call.paths ?? - (first?.kind === 'handler' || - (first?.kind === 'instance' && first.allocation.kind !== 'hono') - ? [''] - : null); - for (const child of values) - if (child?.kind === 'instance' && child.allocation.kind !== 'hono') { - result.mounts.push({ - parentId: receiver.allocation.id, - childId: child.allocation.id, - prefixes, - }); - } - continue; - } - if (!VERBS.has(call.method)) continue; - const builder = receiver.kind === 'builder'; - if (receiver.kind !== 'instance' && !builder) continue; - if (call.arguments.length < (builder ? 1 : 2)) continue; - const paths = builder ? receiver.paths : call.paths; - if (!paths) continue; - const framework = - receiver.kind === 'instance' && receiver.allocation.kind === 'hono' ? 'hono' : 'express'; - // The shared symbol model resolves a bare name only within this file. - // Retain that evidence only for the exact lexical callable declaration; - // parameters, imported aliases and unknown values must not borrow a - // same-named function elsewhere in the file. - const handler = call.arguments.at(-1); - const handlerBinding = handler?.kind === 'binding' ? bindings.get(handler.id) : undefined; - const handlerName = - call.handlerName && - handlerBinding?.filePath === file.filePath && - handlerBinding.values.length === 1 && - handlerBinding.values[0].kind === 'handler' && - handler?.kind === 'binding' && - evaluate(handler, file.filePath)?.kind === 'handler' - ? call.handlerName - : undefined; - for (const routePath of paths) - result.registrations.push({ - receiverId: builder ? receiver.receiverId : receiver.allocation.id, - framework, - route: { - frameworkCall: call, - filePath: file.filePath, - routePath, - httpMethod: call.method === 'all' ? '*' : call.method.toUpperCase(), - decoratorName: `${framework}.${call.method}`, - lineNumber: call.lineNumber, - ...(handlerName ? { handlerName } : {}), - }, - }); - } + for (const call of file.calls) + for (const target of forwardedTargets(call, file.filePath) ?? [ + { + receiver: evaluate(call.receiver, file.filePath), + paths: call.paths, + argument: (expression: FrameworkExpression) => evaluate(expression, file.filePath), + }, + ]) + register(file.filePath, call, target); } result.allocations = [...allocations.values()]; return result; + + function register( + filePath: string, + call: FrameworkCallFact, + { receiver, paths: callPaths, argument }: Target, + ): void { + if (!receiver) { + result.mounts.push(...(uncertainMounts.get(call) ?? [])); + return; + } + if ( + call.method === 'use' && + receiver.kind === 'instance' && + receiver.allocation.kind !== 'hono' + ) { + const values = call.arguments.flatMap((expression) => flatten(argument(expression))); + // An invalidated argument can hide a known child even when its parent + // still resolves. Keep that incoming edge without asserting a path. + if (values.some((value) => value === undefined)) + result.mounts.push(...(uncertainMounts.get(call) ?? [])); + const first = values[0]; + const prefixes = + callPaths ?? + (first?.kind === 'handler' || + (first?.kind === 'instance' && first.allocation.kind !== 'hono') + ? [''] + : null); + for (const child of values) + if (child?.kind === 'instance' && child.allocation.kind !== 'hono') { + result.mounts.push({ + parentId: receiver.allocation.id, + childId: child.allocation.id, + prefixes, + }); + } + return; + } + if (!VERBS.has(call.method)) return; + const builder = receiver.kind === 'builder'; + if (receiver.kind !== 'instance' && !builder) return; + if (call.arguments.length < (builder ? 1 : 2)) return; + const paths = builder ? receiver.paths : callPaths; + if (!paths) return; + const framework = + receiver.kind === 'instance' && receiver.allocation.kind === 'hono' ? 'hono' : 'express'; + // The shared symbol model resolves a bare name only within this file. + // Retain that evidence only for the exact lexical callable declaration; + // parameters, imported aliases and unknown values must not borrow a + // same-named function elsewhere in the file. + const handler = call.arguments.at(-1); + const handlerBinding = handler?.kind === 'binding' ? bindings.get(handler.id) : undefined; + const handlerName = + call.handlerName && + handlerBinding?.filePath === filePath && + handlerBinding.values.length === 1 && + handlerBinding.values[0].kind === 'handler' && + handler?.kind === 'binding' && + evaluate(handler, filePath)?.kind === 'handler' + ? call.handlerName + : undefined; + for (const routePath of paths) + result.registrations.push({ + receiverId: builder ? receiver.receiverId : receiver.allocation.id, + framework, + route: { + frameworkCall: call, + filePath, + routePath, + httpMethod: call.method === 'all' ? '*' : call.method.toUpperCase(), + decoratorName: `${framework}.${call.method}`, + lineNumber: call.lineNumber, + ...(handlerName ? { handlerName } : {}), + }, + }); + } } diff --git a/gitnexus/src/core/ingestion/route-extractors/framework-route-types.ts b/gitnexus/src/core/ingestion/route-extractors/framework-route-types.ts index e3ee44c5f..f406cfa3c 100644 --- a/gitnexus/src/core/ingestion/route-extractors/framework-route-types.ts +++ b/gitnexus/src/core/ingestion/route-extractors/framework-route-types.ts @@ -3,7 +3,12 @@ import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js'; /** Provider-private facts. Only JSON values cross the worker/cache boundary. */ export type FrameworkExpression = | { kind: 'unknown' } - | { kind: 'handler' } + /** + * A function value. `fn` identifies it and `params` holds each parameter's + * binding id (null for a destructured or rest parameter), so a registration + * on a parameter can be resolved from the function's call sites. + */ + | { kind: 'handler'; fn?: string; params?: (string | null)[] } | { kind: 'array'; elements: FrameworkExpression[] } | { kind: 'binding'; id: string } | { kind: 'import'; source: string; name: string } @@ -26,6 +31,14 @@ export interface FrameworkCallFact { handlerName?: string; } +/** A call to a plain or imported function: the sites a parameter is forwarded from. */ +export interface FrameworkInvocationFact { + callee: FrameworkExpression; + arguments: FrameworkExpression[]; + /** Literal paths of each argument, for a path forwarded as a parameter. */ + paths: (string[] | null)[]; +} + export interface FrameworkRouteFacts { filePath: string; bindings: { id: string; value: FrameworkExpression }[]; @@ -35,6 +48,7 @@ export interface FrameworkRouteFacts { /** Property writes invalidate the value behind aliases, including imports. */ mutations: FrameworkExpression[]; calls: FrameworkCallFact[]; + invocations: FrameworkInvocationFact[]; } export interface FrameworkAllocation { diff --git a/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/api.ts b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/api.ts new file mode 100644 index 000000000..8087610b4 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/api.ts @@ -0,0 +1,18 @@ +import express from 'express'; +import { mountMCPEndpoints } from './mcp-http'; +import { mountSSEProgress } from './sse-progress'; + +export const registerWebUI = (app: express.Express, staticDir: string | null): void => { + if (!staticDir) return; + app.get('/', (_req, res) => res.sendFile('index.html', { root: staticDir })); +}; + +export async function createServer(jobs: unknown) { + const app = express(); + app.get('/api/info', (_req, res) => res.json({ ok: true })); + const cleanupMcp = await mountMCPEndpoints(app, {}); + mountSSEProgress(app, '/api/analyze/:jobId/progress', jobs); + mountSSEProgress(app, '/api/embed/:jobId/progress', jobs); + registerWebUI(app, 'web'); + return { app, cleanupMcp }; +} diff --git a/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/mcp-http.ts b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/mcp-http.ts new file mode 100644 index 000000000..81e12be98 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/mcp-http.ts @@ -0,0 +1,6 @@ +import type { Express } from 'express'; + +export async function mountMCPEndpoints(app: Express, backend: unknown): Promise<() => void> { + app.all('/api/mcp', (_req, res) => res.end(String(backend))); + return () => {}; +} diff --git a/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/sse-progress.ts b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/sse-progress.ts new file mode 100644 index 000000000..a63be1886 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/sse-progress.ts @@ -0,0 +1,7 @@ +import type express from 'express'; + +export const mountSSEProgress = (app: express.Express, routePath: string, jm: unknown) => { + app.get(routePath, (_req, res) => { + res.write(`data: ${JSON.stringify(jm)}\n\n`); + }); +}; diff --git a/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/unused.ts b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/unused.ts new file mode 100644 index 000000000..6444fbd99 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/unused.ts @@ -0,0 +1,6 @@ +import type { Express } from 'express'; + +// Never called: a parameter receiver proves nothing without a call site. +export function registerUnused(app: Express) { + app.get('/api/never-mounted', (_req, res) => res.end()); +} diff --git a/gitnexus/test/integration/resolvers/express-routes.test.ts b/gitnexus/test/integration/resolvers/express-routes.test.ts index eec72e3e6..ce031e845 100644 --- a/gitnexus/test/integration/resolvers/express-routes.test.ts +++ b/gitnexus/test/integration/resolvers/express-routes.test.ts @@ -26,6 +26,34 @@ import { const parseIdentity = await runtimeParseIdentity(); +// GitNexus's own server registers through helpers that receive the app and, +// for SSE progress, the path (#3487); the app comes from `express()` in the +// caller, so each registration resolves through exactly one call site. +describe('Express registrations forwarded through helper parameters', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'express-forwarded-registrations'), + () => {}, + ); + }, 60000); + + it('emits each helper registration once per call site, in the helper file', () => { + expect( + getNodesByLabelFull(result, 'Route') + .map((node) => `${node.properties.method} ${node.name} ${node.properties.filePath}`) + .sort(), + ).toEqual([ + '* /api/mcp src/server/mcp-http.ts', + 'GET / src/server/api.ts', + 'GET /api/analyze/:jobId/progress src/server/sse-progress.ts', + 'GET /api/embed/:jobId/progress src/server/sse-progress.ts', + 'GET /api/info src/server/api.ts', + ]); + }); +}); + describe('Express/Hono route detection', () => { let result: PipelineResult; diff --git a/gitnexus/test/unit/framework-route-provenance.test.ts b/gitnexus/test/unit/framework-route-provenance.test.ts index 1e117f508..2a212ca4f 100644 --- a/gitnexus/test/unit/framework-route-provenance.test.ts +++ b/gitnexus/test/unit/framework-route-provenance.test.ts @@ -155,6 +155,176 @@ function handler() {} function other() {}`); expect(urls(`${setup} r.get('/unsafe', handler);`)).toEqual([]); }); + // GitNexus's own server registers through helpers that receive the app and, + // for SSE progress, the path (#3487): one call-site hop, never two. + it('forwards a proven receiver and literal path through one helper call', () => { + const result = resolve({ + 'sse.ts': ` + import type express from 'express'; + export const mountSSEProgress = (app: express.Express, routePath: string, jm: unknown) => { + app.get(routePath, (req, res) => {}); + };`, + 'mcp.ts': ` + export async function mountMCPEndpoints(app, backend) { + app.all('/api/mcp', handler); + app.use('/api/mcp', auth); + return () => {}; + }`, + 'server.ts': ` + import express from 'express'; + import { mountSSEProgress } from './sse'; + import { mountMCPEndpoints } from './mcp'; + export const registerWebUI = (app: express.Express, dir: string | null): void => { + app.get('/', handler); + }; + const app = express(); + const cleanup = await mountMCPEndpoints(app, backend); + mountSSEProgress(app, '/api/analyze/:jobId/progress', jobs); + mountSSEProgress(app, '/api/embed/:jobId/progress', jobs); + registerWebUI(app, null);`, + }); + expect( + result.registrations + .map(({ route }) => `${route.httpMethod} ${route.routePath} ${route.filePath}`) + .sort(), + ).toEqual([ + '* /api/mcp mcp.ts', + 'GET / server.ts', + 'GET /api/analyze/:jobId/progress sse.ts', + 'GET /api/embed/:jobId/progress sse.ts', + ]); + }); + + it('follows a helper exported under another name', () => { + const result = resolve({ + 'routes.ts': `function mountRoutes(app) { app.get('/api/renamed', handler); } + export { mountRoutes as setup };`, + 'server.ts': `import express from 'express'; + import { setup } from './routes'; + setup(express());`, + }); + expect( + result.registrations.map( + ({ route }) => `${route.httpMethod} ${route.routePath} ${route.filePath}`, + ), + ).toEqual(['GET /api/renamed routes.ts']); + }); + + it('follows a helper called through an aliased import', () => { + const result = resolve({ + 'sse.ts': `export const mountSSEProgress = (app, routePath) => { app.get(routePath, handler); };`, + 'server.ts': `import express from 'express'; + import { mountSSEProgress as mountProgress } from './sse'; + const app = express(); + mountProgress(app, '/api/progress');`, + }); + expect( + result.registrations.map( + ({ route }) => `${route.httpMethod} ${route.routePath} ${route.filePath}`, + ), + ).toEqual(['GET /api/progress sse.ts']); + }); + + it('declines forwarding through two hops, unproven arguments, and reassigned parameters', () => { + expect( + urls(` + import express from 'express'; + function inner(app) { app.get('/two-hop', handler); } + function outer(app) { inner(app); } + outer(express()); + function unproven(app) { app.get('/unproven', handler); } + unproven(storage); + function reassigned(app) { app = storage; app.get('/reassigned', handler); } + reassigned(express()); + function runtime(app, path) { app.get(path, handler); } + runtime(express(), runtimePath); + function direct(app, path) { app.post(path, handler); } + direct(express(), '/direct'); + `), + ).toEqual(['POST /direct']); + }); + + it.each([ + { + shape: 'a literal mount path forwarded to a helper', + calls: `function mountApi(prefix) { app.use(prefix, router); } + mountApi('/api');`, + routes: ['GET /api/users'], + }, + { + shape: 'a runtime mount path forwarded to a helper', + calls: `function mountApi(prefix) { app.use(prefix, router); } + mountApi(process.env.API_PREFIX);`, + routes: [], + }, + { + shape: 'a helper whose mount path no call proves', + calls: 'function mountApi(prefix) { app.use(prefix, router); }', + routes: [], + }, + { + shape: 'a router forwarded as the mounted child', + calls: `function mountApi(target, child) { target.use('/api', child); } + mountApi(app, router);`, + routes: ['GET /api/users'], + }, + { + shape: 'a TypeScript this parameter', + calls: `function wire(this: void, target, prefix) { target.use(prefix, router); } + wire(app, '/v1');`, + routes: ['GET /v1/users'], + }, + { + shape: 'an argument after a spread', + calls: `function wire(target, prefix) { target.use(prefix, router); } + wire(...targets, '/spread');`, + // Unproven receiver: the router keeps its standalone paths, as without + // any helper, rather than borrowing '/spread' from the wrong position. + routes: ['GET /users'], + }, + { + shape: 'a parameter reassigned in a nested closure', + calls: `function wire(target) { const swap = () => { target = other; }; swap(); target.use('/api', router); } + wire(app);`, + routes: ['GET /users'], + }, + { + shape: 'a helper forwarding only its mounted child', + calls: `function mountChild(child) { app.use('/api', child); } + mountChild(router);`, + routes: ['GET /api/users'], + }, + { + shape: 'a parameter reassigned by destructuring', + calls: `function wire(target) { [target] = [other]; target.use('/api', router); } + wire(app);`, + routes: ['GET /users'], + }, + { + shape: 'a write to a same-named local in a nested block', + calls: `function wire(target) { { let target; target = other; } target.use('/api', router); } + wire(app);`, + routes: ['GET /api/users'], + }, + { + shape: 'a property write on another function', + calls: `function Button() {} Button.displayName = 'Button'; + function mountApi(prefix) { app.use(prefix, router); } + mountApi('/api');`, + routes: ['GET /api/users'], + }, + ])('composes mounts through helper parameters: $shape', ({ calls, routes }) => { + const result = resolve({ + 'server.ts': `import express from 'express'; + const app = express(); const router = express.Router(); + router.get('/users', handler); + ${calls}`, + }); + expect( + composeFrameworkRoutePaths(result).map((route) => `${route.httpMethod} ${route.routePath}`), + ).toEqual(routes); + }); + it('does not lend later allocations to calls in a temporal dead zone', () => { expect( urls(`