mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-22 00:31:17 +00:00
13 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
ivkond
|
0909a908ee
|
fix(group): bubble local-impact phase errors in groupImpact (#1004) (#1007)
When the Phase 1 local-impact leg returned a structured { error: ... }
payload (missing symbol, graph-load failure, or an exception wrapped by
safeLocalImpact), runGroupImpact previously buried it inside a zero-hit
GroupImpactResult with empty cross / outOfScope arrays and risk 'UNKNOWN'.
Callers branch on top-level `error` (CLI, MCP wrapper), so the failure
path surfaced as a silent "no impact across the group" — a false
negative on a safety-critical blast-radius tool.
Fail closed: bubble the error as a top-level { error } prefixed with the
repoPath, matching how runGroupImpact already handles resolveGroupRepo,
config-load, and bridgePrep failures. Chose option 1 (bubble the error)
over option 2 (partial-result discriminant) because runGroupImpact only
runs local impact for a single member repo at this point — cross-repo
fan-out happens later via the bridge, so there is no partial success
data to preserve on the local-phase failure path.
Added two regression tests covering both the port-returned { error }
case and the thrown-exception case (wrapped by safeLocalImpact).
Made-with: Cursor
|
||
|
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> |
||
|
ivkond
|
00966630c4
|
feat: cross-repo impact analysis (#794) — @repo MCP routing + group resources (#984) | ||
|
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.
|
||
|
Jonas Vanderhaegen
|
385ee037bd
|
[group/sync] Fix ManifestExtractor never called — config.links always produced 0 cross-links (#827)
* fix(group/sync): wire ManifestExtractor into syncGroup pipeline ManifestExtractor was fully implemented in extractors/manifest-extractor.ts but never imported or called in sync.ts. As a result, any links declared in group.yaml were parsed and validated by config-parser.ts but silently dropped — config.links was always an empty dead-end as far as syncGroup was concerned. Changes: - Import ManifestExtractor in sync.ts - Call extractFromManifest(config.links, dbExecutors) inside the outer try block, after all repos are processed but before the finally closes the DB pools (symbol resolution via resolveSymbol requires open executors) - Collect the resulting contracts into autoContracts and the cross-links into a separate manifestCrossLinks array - Merge manifestCrossLinks into the final crossLinks alongside runExactMatch results Without this fix, users who declare explicit service dependencies in group.yaml links (the documented workaround for HTTP clients that use absolute URLs and are invisible to the auto-extractors) get 0 cross-links regardless of what they configure. * test(group/sync): cover manifest links producing cross-links Add a unit test that asserts config.links entries produce contract pairs and a manifest cross-link (matchType: 'manifest') via syncGroup. Also refactors the manifest extraction call to sit outside the else/try block so it runs regardless of extractorOverride arity — makes the code testable without mocked DB pools and ensures links work when callers supply a zero-arity override (e.g. in tests or programmatic usage). * style: prettier format sync.ts and sync.test.ts Also removes the stray empty line in the finally block (noted in review). * fix(group/sync): dedupe cross-links and warn on dangling manifest repos Addresses review feedback on PR #827: 1. Dedupe cross-links. Manifest contracts participate in runExactMatch, so a manifest-declared link also emitted a duplicate matchType:'exact' CrossLink for the same endpoint pair. Dedupe by (from, to, type, contractId) and prefer manifest (operator-declared intent). 2. Warn on dangling repos. When a manifest link references a repo not in config.repos, log a warning. Synthetic UIDs keep the cross-link deterministic, but the operator probably meant something else. 3. Tests: - Assert no duplicate 'exact' CrossLink is emitted alongside the manifest one. - Assert synthetic UID format when no DB executors are available. - New test: dangling manifest repo still produces a cross-link + logs a warning. * perf(group/manifest): parallelize and memoize symbol resolution Previous implementation ran 2N sequential Cypher round-trips per manifest (one for provider side, one for consumer, awaited in-order per link). For manifests with tens of links this dominated syncGroup latency in groups with many declared cross-repo contracts. Changes: - Resolve provider + consumer in parallel per link (Promise.all). - Resolve all links in parallel (outer Promise.all over links.map). Each repo's executor pool is independent, so cross-repo fan-out scales with the number of distinct repos in the manifest. - Memoize by (repo, type, contract). Manifests frequently declare the same contract from both directions or across sibling groups, so duplicate triples now hit the DB once instead of 2× per link. Correctness: - resolveSymbol is a pure LIMIT 1 read, so caching + concurrent invocation is safe. - Iteration order over links is preserved in the final contracts / crossLinks arrays — result shape is identical. Test: - New test asserts that two links sharing (repo, type, contract) produce exactly one DB call per distinct repo-tuple. --------- Co-authored-by: jonasvanderhaegen-xve <> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> |
||
|
Gergő Magyar
|
759c983dce
|
fix(extractors): resolve 3 silent contract mis-resolution bugs (#793) (#817)
* fix(extractors): resolve 3 silent contract mis-resolution bugs (#793) Addresses Codex adversarial review findings for extractor contract resolution on the new group extractor surface. F1 (manifest-extractor): resolveSymbol passed the full "METHOD::path" contract string through normalizeRoutePath, producing "/GET::/api/orders" which never matches Route.name. Adds parseHttpContract() helper that strips the METHOD:: prefix before path normalization. Contract ID construction (buildContractId) is unchanged. F2 (http-route-extractor): graph-assisted backfill used path-only detections.find(), so multi-verb same-URL files attached the wrong verb/handler to provider rows and inferred the wrong verb on FETCHES consumer edges. Now requires path+method match when method is known, and skips backfill when method is unknown and multiple detections tie on path. F3 (grpc-extractor): resolveProtoConflict seeded bestScore=-1 and only replaced on strict >, so all-zero-score ties silently selected candidates[0]. Now computes all scores, counts ties at the top score, and returns null on ambiguity (caller skips contract emission and warns with service name + candidate paths). All three fixes are test-first; 73 tests pass across the three suites. No schema changes, no new dependencies, contract ID wire format (http::METHOD::path, grpc::pkg.Service/Method, http::*::path) preserved. * fix(extractors): address PR #817 review — ambiguous symbol pick + contract id casing Copilot + Claude review on PR #817 flagged two follow-up bugs on top of the F1/F2/F3 fixes: 1. http-route-extractor: ambiguous multi-verb case left handlerName null but still ran the CONTAINS DB query. pickSymbolUid(syms, null) then silently picked pool[0] — reintroducing handler mis-attribution via a different route than the .find() bug F2 fixed. Now gates symbol enrichment on an ambiguousCandidates flag so the file-basename fallback wins instead. 2. manifest-extractor: buildContractId passed raw user casing through for the explicit-method form, so get::/api/orders and GET::/api/orders produced different contract ids even though parseHttpContract upper-cases during lookup. Now reuses parseHttpContract + normalizeRoutePath to canonicalize both method and path, so logically equivalent manifest inputs share a contract id (and share a manifestSymbolUid fallback). Adds one regression test per bug: lowercase vs uppercase manifest contract ids must match, and ambiguous multi-verb with CONTAINS rows must not silently attach a real handler or call the CONTAINS query at all. 75 tests pass across the three extractor suites. * chore: prettier formatting |
||
|
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> |
||
|
ivkond
|
1ff324ca16
|
feat(group): bridge.lbug storage + contract matching expansion (1/4 of #606 split) (#795)
* feat(group): bridge.lbug storage + contract matching expansion Part 1 of 4 in the split of #606 (ticket: #791, closes #790 with a revised plan per @magyargergo's request). ## What changed Adds the LadybugDB-backed bridge storage infrastructure and extends the contract matching algorithm with wildcard support. All changes are additive: storage.ts, sync.ts, service.ts, cli/group.ts, mcp/tools.ts are left on their upstream main versions and will migrate to the new bridge in follow-up PRs (#792, #793, #794). ### Files **New (844 LOC prod):** - `gitnexus/src/core/group/bridge-db.ts` — atomic write-to-temp with `retryRename` for Windows EBUSY/EPERM, per-item write tolerance via `WriteBridgeReport`, `findContractNode` with three-tier symbol lookup (uid → filePath+name → filePath) - `gitnexus/src/core/group/bridge-schema.ts` — schema DDL - `gitnexus/src/core/group/normalization.ts` — contract ID canonicalization + `dedupeContracts` / `dedupeCrossLinks` helpers used by both matching and bridge write **Modified (+137 LOC prod):** - `gitnexus/src/core/group/matching.ts` — adds `runWildcardMatch` for `grpc::Service/*` wildcard consumers, `buildProviderIndex` helper, and canonical gRPC ID handling in `normalizeContractId` - `gitnexus/src/core/group/types.ts` — `MatchType` gains `'wildcard'`; new `BridgeHandle` and `BridgeMeta` interfaces **New tests (658 LOC):** - `gitnexus/test/unit/group/bridge-db.test.ts` — core write/read round trip, `WriteBridgeReport` shape, dropped-links counter, retryRename behavior on EBUSY/ENOENT/EPERM/EACCES - `gitnexus/test/unit/group/bridge-db-edge.test.ts` — edge cases (malformed meta, missing contract nodes, concurrent access) **Modified tests (+225 LOC):** - `gitnexus/test/unit/group/matching.test.ts` — wildcard consumer matching, gRPC canonical ID handling, same-service guard ### Self-review fixes folded in Carried forward from the original #606 self-review: - `writeBridge` try/finally handle lifecycle + `handleClosed` sentinel - `openBridgeDbReadOnly` partial-handle cleanup - `writeBridgeMeta` uses `retryRename` for Windows consistency - `retryRename` unit tests (was zero coverage) - Per-item try/catch around every CREATE loop so one malformed contract doesn't abort the whole write - Dropped cross-link counter (`linksDroppedMissingNode`) ### Why now magyargergo asked for the #606 PR to be split so we can iterate with confidence (https://github.com/abhigyanpatwari/GitNexus/pull/606#issuecomment-4229612271). This is the foundational layer — pure infra, no user-facing surface, no callers of the new APIs in this PR. Later PRs wire it in. ### How to verify - `cd gitnexus && npx tsc --noEmit` - `cd gitnexus && npx vitest run test/unit/group/bridge-db.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/bridge-db-edge.test.ts --pool=forks` - `cd gitnexus && npx vitest run test/unit/group/matching.test.ts --pool=forks` - Pre-commit hook runs clean ### Risk / rollback **Low.** All new code sits under `src/core/group/` in new files plus a minimal `+16/-1` diff to `types.ts` and a `+136/-0` diff to `matching.ts` (both purely additive). No existing callers reference the new APIs (bridge-db, openBridgeOrFallback, runWildcardMatch) — the PRs that wire them in come later in the split chain. Rollback = `git revert` of the merge commit; no state introduced, no schema migration triggered. ### Scope discipline (per GUARDRAILS.md) - Only the 8 files listed above are touched; no drive-by refactors - No CI/release/security config changes - No secrets, tokens, or machine-specific paths - Content is lifted from the #606 branch which already passed CI 11/11 green on `d15b8cb` (before the split) ### Dependencies - **Base:** `main` (no dependencies on other split PRs) - **Blocks:** extractor expansion (#792), sync pipeline (#793), cross-impact feature (#794) - **Related ticket:** #791 Co-authored-by: Claude <noreply@anthropic.com> * fix(group): address @claude review on #795 Addresses the findings from the automated review on PR #795 (https://github.com/abhigyanpatwari/GitNexus/pull/795#issuecomment-4229770000 — posted by @magyargergo / claude-code Action run). ### Medium severity (reviewer flagged as blockers) - **bridge-db.ts `openBridgeDbReadOnly` bak recovery** — the `.bak` recovery path used bare `fsp.rename(bakPath, dbPath)`, which is exactly the scenario most likely to hit Windows EBUSY/EPERM (an interrupted writer still holding the handle for a few ms). Switched to `retryRename` for consistency with the rest of the file's Windows-safe rename path. - **bridge-db.ts `ensureBridgeSchema` error detection** — the inline `msg.includes('already exists')` substring match has been lifted into a named constant `LBUG_ALREADY_EXISTS_MSG` with a comment documenting the coupling to LadybugDB's error message wording and why we can't use `IF NOT EXISTS` (LadybugDB DDL doesn't support it) or typed errors (LadybugDB's JS driver doesn't expose error codes). Also tightened the `catch (err: any)` to `catch (err: unknown)`. - **bridge-db.ts `findContractNode` — extracted out of writeBridge** — the 35-line async closure living inside `writeBridge` has been lifted to three module-level functions: `createContractLookupIndex`, `indexContract`, and `findContractNode`. `findContractNode` is now a pure synchronous function taking a prebuilt index instead of doing its own DB queries. The `writeBridge` cross-link loop is now ~25 lines instead of ~100. - **bridge-db.ts `findContractNode` — N+1 query elimination** — the old inner-closure version issued up to 6 DB round-trips per cross-link (2 endpoints × up to 3 tiers of fallback queries). For a group with 1000 cross-links, that's up to 6000 DB queries just to resolve endpoints. The new version consults an in-memory `ContractLookupIndex` built incrementally as contracts are inserted (`indexContract` called AFTER each successful insert so failed inserts don't poison the index). Cross-link resolution is now O(1) per link instead of O(3) DB queries per link, with zero DB round-trips during the cross-link loop. ### Minor severity - **bridge-db.ts `queryBridge` empty-array guard** — if LadybugDB ever returns an empty `QueryResult[]` at the top level (shouldn't happen with single-statement calls, but driver contract isn't explicit), the old code would call `.getAll()` on `undefined` and crash with a confusing stack. Added an `unwrapQueryResult` helper that throws an explicit `'empty QueryResult array'` error instead, making a potential driver regression visible immediately. - **normalization.ts `contractRichness` weights** — added a block-level comment documenting the weight ordering (+3 for symbolUid, +2 for each symbol-identifying field, +1 for service tag or non-manifest origin) and explicitly noting that the absolute numbers don't matter, only the relative ordering. Matches the "comment for contributors" suggestion in the review. - **bridge-schema.ts `BRIDGE_SCHEMA_VERSION` migration comment** — added a 4-point contract explaining what bumping the constant means ("discard and re-sync" strategy for V1, no in-place migration yet, new migration logic should live in a separate `bridge-migrations.ts` module when it becomes necessary). - **test/unit/group/fixtures.ts** — extracted the `makeContract` helper previously copy-pasted between `bridge-db.test.ts` and `bridge-db-edge.test.ts` into a shared fixtures module. Both test files now import from `./fixtures.js`. Kept the scope minimal: fixtures is NOT a general-purpose factory module, just the shared baseline contract builder. ### New tests Added 9 pure-function unit tests for the now-extracted `findContractNode` in `bridge-db.test.ts`: - returns null on empty index - tier 1 (symbolUid) match, including repo-scope and role-scope isolation - tier 2 (filePath + symbolName) fallback when symbolUid is empty or mismatches - tier 3 (filePath only) when exactly one contract lives in the file, and refusal when multiple do - priority ordering when multiple tiers could resolve These are fully isolated — no DB, no temp directories, no native LadybugDB binding — so they run in <10ms total and are immediately trustworthy as a regression safety net. ### Deliberately deferred (reviewer marked as "fine for now") - `BridgeHandle._db` / `._conn` typing to `unknown` with casts in `bridge-db.ts` — reviewer's note: "The typing is fine for now." - Batch inserts via `UNWIND` — needs LadybugDB support confirmation, tracked as a follow-up; the per-item pattern remains. - `queryBridge` prepared-statement lifecycle — the current pattern (prepare → execute → GC) relies on LadybugDB's internals, worth verifying against their docs in a separate audit. ### Scope discipline (per `GUARDRAILS.md`) - Only files touched by this PR (`bridge-db.ts`, `bridge-schema.ts`, `normalization.ts`, both bridge test files, new `fixtures.ts`) — no drive-by refactors - No CI/release/security config changes - No secrets ### Test + typecheck status - `npx tsc --noEmit` clean - `bridge-db.test.ts`: added 9 `findContractNode` tests, all pass in isolation. The full-file run still hits the pre-existing native LadybugDB cleanup segfault that flakes the reported count — same as every prior commit on this branch, not a regression. - `bridge-db-edge.test.ts`: 4/4 pass - `matching.test.ts`: 28/28 pass - `types.test.ts`: 5/5 pass - `retryRename` tests (4/4) and `findContractNode` tests (9/9) verified in isolation via `-t` filter Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
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 |
||
|
ivkond
|
255e3e79eb |
fix(group): address 4 HIGH-priority issues from PR #626 review
1. Path traversal via group name — add validateGroupName() with regex [a-zA-Z0-9][a-zA-Z0-9_-]*, called in getGroupDir (defense in depth) 2. gRPC proto regex can't handle nested braces — replace serviceRe with extractServiceBlocks() brace-depth counter (init depth=1, skip malformed protos) 3. Service boundary detector directory exclusions — add EXCLUDED_DIRS set (vendor, target, build, dist, __pycache__, .venv, venv, .tox, .mypy_cache, .gradle, .mvn, out, bin) replacing inline node_modules 4. Double-close of LadybugDB pools — remove blanket closeLbug() from cli/group.ts; sync.ts per-id cleanup is sufficient Tests: 22 new tests across 5 files. Full suite: 4706 passed, 0 failed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
ivkond
|
4fed097abb |
feat(group): add sync pipeline, CLI, MCP tools, and monorepo fixture
Wire extractors into the sync pipeline with service boundary detection. GroupService provides high-level API for all group operations. - Sync pipeline: orchestrates extraction (HTTP, gRPC, topics) with service boundary assignment and exact matching - GroupService: groupList, groupSync, groupContracts, groupQuery, groupStatus (groupImpact deferred to cross-repo follow-up PR) - CLI: group create/add/remove/list/sync/contracts/query/status - MCP tools: group_list, group_sync, group_contracts, group_query, group_status - Monorepo fixture: 3 services (auth/orders/gateway) connected via gRPC + Kafka + HTTP — all intra-repo cross-links discovered - Documentation: CLI commands and MCP tools added to both READMEs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
ivkond
|
52277247fe |
feat(group): add group infrastructure and contract matching
Core foundation for repository group analysis: - Type system: ContractType, ExtractedContract, StoredContract, CrossLink with optional `service` field for intra-repo matching - Config parser for group.yaml (repos, detection flags, matching thresholds) - Contract registry storage with atomic writes - Exact matching engine with per-type normalization (HTTP, gRPC, topic) and intra-repo support (different services within same repo can match) - Extract LadybugDB pool-adapter from MCP backend for reuse by sync pipeline - Git staleness checker for group status reporting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |