20 commits

Author	SHA1	Message	Date
henry201605	a93ecee068	fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) (#1917) ... * fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) PR #1904 gated @RequestLine consumer extraction on the enclosing interface also carrying @FeignClient. That guard is wrong: @RequestLine is a core feign.* annotation used with Feign.builder(), while @FeignClient is the Spring Cloud variant that uses Spring MVC annotations (@GetMapping etc.) — the two are effectively mutually exclusive. Requiring @FeignClient therefore excluded the annotation's primary, canonical usage, so the feature recognized nothing on real core-Feign client interfaces. Fix: drop the @FeignClient requirement for @RequestLine. The match still requires an enclosing interface (Feign proxies are always interfaces), and the `RequestLine` annotation name is itself a strong, framework-specific signal, so false-positive risk stays low. A @FeignClient(path=...) prefix is still applied when present. The @(Get|Post|...)Mapping consumer path keeps its @FeignClient requirement: those annotations are generic Spring MVC and need the Feign context to be disambiguated from provider routes. Verification (real-world, not just synthetic fixtures): - A real client-jar consumer (BigModeClientService.java: a plain interface with 12 @RequestLine methods, no @FeignClient) now yields 12 openfeign consumer contracts; it yielded 0 before this change. - End-to-end `group sync` over that consumer repo + its FastAPI provider repo (with zero hand-written links) produces 12 exact cross-links (confidence 1.0), Java @RequestLine consumer → Python route provider. - The prior test that asserted the wrong behavior ("ignores @RequestLine on interfaces without @FeignClient") is reversed into a realistic core-Feign fixture. - Full test/unit/group suite (579) green; tsc and prettier clean. * test(group): add negative cases for relaxed @RequestLine matcher Per review on #1917 — guard the no-@FeignClient relaxation with explicit negative tests: malformed @RequestLine values (no verb / no leading-slash path / unknown verb) yield no contract, and @RequestLine on a concrete class method (not an interface) is not emitted as a consumer. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-30 12:25:13 +01:00
henry201605	5d710413d7	feat(group): extract OpenFeign @RequestLine consumer contracts (#1904) ... * feat(group): extract OpenFeign @RequestLine consumer contracts Adds Java HTTP plugin support for the native OpenFeign annotation `@RequestLine("METHOD /path")`. Previously only `@FeignClient` interfaces using Spring MVC method annotations (`@GetMapping` etc.) were detected; the native annotation form — required by Feign Builder users and non-Spring Feign deployments — was silently ignored. Implementation: - New `FEIGN_REQUEST_LINE_PATTERNS` covers both positional and named-arg (`value =`) forms. - New `parseRequestLine()` parses the verb+path string and drops any query string (consistent with how RestTemplate/WebClient consumers handle inline literal URLs). - The enclosing interface MUST carry `@FeignClient`; otherwise the detection is dropped to avoid false positives from same-named annotations in unrelated libraries. - Reuses the existing `feignPrefixByInterfaceId` map so `@FeignClient(path=)` and `@RequestMapping` interface prefixes apply uniformly across both Spring MVC and `@RequestLine` methods. - Confidence 0.75 — slightly higher than the 0.7 used for Spring MVC annotations because the verb is a string-literal value, not inferred from the annotation name (less ambiguous). Six new unit tests cover: basic two-method extraction; `@FeignClient(path=)` prefix joining; query-string stripping; rejection of `@RequestLine` on non-Feign interfaces; mixing with `@GetMapping` on the same interface; named-argument form (`value = "..."`). Verification: `npx tsc --noEmit`, full `test/unit/group` (31 files / 563 tests), `http-route-extractor.test.ts` (83/83 incl. 6 new), `prettier --check` and `eslint` on touched files all pass. * refactor(group): collapse @RequestLine positional + named-arg into one query Per @magyargergo's review on PR #1904 — uses tree-sitter alternation `[(...) (...)]` so the positional and named-argument forms of the `@RequestLine` annotation are matched by a single compiled query and invoked through one `runCompiledPatterns` pass instead of two. * refactor(group): drop framework prefixes from java http pattern constant names Per review feedback on #1904 — renames the four route-mapper pattern constants to framework-agnostic names (the per-constant comments already document which framework each targets): SPRING_TYPE_PREFIX_PATTERNS -> TYPE_PREFIX_PATTERNS FEIGN_REQUEST_LINE_PATTERNS -> REQUEST_LINE_PATTERNS FEIGN_INTERFACE_PREFIX_PATTERNS -> INTERFACE_PREFIX_PATTERNS SPRING_METHOD_ROUTE_PATTERNS -> METHOD_ROUTE_PATTERNS * refactor(group): collapse Java route-mapper annotations into one query Merge the four annotation pattern bundles (Spring @RequestMapping type prefix, @FeignClient(path) prefix, @(Get|Post|Put|Delete|Patch)Mapping method routes and native @RequestLine) into a single JAVA_ROUTE_ANNOTATION_PATTERNS query, read by scanRouteAnnotations() in exactly one matches() pass per file. Variants are tagged by branch-local captures and discriminated in JS (METHOD_ANNOTATION_TO_HTTP, isRouteMemberKey), per review feedback. This drops the per-file annotation passes from 4->1 in scan() and 2->1 in collectSpringTypes(), and removes the interface-@RequestMapping / @FeignClient prefix redundancy. Verb and path/value key filtering stay in JS rather than in-query: under the pinned tree-sitter 0.21.1 binding a top-level [...] alternation compiles to one pattern whose text predicates share a single bucket keyed by capture name. A #match? against a capture absent from the matched branch evaluates FALSE and silently drops every sibling-branch match, whereas #eq? against an absent capture is vacuously true. So only fixed annotation names use in-query #eq? (on branch-local captures); the variable verb name and member key carry no in-query predicate. Behaviour is unchanged for all compilable Java; existing http-route tests (93) and the full group suite remain green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(group): make Java route-annotation query generic, match name in loop Collapse JAVA_ROUTE_ANNOTATION_PATTERNS from 9 annotation-name-pinned branches to 6 generic structural branches (class/interface/method x positional/named) that capture the annotation name (@ann), declaration (@node), argument (@value) and member key (@key) generically. The query now carries NO #eq?/#match? predicates at all; scanRouteAnnotations reads @ann.text and @node.type in its for-loop to decide what each match means (RequestMapping prefix, FeignClient(path) prefix, @(Get|...)Mapping route, or @RequestLine), ignoring unrecognised annotations. This makes the query framework-agnostic and extensible — adding a new route annotation is a change to the loop and the lookup maps, not the query — and removes the last tree-sitter-0.21.1 shared-predicate-bucket footgun, since a predicate-free alternation cannot drop sibling branches. Behaviour is byte-identical: 93 targeted http-route tests and the full 569-test group suite stay green; tsc and prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): pin newly-reachable Java route-annotation JS branches; clarify invariants Code-review follow-up to the route-annotation query consolidation. No behaviour change to the extractor: - Add two regression tests for branches the generic predicate-free query made reachable in scanRouteAnnotations: (1) a @RequestLine whose named argument is not `value` must be dropped (the in-query `#eq? @key "value"` guard now lives in JS); (2) @FeignClient(path) must win over @RequestMapping even when @RequestMapping is the first annotation in source order, covering the deferred interfaceRequestMappingPrefixes apply (the existing precedence test only covered @FeignClient-first). - Document two invariants flagged in review: why prefixByTypeId and feignPrefixByInterfaceId intentionally diverge for the same interface node (Spring provider vs OpenFeign consumer prefix), and that the query's single-string-argument shape excludes array-valued annotations. http-route-extractor + multi-verb suites: 95/95 (was 93); tsc + prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-05-30 08:50:29 +01:00
henry201605	2f15c1ece1	feat(group): add Kotlin Spring WebClient long-form HTTP consumer extraction (#1884) ... * feat(group): add Kotlin Spring WebClient long-form HTTP consumer extraction Follow-up to #1855. Extends `kotlin.ts` with the long-form WebClient fluent chain that #1855 explicitly deferred: webClient.method(HttpMethod.GET).uri("/x").retrieve().awaitBody<T>() This pattern remains common in Kotlin Spring 4 → 5 migrations and in codebases that prefer the fluent verb-as-enum style. The short form (`webClient.get().uri("/x")`) was already supported in #1855. Approach: - Single deeper tree-sitter query (`WEB_CLIENT_LONG_PATTERNS`) that matches the full chain structurally — both `.method(HttpMethod.X)` and `.uri("...")` in one pattern. Verb is captured as the `simple_identifier` of the `HttpMethod.X` field access. - Verb is whitelisted to GET/POST/PUT/DELETE/PATCH (consistent with the short-form's `WEB_CLIENT_SHORT_TO_HTTP` map). - Receiver constraint `(#eq? @obj "webClient")` mirrors the short form and Java plugin heuristic. Out of scope (intentional): - Variable-bound verbs: `val verb = HttpMethod.PATCH; webClient.method(verb)...` Source-scan can't follow the binding without graph context. Pinned by an anti-overreach test. - HEAD/OPTIONS/TRACE: not in `WEB_CLIENT_SHORT_TO_HTTP` either — keeps polyglot symmetry with java.ts and the short form. Tests: 4 new cases under `consumer extraction — fetch patterns`, gated by tree-sitter-kotlin grammar availability. positive (3) - long form GET - long form POST / PUT / DELETE / PATCH (4 verbs in 1 fixture) - no double-emit pin (long-form chain produces exactly one consumer, not one from each query) anti-regression (1) - variable-bound verb does NOT match (graph-aware concern) The previous `'does NOT match Kotlin WebClient long form (deferred to follow-up)'` test from #1855 is replaced by these — the deferred state is now resolved. Reverse-validated: temporarily disabling the long-form emit makes exactly the 3 positive tests fail; the variable-bound-verb anti- regression test continues to pass (it pins behavior independent of the emit being on or off). Local validation: - test/unit/group/http-route-extractor.test.ts: 66/66 ✅ - test/unit/group: 546/546 ✅ - npx prettier --check (changed files): clean ✅ * test(group): address Claude review findings F1 and F2 on PR #1884 Two minor follow-ups from the production-readiness review: F1 — Stale block comment at the top of the Kotlin consumer suite (was: "Three consumer flavors covered here ... long-form deferred to a follow-up"). Updated to "Four consumer flavors" and removed the deferred sentence — the deferral is resolved by this PR. The kotlin.ts file header was already updated; this brings the test file comment in sync. Per DoD §2.3 (no stale comments). F2 — Replaced `expect(wcConsumers.length).toBeGreaterThanOrEqual(4)` with `expect(wcConsumers).toHaveLength(4)` in the multi-verb test. The fixture is fully deterministic — exactly 4 long-form calls, no other consumer types — so an exact count assertion is the right shape per DoD §2.7 ("use toBe / toEqual for exact expectations"). Added a comment explaining what the assertion catches that the existing per-verb toBeDefined() checks would miss (accidental 5th consumer from a duplicate query firing or a regressed receiver constraint). F3 (HEAD/OPTIONS/TRACE negative test) is intentionally not added in this PR — same precedent as #1855 where HEAD/OPTIONS/TRACE on the short form are also implicitly excluded without a pinning test. Happy to add one in a separate PR if maintainers want explicit pinning across both forms. F4 (CI on pre-merge SHA) is the maintainer's call — the merge from main is theirs to re-trigger CI on. The merge brings only Java consumer changes (PR #1872) and Go provider changes (PR #1886), both in entirely separate files from this PR's Kotlin work. Local validation: - test/unit/group/http-route-extractor.test.ts: 73/73 ✅ (66 from this PR pre-merge + 7 from PR #1872 merged via main) - npx prettier --check (changed files): clean ✅ * refactor(group): hoist Kotlin WebClient long-form verb regex to module scope Address @magyargergo's review request on PR #1884: > Can you please extract the regexp from the for loop? 🙏 (kotlin.ts:510) Compiles the verb whitelist `^(GET|POST|PUT|DELETE|PATCH)$` once at module load instead of every iteration of the long-form scan loop. Mirrors the placement and JSDoc style of the sibling `WEB_CLIENT_SHORT_TO_HTTP` constant. Behavior is unchanged — same verb whitelist, same exclusion of HEAD/OPTIONS/TRACE for symmetry with the short form. The 4 itKotlinConsumer long-form tests added in this PR continue to pass, and the variable-bound-verb anti-overreach test continues to pin the deliberate non-match. Local validation: - test/unit/group/http-route-extractor.test.ts: 77/77 ✅ - test/unit/group: 557/557 ✅ - npx prettier --check (changed file): clean ✅ --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-29 09:27:54 +01:00
JaysonAlbert	7dae4fcc41	fix(group): attribute Spring interface routes to controllers (#1743) ... * fix(group): attribute Spring interface routes to controllers * test(group): normalize Spring route fixture paths --------- Co-authored-by: gfwangjie <gfwangjie@gf.com.cn>	2026-05-29 07:35:01 +01:00
MyShining	7b38b8aae2	feat(java): add HTTP consumer contract extraction (#1872)	2026-05-29 06:05:40 +01:00
henry201605	b565c7c990	feat(ingestion): resolve FastAPI include_router(prefix=...) cross-file routes (#1877) ... * feat(ingestion): resolve FastAPI include_router(prefix=...) cross-file routes FastAPI sub-route files declare paths via @router.<verb> while the entry file mounts the router with app.include_router(<router>, prefix='/x'). Previously both the ingestion-layer Route graph nodes and the group-layer ExtractedContract URLs lost the cross-file prefix, breaking provider <-> consumer matching. Ingestion layer: - parse-worker emits routerIncludes / routerImports + decoratorReceiver - parsing-processor / parse-impl thread the new fields and aggregate prefixesByModule across chunks; decorator routes whose receiver is 'router' are duplicated once per matching prefix - routes.ts joins prefix via normalizeExtractedRoutePath Group layer: - HttpLanguagePlugin gains an optional prepareRepo() pre-pass and a repoContext arg to scan(); python.ts builds prefixesByModule and falls back to the bare path when no entry matches - http-route-extractor caches one repoContext per plugin Tests: - 3 new http-route-extractor cases (attr / named-import / no-prefix) - ParseWorkerResult literals in 3 test files updated to the new shape Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(ingestion,group): address PR #1877 review — relative imports, cross-package collisions, host names, ingestion tests Follow-ups to the FastAPI `include_router(prefix=...)` cross-file fix based on PR #1877's automated production-readiness review. Three correctness gaps and one test coverage gap addressed: 1. Relative-import support in the worker regex (FINDING 2) `FROM_IMPORT_ROUTER_RE` now accepts module paths starting with a `.` (e.g. `from .calls import router as calls_router`). The previous `[A-Za-z_][\w.]*` rejected leading dots and silently dropped every relative-import Shape-B include — a real pattern from the PR description's own motivating example. The matching helpers now strip leading dots before keying so absolute and relative imports collapse to the same module key. 2. Cross-package same-name module collisions (FINDING 3) Two-tier module keying replaces the previous basename-only key: • short key — `users` (file basename without `.py`) • long key — `api/users` (parent dir + stem) `prefixesByLongKey` is consulted first and only falls back to `prefixesByShortKey` when no long-key match is available. Both the ingestion pipeline (parse-impl.ts) and the group extractor (http-patterns/python.ts) carry the same scheme so the graph nodes and HTTP contracts agree on which prefix applies. New protocol field `ExtractedRouterModuleAlias` (parse-worker → parsing-processor → parse-impl) lets Shape-A `<host>.include_router(<mod>.router, prefix='/x')` calls promote to a long key when the same file imports `<mod>` via `from <pkg> import <mod>`. Without this, `api/users.py` and `admin/users.py` collided on the basename `users` and the admin file's routes inherited the `/users` prefix that was only meant for `api/users.py`. 3. Non-`app` host variable names (FINDING 4) The group-layer `INCLUDE_ROUTER_*_PATTERNS` queries pinned the host identifier to the literal `"app"` and dropped every `application = FastAPI()` / `api = FastAPI()` pattern — the constraint was redundant given that the call shape (`include_router` invoked with a router argument and a `prefix=` keyword) is already specific enough. The pin is removed; the ingestion regex was already unrestricted. 4. Ingestion-layer regression tests (FINDING 1) The previous PR added group-layer tests (`http-route-extractor.test.ts`) but zero in-tree tests for the ingestion path. Two new suites pin the worker → parse-impl → routes flow: - `test/unit/fastapi-router-bindings.test.ts` (23 cases): `extractFastAPIRouterBindings()` is split into a stand-alone module so it can be unit-tested without booting a worker thread, then pinned for regex shape, two-tier key emission, relative-import support, and negative cases. - `test/integration/fastapi-prefix-pipeline.test.ts` (5 cases) plus `test/fixtures/fastapi-prefix-app/` — runs the full `runPipelineFromRepo()` against a realistic multi-package fixture (containing both `api/users.py` and `admin/users.py`) and inspects the resulting `Route` graph nodes for cross-file prefix joining and absence of cross-package bleed. Verification - `npx tsc --noEmit`: pass - PR-touched test suites (6 files / 117 cases): all green - `npx prettier --check`: pass on touched files - `npx eslint`: 0 errors on touched files Cache / compatibility The new `routerModuleAliases?` field on `ParseWorkerResult` and `routerModuleAliases` on `WorkerExtractedData` are optional / guarded with `?? []`, so historical parse-cache entries continue to load without forced re-scan. Refs PR #1877. * refactor(ingestion): move fastapi-router-bindings out of workers/ — pure module, not a worker Addresses @magyargergo's `CHANGES_REQUESTED` review on PR #1877: > Sorry I just found that we are introducing a new worker in the PR. `gitnexus/src/core/ingestion/workers/fastapi-router-bindings.ts` was a **pure-function module** — it never imported `worker_threads` or `parentPort`, never spawned a worker, and was never registered as a worker entry. It was placed in `workers/` purely because it was split out of `workers/parse-worker.ts` to make its functions unit-testable without booting a worker thread (parse-worker is itself the worker entry and cannot be loaded from the main thread). To remove the misleading directory placement: • The implementation moves to `gitnexus/src/core/ingestion/route-extractors/fastapi-router-bindings.ts`, alongside the other framework-specific route extractors (`expo`, `nextjs`, `php`, `laravel`, `middleware`, `response-shapes`). • `workers/parse-worker.ts` keeps a thin re-export so the worker entry can keep using `extractFastAPIRouterBindings` directly. The re-export now carries an explicit comment stating that the imported file is **not** a worker and that the `workers/` directory deliberately hosts only true worker entries (`parse-worker.ts`, `worker-pool.ts`, `quarantine.ts`). • The new file's leading docstring opens with "NOT A WORKER" and explains why it exists where it does. • The unit test (`test/unit/fastapi-router-bindings.test.ts`) is updated to import from the new path. No behaviour change. The function body, signatures, and exported types are identical. Verification • `npx tsc --noEmit`: pass • `npx tsc` (dist rebuild): pass • `test/unit/fastapi-router-bindings.test.ts` (23 cases): all green • `test/integration/fastapi-prefix-pipeline.test.ts` (5 cases): all green • `test/unit/group/http-route-extractor.test.ts` (63 cases): all green • `npx prettier --check` on touched files: pass • `npx eslint` on touched files: 0 errors Refs PR #1877. * refactor(ingestion): drop parse-worker re-exports; consumers import router types directly from route-extractors Addresses @magyargergo's two remaining review comments on PR #1877: 1. **`gitnexus/src/core/ingestion/workers/parse-worker.ts:247`** — "Can you please remove them and update the call sites?" The `export type { ExtractedRouterInclude, ExtractedRouterImport, ExtractedRouterModuleAlias } from '../route-extractors/...'` block in parse-worker.ts is gone. The remaining `import type {…}` is purely local — used only to type the corresponding fields on `ParseWorkerResult` below — and the leading comment now says so explicitly ("this file does NOT re-export them"). The `extractFastAPIRouterBindings` symbol is also no longer re-exported from parse-worker.ts; it's still imported here so the worker entry can call it per file, but downstream consumers must reach it via `route-extractors/fastapi-router-bindings` directly. Call sites updated: - `gitnexus/src/core/ingestion/parsing-processor.ts` - `gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts` Both files now `import type { ExtractedRouterInclude, ExtractedRouterImport, ExtractedRouterModuleAlias }` directly from `route-extractors/fastapi-router-bindings.js`. The worker types they still need (`ParseWorkerResult`, `ExtractedToolDef`, etc.) keep coming from `workers/parse-worker.js`. The unit + integration tests already imported from the new path, so no test changes were required. 2. **`gitnexus/src/core/ingestion/parsing-processor.ts:168`** — suggested simplification: for (const item of result.routerIncludes ?? []) allRouterIncludes.push(item); for (const item of result.routerImports ?? []) allRouterImports.push(item); for (const item of result.routerModuleAliases ?? []) allRouterModuleAliases.push(item); Applied verbatim. Replaces the previous `if (result.…) for …` guards. The cache-compat semantics are unchanged — historical parse-cache entries that lack these fields still load cleanly, the new form just spells the fallback inline. No behavior change, no tests touched, no public API change. Verification • `npx tsc --noEmit`: pass • `npx tsc` (dist rebuild): pass • PR-touched test suites (6 files / 117 cases): all green • `npx prettier --check` on touched files: pass • `npx eslint` on touched files: 0 errors Refs PR #1877. * refactor(ingestion): hoist fastapi-router-bindings type imports to top of parse-worker.ts Move the `import type { ExtractedRouterInclude, ExtractedRouterImport, ExtractedRouterModuleAlias }` block to the top of the file with the other type imports, and drop the comment that previously sat next to ExtractedDecoratorRoute. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-28 19:04:19 +01:00
henry201605	d9d6318b64	feat(group): add Kotlin Spring HTTP consumer extraction (#1855) ... * feat(group): add Kotlin Spring HTTP consumer extraction Follow-up to #1849 (Kotlin providers). Extends `http-patterns/kotlin.ts` with three call-site patterns common in Kotlin Spring projects: - RestTemplate: `restTemplate.getForObject("/x", ...)` and the full verb family (getForObject/getForEntity → GET, postForObject/postForEntity → POST, put → PUT, delete → DELETE, patchForObject → PATCH). Mirrors the Java plugin's `REST_TEMPLATE_TO_HTTP` map so polyglot repos coalesce on a single contract id. - WebClient short form: `webClient.get().uri("/x")` and the `.post()` / `.put()` / `.delete()` / `.patch()` siblings. The chain parses as two nested `call_expression` nodes; the query anchors on the outer `.uri(...)` and walks one level inward to constrain the verb. - OkHttp: `Request.Builder().url("/x")`. Kotlin parses `Request.Builder()` as a `call_expression` whose callee is a `navigation_expression` (not Java's `object_creation_expression`), so the query shape differs from `java.ts` but the receiver/method constraints (`Request` / `Builder` / `url`) and emitted contract format match. Out of scope: `webClient.method(HttpMethod.X).uri("/y")` long form. The verb sits on a sibling `call_expression` two hops away, so it needs a walk-up helper rather than a flat tree-sitter query. A dedicated anti-overreach test pins the current behavior so a future short-form change can't accidentally start matching the long form. Receiver name constraints (`#eq? @obj "restTemplate"`, `#eq? @cls "Request"`) match the Java plugin's heuristic — a project that aliases the receiver under a different name won't be picked up. This trade-off keeps false-positive rates low and is documented in the file header. Tests: 5 new cases under `consumer extraction — fetch patterns`, gated by tree-sitter-kotlin grammar availability. positive (3) - RestTemplate verbs (5 calls × 5 verbs) - WebClient short-form verbs (5 calls × 5 verbs) - OkHttp Request.Builder().url("/x") anti-regression (2) - WebClient long form `.method(HttpMethod.X)` produces no consumer (deferred-feature pin) - non-restTemplate receiver does not match (receiver-name pin) Reverse-validated: removing the `(#eq? @obj "restTemplate")` constraint causes the receiver-name anti-regression test to fail. Local validation: - test/unit/group/http-route-extractor.test.ts: 59/59 ✅ - test/unit/group: 539/539 ✅ - npm run format:check: clean ✅ * test(group): pin Kotlin OkHttp POST-chain heuristic-default GET behavior Address Claude review on PR #1855 (Finding 1). The OkHttp query in `kotlin.ts:OK_HTTP_PATTERNS` matches the `.url("/x")` sub-expression of a builder chain, but the verb is encoded on a separate sibling call (`.post(body)` / `.delete()` / ...). The query intentionally does not walk the chain to recover the verb — it emits `method: 'GET'` for every match, mirroring the Java plugin's `OK_HTTP_PATTERNS` (java.ts). Concretely: `Request.Builder().url("/x").post(body).build()` becomes `http::GET::/x`, not `http::POST::/x`. This is an already-accepted Java parity heuristic, but it was untested on the Kotlin side. This commit: - Adds an anti-overreach test pinning the current behavior: * exactly one consumer is emitted with method=GET * no second http::POST::/x consumer appears - Documents the limitation in kotlin.ts as a "Known limitation" block tied to the test, so a future verb-walk implementation has to update the comment in lockstep with the assertion. Rationale for not implementing verb-walk in this PR: - Verb-walk requires walking sibling call_expression nodes (the `.post(body)` chain), which is the same shape as the deferred WebClient long-form work - Java has the same limitation in production today; fixing only Kotlin would create polyglot drift - A coordinated future PR can add verb-walk to both plugins at once and update both comments + the pin tests together Finding 2 (silent test-skip when tree-sitter-kotlin grammar is unavailable) is intentionally NOT addressed here — same gating pattern was accepted in #1849 for Provider tests, and a coordinated follow-up should add a CI sentinel covering both Provider and Consumer suites in one place. Local validation: - test/unit/group/http-route-extractor.test.ts: 60/60 ✅ - test/unit/group: 540/540 ✅ - npm run format:check: clean ✅ --------- Co-authored-by: henry <zhangwei2017@unipus.cn>	2026-05-27 21:32:24 +01:00
henry201605	46eb0ebf56	feat(group): add Kotlin Spring HTTP route extraction (named + positional) (#1849) ... * feat(group): add Kotlin Spring HTTP route extraction (named + positional) Mirror the Java Spring named-argument fix for Kotlin Spring Boot controllers. Adds a new `http-patterns/kotlin.ts` plugin behind the optional `tree-sitter-kotlin` grammar, registered for `.kt`/`.kts`. Both annotation forms produce providers: @RequestMapping("/api") / @GetMapping("/users") @RequestMapping(path = "/api") / @GetMapping(value = "/users") @RequestMapping(value = "/api") / @GetMapping(path = "/users") The Kotlin AST (fwcd/tree-sitter-kotlin) shares one node type (`value_argument`) for positional and named forms, so the queries are split: - positional: anchors `string_literal` as the first named child of `value_argument` via the immediate-child anchor `.` - named: explicitly captures `simple_identifier` and constrains it to `^(path|value)$` via `#match?`, mirroring the same safety bar enforced by `http-patterns/java.ts` and `topic-patterns/java.ts`. Without this constraint the query would also capture non-route attributes like `produces`, `consumes`, `headers`, `name`, `params`. `tree-sitter-kotlin` is an optionalDependency (parser-loader.ts, parse-worker.ts pattern). When the native binding is unavailable the plugin exports `null` and `index.ts` skips registering `.kt`/`.kts` so the orchestrator stays healthy. Scope: providers only. Consumer detection (RestTemplate, WebClient, OkHttp) on Kotlin call-site ASTs differs enough from Java's `method_invocation` shape to warrant a separate, focused PR. Tests: 11 new cases under `provider extraction — source-scan fallback (Strategy B)`, gated by the kotlin grammar availability. positive (8) - class @RequestMapping("/api/v1") (positional) - class @RequestMapping(path = "/api/v2") - class @RequestMapping(value = "/orders") - method @GetMapping(value = "/users") - method @GetMapping(path = "/users") - method @PostMapping(path = "/users") - mixed: class named-arg + method positional - mixed: class positional + method named-arg anti-regression (3) - @GetMapping(produces = "application/json") emits no provider - @GetMapping(name = "x", value = "/users") emits exactly one provider - @RequestMapping(path = "/api", name = "myApi") prefix stays /api Reverse-validated: removing the `(#match? @key "^(path|value)$")` constraint causes precisely the 3 anti-regression tests to fail. Local validation: - test/unit/group/http-route-extractor.test.ts: 54/54 - test/unit/group: 534/534 - npx tsc --noEmit: clean (modulo the pre-existing TS2339 in user-defined-conversions.ts merged from main, unrelated) * style(test): apply prettier line wrapping to long itKotlin titles --------- Co-authored-by: henry <zhangwei2017@unipus.cn>	2026-05-27 09:33:52 +01:00
henry201605	eeea46466b	fix(group): handle named annotation args in Java Spring route extraction (#1834) ... * fix(group): handle named annotation args in Java Spring route extraction The Java HTTP plugin only matched positional `@RequestMapping("/path")` syntax for class-level prefixes and method-level routes. Named argument forms (`path = "/path"` and `value = "/path"`) produce an `element_value_pair` AST node that the tree-sitter queries did not cover, causing the class prefix to be lost and named-arg method routes to be missed entirely during cross-repo contract extraction. Add a second pattern to both SPRING_CLASS_PREFIX_PATTERNS and SPRING_METHOD_ROUTE_PATTERNS matching the element_value_pair structure. * fix(group): constrain Spring named-arg query to path/value keys + add regression tests Address Claude review on PR #1834. The named-argument patterns added in 8b6fa6e used `value: (string_literal)` (a tree-sitter field selector for the right-hand side of element_value_pair), which matched ANY annotation member with a string value — not just `path`/`value`. Concrete fallout (without this fix): @GetMapping(produces = "application/json") → bogus http::GET::/application/json @GetMapping(name = "listUsers", value = "/users") → extra http::GET::/listUsers @RequestMapping(headers = "X-Foo=bar", path = "/api") → class prefix could be set to "X-Foo=bar" because prefixByClassId.set runs per match in document order, so the LAST element_value_pair wins. The sibling topic-patterns/java.ts already demonstrates the correct shape: constrain the `key:` field to the route member names. This commit: - Adds `key: (identifier) @key (#match? @key "^(path|value)$")` to both SPRING_CLASS_PREFIX_PATTERNS and SPRING_METHOD_ROUTE_PATTERNS named-arg queries. - Adds 9 regression tests under `provider extraction — source-scan fallback (Strategy B)`: * @RequestMapping(path = "/api/v3") class prefix * @RequestMapping(value = "/orders") class prefix * @GetMapping(value = "/users") method route * @PostMapping(path = "/users") method route * mixed: class named-arg + method positional * mixed: class positional + method named-arg * @GetMapping(produces = "application/json") → no provider emitted * @GetMapping(name = "listUsers", value = "/users") → exactly one provider with path "/users", no /listUsers route * @RequestMapping(path = "/api", name = "myApi") → prefix is /api, not myApi (verifies the class-prefix overwrite scenario) Tests: 42/42 pass in http-route-extractor.test.ts; 522/522 pass under test/unit/group; npx tsc --noEmit clean. * test(group): add @GetMapping(path = ...) case to match review checklist verbatim Claude review on PR #1834 explicitly asked for the method-level `@GetMapping(path = "/users")` case. The previous commit covered it indirectly by exercising path= on @PostMapping (the Spring method annotations share the same query, so any verb proves the path= field is matched). Add a dedicated GET+path= test so the reviewer's checklist is satisfied 1:1, and keep the POST+path= case as a bonus verb-coverage test. Tests: 43/43 pass in http-route-extractor.test.ts. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-27 07:35:46 +01:00
Minidoracat	2b6e7ffbd9	fix(php): avoid Blade templates entering PHP analysis (#1790)	2026-05-23 23:37:40 +01:00
luyua9	df2ed009ce	fix(group): detect httpx AsyncClient alias imports (#1687) ... * fix(group): detect httpx AsyncClient alias imports * fix(group): anchor httpx dotted imports and skip shadowed aliases Addresses Findings 1-3 of the production-readiness review on PR #1687. - F1: the `(dotted_name (identifier) @module)` capture matches every segment of a dotted module path, so `import package.httpx as hx` and `from package.httpx import AsyncClient` would falsely populate the alias sets. Anchor the check on `moduleNode.parent?.text === 'httpx'` so the full dotted_name must equal `httpx`. - F2: `moduleAliases` and `asyncClientAliases` were file-global and unaware of Python scope. A function-local rebind like `AsyncClient = lambda: MockClient()` left the alias entry intact and any subsequent `client = AsyncClient(); client.get(...)` emitted a false-positive consumer contract. Walk every `(assignment left: (identifier) @name)` whose name matches an alias, record the enclosing function/class scope as poisoned, and skip direct- and module-attribute matches when the call site is inside that scope chain. - F3: extend the existing fixture with dotted-package look-alikes and three local-shadow cases (`shadow_direct_alias`, `shadow_module_alias`, `shadow_direct_context`) and assert the would-be FP contractIds are not emitted. - F6: refresh the module-level docstring to mention the supported import-alias forms and the shadow-exclusion behavior. * refactor(group): tighten httpx alias shadow detection and broaden tests Follow-up addressing the residual review findings on PR #1687. - Replace inline scope-key construction in isAliasShadowed with a getScopeKey call so the two helpers cannot drift apart (M1). - Collapse the double tree traversal in collectHttpxAsyncClients: build one combined alias set and pass it to a single collectAliasShadowScopes call (perf, P2). - Add a `shadowScopeKey` helper that returns the scope a rebind actually shadows under Python LEGB rules: function scope for in-function rebinds, 'module' for top-level rebinds, and `null` for class-body rebinds (class attributes do not shadow bare-name lookups in methods). Removes the previous blanket `scopeKey === 'module'` skip and now correctly poisons module-level rebinds (correctness #1). - Extend `ALIAS_SHADOW_PATTERNS` to cover tuple, list, and pattern_list destructuring targets (correctness #2). - Rename `ALIAS_REBIND_PATTERNS` to `ALIAS_SHADOW_PATTERNS` and update the block comment to say "shadowed" rather than "poisoned" (M4). - Collapse `callScopeKeys` to a single-line return; the dead Set wrap was misleading future readers (M2). Tests: - New negative fixtures for 3-segment dotted import (`import a.b.c.httpx as deep_evil`), relative import (`from .httpx import AsyncClient as rel_evil_async`), tuple destructuring rebind, and an isolated file exercising the module-level rebind path (T1, correctness #2, expanded F2). - New positive fixture confirming that a class-body assignment of `AsyncClient` does NOT poison the surrounding methods. - Add a positive control assertion for `module_direct_client` so the dotted-package negative assertions cannot pass vacuously (T3). --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Test <test@example.com>	2026-05-21 18:24:27 +01:00
azizur100389	aa8f4d6efe	fix(group): Union HTTP graph and source contracts (#1709) ... * Union HTTP graph and source contracts * test(group): Document HTTP source union follow-ups --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-20 17:44:07 +01:00
juyua9	55b7a79beb	fix(group): detect httpx async consumers (#1408) ... * fix(group): detect httpx async consumers * test(group): tighten httpx consumer coverage * test(group): create extractor temp dirs safely * fix(group): scope httpx async client tracking * fix(group): tighten httpx module-scope tracking Prevent module-scope httpx.AsyncClient tracking from matching same-name local variables inside functions. Also documents the intentionally unsupported direct-import, alias, and typed-assignment forms, and extends the httpx extractor regression fixture to cover module-scope shadowing while keeping module-scope calls detected. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-11 13:13:49 +01:00
Hector Prats	d69eadfb7f	fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV (#1433) ... * fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV tree-sitter 0.21.x on Windows crashes with SIGSEGV when parsing source strings longer than 32 767 chars (signed 16-bit integer overflow in the native binding). Five call sites passed raw file content without any length guard: - captures.ts (C# scope extraction) - namespace-siblings.ts (extractFileStructure) - parse-worker.ts (worker thread parse path) - parsing-processor.ts (sequential parse fallback) Fix: truncate at the last newline before the limit so the fragment stays syntactically coherent. Files truncated mid-class produce ERROR roots; captures.ts returns [] for any ERROR-root tree so the legacy DAG handles the file silently without orphaned scope errors. Additional C# scope fixes: - scope-tree.ts: Module scopes may share the same range as a top-level namespace_declaration (files with no leading `using` directives). The rangeStrictlyContains check rejects equal ranges. Added rangeNonStrictlyContains for Module parents. - scope-extractor.ts: pass1BuildScopes stack-pop used strict containment; same Module == Namespace range case caused orphaned scopes. Added moduleAwareContains helper. - scope-extractor-bridge.ts: empty captures from ERROR-root files still called extractScope -> "no Module scope found" warning. Added early return for empty/non-array captures. - namespace-siblings.ts: three sites pushed onto binding arrays frozen by finalize-algorithm. Fixed with spread-copy before mutation. lbug-adapter.ts: INSTALL VECTOR in loadVectorExtension calls the KuzuDB native extension installer, which crashes with SIGSEGV on Windows via an unhandled error path in native code. JS try/catch cannot intercept native signals. Skip extension loading on win32 — vector/embedding search is unavailable on Windows but all graph index queries work correctly. Verified on: Windows 11, Node.js 24, gitnexus 1.6.3, pcf8-game codebase (61 757 nodes / 111 796 edges / 300 flows after fix). * fix(windows): skip FTS extension load in pool-adapter on Windows to prevent SIGSEGV LOAD EXTENSION fts crashes the process with SIGSEGV on Windows when the FTS extension binary is not installed locally. This is an @ladybugdb/core native bug — the extension loader hits an unhandled error path that raises a native signal instead of a JS exception, so try/catch cannot protect here. Add a process.platform === 'win32' guard in both doInitLbug and initLbugWithDb. When skipped, bm25-index.js catches the resulting Kuzu catalog errors (CREATE_FTS_INDEX not defined) and returns empty BM25 results gracefully. All graph queries (cypher, context, impact) are unaffected. This is patch 9 of the Windows fix series for gitnexus on Windows: patch 8 (same PR) already fixed INSTALL VECTOR SIGSEGV in lbug-adapter.ts. pool-adapter.ts is the separate MCP-server code path that was not covered. * fix: address codeql findings on PR #1433 The four `lastIndexOf('\n', ...)` calls were committed with a literal newline inside the single-quoted string instead of the `\n` escape, so the files do not parse — `tsc` and CodeQL both flagged them. Replace the embedded newline with `'\n'`. Also remove the two helpers that were superseded during review and became dead code: `rangeNonStrictlyContains` in scope-tree.ts (the equal-range carve-out is handled by `rangeStrictlyContains` + `rangesEqual` in `canParentScope`) and `moduleAwareContains` in scope-extractor.ts (`pass1BuildScopes` calls `canParentScope` directly). * fix(windows): replace 32767-char truncation with chunked-input parsing The tree-sitter 0.21.x Node binding crashes (SIGSEGV) on Windows when parser.parse(string, ...) is handed a JS string longer than 32 767 chars. The crash is in the bindings V8 string-to-buffer conversion and cannot be intercepted from JS. Previous mitigation truncated source at the last newline before that boundary, silently losing the file tail and producing ERROR-root trees from mid-class cuts. Switch to the callback (Parser.Input) overload via a new parseSourceSafe helper. tree-sitter pulls source in 16 KiB chunks via repeated callback invocations, bypassing the broken conversion path. Files are parsed in full, no data loss, no platform-specific code path. Removes the now-unnecessary ERROR-root short-circuit in csharp/captures.ts and the empty-captures shim in scope-extractor-bridge.ts; both existed only to swallow truncation-induced parse failures. * fix(windows): cover all parse sites and correct vector-extension state Address adversarial review on PR #1433: 1. Extend parseSourceSafe to all remaining parser.parse() call sites that handle full file content. The first commit only converted the four sites with active truncation hacks; cache-miss paths in call-processor (x2), heritage-processor (x2), import-processor, and the Go/Python/TypeScript captures + Go range-binding still called parser.parse() directly. On Windows those would still SIGSEGV for files > 32767 chars. 2. Stop setting vectorExtensionLoaded = true on the win32 short-circuit in lbug-adapter.ts. The flag means "successfully loaded" and is checked by an early-return at the top of loadVectorExtension; setting it on the skip path made the second call return true and let QUERY_VECTOR_INDEX run against a DB without the extension. 3. Drop the placeholder issues/... URL in the same comment. 4. Add unit tests for parseSourceSafe at boundary values: 16 KiB (direct/callback boundary), the 32 767 Windows crash boundary, single-line > chunk size, CRLF near boundary, and large all-Chinese source. Confirms the callback path is correct for non-ASCII content, which is also exercised by the existing csharp-captures large-file test. Researched the chunking concern: tree-sitter Node binding sets TSInputEncodingUTF16 and divides byte_index by 2 in ByteCountToJS before calling the JS callback, so the index argument is a UTF-16 code-unit offset — matching String.prototype.slice. Splitting tokens across chunks is safe by API contract; the lexer is chunk-agnostic. * fix(windows): extend parseSourceSafe to group/embeddings + lint enforcement Closes the remaining Windows SIGSEGV exposure flagged by the Codex adversarial review on PR #1433. Six pre-existing parser.parse(content) call sites bypassed parseSourceSafe and could crash the process on Windows when a contract IDL, route file, or embedding-target source exceeded 32 767 chars. Adds a lint rule so the regression vector closes permanently. Production code: - Relocate parseSourceSafe from ingestion/utils/ to core/tree-sitter/ so group/ and embeddings/ can import without crossing into ingestion internals. core/tree-sitter/ already houses parser-loader.ts and is the natural shared facade. All 11 existing importers updated; no shim left behind in the old location. - Route through parseSourceSafe in 5 group extractors (grpc, thrift, http-route, include, tree-sitter-scanner) and the embeddings ensureAndParse helper. - The seventh direct .parse() call in grpc-patterns/proto.ts:49 is a module-load grammar smoke test parsing a 36-char literal. Trivially safe by inspection, intentionally direct, filtered out by the lint rule via the string-literal-arg skip. Tests: - 5 caller-side regression tests with a vi.spyOn assertion on parseSourceSafe. The spy is what catches a regression: parser.parse on a 40 000-char input succeeds on Linux/macOS, so a "no throw" assertion alone would silently pass with the bypass reintroduced. - The vi.mock boilerplate is centralised in gitnexus/test/helpers/parse-source-safe-mock.ts, dynamic-imported inside each mock factory so vitest's hoister does not race the static import binding. Lint: - New custom ESLint rule gitnexus/require-safe-parse, scoped to gitnexus/src/core/**, fails on direct <parser>.parse(<non-literal>, ...) calls and auto-fixes them to parseSourceSafe(<parser>, ...). Skips JSON/URL/marked/Number/Math, string-literal first args (smoke tests), test files, and the helper itself. Auto-fix rewrites the call site only; the developer adds the import after tsc surfaces the missing identifier — same tradeoff as unused-imports/no-unused-imports. Plan: docs/plans/2026-05-10-001-fix-windows-parse-safety-group-and-embeddings-plan.md * fix(test): use mkdtempSync in http-route-extractor regression test Address CodeQL js/insecure-temporary-file warning on the new Windows- SIGSEGV regression test. The test was using path.join(tmpDir, "large-input") which, when nested inside a Date.now()-based parent tmpDir, lets CodeQL flag the directory as a predictable-name temp file with race-condition risk. Switch to fs.mkdtempSync(path.join(tmpDir, "large-input-")) so the suffix is a secure unique random string. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-10 16:00:36 +01:00
azizur100389	4be4abe8e4	fix(group): contract extractors honour .gitnexusignore via shared IgnoreService (#1185) (#1247) ... * fix(group): contract extractors honour .gitnexusignore via shared IgnoreService (#1185) The HTTP, gRPC, and topic contract extractors each globbed the repo with a hardcoded `ignore: ['**/node_modules/**', '**/.git/**', '**/dist/**', '**/build/**', '**/vendor/**']` array, bypassing the shared `IgnoreService` that the rest of the ingestion pipeline uses for `.gitnexusignore` and `.gitignore` parsing. Result: a vendored Python venv (`mentor_env/`), generated stubs, or any user-defined exclusion silently produced false-positive contracts. Replace each hardcoded array with `createIgnoreFilter(repoPath)`, mirroring the canonical pattern in `filesystem-walker.ts`. The 5 hardcoded names are all in `DEFAULT_IGNORE_LIST`, so default behaviour is preserved; users now also get `.gitnexusignore` patterns, the rest of the hardcoded list (e.g. `__pycache__`, `.pytest_cache`), and the `.gitnexusignore` negation semantics introduced in #771. The topic extractor additionally filters Go `*_test.go` at the glob level. That filter is preserved via a small wrapper around `createIgnoreFilter` that short-circuits before delegating, so glob-level pruning still applies and the existing `_test.go` skip test (with new content asserting the pruning is real) still passes. Tests added to all three `*-extractor.test.ts` files exercising `.gitnexusignore` honouring end-to-end via real temp directories. * test(group): exercise gRPC source-scan ignore + add .gitignore-only coverage (#1185) Addresses two findings from the @claude review on PR #1247: [medium] The gRPC ignore test claimed to cover both proto-context and source-scan paths but only wrote a .proto file under mentor_env/. Added a Python `_pb2_grpc.<Name>Stub(channel)` consumer file under the same ignored dir (mirroring the canonical pattern from `test_extract_python_stub_returns_consumer`); without the `.gitnexusignore` filter that file would emit a consumer contract. The test now exercises both `createIgnoreFilter` calls inside the gRPC extractor (`buildProtoContext` + `extract`) in a single run, with both defence-in-depth path-prefix assertions and a specific `role: consumer` LeakedService assertion. [low] Added one shared .gitignore-only test on the HTTP extractor. `createIgnoreFilter` reads both `.gitignore` and `.gitnexusignore` via `loadIgnoreRules`, but no extractor-level test exercised the `.gitignore` path. One shared test is sufficient because all three extractors consume the same filter object — verified at `IgnoreService` level already. The remaining [low] finding — "negation semantics (!pattern) not tested at extractor level" — is deferred deliberately, not skipped. Three reasons: 1. The negation logic (introduced in #771) lives entirely inside `createIgnoreFilter`'s `hasExplicitUnignore` ancestor-walk in `ignore-service.ts`. The extractors only consume the returned filter object — they never inspect patterns, never call `hasExplicitUnignore` directly, and have no code path that could diverge from the IgnoreService's negation behaviour. 2. Negation is already locked in by 8 dedicated unit tests in `test/unit/ignore-service.test.ts` (the #771 suite), plus the `!parent/` + `parent/child/` last-match-wins regression test added in PR #1046. An extractor-level negation test would re-prove the same code path and would not catch any failure mode the existing tests don't already catch. 3. The bot itself flagged the gap as "Acceptable to leave as follow-up referencing existing IgnoreService negation tests" — the deferral matches its own recommendation. If a future change inserts an extractor-side wrapper around the filter (as topic-extractor.ts already does for `*_test.go`) that could plausibly affect negation, an extractor-level negation test should be added at that point — not pre-emptively here.	2026-05-01 16:42:21 +01:00
Jonas Vanderhaegen	06967e2b66	feat(extractors): add PHP HTTP consumer detection (#993) ... Extend the PHP tree-sitter plugin to emit consumer HttpDetections for three common PHP HTTP call shapes, matching Node plugin parity: - Laravel HTTP client: Http::get/post/put/delete/patch($url) - Guzzle / generic: $client->get/post/...($url) - file_get_contents($url) when the URL is absolute http(s):// String-literal URLs only. Paths built via binary concatenation (`$base . '/path'`), sprintf, or config lookups are intentionally deferred — they need constant-folding of the enclosing scope to be useful and are tracked as follow-up work. Refs #992 Co-authored-by: Jonas Vanderhaegen <jonasvanderh+claude.ai@gmail.com>	2026-04-20 17:35:31 +01:00
azizur100389	f221f93341	feat(extractors): detect jQuery $.ajax/$.get/$.post and axios object-form as HTTP consumers (#887) ... * feat(extractors): detect jQuery $.ajax/$.get/$.post and axios object-form as HTTP consumers The JS/TS HTTP consumer extractor currently recognises fetch() and axios.<verb>() but misses three patterns extremely common in Laravel and legacy frontends: - jQuery shorthand: $.get(url), $.post(url, data) - jQuery ajax form: $.ajax({ url, method }) / $.ajax({ url, type }) - axios object form: axios({ method, url }) Missing them means the frontend->backend cross-link disappears from `group sync`, breaking impact analysis for whole classes of repos. Implementation (node.ts): - 3 new PatternSpecs alongside the existing FETCH_/AXIOS_ specs - NodePatternBundle extended with jqueryShorthand / jqueryAjax / axiosObject slots, compiled for JS / TS / TSX grammars - readStringProp() helper walks object-literal `pair` children and resolves `url` / `method` / `type` keys independent of order, sidestepping the positional S-expression constraint on the query form proposed in the issue - 3 new scan loops in scanBundle() emit HttpDetection with framework 'jquery' (new) or 'axios' (existing), confidence 0.7 to match the existing source-scan consumers, defaulting method to GET when absent (matches both jQuery and axios runtime) Tests (http-route-extractor.test.ts): 4 new cases -- 3 positive (shorthand, ajax with method:/type: and default GET, object-form with swapped key order and default GET) plus 1 negative control that asserts unrelated \$.fn.extend / \$.each / non-axios helper calls with {url, method} literals produce zero consumer contracts. Closes #828 * test(extractors): cover jQuery $.ajax with template-literal URL Extend the existing $.ajax fixture with `url: \`/api/orders/\${id}\`` and assert the consumer is emitted as http::GET::/api/orders/{param}. This makes jQuery + template-URL explicit rather than implicit via the axios object test (readStringProp already accepts template_string for both; this is coverage, not new behaviour). Addresses the single non-blocking finding on PR #887.	2026-04-16 18:36:24 +01:00
ivkond	4d4756fe86	feat(group): extractor expansion + manifest extractor (2/4 of #606 split) (#796) ... * feat(group): extractor expansion + manifest extractor Part 2 of 4 in the split of #606 (ticket: #792). Follows #795 (bridge.lbug storage foundation, already merged), but this PR has no code-level dependency on #795 — it only imports types and the ContractExtractor interface that existed on upstream main before either PR. It could have been reviewed in parallel with #795. ## What changed Expands the 3 existing contract extractors with substantially more language/framework coverage, and adds a new `manifest-extractor` that resolves `group.yaml`-declared cross-links against the per-repo graph via exact-name lookups. ### New file (228 LOC) - `gitnexus/src/core/group/extractors/manifest-extractor.ts` — exact graph lookup for `group.yaml`-declared cross-links. HTTP paths are canonicalized before Route.name matching; gRPC is resolved by service/method name (NO `.proto`-filename fallback); topic and lib use exact-name match. Falls back to a synthetic `manifest::<repo>::<contractId>` uid when the graph has no matching symbol, so cross-impact traversal still has a stable anchor for the contract. ### Modified extractors (+958 LOC prod) - `extractors/grpc-extractor.ts` (+522) — `.proto` parser with comment and string-literal sanitization (braces inside strings no longer truncate service bodies); package/service/method canonical IDs; server/client detection across Go (`grpc.NewServer`, `RegisterXxxServer`, `XxxGrpc.XxxImplBase`), Java (`@GrpcService`, `BlockingStub`), Python (`servicer_to_server`, `XxxStub`), and TypeScript/Node (`@GrpcMethod`, `ClientGrpc`, `loadPackageDefinition`). - `extractors/http-route-extractor.ts` (+174) — Go gin/echo/stdlib `HandleFunc`, NestJS `@Controller`+`@Get`/etc, Python FastAPI decorators, Java Spring `@RequestMapping`/`@GetMapping`, restTemplate / WebClient / OkHttp consumers. - `extractors/topic-extractor.ts` (+98) — sarama `ProducerMessage{}` struct literal detection (replaces a constructor-anchored regex that missed topics inside producer loops), kafka-go Writer/Reader, Python NATS (`await nc.subscribe`/`await nc.publish`), JetStream helpers. ### Modified and new tests (+1264 LOC) - `grpc-extractor.test.ts` (+539) — full coverage of the new proto parser (strings-with-braces regression, comments-with-braces regression), per-language server/client detection - `http-route-extractor.test.ts` (+240) — per-framework route extraction + normalization edge cases - `topic-extractor.test.ts` (+177) — the sarama in-loop regression, JetStream, Python NATS, kafka-go Writer/Reader - `manifest-extractor.test.ts` (+308 NEW) — HTTP path normalization, gRPC exact lookup with proto-fallback regression, lib and topic exact matching, synthetic-uid fallback behavior ### Self-review fixes folded in Carried forward from the #606 self-review (commit `d15b8cb`): - **HIGH #1** — `manifest-extractor.resolveSymbol` was too fuzzy. Previously used `CONTAINS` on route/name fields plus an unconditional `filePath ENDS WITH '.proto'` fallback for gRPC. Consequences: `/orders` matched `/suborders`, and any repo with any `.proto` file returned a random proto symbol for a gRPC manifest entry. Replaced with exact equality + deterministic `ORDER BY` + synthetic-uid fallback for unresolved manifests. Regression tests included. - **MED #3** — gRPC proto parser brace-depth counting now sanitizes strings and comments first (`stripProtoCommentsAndStrings`). A valid proto with `option deprecated_reason = "use NewService { instead"` used to have its service body closed early by the `"{"` inside the literal, silently dropping methods after the offending string. Regression tests for both string-with-brace and comment-with-brace cases. - **MED #4** — sarama Kafka regex changed from `sarama.NewSyncProducer[\s\S]{0,300}?Topic:` (anchored on constructor, caught only first topic in a loop) to `sarama.ProducerMessage{...Topic:}` (matches every struct literal directly). Regression test with a for-loop that constructs multiple `ProducerMessage`s. - **MED #7** — `manifest-extractor.resolveSymbol` no longer has a silent `catch { /* fall through */ }`. Errors from the graph executor are logged via `console.warn` with link type, contract name, repo key, and error message before falling through to the synthetic-uid path. ## Why Reviewer focus here is pure regex / parser correctness — no storage, no Cypher queries, no algorithmic changes to the cross-link algorithm. Separating this from the bridge foundation PR (#795) meant reviewers could stay in a single mental mode (parsing logic) instead of context-switching between DDL, Cypher, and regex. ## How to verify - `cd gitnexus && npx tsc --noEmit` - `cd gitnexus && npx vitest run test/unit/group/grpc-extractor.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/http-route-extractor.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/topic-extractor.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/manifest-extractor.test.ts --pool=forks` Local pre-push: typecheck clean, all 99 extractor unit tests pass (grpc 43, http 18, topic 30, manifest 8). ## Risk / rollback **Low.** Extractors have no user-facing surface in this PR — they produce `ExtractedContract[]` that is consumed by `sync.ts` in the next split (#793). No existing behavior changes for users who don't run a `group sync`. Rollback = `git revert` of the merge commit; the modifications to `grpc-extractor.ts` / `http-route-extractor.ts` / `topic-extractor.ts` revert to the pre-PR versions that still work (they're subsets of the new functionality). ## Scope discipline (per GUARDRAILS.md) - Only the 8 files above are touched; no drive-by refactors - No CI/release/security config changes - No secrets or machine-specific paths - Content lifted from #606 (CI 11/11 green on `d15b8cb`) ## Dependencies - **Base:** `main` (upstream already includes #795 as `1ff324c`) - **Blocks:** sync pipeline (#793) and the cross-impact feature (#794) - **Tracker issue:** #792 - **Parent PR:** #606 Co-authored-by: Claude <noreply@anthropic.com> * refactor(group): migrate topic-extractor from regex to tree-sitter queries Addresses @magyargergo's feedback on #796 that regex-based lookups should use tree-sitter nodes instead, and that the top-level extractors must NOT carry language dependencies. This is phase 1 of a multi-step migration — topic-extractor first because its patterns are the most uniform (16 "call/annotation with first-arg string literal" variants), which makes it a clean proof of the approach before grpc-extractor and http-route-extractor get the same treatment. ## Architecture: language-agnostic orchestrator + per-language plugins The top-level extractor is a thin orchestrator that never imports a tree-sitter grammar or a query string. Per-language knowledge lives in a new `topic-patterns/` folder with one file per language plus a registry that maps file extensions to compiled plugins: ``` src/core/group/extractors/ ├── tree-sitter-scanner.ts # shared, language-agnostic scanning utilities ├── topic-extractor.ts # thin orchestrator (no grammar imports) └── topic-patterns/ ├── types.ts # TopicMeta, Broker ├── index.ts # registry: extension → compiled provider ├── java.ts # tree-sitter-java + JAVA_TOPIC_PROVIDER ├── go.ts # tree-sitter-go + GO_TOPIC_PROVIDER ├── python.ts # tree-sitter-python + PYTHON_TOPIC_PROVIDER └── node.ts # tree-sitter-javascript + tree-sitter-typescript # → JAVASCRIPT_/TYPESCRIPT_/TSX_TOPIC_PROVIDER ``` **Shared scanner (`tree-sitter-scanner.ts`)** — defines `PatternSpec<TMeta>`, `LanguagePatterns<TMeta>`, `CompiledPatterns<TMeta>` and the `scanFile(parser, plugin, content)` helper. Plugins compile their queries eagerly at module load via `compilePatterns()`, so a broken pattern fails loudly at import time instead of silently at scan time. `unquoteLiteral()` handles single/double/template quotes, Python triple-quoted strings, and Go raw backtick strings. **Per-language plugins** own: - the tree-sitter grammar import (this is the ONLY place in `src/core/group/` where tree-sitter grammars are imported), - the query S-expressions, - the `TopicMeta` payload (role, broker, confidence, symbolName) that the orchestrator receives back on every match. Each plugin uses a `@value` capture name to bind the topic literal node. The JavaScript and TypeScript grammars share AST node names for every construct we query, so `node.ts` defines the pattern sources once and compiles them against `JavaScript`, `TypeScript.typescript`, and `TypeScript.tsx` — exporting three providers because `Parser.Query` objects are NOT portable across grammar instances. **Registry (`topic-patterns/index.ts`)** — maps `.java` → Java provider, `.go` → Go, `.py` → Python, `.js`/`.jsx` → JS, `.ts` → TS, `.tsx` → TSX. Also exports `TOPIC_SCAN_GLOB` so adding a new language is a single file-level edit (drop `topic-patterns/<lang>.ts`, import + register it here — zero edits required in `topic-extractor.ts`). **Orchestrator (`topic-extractor.ts`)** — ~110 lines, no grammar or query imports. Per file: `getProviderForFile(rel)` → `scanFile(parser, provider, content)` → `unquoteLiteral(valueText)` → `makeContract(...)`. Reuses one `Parser` instance across files; the scanner calls `setLanguage` per plugin. ## Why this is better than regex 1. **Comments and strings are respected for free.** The old regex would match `// kafkaTemplate.send("fake.topic")` as a real producer; tree-sitter never visits comments or string literals as code nodes, so false positives from commented-out code are eliminated. 2. **Struct/object literal patterns are structural, not textual.** `sarama.ProducerMessage{Topic: "..."}` no longer needs a 300-char lookahead (which was a known cross-match bug partly mitigated by a loop regression test in the self-review). The new query matches a specific `composite_literal` with a specific `qualified_type` and `keyed_element` — exactly one struct literal per match. 3. **No order-of-operations fragility.** Regex for `channel.publish` vs `channel.consume` was independent and file-wide; the AST scopes matches to the specific `call_expression`. 4. **Language-agnostic extension.** Adding Ruby, Rust, or C# topic detection later means dropping one file in `topic-patterns/` — no changes to shared scanner or orchestrator, and no tree-sitter imports leak into top-level code. ## Per-file fault tolerance - Malformed files that tree-sitter can't parse are silently skipped (`parser.parse` is wrapped by `scanFile`). The ingestion pipeline already logs unparseable files at index time. - A syntactically invalid query is caught at `compilePatterns` time, not scan time — broken plugins fail loudly at import. - Per-pattern `matches()` failures are swallowed so one broken query in a plugin doesn't block the rest. ## Tests All 30 existing `topic-extractor.test.ts` tests pass **without any changes to the test file** — they were written as input/output contract tests (given this source file, expect these `ExtractedContract` objects) and that contract is unchanged. Regression coverage includes: - Kafka: Java `@KafkaListener` + `kafkaTemplate.send`; Node `producer.send` + `consumer.subscribe`; Go sarama producer/consumer (sync and async); kafka-go Writer/Reader; Python `KafkaConsumer` + `producer.send/produce` - RabbitMQ: Java `@RabbitListener` + `rabbitTemplate.convertAndSend`; Node `channel.consume/publish/sendToQueue`; Python `basic_consume/ basic_publish` with keyword args - NATS: Go and Node `nc.Subscribe/Publish`; Go and Node JetStream `js.Subscribe/Publish`; Python `await nc.subscribe/publish` Including the regression test for the sarama `ProducerMessage` in-loop case — the AST-based query captures every literal in the file independently, not just the first one after `NewSyncProducer`. ## Neighbor regression check - `topic-extractor.test.ts` — 30/30 pass (rewritten extractor) - `http-route-extractor.test.ts` — 18/18 pass (untouched) - `grpc-extractor.test.ts` — 43/43 pass (untouched) - `manifest-extractor.test.ts` — 8/8 pass (untouched) - Full `npx tsc --noEmit` clean ## Scope discipline (per GUARDRAILS.md) - Only files under `src/core/group/extractors/` are touched; no changes to other extractors, tests, MCP surface, or pipeline.ts. - No CI/release/security config changes, no secrets. - New tree-sitter imports all reference grammars that are already installed as dependencies (`tree-sitter`, `tree-sitter-javascript`, `tree-sitter-typescript`, `tree-sitter-python`, `tree-sitter-java`, `tree-sitter-go` — all in `package.json` for the existing pipeline). ## Phase 2 / phase 3 plan - **Phase 2 (next commit):** rewrite `http-route-extractor.ts` Strategy B (regex fallback) on the same plugin pattern. Graph-assisted Strategy A stays as-is (already uses pipeline-built tree-sitter data via `HANDLES_ROUTE` Cypher queries). - **Phase 3 (commit after):** rewrite `grpc-extractor.ts` for Java / Go / Python / TypeScript detection. `.proto` files are the one outstanding question — there is no `tree-sitter-proto` grammar installed; the in-tree string-sanitizing parser stays as a pragmatic exception with a comment, alternative being to add `tree-sitter-proto` as a dep (open for the maintainer). Co-authored-by: Claude <noreply@anthropic.com> * refactor(group): migrate http-route-extractor Strategy B to tree-sitter plugins Phase 2 of the extractor refactor requested by @magyargergo on #796. Same architecture as the phase 1 topic-extractor rewrite: a thin, language-agnostic orchestrator plus per-language plugins that own tree-sitter grammars and query sources. The top-level extractor file no longer imports any tree-sitter grammar or query string. ## Architecture ``` src/core/group/extractors/ ├── tree-sitter-scanner.ts # shared, language-agnostic primitives ├── http-route-extractor.ts # thin orchestrator (no grammar imports) └── http-patterns/ ├── types.ts # HttpDetection, HttpLanguagePlugin, HttpRole ├── index.ts # registry: ext → plugin + HTTP_SCAN_GLOB ├── java.ts # tree-sitter-java: Spring + RestTemplate/WebClient/OkHttp ├── go.ts # tree-sitter-go: gin/echo/HandleFunc + http/resty consumers ├── python.ts # tree-sitter-python: FastAPI + requests ├── php.ts # tree-sitter-php: Laravel Route::get/... └── node.ts # tree-sitter-javascript + tree-sitter-typescript: # NestJS controllers, Express, fetch, axios ``` **Shared scanner (`tree-sitter-scanner.ts`)** — generalised from phase 1: - `ScanMatch<TMeta>.captures` is now a full `CaptureMap` (every named capture the query binds, not just a single `@value`). Topic extractor updated to read `match.captures.value` accordingly. - New `runCompiledPatterns(plugin, tree)` helper lets plugins run multiple query bundles against the same pre-parsed tree. This is needed for HTTP plugins that combine a class-prefix query with a method-route query (Spring, NestJS). - `scanFile` becomes a thin wrapper over `parser.parse + runCompiledPatterns`. **HTTP plugin shape** — unlike topic plugins, HTTP plugins expose a `scan(tree)` function rather than a flat pattern list. This reflects HTTP's more complex extraction: each detection needs method + path + handler name, and framework patterns like Spring `@RequestMapping` / NestJS `@Controller` require cross-referencing a class-level prefix with method-level annotations. Plugins internally use `compilePatterns` + `runCompiledPatterns` and walk the AST to resolve the class/method relationships. **Per-framework coverage:** - **Java (`java.ts`)** - Spring: `@RequestMapping("/api/v2")` class prefix + `@(Get|Post|Put| Delete|Patch)Mapping("/sub")` method routes, joined via the enclosing `class_declaration` node id. - `RestTemplate.getForObject/postForEntity/put/delete/patchForObject` → method derived from API name. - `WebClient.method(HttpMethod.X, "/path")` → method from `HttpMethod.X` capture. - `new Request.Builder().url("/path")` → OkHttp consumer. - **Go (`go.ts`)** - gin / echo / chi frameworks: `\w+.GET("/path", handler)` captures upper-case verb + handler identifier. - `net/http.HandleFunc("/path", handler)` → provider (default GET). - `http.Get/Post/Head` consumer, `http.NewRequest("METHOD", ...)`, resty `client.R().Get/Post/...`. - **Python (`python.ts`)** - `@app.get("/path")` FastAPI decorators. - `requests.get/post/...` and `requests.request("METHOD", "url")`. - **PHP (`php.ts`)** - Laravel `Route::get/post/.../patch('/path', ...)` via `scoped_call_expression`. Uses `PHP.php_only` to match the existing ingestion pipeline's grammar selection. - **Node (`node.ts`) — JS + TS + TSX** - Pattern sources defined once, compiled against three grammar variants (`JavaScript`, `TypeScript.typescript`, `TypeScript.tsx`) because `Parser.Query` objects are not portable across grammars. Exports three plugins sharing the same `scan` logic. - NestJS: `@Controller('prefix')` decorators are siblings of the class in `export_statement` / `program`; `@Get(':id')` decorators are siblings of the method in `class_body`. The plugin walks decorator → next named sibling to find the decorated class / method, then combines the class prefix with the method path. Only emits NestJS detections when the enclosing class has a real `@Controller` decorator — prevents false positives from generic classes that happen to use `@Get` from another library. - Express: `(router|app).<verb>('/path', ...)`. - `fetch(url)` (default GET) + `fetch(url, { method: 'X' })` (uses two queries + a SyntaxNode-id dedupe set so URL literals aren't double-emitted by the options variant). - `axios.get/post/...`. ## Orchestrator changes `http-route-extractor.ts` drops every `scanXxxProviders` / `scanXxxConsumers` regex method and replaces them with a single source-scan loop that delegates to `getPluginForFile(rel).scan(tree)`. The orchestrator still owns: - **Path normalization** (`normalizeHttpPath`, `normalizeConsumerPath`) — language-agnostic string processing shared by both strategies. - **Graph-assisted Strategy A** (`HANDLES_ROUTE` / `FETCHES` / `CONTAINS` Cypher queries) — unchanged in spirit. The only regex helpers it used (`inferMethodFromFileScan`, `pickJavaHandlerName`) are now replaced by a lookup against the plugin's detections for the same file: for each route row, find the detection whose normalized path matches, and pull the HTTP method + handler name from it. - **Per-file parse cache** — the orchestrator parses each relevant file at most once per `extract()` call. Both the graph-assisted enrichment loop and the source-scan fallback share the same `cachedDetections` map, so we never run the plugin twice for the same file. ## Why this is better than the regex version 1. **Comments and strings for free.** The old regex would match `// router.get('/fake')` as a real Express route; tree-sitter never visits string/comment nodes. 2. **Structural controller-prefix.** Spring and NestJS class-prefix joining is now scoped to the enclosing class via `class_declaration` node ids, eliminating file-wide state that broke when a file had multiple controllers. 3. **Precise NestJS disambiguation.** The plugin only emits a NestJS detection when the enclosing class has a real `@Controller` decorator — the old regex would fire on any `@Get(...)` in the file regardless of surrounding context. 4. **Language-agnostic extension.** Adding Ruby / Rust / Kotlin HTTP detection later means dropping one file in `http-patterns/` — no changes to the shared scanner, the orchestrator, or the Strategy A Cypher queries. ## Tests - `http-route-extractor.test.ts` — **18/18 pass** (tests unchanged; they're contract-style input/output tests and the contract shape is unchanged). Covers Spring class prefix, Express, gin/echo, stdlib HandleFunc, NestJS, Laravel, FastAPI for providers and fetch/axios/python-requests/rest-template/webClient/okhttp/go-stdlib/ resty for consumers, plus graph-first Strategy A for both. - `topic-extractor.test.ts` — **30/30 pass** after the `captures.value` API migration. - `grpc-extractor.test.ts` — 43/43 pass (untouched; phase 3). - `manifest-extractor.test.ts` — 8/8 pass (untouched). - `service.test.ts`, `sync.test.ts`, `storage.test.ts` — 41/41 pass. - `npx tsc -p tsconfig.json --noEmit` clean. ## Scope discipline (per GUARDRAILS.md) - Only files under `src/core/group/extractors/` are touched. - No changes to pipeline.ts, MCP surface, ingestion, or tests. - No CI / release / security / secrets changes. - Tree-sitter grammars imported by plugins (`tree-sitter-java`, `tree-sitter-go`, `tree-sitter-python`, `tree-sitter-php`, `tree-sitter-javascript`, `tree-sitter-typescript`) are all already in `package.json` for the existing ingestion pipeline. ## Phase 3 plan - **grpc-extractor** gets the same treatment: plugin-per-language under `grpc-patterns/` for Java / Go / Python / TS detection. `.proto` files remain an open question — no `tree-sitter-proto` grammar is installed, so the in-tree string-sanitizing parser from PR #796's self-review stays as a pragmatic exception unless the maintainer wants us to add `tree-sitter-proto` as a new dep. Co-authored-by: Claude <noreply@anthropic.com> * refactor(group): migrate grpc-extractor source scans to tree-sitter plugins Phase 3 (final) of the extractor refactor requested by @magyargergo on #796. Same architecture as phase 1 (topic) and phase 2 (http): thin language-agnostic orchestrator + per-language plugins that own tree-sitter grammars and query sources. With this commit the top-level extractors under `src/core/group/extractors/` import ZERO tree-sitter grammars and ZERO query strings — every grammar import lives in a `*-patterns/<lang>.ts` plugin file, and the orchestrators go through the registry indirection. ## Architecture ``` src/core/group/extractors/ ├── tree-sitter-scanner.ts # shared primitives (unchanged) ├── grpc-extractor.ts # orchestrator (only `.proto` parser left) └── grpc-patterns/ ├── types.ts # GrpcDetection, GrpcLanguagePlugin, GrpcRole ├── index.ts # registry: ext → plugin + GRPC_SCAN_GLOB ├── go.ts # tree-sitter-go: RegisterXxxServer, Unimplemented, NewXxxClient ├── java.ts # tree-sitter-java: @GrpcService + XxxImplBase + newBlockingStub ├── python.ts # tree-sitter-python: add_XxxServicer_to_server + XxxStub └── node.ts # tree-sitter-javascript + tree-sitter-typescript: # @GrpcMethod, @GrpcClient field type, # .getService<X>('Svc'), new XxxServiceClient, # loadPackageDefinition dynamic constructors ``` ## Per-language coverage **Go (`go.ts`)** - Provider: `\w+.RegisterXxxServer(...)` via `call_expression → selector_expression → field_identifier` + JS regex filter `^Register(\w+)Server$`. - Provider: `pb.UnimplementedXxxServer` embedded in a struct via `struct_type → field_declaration_list → field_declaration → qualified_type → type_identifier` + JS filter. - Consumer: `\w+.NewXxxClient(...)` via the same call_expression query + JS filter `^New(\w+)Client$`. **Java (`java.ts`)** - Provider: `class X extends YyyGrpc.YyyImplBase` — two queries handle the scoped and plain forms. `scoped_type_identifier`'s children are positional (no `scope:`/`name:` fields), so the query matches the two `type_identifier` children by position. - `#match? @inner "ImplBase$"` restricts matches at query time. - Whether the class has `@GrpcService` or not controls only the `source` metadata label — the plugin walks the class_declaration's `modifiers` child in JS to detect the marker_annotation. - Consumer: `YyyGrpc.newStub(ch)` / `newBlockingStub(ch)` via a `method_invocation` query with `#match? @method "^new(Blocking)?Stub$"`, service name extracted via `^(\w+)Grpc$` on the object identifier. **Python (`python.ts`)** - Single call-expression query covers both bare identifier and `obj.method` attribute forms: `(call function: [(identifier) @fn (attribute attribute: (identifier) @fn)])`. - Plugin filters `@fn.text` against two JS regexes: `^add_(\w+)Servicer_to_server$` (provider) and `^(\w+)Stub$` (consumer), with a reserved-names ignore list for the Stub case (Mock / Test / Fake / Stub). **Node — JavaScript + TypeScript + TSX (`node.ts`)** - Pattern sources defined once, compiled three times (one per grammar) because `Parser.Query` objects are not portable across grammars. Exports three `GrpcLanguagePlugin`s sharing the same `scan`. - `@GrpcMethod('Service', 'Method')`: decorator query captures the two string literals. Confidence is hard-coded 0.8 regardless of proto map resolution (matches the original regex version's behaviour). - `@GrpcClient(...) field: XxxServiceClient`: decorator query captures the decorator node, plugin walks up to find the enclosing `public_field_definition` (decorators on fields are CHILDREN of the field definition in tree-sitter-typescript, not siblings) and reads its first `type_annotation → type_identifier`, then runs the `^(\w+Service)Client$` JS filter. - `client.getService<X>('AuthService')`: call-expression query on `member_expression.property = "getService"` + string literal arg. - `new XxxServiceClient(...)`: `new_expression` with a bare identifier constructor, filtered by `^(\w+Service)Client$` so generic `new AuthClient(...)` (missing the `Service` infix) does NOT falsely register as a consumer. Preserves the regression test `test_extract_ts_non_service_client_constructor_is_ignored`. - `loadPackageDefinition` dynamic loader: gated on `tree.rootNode.text.includes('loadPackageDefinition')`. When set, `new foo.bar.Xxx(...)` qualified constructors with a capitalised property name register as consumers. ## Orchestrator changes `grpc-extractor.ts` loses every `scanGoProviders` / `scanJavaProviders` / ... helper and replaces them with a single source-scan loop that: 1. Parses each file with the plugin's grammar (one shared `Parser` instance across all files, `setLanguage` called per plugin). 2. Calls `plugin.scan(tree)` to get `GrpcDetection[]`. 3. Converts each detection to an `ExtractedContract` via the private `detectionToContract` helper, which: - Looks the short service name up in the proto map (filled by the `.proto` parser). - Picks confidence = `confidenceWithProto` if resolved, else `confidenceWithoutProto`. - Builds a method-level contract id (`grpc::pkg.Svc/Method`) when the detection carries a `methodName` (TS `@GrpcMethod` only), otherwise a service-level id (`grpc::pkg.Svc/*`). Everything else — the `.proto` parser, `buildProtoContext`, `buildProtoMap`, `resolveProtoConflict`, `serviceContractId`, `stripProtoCommentsAndStrings`, `extractServiceBlocks`, the dedupe function — stays exactly as before. The `.proto` parser is kept as a pragmatic exception to the "no regex in extractors" rule because no `tree-sitter-proto` grammar is installed in the repo; a comment at the top of the file explains this and flags the maintainer option of adding `tree-sitter-proto` as a dependency. ## Why this is better than the regex version 1. **Comments and strings are respected for free.** Matched node types are only code constructs, never text inside comments or string literals. 2. **No false positives on partial names.** The old `(\w+?)Grpc`-style regexes would cross-match unrelated identifiers; structural queries restrict matches to the exact AST shape (`scoped_type_identifier → type_identifier` pairs, `method_invocation → identifier` etc.). 3. **NestJS `@GrpcClient` is structural, not regex-based.** The old regex required a specific textual layout (`@GrpcClient(...) private readonly foo!: XxxServiceClient`); the plugin now walks the AST, so modifier order / optional modifiers / multi-line formatting don't break it. 4. **Language-agnostic extension.** Adding Kotlin / Rust / C# gRPC detection later is a one-file edit in `grpc-patterns/index.ts` — no touches to the shared scanner, the orchestrator, or the proto parser. ## Tests - `grpc-extractor.test.ts` — **43/43 pass** (tests unchanged; the contract shape is identical). Covers .proto parsing (including the brace-inside-string regression), Go provider/consumer, Java @GrpcService / plain ImplBase provider + newBlockingStub consumer, Python servicer + stub, TS @GrpcMethod + @GrpcClient + .getService + new XxxServiceClient + loadPackageDefinition + the `AuthClient` vs `AuthServiceClient` discrimination, dedupe across multiple patterns in one file, proto-aware confidence, and the inherited-package resolution for split proto definitions. - `topic-extractor.test.ts` — 30/30 pass. - `http-route-extractor.test.ts` — 18/18 pass. - `manifest-extractor.test.ts` — 8/8 pass. - `service.test.ts`, `sync.test.ts`, `storage.test.ts` — 41/41 pass. - `npx tsc -p tsconfig.json --noEmit` clean. ## Scope discipline (per GUARDRAILS.md) - Only files under `src/core/group/extractors/` are touched. - No pipeline.ts, MCP surface, ingestion, CI / release / security, or test changes. - New tree-sitter grammar imports (`tree-sitter-go`, `tree-sitter-java`, `tree-sitter-python`, `tree-sitter-javascript`, `tree-sitter-typescript`) are all already installed for the ingestion pipeline. ## End of phase series This commit completes the three-phase extractor refactor: - **Phase 1** (`ea06d11`): topic-extractor → `topic-patterns/` - **Phase 2** (`b6015f6`): http-route-extractor → `http-patterns/` - **Phase 3** (this commit): grpc-extractor → `grpc-patterns/` Every remaining regex-based extractor helper under the `src/core/group/ extractors/` directory is either (a) language-agnostic string processing (path normalization, dedupe keys) or (b) the `.proto` parser, which is documented as an explicit exception. Co-authored-by: Claude <noreply@anthropic.com> * feat(group): add tree-sitter-proto for .proto file parsing Addresses @magyargergo's suggestion on #796 to replace the manual string-sanitizing .proto parser with a tree-sitter grammar. - **Vendored `tree-sitter-proto`** in `vendor/tree-sitter-proto/`. Grammar source from [coder3101/tree-sitter-proto](https://github.com/coder3101/tree-sitter-proto) (latest `grammar.js`), parser.c regenerated with `tree-sitter-cli 0.24` to produce ABI version 14 — compatible with the project's `tree-sitter 0.25` runtime (which supports ABI ≤ 14). Added as `optionalDependency` with `file:./vendor/tree-sitter-proto`. - **New `grpc-patterns/proto.ts` plugin** — uses the same `compilePatterns` + `runCompiledPatterns` infrastructure as every other plugin. Two queries: - `(package (full_ident) @pkg)` — package declaration - `(service (service_name) @service_name (rpc (rpc_name) @rpc_name))` — one match per (service, rpc) pair - **Graceful fallback** — `tree-sitter-proto` is an optional dependency. If it fails to install (platform incompatibility) or fails the runtime smoke-test (`setLanguage` + `parse` on a trivial proto), `PROTO_GRPC_PLUGIN` stays `null` and the orchestrator uses the existing manual parser. The smoke-test catches the `SyntaxNode` TDZ error that occurs in vitest's fork-based test runner. - **Orchestrator updated** — when `hasProtoPlugin` is true, `.proto` files are handled by the plugin loop (they're included in `GRPC_SCAN_GLOB`), and the manual `parseProtoFile` loop is skipped. `buildProtoContext` still runs to build the proto map for cross-referencing source-file detections. 1. **No manual comment/string stripping.** The old parser needed `stripProtoCommentsAndStrings` (110 lines) to avoid counting braces inside comments and string literals. tree-sitter handles this natively. 2. **No brace-depth tracking.** `extractServiceBlocks` used a manual depth counter to find service boundaries. tree-sitter's AST gives us `service` → `service_name` + `rpc` → `rpc_name` directly. 3. **Performance.** tree-sitter's C-based parser is faster than character-by-character JS scanning + regex on large proto files. - `grpc-extractor.test.ts` — **43/43 pass** (unchanged) - All other extractor tests — 99/99 pass - `npx tsc -p tsconfig.json --noEmit` clean Co-authored-by: Claude <noreply@anthropic.com> * chore: add .gitignore for vendored tree-sitter-proto build artifacts https://claude.ai/code/session_01SFUCxgKMMQ8EgRHYw91xPU * fix: correct .gitignore paths for vendored tree-sitter-proto Patterns should be relative to the .gitignore file's directory. https://claude.ai/code/session_01SFUCxgKMMQ8EgRHYw91xPU * refactor(group): address Copilot review feedback on #796 Six fixes suggested by the Copilot AI review: 1. **`normalizeHttpPath` root-path edge case** — stripping trailing slashes on the input `/` produced an empty string, yielding malformed contract ids like `http::GET::`. Now preserves `/` for the root handler/fetch case. 2. **Dedupe `scanFiles` call** — `extract()` was globbing the source-scan file list twice (once for the provider fallback, once for the consumer fallback). Moved to a single lazy call that memoizes the result for the rest of the method. 3. **HTTP `scanFiles` now ignores `**/vendor/**`** — every other extractor's glob already ignored vendored sources; the HTTP one didn't. Fixed for consistency. 4. **`loadPackageDefinition` check is now structural** — was calling `tree.rootNode.text.includes('loadPackageDefinition')` which forces materialization of the entire file text from the parse tree (expensive on large files). Replaced with a dedicated compiled query on `(call_expression function: [(identifier) | (member_expression)])` so the check stays in the AST domain. 5. **`grpc-extractor.ts` header docstring updated** — still claimed ".proto parsing is not tree-sitter-based because no grammar is installed". Now describes the actual behaviour: tree-sitter when `tree-sitter-proto` is available (optionalDependency), manual fallback otherwise. 6. **Eliminated the double proto file parse on the fallback path** — `buildProtoContext` already globs + parses every `.proto` file to build `servicesByName`. On the `!hasProtoPlugin` branch the extractor was globbing + parsing again via the now-removed `parseProtoFile` helper. The fallback branch now iterates the map that `buildProtoContext` already produced to emit provider contracts directly — single pass per proto file. ## Tests - `topic-extractor.test.ts` — 30/30 pass - `http-route-extractor.test.ts` — 18/18 pass - `grpc-extractor.test.ts` — 43/43 pass - `manifest-extractor.test.ts` — 8/8 pass - `npx tsc -p tsconfig.json --noEmit` clean Co-authored-by: Claude <noreply@anthropic.com> * refactor(group): address Claude review feedback (bugs + dedup + hygiene) on #796 Follows up `2f28bfc` with the remaining items from the Claude AI review: ## Bugs **Bug 2 — Label-unaware Cypher queries in `resolveSymbol`.** The manifest-extractor's lookup queries were `MATCH (n) WHERE n.name = $x` with no label filter, so a topic/service/package name could silently match any node type (File, Variable, Import, Folder, …). Added label filters: - `topic` → `(n:Function|Method|Class|Interface)` (topics are best-effort symbol-name matches against listener/publisher symbols) - `grpc` method → `(n:Function|Method)` - `grpc` service → `(n:Class|Interface)` - `lib` → `(n:Package|Module)` All 8 manifest-extractor tests still pass (mock executor is label-agnostic, but the production LadybugDB graph now gets correctly scoped queries). **Bug 8 — Tautological `!handlerName` condition.** `http-route-extractor.ts:extractProvidersGraph` had `let handlerName = null; if (!method || !handlerName) { ... }` — the `!handlerName` clause was always true since there was no intervening assignment. Simplified to always run the plugin-scan lookup (we need the handler name even when `methodFromRouteReason` already resolved the method). ## Clean code / dedup **Design 7 — `readSafe` was copy-pasted in all three orchestrators.** Extracted to `extractors/fs-utils.ts` as the single source of truth for the path-traversal guard. Dropped the three local copies and the now-unused `fs`/`path` imports from topic-extractor. **Style 10 — Language-specific `_test.go` skip in the topic orchestrator.** Was `if (rel.endsWith('_test.go')) continue;` inside the language- agnostic extraction loop. Pushed into the glob's ignore list (`'**/*_test.go'`) alongside the existing `node_modules`, `vendor`, `dist`, `build` entries, with a comment explaining that other languages' test file conventions either live in separate directories (Python `tests/`, Java `src/test/`) or are already covered by the existing ignores. ## Already addressed in `2f28bfc` (mentioned again in Claude review) - Bug 3: `normalizeHttpPath('/')` returns `''` — fixed - Bug 4: double glob + double parse of `.proto` — fixed - Bug 5: `scanFiles` called twice in HTTP — fixed - Bug 6: missing `**/vendor/**` in HTTP glob — fixed - Design 9 partially: `tree.rootNode.text.includes('loadPackageDefinition')` replaced with a dedicated structural query ## Deferred - Bug 1 (`http::*::path` vs `http::GET::path` matching) — out of scope; sync.ts matching logic lands in #793, manifest extractor already emits correct synthetic uids for unresolved HTTP contracts. - Design 9 full (change plugin `scan(tree)` → `scan(tree, source)`) — the only real use case (`loadPackageDefinition` gate) is already fixed via a structural query, so the interface change would be cosmetic churn without a concrete consumer. ## Tests - `topic-extractor.test.ts` — 30/30 pass - `http-route-extractor.test.ts` — 18/18 pass - `grpc-extractor.test.ts` — 43/43 pass - `manifest-extractor.test.ts` — 8/8 pass - `npx tsc -p tsconfig.json --noEmit` clean Co-authored-by: Claude <noreply@anthropic.com> * docs+fix(group): address remaining Claude review items + add pipeline flow chart ## Fixes **Remaining 🔴 — HTTP contract id wildcard format.** Documented the `http::*::<path>` format as an intentional wildcard for manifest links that omit the HTTP method, alongside the explicit-method form (`GET::/path` → `http::GET::/path`). The docblock on `buildContractId` now states both forms, notes that wildcard-aware matching is the responsibility of the sync / cross-impact layer (#793), and recommends the explicit-method form whenever the author knows the method (it round-trips through exact equality without needing wildcard logic downstream). Tests unchanged — the wildcard format is what they've always asserted. **Minor 1 — stale comment at `manifest-extractor.ts:124-126`.** The comment claimed "creates a contract with an empty symbolUid/ref" but the code switched to `manifestSymbolUid(repo, contractId)` a few commits back. Updated to describe the actual synthetic-uid fallback semantics and the cross-impact path that relies on both sides of the join deriving the same uid. **Minor 2 — exhaustiveness guard on `buildContractId`.** The `switch(type)` covered all five current `ContractType` variants but silently returned `undefined` if a new variant was added. Added a `default: const _exhaustive: never = type; throw new Error(...)` clause so the build fails loudly on an unhandled variant. **Minor 3 — `tree.rootNode.text` in `grpc-patterns/node.ts`.** Already fixed in `2f28bfc` via a dedicated structural query (`LOAD_PACKAGE_DEFINITION_SPEC`). No action needed. ## New: pipeline flow chart (per @magyargergo's request) Added `src/core/group/PIPELINE.md` with four Mermaid diagrams: 1. **High-level overview** — `group.yaml` → extractors + manifest → contract matching → `bridge.lbug` → `runGroupImpact`. 2. **Per-repo extractor two-strategy shape** — graph-assisted Strategy A vs. source-scan Strategy B. 3. **Plugin architecture** — orchestrator → registry → per-language `*-patterns/<lang>.ts` → `tree-sitter-scanner.ts` → `ExtractedContract`. 4. **Manifest extraction** — label-scoped `resolveSymbol` with the synthetic-uid fallback. 5. **Cross-impact query (#606)** — local impact → bridge join → cross-repo fan-out. Each diagram is annotated with which PRs own which stage (this PR: extractors + manifest; #795: bridge storage; #606: cross-impact runtime) and points at the concrete files/functions involved. ## Tests - 99/99 extractor tests pass - `npx tsc -p tsconfig.json --noEmit` clean Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-04-13 08:49:30 +01:00
Mr. WorldwideBrown	08541e2857	Fix HTTP client vs Express route detection and Spring interface attribution (#780) ... * fix: correctly identify HTTP client calls vs Express routes in receiver extraction * fix: skip Spring route extraction for Feign client interfaces * fix: address review feedback - receiver walk edge case, regex anchoring, add tests * style: fix prettier formatting in route extractor and test files	2026-04-11 11:24:47 +01:00
ivkond	4fa395f4b6	feat(group): add service boundary detection and contract extractors ... Service communication detection for microservice monorepos: - ServiceBoundaryDetector: auto-detects service boundaries via markers (package.json, go.mod, Dockerfile, pom.xml, Cargo.toml, build.gradle, pyproject.toml, etc.) - HttpRouteExtractor: graph-assisted (Strategy A) with source-scan fallback (Strategy B) for Spring, Express, Laravel, FastAPI providers and fetch/axios consumers - GrpcExtractor: parses .proto files, detects Go/Java/Python/TS gRPC servers (RegisterXxxServer, @GrpcService, add_XxxServicer_to_server, @GrpcMethod) and clients (NewXxxClient, newBlockingStub, XxxStub) - TopicExtractor: Kafka (@KafkaListener, producer.send), RabbitMQ (@RabbitListener, channel.publish/consume), NATS (nc.Subscribe/Publish) across Java, Node, Go, and Python Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-02 00:40:12 +03:00