diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 0590855ac..03e3730b3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -98,7 +98,7 @@ scan → structure → [springConfig, markdown, cobol] → parse → [routes, to | `markdown` | `markdown.ts` | `structure` | Section nodes, cross-link edges from .md/.mdx | | `cobol` | `cobol.ts` | `structure` | COBOL program/paragraph/section nodes (regex, no tree-sitter) | | `parse` | `parse.ts` + `parse-impl.ts` | `structure`, `markdown`, `cobol` | Symbol nodes, IMPORTS/CALLS/EXTENDS edges, extracted routes/tools/ORM queries | -| `routes` | `routes.ts` | `parse` | Route nodes + HANDLES_ROUTE edges (Next.js, Expo, PHP, decorators) | +| `routes` | `routes.ts` | `parse` | Route nodes + HANDLES_ROUTE edges (Next.js, Expo, PHP, decorators, and JS/TS dispatch guards — see below) | | `tools` | `tools.ts` | `parse` | Tool nodes + HANDLES_TOOL edges | | `orm` | `orm.ts` | `parse` | QUERIES edges (Prisma, Supabase) | | `crossFile` | `cross-file.ts` + `cross-file-impl.ts` | `parse`, `routes`, `tools`, `orm` | Cross-file type propagation in topological import order | @@ -164,6 +164,48 @@ export const myPhase: PipelinePhase = { }; ``` +### Where routes come from + +`route-extractors/` holds four independent ways a route can be discovered, all +converging on the routes phase's `(method, url)` registry: + +| Source | Shape | Examples | +| --- | --- | --- | +| Filesystem convention | path → URL, no parsing | Next.js `app/`, Expo, PHP | +| Single-file framework route | `isRouteFile` + worker extraction | Laravel `routes/*.php` | +| Cross-file framework route | `discoverRootRouteFiles` + `extractRoutes` | Django `urlpatterns` | +| AST-level route in a normal file | `extractDecoratorRoutes` | Spring, FastAPI, NestJS, **JS/TS dispatch guards** | + +The last row is the one whose name undersells it. A route is DECLARED by a +decorator, but it can also be **inferred** from a raw `node:http` server's own +dispatch — `if (req.method === 'GET' && pathname === '/api/x')` is a route with +a path, a verb and a handler, and nothing else in the pipeline could see it. +`route-extractors/dispatch-guard.ts` reads that shape; the transport, dedup and +handler resolution are shared with decorator routes, and +`ExtractedDecoratorRoute.source` carries the provenance difference through to +the `HANDLES_ROUTE` edge. + +That extractor is deliberately **precision-weighted**: `route_map` presents its +output as fact, so a `startsWith` namespace test, a bare `pathname === '/'` +without a verb, and any regex it cannot translate exactly are all dropped rather +than guessed at. A missing route is a coverage limit; an invented one is a lie. + +Two rules there need more than one comparison to decide, and are worth knowing +about before changing either: + +- **Same-file constant folding.** `` pathname === `${basePath}/rules` `` is + common enough that refusing it loses whole route modules — and loses them + invisibly, since a module with unfoldable paths and a module with no routes + produce the same empty answer. Folding is same-file, string literals only, one + alias hop, and refuses on ambiguity (a name declared twice with different + values is dropped, never guessed). +- **Whole-repo reconciliation** (`reconcileDispatchGuardRoutes`, applied in the + routes phase). A split route table — one module listing every path it + recognises so the dispatcher can 404 early, handlers in others — otherwise + lists every route twice, once verb-less with the table as its "handler". It + applies to dispatch-guard routes only: a framework route with no verb is + method-agnostic *by declaration*, which is a fact, not a weaker observation. + --- ## Semantic model diff --git a/gitnexus/src/core/ingestion/language-provider.ts b/gitnexus/src/core/ingestion/language-provider.ts index 329faf42f..903027f70 100644 --- a/gitnexus/src/core/ingestion/language-provider.ts +++ b/gitnexus/src/core/ingestion/language-provider.ts @@ -282,14 +282,22 @@ interface LanguageProviderConfig { ) => ExtractedRoute[]; /** - * Extract decorator-style route annotations from a parsed file. + * Extract routes that a parsed file declares in its own AST. * * When defined, the parse worker calls this after per-file capture processing - * to extract framework route definitions that require AST-level analysis beyond + * to extract route definitions that require AST-level analysis beyond * generic `@decorator` captures (e.g., Java Spring class-level prefix joining, * multi-class handling). The returned routes are appended to `decoratorRoutes`. * - * Default: undefined (no language-specific decorator route extraction). + * Decorators are the common case and the reason for the name, but not the only + * shape: JS/TS uses this hook for hand-rolled dispatch guards + * (`route-extractors/dispatch-guard.ts`), where a raw `node:http` server + * declares a route by comparing the request path to a literal. Anything that + * yields a `(path, verb, handler)` triple from one file's AST belongs here — + * set `ExtractedDecoratorRoute.source` when the provenance is not a decorator, + * so the `HANDLES_ROUTE` edge does not claim one. + * + * Default: undefined (no language-specific route extraction). */ readonly extractDecoratorRoutes?: ( tree: Parser.Tree, diff --git a/gitnexus/src/core/ingestion/languages/typescript.ts b/gitnexus/src/core/ingestion/languages/typescript.ts index 2ccfbb577..c24ab6ae8 100644 --- a/gitnexus/src/core/ingestion/languages/typescript.ts +++ b/gitnexus/src/core/ingestion/languages/typescript.ts @@ -124,6 +124,7 @@ import { jsMergeBindings, jsArityCompatibility, } from './javascript/index.js'; +import { extractDispatchGuardRoutes } from '../route-extractors/dispatch-guard.js'; /** * TypeScript/JavaScript: arrow_function and function_expression are @@ -454,6 +455,10 @@ export const typescriptProvider = defineLanguage({ receiverBinding: tsReceiverBinding, arityCompatibility: typescriptArityCompatibility, resolveImportTarget: resolveTsImportTarget, + // A raw `node:http` server declares its routes by comparing the request path + // to a literal; nothing else in this pipeline can see that shape. TS and JS + // share the grammar, so they share the extractor. + extractDecoratorRoutes: extractDispatchGuardRoutes, }); export const javascriptProvider = defineLanguage({ @@ -526,4 +531,6 @@ export const javascriptProvider = defineLanguage({ mergeBindings: (_scope, bindings) => jsMergeBindings(bindings), receiverBinding: jsReceiverBinding, arityCompatibility: jsArityCompatibility, + // See the TypeScript provider above. + extractDecoratorRoutes: extractDispatchGuardRoutes, }); diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index 041fbebdf..eca248fad 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -29,6 +29,7 @@ import { compiledMatcherMatchesRoute, } from '../route-extractors/middleware.js'; import { processNextjsFetchRoutes } from '../call-processor.js'; +import { reconcileDispatchGuardRoutes } from '../route-extractors/dispatch-guard.js'; import { normalizeExtractedRoutePath, normalizeRouteMethod, @@ -248,11 +249,18 @@ export const routesPhase: PipelinePhase = { namedRouteRegistry.set(route.routeName, routeUrl); } } - for (const dr of allDecoratorRoutes) { + // A dispatch-guard route observed WITHOUT a verb is dropped when the same + // URL is claimed WITH one anywhere in the repo — the split route-table + // 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)) { const url = normalizeExtractedRoutePath(dr.routePath, dr.prefix ?? null); addRoute(url, { filePath: dr.filePath, - source: `decorator-${dr.decoratorName}`, + // A route extracted from a file's own AST is usually a decorator; a + // dispatch guard is the same transport with different provenance, and + // says so (`ExtractedDecoratorRoute.source`). + source: dr.source ?? `decorator-${dr.decoratorName}`, method: normalizeRouteMethod(dr.httpMethod), }); } diff --git a/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts b/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts new file mode 100644 index 000000000..0454108f2 --- /dev/null +++ b/gitnexus/src/core/ingestion/route-extractors/dispatch-guard.ts @@ -0,0 +1,639 @@ +/** + * Hand-rolled dispatch-guard route extractor (JavaScript / TypeScript). + * + * Every route extractor before this one recognises a route because a FRAMEWORK + * declares it: a decorator, a `Route::get()` call, a filesystem convention. A + * server written against raw `node:http` declares its routes the only way the + * language offers — by COMPARING the request path to a literal: + * + * if (req.method === 'GET' && pathname === '/api/live/portfolio') { … } + * + * That is a route definition in every sense that matters to this graph: it has a + * path, a verb, and a handler. GitNexus simply had no rule that could see it, so + * `route_map` answered "No routes found in this project" for a repo with + * seventeen route modules and 113 such comparisons — the same confident-empty + * failure this whole change set is about, one tool wide. + * + * PRECISION OVER RECALL, deliberately. A missed route is a coverage limit; an + * invented route is a false fact, and `route_map` presents its output as fact. + * So every rule here requires the comparison to be against something that is + * demonstrably a request path, and anything that cannot be converted cleanly is + * dropped rather than guessed at. Specifically NOT extracted: + * + * - `pathname.startsWith('/api/')` — a namespace test ("do I own this?"), + * not a route. Minting `/api` would claim a route nobody serves. + * - a bare `pathname === '/'` with no verb — far more often a normalisation + * branch (`pathname === '/' ? '/index.html' : pathname`) than a route. With + * a verb alongside it the intent is unambiguous, so that form IS extracted. + * - any regex whose body is not a literal path plus single-segment wildcards. + * + * One consequence worth stating rather than discovering: a single-page app that + * branches on `location.pathname === '/settings'` mints a Route too. That is + * intentional — it is the same claim a Next.js filesystem route makes, that this + * file serves this path — and it keeps the rule from needing to guess whether a + * comparison is "backend enough". It does mean `route_map` on a SPA reports + * client routes alongside API ones, distinguishable by their `source`. + * + * @module route-extractors/dispatch-guard + */ + +import type Parser from 'tree-sitter'; +import type { SyntaxNode } from 'tree-sitter'; +import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js'; + +/** Provenance stamped on the Route node, in place of `decorator-`. */ +export const DISPATCH_GUARD_SOURCE = 'dispatch-guard-route'; + +const HTTP_VERBS: ReadonlySet = new Set([ + 'GET', + 'POST', + 'PUT', + 'PATCH', + 'DELETE', + 'HEAD', + 'OPTIONS', +]); + +const EQUALITY_OPERATORS: ReadonlySet = new Set(['===', '==']); + +/** + * Expressions that denote the request path. Kept deliberately narrow: this is + * the predicate standing between "a string comparison" and "a route", so a loose + * match here is how invented routes would get in. `path` alone is excluded — in + * Node it is overwhelmingly the `node:path` module or a filesystem path. + */ +const PATH_IDENTIFIERS: ReadonlySet = new Set([ + 'pathname', + 'pathName', + 'urlPath', + 'routePath', + 'reqPath', + 'requestPath', +]); + +/** `req.url` / `request.url` — the raw form, before a URL parse. */ +const RAW_URL_RECEIVERS: ReadonlySet = new Set(['req', 'request']); + +/** + * Cheap pre-filter, so this costs nothing on the overwhelming majority of files. + * + * Sound by construction rather than by luck: every rule below reaches a route + * only through {@link isPathExpression}, which returns true only for one of the + * {@link PATH_IDENTIFIERS} or for a member access whose property is `pathname` / + * `url`. A file whose source contains none of those substrings cannot produce a + * route, so skipping the walk cannot change the output. Keep this alternation in + * step with those two predicates — widening one without the other would silently + * re-introduce the empty answer this module exists to remove. + */ +const PATH_TOKEN_HINT = /pathname|pathName|urlPath|routePath|reqPath|requestPath|\.\s*url\b/; + +const FUNCTION_NODE_TYPES: ReadonlySet = new Set([ + 'function_declaration', + 'generator_function_declaration', + 'function_expression', + 'generator_function', + 'arrow_function', + 'method_definition', +]); + +/** + * A string literal that could be a URL path: leading slash, no whitespace, and + * no scheme. The character class is permissive about what a path may CONTAIN + * (`{id}`, `:id`, `%20`, `.json` are all legitimate) because the leading slash + * plus a path-denoting operand already carries the discrimination. + */ +function isPathLiteral(value: string): boolean { + if (!value.startsWith('/')) return false; + if (value.includes('://')) return false; + return /^\/[\w\-./{}:$*%~@]*$/.test(value); +} + +/** + * Same-file string constants, for folding a composed path. + * + * Built once per file and passed down, because the idiom it exists for is + * common enough that refusing it loses whole route modules: the reporting repo + * writes `pathname === \`${autoTradeBasePath}/rules\`` throughout one of its + * seventeen route files, so without folding that file contributes NOTHING while + * looking exactly like a file with no routes. + * + * Deliberately flat — no scope tracking. The cost of that shortcut is bounded by + * refusing ambiguity: a name declared twice with DIFFERENT literal values is + * removed from the map entirely, so a shadowed constant produces no route rather + * than the wrong one. + */ +type ConstantMap = ReadonlyMap; + +/** Follow `a = b = 'literal'` chains, with a cap so a cycle cannot hang. */ +const MAX_CONSTANT_HOPS = 4; + +function buildConstantMap(root: SyntaxNode): ConstantMap { + const direct = new Map(); // name -> literal + const alias = new Map(); // name -> other name + const ambiguous = new Set(); + + const record = (map: Map, name: string, value: string): void => { + const existing = map.get(name); + if (existing !== undefined && existing !== value) ambiguous.add(name); + else map.set(name, value); + }; + + const visit = (node: SyntaxNode): void => { + if (node.type === 'variable_declarator') { + const name = node.childForFieldName('name'); + const value = unparenthesize(node.childForFieldName('value')); + if (name !== null && name.type === 'identifier' && value !== null) { + if (value.type === 'string' || value.type === 'template_string') { + const raw = plainLiteralValue(value); + if (raw !== null) record(direct, name.text, raw); + } else if (value.type === 'identifier') { + record(alias, name.text, value.text); + } + } + } + for (const child of node.namedChildren) visit(child); + }; + visit(root); + + const resolved = new Map(); + for (const name of [...direct.keys(), ...alias.keys()]) { + if (ambiguous.has(name)) continue; + let current = name; + for (let hop = 0; hop < MAX_CONSTANT_HOPS; hop++) { + if (ambiguous.has(current)) break; + const literal = direct.get(current); + if (literal !== undefined) { + resolved.set(name, literal); + break; + } + const next = alias.get(current); + if (next === undefined) break; + current = next; + } + } + return resolved; +} + +/** Unquote a plain string / substitution-free template literal. */ +function plainLiteralValue(node: SyntaxNode): string | null { + if (node.type !== 'string' && node.type !== 'template_string') return null; + if ( + node.type === 'template_string' && + node.namedChildren.some((c) => c.type !== 'string_fragment') + ) { + return null; + } + const text = node.text; + if (text.length < 2) return null; + return text.slice(1, -1); +} + +/** + * The string this expression denotes, folding same-file constants where it can. + * + * Handles a plain literal, a template string whose substitutions all resolve to + * known constants, and `+` concatenation of those. Returns `null` the moment any + * part is unknown — a partially-folded path would be a wrong route, and a route + * that is missing is the cheaper of the two failures. + */ +function literalValue(node: SyntaxNode, constants: ConstantMap = new Map()): string | null { + const plain = plainLiteralValue(node); + if (plain !== null) return plain; + + if (node.type === 'identifier') return constants.get(node.text) ?? null; + + if (node.type === 'template_string') { + let out = ''; + for (const child of node.namedChildren) { + if (child.type === 'string_fragment') { + out += child.text; + continue; + } + if (child.type !== 'template_substitution') return null; + const inner = unparenthesize(child.namedChildren[0] ?? null); + if (inner === null) return null; + const value = literalValue(inner, constants); + if (value === null) return null; + out += value; + } + return out; + } + + if (node.type === 'binary_expression' && node.childForFieldName('operator')?.text === '+') { + const left = unparenthesize(node.childForFieldName('left')); + const right = unparenthesize(node.childForFieldName('right')); + if (left === null || right === null) return null; + const leftValue = literalValue(left, constants); + const rightValue = literalValue(right, constants); + if (leftValue === null || rightValue === null) return null; + return leftValue + rightValue; + } + + return null; +} + +/** + * Does this expression denote the request path? Accepts a bare identifier from + * {@link PATH_IDENTIFIERS}, any member access ending in `.pathname`, and the raw + * `req.url` / `request.url` forms. + */ +function isPathExpression(node: SyntaxNode): boolean { + if (node.type === 'identifier') return PATH_IDENTIFIERS.has(node.text); + if (node.type === 'member_expression') { + const property = node.childForFieldName('property'); + if (property === null) return false; + if (PATH_IDENTIFIERS.has(property.text)) return true; + if (property.text === 'url') { + const object = node.childForFieldName('object'); + return object !== null && RAW_URL_RECEIVERS.has(object.text); + } + return false; + } + return false; +} + +/** Strip redundant parentheses, which the grammar keeps as real nodes. */ +function unparenthesize(node: SyntaxNode | null): SyntaxNode | null { + let current = node; + while (current !== null && current.type === 'parenthesized_expression') { + current = current.namedChildren[0] ?? null; + } + return current; +} + +/** Does this expression denote the request METHOD (`req.method`, `method`)? */ +function isMethodExpression(node: SyntaxNode): boolean { + if (node.type === 'identifier') return node.text === 'method' || node.text === 'httpMethod'; + if (node.type === 'member_expression') { + const property = node.childForFieldName('property'); + return property !== null && (property.text === 'method' || property.text === 'httpMethod'); + } + return false; +} + +/** + * The HTTP verb an equality comparison asserts, if it is one — `req.method === + * 'GET'` → `GET`. Case-normalised, so `'get'` works too. + */ +function verbFromComparison(node: SyntaxNode): string | null { + if (node.type !== 'binary_expression') return null; + const operator = node.childForFieldName('operator')?.text ?? ''; + if (!EQUALITY_OPERATORS.has(operator)) return null; + const left = node.childForFieldName('left'); + const right = node.childForFieldName('right'); + if (left === null || right === null) return null; + + for (const [expr, literal] of [ + [left, right], + [right, left], + ] as const) { + if (!isMethodExpression(expr)) continue; + const value = literalValue(literal); + if (value === null) continue; + const verb = value.toUpperCase(); + if (HTTP_VERBS.has(verb)) return verb; + } + return null; +} + +/** + * Find the verb that governs a path comparison, by walking outward. + * + * Two idioms, both common and both handled: + * `if (req.method === 'GET' && pathname === '/x')` — a sibling in the same + * condition; and + * `if (req.method === 'GET') { if (pathname === '/x') … }` — an enclosing + * guard. + * + * The walk stops at the function boundary, and REFUSES to inherit a verb from an + * `if` whose `else` branch we are standing in: in + * `if (req.method === 'POST') {…} else if (pathname === '/x')` the path + * comparison is reached precisely when the method is NOT POST, so attributing + * POST to it would be exactly backwards. + */ +function governingVerb(comparison: SyntaxNode): string | null { + let current: SyntaxNode = comparison; + let parent = current.parent; + + while (parent !== null && !FUNCTION_NODE_TYPES.has(parent.type)) { + if ( + parent.type === 'binary_expression' && + parent.childForFieldName('operator')?.text === '&&' + ) { + const sibling = + parent.childForFieldName('left')?.id === current.id + ? parent.childForFieldName('right') + : parent.childForFieldName('left'); + const verb = sibling === null ? null : findVerbInSubtree(sibling); + if (verb !== null) return verb; + } + if (parent.type === 'if_statement') { + const alternative = parent.childForFieldName('alternative'); + const inElseBranch = alternative !== null && alternative.id === current.id; + const condition = parent.childForFieldName('condition'); + // A comparison inside the condition itself is handled by the `&&` rule + // above; here we only inherit from an ENCLOSING if we are governed by. + if (!inElseBranch && condition !== null && condition.id !== current.id) { + const verb = findVerbInSubtree(condition); + if (verb !== null) return verb; + } + } + current = parent; + parent = current.parent; + } + return null; +} + +/** First verb comparison anywhere in this subtree. */ +function findVerbInSubtree(node: SyntaxNode): string | null { + const direct = verbFromComparison(node); + if (direct !== null) return direct; + for (const child of node.namedChildren) { + const found = findVerbInSubtree(child); + if (found !== null) return found; + } + return null; +} + +/** + * The name of the function containing this comparison — the route's handler. + * + * Covers the declared forms and the two anonymous ones that carry a name from + * their binding site: `const handle = (req) => …` and the object-literal method + * shorthand (`{ async handle(req, res) {…} }`), which is how the reporting + * repo's route modules are written. + */ +function enclosingHandlerName(node: SyntaxNode): string | undefined { + let current: SyntaxNode | null = node.parent; + while (current !== null) { + if (FUNCTION_NODE_TYPES.has(current.type)) { + const own = current.childForFieldName('name'); + if (own !== null) return own.text; + const parent = current.parent; + if (parent === null) return undefined; + if (parent.type === 'variable_declarator' || parent.type === 'pair') { + const bound = parent.childForFieldName('name') ?? parent.childForFieldName('key'); + return bound?.text; + } + if (parent.type === 'assignment_expression') { + const left = parent.childForFieldName('left'); + if (left === null) return undefined; + return left.type === 'member_expression' + ? (left.childForFieldName('property')?.text ?? undefined) + : left.text; + } + return undefined; + } + current = current.parent; + } + return undefined; +} + +/** + * Convert an anchored regex used as a path test into a route path, or `null` if + * any part of it is not cleanly representable. + * + * `^\/api\/research-runs\/[^/]+$` → `/api/research-runs/{param}` + * + * Only two wildcard atoms are recognised, both single-segment (`[^/]+` and + * `[^/]*`, with or without the slash escaped). Anything else — an optional + * group, an alternation, a bare `.*` — bails, because a route path is a claim + * about what the server serves and a mistranslated pattern is a wrong one. + */ +export function regexToRoutePath(source: string): string | null { + if (!source.startsWith('^') || !source.endsWith('$')) return null; + const body = source.slice(1, -1); + if (body.length === 0) return null; + + let out = ''; + let i = 0; + let paramIndex = 0; + while (i < body.length) { + const rest = body.slice(i); + const wildcard = /^\[\^\\?\/\][+*]/.exec(rest); + if (wildcard !== null) { + paramIndex += 1; + out += `{param${paramIndex}}`; + i += wildcard[0].length; + continue; + } + const char = body[i] ?? ''; + if (char === '\\') { + const escaped = body[i + 1]; + if (escaped === undefined) return null; + // Only escapes of literal path punctuation are meaningful here; an escape + // class (`\d`, `\w`, `\s`) is a pattern, not a literal. + if (/[A-Za-z0-9]/.test(escaped)) return null; + out += escaped; + i += 2; + continue; + } + if ('[](){}|+*?^$.'.includes(char)) return null; + out += char; + i += 1; + } + return out.startsWith('/') ? out : null; +} + +/** A route the walk found, before per-file reconciliation. */ +interface GuardRoute { + readonly url: string; + readonly verb: string | null; + readonly handlerName: string | undefined; + readonly line: number; +} + +/** + * Extract routes declared by path-comparison dispatch from one JS/TS file. + * + * Returns the same {@link ExtractedDecoratorRoute} transport every AST-level + * route extractor returns — a route is a route once it has a path, a verb and a + * handler, and reusing the transport means the routes phase, the `(method, url)` + * dedup and the handler-symbol resolution all apply unchanged. `source` + * distinguishes the provenance, which is the part that actually differs: a + * decorator route is DECLARED, a dispatch-guard route is INFERRED from a + * comparison. + */ +export function extractDispatchGuardRoutes( + tree: Parser.Tree, + filePath: string, + lineOffset = 0, +): ExtractedDecoratorRoute[] { + // Every JS/TS file in every repo reaches this hook, so the walk is gated on a + // substring test first — see PATH_TOKEN_HINT for why skipping is sound. + if (!PATH_TOKEN_HINT.test(tree.rootNode.text)) return []; + + const found: GuardRoute[] = []; + + const constants = buildConstantMap(tree.rootNode); + + const visit = (node: SyntaxNode): void => { + if (node.type === 'binary_expression') collectFromComparison(node, found, constants); + else if (node.type === 'call_expression') collectFromRegexTest(node, found); + else if (node.type === 'switch_statement') collectFromSwitch(node, found, constants); + for (const child of node.namedChildren) visit(child); + }; + visit(tree.rootNode); + + return dedupeWithinFile(found).map((route) => ({ + filePath, + routePath: route.url, + httpMethod: route.verb ?? '', + decoratorName: DISPATCH_GUARD_SOURCE, + source: DISPATCH_GUARD_SOURCE, + lineNumber: route.line + lineOffset, + ...(route.handlerName ? { handlerName: route.handlerName } : {}), + })); +} + +function collectFromComparison(node: SyntaxNode, out: GuardRoute[], constants: ConstantMap): void { + const operator = node.childForFieldName('operator')?.text ?? ''; + if (!EQUALITY_OPERATORS.has(operator)) return; + const left = node.childForFieldName('left'); + const right = node.childForFieldName('right'); + if (left === null || right === null) return; + + for (const [expr, literal] of [ + [left, right], + [right, left], + ] as const) { + if (!isPathExpression(expr)) continue; + const value = literalValue(literal, constants); + if (value === null || !isPathLiteral(value)) continue; + const verb = governingVerb(node); + // A bare `/` is only a route when a verb says so — see the module header. + if (value === '/' && verb === null) continue; + out.push({ + url: value, + verb, + handlerName: enclosingHandlerName(node), + line: node.startPosition.row + 1, + }); + return; + } +} + +/** + * `switch (pathname) { case '/api/health': … }` — the other way to write the + * same dispatch, and the reason this module is not a rule about `if`. The + * discriminant carries the path signal for every arm at once, so each + * string-literal case is a route with no further evidence needed. + * + * Not reported by anyone; included because it is the same shape wearing + * different syntax, and waiting for a bug report per shape is how a graph stays + * permanently one idiom behind the code it indexes. + */ +function collectFromSwitch(node: SyntaxNode, out: GuardRoute[], constants: ConstantMap): void { + // The grammar wraps a switch discriminant in `parenthesized_expression`, + // unlike a comparison operand. + const discriminant = unparenthesize(node.childForFieldName('value')); + if (discriminant === null || !isPathExpression(discriminant)) return; + + const body = node.childForFieldName('body'); + if (body === null) return; + + // The verb governing the whole switch, if any (`if (req.method === 'GET') + // switch (pathname) { … }`). Read once — every arm shares it. + const verb = governingVerb(node); + + for (const arm of body.namedChildren) { + if (arm.type !== 'switch_case') continue; + const caseValue = arm.childForFieldName('value'); + if (caseValue === null) continue; + const value = literalValue(caseValue, constants); + if (value === null || !isPathLiteral(value)) continue; + if (value === '/' && verb === null) continue; + out.push({ + url: value, + verb, + handlerName: enclosingHandlerName(arm), + line: arm.startPosition.row + 1, + }); + } +} + +function collectFromRegexTest(node: SyntaxNode, out: GuardRoute[]): void { + const callee = node.childForFieldName('function'); + if (callee === null || callee.type !== 'member_expression') return; + if (callee.childForFieldName('property')?.text !== 'test') return; + const receiver = callee.childForFieldName('object'); + if (receiver === null || receiver.type !== 'regex') return; + + const argument = node.childForFieldName('arguments')?.namedChildren[0]; + if (argument === undefined || !isPathExpression(argument)) return; + + const pattern = receiver.childForFieldName('pattern'); + if (pattern === null) return; + const url = regexToRoutePath(pattern.text); + if (url === null) return; + + out.push({ + url, + verb: governingVerb(node), + handlerName: enclosingHandlerName(node), + line: node.startPosition.row + 1, + }); +} + +/** + * Collapse duplicate `(url, verb)` findings within one file, keeping the first — + * matching the routes phase's own first-writer-wins. The same comparison can + * legitimately appear more than once (an early-return guard and the branch that + * serves it), and each occurrence is the same route. + * + * The verb-less/verb-qualified reconciliation is deliberately NOT here — see + * {@link reconcileDispatchGuardRoutes}, which needs the whole repo to do it. + */ +function dedupeWithinFile(routes: readonly GuardRoute[]): GuardRoute[] { + const seen = new Set(); + const out: GuardRoute[] = []; + for (const route of routes) { + const key = `${route.verb ?? ''} ${route.url}`; + if (seen.has(key)) continue; + seen.add(key); + out.push(route); + } + return out; +} + +/** The minimum a route needs for reconciliation — structural, not nominal. */ +interface ReconcilableRoute { + readonly routePath: string; + readonly httpMethod: string; + readonly source?: string; +} + +/** + * Drop a dispatch-guard route whose URL is claimed WITH a verb somewhere in the + * repository. + * + * The idiom that makes this necessary is the split route table: one module lists + * every path it recognises (`isKnownApiPath`, or a `match(method, pathname)` + * that ORs them all) so the dispatcher can 404 early, and separate modules + * handle each path by verb. Both are path comparisons and both are real, but + * only the second is a route in the sense `route_map` reports — the first is a + * membership test. + * + * Left alone this doubles the map: measured on the reporting repo, 94 routes of + * which 34 were the table's verb-less shadow of a route already listed with its + * verb and its true handler. Reconciling per-FILE cannot see it, because the + * table and the handlers are different files; only the whole registry can. + * + * Applies to dispatch-guard routes only. A framework route with no verb is + * method-agnostic BY DECLARATION (a Django function view, a Laravel resource), + * which is a fact rather than a weaker observation, and must not be dropped. + */ +export function reconcileDispatchGuardRoutes( + routes: readonly T[], +): T[] { + const verbedUrls = new Set( + routes + .filter((r) => r.source === DISPATCH_GUARD_SOURCE && r.httpMethod !== '') + .map((r) => r.routePath), + ); + if (verbedUrls.size === 0) return [...routes]; + return routes.filter( + (r) => + !(r.source === DISPATCH_GUARD_SOURCE && r.httpMethod === '' && verbedUrls.has(r.routePath)), + ); +} diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 9b610a4f5..95b7d37b1 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -363,6 +363,16 @@ export interface ExtractedDecoratorRoute { * resolution then falls back (the Route node simply carries no handlerSymbolId). */ handlerName?: string; + /** + * Provenance for the `HANDLES_ROUTE` edge, overriding the default + * `decorator-`. Present when the route was extracted from a + * shape that is not a decorator at all — today, JS/TS dispatch guards + * (`route-extractors/dispatch-guard.ts`), where the route is INFERRED from a + * path comparison rather than DECLARED by an annotation. That distinction is + * the only thing that differs downstream, so it travels as a field instead of + * as a parallel extraction channel. + */ + source?: string; } /** diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index f0a3f0036..34f571dd4 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -304,7 +304,24 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // parse-time capture, so a warm cache replays ParsedFiles that already carry // everything it reads. Recorded because the reflex on this branch has been to // bump, and a bump nobody needs still forces every user a full re-parse. -const SCHEMA_BUMP = 49; +// +// 49 -> 50 IS needed for dispatch-guard routes (R3-7): the JS/TS providers now +// implement `extractDecoratorRoutes`, and decorator routes are worker output +// carried in the parse cache. A warm cache replays a worker result whose +// `decoratorRoutes` predates the extractor entirely, so every hand-rolled route +// stays invisible and `route_map` keeps answering empty — the exact symptom the +// change exists to fix, wearing the mask of "the extractor does not work". +// +// 50 -> 51 for the same-file constant folding that followed it. The v34 hazard +// again, and this branch has now tripped it TWICE: a build stamped 50 was used +// to analyze before folding existed, so caches stamped 50 carry the unfolded +// route set. Caught by measuring — the post-folding run came back suspiciously +// fast and would have reported the pre-folding number, which is precisely how +// "an intermediate build of the same series is a different capture set wearing +// the same number" shows up in practice. Within one PR the version only has to +// differ from main's; against a cache YOU wrote, it has to differ from itself. +// RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. +const SCHEMA_BUMP = 51; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/dispatch-guard-app/src/server/apiRouteTable.js b/gitnexus/test/fixtures/dispatch-guard-app/src/server/apiRouteTable.js new file mode 100644 index 000000000..1fe0c4102 --- /dev/null +++ b/gitnexus/test/fixtures/dispatch-guard-app/src/server/apiRouteTable.js @@ -0,0 +1,22 @@ +/** + * The path table, in a DIFFERENT file from the handlers — the shape that makes + * per-file reconciliation insufficient. The dispatcher calls this to 404 early; + * it is a membership test, not a route, and every path it lists is served with a + * verb by `liveRoutes.js` (except `/api/live/config`, which only lives here). + */ + +export function isKnownApiPath(pathname) { + if (pathname === '/api/live/portfolio') { + return true + } + + if (pathname === '/api/live/events') { + return true + } + + if (pathname === '/api/live/config') { + return true + } + + return false +} diff --git a/gitnexus/test/fixtures/dispatch-guard-app/src/server/autoTradeRoutes.js b/gitnexus/test/fixtures/dispatch-guard-app/src/server/autoTradeRoutes.js new file mode 100644 index 000000000..23137bfc0 --- /dev/null +++ b/gitnexus/test/fixtures/dispatch-guard-app/src/server/autoTradeRoutes.js @@ -0,0 +1,36 @@ +/** + * The composed-path idiom: every route is built from a base constant rather + * than written as a literal. Before constant folding this whole module was + * invisible — and, worse, indistinguishable from a module with no routes. + */ + +const AUTO_TRADE_BASE_PATH = '/api/live/auto-trade' + +export function createAutoTradeRoutes(ctx) { + return { + async handleAutoTrade(req, res, reqCtx) { + const { pathname } = reqCtx + const autoTradeBasePath = AUTO_TRADE_BASE_PATH + + if (req.method === 'GET' && pathname === `${autoTradeBasePath}/rules`) { + return ctx.listRules() + } + + if (req.method === 'POST' && pathname === `${autoTradeBasePath}/rules`) { + return ctx.createRule() + } + + if (req.method === 'GET' && pathname === AUTO_TRADE_BASE_PATH + '/positions') { + return ctx.listPositions() + } + + // Not foldable — `ruleId` is a runtime value, so no route is claimed + // rather than a wrong one. + if (req.method === 'DELETE' && pathname === `${autoTradeBasePath}/rules/${req.ruleId}`) { + return ctx.deleteRule() + } + + return null + }, + } +} diff --git a/gitnexus/test/fixtures/dispatch-guard-app/src/server/liveRoutes.js b/gitnexus/test/fixtures/dispatch-guard-app/src/server/liveRoutes.js new file mode 100644 index 000000000..35c765cb6 --- /dev/null +++ b/gitnexus/test/fixtures/dispatch-guard-app/src/server/liveRoutes.js @@ -0,0 +1,48 @@ +/** + * A route module in the shape a raw `node:http` server actually uses: a `match` + * that ORs the paths it owns, and a `handle` that dispatches each by verb. + * No framework, no decorator, no filesystem convention — the route exists only + * as a comparison. + */ + +export function createLiveRoutes(ctx) { + return { + match(method, pathname) { + return pathname === '/api/live/portfolio' || pathname === '/api/live/events' + }, + + async handle(req, res, reqCtx) { + const { pathname } = reqCtx + + if (req.method === 'GET' && pathname === '/api/live/portfolio') { + return sendJson(res, await ctx.stores.loadPortfolio()) + } + + if (req.method === 'POST' && pathname === '/api/live/portfolio') { + return sendJson(res, await ctx.stores.resetPortfolio()) + } + + if (req.method === 'GET' && pathname === '/api/live/events') { + return sendJson(res, await ctx.stores.loadEvents()) + } + + // Parameterised: an anchored regex is the only way to express a path + // segment variable without a router. + if (req.method === 'GET' && /^\/api\/live\/runs\/[^/]+$/.test(pathname)) { + return sendJson(res, await ctx.stores.loadRun(pathname)) + } + + return notFound(res) + }, + } +} + +function sendJson(res, body) { + res.writeHead(200, { 'content-type': 'application/json' }) + res.end(JSON.stringify(body)) +} + +function notFound(res) { + res.writeHead(404) + res.end() +} diff --git a/gitnexus/test/fixtures/dispatch-guard-app/src/server/staticServer.js b/gitnexus/test/fixtures/dispatch-guard-app/src/server/staticServer.js new file mode 100644 index 000000000..ffa2374e5 --- /dev/null +++ b/gitnexus/test/fixtures/dispatch-guard-app/src/server/staticServer.js @@ -0,0 +1,35 @@ +/** + * The negative half of the fixture: a static file server and a filesystem + * helper, both full of path-shaped string comparisons that are NOT routes. + * If any of these mint a Route node, the rule is too loose. + */ + +import path from 'node:path' + +export function serveStatic(req, res, pathname) { + // The bare-'/' normalisation idiom — a branch, not a route declaration. + const file = pathname === '/' ? '/index.html' : pathname + return readAsset(file, res) +} + +export function resolveCacheDir(dir) { + // `path` is the node:path module here; `/tmp/gitnexus-cache` is a directory. + if (path === '/tmp/gitnexus-cache') { + return dir + } + return null +} + +export function isApiRequest(pathname) { + // A namespace test, not a route: nothing serves `/api/`. + return pathname.startsWith('/api/') +} + +export function isNotHealth(pathname) { + // Inequality asserts the path is something ELSE. + return pathname !== '/api/live/health' +} + +function readAsset(file, res) { + res.end(file) +} diff --git a/gitnexus/test/integration/dispatch-guard-route-pipeline.test.ts b/gitnexus/test/integration/dispatch-guard-route-pipeline.test.ts new file mode 100644 index 000000000..a8fb9153a --- /dev/null +++ b/gitnexus/test/integration/dispatch-guard-route-pipeline.test.ts @@ -0,0 +1,177 @@ +/** + * End-to-end coverage of hand-rolled dispatch-guard route ingestion (R3-7). + * + * The reported symptom was a whole tool answering empty: `route_map` returned + * `{"routes": [], "total": 0, "message": "No routes found in this project."}` + * for a repo with seventeen route modules and 113 path comparisons. Every route + * extractor before this one requires a FRAMEWORK to declare the route, and a + * raw `node:http` server has none — it declares routes by comparing the path. + * + * The unit suite (`test/unit/dispatch-guard-routes.test.ts`) pins the + * extraction rules. This one pins the parts only the pipeline can prove: that + * the routes reach the graph as `Route` nodes, that they carry the verb and the + * dispatch-guard provenance, and that the handler resolves to a real symbol. + * + * The fixture also carries a static file server whose path comparisons must NOT + * become routes — precision is the property that matters most here, since + * `route_map` presents its output as fact. + */ + +import { describe, it, expect, beforeAll } from 'vitest'; +import path from 'node:path'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; +import type { PipelineResult } from '../../types/pipeline.js'; +import { DISPATCH_GUARD_SOURCE } from '../../src/core/ingestion/route-extractors/dispatch-guard.js'; + +const FIXTURE = path.resolve(__dirname, '..', 'fixtures', 'dispatch-guard-app'); + +describe('hand-rolled dispatch-guard route ingestion pipeline', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(FIXTURE, () => {}, {}); + }, 60_000); + + interface RouteView { + readonly name: string; + readonly method: string | undefined; + readonly handlerSymbolId: string | undefined; + } + + const routes = (): RouteView[] => { + const out: RouteView[] = []; + result.graph.forEachNode((n) => { + if (n.label !== 'Route') return; + out.push({ + name: String(n.properties.name), + method: n.properties.method as string | undefined, + handlerSymbolId: n.properties.handlerSymbolId as string | undefined, + }); + }); + return out.sort((a, b) => `${a.method} ${a.name}`.localeCompare(`${b.method} ${b.name}`)); + }; + + const routeNames = (): string[] => routes().map((r) => r.name); + + it('detects routes at all — the reported symptom was zero', () => { + // Asserted as its own case because every expectation below is vacuous if + // the pipeline emits no Route nodes; `toContain` on an empty array fails + // with a message about the missing element, not about the empty set. + expect(routes().length).toBeGreaterThan(0); + }); + + it('emits one Route per verb on a path dispatched by verb', () => { + const portfolio = routes().filter((r) => r.name === '/api/live/portfolio'); + expect(portfolio.map((r) => r.method).sort()).toEqual(['GET', 'POST']); + }); + + it('converts an anchored regex guard into a parameterised route', () => { + expect(routeNames()).toContain('/api/live/runs/{param1}'); + }); + + it('resolves the enclosing function as the route handler', () => { + const portfolioGet = routes().find( + (r) => r.name === '/api/live/portfolio' && r.method === 'GET', + ); + expect(portfolioGet).toBeDefined(); + // `handle` is the object-literal method that performs the dispatch. The id + // is asserted by shape rather than pinned, so a change to id formatting + // does not read as a resolution failure. + expect(portfolioGet?.handlerSymbolId).toMatch(/handle/); + }); + + // A whole route module built from a base constant. Before folding it produced + // nothing and looked identical to a module with no routes at all. + describe('paths composed from a same-file constant', () => { + it('folds a template substitution through an alias into a real route', () => { + const rules = routes().filter((r) => r.name === '/api/live/auto-trade/rules'); + expect(rules.map((r) => r.method).sort()).toEqual(['GET', 'POST']); + }); + + it('folds + concatenation of the base constant', () => { + expect(routeNames()).toContain('/api/live/auto-trade/positions'); + }); + + it('claims nothing when a substitution is a runtime value', () => { + expect(routeNames().some((n) => n.includes('auto-trade/rules/'))).toBe(false); + }); + + it('attributes them to the composing handler', () => { + const rule = routes().find((r) => r.name === '/api/live/auto-trade/rules'); + expect(rule?.handlerSymbolId).toMatch(/handleAutoTrade/); + }); + }); + + it('records dispatch-guard provenance on the HANDLES_ROUTE edge', () => { + const reasons: string[] = []; + result.graph.forEachRelationship((r) => { + if (r.type === 'HANDLES_ROUTE') reasons.push(String(r.reason)); + }); + expect(reasons.length).toBeGreaterThan(0); + // Not `decorator-…`: the route is INFERRED from a comparison, not DECLARED + // by an annotation, and the map should say which. + expect(reasons).toContain(DISPATCH_GUARD_SOURCE); + }); + + describe('precision — what the static server must NOT contribute', () => { + // Every assertion in this block is an absence, and an absence is satisfied + // just as well by a file that was never read. Prove it WAS read first, + // otherwise the whole block is decoration. + it('ingested the static server at all', () => { + const symbols: string[] = []; + result.graph.forEachNode((n) => { + if (String(n.properties.filePath ?? '').endsWith('staticServer.js')) { + symbols.push(String(n.properties.name)); + } + }); + expect(symbols).toContain('serveStatic'); + expect(symbols).toContain('resolveCacheDir'); + }); + + it('does not mint a route for the bare-"/" normalisation branch', () => { + expect(routeNames()).not.toContain('/'); + expect(routeNames()).not.toContain('/index.html'); + }); + + it('does not mint a route for a filesystem path comparison', () => { + expect(routeNames()).not.toContain('/tmp/gitnexus-cache'); + }); + + it('does not mint a route for a startsWith namespace test', () => { + expect(routeNames()).not.toContain('/api/'); + expect(routeNames()).not.toContain('/api'); + }); + + it('does not mint a route for an inequality comparison', () => { + expect(routeNames()).not.toContain('/api/live/health'); + }); + }); + + // The reconciliation that per-file logic cannot do. `apiRouteTable.js` is a + // membership test in a SEPARATE file from the handlers, so from inside either + // file alone both halves look like routes; only the whole registry can tell + // that `/api/live/events` is one route, not two. + describe('cross-file reconciliation of the split route table', () => { + const byName = (name: string) => routes().filter((r) => r.name === name); + + it('drops the table entry when a handler file claims the URL with a verb', () => { + expect(byName('/api/live/events').map((r) => r.method)).toEqual(['GET']); + expect( + byName('/api/live/portfolio') + .map((r) => r.method) + .sort(), + ).toEqual(['GET', 'POST']); + }); + + it('keeps a table entry no handler claims with a verb', () => { + // `/api/live/config` exists only in the table. Dropping it would trade a + // duplicate for a missing route. + expect(byName('/api/live/config').map((r) => r.method)).toEqual([undefined]); + }); + + it('attributes the surviving route to the handler file, not the table', () => { + const events = byName('/api/live/events')[0]; + expect(events?.handlerSymbolId).toMatch(/handle/); + }); + }); +}); diff --git a/gitnexus/test/unit/dispatch-guard-routes.test.ts b/gitnexus/test/unit/dispatch-guard-routes.test.ts new file mode 100644 index 000000000..a29c37d57 --- /dev/null +++ b/gitnexus/test/unit/dispatch-guard-routes.test.ts @@ -0,0 +1,551 @@ +/** + * Hand-rolled dispatch-guard route extraction. + * + * The gap this closes is a whole TOOL answering empty: `route_map` reported + * "No routes found in this project" for a repo with seventeen route modules, + * because every route extractor before this one needs a framework to declare + * the route. A raw `node:http` server declares it by comparing the path. + * + * The bar here is precision, not recall — `route_map` presents its output as + * fact, so a route that does not exist is worse than a route that is missing. + * Roughly half of these cases are therefore assertions that something is NOT + * extracted. + */ +import { describe, expect, it } from 'vitest'; +import Parser from 'tree-sitter'; +import JavaScript from 'tree-sitter-javascript'; +import TypeScript from 'tree-sitter-typescript'; +import { + extractDispatchGuardRoutes, + reconcileDispatchGuardRoutes, + regexToRoutePath, + DISPATCH_GUARD_SOURCE, +} from '../../src/core/ingestion/route-extractors/dispatch-guard.js'; + +const parser = new Parser(); +parser.setLanguage(JavaScript); + +const extract = (source: string, filePath = 'src/server/routes.js') => + extractDispatchGuardRoutes(parser.parse(source), filePath).map((r) => ({ + routePath: r.routePath, + httpMethod: r.httpMethod, + handlerName: r.handlerName, + source: r.source, + })); + +const paths = (source: string): string[] => extract(source).map((r) => r.routePath); + +describe('dispatch-guard route extraction', () => { + describe('the dominant idiom', () => { + it('extracts a verb-qualified path comparison', () => { + const routes = extract(` + export async function handle(req, res, reqCtx) { + const { pathname } = reqCtx + if (req.method === 'GET' && pathname === '/api/live/portfolio') { + return sendJson(res, await loadPortfolio()) + } + } + `); + expect(routes).toEqual([ + { + routePath: '/api/live/portfolio', + httpMethod: 'GET', + handlerName: 'handle', + source: DISPATCH_GUARD_SOURCE, + }, + ]); + }); + + it('reads the verb when the comparison order is reversed', () => { + expect( + extract(` + function handle(req) { + if ('POST' === req.method && '/api/orders' === pathname) { return 1 } + } + `), + ).toMatchObject([{ routePath: '/api/orders', httpMethod: 'POST' }]); + }); + + it('distributes an outer verb across an inner OR of paths', () => { + const routes = extract(` + function handle(req) { + if (req.method === 'GET' && (pathname === '/api/a' || pathname === '/api/b')) { return 1 } + } + `); + expect(routes).toMatchObject([ + { routePath: '/api/a', httpMethod: 'GET' }, + { routePath: '/api/b', httpMethod: 'GET' }, + ]); + }); + + it('inherits a verb from an ENCLOSING if, not just a sibling', () => { + expect( + extract(` + function handle(req) { + if (req.method === 'DELETE') { + if (pathname === '/api/session') { return 1 } + } + } + `), + ).toMatchObject([{ routePath: '/api/session', httpMethod: 'DELETE' }]); + }); + + // The inverted case, and the reason the ancestor walk tracks which branch it + // came from: in the `else`, the method is precisely NOT POST, so inheriting + // POST would label the route with the one verb it cannot have. + it('refuses to inherit a verb from an if whose ELSE branch holds the comparison', () => { + const routes = extract(` + function handle(req) { + if (req.method === 'POST') { + save() + } else if (pathname === '/api/report') { + return 1 + } + } + `); + expect(routes).toMatchObject([{ routePath: '/api/report', httpMethod: '' }]); + }); + + it('extracts a verb-less path guard', () => { + expect( + extract(` + function match(method, pathname) { + return pathname === '/api/health' + } + `), + ).toMatchObject([{ routePath: '/api/health', httpMethod: '', handlerName: 'match' }]); + }); + }); + + describe('what must NOT become a route', () => { + it('ignores a comparison against something that is not a request path', () => { + // `mode` is not a path expression, so `/full` is just a string. + expect(paths(`function f() { if (mode === '/full') { return 1 } }`)).toEqual([]); + }); + + it('ignores a path-shaped literal compared to a filesystem path variable', () => { + // `path` is excluded on purpose — in Node it is overwhelmingly node:path + // or a file location, never the request path. + expect(paths(`function f() { if (path === '/tmp/cache') { return 1 } }`)).toEqual([]); + }); + + it('ignores startsWith namespace tests', () => { + // A prefix test asks "do I own this?" — minting `/api/` would claim a + // route nothing serves. + expect( + paths(`function f() { if (pathname.startsWith('/api/')) { return route(pathname) } }`), + ).toEqual([]); + }); + + it('ignores a bare "/" normalisation with no verb', () => { + // The static-file idiom, verbatim from the reporting repo. + expect( + paths(`function serve() { const file = pathname === '/' ? '/index.html' : pathname }`), + ).toEqual([]); + }); + + it('DOES extract a bare "/" when a verb makes the intent unambiguous', () => { + expect( + extract( + `function handle(req) { if (req.method === 'GET' && pathname === '/') { return 1 } }`, + ), + ).toMatchObject([{ routePath: '/', httpMethod: 'GET' }]); + }); + + it('ignores a non-equality comparison', () => { + expect(paths(`function f() { if (pathname !== '/api/health') { return 1 } }`)).toEqual([]); + }); + + it('ignores an absolute URL', () => { + expect( + paths(`function f() { if (pathname === 'https://x.test/api/a') { return 1 } }`), + ).toEqual([]); + }); + + it('ignores a template string whose substitution is not a known constant', () => { + expect(paths('function f() { if (pathname === `/api/${id}`) { return 1 } }')).toEqual([]); + }); + + it('ignores a verb literal that is not an HTTP verb', () => { + const routes = extract(` + function handle(req) { + if (req.method === 'SUBSCRIBE' && pathname === '/api/feed') { return 1 } + } + `); + expect(routes).toMatchObject([{ routePath: '/api/feed', httpMethod: '' }]); + }); + }); + + // Not in any report — the same dispatch written with different syntax. A + // graph that waits for a bug report per shape stays permanently one idiom + // behind the code it indexes. + describe('switch dispatch', () => { + it('extracts every string-literal case of a switch on the path', () => { + expect( + extract(` + function handle(req, pathname) { + switch (pathname) { + case '/api/health': return ok() + case '/api/version': return version() + default: return notFound() + } + } + `), + ).toMatchObject([ + { routePath: '/api/health', httpMethod: '', handlerName: 'handle' }, + { routePath: '/api/version', httpMethod: '', handlerName: 'handle' }, + ]); + }); + + it('applies a verb governing the whole switch to every arm', () => { + expect( + extract(` + function handle(req, pathname) { + if (req.method === 'POST') { + switch (pathname) { + case '/api/a': return a() + case '/api/b': return b() + } + } + } + `), + ).toMatchObject([ + { routePath: '/api/a', httpMethod: 'POST' }, + { routePath: '/api/b', httpMethod: 'POST' }, + ]); + }); + + it('ignores a switch on something that is not a request path', () => { + // The file must mention a path token, or PATH_TOKEN_HINT skips it before + // the discriminant rule is ever consulted and this asserts nothing. The + // real route below is the proof the walk ran. + expect( + paths(` + function f(kind, pathname) { + switch (kind) { case '/full': return 1 } + if (pathname === '/api/real') { return 2 } + } + `), + ).toEqual(['/api/real']); + }); + + it('ignores non-path cases in a switch that is on the path', () => { + expect( + paths(` + function handle(pathname) { + switch (pathname) { + case '/api/a': return 1 + case 'unknown': return 2 + } + } + `), + ).toEqual(['/api/a']); + }); + }); + + // A composed path is not an exotic shape — one of the reporting repo's + // seventeen route modules writes every one of its ~20 routes this way, and + // without folding that file contributes NOTHING while looking exactly like a + // file that has no routes. + describe('paths composed from same-file constants', () => { + it('folds a template substitution naming a module-level constant', () => { + expect( + extract( + 'const BASE = "/api/live/auto-trade"\n' + + 'function handle(req) {\n' + + ' if (req.method === "GET" && pathname === `${BASE}/rules`) { return 1 }\n' + + '}', + ), + ).toMatchObject([{ routePath: '/api/live/auto-trade/rules', httpMethod: 'GET' }]); + }); + + it('follows an alias hop, which is how the reporting repo writes it', () => { + // `const autoTradeBasePath = AUTO_TRADE_BASE_PATH` inside the handler, + // with the literal at module scope. + expect( + paths( + 'const AUTO_TRADE_BASE_PATH = "/api/live/auto-trade"\n' + + 'function handle(req) {\n' + + ' const autoTradeBasePath = AUTO_TRADE_BASE_PATH\n' + + ' if (pathname === `${autoTradeBasePath}/positions`) { return 1 }\n' + + '}', + ), + ).toEqual(['/api/live/auto-trade/positions']); + }); + + it('folds + concatenation', () => { + expect( + paths( + 'const BASE = "/api/v2"\n' + + 'function handle() { if (pathname === BASE + "/orders") { return 1 } }', + ), + ).toEqual(['/api/v2/orders']); + }); + + it('folds a bare constant with no suffix', () => { + expect( + paths( + 'const HEALTH = "/api/health"\nfunction handle() { if (pathname === HEALTH) { return 1 } }', + ), + ).toEqual(['/api/health']); + }); + + // The refusals. A partially-folded path is a WRONG route, and a wrong route + // is worse than a missing one — the whole premise of this module. + it('refuses a name declared twice with different values', () => { + expect( + paths( + 'const BASE = "/api/a"\n' + + 'function other() { const BASE = "/api/b"; return BASE }\n' + + 'function handle() { if (pathname === `${BASE}/x`) { return 1 } }', + ), + ).toEqual([]); + }); + + it('refuses when only part of the template resolves', () => { + expect( + paths( + 'const BASE = "/api"\n' + + 'function handle(id) { if (pathname === `${BASE}/x/${id}`) { return 1 } }', + ), + ).toEqual([]); + }); + + it('refuses a constant bound to a call result', () => { + expect( + paths( + 'const BASE = buildBase()\nfunction handle() { if (pathname === `${BASE}/x`) { return 1 } }', + ), + ).toEqual([]); + }); + + it('still rejects a folded value that is not path-shaped', () => { + expect( + paths( + 'const MODE = "full"\nfunction handle() { if (pathname === `${MODE}/x`) { return 1 } }', + ), + ).toEqual([]); + }); + }); + + describe('parameterised routes from anchored regexes', () => { + it('converts a single-segment wildcard to a named parameter', () => { + expect( + extract(` + function handle(req) { + if (req.method === 'GET' && /^\\/api\\/research-runs\\/[^/]+$/.test(pathname)) { return 1 } + } + `), + ).toMatchObject([{ routePath: '/api/research-runs/{param1}', httpMethod: 'GET' }]); + }); + + it('numbers multiple parameters in order', () => { + expect(regexToRoutePath('^\\/api\\/runs\\/[^/]+\\/experiments\\/[^/]+$')).toBe( + '/api/runs/{param1}/experiments/{param2}', + ); + }); + + it('accepts an escaped slash inside the wildcard class', () => { + expect(regexToRoutePath('^\\/api\\/x\\/[^\\/]+$')).toBe('/api/x/{param1}'); + }); + + // Bail cases. A route path is a claim about what the server serves, so a + // pattern that cannot be translated exactly is dropped, not approximated. + it('refuses an unanchored pattern', () => { + expect(regexToRoutePath('\\/api\\/x')).toBeNull(); + expect(regexToRoutePath('^\\/api\\/x')).toBeNull(); + }); + + it('refuses an optional group', () => { + expect(regexToRoutePath('^\\/api\\/runs\\/[^/]+\\/artifacts(?:\\/.*)?$')).toBeNull(); + }); + + it('refuses an alternation and a bare wildcard', () => { + expect(regexToRoutePath('^\\/api\\/(a|b)$')).toBeNull(); + expect(regexToRoutePath('^\\/api\\/.*$')).toBeNull(); + }); + + it('refuses a character-class escape', () => { + expect(regexToRoutePath('^\\/api\\/runs\\/\\d+$')).toBeNull(); + }); + + it('ignores a regex tested against something that is not a request path', () => { + expect(paths(`function f() { if (/^\\/api\\/x$/.test(filename)) { return 1 } }`)).toEqual([]); + }); + }); + + describe('handler attribution', () => { + it('names an object-literal method handler', () => { + // The route-module shape the reporting repo uses throughout. + expect( + extract(` + export function createRoutes(ctx) { + return { + async handle(req, res, reqCtx) { + const { pathname } = reqCtx + if (req.method === 'GET' && pathname === '/api/live/events') { return 1 } + }, + } + } + `), + ).toMatchObject([{ routePath: '/api/live/events', handlerName: 'handle' }]); + }); + + it('names an arrow function bound to a const', () => { + expect( + extract(` + const dispatch = (req) => { + if (req.method === 'GET' && pathname === '/api/ping') { return 1 } + } + `), + ).toMatchObject([{ routePath: '/api/ping', handlerName: 'dispatch' }]); + }); + + it('reports no handler for a top-level comparison', () => { + expect(extract(`if (pathname === '/api/top') { go() }`)).toMatchObject([ + { routePath: '/api/top', handlerName: undefined }, + ]); + }); + }); + + describe('per-file dedup', () => { + it('collapses a repeated (url, verb) pair', () => { + const routes = extract(` + function handle(req) { + if (req.method === 'GET' && pathname === '/api/a') { return 1 } + if (req.method === 'GET' && pathname === '/api/a') { return 2 } + } + `); + expect(routes).toHaveLength(1); + }); + + it('keeps distinct verbs on the same URL as separate routes', () => { + expect( + extract(` + function handle(req) { + if (req.method === 'GET' && pathname === '/api/a') { return 1 } + if (req.method === 'DELETE' && pathname === '/api/a') { return 2 } + } + `), + ).toHaveLength(2); + }); + }); + + // Whole-repo reconciliation. Deliberately NOT per-file: the reporting repo + // keeps its path table (`isKnownApiPath`) in one module and its handlers in + // sixteen others, so a per-file rule sees each half separately and the map + // ends up listing every route twice — once verb-less with the table as its + // "handler", once properly. Measured there: 94 routes, 34 of them shadows. + describe('cross-file reconciliation', () => { + const route = (routePath: string, httpMethod: string, source = DISPATCH_GUARD_SOURCE) => ({ + routePath, + httpMethod, + source, + }); + + it('drops a verb-less guard route when another file claims the URL with a verb', () => { + expect( + reconcileDispatchGuardRoutes([ + route('/api/live/health', ''), // the table + route('/api/live/health', 'GET'), // the handler + ]), + ).toEqual([route('/api/live/health', 'GET')]); + }); + + it('keeps a verb-less guard route no verb claims', () => { + const only = [route('/api/plans/examples/{param1}', ''), route('/api/other', 'GET')]; + expect(reconcileDispatchGuardRoutes(only)).toEqual(only); + }); + + it('keeps every verb on a multi-verb URL', () => { + const multi = [route('/api/x', 'GET'), route('/api/x', 'POST'), route('/api/x', '')]; + expect(reconcileDispatchGuardRoutes(multi)).toEqual([ + route('/api/x', 'GET'), + route('/api/x', 'POST'), + ]); + }); + + // A framework route without a verb is method-agnostic BY DECLARATION — a + // Django function view, a Laravel resource. That is a fact, not a weaker + // observation of the same thing, so the rule must not reach it. + it('never drops a non-dispatch-guard route', () => { + const mixed = [ + { routePath: '/api/x', httpMethod: '', source: undefined }, + route('/api/x', 'GET'), + ]; + expect(reconcileDispatchGuardRoutes(mixed)).toEqual(mixed); + }); + + it('does not let a framework verb suppress a guard route', () => { + const mixed = [ + route('/api/x', ''), + { routePath: '/api/x', httpMethod: 'GET', source: undefined }, + ]; + expect(reconcileDispatchGuardRoutes(mixed)).toEqual(mixed); + }); + }); + + // Both providers are wired to this extractor, and TypeScript is where the + // grammar can differ — an annotated parameter, a non-null assertion, an `as` + // cast all wrap nodes the rules read. Asserted rather than assumed. + describe('TypeScript', () => { + const tsParser = new Parser(); + tsParser.setLanguage(TypeScript.typescript); + + const tsPaths = (source: string): string[] => + extractDispatchGuardRoutes(tsParser.parse(source), 'src/server/routes.ts').map( + (r) => r.routePath, + ); + + it('extracts through annotated parameters', () => { + expect( + tsPaths(` + export async function handle(req: IncomingMessage, pathname: string): Promise { + if (req.method === 'GET' && pathname === '/api/live/portfolio') { return } + } + `), + ).toEqual(['/api/live/portfolio']); + }); + + it('extracts a switch on a typed discriminant', () => { + expect( + tsPaths(` + function handle(pathname: string): number { + switch (pathname) { + case '/api/health': return 1 + default: return 0 + } + } + `), + ).toEqual(['/api/health']); + }); + + it('folds a typed constant', () => { + expect( + tsPaths( + 'const BASE: string = "/api/v1"\n' + + 'function handle(pathname: string) { if (pathname === `${BASE}/orders`) { return 1 } }', + ), + ).toEqual(['/api/v1/orders']); + }); + }); + + describe('path expressions the rule accepts', () => { + it('accepts a member access ending in .pathname', () => { + expect( + paths(`function f(req) { if (new URL(req.url, base).pathname === '/api/x') { return 1 } }`), + ).toEqual(['/api/x']); + }); + + it('accepts raw req.url', () => { + expect(paths(`function f(req) { if (req.url === '/api/x') { return 1 } }`)).toEqual([ + '/api/x', + ]); + }); + + it('rejects a bare .url on an unrelated receiver', () => { + // `link.url` is not a request path; only req/request carry the raw form. + expect(paths(`function f(link) { if (link.url === '/api/x') { return 1 } }`)).toEqual([]); + }); + }); +}); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 192360e9d..761ee3c0e 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -141,8 +141,15 @@ describe('PARSE_CACHE_VERSION', () => { // a row. Same lesson as the note above — the pin cannot detect the tie, since // both sides asserted `toBe(45)` and that passes while main is already 45. // Only the merge-time diff against origin/main surfaces it. - it('pins SCHEMA_BUMP to 49 so concurrent bumps cannot silently collide (#2766)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(49); + // Moved 49 -> 50 for dispatch-guard routes (R3-7): the JS/TS providers now + // implement `extractDecoratorRoutes`, and decorator routes are worker output + // carried in the cache. A v49 warm cache replays a worker result whose + // `decoratorRoutes` predates the extractor, so `route_map` keeps answering + // empty — the exact symptom the change fixes, disguised as "it does not work". + // Moved 50 -> 51 for the same-file constant folding that followed, because a + // build stamped 50 had already been used to analyze without it. + it('pins SCHEMA_BUMP to 51 so concurrent bumps cannot silently collide (#2766)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(51); }); it('embeds the gitnexus package version (so upgrades invalidate the cache)', () => {