From 49ffd8e316956d1f3502aa40348f5dcb37c45e18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 23 Jun 2026 12:12:49 +0100 Subject: [PATCH 1/2] feat(group): resolve cross-file named HTTP handlers (#2275) (#2277) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(group): resolve cross-file named HTTP handlers via unique repo-wide lookup U1 of #2275. When a provider's named handler is defined in a file other than its route registration (e.g. router.get('/x', listUsers) with listUsers imported), the registration file's symbols don't contain it, so resolution fell back to the file-level boundary. Add a repo-wide name query (RESOLVE_BY_NAME_QUERY, the label-union pattern from manifest-extractor) consulted only after the file-scoped lookup misses, and honored ONLY when exactly one Function/Method/CodeElement carries that name (zero/many → keep the file fallback, no wrong-symbol attribution). Provider-only, cached by name. 4 unit tests; 743 group tests pass. * test(bench): cross-file named handler scenario (end-to-end proof of #2275) U2 of #2275. Adds a fifth bench scenario: a backend route whose handler (listUsers) is imported from another file than its registration, with a frontend consumer. Asserts the provider resolves to the handler via the repo-wide unique name lookup (sym=listUsers, uid set) and that the cross-repo trace is symbol- precise (no file-level fallback). verify.mjs now 12/12 on the real pipeline. * fix(review): apply autofix feedback ce-code-review (autofix) — no correctness/security findings; applied test-coverage + robustness fixes: repo-wide query throw -> empty (no exception); by-name lookup cache fires once across same-named handlers; consumers never consult the repo-wide lookup; same-file-wins now asserts the global path is bypassed; bench provider find scoped by contractId; clarified the uniqueness-guard comment. 167 extractor tests. * fix(group): tri-review fixes for cross-file handler resolution Two-engine PR tri-review (Claude swarm+ce, Codex gpt-5.5 swarm+ce+adversarial) on #2277. Correctness/security clean (injection refuted, bind-param). Fixes: - Named-provider wrapper-attach (Codex swarm P1 + Claude ce-adversarial, cross-engine): a named handler that fails both name lookups no longer falls through to line-span containment, which attached the route to the enclosing registrar (e.g. a setupRoutes() wrapper) instead of leaving it empty. Containment now applies only to consumers and inline-arrow providers. - CodeElement/ORM empty-file nodes (Claude ce-adversarial reproduced + ce-maintainability): RESOLVE_BY_NAME_QUERY gains 'AND n.filePath <> ""' so a handler name colliding with a synthetic ORM model node (orm.ts emits filePath:'') neither resolves to an edge-less node nor inflates the uniqueness count and masks the real handler; + a defensive empty-filePath guard in resolveSymbolByNameUnique. Added LIMIT 2 (Codex swarm P3 + ce-maintainability) to bound homonym materialization (count guard stays exact). - Documented the aliased-import limitation (Codex adversarial): the route-site identifier is the local alias, fix deferred to #2275 import narrowing. - README expected verdict 9/9 -> 12/12 (Codex swarm+ce P3). Tests: +3 (wrapper-no-attach, empty-filePath reject, empty-registration-file resolves) covering the cross-engine gaps. 170 extractor / 748 group+integration pass; bench 12/12 end-to-end. * feat(group): import-pinned handler resolution (fixes deferred alias case) Resolves the tri-review's deferred item: cross-file named handlers are now pinned to their import's target module instead of resolved by name alone, so aliases and names that collide with a local symbol resolve correctly. - node.ts builds a local-binding -> {declared name, module} map from the file's named imports; the express handler emits the DECLARED name + a handlerImport {name, module} (HttpDetection gains the optional field). - resolveDetectionSymbol gains an imported-handler rung: resolveImportedSymbol pins to the import's target file via RESOLVE_IN_MODULE_QUERY (n.name= AND filePath STARTS WITH the resolved module path), unique-match only. An imported handler never uses file-scoped lookup (it is defined elsewhere); on a module miss it falls back to a unique repo-wide name match on the DECLARED name, then null. Relative imports only; bare/non-relative imports keep the repo-wide fallback. Cached by (module-prefix, name). - Closes the Codex-adversarial alias finding: import { listUsers as handleUsers } + an unrelated handleUsers no longer mis-resolves — the route resolves to the imported listUsers in its module, and the alias is never looked up. - Shared toResolvedSymbol helper (dedups the row->symbol + empty-filePath guard). Tests: alias-resolves-to-declared-name + module-pin-resolves-ambiguous-name unit tests; same-file-wins reworked to a genuinely LOCAL handler. Bench scenario 6 (aliased import with a decoy) proves it end-to-end. 172 extractor / 751 group+integration pass; bench 14/14. * feat(group): import-pinned resolution for Python aliased handlers Extends the JS/TS import-pinning to Python. The Python analog of express router.get(path, handler) is Flask's imperative add_url_rule(view_func=...), whose view is often an imported (aliased) symbol. - New Flask add_url_rule provider pattern (path + view_func handler + methods; default GET, methods=[...] honored). High Flask-specificity keeps false positives low — unlike bare path()/Route(), which the plugin deliberately leaves to graph Route nodes. - buildPythonImportMap resolves 'from .mod import name as alias' (and plain 'from mod import name') to the declared name + raw module spec. - resolveModuleBase generalized to two relative-import dialects: path-style (JS './h/users') and dotted (Python '.handlers.users', '..pkg.users' — leading dots are package levels). Bare/absolute imports keep the repo-wide fallback. - Django stays graph-resolved (handlerSymbolId); FastAPI/Flask decorators stay same-file (decorated function). This only adds the imperative imported-view case Python lacked. Tests: Flask aliased add_url_rule unit test (relative dotted module pinned, alias never queried) + bench scenario 7 (end-to-end, 16/16). 173 extractor / 752 group+integration pass. --- gitnexus/bench/cross-repo-trace/README.md | 14 +- gitnexus/bench/cross-repo-trace/verify.mjs | 171 ++++++++ .../group/extractors/http-patterns/node.ts | 54 ++- .../group/extractors/http-patterns/python.ts | 124 ++++++ .../group/extractors/http-patterns/types.ts | 11 + .../group/extractors/http-route-extractor.ts | 161 ++++++- .../unit/group/http-route-extractor.test.ts | 408 ++++++++++++++++++ 7 files changed, 936 insertions(+), 7 deletions(-) diff --git a/gitnexus/bench/cross-repo-trace/README.md b/gitnexus/bench/cross-repo-trace/README.md index 93781fe9a..3d5a215f7 100644 --- a/gitnexus/bench/cross-repo-trace/README.md +++ b/gitnexus/bench/cross-repo-trace/README.md @@ -13,7 +13,7 @@ node bench/cross-repo-trace/verify.mjs `verify.mjs` is self-contained — it generates each fixture inline, runs the real analyze → sync → trace/impact pipeline, and prints PASS/FAIL per assertion -(exit non-zero on any failure). Expected verdict: **9/9 checks passed**. +(exit non-zero on any failure). Expected verdict: **16/16 checks passed**. ## Cases covered (one scenario each) @@ -32,6 +32,18 @@ analyze → sync → trace/impact pipeline, and prints PASS/FAIL per assertion 4. **Multi-language (Python)** — a Flask provider + `requests` consumer; asserts the Python line wiring resolves the consumer and the cross-repo `trace` stitches `fetch_items -> list_items`. +5. **Cross-file named handler** (#2275) — a route whose handler (`listUsers`) is + imported from another file than its registration. Asserts the provider + resolves to the handler via the import-pinned module lookup, and the trace is + symbol-precise (no file-level fallback). +6. **Aliased cross-file import** (#2275) — `import { listUsers as handleUsers }` + with an unrelated decoy `handleUsers` elsewhere. Asserts the route resolves + through the import to the declared `listUsers` (not the alias or the decoy), + proving import-pinned resolution. +7. **Python aliased import** (#2275) — a Flask `add_url_rule('/api/users', + view_func=handle_users)` whose view is `from .handlers.users import list_users + as handle_users`. Asserts the handler resolves through Python's dotted + relative module to `list_users`, symbol-precise. The **ambiguous-destination** (a file making several HTTP calls whose consumer contracts have no resolved uid) and **degraded-member** (a member DB that throws diff --git a/gitnexus/bench/cross-repo-trace/verify.mjs b/gitnexus/bench/cross-repo-trace/verify.mjs index fa63ff47a..8497af38d 100644 --- a/gitnexus/bench/cross-repo-trace/verify.mjs +++ b/gitnexus/bench/cross-repo-trace/verify.mjs @@ -271,6 +271,177 @@ def fetch_items(): fs.rmSync(home, { recursive: true, force: true }); } +// ── Scenario: Python Flask add_url_rule with an ALIASED relative import — +// import-pinned resolution across Python's dotted module syntax. ─────────── +line('\n## Scenario: Python aliased import (Flask add_url_rule) — import-pinned'); +{ + const { sync, backend, home } = await setup( + 'pyalias', + { + 'pyalias-backend': { + 'app/handlers/users.py': `def list_users(): + return [] +`, + 'app/routes.py': `from flask import Flask +from .handlers.users import list_users as handle_users +app = Flask(__name__) +app.add_url_rule('/api/users', view_func=handle_users) +`, + }, + 'pyalias-frontend': { + 'client.py': `import requests + +def fetch_users(): + return requests.get('/api/users').json() +`, + }, + }, + 'pyalias-group', + { 'app/backend': 'pyalias-backend', 'app/frontend': 'pyalias-frontend' }, + ); + + const provider = sync.contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/users', + ); + check( + provider?.symbolName === 'list_users', + 'Python Flask aliased view resolves through the relative import to list_users', + `sym=${provider?.symbolName} uid=${provider?.symbolUid ? 'set' : 'empty'}`, + ); + + const tr = await backend.callTool('trace', { + repo: '@pyalias-group', + from: 'fetch_users', + to: 'list_users', + }); + check( + tr.status === 'ok' && crossingId(tr) === 'http::GET::/api/users' && !hasNote(tr, 'FILE'), + 'Python aliased-import trace is symbol-precise (no file-level fallback)', + `status=${tr.status} crossing=${crossingId(tr)}`, + ); + + fs.rmSync(home, { recursive: true, force: true }); +} + +// ── Scenario: cross-file named handler (#2275) — repo-wide unique resolution ── +line('\n## Scenario: cross-file named handler — repo-wide unique resolution'); +{ + const { sync, backend, home } = await setup( + 'xfile', + { + 'xfile-backend': { + 'src/handlers/users.ts': `export function listUsers(req: { body: unknown }, res: { json: (v: unknown) => void }) { + res.json([]); +} +`, + 'src/routes.ts': `import { Router } from 'express'; +import { listUsers } from './handlers/users'; +const router = Router(); +router.get('/api/users', listUsers); +export default router; +`, + 'package.json': '{ "name": "xfile-backend", "version": "1.0.0" }', + }, + 'xfile-frontend': { + 'src/api.ts': `export async function fetchUsers() { + const r = await fetch('/api/users'); + return r.json(); +} +`, + 'package.json': '{ "name": "xfile-frontend", "version": "1.0.0" }', + }, + }, + 'xfile-group', + { 'app/backend': 'xfile-backend', 'app/frontend': 'xfile-frontend' }, + ); + + const provider = sync.contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/users', + ); + check( + Boolean(provider?.symbolUid) && provider?.symbolName === 'listUsers', + 'cross-file provider resolves to the handler defined in another file (repo-wide unique)', + `sym=${provider?.symbolName} uid=${provider?.symbolUid ? 'set' : 'empty'}`, + ); + + const tr = await backend.callTool('trace', { + repo: '@xfile-group', + from: 'fetchUsers', + to: 'listUsers', + pdg: true, + }); + check( + tr.status === 'ok' && crossingId(tr) === 'http::GET::/api/users' && !hasNote(tr, 'FILE'), + 'cross-file trace is symbol-precise (no file-level fallback)', + `status=${tr.status} crossing=${crossingId(tr)}`, + ); + + fs.rmSync(home, { recursive: true, force: true }); +} + +// ── Scenario: ALIASED cross-file import — resolved through the import to the +// declared symbol, not the local alias (and not a same-named decoy). ─────── +line('\n## Scenario: aliased cross-file import — import-pinned resolution'); +{ + const { sync, backend, home } = await setup( + 'alias', + { + 'alias-backend': { + 'src/handlers/users.ts': `export function listUsers(req: { body: unknown }, res: { json: (v: unknown) => void }) { + res.json([]); +} +`, + // Decoy: a DIFFERENT, unrelated symbol named handleUsers. Name-only + // resolution of the local alias would wrongly pick this one. + 'src/util.ts': `export function handleUsers() { + return 1; +} +`, + 'src/routes.ts': `import { Router } from 'express'; +import { listUsers as handleUsers } from './handlers/users'; +const router = Router(); +router.get('/api/users', handleUsers); +export default router; +`, + 'package.json': '{ "name": "alias-backend", "version": "1.0.0" }', + }, + 'alias-frontend': { + 'src/api.ts': `export async function fetchUsers() { + const r = await fetch('/api/users'); + return r.json(); +} +`, + 'package.json': '{ "name": "alias-frontend", "version": "1.0.0" }', + }, + }, + 'alias-group', + { 'app/backend': 'alias-backend', 'app/frontend': 'alias-frontend' }, + ); + + const provider = sync.contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/users', + ); + check( + provider?.symbolName === 'listUsers', + 'aliased handler resolves through the import to the declared symbol (not the alias/decoy)', + `sym=${provider?.symbolName} uid=${provider?.symbolUid ? 'set' : 'empty'}`, + ); + + const tr = await backend.callTool('trace', { + repo: '@alias-group', + from: 'fetchUsers', + to: 'listUsers', + pdg: true, + }); + check( + tr.status === 'ok' && crossingId(tr) === 'http::GET::/api/users' && !hasNote(tr, 'FILE'), + 'aliased-import trace is symbol-precise (no file-level fallback)', + `status=${tr.status} crossing=${crossingId(tr)}`, + ); + + fs.rmSync(home, { recursive: true, force: true }); +} + // ── Summary ──────────────────────────────────────────────────────────────── const passed = results.filter((r) => r.pass).length; line(`\n## Verdict: ${passed}/${results.length} checks passed`); diff --git a/gitnexus/src/core/group/extractors/http-patterns/node.ts b/gitnexus/src/core/group/extractors/http-patterns/node.ts index b316cbf6e..0d1298f72 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/node.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/node.ts @@ -295,8 +295,53 @@ function findDecoratedMethod(decoratorNode: Parser.SyntaxNode): Parser.SyntaxNod return null; } +/** + * Map each named import's LOCAL binding to its DECLARED export name and source + * module, by walking the file's `import { x as y } from 'm'` statements. Lets + * the express handler resolve through an alias (the local `y`) to the real + * symbol (`x` in `m`) instead of looking up the alias text. Only named imports + * are mapped — default and namespace imports are left to fall through as + * locally-scoped identifiers. + */ +function buildImportMap(tree: Parser.Tree): Map { + const map = new Map(); + const walk = (node: Parser.SyntaxNode): void => { + if (node.type === 'import_statement') { + const sourceNode = node.childForFieldName('source'); + const module = sourceNode ? unquoteLiteral(sourceNode.text) : null; + if (module !== null) { + const collect = (n: Parser.SyntaxNode): void => { + if (n.type === 'import_specifier') { + const nameNode = n.childForFieldName('name'); + const aliasNode = n.childForFieldName('alias'); + const local = aliasNode ?? nameNode; + if (nameNode && local && local.type === 'identifier') { + map.set(local.text, { name: nameNode.text, module }); + } + } + for (let i = 0; i < n.namedChildCount; i++) { + const c = n.namedChild(i); + if (c) collect(c); + } + }; + collect(node); + } + } + for (let i = 0; i < node.namedChildCount; i++) { + const c = node.namedChild(i); + if (c) walk(c); + } + }; + walk(tree.rootNode); + return map; +} + function scanBundle(bundle: NodePatternBundle, tree: Parser.Tree): HttpDetection[] { const out: HttpDetection[] = []; + // Local-binding → { declared export name, module } for the file's named + // imports, so an express handler that is an imported (possibly aliased) + // symbol resolves to the real definition rather than its local alias text. + const importMap = buildImportMap(tree); // NestJS: collect `@Controller('prefix')` class decorators, keyed by // the `class_declaration` they decorate. @@ -364,14 +409,19 @@ function scanBundle(bundle: NodePatternBundle, tree: Parser.Tree): HttpDetection // → `listUsers`) so a named handler resolves by name. For an inline/anonymous // handler emit `name: null` (NOT the sentinel `'handler'`) so the resolver // does NOT match an unrelated function that happens to be named `handler` — - // it uses the registration line for containment instead. + // it uses the registration line for containment instead. When the handler is + // an imported (possibly aliased) symbol, carry the resolved import so the + // extractor can pin it to the source module rather than the local alias text. const handlerNode = match.captures.handler; + const localHandler = handlerNode?.type === 'identifier' ? handlerNode.text : null; + const imported = localHandler !== null ? importMap.get(localHandler) : undefined; out.push({ role: 'provider', framework: 'express', method: methodNode.text.toUpperCase(), path, - name: handlerNode?.type === 'identifier' ? handlerNode.text : null, + name: imported ? imported.name : localHandler, + handlerImport: imported, line: (handlerNode ?? pathNode).startPosition.row + 1, confidence: 0.8, }); diff --git a/gitnexus/src/core/group/extractors/http-patterns/python.ts b/gitnexus/src/core/group/extractors/http-patterns/python.ts index d4e61b8f9..5a02c7e26 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/python.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/python.ts @@ -79,6 +79,33 @@ const FASTAPI_ROUTER_PATTERNS = compilePatterns({ ], } satisfies LanguagePatterns>); +// ─── Provider: Flask `app.add_url_rule('/path', view_func=handler)` ─── +// The imperative Flask route registration: unlike `@app.route` (whose handler +// is the decorated function, same-file), `view_func` is frequently an IMPORTED +// (and sometimes aliased) view, so the handler resolves through the file's +// imports. `add_url_rule` + a `view_func=` keyword is highly Flask-specific, so +// the false-positive risk is low. Method(s) come from a `methods=[...]` keyword +// (default GET), extracted in code from the captured call. +const FLASK_ADD_URL_RULE_PATTERNS = compilePatterns({ + name: 'python-flask-add-url-rule', + language: Python, + patterns: [ + { + meta: {}, + query: ` + (call + function: (attribute + attribute: (identifier) @fn (#eq? @fn "add_url_rule")) + arguments: (argument_list + . (string) @path + (keyword_argument + name: (identifier) @kw (#eq? @kw "view_func") + value: (identifier) @handler))) @call + `, + }, + ], +} satisfies LanguagePatterns>); + // ─── include_router(, prefix='/x') across the repo ──────── // Two shapes are common: // app.include_router(assistant.router, prefix='/ai') @@ -331,6 +358,73 @@ const WRAPPER_URI_VAR_PATTERNS = compilePatterns({ ], } satisfies LanguagePatterns>); +/** + * Map each `from import [as ]` binding to its declared + * name + raw module specifier (the spec keeps the leading dots for relative + * imports — `.users`, `..pkg.users` — which the extractor resolves to a target + * file). Lets a Flask `view_func` handler resolve through an alias to the real + * symbol in its module rather than the local alias text. `import x` / `import x + * as y` (module imports, not symbol imports) are left out — a route handler is a + * symbol, addressed via `from … import …`. + */ +function buildPythonImportMap(tree: Parser.Tree): Map { + const map = new Map(); + const walk = (node: Parser.SyntaxNode): void => { + if (node.type === 'import_from_statement') { + const moduleNode = node.childForFieldName('module_name'); + const module = moduleNode?.text ?? null; + if (module !== null) { + for (let i = 0; i < node.namedChildCount; i++) { + const c = node.namedChild(i); + if (!c || c.id === moduleNode?.id) continue; + if (c.type === 'dotted_name') { + map.set(c.text, { name: c.text, module }); + } else if (c.type === 'aliased_import') { + const nameNode = c.childForFieldName('name'); + const aliasNode = c.childForFieldName('alias'); + if (nameNode && aliasNode) { + map.set(aliasNode.text, { name: nameNode.text, module }); + } + } + } + } + } + for (let i = 0; i < node.namedChildCount; i++) { + const c = node.namedChild(i); + if (c) walk(c); + } + }; + walk(tree.rootNode); + return map; +} + +/** + * HTTP verbs declared on a Flask `add_url_rule(..., methods=[...])` call, upper- + * cased. Defaults to `['GET']` when no `methods` keyword is present (Flask's own + * default). Reads the captured call node directly since the list value is awkward + * to capture in a tree-sitter query. + */ +function extractFlaskMethods(callNode: Parser.SyntaxNode): string[] { + const args = callNode.childForFieldName('arguments'); + if (args) { + for (let i = 0; i < args.namedChildCount; i++) { + const kw = args.namedChild(i); + if (!kw || kw.type !== 'keyword_argument') continue; + if (kw.childForFieldName('name')?.text !== 'methods') continue; + const list = kw.childForFieldName('value'); + if (!list) continue; + const methods: string[] = []; + for (let j = 0; j < list.namedChildCount; j++) { + const el = list.namedChild(j); + const v = el && el.type === 'string' ? unquoteLiteral(el.text) : null; + if (v) methods.push(v.toUpperCase()); + } + if (methods.length > 0) return methods; + } + } + return ['GET']; +} + // Pre-scan: collect local string assignments (uri = "api/v1/endpoint/") function buildLocalStringMap(tree: Parser.Tree): Map { const map = new Map(); @@ -943,6 +1037,10 @@ export const PYTHON_HTTP_PLUGIN: HttpLanguagePlugin = { const out: HttpDetection[] = []; const httpxAsyncClients = collectHttpxAsyncClients(tree); const ctx = repoContext as PythonRepoContext | undefined; + // Local-binding → { declared name, module } for the file's `from … import …` + // statements, so an imperatively-registered handler (Flask `view_func`) that + // is an imported (possibly aliased) symbol resolves to its real definition. + const importMap = buildPythonImportMap(tree); // Providers: FastAPI @app.("/path") — already absolute path. for (const match of runCompiledPatterns(FASTAPI_APP_PATTERNS, tree)) { @@ -1008,6 +1106,32 @@ export const PYTHON_HTTP_PLUGIN: HttpLanguagePlugin = { } } + // Providers: Flask `app.add_url_rule('/path', view_func=handler, methods=[…])`. + // The handler is a `view_func` identifier, frequently an imported (possibly + // aliased) view, so resolve it through the file's imports to the declared + // symbol + its module for import-pinned resolution downstream. + for (const match of runCompiledPatterns(FLASK_ADD_URL_RULE_PATTERNS, tree)) { + const pathNode = match.captures.path; + const handlerNode = match.captures.handler; + const callNode = match.captures.call; + if (!pathNode || !handlerNode || !callNode) continue; + const path = unquoteLiteral(pathNode.text); + if (path === null) continue; + const imported = importMap.get(handlerNode.text); + for (const method of extractFlaskMethods(callNode)) { + out.push({ + role: 'provider', + framework: 'flask', + method, + path, + name: imported ? imported.name : handlerNode.text, + handlerImport: imported, + line: (imported ? pathNode : handlerNode).startPosition.row + 1, + confidence: 0.8, + }); + } + } + // Consumers: requests. for (const match of runCompiledPatterns(REQUESTS_VERB_PATTERNS, tree)) { const methodNode = match.captures.method; diff --git a/gitnexus/src/core/group/extractors/http-patterns/types.ts b/gitnexus/src/core/group/extractors/http-patterns/types.ts index 28bedb1bb..ffe59cc59 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/types.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/types.ts @@ -45,6 +45,17 @@ export interface HttpDetection { * not set it falls back to file-level boundary resolution downstream. */ line?: number; + /** + * When the handler is an IMPORTED symbol, the import resolved to its declared + * (exported) `name` and the `module` specifier it came from. The extractor + * pins resolution to the import's target file, so an aliased import + * (`import { listUsers as handleUsers }`) or a name that collides with a local + * symbol resolves to the right handler instead of a same-named decoy. `name` + * here is the DECLARED export name (not the local alias); `module` is the raw + * specifier (e.g. `./handlers/users`). Set only for named imports; omitted for + * locally-defined or anonymous handlers. + */ + handlerImport?: { name: string; module: string }; /** Confidence in (0, 1]. Source-scan plugins typically use 0.7–0.8. */ confidence: number; } diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index 62ff1de14..c94accad7 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -80,6 +80,67 @@ WHERE sym.filePath = $filePath AND sym.startLine IS NOT NULL AND sym.endLine IS RETURN sym.id AS uid, sym.name AS name, sym.filePath AS filePath, sym.startLine AS startLine, sym.endLine AS endLine, labels(sym) AS labels`; +// Repo-wide lookup of a symbol by exact name (label-union, as in +// manifest-extractor.ts). Used to resolve a provider's named handler when it is +// defined in a file OTHER than its route registration — and only honored when +// the result is unique (see resolveSymbolByNameUnique). +// +// `n.filePath <> ''` excludes synthetic non-source `CodeElement` nodes that +// carry no real file — ORM model/table nodes (orm.ts emits `filePath: ''`) and +// similar — so a handler name colliding with an ORM model neither resolves to a +// degenerate edge-less node NOR inflates the uniqueness count and masks the real +// handler. `LIMIT 2` bounds materialization: distinguishing unique (1) from +// ambiguous (>=2) never needs more than two rows (the count guard stays exact). +const RESOLVE_BY_NAME_QUERY = ` +MATCH (n:Function|Method|CodeElement) +WHERE n.name = $name AND n.filePath <> '' +RETURN n.id AS uid, n.name AS name, n.filePath AS filePath +LIMIT 2`; + +// Resolve an IMPORTED handler by pinning it to the import's target module: the +// declared export `$name` whose file is the module the handler was imported from +// (`$fileDot` matches `mod.ext`, `$fileSlash` matches `mod/index.ext`). This is +// the precise rung — it survives aliases and local same-name collisions that a +// repo-wide name lookup cannot, and only resolves on a unique match within that +// module. `LIMIT 2` keeps the uniqueness count exact (see RESOLVE_BY_NAME_QUERY). +const RESOLVE_IN_MODULE_QUERY = ` +MATCH (n:Function|Method|CodeElement) +WHERE n.name = $name AND (n.filePath STARTS WITH $fileDot OR n.filePath STARTS WITH $fileSlash) +RETURN n.id AS uid, n.name AS name, n.filePath AS filePath +LIMIT 2`; + +// Source-file extensions an import specifier may resolve to (stripped before +// building the module file-prefix so `./h/users` and `./h/users.ts` agree). +const SOURCE_EXT_RE = /\.(?:m|c)?[jt]sx?$/; + +/** + * Resolve an import specifier to a repo-relative FILE BASE (path without + * extension) so the target module can be matched by `filePath STARTS WITH`. + * Handles two relative-import dialects and returns null for bare/absolute + * imports (which fall back to a repo-wide name lookup): + * - path-style (JS/TS): `./handlers/users`, `../x` → joined against the + * importing file's directory. + * - dotted-relative (Python): `.users`, `..pkg.users` → leading dots are + * package levels (one dot = the file's own package), the rest dot→slash. + */ +function resolveModuleBase(fromFile: string, module: string): string | null { + const dir = path.posix.dirname(fromFile.replace(/\\/g, '/')); + if (module.includes('/')) { + // path-style relative import + if (!module.startsWith('.')) return null; + return path.posix.normalize(path.posix.join(dir, module)).replace(SOURCE_EXT_RE, ''); + } + if (module.startsWith('.')) { + // Python dotted-relative import + const dots = module.length - module.replace(/^\.+/, '').length; + const rest = module.slice(dots).replace(/\./g, '/'); + let base = dir; + for (let i = 1; i < dots; i++) base = path.posix.dirname(base); + return rest ? path.posix.normalize(path.posix.join(base, rest)) : base; + } + return null; // bare / absolute import — repo-wide fallback +} + interface ResolvedSymbol { uid: string; name: string; @@ -357,22 +418,114 @@ export class HttpRouteExtractor implements ContractExtractor { fileSymbolCache.set(filePath, rows); return rows; }; + // Repo-wide UNAMBIGUOUS resolution for a provider handler defined in a file + // other than its route registration (e.g. `router.get('/x', listUsers)` with + // `listUsers` imported from another module). Returns the symbol ONLY when + // exactly one Function/Method/CodeElement carries that name across the repo. + // The strict uniqueness guard is intentionally conservative: when a name is + // shared across files (homonyms like `handler`/`index`), we prefer a + // false-negative (no attribution → file-level fallback) over a false-positive + // (wrong symbol). + // + // An IMPORTED handler (the common cross-file case) is pinned to its source + // module first by resolveImportedSymbol, so an alias or a name colliding with + // a local symbol resolves correctly; this repo-wide-by-name rung is the + // fallback for non-relative/bare imports and for plugins that supply only a + // name. Cached by name for the lifetime of this extract(). + const globalNameCache = new Map(); + const toResolvedSymbol = (rows: Record[]): ResolvedSymbol | null => { + const norm = (x: unknown): string => String(x ?? ''); + const uid = rows.length === 1 ? norm(rows[0]!.uid ?? rows[0]![0]) : ''; + const filePath = uid ? norm(rows[0]!.filePath ?? rows[0]![2]) : ''; + // Reject a unique match that carries no real file (a synthetic ORM / + // non-source node) so it can never anchor a cross-trace on an edge-less + // node — defence in depth alongside the queries' filePath predicates. + return uid && filePath ? { uid, name: norm(rows[0]!.name ?? rows[0]![1]), filePath } : null; + }; + const resolveSymbolByNameUnique = async (name: string): Promise => { + if (!dbExecutor) return null; + const cached = globalNameCache.get(name); + if (cached !== undefined) return cached; + let rows: Record[] = []; + try { + rows = await dbExecutor(RESOLVE_BY_NAME_QUERY, { name }); + } catch { + rows = []; + } + const result = toResolvedSymbol(rows); + globalNameCache.set(name, result); + return result; + }; + // Resolve a handler imported from a RELATIVE module to the unique declared + // symbol of that name inside the import's target file. Returns null for + // non-relative (bare/aliased-path) imports — those fall back to the repo-wide + // name lookup. Cached by (target-file-prefix, declared name). + const importedSymbolCache = new Map(); + const resolveImportedSymbol = async ( + fromFile: string, + imp: { name: string; module: string }, + ): Promise => { + if (!dbExecutor) return null; + const base = resolveModuleBase(fromFile, imp.module); + if (base === null) return null; // bare/absolute import → repo-wide fallback + const cacheKey = JSON.stringify([base, imp.name]); + const cached = importedSymbolCache.get(cacheKey); + if (cached !== undefined) return cached; + let rows: Record[] = []; + try { + rows = await dbExecutor(RESOLVE_IN_MODULE_QUERY, { + name: imp.name, + fileDot: `${base}.`, + fileSlash: `${base}/`, + }); + } catch { + rows = []; + } + const result = toResolvedSymbol(rows); + importedSymbolCache.set(cacheKey, result); + return result; + }; const resolveDetectionSymbol = async ( filePath: string, d: HttpDetection, ): Promise => { if (!dbExecutor) return null; const syms = await loadFileSymbols(filePath); - if (syms.length === 0) return null; // Name resolution does NOT need a detection line — a named provider // handler (Spring/Go/etc. method name) resolves by name even when the - // plugin didn't set `line`. Try it FIRST; only the containment fallback - // requires a line. + // plugin didn't set `line`. Try the registration file FIRST; then, for a + // handler defined in another file, the unique repo-wide match. Only the + // containment fallback requires a line. if (d.role === 'provider' && d.name) { + // IMPORTED handler: pin to the import's target module first. This is the + // precise rung — it survives aliases and names that collide with a local + // symbol. The handler is defined ELSEWHERE, so a file-scoped lookup of + // its (declared) name would be wrong; on a miss go straight to a unique + // repo-wide match on the declared name, never file-scoped. + if (d.handlerImport) { + const byImport = await resolveImportedSymbol(filePath, d.handlerImport); + if (byImport) return byImport; + const byGlobal = await resolveSymbolByNameUnique(d.handlerImport.name); + if (byGlobal) return byGlobal; + return null; + } const byName = resolveSymbolByName(syms, d.name); if (byName) return byName; + const byGlobal = await resolveSymbolByNameUnique(d.name); + if (byGlobal) return byGlobal; + // A NAMED handler we could not resolve by name (neither file-scoped nor + // the unique repo-wide match) must NOT fall through to line-span + // containment: `d.line` is the route REGISTRATION site, so containment + // would attach the route to the enclosing registrar (e.g. a + // `setupRoutes()` wrapper) rather than the handler. Leave it empty → + // file-level boundary fallback, upholding the invariant that a + // zero/ambiguous name match never yields a wrong-symbol attribution. + return null; } - if (d.line == null) return null; + // Consumers (the function making the fetch) and inline-arrow providers + // (d.name === null) DO resolve by containment — there the enclosing symbol + // is the right one. + if (syms.length === 0 || d.line == null) return null; return resolveContainingSymbol(syms, d.line); }; diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index c55198f68..55442f088 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -171,6 +171,414 @@ export default router; symbolName: 'listUsers', }); }); + + // A handler defined in a different file than its route registration: the + // registration file's symbols do not contain it, so resolution falls through + // to the unique repo-wide name lookup (#2275). + const crossFileRoutes = `import { Router } from 'express'; +import { listUsers } from './handlers/users'; +const router = Router(); +router.get('/api/users', listUsers); +export default router; +`; + const routesFileSyms = [ + { + uid: 'const-router', + name: 'router', + filePath: 'src/routes.ts', + startLine: 3, + endLine: 3, + labels: ['Const'], + }, + ]; + const writeCrossFile = (sub: string) => { + const dir = path.join(tmpDir, sub); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'src/routes.ts'), crossFileRoutes); + return dir; + }; + const providerOf = (contracts: Awaited>) => + contracts.find((c) => c.role === 'provider' && c.contractId === 'http::GET::/api/users'); + + it('resolves a cross-file named handler via the unique repo-wide lookup', async () => { + const dir = writeCrossFile('xfile-unique'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') ? routesFileSyms : []; + if (query.includes('n.name = $name')) + return params?.name === 'listUsers' + ? [{ uid: 'fn-listUsers-xfile', name: 'listUsers', filePath: 'src/handlers/users.ts' }] + : []; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider).toMatchObject({ symbolUid: 'fn-listUsers-xfile', symbolName: 'listUsers' }); + }); + + it('leaves symbolUid empty when the repo-wide name is AMBIGUOUS (multiple matches)', async () => { + const dir = writeCrossFile('xfile-ambiguous'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') ? routesFileSyms : []; + if (query.includes('n.name = $name')) + return [ + { uid: 'fn-a', name: 'listUsers', filePath: 'src/a.ts' }, + { uid: 'fn-b', name: 'listUsers', filePath: 'src/b.ts' }, + ]; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe(''); + }); + + it('leaves symbolUid empty when no repo-wide name matches', async () => { + const dir = writeCrossFile('xfile-zero'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') ? routesFileSyms : []; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe(''); + }); + + it('prefers a LOCALLY-DEFINED handler and never consults the repo-wide lookup', async () => { + // Handler defined in the registration file itself (not imported) → no + // handlerImport → file-scoped resolution wins; the global / module lookups + // are never consulted. + const dir = path.join(tmpDir, 'local-handler-wins'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/routes.ts'), + `import { Router } from 'express'; +const router = Router(); +function listUsers(req, res) { + res.json([]); +} +router.get('/api/users', listUsers); +export default router; +`, + ); + let globalQueries = 0; + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') + ? [ + { + uid: 'fn-samefile', + name: 'listUsers', + filePath: 'src/routes.ts', + startLine: 3, + endLine: 5, + labels: ['Function'], + }, + ] + : []; + if (query.includes('n.name = $name')) { + globalQueries += 1; + return [{ uid: 'fn-global', name: 'listUsers', filePath: 'src/elsewhere.ts' }]; + } + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe('fn-samefile'); + expect(globalQueries).toBe(0); + }); + + it('leaves symbolUid empty (no exception) when the repo-wide query throws', async () => { + const dir = writeCrossFile('xfile-throws'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') ? routesFileSyms : []; + if (query.includes('n.name = $name')) throw new Error('DB locked'); + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe(''); + }); + + it('caches the repo-wide lookup by name across detections in one extract()', async () => { + // Two routes in the same file referencing the SAME cross-file handler must + // issue the by-name query at most once (memoized by name). + const dir = path.join(tmpDir, 'xfile-cache'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/routes.ts'), + `import { Router } from 'express'; +import { listUsers } from './handlers/users'; +const router = Router(); +router.get('/api/users', listUsers); +router.post('/api/users', listUsers); +export default router; +`, + ); + let globalQueries = 0; + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') ? routesFileSyms : []; + if (query.includes('n.name = $name')) { + globalQueries += 1; + return [{ uid: 'fn-listUsers-x', name: 'listUsers', filePath: 'src/handlers/users.ts' }]; + } + return []; + }; + await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + expect(globalQueries).toBe(1); + }); + + it('does NOT consult the repo-wide lookup for consumers', async () => { + // A consumer (the function making a fetch) resolves by containment in its + // own file; the provider-only repo-wide lookup must never fire for it. + const dir = path.join(tmpDir, 'xfile-consumer-gate'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/api.ts'), + `export async function fetchUsers() { + const r = await fetch('/api/users'); + return r.json(); +} +`, + ); + let globalQueries = 0; + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('api.ts') + ? [ + { + uid: 'fn-fetchUsers', + name: 'fetchUsers', + filePath: 'src/api.ts', + startLine: 1, + endLine: 4, + labels: ['Function'], + }, + ] + : []; + if (query.includes('n.name = $name')) { + globalQueries += 1; + return [{ uid: 'should-not-be-used', name: 'fetchUsers', filePath: 'src/x.ts' }]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const consumer = contracts.find((c) => c.role === 'consumer'); + expect(consumer?.symbolName).toBe('fetchUsers'); + expect(globalQueries).toBe(0); + }); + + it('does NOT attach a named provider to its registrar when the name is unresolvable', async () => { + // router.get(...) is registered INSIDE setupRoutes(); the handler + // `listUsers` is ambiguous repo-wide (2 matches) so name resolution fails. + // The route must NOT fall through to line-span containment and attach to + // the enclosing `setupRoutes` wrapper — it stays empty (file fallback). + const dir = path.join(tmpDir, 'xfile-wrapper-no-attach'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/routes.ts'), + `import { Router } from 'express'; +import { listUsers } from './handlers/users'; +const router = Router(); +export function setupRoutes() { + router.get('/api/users', listUsers); +} +export default router; +`, + ); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') + ? [ + { + uid: 'fn-setupRoutes', + name: 'setupRoutes', + filePath: 'src/routes.ts', + startLine: 1, + endLine: 99, + labels: ['Function'], + }, + ] + : []; + if (query.includes('n.name = $name')) + return [ + { uid: 'fn-a', name: 'listUsers', filePath: 'src/a.ts' }, + { uid: 'fn-b', name: 'listUsers', filePath: 'src/b.ts' }, + ]; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe(''); + }); + + it('rejects a unique repo-wide match that carries no real file (synthetic node)', async () => { + // A handler name colliding with an ORM model node (orm.ts emits + // filePath: '') yields a single match with no file. It must be rejected, + // not attached as an edge-less cross-trace anchor. + const dir = writeCrossFile('xfile-empty-filepath'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) + return String(params?.filePath ?? '').includes('routes.ts') ? routesFileSyms : []; + if (query.includes('n.name = $name')) + return [{ uid: 'orm-listUsers', name: 'listUsers', filePath: '' }]; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe(''); + }); + + it('resolves via the repo-wide lookup when the registration file has NO indexed symbols', async () => { + // Pins the reordered early-return: CONTAINING_QUERY returns [] for the + // registration file (no in-file symbols at all), yet the unique repo-wide + // match still resolves the cross-file handler. Before the reorder, the + // `syms.length === 0` guard short-circuited above the provider name branch. + const dir = writeCrossFile('xfile-empty-regfile'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL')) return []; + if (query.includes('n.name = $name')) + return params?.name === 'listUsers' + ? [{ uid: 'fn-listUsers-xfile', name: 'listUsers', filePath: 'src/handlers/users.ts' }] + : []; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider).toMatchObject({ symbolUid: 'fn-listUsers-xfile', symbolName: 'listUsers' }); + }); + + it('resolves an ALIASED import to its declared name in the target module (not the alias)', async () => { + // import { listUsers as handleUsers } from './handlers/users'; + // router.get('/api/users', handleUsers); + an UNRELATED function handleUsers + // elsewhere. The route must resolve to the imported `listUsers`, and the + // local alias `handleUsers` must NEVER be looked up. + const dir = path.join(tmpDir, 'xfile-alias'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/routes.ts'), + `import { Router } from 'express'; +import { listUsers as handleUsers } from './handlers/users'; +const router = Router(); +router.get('/api/users', handleUsers); +export default router; +`, + ); + const queriedNames: string[] = []; + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('STARTS WITH')) { + queriedNames.push(`module:${String(params?.name)}`); + return params?.name === 'listUsers' && + String(params?.fileDot ?? '').startsWith('src/handlers/users') + ? [{ uid: 'fn-listUsers', name: 'listUsers', filePath: 'src/handlers/users.ts' }] + : []; + } + if (query.includes('n.name = $name')) { + queriedNames.push(`global:${String(params?.name)}`); + return params?.name === 'handleUsers' + ? [{ uid: 'fn-unrelated', name: 'handleUsers', filePath: 'src/other.ts' }] + : []; + } + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider).toMatchObject({ symbolUid: 'fn-listUsers', symbolName: 'listUsers' }); + expect(queriedNames).not.toContain('module:handleUsers'); + expect(queriedNames).not.toContain('global:handleUsers'); + }); + + it('pins an imported handler to its module, resolving a name that is ambiguous repo-wide', async () => { + // `listUsers` exists in two files; the import pins to ./handlers/users, so + // the module-scoped query returns exactly one even though a repo-wide + // name lookup would be ambiguous (and would decline). + const dir = writeCrossFile('xfile-module-pin'); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('STARTS WITH')) + return String(params?.fileDot ?? '').startsWith('src/handlers/users') + ? [{ uid: 'fn-the-right-one', name: 'listUsers', filePath: 'src/handlers/users.ts' }] + : []; + if (query.includes('n.name = $name')) + return [ + { uid: 'fn-a', name: 'listUsers', filePath: 'src/handlers/users.ts' }, + { uid: 'fn-b', name: 'listUsers', filePath: 'src/admin/users.ts' }, + ]; + return []; + }; + const provider = providerOf(await extractor.extract(mockDbExecutor, dir, makeRepo(dir))); + expect(provider?.symbolUid).toBe('fn-the-right-one'); + }); + + it('resolves a Python Flask add_url_rule ALIASED view through the import (relative module)', async () => { + // from .handlers.users import list_users as handle_users + // app.add_url_rule('/api/users', view_func=handle_users) + // resolves to the declared `list_users` in app/handlers/users.py — the + // relative dotted module `.handlers.users` is pinned, the alias never used. + const dir = path.join(tmpDir, 'py-flask-alias'); + fs.mkdirSync(path.join(dir, 'app', 'handlers'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'app/routes.py'), + `from flask import Flask +from .handlers.users import list_users as handle_users +app = Flask(__name__) +app.add_url_rule('/api/users', view_func=handle_users) +`, + ); + const queriedNames: string[] = []; + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('STARTS WITH')) { + queriedNames.push(`module:${String(params?.name)}`); + return params?.name === 'list_users' && + String(params?.fileDot ?? '').startsWith('app/handlers/users') + ? [{ uid: 'fn-list_users', name: 'list_users', filePath: 'app/handlers/users.py' }] + : []; + } + if (query.includes('n.name = $name')) { + queriedNames.push(`global:${String(params?.name)}`); + return []; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/users', + ); + expect(provider).toMatchObject({ symbolUid: 'fn-list_users', symbolName: 'list_users' }); + expect(queriedNames).not.toContain('module:handle_users'); + }); }); describe('provider extraction — graph-first (Strategy A)', () => { From 698f5efc82e1e7cad2c75aac59bcccc11858bce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 23 Jun 2026 17:51:11 +0100 Subject: [PATCH 2/2] feat(group): resolve inline HTTP provider handlers via call-site line (#2276) (#2282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(group): resolve Go inline provider handlers via line containment (#2276) Widen the Go HandleFunc + framework-route handler capture to match func literals and emit name:null + call-site line for them, so an inline handler resolves to its containing/closure symbol instead of file-level. Named identifier handlers keep resolving by name. * feat(group): resolve Laravel closure provider handlers via line containment (#2276) Capture the Laravel route handler argument; a closure (anonymous function or arrow fn) now emits name:null + the registration line so it resolves to its containing symbol (service-provider boot, controller method) by containment. Named-controller routes keep the 'route' label. File-scope closures stay file-level (PHP closures not yet indexed). * feat(group): wire call-site line on FastAPI provider emits (#2276) Set line on the FastAPI @app/@router provider detections (already name:null) so the source-scan fallback resolves the decorated handler by line-span containment. Best-effort: FastAPI routes are graph-backed and the function span starts at def, so this lands the single-decorator case. Flask add_url_rule already carried line. * feat(group): wire call-site line on Kotlin/Java Spring provider emits (#2276) Add line to the Kotlin and Java Spring @*Mapping provider detections for parity with the consumer emits and a future inline DSL. Inert for current resolution: a named Spring controller method resolves by name and never falls through to line-span containment. * fix(review): apply autofix feedback Pin two documented limitations with tests: a file-scope Laravel closure and a multi-decorator FastAPI handler both degrade to file-level rather than mis-attributing (#2276 ce-code-review autofix). * test(group): lock named gin framework-route resolves by name not registrar (#2276) Reviewer verified named Go handlers still resolve by name across the widened queries; the HandleFunc path was already pinned, this adds the framework-route (gin/echo) path with a DB + enclosing registrar whose span covers the registration line, proving the emitted line never diverts a named provider to its registrar via containment. * test(group): end-to-end inline Go provider resolution against real LadybugDB (#2276) Closes the validation gap that all prior coverage mocked CONTAINING_QUERY: runs the real pipeline over a Go file with an inline http.HandleFunc func-literal handler, persists into a real LadybugDB, and runs the production HttpRouteExtractor against the real executor — proving the emitted call-site line lands inside main()'s real 0-based span and yields source_scan_resolved, not the file-level fallback. * fix(test): use fs.mkdtemp to satisfy CodeQL insecure-temporary-file gate (#2276) The new integration test created its temp base via a predictable os.tmpdir()+name join, which CodeQL flags as js/insecure-temporary-file (1 high). Switch to fs.mkdtemp for an atomic, randomly-named base dir. * fix(group): anchor Go provider @handler to the trailing argument (#2276) The widened framework-route and HandleFunc handler captures (`[(identifier) (func_literal)] @handler`) were unanchored, so a variadic middleware route `r.GET("/x", mw, func(){})` produced two provider detections — one for the middleware identifier and one for the closure. The contractId-only merge then kept the middleware detection and mis-attributed the route to it (and the pre-existing `mw, namedHandler` shape had the same defect), silently neutralizing the inline-handler containment resolution from #2276. Add a trailing tree-sitter anchor (`@handler .`) so the handler binds the LAST argument of the call, leaving middleware args before it unconstrained. Verified against tree-sitter-go: the multi-arg shapes now yield exactly one detection (the real handler) while every 2-arg case is unchanged. Adds two regression tests pinning that a middleware + inline closure resolves to its containing function and a middleware + named handler resolves by name. * test(group): cover FastAPI @router inline-handler containment (#2276) The @router/APIRouter provider emit gained a call-site `line` in #2276 but only the @app path was tested; the existing @router tests call `extract(null, …)` so the resolver/containment path never ran for @router. Add two tests mirroring the @app cases: a single-decorator @router handler resolves to its function via source_scan_resolved (which fails if `line` is dropped), and a multi-decorator one degrades to file-level. * fix(group): treat synthetic 'route' label as anonymous in cross-trace (#2276) After #2276 an unresolved file-scope Laravel closure emits name:null, so its persisted symbolName falls back to 'handler' — which providerLabel already anonymizes to ''. But an unresolved named-controller route still carries the synthetic 'route' placeholder, which the sentinel did NOT cover, so group_trace/group_cross_impact rendered it as the literal 'route' while equivalent closures showed '<... handler>'. 'route' is only ever the synthetic Laravel placeholder (php.ts), never a resolved handler name, so add it to the unresolved-generic sentinel set alongside 'handler'/'fetch'. The resolved branch is untouched, so a real symbol genuinely named 'route' still displays its name. Adds a cross-trace test pinning the anonymized label. * fix(group): gate Spring provider line on a present method name (#2276) The Java/Kotlin Spring @*Mapping provider emits set `line` unconditionally while the method name is typed string|null. The 'a named provider never reaches containment' guarantee held only because the grammar always captures a method name — the type did not enforce it. A (grammar-impossible) null name would emit name:null + line and resolve by containment to the enclosing class body instead of staying file-level. Emit `line` only when the method name is truthy, so a nameless provider degrades to file-level (the safe no-mis-attribution outcome). Behavior is unchanged for every real Spring route (name is always present), but the inertness is now enforced rather than incidental. --- gitnexus/src/core/group/cross-trace.ts | 12 +- .../core/group/extractors/http-patterns/go.ts | 27 +- .../group/extractors/http-patterns/java.ts | 7 + .../group/extractors/http-patterns/kotlin.ts | 7 + .../group/extractors/http-patterns/php.ts | 16 +- .../group/extractors/http-patterns/python.ts | 8 + ...tp-inline-handler-symbol-roundtrip.test.ts | 114 +++ gitnexus/test/unit/group/cross-trace.test.ts | 67 ++ .../unit/group/http-route-extractor.test.ts | 648 ++++++++++++++++++ 9 files changed, 893 insertions(+), 13 deletions(-) create mode 100644 gitnexus/test/integration/http-inline-handler-symbol-roundtrip.test.ts diff --git a/gitnexus/src/core/group/cross-trace.ts b/gitnexus/src/core/group/cross-trace.ts index 93df99a1a..16293b179 100644 --- a/gitnexus/src/core/group/cross-trace.ts +++ b/gitnexus/src/core/group/cross-trace.ts @@ -1036,12 +1036,13 @@ const FILE_BASENAME_RE = /** * A provider endpoint's display label. A resolved handler has a real function - * name; the source-scan fallbacks leave a generic token (`'handler'`/`'fetch'`) - * or a file basename. Those are treated as anonymous and shown as + * name; the source-scan fallbacks leave a generic token (`'handler'`/`'fetch'`, + * or `'route'` for an unresolved named-controller / closure Laravel route) or a + * file basename. Those are treated as anonymous and shown as * `` so the endpoint is still identifiable by route. When * the bridge row carries a resolved `providerUid`, the name IS a real symbol — - * the `'handler'`/`'fetch'` sentinel check is suppressed so a function genuinely - * named `handler` is not mislabeled anonymous. + * the sentinel check is suppressed so a function genuinely named `handler` (or, + * hypothetically, `route`) is not mislabeled anonymous. */ function providerLabel( providerName: string, @@ -1052,7 +1053,8 @@ function providerLabel( const generic = providerName === '' || FILE_BASENAME_RE.test(providerName) || - (!resolved && (providerName === 'handler' || providerName === 'fetch')); + (!resolved && + (providerName === 'handler' || providerName === 'fetch' || providerName === 'route')); return generic ? { label: `<${contractId} handler>`, anon: true } : { label: providerName, anon: false }; diff --git a/gitnexus/src/core/group/extractors/http-patterns/go.ts b/gitnexus/src/core/group/extractors/http-patterns/go.ts index 45507a948..ebf87f0e1 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/go.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/go.ts @@ -17,8 +17,11 @@ import type { HttpDetection, HttpLanguagePlugin } from './types.js'; // ─── Provider: framework routing ────────────────────────────────────── // Matches `\w+\.GET(...)` etc. (gin, echo, chi all share this shape). -// Captures the HTTP method (field name), path literal, and handler -// identifier passed as the second argument. +// Captures the HTTP method (field name), path literal, and the handler — +// anchored to the LAST argument (`@handler .`) so a variadic middleware +// chain (`r.GET("/x", mw, handler)`, gin/echo/chi style) binds the real +// handler, not a middleware identifier (which would otherwise over-match +// and attach the route to the wrong symbol — see #2276 review). const FRAMEWORK_ROUTE_PATTERNS = compilePatterns({ name: 'go-framework-route', language: Go, @@ -31,7 +34,8 @@ const FRAMEWORK_ROUTE_PATTERNS = compilePatterns({ field: (field_identifier) @http_method (#match? @http_method "^(GET|POST|PUT|DELETE|PATCH)$")) arguments: (argument_list (interpreted_string_literal) @path - (identifier) @handler)) + [(identifier) (func_literal)] @handler + .)) `, }, ], @@ -51,7 +55,8 @@ const HANDLE_FUNC_PATTERNS = compilePatterns({ field: (field_identifier) @fn (#eq? @fn "HandleFunc")) arguments: (argument_list (interpreted_string_literal) @path - (identifier) @handler)) + [(identifier) (func_literal)] @handler + .)) `, }, ], @@ -138,12 +143,18 @@ export const GO_HTTP_PLUGIN: HttpLanguagePlugin = { if (!methodNode || !pathNode) continue; const path = unquoteLiteral(pathNode.text); if (path === null) continue; + // An inline `func(){…}` handler has no name → emit `name: null` and a + // `line` so it resolves to its containing/closure symbol by line-span + // containment (like a consumer). A named identifier handler keeps its + // name and resolves by name; `line` is harmless there. + const isInlineHandler = handlerNode?.type === 'func_literal'; out.push({ role: 'provider', framework: 'go-framework', method: methodNode.text.toUpperCase(), path, - name: handlerNode?.text ?? null, + name: isInlineHandler ? null : (handlerNode?.text ?? null), + line: (handlerNode ?? pathNode).startPosition.row + 1, confidence: 0.8, }); } @@ -155,12 +166,16 @@ export const GO_HTTP_PLUGIN: HttpLanguagePlugin = { if (!pathNode) continue; const path = unquoteLiteral(pathNode.text); if (path === null) continue; + // Inline `func(){…}` handler → resolve by containment (see go-framework + // note above); a named handler resolves by name. + const isInlineHandler = handlerNode?.type === 'func_literal'; out.push({ role: 'provider', framework: 'go-stdlib', method: 'GET', path, - name: handlerNode?.text ?? null, + name: isInlineHandler ? null : (handlerNode?.text ?? null), + line: (handlerNode ?? pathNode).startPosition.row + 1, confidence: 0.8, }); } diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 491d14ec4..0e98159dd 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -746,6 +746,13 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { method: route.httpMethod, path: joinPath(prefix, route.rawPath), name: route.methodName, + // Spring providers are named controller methods resolved BY NAME, so + // `line` is inert — a named provider never falls through to line-span + // containment. Gate it on a present name so a (grammar-impossible) + // nameless provider degrades to file-level rather than resolving by + // containment to the enclosing class. Wired for consumer-emit parity + // and a future inline DSL. + line: route.methodName ? route.methodNode.startPosition.row + 1 : undefined, confidence: 0.8, }); } diff --git a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts index 9960dc6ed..edb66faf6 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts @@ -1019,6 +1019,13 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { method: httpMethod, path: joinPath(prefix, rawPath), name: nameNode?.text ?? null, + // Spring providers are named controller methods resolved BY NAME, so + // `line` is inert — a named provider never falls through to line-span + // containment. Gate it on a present name so a (grammar-impossible) + // nameless provider degrades to file-level rather than resolving by + // containment to the enclosing class. Wired for consumer-emit parity + // and a future inline DSL. + line: nameNode?.text ? methodNode.startPosition.row + 1 : undefined, confidence: 0.8, }); } diff --git a/gitnexus/src/core/group/extractors/http-patterns/php.ts b/gitnexus/src/core/group/extractors/http-patterns/php.ts index a033d9941..bf2eb7aba 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/php.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/php.ts @@ -37,7 +37,9 @@ const LARAVEL_ROUTE_SPEC: PatternSpec> = { (scoped_call_expression scope: (name) @scope (#eq? @scope "Route") name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") - arguments: (arguments . (argument (string) @path))) + arguments: (arguments + . (argument (string) @path) + (argument [(anonymous_function) (arrow_function)] @closure)?)) `, }; @@ -150,12 +152,22 @@ export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { if (!methodNode || !pathNode) continue; const path = phpStringText(pathNode); if (path === null) continue; + // A closure handler (`Route::get('/x', function(){…})` / `fn() => …`) has + // no name → emit `name: null` + the registration line so it resolves to + // its containing symbol (e.g. a service-provider `boot()` or controller + // method) by line-span containment. A named-controller route keeps the + // `'route'` label — resolving its array/string handler to a real method is + // a separate, graph-backed concern. NOTE: a closure at FILE scope + // (routes/web.php) has no enclosing function and PHP closures are not yet + // indexed as symbols, so it still degrades to file-level (see #2276). + const closureNode = match.captures.closure; out.push({ role: 'provider', framework: 'laravel', method: methodNode.text.toUpperCase(), path, - name: 'route', + name: closureNode ? null : 'route', + line: (closureNode ?? pathNode).startPosition.row + 1, confidence: 0.8, }); } diff --git a/gitnexus/src/core/group/extractors/http-patterns/python.ts b/gitnexus/src/core/group/extractors/http-patterns/python.ts index 5a02c7e26..e432a958a 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/python.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/python.ts @@ -1057,6 +1057,12 @@ export const PYTHON_HTTP_PLUGIN: HttpLanguagePlugin = { method: httpMethod, path, name: null, + // The decorated handler has no captured name → resolve by line-span + // containment. Best-effort fallback: FastAPI routes are graph-backed + // (ingestion decorator routes) and the function span starts at `def` + // (decorators excluded), so this lands the single-decorator case and + // degrades to file-level for multi-decorator stacks. + line: pathNode.startPosition.row + 1, confidence: 0.8, }); } @@ -1101,6 +1107,8 @@ export const PYTHON_HTTP_PLUGIN: HttpLanguagePlugin = { method: httpMethod, path: p, name: null, + // Best-effort containment fallback — see the @app provider note above. + line: pathNode.startPosition.row + 1, confidence: 0.8, }); } diff --git a/gitnexus/test/integration/http-inline-handler-symbol-roundtrip.test.ts b/gitnexus/test/integration/http-inline-handler-symbol-roundtrip.test.ts new file mode 100644 index 000000000..5332bd7a4 --- /dev/null +++ b/gitnexus/test/integration/http-inline-handler-symbol-roundtrip.test.ts @@ -0,0 +1,114 @@ +/** + * End-to-end validation of the INLINE provider source-scan containment path + * (#2276). + * + * The unit suite (`test/unit/group/http-route-extractor.test.ts`) proves the + * resolver logic by MOCKING `CONTAINING_QUERY` with hand-picked spans. That + * leaves one assumption unverified: that the REAL ingestion pipeline records a + * Go enclosing function with a 0-based span that actually contains the emitted + * call-site line. This test closes that gap. + * + * It runs the real pipeline over a Go file whose `http.HandleFunc` handler is an + * inline `func(){…}` (the issue's headline Go example), persists the resulting + * graph into a real LadybugDB, and runs the production `HttpRouteExtractor` + * against the real executor. The provider must resolve to the containing + * `main()` symbol via line-span containment (`source_scan_resolved`) — not the + * file-level fallback. Go does not index anonymous func literals as symbols + * (only `function_declaration`/`method_declaration`), so the innermost + * containing symbol is `main` itself. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import fs from 'fs/promises'; +import path from 'path'; +import os from 'os'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; +import { HttpRouteExtractor } from '../../src/core/group/extractors/http-route-extractor.js'; +import type { CypherExecutor } from '../../src/core/group/contract-extractor.js'; +import type { RepoHandle } from '../../src/core/group/types.js'; + +let tmpBase: string; +let repoDir: string; +let storagePath: string; +let dbPath: string; + +beforeAll(async () => { + // Atomic, unique temp dir (fs.mkdtemp) — avoids the predictable + // os.tmpdir()+name pattern CodeQL flags as an insecure temporary file. + tmpBase = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-http-inline-e2e-')); + repoDir = path.join(tmpBase, 'repo'); + storagePath = path.join(tmpBase, '.gitnexus'); + dbPath = path.join(storagePath, 'lbug'); + await fs.mkdir(path.join(repoDir, 'cmd'), { recursive: true }); + await fs.mkdir(dbPath, { recursive: true }); + + // net/http inline handler INSIDE main() — the #2276 Go example. Before this + // change the func literal was not even captured; now it emits name:null + the + // call-site line so it resolves to main() by containment. + await fs.writeFile( + path.join(repoDir, 'cmd', 'server.go'), + `package main + +import "net/http" + +func main() { +\thttp.HandleFunc("/api/health", func(w http.ResponseWriter, r *http.Request) { +\t\tw.Write([]byte("ok")) +\t}) +\thttp.ListenAndServe(":8080", nil) +} +`, + ); + + const result = await runPipelineFromRepo(repoDir, () => {}, {}); + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + await adapter.loadGraphToLbug(result.graph, tmpBase, storagePath); +}, 120_000); + +afterAll(async () => { + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.closeLbug(); + } catch { + /* may not have opened */ + } + if (tmpBase) { + for (let attempt = 0; attempt < 5; attempt++) { + try { + await fs.rm(tmpBase, { recursive: true, force: true }); + return; + } catch { + if (attempt < 4) await new Promise((r) => setTimeout(r, 200 * (attempt + 1))); + } + } + } +}); + +describe('inline Go provider handler resolves via real source-scan containment (#2276)', () => { + it('resolves an inline http.HandleFunc closure to the containing main() with a real symbolUid', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + // Param-aware executor (CONTAINING_QUERY binds $filePath) — the same shape + // production passes to ContractExtractors. + const dbExecutor: CypherExecutor = (query, params = {}) => + adapter.executePrepared(query, params); + const repo: RepoHandle = { + id: 'test-repo', + path: 'repo', + repoPath: repoDir, + storagePath, + }; + + const contracts = await new HttpRouteExtractor().extract(dbExecutor, repoDir, repo); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/health', + ); + + expect(provider).toBeDefined(); + // The real pipeline indexed main() with its true 0-based span; the emitted + // call-site line lands inside it, so containment yields a real symbolUid + // rather than the empty file-level fallback. + expect(provider?.symbolUid).toBeTruthy(); + expect(provider?.symbolName).toBe('main'); + expect(provider?.meta.extractionStrategy).toBe('source_scan_resolved'); + }); +}); diff --git a/gitnexus/test/unit/group/cross-trace.test.ts b/gitnexus/test/unit/group/cross-trace.test.ts index 28a60d210..530dc2264 100644 --- a/gitnexus/test/unit/group/cross-trace.test.ts +++ b/gitnexus/test/unit/group/cross-trace.test.ts @@ -478,6 +478,73 @@ describe('runGroupTrace', () => { }, ); + itLbugReopen( + 'destination trace anonymizes an unresolved Laravel `route` placeholder (#2276)', + async () => { + // A named-controller / closure Laravel provider that did not resolve keeps + // the synthetic `'route'` placeholder (never a real symbol name). It must + // be treated as anonymous — shown as `` — exactly like the + // `'handler'`/`'fetch'` sentinels, not displayed as the literal `route`. + const consumer = makeContract({ + repo: 'app/frontend', + role: 'consumer', + symbolUid: 'callUsers-uid', + symbolRef: { filePath: 'src/api.ts', name: 'callUsers' }, + symbolName: 'callUsers', + contractId: 'http::GET::/api/users', + }); + const provider = makeContract({ + repo: 'app/backend', + role: 'provider', + symbolUid: '', // unresolved file-scope closure / named-controller route + symbolRef: { filePath: 'routes/web.php', name: 'route' }, + symbolName: 'route', + contractId: 'http::GET::/api/users', + }); + const link: CrossLink = { + from: { repo: 'app/frontend', symbolUid: 'callUsers-uid', symbolRef: consumer.symbolRef }, + to: { repo: 'app/backend', symbolUid: '', symbolRef: provider.symbolRef }, + type: 'http', + contractId: 'http::GET::/api/users', + matchType: 'exact', + confidence: 1, + }; + await writeBridge(groupDir, { + contracts: [consumer, provider], + crossLinks: [link], + repoSnapshots: {}, + missingRepos: [], + }); + + const port = makePort( + { 'reg-fe:callUsers': okSym('callUsers-uid', 'callUsers', 'src/api.ts', 3) }, + { + 'reg-fe:callUsers-uid->callUsers-uid': okTrace( + [{ name: 'callUsers', filePath: 'src/api.ts', startLine: 3 }], + [], + ), + }, + ); + const r = await runGroupTrace( + { port, gitnexusDir: tmpDir }, + { name: 'g1', from: 'callUsers' }, + ); + expect(r).toMatchObject({ + status: 'ok', + to: { + name: '', + repo: 'app/backend', + filePath: 'routes/web.php', + }, + hops: [ + { name: 'callUsers', repo: 'app/frontend' }, + { name: '', repo: 'app/backend' }, + ], + notes: expect.arrayContaining([expect.stringContaining('anonymous')]), + }); + }, + ); + itLbugReopen('destination trace not_found when no HTTP link leaves the repo', async () => { await writeUnlinkedBridge(groupDir); const port = makePort( diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index 55442f088..99c5b1b0a 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -579,6 +579,654 @@ app.add_url_rule('/api/users', view_func=handle_users) expect(provider).toMatchObject({ symbolUid: 'fn-list_users', symbolName: 'list_users' }); expect(queriedNames).not.toContain('module:handle_users'); }); + + // ── Inline / closure provider handlers (#2276) ────────────────────── + // An inline provider handler has no name, so it must resolve by line-span + // containment to the symbol it lives in — exactly like a consumer. Mirrors + // the Node/Express inline-arrow behavior for the non-Node plugins. + + it('resolves a Go inline http.HandleFunc closure to its containing function (#2276)', async () => { + const dir = path.join(tmpDir, 'go-inline-handlefunc'); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + `package main + +func main() { + http.HandleFunc("/api/inline", func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) +} +`, + ); + // main() spans source lines 3-7 → 0-based [2,6]; the HandleFunc + // registration and its anonymous func are on line 4 (row 3), inside span. + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('server.go')) { + return [ + { + uid: 'fn-main', + name: 'main', + filePath: 'cmd/server.go', + startLine: 2, + endLine: 6, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/inline', + ); + expect(provider).toMatchObject({ + symbolUid: 'fn-main', + symbolName: 'main', + meta: { extractionStrategy: 'source_scan_resolved' }, + }); + }); + + it('resolves a Go inline gin framework-route closure to its containing function (#2276)', async () => { + const dir = path.join(tmpDir, 'go-inline-gin'); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + `package main + +func registerRoutes(r *gin.Engine) { + r.GET("/api/ping", func(c *gin.Context) { + c.String(200, "pong") + }) +} +`, + ); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('server.go')) { + return [ + { + uid: 'fn-registerRoutes', + name: 'registerRoutes', + filePath: 'cmd/server.go', + startLine: 2, + endLine: 6, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/ping', + ); + expect(provider).toMatchObject({ + symbolUid: 'fn-registerRoutes', + symbolName: 'registerRoutes', + }); + }); + + it('keeps Go NAMED HandleFunc handler resolving by name, not containment (#2276)', async () => { + const dir = path.join(tmpDir, 'go-named-handlefunc'); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + `package main + +func healthHandler(w http.ResponseWriter, r *http.Request) {} + +func main() { + http.HandleFunc("/api/health", healthHandler) +} +`, + ); + // Both the named handler and the registrar main() are indexed. A named + // provider must resolve to the HANDLER by name, never to main by line. + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('server.go')) { + return [ + { + uid: 'fn-healthHandler', + name: 'healthHandler', + filePath: 'cmd/server.go', + startLine: 2, + endLine: 2, + labels: ['Function'], + }, + { + uid: 'fn-main', + name: 'main', + filePath: 'cmd/server.go', + startLine: 4, + endLine: 6, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/health', + ); + expect(provider).toMatchObject({ + symbolUid: 'fn-healthHandler', + symbolName: 'healthHandler', + }); + }); + + it('keeps a NAMED gin framework-route handler resolving by name, not its registrar (#2276)', async () => { + // The framework-route query was also widened to accept func literals; this + // pins that a NAMED identifier handler still resolves by name even though + // a `line` is now emitted and the enclosing registrar span covers it. + const dir = path.join(tmpDir, 'go-named-gin'); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + `package main + +func listOrders(c *gin.Context) {} + +func registerRoutes(r *gin.Engine) { + r.GET("/api/orders", listOrders) +} +`, + ); + // listOrders spans [2,2]; registerRoutes spans [4,6] and its span CONTAINS + // the r.GET line (row 5). A named provider must resolve to listOrders by + // name — never to registerRoutes by line-span containment. + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('server.go')) { + return [ + { + uid: 'fn-listOrders', + name: 'listOrders', + filePath: 'cmd/server.go', + startLine: 2, + endLine: 2, + labels: ['Function'], + }, + { + uid: 'fn-registerRoutes', + name: 'registerRoutes', + filePath: 'cmd/server.go', + startLine: 4, + endLine: 6, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/orders', + ); + expect(provider).toMatchObject({ + symbolUid: 'fn-listOrders', + symbolName: 'listOrders', + }); + }); + + it('binds the LAST arg as handler for a middleware + inline route, not the middleware (#2276)', async () => { + // `r.GET(path, mw, func(){})` — gin/echo variadic middleware before an + // inline handler. The trailing-anchor on @handler must select the closure + // (→ containment to the enclosing fn), NOT the middleware identifier. With + // the prior unanchored capture this emitted a second detection for `mw` + // that won the contractId merge and mis-attributed the route to it. + const dir = path.join(tmpDir, 'go-mw-inline-gin'); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + `package main + +func authMiddleware(c *gin.Context) {} + +func registerRoutes(r *gin.Engine) { + r.GET("/api/guarded", authMiddleware, func(c *gin.Context) { + c.String(200, "ok") + }) +} +`, + ); + // Both the middleware and the enclosing registrar are indexed. The closure + // sits at line 6, contained by registerRoutes [5,9]; authMiddleware [3,3] + // does not contain it. The route must resolve to registerRoutes. + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('server.go')) { + return [ + { + uid: 'fn-authMiddleware', + name: 'authMiddleware', + filePath: 'cmd/server.go', + startLine: 3, + endLine: 3, + labels: ['Function'], + }, + { + uid: 'fn-registerRoutes', + name: 'registerRoutes', + filePath: 'cmd/server.go', + startLine: 5, + endLine: 9, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const providers = contracts.filter( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/guarded', + ); + // Exactly one provider (no middleware over-match), resolved by containment. + expect(providers).toHaveLength(1); + expect(providers[0]).toMatchObject({ + symbolUid: 'fn-registerRoutes', + symbolName: 'registerRoutes', + }); + }); + + it('binds the LAST arg as handler for a middleware + NAMED route, not the middleware (#2276)', async () => { + // `r.GET(path, mw, namedHandler)` — the named handler is the last arg and + // must resolve by name; the leading middleware identifier must not win. + const dir = path.join(tmpDir, 'go-mw-named-gin'); + fs.mkdirSync(path.join(dir, 'cmd'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'cmd/server.go'), + `package main + +func authMiddleware(c *gin.Context) {} + +func listOrders(c *gin.Context) {} + +func registerRoutes(r *gin.Engine) { + r.GET("/api/orders", authMiddleware, listOrders) +} +`, + ); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('server.go')) { + return [ + { + uid: 'fn-authMiddleware', + name: 'authMiddleware', + filePath: 'cmd/server.go', + startLine: 3, + endLine: 3, + labels: ['Function'], + }, + { + uid: 'fn-listOrders', + name: 'listOrders', + filePath: 'cmd/server.go', + startLine: 5, + endLine: 5, + labels: ['Function'], + }, + { + uid: 'fn-registerRoutes', + name: 'registerRoutes', + filePath: 'cmd/server.go', + startLine: 7, + endLine: 9, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const providers = contracts.filter( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/orders', + ); + expect(providers).toHaveLength(1); + expect(providers[0]).toMatchObject({ + symbolUid: 'fn-listOrders', + symbolName: 'listOrders', + }); + }); + + it('resolves a Laravel closure route nested in a method to that method (#2276)', async () => { + const dir = path.join(tmpDir, 'php-closure-in-method'); + fs.mkdirSync(path.join(dir, 'app'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'app/RouteServiceProvider.php'), + `, + ): Promise[]> => { + if ( + query.includes('UNION ALL') && + String(params?.filePath ?? '').includes('RouteServiceProvider.php') + ) { + return [ + { + uid: 'method-boot', + name: 'boot', + filePath: 'app/RouteServiceProvider.php', + startLine: 2, + endLine: 6, + labels: ['Method'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/closure', + ); + expect(provider).toMatchObject({ + symbolUid: 'method-boot', + symbolName: 'boot', + meta: { extractionStrategy: 'source_scan_resolved' }, + }); + }); + + it('resolves a Laravel arrow-fn closure route by containment (#2276)', async () => { + const dir = path.join(tmpDir, 'php-arrow-in-method'); + fs.mkdirSync(path.join(dir, 'app'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'app/RouteServiceProvider.php'), + ` response()); + } +} +`, + ); + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if ( + query.includes('UNION ALL') && + String(params?.filePath ?? '').includes('RouteServiceProvider.php') + ) { + return [ + { + uid: 'method-boot', + name: 'boot', + filePath: 'app/RouteServiceProvider.php', + startLine: 2, + endLine: 4, + labels: ['Method'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::POST::/api/arrow', + ); + expect(provider).toMatchObject({ symbolUid: 'method-boot', symbolName: 'boot' }); + }); + + it('leaves a Laravel NAMED-controller route at the prior behavior (no closure path) (#2276)', async () => { + const dir = path.join(tmpDir, 'php-named-controller'); + fs.mkdirSync(path.join(dir, 'routes'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'routes/web.php'), + `[]> => []; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::PUT::/api/named', + ); + expect(provider).toBeDefined(); + expect(provider?.symbolUid).toBe(''); + }); + + it('resolves a FastAPI @app provider to its decorated function (source-scan fallback) (#2276)', async () => { + const dir = path.join(tmpDir, 'py-fastapi-app'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'main.py'), + `from fastapi import FastAPI +app = FastAPI() + +@app.get("/api/items") +def list_items(): + return [] +`, + ); + // The decorator is on line 4 (row 3); `def list_items` is on line 5 + // (row 4). tree-sitter records the function_definition span from `def`, + // so list_items spans 0-based [4,5]. The detection line is the decorator + // row + 1 = 5, and the resolver's direct `line` probe (row 4) lands in + // the def span. (Graph routes are authoritative; this is the fallback.) + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('main.py')) { + return [ + { + uid: 'fn-list_items', + name: 'list_items', + filePath: 'main.py', + startLine: 4, + endLine: 5, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/items', + ); + expect(provider).toMatchObject({ + symbolUid: 'fn-list_items', + symbolName: 'list_items', + meta: { extractionStrategy: 'source_scan_resolved' }, + }); + }); + + // Documented limitations pinned by tests (#2276): a closure with no + // enclosing function symbol, and a multi-decorator FastAPI handler whose + // detection line falls above the def-span, both degrade to file-level + // rather than mis-attributing. These lock the comments in php.ts/python.ts. + + it('leaves a FILE-scope Laravel closure at file-level (no enclosing symbol) (#2276)', async () => { + const dir = path.join(tmpDir, 'php-closure-file-scope'); + fs.mkdirSync(path.join(dir, 'routes'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'routes/web.php'), + `[]> => []; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/home', + ); + expect(provider).toBeDefined(); + expect(provider?.symbolUid).toBe(''); + }); + + it('leaves a multi-decorator FastAPI handler at file-level (line above def-span) (#2276)', async () => { + const dir = path.join(tmpDir, 'py-fastapi-multidecorator'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'main.py'), + `from fastapi import FastAPI +app = FastAPI() + +@app.get("/api/items") +@require_auth +def list_items(): + return [] +`, + ); + // The path literal is on line 4 (row 3) → detection line = row 3 + 1 = 4. + // With a second decorator the `def` is on line 6 (row 5), so list_items + // spans 0-based [5,6]. The resolver probes row 3 (line-1) then row 4 + // (line); both fall ABOVE the [5,6] span → no containment → file-level. + // (Single-decorator resolves because there the def sits at the line probe.) + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('main.py')) { + return [ + { + uid: 'fn-list_items', + name: 'list_items', + filePath: 'main.py', + startLine: 5, + endLine: 6, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/items', + ); + expect(provider).toBeDefined(); + expect(provider?.symbolUid).toBe(''); + }); + + // The @router/APIRouter provider emit also carries `line` (#2276), but only + // @app was covered above. These pin the @router containment path: a + // single-decorator router handler resolves to its function, and a + // multi-decorator one degrades to file-level — same as @app. + + it('resolves a FastAPI @router provider to its decorated function (source-scan fallback) (#2276)', async () => { + const dir = path.join(tmpDir, 'py-fastapi-router'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'main.py'), + `from fastapi import APIRouter +router = APIRouter() + +@router.get("/api/items") +def list_items(): + return [] +`, + ); + // Same shape as the @app case: @router.get is on line 4 (row 3), `def` + // on line 5 → list_items spans 0-based [4,5]; the `line` probe lands in + // the def span and resolves via source_scan_resolved. No include_router + // prefix in scope, so the unprefixed path is emitted. + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('main.py')) { + return [ + { + uid: 'fn-list_items', + name: 'list_items', + filePath: 'main.py', + startLine: 4, + endLine: 5, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::GET::/api/items', + ); + expect(provider).toMatchObject({ + symbolUid: 'fn-list_items', + symbolName: 'list_items', + meta: { extractionStrategy: 'source_scan_resolved' }, + }); + }); + + it('leaves a module-scope multi-decorator @router handler at file-level (#2276)', async () => { + const dir = path.join(tmpDir, 'py-fastapi-router-multidecorator'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'main.py'), + `from fastapi import APIRouter +router = APIRouter() + +@router.post("/api/items") +@require_auth +def create_item(): + return {} +`, + ); + // With a second decorator the `def` is on line 6 → create_item spans + // 0-based [5,6]; the path-line probe falls ABOVE that span → no + // containment → file-level (symbolUid stays empty), matching @app. + const mockDbExecutor = async ( + query: string, + params?: Record, + ): Promise[]> => { + if (query.includes('UNION ALL') && String(params?.filePath ?? '').includes('main.py')) { + return [ + { + uid: 'fn-create_item', + name: 'create_item', + filePath: 'main.py', + startLine: 5, + endLine: 6, + labels: ['Function'], + }, + ]; + } + return []; + }; + const contracts = await extractor.extract(mockDbExecutor, dir, makeRepo(dir)); + const provider = contracts.find( + (c) => c.role === 'provider' && c.contractId === 'http::POST::/api/items', + ); + expect(provider).toBeDefined(); + expect(provider?.symbolUid).toBe(''); + }); }); describe('provider extraction — graph-first (Strategy A)', () => {