diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 1b83a1e4b..7cd8b678c 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -135,6 +135,9 @@ jobs: # the type=semver patterns would not match. In that case we add an explicit # type=raw tag using the version already verified above, so the same # image-naming rules apply regardless of how the workflow was triggered. + # NOTE: We check `inputs.tag` rather than `github.event_name` because in a + # reusable workflow the github context is inherited from the caller — + # `github.event_name` would still be "push", not "workflow_call". - name: Extract Docker metadata id: meta uses: docker/metadata-action@030e881283bb7a6894de51c315a6bfe6a94e05cf # v6.0.0 @@ -145,7 +148,7 @@ jobs: type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=semver,pattern={{major}} - type=raw,value=${{ steps.version.outputs.version }},enable=${{ github.event_name == 'workflow_call' }} + type=raw,value=${{ steps.version.outputs.version }},enable=${{ inputs.tag != '' }} - name: Build and push id: build diff --git a/.gitignore b/.gitignore index 95c9164e0..92027b529 100644 --- a/.gitignore +++ b/.gitignore @@ -103,3 +103,6 @@ gitnexus/vendor/**/node_modules/ local_docs/ +# Local agent scratch / review prompts (never commit) +.tmp/ +.agents/ diff --git a/AGENTS.md b/AGENTS.md index f4fbcadc0..ef779a45f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,6 +47,7 @@ Commands and gotchas live under **Repo reference** below and in **[CONTRIBUTING. | Date | Version | Change | |------|---------|--------| +| 2026-04-19 | 1.5.0 | Cross-repo impact (#794): `impact`/`query`/`context` accept `repo: "@"` + `service`. Removed `group_query`/`group_contracts`/`group_status` MCP tools; added `gitnexus://group/{name}/contracts` and `gitnexus://group/{name}/status` resources. | | 2026-04-16 | 1.4.0 | Fixed: web UI description, pre-commit behavior, MCP tools (7->16), added gitnexus-shared, removed stale vite-plugin-wasm gotcha. | | 2026-04-13 | 1.3.0 | Updated GitNexus index stats after DAG refactor. | | 2026-03-24 | 1.2.0 | Fixed gitnexus:start block duplication. | @@ -107,10 +108,14 @@ Indexed as **GitNexus** (4325 symbols, 10556 relationships, 300 execution flows) | `tool_map` | MCP/RPC tool definitions | `gitnexus_tool_map({})` | | `shape_check` | Response shape vs consumer access | `gitnexus_shape_check({route: "/api/users"})` | | `group_list` | List repo groups | `gitnexus_group_list({})` | -| `group_query` | Cross-repo search in a group | `gitnexus_group_query({name: "myGroup", query: "auth"})` | | `group_sync` | Rebuild group Contract Registry | `gitnexus_group_sync({name: "myGroup"})` | -| `group_contracts` | Inspect group contracts | `gitnexus_group_contracts({name: "myGroup"})` | -| `group_status` | Group staleness report | `gitnexus_group_status({name: "myGroup"})` | +| `query` (group mode) | Cross-repo search in a group (RRF-merged) | `gitnexus_query({repo: "@myGroup", query: "auth"})` | +| `context` (group mode) | 360° view across all member repos | `gitnexus_context({repo: "@myGroup", name: "validateUser"})` | +| `impact` (group mode) | Cross-repo blast radius via Contract Bridge | `gitnexus_impact({repo: "@myGroup", target: "X", direction: "upstream"})` | + +> Group mode: pass `repo: "@"` to fan out across all member repos, or `repo: "@/"` to target a single member (path keys from `group.yaml`). Optional `service: ""` filters by service root. Group-level state (contracts, staleness) lives in the resources table below — there are **no** `group_query` / `group_context` / `group_impact` / `group_contracts` / `group_status` MCP tools. +> +> For a full walkthrough of setting up a group across multiple repos that communicate over gRPC, see [docs/guides/microservices-grpc.md](docs/guides/microservices-grpc.md). ## Impact Risk Levels @@ -128,6 +133,8 @@ Indexed as **GitNexus** (4325 symbols, 10556 relationships, 300 execution flows) | `gitnexus://repo/GitNexus/clusters` | All functional areas | | `gitnexus://repo/GitNexus/processes` | All execution flows | | `gitnexus://repo/GitNexus/process/{name}` | Step-by-step execution trace | +| `gitnexus://group/{name}/contracts` | Group Contract Registry (provider/consumer rows + cross-links) | +| `gitnexus://group/{name}/status` | Per-member index + Contract Registry staleness report | ## Self-Check Before Finishing diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d934cd8b6..ceb2f9583 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -42,10 +42,14 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). | `tool_map` | MCP/RPC tool definitions and handlers | | `shape_check` | Response shape vs consumer property access mismatches | | `group_list` | List repo groups or details for one group | -| `group_query` | Cross-repo search in a group (reciprocal rank fusion) | -| `group_sync` | Rebuild group Contract Registry (`contracts.json`) | -| `group_contracts` | Inspect group contracts and cross-links | -| `group_status` | Index and Contract Registry staleness per repo in a group | +| `group_sync` | Rebuild group Contract Registry (`contracts.json`) and bridge graph | + +`query`, `context`, and `impact` are group-aware: pass `repo: "@"` (or `"@/"` to scope to one member) plus optional `service: ""`. Group-mode `query` merges per-repo results via Reciprocal Rank Fusion; group-mode `impact` runs the local walk in the chosen member and fans out across boundaries via the Contract Bridge (`gitnexus/src/core/group/cross-impact.ts`). The previously-planned `group_query`, `group_context`, `group_impact`, `group_contracts`, `group_status` MCP tools are intentionally not introduced — group-level state is exposed via resources instead: + +| Resource URI | Purpose | +|--------------|---------| +| `gitnexus://group/{name}/contracts` | Contract Registry (provider/consumer rows + cross-links) | +| `gitnexus://group/{name}/status` | Per-member index + Contract Registry staleness | ## Where to change what @@ -55,6 +59,7 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). | Parsing/graph construction | `src/core/ingestion/pipeline-phases/` + `pipeline.ts` | | Graph schema/DB | `src/core/lbug/` (`schema.ts`, `lbug-adapter.ts`) | | MCP tools/resources | `src/mcp/server.ts`, `tools.ts`, `resources.ts` | +| Cross-repo groups (sync, contracts, `@` routing) | `src/core/group/` (`service.ts`, `cross-impact.ts`, `sync.ts`, `bridge-db.ts`) | | Search ranking | `src/core/search/` (BM25, hybrid fusion) | | Embeddings | `src/core/embeddings/` + `src/core/run-analyze.ts` | | Wiki generation | `src/core/wiki/` | diff --git a/Dockerfile.web b/Dockerfile.web index d4f342509..7d20e09ce 100644 --- a/Dockerfile.web +++ b/Dockerfile.web @@ -11,6 +11,8 @@ RUN npm ci --prefix gitnexus-shared COPY gitnexus-shared ./gitnexus-shared RUN npm run build --prefix gitnexus-shared +COPY gitnexus/package.json ./gitnexus/ + COPY gitnexus-web/package.json gitnexus-web/package-lock.json ./gitnexus-web/ RUN npm ci --prefix gitnexus-web diff --git a/README.md b/README.md index 4c273ab47..9e20c274c 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@

Join the official Discord to discuss ideas, issues etc!

- + Discord @@ -194,6 +194,7 @@ gitnexus analyze --force # Force full re-index gitnexus analyze --skills # Generate repo-specific skill files from detected communities gitnexus analyze --skip-embeddings # Skip embedding generation (faster) gitnexus analyze --skip-agents-md # Preserve custom AGENTS.md/CLAUDE.md gitnexus section edits +gitnexus analyze --skip-git # Index folders that are not Git repositories gitnexus analyze --embeddings # Enable embedding generation (slower, better search) gitnexus analyze --verbose # Log skipped files when parsers are unavailable gitnexus mcp # Start MCP server (stdio) — serves all indexed repos diff --git a/docs/guides/microservices-grpc.md b/docs/guides/microservices-grpc.md new file mode 100644 index 000000000..afe6b2499 --- /dev/null +++ b/docs/guides/microservices-grpc.md @@ -0,0 +1,295 @@ +# Using GitNexus across gRPC microservices + +## When to use this guide + +This guide is for teams whose product lives in **several separate Git repositories** — one per service — and whose services talk to each other over **gRPC** (possibly alongside HTTP and message topics). GitNexus indexes each repo independently, then a _group_ stitches the per-repo indexes into a single cross-repo view that the `impact`, `query`, and `context` tools can traverse. If your services live in one monorepo, much of this still applies — set each service as a member of a group and use the `service` prefix to scope queries — but the walkthrough assumes the harder multi-repo case. + +## Mental model + +- Each repository has its own `.gitnexus/` index (a LadybugDB graph of symbols, relationships, processes). `gitnexus analyze` in each repo produces that index completely independently. +- A **group** is a higher-level construct stored at `~/.gitnexus/groups//` that references the per-repo indexes by their registry name. +- Sync-time extractors walk each member repo and emit **contracts** — provider or consumer records keyed by a canonical `contractId` (`grpc::auth.AuthService/Login`, `http::GET::/orders`, etc.). +- The sync step matches providers and consumers that share a `contractId` and writes **cross-links** to `/contracts.json`. Those cross-links are what lets `impact({repo: "@", target: "X"})` hop from one repo into another. +- Contracts come from three places: automatic contract extractors (`grpc-extractor`, `http-route-extractor`, `topic-extractor`), a manifest escape hatch (`config.links` in `group.yaml`), and — for same-name symbol matches where no contract is declared — the exact-match matching cascade in [`matching.ts`](../../gitnexus/src/core/group/matching.ts). +- Each repo stays editable and re-indexable on its own. Re-run `gitnexus analyze` in a repo when it changes, then `gitnexus group sync ` to refresh `contracts.json`. `gitnexus group status` reports which members are stale. + +## Prerequisites + +- GitNexus installed and runnable as `gitnexus` or `npx gitnexus` (see the root [README.md](../../README.md)). +- Each service repository checked out locally. No requirement that they share a parent directory — the group references them by registry name. +- Write access to `~/.gitnexus/` (the default gitnexus home; see `getDefaultGitnexusDir` in [`storage.ts`](../../gitnexus/src/core/group/storage.ts)). + +## Step-by-step walkthrough + +The example uses three services — a TypeScript API gateway, a Go orders service, and a Python inventory service — with gRPC between them. The gateway is an `orders` consumer; the orders service is both an `orders` provider and an `inventory` consumer; the inventory service is an `inventory` provider. + +### 1. Index each repository + +Run `analyze` from inside each service repo (or pass the path). The CLI surface lives in [`gitnexus/src/cli/analyze.ts`](../../gitnexus/src/cli/analyze.ts) and is wired in [`gitnexus/src/cli/index.ts`](../../gitnexus/src/cli/index.ts). + +```bash +cd ~/code/gateway && npx gitnexus analyze +cd ~/code/orders && npx gitnexus analyze +cd ~/code/inventory && npx gitnexus analyze +``` + +Useful flags: + +- `--force` — reindex even if up to date. +- `--embeddings` — generate embedding vectors (needed only if you want semantic search; the exact-match cross-repo cascade does **not** need them). +- `--name ` — register the repo under a specific alias when two repos share a basename (e.g. two `api/` folders). +- `--skip-git` — index a checkout that isn't a git repo. + +Each run writes a `.gitnexus/` folder in the repo and registers the repo in `~/.gitnexus/registry.json`. Confirm with `npx gitnexus list`. + +### 2. Author `group.yaml` + +Create the group directory and edit the config. Either use the CLI scaffolder or write the file directly — both produce the same shape consumed by [`config-parser.ts`](../../gitnexus/src/core/group/config-parser.ts). + +```bash +npx gitnexus group create payments-platform +# or manually: +mkdir -p ~/.gitnexus/groups/payments-platform +$EDITOR ~/.gitnexus/groups/payments-platform/group.yaml +``` + +Minimal working `group.yaml`: + +```yaml +version: 1 +name: payments-platform +description: Gateway + orders + inventory (gRPC) + +repos: + gateway: gateway + orders: orders + inventory: inventory + +# Only add explicit links when the automatic extractors miss something — +# see "When automatic extraction isn't enough" below. +links: [] + +packages: {} + +detect: + http: true + grpc: true + topics: true + shared_libs: true + embedding_fallback: false + +matching: + bm25_threshold: 0.7 + embedding_threshold: 0.65 + max_candidates_per_step: 3 +``` + +Field notes (schema in [`types.ts`](../../gitnexus/src/core/group/types.ts)): + +- `version` — must be `1`. The parser rejects anything else. +- `name` — required; used for the group directory name and all CLI / MCP calls. +- `repos` — a mapping from **group path** (a logical name you choose; can be a hierarchy like `backend/orders`) to **registry name** (the name shown by `npx gitnexus list`). Both sides appear throughout the tooling: contract rows use the group path; `@/` routes tools to a single member. +- `links` — optional manifest escape hatch, one entry per explicit cross-repo contract. Validated by the parser: `from` and `to` must be known repo paths, `type` must be one of `http | grpc | topic | lib | custom`, and `role` must be `provider | consumer`. +- `detect` — toggles per extractor family. Defaults (set in `config-parser.ts`) turn `http`, `grpc`, `topics`, and `shared_libs` on; disable the ones you don't use to speed up sync. +- `matching` — thresholds for the matching cascade. The exact match is always run; other strategies depend on indexer state. + +### 3. Sync the group + +```bash +npx gitnexus group sync payments-platform --verbose +``` + +What this does (see [`sync.ts`](../../gitnexus/src/core/group/sync.ts)): + +1. Opens each member's per-repo LadybugDB. +2. Runs the HTTP, gRPC, and topic extractors against the source files. +3. Applies manifest `links` through [`manifest-extractor.ts`](../../gitnexus/src/core/group/extractors/manifest-extractor.ts). +4. Runs the exact-match cascade, joining providers and consumers that share a normalized `contractId`. +5. Writes `contracts.json` in the group directory. + +Flags: + +- `--exact-only` — stop after the exact cascade; skip BM25 and embedding fallback. +- `--skip-embeddings` — run exact plus BM25 but not embedding-based matching. +- `--allow-stale` — don't warn if a member's index is stale. +- `--json` — machine-readable output. + +The same operation is available over MCP as `group_sync({ name: "payments-platform" })` — see [`tools.ts`](../../gitnexus/src/mcp/tools.ts). + +### 4. Inspect the registry + +Use `gitnexus group contracts` for the CLI view or read the `gitnexus://group//contracts` MCP resource for the same data. + +```bash +npx gitnexus group contracts payments-platform --type grpc --json +``` + +A shortened response: + +```json +{ + "contracts": [ + { + "contractId": "grpc::orders.OrderService/PlaceOrder", + "type": "grpc", + "role": "provider", + "repo": "orders", + "symbolRef": { "filePath": "internal/grpc/order_server.go", "name": "RegisterOrderServiceServer" }, + "confidence": 0.8, + "meta": { "service": "OrderService", "method": "PlaceOrder", "source": "go_register" } + }, + { + "contractId": "grpc::orders.OrderService/PlaceOrder", + "type": "grpc", + "role": "consumer", + "repo": "gateway", + "symbolRef": { "filePath": "src/clients/orders.ts", "name": "OrderServiceClient" }, + "confidence": 0.75, + "meta": { "service": "OrderService", "source": "ts_generated_client" } + } + ], + "crossLinks": [ + { + "from": { "repo": "gateway", "symbolUid": "…", "symbolRef": { "filePath": "src/clients/orders.ts", "name": "OrderServiceClient" } }, + "to": { "repo": "orders", "symbolUid": "…", "symbolRef": { "filePath": "internal/grpc/order_server.go", "name": "RegisterOrderServiceServer" } }, + "type": "grpc", + "contractId": "grpc::orders.OrderService/PlaceOrder", + "matchType": "exact", + "confidence": 1.0 + } + ] +} +``` + +Staleness of the underlying indexes shows up in `npx gitnexus group status payments-platform` or the `gitnexus://group//status` resource. + +### 5. Run cross-repo impact with `@` routing + +From any shell (you do **not** have to `cd` into a member repo), the normal `impact` / `query` / `context` tools accept `repo: "@"` to fan out across all members, or `repo: "@/"` to target one member. Routing is implemented in [`resolve-at-member.ts`](../../gitnexus/src/core/group/resolve-at-member.ts) and described in [`tools.ts`](../../gitnexus/src/mcp/tools.ts). + +Example MCP calls: + +```json +{"tool": "impact", "arguments": { + "repo": "@payments-platform/orders", + "target": "PlaceOrder", + "direction": "upstream", + "crossDepth": 2 +}} +``` + +```json +{"tool": "query", "arguments": { + "repo": "@payments-platform", + "query": "retry logic around PlaceOrder" +}} +``` + +The CLI equivalents still exist for scripting: + +```bash +npx gitnexus group impact payments-platform \ + --repo orders --target PlaceOrder --direction upstream --cross-depth 2 +``` + +Phase 1 walks within the anchor member; Phase 2 hops across the Contract Bridge wherever a cross-link endpoint matches an impacted symbol. See [`cross-impact.ts`](../../gitnexus/src/core/group/cross-impact.ts) for the bridge query. + +## How gRPC extraction works + +`GrpcExtractor` ([`grpc-extractor.ts`](../../gitnexus/src/core/group/extractors/grpc-extractor.ts)) runs two passes per member repo: + +1. **Proto map.** Every `**/*.proto` file is parsed to enumerate `service Foo { rpc Bar(...) }` blocks and (transitively) resolve the package name. Each RPC method becomes a provider contract with `contractId = grpc::./` and `confidence = 0.85`. Parsing uses the vendored `tree-sitter-proto` grammar when available and falls back to a length-preserving manual parser (`extractServiceBlocks`) otherwise, so `.proto` extraction works on platforms where the grammar fails to build. +2. **Source scan.** Every source file whose extension matches [`GRPC_SCAN_GLOB`](../../gitnexus/src/core/group/extractors/grpc-patterns/index.ts) is parsed by its language plugin: + +| Language | Provider signal | Consumer signal | +|----------|-----------------|-----------------| +| Go ([`go.ts`](../../gitnexus/src/core/group/extractors/grpc-patterns/go.ts)) | `pb.RegisterXxxServer(...)`, `pb.UnimplementedXxxServer` embedded in struct | `pb.NewXxxClient(conn)` | +| Java ([`java.ts`](../../gitnexus/src/core/group/extractors/grpc-patterns/java.ts)) | `extends XxxServiceGrpc.XxxServiceImplBase` (with or without `@GrpcService`) | `XxxServiceGrpc.newBlockingStub(...)`, `newStub(...)` | +| Python ([`python.ts`](../../gitnexus/src/core/group/extractors/grpc-patterns/python.ts)) | `add_XxxServicer_to_server(...)` (bare or `_pb2_grpc.` attribute form) | `XxxStub(channel)` (ignores `Mock`/`Test`/`Fake`/`Stub`) | +| Node / TS ([`node.ts`](../../gitnexus/src/core/group/extractors/grpc-patterns/node.ts)) | NestJS `@GrpcMethod('Service','Method')` | `@GrpcClient` field typed `XxxServiceClient`, `client.getService('Service')`, `new XxxServiceClient(...)`, `new foo.bar.XxxService(...)` in files that call `loadPackageDefinition` | + +For each source-scan detection the extractor looks up the short service name in the proto map and picks: + +- `grpc::./` when a method is named and the service resolves against the proto map, +- `grpc::./*` (wildcard) when only the service is known, or +- `grpc::/*` when no `.proto` is available at all. + +Provider detections land at confidence 0.8 (with proto) or 0.65 (without); consumers at 0.75 or 0.55. NestJS `@GrpcMethod` is fixed at 0.8 because the decorator is self-describing. + +### Matching + +`matching.ts` lowercases the package/service segment before comparing contract ids, so bindings that capitalize names differently (`auth.AuthService` vs `auth.authservice`) still match. Method names are compared case-sensitively because gRPC's wire path is case-sensitive. Service-only wildcards (`grpc::pkg.Svc/*`) match any method on the same service during cross-linking. + +### Known limitations + +- **Ambiguous proto resolution.** If a short service name exists in more than one `.proto` file and the source-scan hit can't be narrowed down by shared directory segments (`resolveProtoConflict` refuses to guess), the extractor skips contract emission and logs a warning. +- **Proto packages must be resolvable locally.** Transitive imports that point outside the repo produce an empty package segment, which means the contract id collapses to `grpc::/`. Cross-repo matches still work as long as both sides agree on the empty package. +- **Rewrite rules are not implemented.** If the provider repo writes `grpc::orders.OrderService/PlaceOrder` and the consumer repo writes `grpc::orderspb.OrderService/PlaceOrder`, they won't cross-link automatically. Use `config.links` to declare the correspondence (see below). +- **One sync = one snapshot.** Contracts are extracted against the indexed snapshot of each repo. Re-index first, then re-sync; the `status` command and resource surface staleness. + +## When automatic extraction isn't enough + +The escape hatch is the `links` list in `group.yaml`, handled by [`ManifestExtractor`](../../gitnexus/src/core/group/extractors/manifest-extractor.ts). Each entry is a **one-directional** provider/consumer declaration: + +```yaml +version: 1 +name: payments-platform +repos: + gateway: gateway + orders: orders + inventory: inventory + +links: + # Explicit gRPC method: use when naming mismatches stop the + # automatic matcher from cross-linking. + - from: gateway + to: orders + type: grpc + contract: OrderService/PlaceOrder + role: consumer + + # Service-level link when you don't want to enumerate methods. + - from: orders + to: inventory + type: grpc + contract: InventoryService + role: consumer + + # Works for HTTP too — use `METHOD::/path` form for the exact + # handler, or just `/path` for a method-agnostic wildcard. + - from: gateway + to: orders + type: http + contract: POST::/orders + role: consumer +``` + +What the manifest extractor does (see [`manifest-extractor.ts`](../../gitnexus/src/core/group/extractors/manifest-extractor.ts)): + +1. Builds a canonical `contractId` with `buildContractId` — the same canonicalization used by the automatic extractors, so manifest links cross-match automatic contracts on the other side. +2. Tries to resolve each side to a real graph symbol (the `Route` node for HTTP, a `Function|Method` / `Class|Interface` for gRPC, a `Package|Module` for `lib`). +3. If resolution fails, falls back to a deterministic synthetic uid (`manifest::::`) so both sides still line up in cross-impact — name-only links still work when the symbol isn't in the graph. +4. Emits both a provider and a consumer `StoredContract` (confidence `1.0`, `source: "manifest"`) and a `CrossLink` with `matchType: "manifest"`. + +Use `links` for exactly the cases the extractor can't infer: different package names across repos (see #701), hand-rolled transports, cases where the provider repo isn't checked out locally but you still want a record, or any contract whose provider and consumer simply don't share a surface the extractors know how to pattern-match. + +History: the manifest extractor used to be silently skipped by the sync pipeline; that was fixed in [#827](https://github.com/abhigyanpatwari/GitNexus/pull/827) (tracking issue #826). If you ever see `config.links` with zero cross-links in `contracts.json`, make sure you're on a build that includes that fix, then re-run `group sync`. + +## Troubleshooting + +1. **`contracts.json` is empty after a sync.** Either no member repo contained a recognizable gRPC pattern, or the extractors are disabled in `detect`. Confirm `detect.grpc: true` and re-run with `--verbose`. +2. **A known provider/consumer pair doesn't cross-link.** Most common cause: the package segment differs. Check the raw contract ids with `gitnexus group contracts --unmatched` — if you see two same-method contracts with different package prefixes, add a manifest `links:` entry to bridge them (no automatic rewrite rules yet). +3. **`matchType: "manifest"` is missing entirely.** The extractor needs `config.links` to be non-empty and the sync pipeline to actually call it — verify you're on a post-#827 build. Empty contract rows for manifest links usually mean `resolveSymbol` couldn't find a graph match; the synthetic uid still lets cross-impact work, it just won't carry a file path. +4. **Ambiguous proto warnings.** Look for `[grpc-extractor] Ambiguous proto resolution` in the sync logs; that means a service name exists in multiple `.proto` files under the same repo and the path-distance heuristic couldn't pick a winner. Resolve by renaming the service or declaring the intended pairing in `config.links`. +5. **Cross-impact says "stale".** Both sides need a fresh per-repo index _and_ a fresh group sync. Order matters: `gitnexus analyze` in each changed repo, then `gitnexus group sync `. Use `gitnexus group status ` to see which side is behind. + +## Related docs and references + +- [AGENTS.md](../../AGENTS.md) — authoritative list of MCP tools and resources, including group-mode routing and the `gitnexus://group/…` resources. +- [ARCHITECTURE.md](../../ARCHITECTURE.md) — overall data flow and the call-resolution DAG that the per-repo indexer uses. +- [`gitnexus/src/core/group/`](../../gitnexus/src/core/group/) — `service.ts`, `sync.ts`, `config-parser.ts`, `matching.ts`. +- [`gitnexus/src/core/group/extractors/grpc-extractor.ts`](../../gitnexus/src/core/group/extractors/grpc-extractor.ts) and [`grpc-patterns/`](../../gitnexus/src/core/group/extractors/grpc-patterns/) — gRPC detection. +- [`gitnexus/src/core/group/extractors/manifest-extractor.ts`](../../gitnexus/src/core/group/extractors/manifest-extractor.ts) — the `config.links` escape hatch. +- [`gitnexus/src/mcp/tools.ts`](../../gitnexus/src/mcp/tools.ts) — MCP tool schemas (`group_list`, `group_sync`, plus `@` routing on `impact` / `query` / `context`). +- [`gitnexus/src/cli/group.ts`](../../gitnexus/src/cli/group.ts) — CLI command definitions and flags. +- Upstream issues: [#701](https://github.com/abhigyanpatwari/GitNexus/issues/701), [#826](https://github.com/abhigyanpatwari/GitNexus/issues/826), [#906](https://github.com/abhigyanpatwari/GitNexus/issues/906). diff --git a/gitnexus/CHANGELOG.md b/gitnexus/CHANGELOG.md index 203089d93..e4a5041ba 100644 --- a/gitnexus/CHANGELOG.md +++ b/gitnexus/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to GitNexus will be documented in this file. +## [Unreleased] + +### Performance + +- **`analyze` ~33% faster** — moved FTS index creation from the analyze pipeline to first-use lazy initialisation. The 5 `CREATE_FTS_INDEX` calls cost ~440 ms each in LadybugDB regardless of table size (≈2 s fixed overhead) and dominated runtime on small repos and slow CI runners. The cost now amortises across the first `query`/`context` call in a session via a new `ensureFTSIndex` helper. Mini-repo `analyze` measured locally on Windows: 6.4 s → 4.0 s warm; on CI Windows runners (≈3× slower) restores comfortable headroom against the 30 s e2e test budget. + ## [1.6.2] - 2026-04-18 ### Added diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index 76afc1fde..14d9cb2c8 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -19,7 +19,7 @@ "cors": "^2.8.5", "express": "^4.19.2", "glob": "^13.0.6", - "graphology": "^0.25.4", + "graphology": "^0.26.0", "graphology-indices": "^0.17.0", "graphology-utils": "^2.3.0", "ignore": "^7.0.5", @@ -40,7 +40,7 @@ "tree-sitter-ruby": "^0.23.1", "tree-sitter-rust": "0.23.1", "tree-sitter-typescript": "^0.23.2", - "uuid": "^13.0.0" + "uuid": "^14.0.0" }, "bin": { "gitnexus": "dist/cli/index.js" @@ -50,7 +50,7 @@ "@types/cors": "^2.8.17", "@types/express": "^4.17.21", "@types/js-yaml": "^4.0.9", - "@types/node": "^20.0.0", + "@types/node": "^25.6.0", "@types/uuid": "^10.0.0", "@vitest/coverage-v8": "^4.0.18", "gitnexus-shared": "file:../gitnexus-shared", @@ -2041,12 +2041,12 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "20.19.37", - "resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.37.tgz", - "integrity": "sha512-8kzdPJ3FsNsVIurqBs7oodNnCEVbni9yUEkaHbgptDACOPW04jimGagZ51E6+lXUwJjgnBw+hyko/lkFWCldqw==", + "version": "25.6.0", + "resolved": "https://registry.npmjs.org/@types/node/-/node-25.6.0.tgz", + "integrity": "sha512-+qIYRKdNYJwY3vRCZMdJbPLJAtGjQBudzZzdzwQYkEPQd+PJGixUL5QfvCLDaULoLv+RhT3LDkwEfKaAkgSmNQ==", "license": "MIT", "dependencies": { - "undici-types": "~6.21.0" + "undici-types": "~7.19.0" } }, "node_modules/@types/qs": { @@ -3316,13 +3316,12 @@ "license": "ISC" }, "node_modules/graphology": { - "version": "0.25.4", - "resolved": "https://registry.npmjs.org/graphology/-/graphology-0.25.4.tgz", - "integrity": "sha512-33g0Ol9nkWdD6ulw687viS8YJQBxqG5LWII6FI6nul0pq6iM2t5EKquOTFDbyTblRB3O9I+7KX4xI8u5ffekAQ==", + "version": "0.26.0", + "resolved": "https://registry.npmjs.org/graphology/-/graphology-0.26.0.tgz", + "integrity": "sha512-8SSImzgUUYC89Z042s+0r/vMibY7GX/Emz4LDO5e7jYXhuoWfHISPFJYjpRLUSJGq6UQ6xlenvX1p/hJdfXuXg==", "license": "MIT", "dependencies": { - "events": "^3.3.0", - "obliterator": "^2.0.2" + "events": "^3.3.0" }, "peerDependencies": { "graphology-types": ">=0.24.0" @@ -5412,9 +5411,9 @@ } }, "node_modules/undici-types": { - "version": "6.21.0", - "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.21.0.tgz", - "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==", + "version": "7.19.2", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.19.2.tgz", + "integrity": "sha512-qYVnV5OEm2AW8cJMCpdV20CDyaN3g0AjDlOGf1OW4iaDEx8MwdtChUp4zu4H0VP3nDRF/8RKWH+IPp9uW0YGZg==", "license": "MIT" }, "node_modules/universalify": { @@ -5451,9 +5450,9 @@ } }, "node_modules/uuid": { - "version": "13.0.0", - "resolved": "https://registry.npmjs.org/uuid/-/uuid-13.0.0.tgz", - "integrity": "sha512-XQegIaBTVUjSHliKqcnFqYypAd4S+WCYt5NIeRs6w/UAry7z8Y9j5ZwRRL4kzq9U3sD6v+85er9FvkEaBpji2w==", + "version": "14.0.0", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-14.0.0.tgz", + "integrity": "sha512-Qo+uWgilfSmAhXCMav1uYFynlQO7fMFiMVZsQqZRMIXp0O7rR7qjkj+cPvBHLgBqi960QCoo/PH2/6ZtVqKvrg==", "funding": [ "https://github.com/sponsors/broofa", "https://github.com/sponsors/ctavan" diff --git a/gitnexus/package.json b/gitnexus/package.json index d8f2c126a..f89124c73 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -60,7 +60,7 @@ "cors": "^2.8.5", "express": "^4.19.2", "glob": "^13.0.6", - "graphology": "^0.25.4", + "graphology": "^0.26.0", "graphology-indices": "^0.17.0", "graphology-utils": "^2.3.0", "ignore": "^7.0.5", @@ -81,7 +81,7 @@ "tree-sitter-ruby": "^0.23.1", "tree-sitter-rust": "0.23.1", "tree-sitter-typescript": "^0.23.2", - "uuid": "^13.0.0" + "uuid": "^14.0.0" }, "optionalDependencies": { "node-addon-api": "^8.0.0", @@ -97,7 +97,7 @@ "@types/cors": "^2.8.17", "@types/express": "^4.17.21", "@types/js-yaml": "^4.0.9", - "@types/node": "^20.0.0", + "@types/node": "^25.6.0", "@types/uuid": "^10.0.0", "@vitest/coverage-v8": "^4.0.18", "tsx": "^4.0.0", diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 984d1432a..984a16f7b 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -121,7 +121,7 @@ ${ groupNames && groupNames.length > 0 ? `## Cross-Repo Groups -This repository is listed under GitNexus **group(s): ${groupNames.join(', ')}** (see \`~/.gitnexus/groups/\`). For blast radius across repository boundaries, use MCP tools \`group_impact\`, \`group_sync\`, \`group_query\`, \`group_contracts\`, \`group_status\`, and \`group_list\`. From the terminal: \`npx gitnexus group list\`, \`npx gitnexus group sync \`, \`npx gitnexus group impact --target --repo \`. +This repository is listed under GitNexus **group(s): ${groupNames.join(', ')}** (see \`~/.gitnexus/groups/\`). For cross-repo analysis, use MCP tools \`impact\`, \`query\`, and \`context\` with \`repo\` set to \`@\` or \`@/\` (paths match keys in that group’s \`group.yaml\`). Use \`group_list\` / \`group_sync\` for membership and sync. From the terminal: \`npx gitnexus group list\`, \`npx gitnexus group sync \`, \`npx gitnexus group impact --target --repo \`. ` : '' diff --git a/gitnexus/src/cli/clean.ts b/gitnexus/src/cli/clean.ts index e89cc7f23..4681508fb 100644 --- a/gitnexus/src/cli/clean.ts +++ b/gitnexus/src/cli/clean.ts @@ -6,7 +6,13 @@ */ import fs from 'fs/promises'; -import { findRepo, unregisterRepo, listRegisteredRepos } from '../storage/repo-manager.js'; +import { + findRepo, + unregisterRepo, + listRegisteredRepos, + assertSafeStoragePath, + UnsafeStoragePathError, +} from '../storage/repo-manager.js'; export const cleanCommand = async (options?: { force?: boolean; all?: boolean }) => { // --all flag: clean all indexed repos @@ -27,6 +33,24 @@ export const cleanCommand = async (options?: { force?: boolean; all?: boolean }) const entries = await listRegisteredRepos(); for (const entry of entries) { + // Safety guard (#1003 review — @magyargergo): same rationale as + // remove.ts. `~/.gitnexus/registry.json` is user-writable, so a + // corrupted or hand-edited entry could point storagePath at the + // repo root, an empty string, or anywhere else — and + // fs.rm(recursive: true) on any of those would be catastrophic. + // Skip poisoned entries without touching disk, but keep going + // through the rest of the registry (preserves the existing + // per-repo error-tolerance semantics of `clean --all`). + try { + assertSafeStoragePath(entry); + } catch (err) { + if (err instanceof UnsafeStoragePathError) { + console.error(`Refusing to clean ${entry.name}: ${err.message}`); + continue; + } + throw err; + } + try { await fs.rm(entry.storagePath, { recursive: true, force: true }); await unregisterRepo(entry.path); diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index 70ca9537a..eb0dffc3d 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -184,6 +184,83 @@ export function registerGroupCommands(program: Command): void { } }); + group + .command('impact ') + .description('Cross-repo impact for a symbol in one member repo of a group') + .requiredOption('--target ', 'Symbol or file name to analyze') + .requiredOption( + '--repo ', + 'Member path from group.yaml (e.g. app/backend), not the indexed repo name', + ) + .option('--direction ', 'upstream or downstream', 'upstream') + .option('--service ', 'Optional monorepo service directory prefix (path filter)') + .option( + '--subgroup ', + 'Optional prefix limiting which group repos participate in cross fan-out', + ) + .option('--max-depth ', 'Max graph traversal depth') + .option('--cross-depth ', 'Cross-repository hop depth') + .option('--min-confidence ', 'Minimum relation confidence (0–1)') + .option('--include-tests', 'Include test files in traversal', false) + .option('--timeout-ms ', 'Phase-1 local impact wall time in milliseconds') + .option('--json', 'JSON output') + .action(async (name: string, opts: Record) => { + const { LocalBackend } = await import('../mcp/local/local-backend.js'); + + const backend = new LocalBackend(); + try { + await backend.init(); + + const payload: Record = { + name, + repo: opts.repo, + target: opts.target, + direction: (opts.direction as string) || 'upstream', + }; + if (opts.service) payload.service = opts.service; + if (opts.subgroup) payload.subgroup = opts.subgroup; + if (opts.maxDepth !== undefined && opts.maxDepth !== '') { + const n = parseInt(String(opts.maxDepth), 10); + if (!Number.isNaN(n)) payload.maxDepth = n; + } + if (opts.crossDepth !== undefined && opts.crossDepth !== '') { + const n = parseInt(String(opts.crossDepth), 10); + if (!Number.isNaN(n)) payload.crossDepth = n; + } + if (opts.minConfidence !== undefined && opts.minConfidence !== '') { + const n = parseFloat(String(opts.minConfidence)); + if (!Number.isNaN(n)) payload.minConfidence = n; + } + if (opts.timeoutMs !== undefined && opts.timeoutMs !== '') { + const n = parseInt(String(opts.timeoutMs), 10); + if (!Number.isNaN(n)) payload.timeoutMs = n; + } + if (opts.includeTests) payload.includeTests = true; + + const raw = await backend.getGroupService().groupImpact(payload); + if (raw && typeof raw === 'object' && 'error' in raw) { + console.error(String((raw as { error: string }).error)); + process.exitCode = 1; + return; + } + + if (opts.json) { + console.log(JSON.stringify(raw, null, 2)); + } else { + const summary = (raw as { summary?: Record })?.summary; + const risk = (raw as { risk?: string })?.risk; + console.log(`Group impact for "${name}" (${String(opts.repo)}): risk=${risk ?? '?'}`); + if (summary) { + console.log( + ` direct=${summary.direct ?? 0} processes=${summary.processes_affected ?? 0} cross=${summary.cross_repo_hits ?? 0}`, + ); + } + } + } finally { + await backend.dispose().catch(() => {}); + } + }); + group .command('query ') .description('Search execution flows across all repos in a group') diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index dca5983e0..fe19ba753 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -83,6 +83,15 @@ program .option('--all', 'Clean all indexed repos') .action(createLazyAction(() => import('./clean.js'), 'cleanCommand')); +program + .command('remove ') + .description( + 'Delete the GitNexus index for a registered repo (by alias, name, or absolute path). ' + + 'Unlike `clean`, does not require being inside the repo. Idempotent on unknown targets.', + ) + .option('-f, --force', 'Skip confirmation prompt') + .action(createLazyAction(() => import('./remove.js'), 'removeCommand')); + program .command('wiki [path]') .description('Generate repository wiki from knowledge graph') @@ -151,6 +160,15 @@ program .option('-r, --repo ', 'Target repository') .action(createLazyAction(() => import('./tool.js'), 'cypherCommand')); +program + .command('detect-changes') + .alias('detect_changes') + .description('Map git diff hunks to indexed symbols and affected execution flows') + .option('-s, --scope ', 'What to analyze: unstaged, staged, all, or compare', 'unstaged') + .option('-b, --base-ref ', 'Branch/commit for compare scope (e.g. main)') + .option('-r, --repo ', 'Target repository') + .action(createLazyAction(() => import('./tool.js'), 'detectChangesCommand')); + // ─── Eval Server (persistent daemon for SWE-bench) ───────────────── program diff --git a/gitnexus/src/cli/remove.ts b/gitnexus/src/cli/remove.ts new file mode 100644 index 000000000..4d2ce0771 --- /dev/null +++ b/gitnexus/src/cli/remove.ts @@ -0,0 +1,110 @@ +/** + * Remove Command (#664) + * + * Delete the `.gitnexus/` index for a registered repo and unregister it + * from the global registry (~/.gitnexus/registry.json). The target is + * identified by alias / basename-derived name / remote-inferred name / + * absolute path — no `--repo` flag, just a positional argument so the + * destructive-command ergonomics match `clean` (which is also + * destructive but scoped to `process.cwd()`). + * + * Compared to `clean`: + * - `clean` acts on the repo discovered by walking up from cwd. + * - `remove` acts on any registered repo identified by name or path. + * + * Behaviour notes: + * - Idempotent on unknown targets: exits 0 with a warning so that + * `remove X && analyze Y` keeps working in scripts. Per #664: + * "behave atomically and idempotently so retries are safe". + * - Atomic order mirrors `clean`: fs.rm FIRST, then unregister. A + * partial failure leaves the registry pointing at a missing dir + * (recoverable by `listRegisteredRepos({ validate: true })` on + * next read) rather than the opposite, which would orphan + * .gitnexus/ directories on disk. + * - `-f` / `--force` matches the confirmation-skip semantics of + * `clean -f`. (Distinct from `analyze --force`, which re-indexes; + * here there is no pipeline, so no conflation.) + */ + +import fs from 'fs/promises'; +import { + readRegistry, + resolveRegistryEntry, + assertSafeStoragePath, + unregisterRepo, + RegistryNotFoundError, + RegistryAmbiguousTargetError, + UnsafeStoragePathError, +} from '../storage/repo-manager.js'; + +export const removeCommand = async (target: string, options?: { force?: boolean }) => { + // Read the registry snapshot once and pass it to the resolver — this + // lets us render the "before" state in the dry-run path without a + // second disk read. + const entries = await readRegistry(); + + let entry; + try { + entry = resolveRegistryEntry(entries, target); + } catch (err) { + if (err instanceof RegistryNotFoundError) { + // Idempotent: missing target is a no-op warning, not an error. + // The `availableNames` hint comes from the error itself so users + // can see what they might have meant. + console.warn(`Nothing to remove: ${err.message}`); + return; + } + if (err instanceof RegistryAmbiguousTargetError) { + // Duplicate aliases are allowed via --allow-duplicate-name (#829); + // refuse to guess which one the user meant — surface the full list + // and exit non-zero so scripts don't silently pick the wrong repo. + console.error(`Error: ${err.message}`); + process.exit(1); + } + throw err; + } + + // Confirmation gate — same shape as `clean`. Default is a dry-run + // that describes what would be deleted; `--force` actually deletes. + if (!options?.force) { + console.log(`This will delete the GitNexus index for: ${entry.name}`); + console.log(` Path: ${entry.path}`); + console.log(` Storage: ${entry.storagePath}`); + console.log('\nRun with --force to confirm deletion.'); + return; + } + + // Safety guard (#1003 review — @magyargergo): refuse to proceed if + // the registry entry's `storagePath` isn't the canonical + // `/.gitnexus` subfolder. `~/.gitnexus/registry.json` is + // user-writable, so a corrupted or hand-edited entry could point + // storagePath at the repo root, an empty string (→ cwd), a parent + // dir, or anywhere else; `fs.rm(recursive: true, force: true)` on + // any of those would be a runtime disaster. Bail before touching + // disk, with an actionable hint for recovering a broken registry. + try { + assertSafeStoragePath(entry); + } catch (err) { + if (err instanceof UnsafeStoragePathError) { + console.error(`Error: ${err.message}`); + process.exit(1); + } + throw err; + } + + // Deletion order: fs.rm first, then unregister. If fs.rm fails mid-way, + // the registry entry stays so the user can retry. If fs.rm succeeds but + // unregister throws (e.g. ENOSPC on registry write), the entry becomes + // orphaned — `listRegisteredRepos({ validate: true })` prunes those on + // next read, so the failure is self-healing. + try { + await fs.rm(entry.storagePath, { recursive: true, force: true }); + await unregisterRepo(entry.path); + console.log(`Removed: ${entry.name}`); + console.log(` Path: ${entry.path}`); + console.log(` Storage: ${entry.storagePath}`); + } catch (err) { + console.error(`Failed to remove ${entry.name}:`, err); + process.exit(1); + } +}; diff --git a/gitnexus/src/cli/tool.ts b/gitnexus/src/cli/tool.ts index e5219d2d7..443f12f4c 100644 --- a/gitnexus/src/cli/tool.ts +++ b/gitnexus/src/cli/tool.ts @@ -164,3 +164,55 @@ export async function cypherCommand( }); output(result); } + +function formatDetectChangesResult(result: any): string { + if (result?.error) return `Error: ${result.error}`; + + const summary = result?.summary || {}; + if ((summary.changed_count || 0) === 0) { + return 'No changes detected.'; + } + + const lines: string[] = []; + lines.push(`Changes: ${summary.changed_files || 0} files, ${summary.changed_count || 0} symbols`); + lines.push(`Affected processes: ${summary.affected_count || 0}`); + lines.push(`Risk level: ${summary.risk_level || 'unknown'}`); + lines.push(''); + + const changed = result?.changed_symbols || []; + if (changed.length > 0) { + lines.push('Changed symbols:'); + for (const symbol of changed.slice(0, 15)) { + lines.push(` ${symbol.type} ${symbol.name} → ${symbol.filePath}`); + } + if (changed.length > 15) { + lines.push(` ... and ${changed.length - 15} more`); + } + lines.push(''); + } + + const affected = result?.affected_processes || []; + if (affected.length > 0) { + lines.push('Affected execution flows:'); + for (const processInfo of affected.slice(0, 10)) { + const steps = (processInfo.changed_steps || []).map((s: any) => s.symbol).join(', '); + lines.push(` • ${processInfo.name} (${processInfo.step_count} steps) — changed: ${steps}`); + } + } + + return lines.join('\n').trim(); +} + +export async function detectChangesCommand(options?: { + scope?: string; + baseRef?: string; + repo?: string; +}): Promise { + const backend = await getBackend(); + const result = await backend.callTool('detect_changes', { + scope: options?.scope || 'unstaged', + base_ref: options?.baseRef, + repo: options?.repo, + }); + output(formatDetectChangesResult(result)); +} diff --git a/gitnexus/src/core/embeddings/ast-utils.ts b/gitnexus/src/core/embeddings/ast-utils.ts index fb51ac052..8456b249e 100644 --- a/gitnexus/src/core/embeddings/ast-utils.ts +++ b/gitnexus/src/core/embeddings/ast-utils.ts @@ -64,16 +64,16 @@ const FUNCTION_LIKE_TYPES = new Set([ * numbers don't apply. */ export const findFunctionNode = (root: any): any | null => { - if (FUNCTION_LIKE_TYPES.has(root.type)) return root; - - for (let i = 0; i < root.namedChildCount; i++) { - const child = root.namedChild(i); - if (!child) continue; - if (FUNCTION_LIKE_TYPES.has(child.type)) return child; - const found = findFunctionNode(child); - if (found) return found; + // Iterative DFS — avoids stack overflow on deeply nested ASTs. + const stack = [root]; + while (stack.length > 0) { + const node = stack.pop()!; + if (FUNCTION_LIKE_TYPES.has(node.type)) return node; + for (let i = node.namedChildCount - 1; i >= 0; i--) { + const child = node.namedChild(i); + if (child) stack.push(child); + } } - return null; }; @@ -98,15 +98,15 @@ export const findDeclarationNode = (root: any): any | null => { 'impl_item', // Rust: impl ]); - if (CLASS_LIKE_TYPES.has(root.type)) return root; - - for (let i = 0; i < root.namedChildCount; i++) { - const child = root.namedChild(i); - if (!child) continue; - if (CLASS_LIKE_TYPES.has(child.type)) return child; - const found = findDeclarationNode(child); - if (found) return found; + // Iterative DFS — avoids stack overflow on deeply nested ASTs. + const stack = [root]; + while (stack.length > 0) { + const node = stack.pop()!; + if (CLASS_LIKE_TYPES.has(node.type)) return node; + for (let i = node.namedChildCount - 1; i >= 0; i--) { + const child = node.namedChild(i); + if (child) stack.push(child); + } } - return null; }; diff --git a/gitnexus/src/core/embeddings/chunker.ts b/gitnexus/src/core/embeddings/chunker.ts index 114a5f68c..073abfb9c 100644 --- a/gitnexus/src/core/embeddings/chunker.ts +++ b/gitnexus/src/core/embeddings/chunker.ts @@ -13,6 +13,12 @@ import { characterChunk } from './character-chunk.js'; import type { Chunk } from './character-chunk.js'; import { ensureAndParse, findDeclarationNode, findFunctionNode } from './ast-utils.js'; import { buildLineIndex, resolveChunkLines } from './line-index.js'; +import { + CHUNKING_RULES, + CHUNK_MODE_AST_DECLARATION, + CHUNK_MODE_AST_FUNCTION, + type ChunkingRule, +} from './types.js'; /** * Main chunkNode function: dispatches by label @@ -40,31 +46,39 @@ export const chunkNode = async ( ]; } - // Only function-like labels get AST chunking - if (label === 'Function' || label === 'Method' || label === 'Constructor') { - try { - const astChunks = await astChunk(content, filePath, startLine, endLine, chunkSize, overlap); - if (astChunks.length > 0) return astChunks; - } catch { - // AST parsing failed — fall through to character fallback - } + const rule = CHUNKING_RULES[label]; + if (!rule) { + return characterChunk(content, startLine, endLine, chunkSize, overlap); } - if (label === 'Class' || label === 'Interface') { - try { - const declarationChunks = await declarationChunk( - label, + try { + if (rule.mode === CHUNK_MODE_AST_FUNCTION) { + const astChunks = await astChunk( content, filePath, startLine, endLine, chunkSize, overlap, + rule, + ); + if (astChunks.length > 0) return astChunks; + } + + if (rule.mode === CHUNK_MODE_AST_DECLARATION) { + const declarationChunks = await declarationChunk( + content, + filePath, + startLine, + endLine, + chunkSize, + overlap, + rule, ); if (declarationChunks.length > 0) return declarationChunks; - } catch { - // AST parsing failed — fall through to character fallback } + } catch { + // AST parsing failed — fall through to character fallback } // Character-based fallback for everything else @@ -83,6 +97,7 @@ const astChunk = async ( endLine: number, chunkSize: number, overlap: number, + rule: ChunkingRule, ): Promise => { const tree = await ensureAndParse(content, filePath); if (!tree) return []; @@ -121,8 +136,8 @@ const astChunk = async ( statements, targetNode.startIndex, targetNode.endIndex, - true, - true, + rule.includePrefix, + rule.includeSuffix, ); }; @@ -145,13 +160,13 @@ const FIELD_LIKE_MEMBER_TYPES = new Set([ ]); const declarationChunk = async ( - label: 'Class' | 'Interface', content: string, filePath: string, startLine: number, endLine: number, chunkSize: number, overlap: number, + rule: ChunkingRule, ): Promise => { const tree = await ensureAndParse(content, filePath); if (!tree) return []; @@ -162,7 +177,7 @@ const declarationChunk = async ( const bodyNode = getDeclarationBodyNode(targetNode); if (!bodyNode) return []; - const members = collectDeclarationUnits(bodyNode, label); + const members = collectDeclarationUnits(bodyNode, rule.groupFields); if (members.length === 0) return []; return chunkByUnits( @@ -174,8 +189,8 @@ const declarationChunk = async ( members, targetNode.startIndex, targetNode.endIndex, - false, - false, + rule.includePrefix, + rule.includeSuffix, ); }; @@ -237,14 +252,22 @@ const chunkByUnits = ( if (candidateEndOffset - chunkStartOffset > chunkSize) { const oversizedUnit = units[chunkStartUnitIdx]; + const oversizedStartOffset = + chunkStartUnitIdx === 0 && includeContainerPrefixOnFirstChunk + ? containerStartOffset + : oversizedUnit.startIndex; + const oversizedEndOffset = + chunkStartUnitIdx === units.length - 1 && includeContainerSuffixOnLastChunk + ? containerEndOffset + : oversizedUnit.endIndex; const oversizedLineRange = resolveChunkLines( lineOffsets, - oversizedUnit.startIndex, - oversizedUnit.endIndex, + oversizedStartOffset, + oversizedEndOffset, baseStartLine, ); const oversizedChunks = characterChunk( - content.slice(oversizedUnit.startIndex, oversizedUnit.endIndex), + content.slice(oversizedStartOffset, oversizedEndOffset), oversizedLineRange.startLine, oversizedLineRange.endLine, chunkSize, @@ -252,8 +275,8 @@ const chunkByUnits = ( ).map((chunk, offsetIdx) => ({ ...chunk, chunkIndex: chunks.length + offsetIdx, - startOffset: chunk.startOffset + oversizedUnit.startIndex, - endOffset: chunk.endOffset + oversizedUnit.startIndex, + startOffset: chunk.startOffset + oversizedStartOffset, + endOffset: chunk.endOffset + oversizedStartOffset, })); chunks.push(...oversizedChunks); chunkStartUnitIdx += 1; @@ -325,7 +348,7 @@ const getDeclarationBodyNode = (node: any): any | null => { const collectDeclarationUnits = ( bodyNode: any, - label: 'Class' | 'Interface', + groupFields: boolean, ): Array<{ startIndex: number; endIndex: number }> => { const members: Array<{ startIndex: number; endIndex: number; groupable: boolean }> = []; @@ -335,7 +358,7 @@ const collectDeclarationUnits = ( members.push({ startIndex: child.startIndex, endIndex: child.endIndex, - groupable: label === 'Class' && FIELD_LIKE_MEMBER_TYPES.has(child.type), + groupable: groupFields && FIELD_LIKE_MEMBER_TYPES.has(child.type), }); } diff --git a/gitnexus/src/core/embeddings/embedding-pipeline.ts b/gitnexus/src/core/embeddings/embedding-pipeline.ts index 302903f8b..be16789c2 100644 --- a/gitnexus/src/core/embeddings/embedding-pipeline.ts +++ b/gitnexus/src/core/embeddings/embedding-pipeline.ts @@ -30,6 +30,7 @@ import { DEFAULT_EMBEDDING_CONFIG, EMBEDDABLE_LABELS, isShortLabel, + LABEL_METHOD, LABELS_WITH_EXPORTED, STRUCTURAL_LABELS, collectBestChunks, @@ -43,6 +44,12 @@ import { import { loadVectorExtension } from '../lbug/lbug-adapter.js'; const isDev = process.env.NODE_ENV === 'development'; +/** + * Bump this when the embedding text template changes in a way that should + * invalidate existing vectors, such as metadata/header shape changes, + * structural container context changes, or preceding-context formatting rules. + */ +export const EMBEDDING_TEXT_VERSION = 'v2'; /** * Compute a stable content fingerprint for an embeddable node. @@ -57,12 +64,13 @@ export const contentHashForNode = ( // Hash must be deterministic across runs, so exclude methodNames/fieldNames // which are populated during the batch loop via AST extraction. // Using only node.content ensures the hash stays stable. + // NOTE: A change to extractStructuralNames behavior requires bumping EMBEDDING_TEXT_VERSION. const text = generateEmbeddingText( { ...node, methodNames: undefined, fieldNames: undefined }, node.content, config, ); - return createHash('sha1').update(text).digest('hex'); + return createHash('sha1').update(EMBEDDING_TEXT_VERSION).update('\n').update(text).digest('hex'); }; /** @@ -83,7 +91,7 @@ const queryEmbeddableNodes = async ( try { let query: string; - if (label === 'Method') { + if (label === LABEL_METHOD) { // Method has parameterCount and returnType query = ` MATCH (n:Method) @@ -115,7 +123,7 @@ const queryEmbeddableNodes = async ( const rows = await executeQuery(query); for (const row of rows) { - const hasExportedColumn = label === 'Method' || LABELS_WITH_EXPORTED.has(label); + const hasExportedColumn = label === LABEL_METHOD || LABELS_WITH_EXPORTED.has(label); allNodes.push({ id: row.id ?? row[0], name: row.name ?? row[1], @@ -126,7 +134,7 @@ const queryEmbeddableNodes = async ( endLine: row.endLine ?? row[6], isExported: hasExportedColumn ? (row.isExported ?? row[7]) : undefined, description: row.description ?? (hasExportedColumn ? row[8] : row[7]), - ...(label === 'Method' + ...(label === LABEL_METHOD ? { parameterCount: row.parameterCount ?? row[9], returnType: row.returnType ?? row[10], @@ -415,8 +423,15 @@ export const runEmbeddingPipeline = async ( } } + let prevTail = ''; for (const chunk of chunks) { - const text = generateEmbeddingText(node, chunk.text, finalConfig); + const text = generateEmbeddingText( + node, + chunk.text, + finalConfig, + chunk.chunkIndex, + prevTail, + ); allTexts.push(text); allUpdates.push({ nodeId: node.id, @@ -425,6 +440,7 @@ export const runEmbeddingPipeline = async ( endLine: chunk.endLine, contentHash: hash, }); + prevTail = overlap > 0 ? chunk.text.slice(-overlap) : ''; } } diff --git a/gitnexus/src/core/embeddings/text-generator.ts b/gitnexus/src/core/embeddings/text-generator.ts index 5b96f6b5e..74e90e9ce 100644 --- a/gitnexus/src/core/embeddings/text-generator.ts +++ b/gitnexus/src/core/embeddings/text-generator.ts @@ -10,7 +10,12 @@ */ import type { EmbeddableNode, EmbeddingConfig } from './types.js'; -import { DEFAULT_EMBEDDING_CONFIG, isShortLabel } from './types.js'; +import { + CHUNKING_RULES, + DEFAULT_EMBEDDING_CONFIG, + STRUCTURAL_TEXT_MODE_DECLARATION, + isShortLabel, +} from './types.js'; /** * Truncate description to max length at sentence/word boundary @@ -95,47 +100,62 @@ const generateCodeBodyText = ( node: EmbeddableNode, codeBody: string, config: Partial, + prevTail?: string, ): string => { const header = buildMetadataHeader(node, config); - const cleaned = cleanContent(codeBody); - return `${header}\n\n${cleaned}`; + const parts = [header]; + if (prevTail) { + parts.push(`[preceding context]: ...${cleanContent(prevTail)}`); + } + parts.push('', cleanContent(codeBody)); + return parts.join('\n'); }; -/** - * Generate embedding text for Class nodes - * Signature + properties + method name list only (no method bodies) - * Method/field names come from AST extractors via node.methodNames/node.fieldNames. - */ -const generateClassText = ( - node: EmbeddableNode, - codeBody: string, - config: Partial, -): string => { - return generateStructuralTypeText(node, codeBody, config); +const getCompactContainerContext = ( + cleanedContent: string, + declarationOnly: string, +): string | undefined => { + const source = declarationOnly || cleanedContent; + const nlIdx = source.indexOf('\n'); + const firstLine = (nlIdx === -1 ? source : source.substring(0, nlIdx)).trim(); + return firstLine ? `Container: ${firstLine}` : undefined; }; const generateStructuralTypeText = ( node: EmbeddableNode, codeBody: string, config: Partial, + chunkIndex?: number, + prevTail?: string, ): string => { const header = buildMetadataHeader(node, config); const parts: string[] = [header]; + const isFirstChunk = chunkIndex === undefined || chunkIndex === 0; + const cleanedContent = cleanContent(node.content); + const declarationOnly = extractDeclarationOnly(cleanedContent); + const compactContainerContext = getCompactContainerContext(cleanedContent, declarationOnly); - if (node.methodNames?.length) { + if (compactContainerContext) { + parts.push(compactContainerContext); + } + + if (prevTail) { + parts.push(`[preceding context]: ...${cleanContent(prevTail)}`); + } + + if (isFirstChunk && node.methodNames?.length) { parts.push(`Methods: ${node.methodNames.join(', ')}`); } - if (node.fieldNames?.length) { + if (isFirstChunk && node.fieldNames?.length) { parts.push(`Properties: ${node.fieldNames.join(', ')}`); } - const declarationOnly = extractDeclarationOnly(cleanContent(node.content)); - if (declarationOnly) { + if (isFirstChunk && declarationOnly) { parts.push('', declarationOnly); } const cleanedChunk = cleanContent(codeBody); - if (cleanedChunk && cleanedChunk !== cleanContent(node.content)) { + if (cleanedChunk && cleanedChunk !== cleanedContent) { parts.push('', cleanedChunk); } @@ -229,6 +249,8 @@ export const generateEmbeddingText = ( node: EmbeddableNode, codeBody: string, config: Partial = {}, + chunkIndex?: number, + prevTail?: string, ): string => { if (isShortLabel(node.label)) { const header = buildMetadataHeader(node, config); @@ -236,15 +258,12 @@ export const generateEmbeddingText = ( return `${header}\n\n${cleaned}`; } - if (node.label === 'Class') { - return generateClassText(node, codeBody, config); + const chunkingRule = CHUNKING_RULES[node.label]; + if (chunkingRule?.structuralTextMode === STRUCTURAL_TEXT_MODE_DECLARATION) { + return generateStructuralTypeText(node, codeBody, config, chunkIndex, prevTail); } - if (node.label === 'Interface') { - return generateStructuralTypeText(node, codeBody, config); - } - - return generateCodeBodyText(node, codeBody, config); + return generateCodeBodyText(node, codeBody, config, prevTail); }; /** diff --git a/gitnexus/src/core/embeddings/types.ts b/gitnexus/src/core/embeddings/types.ts index c24dcdf40..4156e9b64 100644 --- a/gitnexus/src/core/embeddings/types.ts +++ b/gitnexus/src/core/embeddings/types.ts @@ -4,35 +4,76 @@ * Type definitions for the embedding generation and semantic search system. */ +export const LABEL_FUNCTION = 'Function' as const; +export const LABEL_METHOD = 'Method' as const; +export const LABEL_CONSTRUCTOR = 'Constructor' as const; +export const LABEL_CLASS = 'Class' as const; +export const LABEL_INTERFACE = 'Interface' as const; +export const LABEL_STRUCT = 'Struct' as const; +export const LABEL_ENUM = 'Enum' as const; +export const LABEL_TRAIT = 'Trait' as const; +export const LABEL_IMPL = 'Impl' as const; +export const LABEL_MACRO = 'Macro' as const; +export const LABEL_NAMESPACE = 'Namespace' as const; +export const LABEL_TYPE_ALIAS = 'TypeAlias' as const; +export const LABEL_TYPEDEF = 'Typedef' as const; +export const LABEL_CONST = 'Const' as const; +export const LABEL_PROPERTY = 'Property' as const; +export const LABEL_RECORD = 'Record' as const; +export const LABEL_UNION = 'Union' as const; +export const LABEL_STATIC = 'Static' as const; +export const LABEL_VARIABLE = 'Variable' as const; +export const LABEL_CODE_ELEMENT = 'CodeElement' as const; + +export const CHUNK_MODE_AST_FUNCTION = 'ast-function' as const; +export const CHUNK_MODE_AST_DECLARATION = 'ast-declaration' as const; +// CHUNK_MODE_CHARACTER exists for type completeness but is a no-op in CHUNKING_RULES — +// omit the entry entirely to get character fallback via chunker.ts dispatch. +export const CHUNK_MODE_CHARACTER = 'character' as const; + +export const STRUCTURAL_TEXT_MODE_NONE = 'none' as const; +export const STRUCTURAL_TEXT_MODE_DECLARATION = 'declaration' as const; + +export interface ChunkingRule { + mode: + | typeof CHUNK_MODE_AST_FUNCTION + | typeof CHUNK_MODE_AST_DECLARATION + | typeof CHUNK_MODE_CHARACTER; + includePrefix: boolean; + includeSuffix: boolean; + groupFields: boolean; + structuralTextMode: typeof STRUCTURAL_TEXT_MODE_NONE | typeof STRUCTURAL_TEXT_MODE_DECLARATION; +} + /** * Node labels that need chunking (have code body, potentially long) */ export const CHUNKABLE_LABELS = [ - 'Function', - 'Method', - 'Constructor', - 'Class', - 'Interface', - 'Struct', - 'Enum', - 'Trait', - 'Impl', - 'Macro', - 'Namespace', + LABEL_FUNCTION, + LABEL_METHOD, + LABEL_CONSTRUCTOR, + LABEL_CLASS, + LABEL_INTERFACE, + LABEL_STRUCT, + LABEL_ENUM, + LABEL_TRAIT, + LABEL_IMPL, + LABEL_MACRO, + LABEL_NAMESPACE, ] as const; /** * Node labels that are short (no chunking needed, embed directly) */ export const SHORT_LABELS = [ - 'TypeAlias', - 'Typedef', - 'Const', - 'Property', - 'Record', - 'Union', - 'Static', - 'Variable', + LABEL_TYPE_ALIAS, + LABEL_TYPEDEF, + LABEL_CONST, + LABEL_PROPERTY, + LABEL_RECORD, + LABEL_UNION, + LABEL_STATIC, + LABEL_VARIABLE, ] as const; /** @@ -61,26 +102,78 @@ export const isShortLabel = (label: string): boolean => (SHORT_LABELS as readonly string[]).includes(label); /** - * Node labels that have structural names (methods/fields) extractable via AST + * Node labels that have structural names (methods/fields) extractable via AST. + * Only labels that consume methodNames/fieldNames in their embedding text should + * be listed here — extra entries trigger wasted AST parses with no effect on output. */ export const STRUCTURAL_LABELS: ReadonlySet = new Set([ - 'Class', - 'Struct', - 'Interface', - 'Enum', + LABEL_CLASS, + LABEL_STRUCT, + LABEL_INTERFACE, ]); /** * Node labels that have isExported column in their schema */ export const LABELS_WITH_EXPORTED = new Set([ - 'Function', - 'Class', - 'Interface', - 'Method', - 'CodeElement', + LABEL_FUNCTION, + LABEL_CLASS, + LABEL_INTERFACE, + LABEL_METHOD, + LABEL_CODE_ELEMENT, ]) as ReadonlySet; +/** + * Labels that need special chunking and/or structural text semantics. + * Any chunkable label omitted here intentionally falls back to characterChunk + * plus generateCodeBodyText (for example Enum/Trait/Impl/Macro/Namespace). + */ +type ChunkableLabel = (typeof CHUNKABLE_LABELS)[number]; +export const CHUNKING_RULES: Readonly>> = { + [LABEL_FUNCTION]: { + mode: CHUNK_MODE_AST_FUNCTION, + includePrefix: true, + includeSuffix: true, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_NONE, + }, + [LABEL_METHOD]: { + mode: CHUNK_MODE_AST_FUNCTION, + includePrefix: true, + includeSuffix: true, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_NONE, + }, + [LABEL_CONSTRUCTOR]: { + mode: CHUNK_MODE_AST_FUNCTION, + includePrefix: true, + includeSuffix: true, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_NONE, + }, + [LABEL_CLASS]: { + mode: CHUNK_MODE_AST_DECLARATION, + includePrefix: true, + includeSuffix: false, + groupFields: true, + structuralTextMode: STRUCTURAL_TEXT_MODE_DECLARATION, + }, + [LABEL_INTERFACE]: { + mode: CHUNK_MODE_AST_DECLARATION, + includePrefix: true, + includeSuffix: false, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_DECLARATION, + }, + [LABEL_STRUCT]: { + mode: CHUNK_MODE_AST_DECLARATION, + includePrefix: true, + includeSuffix: false, + groupFields: true, + structuralTextMode: STRUCTURAL_TEXT_MODE_DECLARATION, + }, +}; + /** * Embedding pipeline phases */ diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts new file mode 100644 index 000000000..a80336b26 --- /dev/null +++ b/gitnexus/src/core/group/cross-impact.ts @@ -0,0 +1,551 @@ +/** + * Cross-repo impact (Phase 1 local walk + Phase 2 bridge fan-out). + * All bridge Cypher for this feature lives in this module. + */ + +import fsp from 'node:fs/promises'; +import path from 'node:path'; +import type { + BridgeHandle, + ContractType, + CrossRepoImpact, + GroupConfig, + GroupImpactResult, + MatchType, + OutOfScopeLink, +} from './types.js'; +import type { GroupRepoHandle, GroupToolPort } from './service.js'; +import { loadGroupConfig } from './config-parser.js'; +import { + fileMatchesServicePrefix, + normalizeServicePrefix, + repoInSubgroup, +} from './group-path-utils.js'; +import { getGroupDir } from './storage.js'; +import { closeBridgeDb, openBridgeDbReadOnly, queryBridge, readBridgeMeta } from './bridge-db.js'; +import { BRIDGE_SCHEMA_VERSION } from './bridge-schema.js'; + +/** Cross-boundary hops beyond this value are clamped (multi-hop reserved for future work). */ +export const MAX_SUPPORTED_CROSS_DEPTH = 1; + +/** Default wall-clock budget for the Phase 1 `impact` leg when callers omit `timeoutMs`. */ +export const DEFAULT_LOCAL_IMPACT_TIMEOUT_MS = 30_000; + +const CY_NEIGHBORS_UPSTREAM = ` +MATCH (consumer:Contract)-[l:ContractLink]->(provider:Contract) +WHERE provider.repo = $localRepo + AND provider.symbolUid IN $uids + AND provider.role = 'provider' +RETURN consumer.repo AS neighborRepo, + consumer.symbolUid AS neighborUid, + consumer.filePath AS neighborFilePath, + l.matchType AS matchType, + l.confidence AS confidence, + l.contractId AS contractId, + consumer.type AS contractType +`; + +const CY_NEIGHBORS_DOWNSTREAM = ` +MATCH (consumer:Contract)-[l:ContractLink]->(provider:Contract) +WHERE consumer.repo = $localRepo + AND consumer.symbolUid IN $uids + AND consumer.role = 'consumer' +RETURN provider.repo AS neighborRepo, + provider.symbolUid AS neighborUid, + provider.filePath AS neighborFilePath, + l.matchType AS matchType, + l.confidence AS confidence, + l.contractId AS contractId, + provider.type AS contractType +`; + +type BridgeNeighborRow = { + neighborRepo: string; + neighborUid: string; + neighborFilePath?: string; + matchType: string; + confidence: number; + contractId: string; + contractType: string; +}; + +export interface RunGroupImpactDeps { + port: GroupToolPort; + gitnexusDir: string; +} + +function parseDirection(raw: unknown): 'upstream' | 'downstream' | null { + if (raw === 'upstream' || raw === 'downstream') return raw; + return null; +} + +function clampCrossDepth(raw: unknown): { depth: number; warning?: string } { + const n = typeof raw === 'number' && Number.isFinite(raw) ? Math.floor(raw) : 1; + const d = n < 1 ? 1 : n; + if (d > MAX_SUPPORTED_CROSS_DEPTH) { + return { + depth: MAX_SUPPORTED_CROSS_DEPTH, + warning: `crossDepth was ${d}; multi-hop cross-boundary traversal beyond ${MAX_SUPPORTED_CROSS_DEPTH} is not implemented yet. Using crossDepth ${MAX_SUPPORTED_CROSS_DEPTH}.`, + }; + } + return { depth: d }; +} + +export function validateGroupImpactParams(params: Record): + | { + ok: true; + name: string; + repoPath: string; + target: string; + direction: 'upstream' | 'downstream'; + maxDepth: number; + crossDepth: number; + crossDepthWarning?: string; + relationTypes?: string[]; + includeTests: boolean; + minConfidence: number; + service?: string; + subgroup?: string; + timeoutMs: number; + } + | { ok: false; error: string } { + const name = String(params.name ?? '').trim(); + const repoPath = String(params.repo ?? '').trim(); + const target = String(params.target ?? '').trim(); + if (!name) return { ok: false, error: 'name is required' }; + if (!repoPath) + return { ok: false, error: 'repo is required (group repo path, e.g. app/backend)' }; + if (!target) return { ok: false, error: 'target is required' }; + if ( + params.service !== undefined && + params.service !== null && + String(params.service).trim() === '' + ) { + return { ok: false, error: 'service must not be an empty string' }; + } + const direction = parseDirection(params.direction); + if (!direction) return { ok: false, error: 'direction must be upstream or downstream' }; + + let maxDepth = typeof params.maxDepth === 'number' && params.maxDepth > 0 ? params.maxDepth : 3; + if (maxDepth > 32) maxDepth = 32; + + const { depth: crossDepth, warning: crossDepthWarning } = clampCrossDepth(params.crossDepth); + + const relationTypes = Array.isArray(params.relationTypes) + ? params.relationTypes.filter((t): t is string => typeof t === 'string') + : undefined; + + const includeTests = Boolean(params.includeTests); + let minConfidence = typeof params.minConfidence === 'number' ? params.minConfidence : 0; + if (minConfidence < 0) minConfidence = 0; + if (minConfidence > 1) minConfidence = 1; + + const service = normalizeServicePrefix(params.service); + const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; + + let timeoutMs = + typeof params.timeoutMs === 'number' && params.timeoutMs > 0 + ? params.timeoutMs + : typeof params.timeout === 'number' && params.timeout > 0 + ? params.timeout + : DEFAULT_LOCAL_IMPACT_TIMEOUT_MS; + if (timeoutMs > 3_600_000) timeoutMs = 3_600_000; + + return { + ok: true, + name, + repoPath, + target, + direction, + maxDepth, + crossDepth, + crossDepthWarning, + relationTypes, + includeTests, + minConfidence, + service, + subgroup, + timeoutMs, + }; +} + +async function resolveGroupRepo( + port: GroupToolPort, + config: GroupConfig, + repoPath: string, +): Promise { + const registryName = config.repos[repoPath]; + if (!registryName) { + return { error: `Unknown repo path "${repoPath}" in this group.` }; + } + try { + return await port.resolveRepo(registryName); + } catch (e) { + return { error: e instanceof Error ? e.message : String(e) }; + } +} + +async function safeLocalImpact( + port: GroupToolPort, + repo: GroupRepoHandle, + impactParams: Parameters[1], + timeoutMs: number, +): Promise<{ value: unknown; timedOut: boolean }> { + let timer: ReturnType | undefined; + const impactP = port.impact(repo, impactParams).catch((err) => ({ + error: err instanceof Error ? err.message : String(err), + })); + const timeoutP = new Promise<'timeout'>((resolve) => { + timer = setTimeout(() => resolve('timeout'), timeoutMs); + }); + const won = await Promise.race([ + impactP.then((v) => ({ tag: 'impact' as const, v })), + timeoutP.then(() => ({ tag: 'timeout' as const })), + ]); + if (timer !== undefined) clearTimeout(timer); + if (won.tag === 'timeout') { + return { + value: { error: 'Local impact timed out', partial: true }, + timedOut: true, + }; + } + return { value: won.v, timedOut: false }; +} + +export function collectImpactSymbolUids( + local: unknown, + servicePrefix: string | undefined, +): { uids: string[]; targetFilePath?: string } { + const uids = new Set(); + let targetFilePath: string | undefined; + const obj = local as Record | null; + if (!obj || typeof obj !== 'object') return { uids: [], targetFilePath }; + + const target = obj.target as { id?: string; filePath?: string } | undefined; + if (target?.id) { + targetFilePath = typeof target.filePath === 'string' ? target.filePath : undefined; + if (fileMatchesServicePrefix(targetFilePath, servicePrefix)) { + uids.add(String(target.id)); + } + } + + const byDepth = obj.byDepth as Record | undefined; + if (byDepth && typeof byDepth === 'object') { + for (const items of Object.values(byDepth)) { + if (!Array.isArray(items)) continue; + for (const it of items) { + const row = it as { id?: string; filePath?: string }; + if (row?.id && fileMatchesServicePrefix(row.filePath, servicePrefix)) { + uids.add(String(row.id)); + } + } + } + } + return { uids: [...uids], targetFilePath }; +} + +function extractProcessNames(impact: unknown): string[] { + const o = impact as { affected_processes?: Array<{ name?: string }> }; + if (!o?.affected_processes) return []; + return o.affected_processes.map((p) => String(p.name ?? '')).filter(Boolean); +} + +function mergeRisk(localRisk: string, cross: CrossRepoImpact[]): string { + const highConf = cross.some((c) => c.contract.confidence >= 0.85); + if (localRisk === 'CRITICAL') return 'CRITICAL'; + if (cross.length >= 3) return 'CRITICAL'; + if (highConf) return 'HIGH'; + if (cross.length > 0 && (localRisk === 'LOW' || localRisk === 'UNKNOWN')) return 'MEDIUM'; + return localRisk; +} + +async function ensureBridgeReady( + groupDir: string, +): Promise<{ handle: BridgeHandle } | { error: string }> { + const meta = await readBridgeMeta(groupDir); + if (meta.version > 0 && meta.version !== BRIDGE_SCHEMA_VERSION) { + return { + error: `Bridge schema version mismatch (meta.json has ${meta.version}, expected ${BRIDGE_SCHEMA_VERSION}). Run gitnexus group sync for this group.`, + }; + } + const dbPath = path.join(groupDir, 'bridge.lbug'); + try { + await fsp.access(dbPath); + } catch { + return { + error: `No bridge.lbug in this group directory. Run gitnexus group sync (schema ${BRIDGE_SCHEMA_VERSION}).`, + }; + } + const handle = await openBridgeDbReadOnly(groupDir); + if (!handle) { + return { + error: `Could not open bridge.lbug read-only (schema ${BRIDGE_SCHEMA_VERSION}). Run gitnexus group sync.`, + }; + } + return { handle }; +} + +function rowToNeighbor(r: Record): BridgeNeighborRow | null { + const neighborRepo = String(r.neighborRepo ?? r[0] ?? ''); + const neighborUid = String(r.neighborUid ?? r[1] ?? ''); + if (!neighborRepo || !neighborUid) return null; + return { + neighborRepo, + neighborUid, + neighborFilePath: + r.neighborFilePath !== undefined ? String(r.neighborFilePath) : String(r[2] ?? ''), + matchType: String(r.matchType ?? r[3] ?? 'exact'), + confidence: Number(r.confidence ?? r[4] ?? 0), + contractId: String(r.contractId ?? r[5] ?? ''), + contractType: String(r.contractType ?? r[6] ?? 'custom'), + }; +} + +export async function runGroupImpact( + deps: RunGroupImpactDeps, + params: Record, +): Promise { + const parsed = validateGroupImpactParams(params); + if (parsed.ok === false) return { error: parsed.error }; + + const { + name, + repoPath, + target, + direction, + maxDepth, + crossDepth: _crossDepth, + crossDepthWarning, + relationTypes, + includeTests, + minConfidence, + service: servicePrefix, + subgroup, + timeoutMs, + } = parsed; + + const groupDir = getGroupDir(deps.gitnexusDir, name); + let config: GroupConfig; + try { + config = await loadGroupConfig(groupDir); + } catch (e) { + return { error: e instanceof Error ? e.message : String(e) }; + } + + const resolved = await resolveGroupRepo(deps.port, config, repoPath); + if ('error' in resolved) return { error: resolved.error }; + + const impactParams: Parameters[1] = { + target, + direction, + maxDepth, + relationTypes: relationTypes && relationTypes.length > 0 ? relationTypes : undefined, + includeTests, + minConfidence, + }; + + // Single shared deadline for Phase 1 (local walk) + Phase 2 (bridge fan-out). + // Phase 1 still gets the full budget; Phase 2 only uses whatever wall-clock + // time is left, so total work cannot exceed `timeoutMs`. + const deadline = Date.now() + Math.max(0, timeoutMs); + + const { value: local, timedOut: localTimedOut } = await safeLocalImpact( + deps.port, + resolved, + impactParams, + timeoutMs, + ); + + if (localTimedOut) { + const base = local as Record; + return { + local, + group: name, + cross: [], + outOfScope: [], + truncated: true, + truncatedRepos: [], + summary: { + direct: 0, + processes_affected: 0, + modules_affected: 0, + cross_repo_hits: 0, + }, + risk: 'UNKNOWN', + timeoutMs, + truncationReason: 'timeout', + crossDepthWarning, + }; + } + + const localObj = local as Record | null; + if (localObj?.error && typeof localObj.error === 'string') { + // Fail closed: the local-impact phase errored (missing symbol, graph-load + // failure, thrown exception wrapped by safeLocalImpact, or port-returned + // `{ error }`). Do NOT wrap it into a zero-hit success payload — callers + // branch on top-level `error`, and a blast-radius tool reporting "no + // impact" on the failure path is a false negative on a safety-critical + // signal. Bubble the error so consumers treat it as a failure. + return { error: `Local impact failed for ${repoPath}: ${localObj.error}` }; + } + + if (servicePrefix) { + const tf = (localObj?.target as { filePath?: string } | undefined)?.filePath; + if (!fileMatchesServicePrefix(tf, servicePrefix)) { + return { + local: {}, + group: name, + cross: [], + outOfScope: [], + truncated: false, + truncatedRepos: [], + summary: { + direct: 0, + processes_affected: 0, + modules_affected: 0, + cross_repo_hits: 0, + }, + risk: 'LOW', + timeoutMs, + crossDepthWarning, + }; + } + } + + const { uids } = collectImpactSymbolUids(local, servicePrefix); + if (uids.length === 0) { + const s = (local as { summary?: Record })?.summary || {}; + return { + local, + group: name, + cross: [], + outOfScope: [], + truncated: Boolean((local as { partial?: boolean }).partial), + truncatedRepos: [], + summary: { + direct: s.direct ?? 0, + processes_affected: s.processes_affected ?? 0, + modules_affected: s.modules_affected ?? 0, + cross_repo_hits: 0, + }, + risk: String((local as { risk?: string }).risk ?? 'LOW'), + timeoutMs, + truncationReason: (local as { partial?: boolean }).partial ? 'partial' : undefined, + crossDepthWarning, + }; + } + + const bridgePrep = await ensureBridgeReady(groupDir); + if ('error' in bridgePrep) return { error: bridgePrep.error }; + + const handle = bridgePrep.handle; + const cross: CrossRepoImpact[] = []; + const outOfScope: OutOfScopeLink[] = []; + const truncatedRepos: string[] = []; + + try { + const cypher = direction === 'upstream' ? CY_NEIGHBORS_UPSTREAM : CY_NEIGHBORS_DOWNSTREAM; + const rows = await queryBridge>(handle, cypher, { + localRepo: repoPath, + uids, + }); + + const neighbors: BridgeNeighborRow[] = []; + for (const raw of rows) { + const n = rowToNeighbor(raw); + if (n) neighbors.push(n); + } + neighbors.sort((a, b) => b.confidence - a.confidence); + + const seen = new Set(); + + for (const n of neighbors) { + if (servicePrefix && !fileMatchesServicePrefix(n.neighborFilePath, servicePrefix)) { + continue; + } + if (!repoInSubgroup(n.neighborRepo, subgroup)) { + // CrossLink convention: consumer -> provider + outOfScope.push({ + from: direction === 'upstream' ? n.neighborRepo : repoPath, + to: direction === 'upstream' ? repoPath : n.neighborRepo, + contractId: n.contractId, + confidence: n.confidence, + }); + continue; + } + + const key = `${n.neighborRepo}\0${n.neighborUid}\0${n.contractId}`; + if (seen.has(key)) continue; + seen.add(key); + + if (Date.now() > deadline) { + truncatedRepos.push(n.neighborRepo); + continue; + } + + const regName = config.repos[n.neighborRepo]; + if (!regName) continue; + + let neighborHandle: GroupRepoHandle; + try { + neighborHandle = await deps.port.resolveRepo(regName); + } catch { + truncatedRepos.push(n.neighborRepo); + continue; + } + + const fan = await deps.port.impactByUid(neighborHandle.id, n.neighborUid, direction, { + maxDepth, + relationTypes: relationTypes ?? [], + minConfidence, + includeTests, + }); + if (fan == null) { + truncatedRepos.push(n.neighborRepo); + continue; + } + + cross.push({ + repo: regName, + repo_path: n.neighborRepo, + contract: { + id: n.contractId, + type: n.contractType as ContractType, + match_type: (n.matchType as MatchType) || 'exact', + confidence: n.confidence, + }, + by_depth: ((fan as { byDepth?: unknown }).byDepth ?? {}) as Record, + affected_processes: extractProcessNames(fan), + }); + } + } finally { + await closeBridgeDb(handle); + } + + const localSum = (local as { summary?: Record })?.summary || {}; + const localRisk = String((local as { risk?: string }).risk ?? 'LOW'); + const localPartial = Boolean((local as { partial?: boolean }).partial); + const truncated = truncatedRepos.length > 0 || localPartial; + + const result: GroupImpactResult = { + local, + group: name, + cross, + outOfScope, + truncated, + truncatedRepos: [...new Set(truncatedRepos)], + summary: { + direct: localSum.direct ?? 0, + processes_affected: localSum.processes_affected ?? 0, + modules_affected: localSum.modules_affected ?? 0, + cross_repo_hits: cross.length, + }, + risk: mergeRisk(localRisk, cross), + timeoutMs, + truncationReason: truncated ? 'partial' : undefined, + crossDepthWarning, + }; + return result; +} + +export { normalizeServicePrefix, fileMatchesServicePrefix } from './group-path-utils.js'; diff --git a/gitnexus/src/core/group/extractors/http-patterns/php.ts b/gitnexus/src/core/group/extractors/http-patterns/php.ts index ae91c141b..c1c40a09c 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/php.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/php.ts @@ -3,33 +3,92 @@ import { compilePatterns, runCompiledPatterns, unquoteLiteral, + type CompiledPatterns, type LanguagePatterns, + type PatternSpec, } from '../tree-sitter-scanner.js'; import type { HttpDetection, HttpLanguagePlugin } from './types.js'; /** - * PHP HTTP plugin — Laravel `Route::get/post/...` declarations. + * PHP HTTP plugin. + * + * Providers: + * - Laravel `Route::get/post/...` + * + * Consumers (string-literal URLs only): + * - Laravel HTTP client: `Http::get/post/put/delete/patch($url)` + * - Guzzle / generic object method: `$client->get/post/...($url)` + * - `file_get_contents($url)` * * The pipeline already uses `PHP.php_only` for ingesting plain `.php` * files (see `core/tree-sitter/parser-loader.ts`), and we do the same * here so Laravel route files are parsed with the right grammar dialect. + * + * Scope notes: consumer patterns match string literals only. URLs built + * via binary concatenation (`$base . '/path'`), `sprintf`, or config + * lookup (`config('services.foo.base').'/path'`) are intentionally left + * for a follow-up — they require constant-folding the surrounding + * scope to be meaningful. */ -const LARAVEL_PATTERNS = compilePatterns({ - name: 'php-laravel', - language: PHP.php_only, - patterns: [ - { - meta: {}, - query: ` - (scoped_call_expression - scope: (name) @scope (#eq? @scope "Route") - name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") - arguments: (arguments . (argument (string) @path))) - `, - }, - ], -} satisfies LanguagePatterns>); +const LARAVEL_ROUTE_SPEC: PatternSpec> = { + meta: {}, + query: ` + (scoped_call_expression + scope: (name) @scope (#eq? @scope "Route") + name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") + arguments: (arguments . (argument (string) @path))) + `, +}; + +const HTTP_FACADE_SPEC: PatternSpec> = { + meta: {}, + query: ` + (scoped_call_expression + scope: (name) @scope (#eq? @scope "Http") + name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") + arguments: (arguments . (argument (string) @path))) + `, +}; + +const GUZZLE_MEMBER_SPEC: PatternSpec> = { + meta: {}, + query: ` + (member_call_expression + name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") + arguments: (arguments . (argument (string) @path))) + `, +}; + +const FILE_GET_CONTENTS_SPEC: PatternSpec> = { + meta: {}, + query: ` + (function_call_expression + function: (name) @fn (#eq? @fn "file_get_contents") + arguments: (arguments . (argument (string) @path))) + `, +}; + +interface PhpPatternBundle { + laravelRoute: CompiledPatterns>; + httpFacade: CompiledPatterns>; + guzzleMember: CompiledPatterns>; + fileGetContents: CompiledPatterns>; +} + +const mk = (spec: PatternSpec>, suffix: string) => + compilePatterns({ + name: `php-${suffix}`, + language: PHP.php_only, + patterns: [spec], + } satisfies LanguagePatterns>); + +const PHP_PATTERNS: PhpPatternBundle = { + laravelRoute: mk(LARAVEL_ROUTE_SPEC, 'laravel-route'), + httpFacade: mk(HTTP_FACADE_SPEC, 'http-facade'), + guzzleMember: mk(GUZZLE_MEMBER_SPEC, 'guzzle-member'), + fileGetContents: mk(FILE_GET_CONTENTS_SPEC, 'file-get-contents'), +}; /** * Extract the inner text of a PHP `string` node. The tree-sitter-php @@ -39,11 +98,8 @@ const LARAVEL_PATTERNS = compilePatterns({ * child nodes. */ function phpStringText(node: import('tree-sitter').SyntaxNode): string | null { - // Most single-quoted strings expose their inner content through the - // full node text (including quotes), which unquoteLiteral strips. const direct = unquoteLiteral(node.text); if (direct !== null && direct !== node.text) return direct; - // Fall back to child string_content / string_value node if present. for (const child of node.children) { if (child.type === 'string_content' || child.type === 'string_value') { return child.text; @@ -52,13 +108,32 @@ function phpStringText(node: import('tree-sitter').SyntaxNode): string | null { return direct; } +/** + * HTTP client helpers (`Http::`, Guzzle) are almost always called with + * a path relative to a configured base URL, or a full URL. File paths + * are rare. Accept both relative (`/api/...`) and absolute (`http(s)://`). + */ +function isHttpClientPath(path: string): boolean { + return path.startsWith('/') || path.startsWith('http://') || path.startsWith('https://'); +} + +/** + * `file_get_contents` is used for both HTTP and filesystem reads. Only + * emit a consumer contract when the URL is an absolute HTTP(S) URL to + * avoid false positives for local file paths and stream wrappers + * (`php://input`, `file://`, `data:`, ...). + */ +function isHttpUrlLiteral(path: string): boolean { + return path.startsWith('http://') || path.startsWith('https://'); +} + export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { name: 'php-http', language: PHP.php_only, scan(tree) { const out: HttpDetection[] = []; - for (const match of runCompiledPatterns(LARAVEL_PATTERNS, tree)) { + for (const match of runCompiledPatterns(PHP_PATTERNS.laravelRoute, tree)) { const methodNode = match.captures.method; const pathNode = match.captures.path; if (!methodNode || !pathNode) continue; @@ -74,6 +149,53 @@ export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { }); } + for (const match of runCompiledPatterns(PHP_PATTERNS.httpFacade, tree)) { + const methodNode = match.captures.method; + const pathNode = match.captures.path; + if (!methodNode || !pathNode) continue; + const path = phpStringText(pathNode); + if (path === null || !isHttpClientPath(path)) continue; + out.push({ + role: 'consumer', + framework: 'laravel-http', + method: methodNode.text.toUpperCase(), + path, + name: null, + confidence: 0.7, + }); + } + + for (const match of runCompiledPatterns(PHP_PATTERNS.guzzleMember, tree)) { + const methodNode = match.captures.method; + const pathNode = match.captures.path; + if (!methodNode || !pathNode) continue; + const path = phpStringText(pathNode); + if (path === null || !isHttpClientPath(path)) continue; + out.push({ + role: 'consumer', + framework: 'guzzle', + method: methodNode.text.toUpperCase(), + path, + name: null, + confidence: 0.7, + }); + } + + for (const match of runCompiledPatterns(PHP_PATTERNS.fileGetContents, tree)) { + const pathNode = match.captures.path; + if (!pathNode) continue; + const path = phpStringText(pathNode); + if (path === null || !isHttpUrlLiteral(path)) continue; + out.push({ + role: 'consumer', + framework: 'file-get-contents', + method: 'GET', + path, + name: null, + confidence: 0.7, + }); + } + return out; }, }; diff --git a/gitnexus/src/core/group/group-path-utils.ts b/gitnexus/src/core/group/group-path-utils.ts new file mode 100644 index 000000000..ab6a1effb --- /dev/null +++ b/gitnexus/src/core/group/group-path-utils.ts @@ -0,0 +1,42 @@ +/** + * Shared service-path normalization for group tools (`service` monorepo filter) + * and subgroup membership checks. + * + * Inputs may originate from tree-sitter, the OS file API, or user-supplied + * MCP arguments, so both `\` and `/` separators are accepted. Internally we + * normalize to POSIX-style `/` for case-sensitive segment comparisons. + */ + +function toPosix(p: string): string { + return p.replace(/\\/g, '/'); +} + +export function normalizeServicePrefix(service: unknown): string | undefined { + if (service === undefined || service === null) return undefined; + const s = toPosix(String(service)).trim().replace(/\/+$/, ''); + return s.length > 0 ? s : undefined; +} + +export function fileMatchesServicePrefix( + filePath: string | undefined, + prefix: string | undefined, +): boolean { + if (!prefix) return true; + if (!filePath) return false; + const normalized = toPosix(filePath); + return normalized === prefix || normalized.startsWith(`${prefix}/`); +} + +/** + * True if `repoPath` is at or beneath `subgroup` (member-path prefix in + * `group.yaml`). Empty / missing `subgroup` matches every repo. + * + * @param exact When set, requires an exact equality match (no descendant repos). + */ +export function repoInSubgroup(repoPath: string, subgroup?: string, exact?: boolean): boolean { + if (!subgroup?.trim()) return true; + const s = toPosix(subgroup).replace(/\/+$/, ''); + const r = toPosix(repoPath); + if (exact) return r === s; + return r === s || r.startsWith(`${s}/`); +} diff --git a/gitnexus/src/core/group/resolve-at-member.ts b/gitnexus/src/core/group/resolve-at-member.ts new file mode 100644 index 000000000..e36506c38 --- /dev/null +++ b/gitnexus/src/core/group/resolve-at-member.ts @@ -0,0 +1,34 @@ +/** + * Map MCP/CLI `@groupName` or `@groupName/memberPath` to a concrete member path in group.yaml. + */ + +import { loadGroupConfig } from './config-parser.js'; +import { getDefaultGitnexusDir, getGroupDir } from './storage.js'; + +export async function resolveAtGroupMemberRepoPath( + groupName: string, + explicitMemberPath: string | undefined, +): Promise<{ ok: true; repoPath: string } | { ok: false; error: string }> { + const trimmed = groupName.trim(); + if (!trimmed) return { ok: false, error: 'Group name is empty.' }; + try { + const groupDir = getGroupDir(getDefaultGitnexusDir(), trimmed); + const config = await loadGroupConfig(groupDir); + const keys = Object.keys(config.repos).sort((a, b) => a.localeCompare(b)); + if (keys.length === 0) { + return { ok: false, error: `Group "${trimmed}" has no repos in group.yaml.` }; + } + if (explicitMemberPath !== undefined && explicitMemberPath !== '') { + if (!(explicitMemberPath in config.repos)) { + return { + ok: false, + error: `Unknown member path "${explicitMemberPath}" in group "${trimmed}". Known paths: ${keys.join(', ')}`, + }; + } + return { ok: true, repoPath: explicitMemberPath }; + } + return { ok: true, repoPath: keys[0]! }; + } catch (e) { + return { ok: false, error: e instanceof Error ? e.message : String(e) }; + } +} diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index 1530cd6dd..afbb66e0e 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -3,10 +3,24 @@ * DB access is injected via GroupToolPort so this module stays free of LocalBackend private API. */ +import fsp from 'node:fs/promises'; +import path from 'node:path'; import { checkStaleness } from '../git-staleness.js'; import { loadGroupConfig } from './config-parser.js'; +import { + fileMatchesServicePrefix, + normalizeServicePrefix, + repoInSubgroup, +} from './group-path-utils.js'; import { getDefaultGitnexusDir, getGroupDir, listGroups, readContractRegistry } from './storage.js'; import { syncGroup } from './sync.js'; +import type { + ContractRegistry, + CrossLink, + GroupConfig, + GroupContextResult, + StoredContract, +} from './types.js'; export interface GroupRepoHandle { id: string; @@ -52,12 +66,149 @@ export interface GroupToolPort { includeTests: boolean; }, ): Promise; + context( + repo: GroupRepoHandle, + params: { + name?: string; + uid?: string; + file_path?: string; + include_content?: boolean; + }, + ): Promise; } -function repoInSubgroup(repoPath: string, subgroup?: string): boolean { - if (!subgroup?.trim()) return true; - const s = subgroup.replace(/\/+$/, ''); - return repoPath === s || repoPath.startsWith(`${s}/`); +function isStoredContract(raw: unknown): raw is StoredContract { + if (!raw || typeof raw !== 'object') return false; + const o = raw as Record; + return ( + typeof o.contractId === 'string' && + typeof o.type === 'string' && + typeof o.repo === 'string' && + typeof o.role === 'string' && + (o.role === 'provider' || o.role === 'consumer') && + typeof o.symbolUid === 'string' && + typeof o.symbolName === 'string' && + typeof o.confidence === 'number' && + o.meta !== undefined && + typeof o.meta === 'object' && + o.meta !== null && + o.symbolRef !== undefined && + typeof o.symbolRef === 'object' && + o.symbolRef !== null && + typeof (o.symbolRef as Record).filePath === 'string' && + typeof (o.symbolRef as Record).name === 'string' + ); +} + +function filterQueryByServicePrefix( + queryResult: { + processes?: Array>; + process_symbols?: Array>; + }, + servicePrefix: string, +): { processes: Array>; process_symbols: Array> } { + const symbols = (queryResult.process_symbols || []).filter((s) => + fileMatchesServicePrefix( + typeof s.filePath === 'string' ? s.filePath : undefined, + servicePrefix, + ), + ); + const allowed = new Set( + symbols.map((s) => String((s as { process_id?: string }).process_id ?? '')).filter(Boolean), + ); + const processes = (queryResult.processes || []).filter((p) => allowed.has(String(p.id))); + return { processes, process_symbols: symbols }; +} + +function isCrossLink(raw: unknown): raw is CrossLink { + if (!raw || typeof raw !== 'object') return false; + const o = raw as Record; + const from = o.from as Record | undefined; + const to = o.to as Record | undefined; + if (!from || !to) return false; + if (typeof from.repo !== 'string' || typeof to.repo !== 'string') return false; + return typeof o.contractId === 'string' && typeof o.type === 'string'; +} + +async function loadContractRegistryResilient( + groupDir: string, +): Promise< + { ok: true; registry: ContractRegistry; skippedCorrupt: number } | { ok: false; error: string } +> { + const filePath = path.join(groupDir, 'contracts.json'); + let raw: string; + try { + raw = await fsp.readFile(filePath, 'utf-8'); + } catch (e) { + if ((e as NodeJS.ErrnoException).code === 'ENOENT') { + return { ok: false, error: `No contracts.json for this group. Run group_sync first.` }; + } + return { ok: false, error: e instanceof Error ? e.message : String(e) }; + } + + let root: unknown; + try { + root = JSON.parse(raw); + } catch { + return { ok: false, error: 'contracts.json is not valid JSON' }; + } + + if (!root || typeof root !== 'object' || Array.isArray(root)) { + return { ok: false, error: 'contracts.json has an invalid root object' }; + } + + const base = root as Record; + const contractsRaw = base.contracts; + const crossRaw = base.crossLinks; + let skippedCorrupt = 0; + + const contracts: StoredContract[] = []; + if (Array.isArray(contractsRaw)) { + for (const row of contractsRaw) { + try { + if (isStoredContract(row)) { + contracts.push(row); + } else { + skippedCorrupt++; + console.warn('[group] skipping corrupt contract row in contracts.json'); + } + } catch { + skippedCorrupt++; + console.warn('[group] skipping corrupt contract row in contracts.json'); + } + } + } + + const crossLinks: CrossLink[] = []; + if (Array.isArray(crossRaw)) { + for (const row of crossRaw) { + try { + if (isCrossLink(row)) { + crossLinks.push(row); + } else { + skippedCorrupt++; + console.warn('[group] skipping corrupt crossLinks row in contracts.json'); + } + } catch { + skippedCorrupt++; + console.warn('[group] skipping corrupt crossLinks row in contracts.json'); + } + } + } + + const registry: ContractRegistry = { + version: typeof base.version === 'number' ? base.version : 0, + generatedAt: typeof base.generatedAt === 'string' ? base.generatedAt : '', + repoSnapshots: + base.repoSnapshots && typeof base.repoSnapshots === 'object' && base.repoSnapshots !== null + ? (base.repoSnapshots as Record) + : {}, + missingRepos: Array.isArray(base.missingRepos) ? (base.missingRepos as string[]) : [], + contracts, + crossLinks, + }; + + return { ok: true, registry, skippedCorrupt }; } export class GroupService { @@ -103,10 +254,14 @@ export class GroupService { const name = String(params.name ?? '').trim(); if (!name) return { error: 'name is required' }; const groupDir = getGroupDir(getDefaultGitnexusDir(), name); - const registry = await readContractRegistry(groupDir); - if (!registry) { - return { error: `No contracts.json for group "${name}". Run group_sync first.` }; + const loaded = await loadContractRegistryResilient(groupDir); + if (loaded.ok === false) { + if (loaded.error.includes('No contracts.json')) { + return { error: `No contracts.json for group "${name}". Run group_sync first.` }; + } + return { error: loaded.error }; } + const { registry, skippedCorrupt } = loaded; let contracts = registry.contracts; if (params.type) contracts = contracts.filter((c) => c.type === params.type); if (params.repo) contracts = contracts.filter((c) => c.repo === params.repo); @@ -119,41 +274,151 @@ export class GroupService { ); contracts = contracts.filter((c) => !matchedIds.has(`${c.repo}::${c.contractId}`)); } - return { contracts, crossLinks: registry.crossLinks }; + const out: Record = { contracts, crossLinks: registry.crossLinks }; + if (skippedCorrupt > 0) out.skippedCorrupt = skippedCorrupt; + return out; + } + + async groupImpact(params: Record): Promise { + const { runGroupImpact } = await import('./cross-impact.js'); + return runGroupImpact({ port: this.port, gitnexusDir: getDefaultGitnexusDir() }, params); + } + + async groupContext(params: Record): Promise { + const name = String(params.name ?? '').trim(); + const target = typeof params.target === 'string' ? params.target.trim() : ''; + const uid = typeof params.uid === 'string' ? params.uid.trim() : undefined; + const file_path = typeof params.file_path === 'string' ? params.file_path : undefined; + const include_content = Boolean(params.include_content); + if ( + params.service !== undefined && + params.service !== null && + String(params.service).trim() === '' + ) { + return { group: name || '', error: 'service must not be an empty string', results: [] }; + } + const servicePrefix = normalizeServicePrefix(params.service); + const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; + const subgroupExact = params.subgroupExact === true; + + if (!name) { + return { group: '', error: 'name is required', results: [] }; + } + if (!uid && !target) { + return { group: name, error: 'target or uid is required', results: [] }; + } + + const groupDir = getGroupDir(getDefaultGitnexusDir(), name); + let config: GroupConfig; + try { + config = await loadGroupConfig(groupDir); + } catch (e) { + return { + group: name, + target: target || uid, + service: servicePrefix, + error: e instanceof Error ? e.message : String(e), + results: [], + }; + } + + const memberEntries = Object.entries(config.repos).filter(([repoPath]) => + repoInSubgroup(repoPath, subgroup, subgroupExact), + ); + + // Per-repo work is independent (each repo opens its own DB handle and the + // group-level result preserves repo iteration order via the indexed map). + // Errors are caught per repo so one slow/failed member does not block the rest. + const results: GroupContextResult['results'] = await Promise.all( + memberEntries.map(async ([repoPath, registryName]) => { + try { + const repoObj = await this.port.resolveRepo(registryName); + const payload = await this.port.context(repoObj, { + name: target || undefined, + uid, + file_path, + include_content, + }); + + if (servicePrefix) { + const st = (payload as { status?: string })?.status; + const sym = (payload as { symbol?: { filePath?: string } })?.symbol; + if (st === 'found' && !fileMatchesServicePrefix(sym?.filePath, servicePrefix)) { + return { repoPath, registryName, payload: {} }; + } + } + + return { repoPath, registryName, payload }; + } catch (e) { + return { + repoPath, + registryName, + payload: { error: e instanceof Error ? e.message : String(e) }, + }; + } + }), + ); + + return { + group: name, + target: target || uid, + service: servicePrefix, + results, + }; } async groupQuery(params: Record): Promise { const name = String(params.name ?? '').trim(); const queryText = String(params.query ?? '').trim(); if (!name || !queryText) return { error: 'name and query are required' }; + if ( + params.service !== undefined && + params.service !== null && + String(params.service).trim() === '' + ) { + return { error: 'service must not be an empty string' }; + } + const servicePrefix = normalizeServicePrefix(params.service); const limit = typeof params.limit === 'number' && params.limit > 0 ? params.limit : 5; const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; + const subgroupExact = params.subgroupExact === true; const groupDir = getGroupDir(getDefaultGitnexusDir(), name); const config = await loadGroupConfig(groupDir); - const perRepo: Array<{ repo: string; score: number; processes: unknown[] }> = []; - for (const [repoPath, registryName] of Object.entries(config.repos)) { - if (!repoInSubgroup(repoPath, subgroup)) continue; - try { - const repoObj = await this.port.resolveRepo(registryName); - const queryResult = (await this.port.query(repoObj, { - query: queryText, - limit, - max_symbols: 10, - include_content: false, - })) as { processes?: Array> }; - const processes = queryResult.processes || []; - const scored = processes.map((p, idx) => ({ - ...p, - _rrf_score: 1 / (idx + 1 + 60), - _repo: repoPath, - })); - perRepo.push({ repo: repoPath, score: 0, processes: scored }); - } catch { - perRepo.push({ repo: repoPath, score: 0, processes: [] }); - } - } + const memberEntries = Object.entries(config.repos).filter(([repoPath]) => + repoInSubgroup(repoPath, subgroup, subgroupExact), + ); + + // Per-repo query is independent; run them concurrently and isolate + // failures so one slow/failed member does not block the rest. + const perRepo = await Promise.all( + memberEntries.map(async ([repoPath, registryName]) => { + try { + const repoObj = await this.port.resolveRepo(registryName); + const queryResult = (await this.port.query(repoObj, { + query: queryText, + limit, + max_symbols: 10, + include_content: false, + })) as { + processes?: Array>; + process_symbols?: Array>; + }; + const processes = servicePrefix + ? filterQueryByServicePrefix(queryResult, servicePrefix).processes + : queryResult.processes || []; + const scored = processes.map((p, idx) => ({ + ...p, + _rrf_score: 1 / (idx + 1 + 60), + _repo: repoPath, + })); + return { repo: repoPath, score: 0, processes: scored as unknown[] }; + } catch { + return { repo: repoPath, score: 0, processes: [] as unknown[] }; + } + }), + ); const allProcesses = perRepo.flatMap((r) => r.processes as Array>); allProcesses.sort((a, b) => (b._rrf_score as number) - (a._rrf_score as number)); @@ -184,13 +449,10 @@ export class GroupService { } > = {}; - const fsp = await import('node:fs/promises'); - const pathMod = await import('node:path'); - for (const [repoPath, registryName] of Object.entries(config.repos)) { try { const repoObj = await this.port.resolveRepo(registryName); - const metaPath = pathMod.join(repoObj.storagePath, 'meta.json'); + const metaPath = path.join(repoObj.storagePath, 'meta.json'); const metaRaw = await fsp.readFile(metaPath, 'utf-8').catch(() => '{}'); const meta = JSON.parse(metaRaw) as { lastCommit?: string; indexedAt?: string }; diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index b9ba97582..793d3d0ad 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -96,6 +96,9 @@ export interface RepoHandle { storagePath: string; } +/** Why local impact or fan-out stopped early (e.g. wall-clock budget exhausted). */ +export type GroupImpactTruncationReason = 'timeout' | 'partial'; + export interface GroupImpactResult { local: unknown; group: string; @@ -110,6 +113,36 @@ export interface GroupImpactResult { cross_repo_hits: number; }; risk: string; + /** + * Milliseconds budget applied to the **Phase 1 local impact** leg (`safeLocalImpact`). + * If the walk hits this wall first, expect `truncationReason: 'timeout'` and a partial `local` payload. + */ + timeoutMs?: number; + /** Present when local impact or fan-out stopped early (timeout, graph cap, etc.). */ + truncationReason?: GroupImpactTruncationReason; + /** + * Human-readable note when `crossDepth` was clamped (e.g. multi-hop not implemented yet). + */ + crossDepthWarning?: string; +} + +/** One repo’s `context` tool payload in a group-scoped context run. */ +export interface GroupContextRepoEntry { + repoPath: string; + registryName: string; + payload: unknown; +} + +/** + * Aggregated group `context`: explicit per-repo rows (no merged symbol payloads). + * Use top-level `error` only for unrecoverable failures, not for “no matches” or service scope misses. + */ +export interface GroupContextResult { + group: string; + target?: string; + service?: string; + error?: string; + results: GroupContextRepoEntry[]; } export interface CrossRepoImpact { diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index f019960c5..e4190c323 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -144,6 +144,18 @@ let currentDbPath: string | null = null; let ftsLoaded = false; let vectorExtensionLoaded = false; +/** + * In-process cache of FTS indexes that have been ensured against the current + * connection. Prevents repeated `CALL CREATE_FTS_INDEX` round-trips inside a + * single CLI/MCP session — the first call to `ensureFTSIndex` for a given + * `(tableName, indexName)` pays the LadybugDB cost (~440 ms even when the + * index already exists on disk), subsequent calls are a Set lookup. Cleared + * by `closeLbug` so a re-init starts fresh. + * + * Key format: `${tableName}:${indexName}`. + */ +const ensuredFTSIndexes = new Set(); + /** * Check if an error indicates a missing column or table (schema-level problem) * rather than a transient/connection error. Used for legacy DB fallback logic. @@ -1037,6 +1049,7 @@ export const closeLbug = async (): Promise => { currentDbPath = null; ftsLoaded = false; vectorExtensionLoaded = false; + ensuredFTSIndexes.clear(); }; export const isLbugReady = (): boolean => conn !== null && db !== null; @@ -1219,6 +1232,29 @@ export const createFTSIndex = async ( } }; +/** + * Lazy-create an FTS index, caching the fact in-process. + * + * Used by `queryFTS` so that `analyze` doesn't pay the ~440 ms × 5 fixed + * LadybugDB cost up-front (it dominates analyze on small repos). Instead, + * the cost is moved to the first `query`/`context` call in a session, + * where it's amortised across many lookups. + * + * Safe to call repeatedly — the in-process Set guarantees only the first + * call hits LadybugDB. `closeLbug` clears the cache so re-init starts fresh. + */ +export const ensureFTSIndex = async ( + tableName: string, + indexName: string, + properties: string[], + stemmer: string = 'porter', +): Promise => { + const key = `${tableName}:${indexName}`; + if (ensuredFTSIndexes.has(key)) return; + await createFTSIndex(tableName, indexName, properties, stemmer); + ensuredFTSIndexes.add(key); +}; + /** * Query a full-text search index * @param tableName - The node table name diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index 162ddbfa6..bc7402637 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -35,6 +35,30 @@ interface PoolEntry { const pool = new Map(); +/** + * Listeners notified when a pool entry is torn down (LRU eviction, idle + * timeout, explicit close). Used by upper layers (e.g. the BM25 search + * module) to invalidate per-repo caches that must not outlive the pool + * entry that produced them. + * + * Listeners run synchronously inside `closeOne` after the pool entry has + * been removed; throwing listeners are isolated so one bad listener does + * not prevent others from firing or break teardown. + */ +type PoolCloseListener = (repoId: string) => void; +const poolCloseListeners = new Set(); + +/** + * Subscribe to pool-close events. Returns a disposer that removes the + * listener (handy for tests). + */ +export function addPoolCloseListener(listener: PoolCloseListener): () => void { + poolCloseListeners.add(listener); + return () => { + poolCloseListeners.delete(listener); + }; +} + /** * Shared Database cache keyed by resolved dbPath. * Multiple repoIds pointing to the same path share one native Database @@ -159,6 +183,16 @@ function closeOne(repoId: string): void { } pool.delete(repoId); + + // Notify listeners AFTER the pool entry is gone so any cache-invalidation + // they perform is consistent with `isLbugReady(repoId) === false`. + for (const listener of poolCloseListeners) { + try { + listener(repoId); + } catch { + // Isolate listener failures — teardown must complete. + } + } } /** diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 0e7eaf25a..021b69bc6 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -19,7 +19,6 @@ import { executeQuery, executeWithReusedStatement, closeLbug, - createFTSIndex, loadCachedEmbeddings, } from './lbug/lbug-adapter.js'; import { @@ -220,17 +219,12 @@ export async function runFullAnalysis( }); // ── Phase 3: FTS (85–90%) ───────────────────────────────────────── - progress('fts', 85, 'Creating search indexes...'); - - try { - await createFTSIndex('File', 'file_fts', ['name', 'content']); - await createFTSIndex('Function', 'function_fts', ['name', 'content']); - await createFTSIndex('Class', 'class_fts', ['name', 'content']); - await createFTSIndex('Method', 'method_fts', ['name', 'content']); - await createFTSIndex('Interface', 'interface_fts', ['name', 'content']); - } catch { - // Non-fatal — FTS is best-effort - } + // FTS indexes are created lazily on first `query`/`context` call instead + // of eagerly here. On small repos / CI runners the LadybugDB + // CREATE_FTS_INDEX cost is ~440 ms × 5 (≈2 s) regardless of table size, + // which dominated `analyze` runtime and pushed Windows CI past its + // 30 s test budget. Lazy creation is implemented in + // `core/search/bm25-index.ts` via `ensureFTSIndex`. // ── Phase 3.5: Re-insert cached embeddings ──────────────────────── if (cachedEmbeddings.length > 0) { diff --git a/gitnexus/src/core/search/bm25-index.ts b/gitnexus/src/core/search/bm25-index.ts index ae433ad28..d32bce403 100644 --- a/gitnexus/src/core/search/bm25-index.ts +++ b/gitnexus/src/core/search/bm25-index.ts @@ -3,14 +3,119 @@ * * Uses LadybugDB's built-in full-text search indexes for keyword-based search. * Always reads from the database (no cached state to drift). + * + * FTS indexes are created lazily on first query (via `ensureFTSIndex`) — see + * `lbug-adapter.ts` for the rationale. This keeps `analyze` fast (the + * ~440 ms × 5 LadybugDB CREATE_FTS_INDEX cost dominates pipeline time on + * small repos / CI runners) at the cost of paying that overhead on the + * first `query`/`context` call in a session. */ -import { queryFTS } from '../lbug/lbug-adapter.js'; +import { queryFTS, ensureFTSIndex } from '../lbug/lbug-adapter.js'; export interface BM25SearchResult { filePath: string; score: number; rank: number; + nodeIds?: string[]; +} + +/** + * FTS schema served by `searchFTSFromLbug`. Centralised so that both the + * CLI/pipeline path and the MCP pool path use identical (table, index, + * properties) tuples and the lazy-create logic stays in one place. + */ +const FTS_INDEXES: ReadonlyArray<{ + table: string; + indexName: string; + properties: readonly string[]; +}> = [ + { table: 'File', indexName: 'file_fts', properties: ['name', 'content'] }, + { table: 'Function', indexName: 'function_fts', properties: ['name', 'content'] }, + { table: 'Class', indexName: 'class_fts', properties: ['name', 'content'] }, + { table: 'Method', indexName: 'method_fts', properties: ['name', 'content'] }, + { table: 'Interface', indexName: 'interface_fts', properties: ['name', 'content'] }, +]; + +/** + * Per-process cache for the MCP pool path: tracks which `(repoId, table)` + * pairs have been ensured. The CLI/pipeline path gets its own cache inside + * `lbug-adapter.ts` keyed by table/index, scoped to the singleton connection. + * + * IMPORTANT: an entry is added ONLY when the index was confirmed to exist + * (CREATE_FTS_INDEX succeeded, or failed with `'already exists'`). Other + * failures (transient lock errors, missing extension, etc.) leave the key + * unset so the next query retries instead of silently caching the failure. + * + * Entries for a given repoId are invalidated when its pool is closed — + * see the `addPoolCloseListener` registration in `searchFTSFromLbug`. + */ +const ensuredPoolFTS = new Set(); + +/** + * Drop all ensured-FTS cache entries for a given repoId. + * + * Called from the pool-close listener so that a pool teardown / recreation + * forces the next `searchFTSFromLbug` call to re-issue `CREATE_FTS_INDEX` + * against the fresh connection rather than trust stale ensure-state from a + * previous pool lifetime. + * + * Exported for tests; the listener wiring is internal. + */ +export function invalidateEnsuredFTSForRepo(repoId: string): void { + const prefix = `${repoId}:`; + for (const key of ensuredPoolFTS) { + if (key.startsWith(prefix)) ensuredPoolFTS.delete(key); + } +} + +/** + * Tracks whether we've already wired the pool-close listener for this + * process. The pool adapter is dynamically imported, so registration + * happens lazily on the first MCP-pool-backed FTS query. + */ +let poolCloseListenerRegistered = false; +function registerPoolCloseListenerOnce( + addPoolCloseListener: (listener: (repoId: string) => void) => void, +): void { + if (poolCloseListenerRegistered) return; + poolCloseListenerRegistered = true; + addPoolCloseListener((repoId) => invalidateEnsuredFTSForRepo(repoId)); +} + +async function ensureFTSIndexViaExecutor( + executor: (cypher: string) => Promise, + repoId: string, + table: string, + indexName: string, + properties: readonly string[], +): Promise { + const key = `${repoId}:${table}:${indexName}`; + if (ensuredPoolFTS.has(key)) return; + const propList = properties.map((p) => `'${p}'`).join(', '); + try { + await executor( + `CALL CREATE_FTS_INDEX('${table}', '${indexName}', [${propList}], stemmer := 'porter')`, + ); + // Index was created successfully — safe to cache. + ensuredPoolFTS.add(key); + } catch (e: any) { + // 'already exists' is the happy path (index persists on disk between + // process invocations) — cache it. Anything else is treated as a + // transient failure: surface a one-time warning and leave the key + // unset so the NEXT query retries rather than silently using a + // cached failure (which previously disabled BM25 for the whole + // process for that repo). + const msg = String(e?.message ?? ''); + if (msg.includes('already exists')) { + ensuredPoolFTS.add(key); + } else { + console.warn( + `[gitnexus] FTS index ensure failed for repo "${repoId}" table "${table}" ` + + `(index "${indexName}"): ${msg || e}. Will retry on next query.`, + ); + } + } } /** @@ -23,7 +128,7 @@ async function queryFTSViaExecutor( indexName: string, query: string, limit: number, -): Promise> { +): Promise> { // Escape single quotes and backslashes to prevent Cypher injection const escapedQuery = query.replace(/\\/g, '\\\\').replace(/'/g, "''"); const cypher = ` @@ -40,6 +145,7 @@ async function queryFTSViaExecutor( return { filePath: node.filePath || '', score: typeof score === 'number' ? score : parseFloat(score) || 0, + nodeId: node.nodeId || node.id || '', }; }); } catch { @@ -73,8 +179,21 @@ export const searchFTSFromLbug = async ( // Use MCP connection pool via dynamic import // IMPORTANT: FTS queries run sequentially to avoid connection contention. // The MCP pool supports multiple connections, but FTS is best run serially. - const { executeQuery } = await import('../lbug/pool-adapter.js'); + const poolMod = await import('../lbug/pool-adapter.js'); + const { executeQuery, addPoolCloseListener } = poolMod; + // Register the pool-close listener lazily on first use so a teardown of + // the pool entry (LRU eviction, idle timeout, explicit close) drops the + // matching `ensuredPoolFTS` entries. Without this, stale ensure-state + // can outlive the pool that produced it. + registerPoolCloseListenerOnce(addPoolCloseListener); const executor = (cypher: string) => executeQuery(repoId, cypher); + + // Lazy-create FTS indexes on first query for this repo (analyze no longer + // creates them up-front, so we ensure them here). Cached per-process. + for (const { table, indexName, properties } of FTS_INDEXES) { + await ensureFTSIndexViaExecutor(executor, repoId, table, indexName, properties); + } + fileResults = await queryFTSViaExecutor(executor, 'File', 'file_fts', query, limit); functionResults = await queryFTSViaExecutor(executor, 'Function', 'function_fts', query, limit); classResults = await queryFTSViaExecutor(executor, 'Class', 'class_fts', query, limit); @@ -87,7 +206,12 @@ export const searchFTSFromLbug = async ( limit, ); } else { - // Use core lbug adapter (CLI / pipeline context) — also sequential for safety + // Use core lbug adapter (CLI / pipeline context) — also sequential for safety. + // Lazy-create FTS indexes on first query (analyze no longer does it). + for (const { table, indexName, properties } of FTS_INDEXES) { + await ensureFTSIndex(table, indexName, [...properties]).catch(() => {}); + } + fileResults = await queryFTS('File', 'file_fts', query, limit, false).catch(() => []); functionResults = await queryFTS('Function', 'function_fts', query, limit, false).catch( () => [], @@ -99,17 +223,13 @@ export const searchFTSFromLbug = async ( ); } - // Merge results by filePath, summing scores for same file - const merged = new Map(); + // Collect all node scores per filePath to track which nodes actually matched + const fileNodeScores = new Map>(); const addResults = (results: any[]) => { for (const r of results) { - const existing = merged.get(r.filePath); - if (existing) { - existing.score += r.score; - } else { - merged.set(r.filePath, { filePath: r.filePath, score: r.score }); - } + if (!fileNodeScores.has(r.filePath)) fileNodeScores.set(r.filePath, []); + fileNodeScores.get(r.filePath)!.push({ score: r.score, nodeId: r.nodeId }); } }; @@ -119,6 +239,19 @@ export const searchFTSFromLbug = async ( addResults(methodResults); addResults(interfaceResults); + // Sum the top-3 highest-scoring nodes per file and collect their nodeIds. + // Summing all nodes naively inflates scores for files with many mediocre + // matches (e.g. test files) over files with a single highly-relevant symbol. + const merged = new Map(); + for (const [filePath, entries] of fileNodeScores) { + const top3 = [...entries].sort((a, b) => b.score - a.score).slice(0, 3); + merged.set(filePath, { + filePath, + score: top3.reduce((acc, e) => acc + e.score, 0), + nodeIds: top3.map((e) => e.nodeId).filter((id) => id), + }); + } + // Sort by score descending and add rank const sorted = Array.from(merged.values()) .sort((a, b) => b.score - a.score) @@ -128,5 +261,6 @@ export const searchFTSFromLbug = async ( filePath: r.filePath, score: r.score, rank: index + 1, + nodeIds: r.nodeIds, })); }; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index f8a049eb3..4a1e3c41c 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -28,6 +28,7 @@ import { type RegistryEntry, } from '../../storage/repo-manager.js'; import { GroupService, type GroupToolPort } from '../../core/group/service.js'; +import { resolveAtGroupMemberRepoPath } from '../../core/group/resolve-at-member.js'; import { collectBestChunks } from '../../core/embeddings/types.js'; import { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME } from '../../core/lbug/schema.js'; import { PhaseTimer } from '../../core/search/phase-timer.js'; @@ -227,6 +228,7 @@ export class LocalBackend { impact: (r, p) => this.impact(r as RepoHandle, p), query: (r, p) => this.query(r as RepoHandle, p), impactByUid: (id, uid, d, o) => this.impactByUid(id, uid, d, o), + context: (r, p) => this.context(r as RepoHandle, p), }; this.groupToolSvc = new GroupService(port); } @@ -633,8 +635,17 @@ export class LocalBackend { return this.handleGroupTool(method, params || {}); } + const p = params && typeof params === 'object' ? (params as Record) : {}; + if ( + (method === 'impact' || method === 'query' || method === 'context') && + typeof p.repo === 'string' && + p.repo.startsWith('@') + ) { + return this.callToolAtGroupRepo(method, p); + } + // Resolve repo from optional param (re-reads registry on miss) - const repo = await this.resolveRepo(params?.repo); + const repo = await this.resolveRepo((params as { repo?: string } | undefined)?.repo); switch (method) { case 'query': @@ -974,16 +985,30 @@ export class LocalBackend { for (const bm25Result of bm25Results) { const fullPath = bm25Result.filePath; try { - const symbols = await executeParameterized( - repo.id, - ` - MATCH (n) - WHERE n.filePath = $filePath - RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine - LIMIT 3 - `, - { filePath: fullPath }, - ); + // Prefer direct nodeId lookup (exact FTS-matched nodes) over filePath fallback. + // Without this, LIMIT 3 on filePath returns arbitrary symbols rather than + // the nodes that actually scored highest in the BM25 index. + const nodeIds = bm25Result.nodeIds?.length ? bm25Result.nodeIds : null; + const symbols = nodeIds + ? await executeParameterized( + repo.id, + ` + MATCH (n) + WHERE n.id IN $nodeIds + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine + `, + { nodeIds }, + ) + : await executeParameterized( + repo.id, + ` + MATCH (n) + WHERE n.filePath = $filePath + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine + LIMIT 3 + `, + { filePath: fullPath }, + ); if (symbols.length > 0) { for (const sym of symbols) { @@ -2969,17 +2994,103 @@ export class LocalBackend { return this.groupList(params); case 'group_sync': return this.groupSync(params); - case 'group_contracts': - return this.groupContracts(params); - case 'group_query': - return this.groupQuery(params); - case 'group_status': - return this.groupStatus(params); default: - throw new Error(`Unknown group tool: ${method}`); + throw new Error( + `Unknown group tool: ${method}. Removed tools: use repo "@" on impact, query, or context (optional "/"), or MCP resources.`, + ); } } + /** + * Dispatch impact/query/context when `repo` is `@groupName` or `@groupName/memberPath` + * (group mode — not the global indexed-repo `repo` parameter). + */ + private async callToolAtGroupRepo( + method: string, + params: Record, + ): Promise { + await this.refreshRepos(); + + if ( + params.service !== undefined && + params.service !== null && + String(params.service).trim() === '' + ) { + return { error: 'service must not be an empty string' }; + } + + const raw = String(params.repo).slice(1); + const slash = raw.indexOf('/'); + const groupName = (slash === -1 ? raw : raw.slice(0, slash)).trim(); + const memberRest = slash === -1 ? undefined : raw.slice(slash + 1).trim() || undefined; + + const resolved = await resolveAtGroupMemberRepoPath(groupName, memberRest); + if (resolved.ok === false) return { error: resolved.error }; + + const svc = this.getGroupService(); + if (method === 'impact') { + const impactArgs: Record = { + name: groupName, + repo: resolved.repoPath, + target: params.target, + direction: params.direction, + }; + if (params.maxDepth !== undefined) impactArgs.maxDepth = params.maxDepth; + if (params.crossDepth !== undefined) impactArgs.crossDepth = params.crossDepth; + if (params.relationTypes !== undefined) impactArgs.relationTypes = params.relationTypes; + if (params.includeTests !== undefined) impactArgs.includeTests = params.includeTests; + if (params.minConfidence !== undefined) impactArgs.minConfidence = params.minConfidence; + if (params.service !== undefined && params.service !== null) + impactArgs.service = params.service; + if (typeof params.subgroup === 'string') impactArgs.subgroup = params.subgroup; + if (params.timeoutMs !== undefined) impactArgs.timeoutMs = params.timeoutMs; + if (params.timeout !== undefined) impactArgs.timeout = params.timeout; + return svc.groupImpact(impactArgs); + } + if (method === 'query') { + const queryArgs: Record = { + name: groupName, + query: params.query, + }; + if (typeof params.task_context === 'string') queryArgs.task_context = params.task_context; + if (typeof params.goal === 'string') queryArgs.goal = params.goal; + if (typeof params.limit === 'number') queryArgs.limit = params.limit; + if (typeof params.max_symbols === 'number') queryArgs.max_symbols = params.max_symbols; + if (params.include_content !== undefined) queryArgs.include_content = params.include_content; + if (params.service !== undefined && params.service !== null) + queryArgs.service = params.service; + if (memberRest !== undefined) { + queryArgs.subgroup = memberRest; + queryArgs.subgroupExact = true; + } + return svc.groupQuery(queryArgs); + } + if (method === 'context') { + const targetSym = + typeof params.target === 'string' && params.target.trim() !== '' + ? params.target.trim() + : typeof params.name === 'string' && params.name.trim() !== '' + ? params.name.trim() + : undefined; + const contextArgs: Record = { + name: groupName, + target: targetSym, + }; + if (typeof params.uid === 'string') contextArgs.uid = params.uid; + if (typeof params.file_path === 'string') contextArgs.file_path = params.file_path; + if (params.include_content !== undefined) + contextArgs.include_content = params.include_content; + if (params.service !== undefined && params.service !== null) + contextArgs.service = params.service; + if (memberRest !== undefined) { + contextArgs.subgroup = memberRest; + contextArgs.subgroupExact = true; + } + return svc.groupContext(contextArgs); + } + throw new Error(`Internal: unsupported group-repo tool ${method}`); + } + private async groupList(params: Record): Promise { return this.getGroupService().groupList(params); } @@ -2988,18 +3099,45 @@ export class LocalBackend { return this.getGroupService().groupSync(params); } - private async groupContracts(params: Record): Promise { - return this.getGroupService().groupContracts(params); + /** + * MCP resource body for `gitnexus://group/{name}/contracts` (Issue #794). + */ + async readGroupContractsResource( + groupName: string, + filter: { type?: string; repo?: string; unmatchedOnly?: boolean }, + ): Promise { + try { + const params: Record = { name: groupName }; + if (filter.type !== undefined) params.type = filter.type; + if (filter.repo !== undefined) params.repo = filter.repo; + if (filter.unmatchedOnly === true) params.unmatchedOnly = true; + const raw = await this.getGroupService().groupContracts(params); + return LocalBackend.formatGroupResourcePayload(raw); + } catch (e) { + return `error: ${e instanceof Error ? e.message : String(e)}`; + } } - private async groupQuery(params: Record): Promise { - await this.refreshRepos(); - return this.getGroupService().groupQuery(params); + /** + * MCP resource body for `gitnexus://group/{name}/status` (Issue #794). + */ + async readGroupStatusResource(groupName: string): Promise { + try { + const raw = await this.getGroupService().groupStatus({ name: groupName }); + return LocalBackend.formatGroupResourcePayload(raw); + } catch (e) { + return `error: ${e instanceof Error ? e.message : String(e)}`; + } } - private async groupStatus(params: Record): Promise { - await this.refreshRepos(); - return this.getGroupService().groupStatus(params); + private static formatGroupResourcePayload(raw: unknown): string { + if (raw && typeof raw === 'object' && 'error' in raw) { + const err = (raw as { error?: unknown }).error; + if (typeof err === 'string' && err.length > 0) { + return `error: ${err}`; + } + } + return JSON.stringify(raw, null, 2); } /** diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index dce81bab4..88e7a99cc 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -84,38 +84,140 @@ export function getResourceTemplates(): ResourceTemplate[] { description: 'Step-by-step execution trace', mimeType: 'text/yaml', }, + { + uriTemplate: 'gitnexus://group/{name}/contracts', + name: 'Group Contract Registry', + description: + 'Cross-repo contract registry for a repository group. Optional query: type, repo, unmatchedOnly (true|false).', + mimeType: 'text/yaml', + }, + { + uriTemplate: 'gitnexus://group/{name}/status', + name: 'Group Index Status', + description: 'Per-repo index and contract-registry staleness for a repository group', + mimeType: 'text/yaml', + }, ]; } -/** - * Parse a resource URI to extract the repo name and resource type. - */ -function parseUri(uri: string): { repoName?: string; resourceType: string; param?: string } { - if (uri === 'gitnexus://repos') return { resourceType: 'repos' }; - if (uri === 'gitnexus://setup') return { resourceType: 'setup' }; +/** Query parameters for `gitnexus://group/{name}/contracts` */ +export type GroupContractsResourceFilter = { + type?: string; + repo?: string; + unmatchedOnly?: boolean; +}; - // Repo-scoped: gitnexus://repo/{name}/context - const repoMatch = uri.match(/^gitnexus:\/\/repo\/([^/]+)\/(.+)$/); - if (repoMatch) { - const repoName = decodeURIComponent(repoMatch[1]); - const rest = repoMatch[2]; +/** Normalized parse result for GitNexus MCP resource URIs */ +export type ParsedGitnexusResource = + | { kind: 'repos' } + | { kind: 'setup' } + | { + kind: 'repo'; + repoName: string; + resourceType: string; + param?: string; + } + | { + kind: 'group'; + groupName: string; + resourceType: 'contracts'; + contractsFilter: GroupContractsResourceFilter; + } + | { kind: 'group'; groupName: string; resourceType: 'status' }; + +function parseUnmatchedOnlyParam(raw: string | null): boolean | undefined { + if (raw === null) return undefined; + const v = raw.trim().toLowerCase(); + if (v === 'true' || v === '1') return true; + if (v === 'false' || v === '0') return false; + return undefined; +} + +/** + * Parse a GitNexus resource URI (repos, setup, per-repo, or per-group templates). + * Used by `readResource` and tests (round-trip / dispatch coverage). + */ +export function parseResourceUri(uri: string): ParsedGitnexusResource { + if (uri === 'gitnexus://repos') return { kind: 'repos' }; + if (uri === 'gitnexus://setup') return { kind: 'setup' }; + + let u: URL; + try { + u = new URL(uri); + } catch { + throw new Error(`Unknown resource URI: ${uri}`); + } + + if (u.protocol !== 'gitnexus:') { + throw new Error(`Unknown resource URI: ${uri}`); + } + + if (u.hostname === 'group') { + const segments = u.pathname + .replace(/^\/+|\/+$/g, '') + .split('/') + .filter(Boolean); + if (segments.length < 2) { + throw new Error( + `Invalid group resource URI (expected gitnexus://group/{name}/contracts or .../status): ${uri}`, + ); + } + const tail = segments[segments.length - 1]!; + if (tail !== 'contracts' && tail !== 'status') { + throw new Error(`Unknown group resource path in URI: ${uri}`); + } + const groupName = segments + .slice(0, -1) + .map((s) => decodeURIComponent(s)) + .join('/'); + if (!groupName) { + throw new Error(`Invalid group resource URI (empty group name): ${uri}`); + } + if (tail === 'status') { + return { kind: 'group', groupName, resourceType: 'status' }; + } + const contractsFilter: GroupContractsResourceFilter = {}; + const type = u.searchParams.get('type'); + if (type && type.trim()) contractsFilter.type = type.trim(); + const repo = u.searchParams.get('repo'); + if (repo && repo.trim()) contractsFilter.repo = repo.trim(); + if (u.searchParams.has('unmatchedOnly')) { + const coerced = parseUnmatchedOnlyParam(u.searchParams.get('unmatchedOnly')); + if (coerced !== undefined) contractsFilter.unmatchedOnly = coerced; + } + return { kind: 'group', groupName, resourceType: 'contracts', contractsFilter }; + } + + if (u.hostname === 'repo') { + const segments = u.pathname + .replace(/^\/+|\/+$/g, '') + .split('/') + .filter(Boolean); + if (segments.length < 2) { + throw new Error(`Unknown resource URI: ${uri}`); + } + const repoName = decodeURIComponent(segments[0]!); + const restEncoded = segments.slice(1); + const rest = restEncoded.map((s) => decodeURIComponent(s)).join('/'); if (rest.startsWith('cluster/')) { return { + kind: 'repo', repoName, resourceType: 'cluster', - param: decodeURIComponent(rest.replace('cluster/', '')), + param: rest.replace(/^cluster\//, ''), }; } if (rest.startsWith('process/')) { return { + kind: 'repo', repoName, resourceType: 'process', - param: decodeURIComponent(rest.replace('process/', '')), + param: rest.replace(/^process\//, ''), }; } - return { repoName, resourceType: rest }; + return { kind: 'repo', repoName, resourceType: rest }; } throw new Error(`Unknown resource URI: ${uri}`); @@ -125,18 +227,23 @@ function parseUri(uri: string): { repoName?: string; resourceType: string; param * Read a resource and return its content */ export async function readResource(uri: string, backend: LocalBackend): Promise { - const parsed = parseUri(uri); + const parsed = parseResourceUri(uri); - // Global repos list — no repo context needed - if (parsed.resourceType === 'repos') { + if (parsed.kind === 'repos') { return getReposResource(backend); } - // Setup resource — returns AGENTS.md content for all repos - if (parsed.resourceType === 'setup') { + if (parsed.kind === 'setup') { return getSetupResource(backend); } + if (parsed.kind === 'group') { + if (parsed.resourceType === 'contracts') { + return backend.readGroupContractsResource(parsed.groupName, parsed.contractsFilter); + } + return backend.readGroupStatusResource(parsed.groupName); + } + const repoName = parsed.repoName; switch (parsed.resourceType) { @@ -241,6 +348,10 @@ async function getContextResource(backend: LocalBackend, repoName?: string): Pro lines.push(` - gitnexus://repo/${context.projectName}/processes: All execution flows`); lines.push(` - gitnexus://repo/${context.projectName}/cluster/{name}: Module details`); lines.push(` - gitnexus://repo/${context.projectName}/process/{name}: Process trace`); + lines.push( + ' - gitnexus://group/{name}/contracts: Group contract registry (optional ?type=&repo=&unmatchedOnly=)', + ); + lines.push(' - gitnexus://group/{name}/status: Group index / contract staleness'); return lines.join('\n'); } diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 7beb2e970..491c24557 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -15,9 +15,12 @@ export interface ToolDefinition { { type: string; description?: string; - default?: any; + default?: unknown; items?: { type: string }; enum?: string[]; + minimum?: number; + maximum?: number; + minLength?: number; } >; required: string[]; @@ -55,7 +58,11 @@ Returns results grouped by process (execution flow): - process_symbols: all symbols in those flows with file locations and module (functional area) - definitions: standalone types/interfaces not in any process -Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank Fusion.`, +Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank Fusion. + +GROUP MODE: set "repo" to "@" to search all member repos in that group (merged via RRF), or "@/" to run against a single member (same path keys as in group.yaml). If you use "@" only, the member repo defaults to the lexicographically first key in group.yaml "repos". Prefer resources for contracts/status (see migration from legacy group_* tools). + +SERVICE: optional monorepo path prefix (POSIX-style, case-sensitive segments). When "repo" starts with "@", only processes whose symbols fall under that prefix are included. For a normal indexed repo name (no leading @), this field is currently ignored by the server.`, inputSchema: { type: 'object', properties: { @@ -69,11 +76,19 @@ Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank description: 'What you want to find (e.g., "existing auth validation logic"). Helps ranking.', }, - limit: { type: 'number', description: 'Max processes to return (default: 5)', default: 5 }, + limit: { + type: 'number', + description: 'Max processes to return (default: 5)', + default: 5, + minimum: 1, + maximum: 100, + }, max_symbols: { type: 'number', description: 'Max symbols per process (default: 10)', default: 10, + minimum: 1, + maximum: 200, }, include_content: { type: 'boolean', @@ -82,7 +97,14 @@ Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: + 'Indexed repository name or path, or group mode "@" / "@/" (member path keys from group.yaml). Omit when only one indexed repo exists.', + }, + service: { + type: 'string', + minLength: 1, + description: + 'Optional monorepo service root (relative path, "/" separators). In group mode (@repo), prefix-matches symbol file paths; ignored for a normal repo name. Empty string is rejected server-side.', }, }, required: ['query'], @@ -156,7 +178,11 @@ AFTER THIS: Use impact() if planning changes, or READ gitnexus://repo/{name}/pro Handles disambiguation: if multiple symbols share the same name, returns ranked candidates (each with a relevance score) for you to pick from. Use uid for zero-ambiguity lookup, or narrow the search with file_path and/or kind hints. -NOTE: ACCESSES edges (field read/write tracking) are included in context results with reason 'read' or 'write'. CALLS edges resolve through field access chains and method-call chains (e.g., user.address.getCity().save() produces CALLS edges at each step).`, +NOTE: ACCESSES edges (field read/write tracking) are included in context results with reason 'read' or 'write'. CALLS edges resolve through field access chains and method-call chains (e.g., user.address.getCity().save() produces CALLS edges at each step). + +GROUP MODE: set "repo" to "@" to run context in each member repo (aggregated list), or "@/" for one member. If you use "@" only, the member defaults to the lexicographically first key in group.yaml "repos". + +SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", prefix-matches resolved symbol file paths; when a hit is outside the prefix, that member returns an empty payload for the symbol. Ignored for a normal indexed repo name.`, inputSchema: { type: 'object', properties: { @@ -178,7 +204,14 @@ NOTE: ACCESSES edges (field read/write tracking) are included in context results }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: + 'Indexed repository name or path, or group mode "@" / "@/". Omit if only one repo is indexed.', + }, + service: { + type: 'string', + minLength: 1, + description: + 'Optional monorepo service root (relative path). Applies in group mode (@repo) only; ignored for a normal repo name. Empty string is rejected server-side.', }, }, required: [], @@ -273,7 +306,11 @@ TIP: Default traversal uses CALLS/IMPORTS/EXTENDS/IMPLEMENTS. For class members, Handles disambiguation: when multiple symbols share the target name, returns ranked candidates (each with a relevance score) instead of silently picking one. Use target_uid for zero-ambiguity lookup, or narrow with file_path and/or kind hints. EdgeType: CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, METHOD_OVERRIDES, METHOD_IMPLEMENTS, ACCESSES -Confidence: 1.0 = certain, <0.8 = fuzzy match`, +Confidence: 1.0 = certain, <0.8 = fuzzy match + +GROUP MODE: set "repo" to "@" for cross-repo impact anchored at the default member (lexicographically first key in group.yaml "repos"), or "@/" to choose the member (same path keys as in group.yaml). Phase-1 walk runs in that member; cross-boundary fan-out uses the group bridge. + +SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", scopes the local impact walk and cross-repo symbol paths to files under that prefix; ignored for a normal indexed repo name.`, inputSchema: { type: 'object', properties: { @@ -298,8 +335,18 @@ Confidence: 1.0 = certain, <0.8 = fuzzy match`, }, maxDepth: { type: 'number', - description: 'Max relationship depth (default: 3)', + description: 'Max relationship depth (default: 3, server clamps to 1–32)', default: 3, + minimum: 1, + maximum: 32, + }, + crossDepth: { + type: 'number', + description: + 'Cross-repository hop depth via contract bridge (default: 1; values above server maximum are clamped)', + default: 1, + minimum: 1, + maximum: 32, }, relationTypes: { type: 'array', @@ -308,10 +355,42 @@ Confidence: 1.0 = certain, <0.8 = fuzzy match`, 'Filter: CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, METHOD_OVERRIDES, METHOD_IMPLEMENTS, ACCESSES (default: usage-based, ACCESSES excluded by default)', }, includeTests: { type: 'boolean', description: 'Include test files (default: false)' }, - minConfidence: { type: 'number', description: 'Minimum confidence 0-1 (default: 0.7)' }, + minConfidence: { + type: 'number', + description: + 'Minimum edge confidence 0–1 (default: 0 when omitted; server clamps to 0–1)', + default: 0, + minimum: 0, + maximum: 1, + }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: + 'Indexed repository name or path, or group mode "@" / "@/". Omit if only one repo is indexed.', + }, + service: { + type: 'string', + minLength: 1, + description: + 'Optional monorepo service root (relative path). Applies when "repo" is group mode (@…); ignored for a normal repo name. Empty string is rejected server-side.', + }, + subgroup: { + type: 'string', + description: + 'Optional group subgroup prefix (member repo paths) limiting which repos participate in cross fan-out.', + }, + timeoutMs: { + type: 'number', + description: + 'Wall-clock budget in milliseconds for the Phase-1 local impact leg (default 30000)', + minimum: 1, + maximum: 3600000, + }, + timeout: { + type: 'number', + description: 'Alias of timeoutMs (milliseconds) when timeoutMs is omitted', + minimum: 1, + maximum: 3600000, }, }, required: ['target', 'direction'], @@ -429,49 +508,4 @@ WHEN TO USE: After changing group.yaml or re-indexing member repos.`, required: ['name'], }, }, - { - name: 'group_contracts', - description: `Inspect contracts and cross-links from the group's contracts.json. - -WHEN TO USE: Debug cross-repo links after group_sync.`, - inputSchema: { - type: 'object', - properties: { - name: { type: 'string', description: 'Group name' }, - type: { type: 'string', description: 'Filter by contract type (http, topic, …)' }, - repo: { type: 'string', description: 'Filter by group repo path (e.g. app/backend)' }, - unmatchedOnly: { type: 'boolean', description: 'Only contracts with no cross-link' }, - }, - required: ['name'], - }, - }, - { - name: 'group_query', - description: `Run the query tool across all repos in a group and merge process results via reciprocal rank fusion. - -WHEN TO USE: Semantic / hybrid search across a whole product group.`, - inputSchema: { - type: 'object', - properties: { - name: { type: 'string', description: 'Group name' }, - query: { type: 'string', description: 'Search query' }, - subgroup: { type: 'string', description: 'Limit to repo paths under this prefix' }, - limit: { type: 'number', description: 'Max merged results (default 5)' }, - }, - required: ['name', 'query'], - }, - }, - { - name: 'group_status', - description: `Report index staleness (commit vs HEAD) and Contract Registry staleness (indexedAt) for each repo in a group. - -WHEN TO USE: Before group_sync or when agents should refresh indexes.`, - inputSchema: { - type: 'object', - properties: { - name: { type: 'string', description: 'Group name' }, - }, - required: ['name'], - }, - }, ]; diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 6daea8291..5c592d570 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -7,10 +7,50 @@ */ import fs from 'fs/promises'; +import { realpathSync } from 'fs'; import path from 'path'; import os from 'os'; import { getInferredRepoName } from './git.js'; +/** + * Normalise a repo path for registry comparison across platforms + * (#664 review feedback from @evander-wang). + * + * Why this exists: `path.resolve` alone is NOT enough for + * cross-platform registry stability. + * - **macOS**: tmpdirs and `/var` are symlinks to `/private/var`. + * A child process that stored `/private/var/folders/.../repo` in + * the registry cannot later be matched by an outer caller that + * supplies the symlink form `/var/folders/.../repo`. `path.resolve` + * does not follow symlinks; `realpathSync.native` does. + * - **Windows**: GitHub runners surface tmpdirs in 8.3 short-name + * form (`RUNNERA~1\...`), but `process.cwd()` often returns the + * long form (`runneradmin\...`). `realpathSync.native` normalises + * both sides to the long-name canonical path. + * + * Fallback behaviour: if the path does not exist on disk (e.g. a user + * passed `gitnexus remove some-alias` and the alias misses every + * registry entry, or the caller is resolving a path that was deleted + * after registration), we return `path.resolve(p)` rather than + * throwing. This preserves the idempotent-on-missing semantics of + * `resolveRegistryEntry` / `remove`. + * + * Backwards compatibility: this function is applied to BOTH the + * caller-supplied input AND each stored `entry.path` at compare time + * inside `resolveRegistryEntry`, so registries written by older + * versions (where `registerRepo` only ran `path.resolve`) still match + * correctly. Newly-written entries are canonicalised at write time too + * so the registry stabilises over analyze/re-analyze cycles. + */ +export const canonicalizePath = (p: string): string => { + const resolved = path.resolve(p); + try { + return realpathSync.native(resolved); + } catch { + return resolved; + } +}; + export interface RepoMeta { repoPath: string; lastCommit: string; @@ -359,13 +399,33 @@ export const registerRepo = async ( meta: RepoMeta, opts?: RegisterRepoOptions, ): Promise => { + // Preserve the caller's chosen path form in the registry — don't + // canonicalise at write time. This matters for two reasons: + // 1. `list` and error messages show the path the user actually + // knows (e.g. the 8.3 short form they typed), not a runtime- + // resolved long form they've never seen. + // 2. Keeps pre-existing #829 test assertions that compare + // `err.existingPath` against `path.resolve(tmpPath)` stable. + // Canonicalisation is applied at COMPARE points only (see below), + // which is where the cross-platform divergence actually matters. const resolved = path.resolve(repoPath); const { storagePath } = getStoragePaths(resolved); + // Canonical form used strictly for comparison — `realpathSync.native` + // expands macOS /var → /private/var and Windows 8.3 → long-name, + // falling back to `path.resolve` when the path doesn't exist. + const canonicalInput = canonicalizePath(repoPath); + const entries = await readRegistry(); const existingIdx = entries.findIndex((e) => { - const a = path.resolve(e.path); - const b = resolved; + // Canonicalise the STORED entry too so pre-canonicalisation + // registries (written by older versions, or paths passed in a + // different form) still match correctly. `canonicalizePath` falls + // back to `path.resolve` when the path no longer exists on disk, + // so stale entries that have been rm'd externally still resolve + // to a stable key instead of throwing. + const a = canonicalizePath(e.path); + const b = canonicalInput; return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; }); const existing = existingIdx >= 0 ? entries[existingIdx] : null; @@ -399,11 +459,14 @@ export const registerRepo = async ( // messages and list output #829 ships). const explicitName = opts?.name !== undefined || isPreservedAlias; if (explicitName && !opts?.allowDuplicateName) { + // Compare canonical-vs-canonical here too so `/var/foo` and + // `/private/var/foo` (same repo, different form) aren't treated as + // two colliding paths. const collidingEntry = entries.find( (e, i) => i !== existingIdx && e.name.toLowerCase() === name.toLowerCase() && - path.resolve(e.path) !== resolved, + canonicalizePath(e.path) !== canonicalInput, ); if (collidingEntry) { throw new RegistryNameCollisionError(name, collidingEntry.path, resolved); @@ -435,12 +498,204 @@ export const registerRepo = async ( * Called after `gitnexus clean`. */ export const unregisterRepo = async (repoPath: string): Promise => { - const resolved = path.resolve(repoPath); + // Canonicalise BOTH sides so an unregister call issued with the + // symlink form (`/var/folders/.../repo`) still matches an entry + // written with the realpath form (`/private/var/folders/.../repo`), + // and vice versa. Matches the semantics of `registerRepo` and + // `resolveRegistryEntry` post-#1003 review. + const resolved = canonicalizePath(repoPath); const entries = await readRegistry(); - const filtered = entries.filter((e) => path.resolve(e.path) !== resolved); + const matches = (a: string, b: string) => + process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; + const filtered = entries.filter((e) => !matches(canonicalizePath(e.path), resolved)); await writeRegistry(filtered); }; +/** + * Thrown by {@link resolveRegistryEntry} when no registered repo matches + * the caller's target string (by alias, basename, remote-inferred name, + * or resolved path). CLI callers that want idempotent "remove" semantics + * should catch this and exit 0 with a warning; non-idempotent callers + * (e.g. MCP tools) can surface the error directly. + */ +export class RegistryNotFoundError extends Error { + readonly kind = 'RegistryNotFoundError' as const; + constructor( + public readonly target: string, + public readonly availableNames: string[], + ) { + const hint = + availableNames.length > 0 + ? ` Available: ${availableNames.join(', ')}.` + : ' No repositories are currently registered.'; + super(`No registered repo matches "${target}".${hint}`); + this.name = 'RegistryNotFoundError'; + } +} + +/** + * Thrown by {@link resolveRegistryEntry} when the target string matches + * the `name` of two or more entries — only possible when the user + * previously registered duplicates via `analyze --name X + * --allow-duplicate-name` (#829). The error carries enough information + * for the caller to render an actionable disambiguation hint without + * string-matching on `.message`. + * + * `kind` is a string literal discriminant (same pattern as + * {@link RegistryNameCollisionError}) so callers can narrow via + * `err.kind === 'RegistryAmbiguousTargetError'` without importing the + * class. + */ +export class RegistryAmbiguousTargetError extends Error { + readonly kind = 'RegistryAmbiguousTargetError' as const; + constructor( + public readonly target: string, + public readonly matches: RegistryEntry[], + ) { + const listing = matches.map((m) => ` - ${m.name} (${m.path})`).join('\n'); + super( + `Multiple registered repos match "${target}":\n${listing}\n` + + `Pass the absolute path instead to disambiguate.`, + ); + this.name = 'RegistryAmbiguousTargetError'; + } +} + +/** + * Thrown by {@link assertSafeStoragePath} when a registry entry's + * `storagePath` does NOT point at the expected `/.gitnexus` + * subfolder. CLI destructive commands (`remove`, `clean --all`) should + * catch this and exit non-zero without deleting anything — the usual + * cause is a corrupted or hand-edited `~/.gitnexus/registry.json`, and + * proceeding would mean `fs.rm(recursive: true)` on whatever odd path + * the entry is pointing at. + */ +export class UnsafeStoragePathError extends Error { + readonly kind = 'UnsafeStoragePathError' as const; + constructor( + public readonly entry: RegistryEntry, + public readonly expectedStoragePath: string, + public readonly actualStoragePath: string, + ) { + super( + `Refusing to remove storage path for safety: expected ` + + `"${expectedStoragePath}" under the repo's .gitnexus subfolder, ` + + `but the registry entry has "${actualStoragePath}". ` + + `This usually means the registry entry is corrupted or was ` + + `hand-edited. Delete the entry manually from ~/.gitnexus/registry.json ` + + `and re-run analyze.`, + ); + this.name = 'UnsafeStoragePathError'; + } +} + +/** + * Guard rail for destructive CLI paths (`remove` #664, + * `clean --all` #258, future MCP `remove` tool): verify that a + * registry entry's `storagePath` is the canonical `/.gitnexus` + * subfolder of its `path`. If not, throw {@link UnsafeStoragePathError} + * so the caller exits without touching disk. + * + * Why this exists (#1003 review — @magyargergo): + * - `~/.gitnexus/registry.json` is a plain-text user-writable file. + * A corrupted, hand-edited, or downgrade/upgrade-racing entry + * could plausibly end up with `storagePath === ""` (resolves to + * cwd), `storagePath === path` (the repo root!), `storagePath` + * equal to a parent/sibling of the repo, or simply any arbitrary + * filesystem path. + * - `fs.rm(recursive: true, force: true)` on ANY of those would be + * a runtime disaster — at best delete the user's working tree, at + * worst nuke an unrelated directory tree they happen to own. + * - `clean` (default, cwd-scoped) is safe by construction — it + * re-derives storagePath from `findRepo(cwd)` and never trusts + * the registry field. But `clean --all` DOES iterate the registry + * and trust each entry's stored storagePath (same shape as + * `remove`), so this helper must be wired into that loop too. + * - `server/api.ts` recomputes storagePath from `getStoragePath(entry.path)` + * and so is likewise safe-by-construction. + * + * Pure string check — does NOT require the paths to exist on disk. + * Windows: case-insensitive; POSIX: case-sensitive. Matches the + * comparison shape used elsewhere in this module. + */ +export const assertSafeStoragePath = (entry: RegistryEntry): void => { + const expected = path.join(path.resolve(entry.path), '.gitnexus'); + const actual = path.resolve(entry.storagePath); + const matches = + process.platform === 'win32' + ? expected.toLowerCase() === actual.toLowerCase() + : expected === actual; + if (!matches) { + throw new UnsafeStoragePathError(entry, expected, actual); + } +}; + +/** + * Resolve a user-supplied target string (from `gitnexus remove ` + * or equivalent MCP tool argument) to a single registry entry. + * + * Match precedence (first hit wins, subsequent tiers are only tried if + * the prior tier produces zero matches): + * 1. Exact resolved-path match (Windows: case-insensitive). + * Paths are unique by registry construction, so a path match can + * never be ambiguous. + * 2. Exact `name` match (case-insensitive). If ≥ 2 entries share the + * name — only possible via `--allow-duplicate-name` (#829) — + * throws {@link RegistryAmbiguousTargetError}. + * + * No fuzzy / partial matching — unambiguous, scriptable behaviour is + * more important than convenience for destructive commands. + * + * Throws {@link RegistryNotFoundError} if no entry matches. + * + * `entries` is passed in (rather than re-read) so callers that already + * hold the registry snapshot (e.g. to print a "before" state) can avoid + * a second disk read, and so tests can inject fixtures without touching + * `GITNEXUS_HOME`. + */ +export const resolveRegistryEntry = (entries: RegistryEntry[], target: string): RegistryEntry => { + // Tier 1: path match. Canonicalise BOTH sides so symlink and + // Windows-8.3 quirks don't cause a false miss — e.g. the caller + // passes `/var/folders/.../repo` while the registry has + // `/private/var/folders/.../repo` (both resolve to the same + // `realpath.native`). See `canonicalizePath` for the rationale. + // + // Canonicalising the STORED entry (not just the input) is what gives + // us backward-compat for registries written by versions that only + // ran `path.resolve` — both get canonicalised here at compare time. + const canonicalTarget = canonicalizePath(target); + const pathMatch = entries.find((e) => { + const a = canonicalizePath(e.path); + const b = canonicalTarget; + return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; + }); + if (pathMatch) return pathMatch; + + // Tier 2: name match. Case-insensitive on all platforms — registry + // name collisions are already filtered case-insensitively in + // `registerRepo`, so "APP" vs "app" are considered the same key. + const targetLower = target.toLowerCase(); + const nameMatches = entries.filter((e) => e.name.toLowerCase() === targetLower); + if (nameMatches.length === 1) return nameMatches[0]; + if (nameMatches.length > 1) { + throw new RegistryAmbiguousTargetError(target, nameMatches); + } + + // Tier 3: miss. Build the available-names hint ONCE; resolveRepo-style + // disambiguated labels (`app (/path)`) are applied when the same name + // appears in multiple entries so the user sees the same hint shape as + // `-r ` errors. + const nameCounts = new Map(); + for (const e of entries) { + const key = e.name.toLowerCase(); + nameCounts.set(key, (nameCounts.get(key) ?? 0) + 1); + } + const availableNames = entries.map((e) => + (nameCounts.get(e.name.toLowerCase()) ?? 0) > 1 ? `${e.name} (${e.path})` : e.name, + ); + throw new RegistryNotFoundError(target, availableNames); +}; + /** * List all registered repos from the global registry. * Optionally validates that each entry's .gitnexus/ still exists. diff --git a/gitnexus/test/integration/cli-e2e.test.ts b/gitnexus/test/integration/cli-e2e.test.ts index 844e3978d..f994b42ae 100644 --- a/gitnexus/test/integration/cli-e2e.test.ts +++ b/gitnexus/test/integration/cli-e2e.test.ts @@ -345,6 +345,410 @@ describe('CLI end-to-end', () => { }, 360000); // 6-min outer budget (4 × ~60s analyze calls + fixture setup) }); + // ─── gitnexus remove (#664) ───────────────────────────── + // + // End-to-end regression guard for the remove command: + // 1. `remove ` without --force is a dry-run (exit 0, preserves state) + // 2. `remove --force` deletes the .gitnexus/ directory + // AND unregisters from the global registry + // 3. `remove ` is idempotent (exit 0 with a warning) + // 4. `remove ` (two entries share the alias via + // --allow-duplicate-name) exits 1 with a disambiguation hint + // and leaves the registry unchanged. + // + // Every assertion reads the real registry.json on disk, so any + // regression in remove.ts → resolveRegistryEntry → unregisterRepo + // will surface here. + describe('remove (#664)', () => { + it('dry-run lists, --force deletes, missing target is a no-op warning', () => { + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-remove-')); + const repoA = makeMiniRepoCopy('remove-me', 'gn-rm-a-'); + const parentA = path.dirname(repoA); + + try { + // Index the repo under a custom alias so we can target it by + // name below. `--name` guarantees a stable alias regardless of + // how the host resolves the basename/remote-inferred name. + const r1 = runCliWithEnv( + ['analyze', '--name', 'alias-a'], + repoA, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; + expect( + r1.status, + [`analyze exited with ${r1.status}`, `stdout: ${r1.stdout}`, `stderr: ${r1.stderr}`].join( + '\n', + ), + ).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const afterIndex = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterIndex).toHaveLength(1); + expect(afterIndex[0].name).toBe('alias-a'); + // Storage dir must exist before remove so we can assert its + // disappearance below. + const storagePath = afterIndex[0].storagePath; + expect(fs.existsSync(storagePath)).toBe(true); + + // Dry-run: must NOT delete. Use parentA as cwd so the test + // never runs with the to-be-removed storage dir as its cwd. + // + // Assert the FULL dry-run output shape, not just the `--force` + // hint (#1003 senior-reviewer NIT): `remove.ts` prints the + // alias, the resolved path, AND the storage path. Verifying + // all three appear catches silent format regressions + // (e.g. a future refactor that accidentally drops one of the + // three `console.log` lines, or swaps `entry.path` for + // `entry.name` in the output). + const r2 = runCliWithEnv(['remove', 'alias-a'], parentA, { GITNEXUS_HOME: gnHome }, 15000); + if (r2.status === null) return; + expect(r2.status).toBe(0); + const r2Output = `${r2.stdout}${r2.stderr}`; + expect(r2Output).toMatch(/Run with --force/i); + expect(r2Output, 'dry-run must surface the alias').toContain('alias-a'); + expect(r2Output, 'dry-run must surface the repo path').toContain(afterIndex[0].path); + expect(r2Output, 'dry-run must surface the storage path').toContain(storagePath); + expect(fs.existsSync(storagePath)).toBe(true); + // Registry still has the entry. + expect(JSON.parse(fs.readFileSync(registryPath, 'utf-8'))).toHaveLength(1); + + // --force: must delete storage AND unregister. + const r3 = runCliWithEnv( + ['remove', 'alias-a', '--force'], + parentA, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r3.status === null) return; + expect( + r3.status, + [ + `remove --force exited with ${r3.status}`, + `stdout: ${r3.stdout}`, + `stderr: ${r3.stderr}`, + ].join('\n'), + ).toBe(0); + // Success-case output shape: `Removed: ` header plus the + // same path-and-storagePath lines the dry-run prints (same NIT + // rationale — the success branch mirrors the dry-run's three + // console.log calls, so it has the same silent-regression risk). + const r3Output = `${r3.stdout}${r3.stderr}`; + expect(r3Output).toMatch(/Removed/i); + expect(r3Output, 'success output must surface the alias').toContain('alias-a'); + expect(r3Output, 'success output must surface the repo path').toContain(afterIndex[0].path); + expect(r3Output, 'success output must surface the storage path').toContain(storagePath); + expect(fs.existsSync(storagePath)).toBe(false); + expect(JSON.parse(fs.readFileSync(registryPath, 'utf-8'))).toHaveLength(0); + + // Idempotent: removing the same alias AGAIN must exit 0 with a + // warning (so `remove X && analyze Y` keeps working in scripts). + const r4 = runCliWithEnv(['remove', 'alias-a'], parentA, { GITNEXUS_HOME: gnHome }, 15000); + if (r4.status === null) return; + expect(r4.status).toBe(0); + expect(`${r4.stdout}${r4.stderr}`).toMatch(/Nothing to remove/i); + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentA, { recursive: true, force: true }); + } + }, 180000); // 3-min outer budget (1 × ~60s analyze + 3 × fast remove calls) + + it('ambiguous target (two entries share alias via --allow-duplicate-name) errors without mutating registry', () => { + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-rm-amb-')); + const repoA = makeMiniRepoCopy('dup', 'gn-dup-a-'); + const repoB = makeMiniRepoCopy('dup', 'gn-dup-b-'); + const parentA = path.dirname(repoA); + const parentB = path.dirname(repoB); + + try { + // Two repos registered under the same alias — only possible via + // --allow-duplicate-name (#829). + const r1 = runCliWithEnv( + ['analyze', '--name', 'shared'], + repoA, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; + expect(r1.status).toBe(0); + + const r2 = runCliWithEnv( + ['analyze', '--name', 'shared', '--allow-duplicate-name'], + repoB, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r2.status === null) return; + expect(r2.status).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const before = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(before).toHaveLength(2); + + // `remove shared` must refuse to guess — exit 1, disambiguation hint. + const r3 = runCliWithEnv( + ['remove', 'shared', '--force'], + parentA, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r3.status === null) return; + expect(r3.status).toBe(1); + const r3Output = `${r3.stdout}${r3.stderr}`; + expect(r3Output).toMatch(/Multiple registered repos match/i); + // Both paths must be surfaced in the hint so the user knows + // which ones to disambiguate between. + expect(r3Output).toMatch(/dup/); + + // Registry unchanged — the failed resolution must NOT have + // mutated state. + const after = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(after).toHaveLength(2); + + // And path-based remove still works: pass the absolute path of + // repoA and it resolves unambiguously. + // + // We pull the path from the registry snapshot rather than + // passing the outer `repoA` variable directly. This is the + // belt-and-suspenders for cross-platform path normalisation + // (#1003 review): the path the registry recorded has already + // gone through the analyze-side canonicalisation (which on + // macOS expands /var → /private/var and on Windows expands 8.3 + // → long-name). Passing that exact string back to `remove` + // guarantees the comparison succeeds even on runners where the + // outer `repoA` is the symlink/short-name form. The code-side + // fix in `canonicalizePath` makes this redundant in practice, + // but the test shouldn't depend on the code fix being perfect + // on every platform — it should prove correctness against the + // registry contract. + const repoAEntry = before.find( + (e: { path: string }) => + path.basename(e.path) === 'dup' && e.path.includes(path.basename(parentA)), + ); + expect( + repoAEntry, + 'repoA entry must exist in registry before path-remove step', + ).toBeDefined(); + + const r4 = runCliWithEnv( + ['remove', repoAEntry.path, '--force'], + parentA, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r4.status === null) return; + expect( + r4.status, + [ + `remove-by-path exited with ${r4.status}`, + `stdout: ${r4.stdout}`, + `stderr: ${r4.stderr}`, + ].join('\n'), + ).toBe(0); + const finalEntries = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(finalEntries).toHaveLength(1); + // The survivor is repoB (its path stays in the registry). + expect(path.basename(finalEntries[0].path)).toBe('dup'); + // And it's NOT the one we just removed. + expect(finalEntries[0].path).not.toBe(repoAEntry.path); + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentA, { recursive: true, force: true }); + fs.rmSync(parentB, { recursive: true, force: true }); + } + }, 240000); // 4-min outer budget (2 × ~60s analyze + 2 × fast remove) + + it('refuses to proceed when a registry entry points storagePath outside /.gitnexus (#1003)', () => { + // Regression guard for the safety gap flagged by @magyargergo on + // PR #1003: `~/.gitnexus/registry.json` is a user-writable JSON + // file, so a corrupted or hand-edited entry could point + // storagePath at the repo root (catastrophic: rm the working + // tree) or at any other arbitrary path. `remove --force` must + // refuse to call fs.rm when storagePath isn't the canonical + // `/.gitnexus`. We verify: + // 1. Exit code 1 with the actionable "registry entry corrupted" + // hint. + // 2. The .gitnexus/ storage dir is UNTOUCHED. + // 3. The repo itself (entry.path) is UNTOUCHED. + // 4. The registry entry is NOT removed (no partial mutation). + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-poison-')); + const repo = makeMiniRepoCopy('poisoned', 'gn-poison-'); + const parent = path.dirname(repo); + + try { + // Index the repo normally first so the registry has a valid + // entry we can then poison. + const r1 = runCliWithEnv( + ['analyze', '--name', 'poisoned-alias'], + repo, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; + expect(r1.status).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const original = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(original).toHaveLength(1); + + // Poison the entry: set storagePath to the REPO ROOT itself. + // If the guard isn't in place, `remove --force` would call + // `fs.rm(repo, {recursive: true, force: true})` and wipe the + // entire working tree. + const poisoned = [{ ...original[0], storagePath: repo }]; + fs.writeFileSync(registryPath, JSON.stringify(poisoned, null, 2)); + + // Sanity: storage dir and working tree both still exist. + expect(fs.existsSync(path.join(repo, '.gitnexus'))).toBe(true); + expect(fs.existsSync(repo)).toBe(true); + expect(fs.existsSync(path.join(repo, '.git'))).toBe(true); + + // Attempt the remove — must FAIL without deleting anything. + const r2 = runCliWithEnv( + ['remove', 'poisoned-alias', '--force'], + parent, + { GITNEXUS_HOME: gnHome }, + 15000, + ); + if (r2.status === null) return; + + expect( + r2.status, + [`remove should have exited 1`, `stdout: ${r2.stdout}`, `stderr: ${r2.stderr}`].join( + '\n', + ), + ).toBe(1); + const r2Output = `${r2.stdout}${r2.stderr}`; + // Must surface the actionable "registry corrupted" hint, not + // just a raw fs.rm error. + expect(r2Output).toMatch(/Refusing to remove/i); + expect(r2Output).toMatch(/registry\.json/i); + + // Repo + .gitnexus dir + .git dir must all still exist — the + // guard aborts BEFORE fs.rm. This is the whole point of the + // test: the working tree is not allowed to disappear. + expect(fs.existsSync(repo), 'repo working tree must survive').toBe(true); + expect(fs.existsSync(path.join(repo, '.gitnexus')), 'storage dir must survive').toBe(true); + expect(fs.existsSync(path.join(repo, '.git')), '.git must survive').toBe(true); + + // Registry unchanged — no partial mutation. + const afterRegistry = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterRegistry).toHaveLength(1); + expect(afterRegistry[0].storagePath).toBe(repo); // still poisoned (we did that) + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parent, { recursive: true, force: true }); + } + }, 120000); // 2-min budget (1 × ~60s analyze + 1 × fast remove-refused) + }); + + // ─── clean --all: same safety guard applies (#1003 review) ─────── + // + // The `clean --all` path iterates over the registry and calls + // `fs.rm(entry.storagePath)` — identical trust-the-registry pattern + // as `remove` had before the guard. A poisoned entry must be SKIPPED + // (not aborted), so clean --all preserves its existing per-repo + // error-tolerance semantics: one bad entry does not halt cleanup of + // the rest. We verify: + // 1. The poisoned entry is NOT deleted (working tree + .gitnexus + // survive), and the CLI prints a "Refusing to clean" message. + // 2. The poisoned entry is left in the registry (nothing was + // mutated for it). + // 3. A co-existing well-formed entry IS still cleaned (both its + // .gitnexus dir AND its registry entry are gone). + describe('clean --all with a poisoned registry entry (#1003)', () => { + it('skips poisoned entries, cleans valid ones, never deletes the working tree', () => { + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-clean-poison-')); + const repoBad = makeMiniRepoCopy('bad-repo', 'gn-clean-bad-'); + const repoGood = makeMiniRepoCopy('good-repo', 'gn-clean-good-'); + const parentBad = path.dirname(repoBad); + const parentGood = path.dirname(repoGood); + + try { + // Analyze both so the registry has two well-formed entries. + for (const [repo, alias] of [ + [repoBad, 'bad-alias'], + [repoGood, 'good-alias'], + ] as const) { + const r = runCliWithEnv( + ['analyze', '--name', alias], + repo, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r.status === null) return; + expect(r.status, `analyze ${alias} exited ${r.status}: ${r.stdout}${r.stderr}`).toBe(0); + } + + const registryPath = path.join(gnHome, 'registry.json'); + const original = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(original).toHaveLength(2); + + // Poison the 'bad-alias' entry by pointing its storagePath at + // the repo root itself. If the guard isn't wired into the + // clean --all loop, `clean --all --force` would fs.rm the + // working tree. + const poisoned = original.map((e: { name: string; storagePath: string; path: string }) => + e.name === 'bad-alias' ? { ...e, storagePath: repoBad } : e, + ); + fs.writeFileSync(registryPath, JSON.stringify(poisoned, null, 2)); + + // Sanity: both working trees and .gitnexus dirs still exist. + expect(fs.existsSync(repoBad)).toBe(true); + expect(fs.existsSync(path.join(repoBad, '.gitnexus'))).toBe(true); + expect(fs.existsSync(path.join(repoBad, '.git'))).toBe(true); + expect(fs.existsSync(path.join(repoGood, '.gitnexus'))).toBe(true); + + // clean --all --force from a neutral cwd (parentBad), so the + // command isn't "inside" either repo. + const r = runCliWithEnv( + ['clean', '--all', '--force'], + parentBad, + { GITNEXUS_HOME: gnHome }, + 30000, + ); + if (r.status === null) return; + + // clean --all's per-entry error handling always exits 0 at + // the end (it only logs per-repo failures). The important + // assertions are on side effects, not the exit code. + const output = `${r.stdout}${r.stderr}`; + expect(output).toMatch(/Refusing to clean/i); + expect(output).toMatch(/bad-alias/); + + // Poisoned repo: working tree + .gitnexus + .git all SURVIVE. + expect(fs.existsSync(repoBad), 'poisoned repo working tree must survive').toBe(true); + expect( + fs.existsSync(path.join(repoBad, '.gitnexus')), + 'poisoned repo .gitnexus must survive (guard refused to rm repo root)', + ).toBe(true); + expect(fs.existsSync(path.join(repoBad, '.git')), '.git must survive').toBe(true); + + // Good repo: its .gitnexus IS gone (cleanup succeeded despite + // the poisoned sibling entry — per-entry error tolerance is + // preserved). + expect( + fs.existsSync(path.join(repoGood, '.gitnexus')), + 'good repo .gitnexus should be cleaned', + ).toBe(false); + // But the good repo's working tree stays (clean never touches + // anything outside .gitnexus). + expect(fs.existsSync(repoGood), 'good repo working tree must survive').toBe(true); + + // Registry post-state: poisoned entry still present (skipped, + // not mutated); good entry unregistered. + const afterRegistry = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterRegistry).toHaveLength(1); + expect(afterRegistry[0].name).toBe('bad-alias'); + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentBad, { recursive: true, force: true }); + fs.rmSync(parentGood, { recursive: true, force: true }); + } + }, 240000); // 4-min budget (2 × ~60s analyze + 1 × fast clean --all) + }); + describe('unhappy path', () => { it('exits with error when no command is given', () => { const result = runCliRaw([], MINI_REPO); diff --git a/gitnexus/test/integration/group/group-cli.test.ts b/gitnexus/test/integration/group/group-cli.test.ts index 02da904dc..7a76f86fd 100644 --- a/gitnexus/test/integration/group/group-cli.test.ts +++ b/gitnexus/test/integration/group/group-cli.test.ts @@ -65,4 +65,51 @@ describe('group CLI', () => { const blanketClosePattern = /closeLbug\s*\(\s*\)/; expect(source).not.toMatch(blanketClosePattern); }); + + it('group impact requires --target and --repo', () => { + const c = runGroup(['create', 'impcli']); + expect(c.status).toBe(0); + const r = runGroup(['impact', 'impcli']); + expect(r.status).not.toBe(0); + }); + + it('group impact runs with Issue #794 style flags (fixture-backed home)', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-cli-impact-')); + try { + const gd = path.join(home, 'groups', 'test-group'); + fs.mkdirSync(gd, { recursive: true }); + fs.copyFileSync( + path.join(repoRoot, 'test', 'fixtures', 'group', 'group.yaml'), + path.join(gd, 'group.yaml'), + ); + const r = spawnSync( + process.execPath, + [ + '--import', + tsxImportUrl, + cliEntry, + 'group', + 'impact', + 'test-group', + '--target', + 'health', + '--repo', + 'app/backend', + '--json', + ], + { + cwd: repoRoot, + encoding: 'utf8', + timeout: 20000, + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, GITNEXUS_HOME: home }, + }, + ); + expect(r.status).not.toBe(0); + const msg = `${r.stderr}\n${r.stdout}`; + expect(msg).toMatch(/error|indexed|not found|repository/i); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); }); diff --git a/gitnexus/test/integration/group/group-impact.test.ts b/gitnexus/test/integration/group/group-impact.test.ts new file mode 100644 index 000000000..50331cc90 --- /dev/null +++ b/gitnexus/test/integration/group/group-impact.test.ts @@ -0,0 +1,74 @@ +/** + * Group impact: exercise GroupService.groupImpact with fixture-backed group config + * and a stubbed port (no LadybugDB / bridge required when local impact yields no UIDs). + */ +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import os from 'node:os'; +import { GroupService, type GroupToolPort } from '../../../src/core/group/service.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const fixturesDir = path.resolve(__dirname, '../../fixtures/group'); + +let tmpHome: string; + +beforeAll(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-grp-impact-int-')); + const groupDir = path.join(tmpHome, 'groups', 'test-group'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.copyFileSync(path.join(fixturesDir, 'group.yaml'), path.join(groupDir, 'group.yaml')); +}); + +afterAll(() => { + if (tmpHome) fs.rmSync(tmpHome, { recursive: true, force: true }); +}); + +function stubPort(): GroupToolPort { + return { + resolveRepo: vi.fn(async () => ({ + id: 'stub', + name: 'stub', + repoPath: '/tmp/repo', + storagePath: '/tmp/.gitnexus', + })), + impact: vi.fn(async () => ({ + target: {}, + byDepth: {}, + summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, + risk: 'LOW', + })), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; +} + +describe('group impact integration', () => { + it('returns validation error when parameters are incomplete', async () => { + const svc = new GroupService(stubPort()); + const r = (await svc.groupImpact({ name: 'x', direction: 'upstream' })) as { error: string }; + expect(r.error).toMatch(/repo is required|target is required/); + }); + + it('runs happy-path stub against fixture group (stops before bridge when no symbol UIDs)', async () => { + const prev = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome; + try { + const svc = new GroupService(stubPort()); + const r = (await svc.groupImpact({ + name: 'test-group', + repo: 'app/backend', + target: 'health', + direction: 'upstream', + })) as { group?: string; error?: string; cross?: unknown[] }; + expect(r.error).toBeUndefined(); + expect(r.group).toBe('test-group'); + expect(Array.isArray(r.cross)).toBe(true); + } finally { + if (prev === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = prev; + } + }); +}); diff --git a/gitnexus/test/unit/bm25-search.test.ts b/gitnexus/test/unit/bm25-search.test.ts index 1d8be757f..131743099 100644 --- a/gitnexus/test/unit/bm25-search.test.ts +++ b/gitnexus/test/unit/bm25-search.test.ts @@ -1,5 +1,33 @@ -import { describe, it, expect } from 'vitest'; -import { searchFTSFromLbug, type BM25SearchResult } from '../../src/core/search/bm25-index.js'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + searchFTSFromLbug, + invalidateEnsuredFTSForRepo, + type BM25SearchResult, +} from '../../src/core/search/bm25-index.js'; + +vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + queryFTS: vi.fn().mockResolvedValue([]), + }; +}); + +// Pool adapter is dynamically imported by the MCP-pool path of +// `searchFTSFromLbug`. We mock it so we can drive the executor and the +// pool-close listener without spinning up a real LadybugDB pool. +const poolCloseListeners: Array<(repoId: string) => void> = []; +const mockExecuteQuery = vi.fn(); +vi.mock('../../src/core/lbug/pool-adapter.js', () => ({ + executeQuery: (repoId: string, cypher: string) => mockExecuteQuery(repoId, cypher), + addPoolCloseListener: (listener: (repoId: string) => void) => { + poolCloseListeners.push(listener); + return () => { + const idx = poolCloseListeners.indexOf(listener); + if (idx !== -1) poolCloseListeners.splice(idx, 1); + }; + }, +})); describe('BM25 search', () => { describe('searchFTSFromLbug', () => { @@ -32,5 +60,255 @@ describe('BM25 search', () => { expect(result.score).toBe(1.5); expect(result.rank).toBe(1); }); + + it('accepts optional nodeIds field', () => { + const result: BM25SearchResult = { + filePath: 'src/index.ts', + score: 1.5, + rank: 1, + nodeIds: ['func:id1', 'func:id2'], + }; + expect(result.nodeIds).toEqual(['func:id1', 'func:id2']); + }); + }); + + describe('score aggregation', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('sums only top-3 scoring nodes per file when more than 3 match', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + // File table: empty; Function table: 5 hits for the same file; rest: empty + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — 5 hits, scores 10/9/8/7/6 + { filePath: 'src/views.py', score: 10, nodeId: 'func:node1', name: 'get_queryset' }, + { filePath: 'src/views.py', score: 9, nodeId: 'func:node2', name: 'post' }, + { filePath: 'src/views.py', score: 8, nodeId: 'func:node3', name: 'delete' }, + { filePath: 'src/views.py', score: 7, nodeId: 'func:node4', name: 'patch' }, + { filePath: 'src/views.py', score: 6, nodeId: 'func:node5', name: 'put' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('queryset'); + + expect(results).toHaveLength(1); + expect(results[0].filePath).toBe('src/views.py'); + // Only top-3 scores (10+9+8=27), not naive sum of all 5 (10+9+8+7+6=40) + expect(results[0].score).toBe(27); + expect(results[0].nodeIds).toEqual(['func:node1', 'func:node2', 'func:node3']); + }); + + it('propagates nodeIds for files with fewer than 3 matching nodes', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — 2 hits + { filePath: 'src/models.py', score: 5, nodeId: 'func:m1', name: 'save' }, + { filePath: 'src/models.py', score: 3, nodeId: 'func:m2', name: 'delete' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('model'); + + expect(results).toHaveLength(1); + expect(results[0].score).toBe(8); // 5+3 + expect(results[0].nodeIds).toEqual(['func:m1', 'func:m2']); + }); + + it('filters out empty nodeIds', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — nodes with no id + { filePath: 'src/utils.py', score: 5, nodeId: '', name: 'helper' }, + { filePath: 'src/utils.py', score: 3, nodeId: '', name: 'util' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('util'); + + expect(results).toHaveLength(1); + expect(results[0].nodeIds).toEqual([]); + }); + + it('merges hits across multiple index tables for the same file', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([ + // File table + { filePath: 'src/auth.py', score: 4, nodeId: 'file:auth', name: 'auth.py' }, + ]) + .mockResolvedValueOnce([ + // Function table + { filePath: 'src/auth.py', score: 9, nodeId: 'func:login', name: 'login' }, + ]) + .mockResolvedValueOnce([ + // Class table + { filePath: 'src/auth.py', score: 7, nodeId: 'cls:User', name: 'User' }, + ]) + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('auth'); + + expect(results).toHaveLength(1); + // All 3 hits (scores 9+7+4=20) — each from a different table, all top-3 + expect(results[0].score).toBe(20); + expect(results[0].nodeIds).toEqual(['func:login', 'cls:User', 'file:auth']); + }); + + it('ranks files by aggregated score descending', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — hits across two files + { filePath: 'src/low.py', score: 2, nodeId: 'func:a', name: 'a' }, + { filePath: 'src/high.py', score: 9, nodeId: 'func:b', name: 'b' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('fn'); + + expect(results[0].filePath).toBe('src/high.py'); + expect(results[1].filePath).toBe('src/low.py'); + expect(results[0].rank).toBe(1); + expect(results[1].rank).toBe(2); + }); + }); + + describe('ensureFTS cache (MCP pool path)', () => { + const REPO = 'test-repo-fts-cache'; + + beforeEach(() => { + // Clean state so cases don't bleed into each other. + mockExecuteQuery.mockReset(); + invalidateEnsuredFTSForRepo(REPO); + // Suppress the surfaced warn so test output stays readable. + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + it('does NOT cache a transient CREATE_FTS_INDEX failure — second call retries', async () => { + // First call: every CREATE_FTS_INDEX fails transiently; QUERY_FTS_INDEX returns nothing. + mockExecuteQuery.mockImplementation(async (_repo: string, cypher: string) => { + if (cypher.includes('CREATE_FTS_INDEX')) { + throw new Error('transient lock error: Could not set lock'); + } + return []; + }); + + const r1 = await searchFTSFromLbug('anything', 5, REPO); + expect(Array.isArray(r1)).toBe(true); + + const createCallsAfterFirst = mockExecuteQuery.mock.calls.filter((c) => + String(c[1]).includes('CREATE_FTS_INDEX'), + ).length; + // 5 FTS index tables — all five attempted on first call. + expect(createCallsAfterFirst).toBe(5); + + // Second call: CREATE succeeds this time. The bug being fixed: if the + // first failure was cached, we'd see ZERO additional CREATE calls. + mockExecuteQuery.mockReset(); + mockExecuteQuery.mockResolvedValue([]); + + await searchFTSFromLbug('anything', 5, REPO); + + const createCallsOnRetry = mockExecuteQuery.mock.calls.filter((c) => + String(c[1]).includes('CREATE_FTS_INDEX'), + ).length; + expect(createCallsOnRetry).toBe(5); + }); + + it("treats 'already exists' as success and caches it (no retry on second call)", async () => { + mockExecuteQuery.mockImplementation(async (_repo: string, cypher: string) => { + if (cypher.includes('CREATE_FTS_INDEX')) { + throw new Error("Catalog exception: index 'file_fts' already exists"); + } + return []; + }); + + await searchFTSFromLbug('anything', 5, REPO); + mockExecuteQuery.mockReset(); + mockExecuteQuery.mockResolvedValue([]); + + await searchFTSFromLbug('anything', 5, REPO); + + const createCallsOnSecond = mockExecuteQuery.mock.calls.filter((c) => + String(c[1]).includes('CREATE_FTS_INDEX'), + ).length; + expect(createCallsOnSecond).toBe(0); + }); + + it('invalidateEnsuredFTSForRepo drops cached entries so next call re-issues CREATE', async () => { + // Prime the cache with successful creates. + mockExecuteQuery.mockResolvedValue([]); + await searchFTSFromLbug('anything', 5, REPO); + + mockExecuteQuery.mockReset(); + mockExecuteQuery.mockResolvedValue([]); + + // Without invalidation: no re-CREATE. + await searchFTSFromLbug('anything', 5, REPO); + expect( + mockExecuteQuery.mock.calls.filter((c) => String(c[1]).includes('CREATE_FTS_INDEX')).length, + ).toBe(0); + + // After invalidation: next call re-issues CREATE for all 5 tables. + invalidateEnsuredFTSForRepo(REPO); + mockExecuteQuery.mockReset(); + mockExecuteQuery.mockResolvedValue([]); + await searchFTSFromLbug('anything', 5, REPO); + expect( + mockExecuteQuery.mock.calls.filter((c) => String(c[1]).includes('CREATE_FTS_INDEX')).length, + ).toBe(5); + }); + + it('a pool-close listener fired by the pool adapter invalidates this repo only', async () => { + const OTHER = 'other-repo'; + + mockExecuteQuery.mockResolvedValue([]); + // Prime both repos. + await searchFTSFromLbug('anything', 5, REPO); + await searchFTSFromLbug('anything', 5, OTHER); + + // Confirm at least one listener was registered by the search module. + expect(poolCloseListeners.length).toBeGreaterThanOrEqual(1); + + // Simulate the pool adapter closing REPO. + for (const l of poolCloseListeners) l(REPO); + + mockExecuteQuery.mockReset(); + mockExecuteQuery.mockResolvedValue([]); + + await searchFTSFromLbug('anything', 5, REPO); + const createForRepo = mockExecuteQuery.mock.calls.filter( + (c) => c[0] === REPO && String(c[1]).includes('CREATE_FTS_INDEX'), + ).length; + expect(createForRepo).toBe(5); + + // OTHER repo's cache must remain intact — no re-CREATE for it. + mockExecuteQuery.mockReset(); + mockExecuteQuery.mockResolvedValue([]); + await searchFTSFromLbug('anything', 5, OTHER); + const createForOther = mockExecuteQuery.mock.calls.filter( + (c) => c[0] === OTHER && String(c[1]).includes('CREATE_FTS_INDEX'), + ).length; + expect(createForOther).toBe(0); + + invalidateEnsuredFTSForRepo(OTHER); + }); }); }); diff --git a/gitnexus/test/unit/chunker.test.ts b/gitnexus/test/unit/chunker.test.ts index 77ac839ba..91bda726a 100644 --- a/gitnexus/test/unit/chunker.test.ts +++ b/gitnexus/test/unit/chunker.test.ts @@ -209,11 +209,12 @@ describe('chunkNode', () => { const result = await chunkNode('Class', content, 'test.ts', 1, 6, 90, 0); expect(result).toHaveLength(2); + expect(result[0].text).toContain('class Parser {'); expect(result[0].text).toContain('options: ParserOptions;'); expect(result[0].text).toContain('cache: Map;'); expect(result[1].text).toContain('parseJSON()'); expect(result[1].text).toContain('validate()'); - expect(result[0].startLine).toBe(2); + expect(result[0].startLine).toBe(1); expect(result[1].startLine).toBe(4); }); @@ -237,11 +238,44 @@ describe('chunkNode', () => { const result = await chunkNode('Interface', content, 'test.ts', 10, 14, 500, 0); expect(result).toHaveLength(1); + expect(result[0].text).toContain('interface Handler {'); expect(result[0].text).toContain('handle(event: Event): void;'); expect(result[0].text).toContain('validate(input: string): boolean;'); expect(result[0].text).toContain('readonly name: string;'); }); + it('uses declaration-aware chunking for Struct labels', async () => { + const content = [ + 'struct User {', + ' name: String,', + ' email: String,', + ' age: u32,', + ' address: String,', + '}', + ].join('\n'); + const tree = makeDeclarationTree('struct_item', 'declaration_list', content, [ + 'name: String,', + 'email: String,', + 'age: u32,', + 'address: String,', + ]); + createParserForLanguage.mockResolvedValue({ + parse: vi.fn().mockReturnValue(tree), + }); + + const result = await chunkNode('Struct', content, 'test.rs', 40, 45, 45, 0); + + expect(result).toHaveLength(2); + expect(result[0].text).toContain('struct User {'); + expect(result[0].text).toContain('name: String,'); + expect(result[0].text).toContain('email: String'); + const combinedText = result.map((chunk) => chunk.text).join('\n'); + expect(combinedText).toContain('email: String'); + expect(combinedText).toContain('age: u32'); + expect(combinedText).toContain('address: String'); + expect(result[0].startLine).toBe(40); + }); + it('splits a function into multiple AST-aware chunks using snippet offsets', async () => { const content = [ 'function example() {', diff --git a/gitnexus/test/unit/cli-index-help.test.ts b/gitnexus/test/unit/cli-index-help.test.ts index 96e3eab81..59109c8d9 100644 --- a/gitnexus/test/unit/cli-index-help.test.ts +++ b/gitnexus/test/unit/cli-index-help.test.ts @@ -43,6 +43,16 @@ describe('CLI help surface', () => { expect(result.stdout).toContain('--repo '); }); + it('detect-changes help exposes compare scope and base-ref flags', () => { + const result = runHelp('detect-changes'); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('gitnexus detect-changes|detect_changes [options]'); + expect(result.stdout).toContain('--scope '); + expect(result.stdout).toContain('--base-ref '); + expect(result.stdout).toContain('--repo '); + }); + it('wiki help shows provider, review, and verbose flags', () => { const result = runHelp('wiki'); diff --git a/gitnexus/test/unit/embedding-chunking.test.ts b/gitnexus/test/unit/embedding-chunking.test.ts index 62fdd2b23..244efe63d 100644 --- a/gitnexus/test/unit/embedding-chunking.test.ts +++ b/gitnexus/test/unit/embedding-chunking.test.ts @@ -18,12 +18,19 @@ vi.mock('../../src/core/tree-sitter/parser-loader.js', () => ({ resolveLanguageKey: vi.fn((language: string) => language), })); -vi.mock('gitnexus-shared', () => ({ +const { getLanguageFromFilename } = vi.hoisted(() => ({ getLanguageFromFilename: vi.fn().mockReturnValue('typescript'), })); +vi.mock('gitnexus-shared', () => ({ + getLanguageFromFilename, +})); + import { chunkNode } from '../../src/core/embeddings/chunker.js'; +const CLASS_PREV_TAIL_SAMPLE = 30; +const STRUCT_PREV_TAIL_SAMPLE = 20; + type FakeNode = { type: string; startIndex: number; @@ -183,10 +190,18 @@ describe('embedding-chunking integration', () => { const chunks = await chunkNode(node.label, node.content, node.filePath, 20, 25, 90, 0); expect(chunks).toHaveLength(2); - const secondText = generateEmbeddingText(node, chunks[1].text); + const secondText = generateEmbeddingText( + node, + chunks[1].text, + {}, + chunks[1].chunkIndex, + chunks[0].text.slice(-CLASS_PREV_TAIL_SAMPLE), + ); expect(secondText).toContain('Class: Parser'); - expect(secondText).toContain('Methods: parseJSON, validate'); - expect(secondText).toContain('Properties: options, cache'); + expect(secondText).toContain('Container: class Parser {'); + expect(secondText).toContain('[preceding context]: ...'); + expect(secondText).not.toContain('Methods: parseJSON, validate'); + expect(secondText).not.toContain('Properties: options, cache'); expect(secondText).toContain('parseJSON(text: string)'); }); @@ -220,9 +235,53 @@ describe('embedding-chunking integration', () => { const text = generateEmbeddingText(node, chunks[0].text); expect(text).toContain('Interface: Handler'); expect(text).toContain('Methods: handle, validate'); + expect(text).toContain('Container: interface Handler {'); expect(text).toContain('readonly name: string;'); }); + it('struct chunks retain structural container context', async () => { + getLanguageFromFilename.mockReturnValue('rust'); + const node = makeNode({ + label: 'Struct', + name: 'User', + fieldNames: ['name', 'email', 'age', 'address'], + content: `struct User { + name: String, + email: String, + age: u32, + address: String, +}`, + startLine: 40, + endLine: 45, + filePath: 'src/user.rs', + }); + createParserForLanguage.mockResolvedValue({ + parse: vi.fn().mockReturnValue( + makeDeclarationTree('struct_item', 'declaration_list', node.content, [ + { text: 'name: String,', type: 'field_definition' }, + { text: 'email: String,', type: 'field_definition' }, + { text: 'age: u32,', type: 'field_definition' }, + { text: 'address: String,', type: 'field_definition' }, + ]), + ), + }); + + const chunks = await chunkNode(node.label, node.content, node.filePath, 40, 45, 45, 0); + expect(chunks).toHaveLength(2); + + const secondText = generateEmbeddingText( + node, + chunks[1].text, + {}, + chunks[1].chunkIndex, + chunks[0].text.slice(-STRUCT_PREV_TAIL_SAMPLE), + ); + expect(secondText).toContain('Struct: User'); + expect(secondText).toContain('Container: struct User {'); + expect(secondText).not.toContain('Properties: name, email, age, address'); + expect(secondText).toContain('age: u32,'); + }); + it('metadata is present in every chunk', () => { const longContent = 'x'.repeat(3000); const node = makeNode({ diff --git a/gitnexus/test/unit/embedding-pipeline.test.ts b/gitnexus/test/unit/embedding-pipeline.test.ts index 5276fd3da..caa160d1e 100644 --- a/gitnexus/test/unit/embedding-pipeline.test.ts +++ b/gitnexus/test/unit/embedding-pipeline.test.ts @@ -1,11 +1,17 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { createHash } from 'crypto'; -import { contentHashForNode } from '../../src/core/embeddings/embedding-pipeline.js'; +import { + contentHashForNode, + EMBEDDING_TEXT_VERSION, +} from '../../src/core/embeddings/embedding-pipeline.js'; import { generateEmbeddingText } from '../../src/core/embeddings/text-generator.js'; import type { EmbeddableNode, EmbeddingProgress } from '../../src/core/embeddings/types.js'; import { DEFAULT_EMBEDDING_CONFIG, EMBEDDABLE_LABELS } from '../../src/core/embeddings/types.js'; import { STALE_HASH_SENTINEL } from '../../src/core/lbug/schema.js'; +const CLASS_CHUNK_SIZE = 90; +const CLASS_OVERLAP = 10; + // ──────────────────────────────────────────────────────────────────────────── // contentHashForNode // ──────────────────────────────────────────────────────────────────────────── @@ -32,6 +38,8 @@ describe('contentHashForNode', () => { it('matches sha1(generateEmbeddingText(node, node.content))', () => { const node = makeNode(); const expected = createHash('sha1') + .update(EMBEDDING_TEXT_VERSION) + .update('\n') .update(generateEmbeddingText(node, node.content)) .digest('hex'); expect(contentHashForNode(node)).toBe(expected); @@ -56,6 +64,10 @@ describe('contentHashForNode', () => { const hashWithFullDefaults = contentHashForNode(node, DEFAULT_EMBEDDING_CONFIG); expect(hashWithEmptyConfig).toBe(hashWithFullDefaults); }); + + it('exports a text template version marker', () => { + expect(EMBEDDING_TEXT_VERSION).toBe('v2'); + }); }); // ──────────────────────────────────────────────────────────────────────────── @@ -439,6 +451,118 @@ describe('runEmbeddingPipeline incremental filter', () => { expect(vectorIndexCalls.length).toBeGreaterThanOrEqual(1); }); + it('does not inject preceding context when overlap is disabled', async () => { + const embedBatchSpy = vi + .fn() + .mockImplementation((texts: string[]) => + Promise.resolve(texts.map(() => new Float32Array(384))), + ); + vi.doMock('../../src/core/embeddings/embedder.js', () => ({ + initEmbedder: vi.fn().mockResolvedValue(undefined), + embedBatch: embedBatchSpy, + embedText: vi.fn().mockResolvedValue(new Float32Array(384)), + embeddingToArray: vi.fn().mockImplementation((emb: Float32Array) => Array.from(emb)), + isEmbedderReady: vi.fn().mockReturnValue(true), + })); + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + loadVectorExtension: vi.fn().mockResolvedValue(undefined), + })); + + const node = makeNode({ + label: 'Class', + name: 'Parser', + content: `class Parser { + options: ParserOptions; + cache: Map; + parseJSON() { return JSON.parse("{}"); } + validate() { return true; } +}`, + startLine: 1, + endLine: 6, + }); + + const executeQuery = mockExecuteQuery([node]); + const executeWithReusedStatement = mockExecuteWithReusedStatement(); + + const { runEmbeddingPipeline } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + + await runEmbeddingPipeline( + executeQuery, + executeWithReusedStatement, + onProgress, + { chunkSize: 90, overlap: 0 }, + undefined, + undefined, + new Map(), + ); + + const embeddedTexts = embedBatchSpy.mock.calls.flatMap((call) => call[0] as string[]); + const laterChunks = embeddedTexts.slice(1); + expect(laterChunks.length).toBeGreaterThan(0); + for (const text of laterChunks) { + expect(text).not.toContain('[preceding context]:'); + } + }); + + it('truncates preceding context to the configured overlap size', async () => { + const embedBatchSpy = vi + .fn() + .mockImplementation((texts: string[]) => + Promise.resolve(texts.map(() => new Float32Array(384))), + ); + vi.doMock('../../src/core/embeddings/embedder.js', () => ({ + initEmbedder: vi.fn().mockResolvedValue(undefined), + embedBatch: embedBatchSpy, + embedText: vi.fn().mockResolvedValue(new Float32Array(384)), + embeddingToArray: vi.fn().mockImplementation((emb: Float32Array) => Array.from(emb)), + isEmbedderReady: vi.fn().mockReturnValue(true), + })); + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + loadVectorExtension: vi.fn().mockResolvedValue(undefined), + })); + + const node = makeNode({ + label: 'Class', + name: 'Parser', + content: `class Parser { + options: ParserOptions; + cache: Map; + parseJSON() { return JSON.parse("{}"); } + validate() { return true; } +}`, + startLine: 1, + endLine: 6, + }); + + const executeQuery = mockExecuteQuery([node]); + const executeWithReusedStatement = mockExecuteWithReusedStatement(); + + const { runEmbeddingPipeline } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + + await runEmbeddingPipeline( + executeQuery, + executeWithReusedStatement, + onProgress, + { chunkSize: CLASS_CHUNK_SIZE, overlap: CLASS_OVERLAP }, + undefined, + undefined, + new Map(), + ); + + const embeddedTexts = embedBatchSpy.mock.calls.flatMap((call) => call[0] as string[]); + const laterChunk = embeddedTexts.find((text) => text.includes('[preceding context]:')); + expect(laterChunk).toBeDefined(); + expect(laterChunk).toContain('[preceding context]: ...'); + const precedingContextLine = laterChunk + ?.split('\n') + .find((line) => line.startsWith('[preceding context]: ...')); + expect(precedingContextLine).toBeDefined(); + expect(precedingContextLine).toContain('ring, any>'); + expect(precedingContextLine).not.toContain('parseJSON() {'); + }); + it('throws when DELETE for stale nodes fails with non-trivial error', async () => { mockEmbedderSetup(); diff --git a/gitnexus/test/unit/group/cross-impact.test.ts b/gitnexus/test/unit/group/cross-impact.test.ts new file mode 100644 index 000000000..e68ab6209 --- /dev/null +++ b/gitnexus/test/unit/group/cross-impact.test.ts @@ -0,0 +1,282 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + validateGroupImpactParams, + runGroupImpact, + MAX_SUPPORTED_CROSS_DEPTH, + DEFAULT_LOCAL_IMPACT_TIMEOUT_MS, + collectImpactSymbolUids, + fileMatchesServicePrefix, +} from '../../../src/core/group/cross-impact.js'; +import type { GroupToolPort } from '../../../src/core/group/service.js'; +import { writeBridgeMeta } from '../../../src/core/group/bridge-db.js'; +import { BRIDGE_SCHEMA_VERSION } from '../../../src/core/group/bridge-schema.js'; + +function tmpGroup(): { tmpDir: string; groupDir: string; cleanup: () => void } { + const tmpDir = path.join(os.tmpdir(), `gitnexus-ci-${Date.now()}-${Math.random()}`); + const groupDir = path.join(tmpDir, 'groups', 'g1'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +description: "" +repos: + app/backend: reg-be + app/frontend: reg-fe +links: [] +packages: {} +detect: + http: true + grpc: true + topics: true + shared_libs: true + embedding_fallback: true +matching: + bm25_threshold: 0.7 + embedding_threshold: 0.65 + max_candidates_per_step: 3 +`, + ); + return { + tmpDir, + groupDir, + cleanup: () => fs.rmSync(tmpDir, { recursive: true, force: true }), + }; +} + +describe('cross-impact', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('test_validateGroupImpactParams_rejects_bad_direction', () => { + const r = validateGroupImpactParams({ + name: 'g', + repo: 'a', + target: 't', + direction: 'sideways', + }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain('direction'); + }); + + it('test_validateGroupImpactParams_clamps_crossDepth_and_warns', () => { + const r = validateGroupImpactParams({ + name: 'g', + repo: 'a', + target: 't', + direction: 'upstream', + crossDepth: 99, + }); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.crossDepth).toBe(MAX_SUPPORTED_CROSS_DEPTH); + expect(r.crossDepthWarning).toBeDefined(); + } + }); + + it('test_validateGroupImpactParams_default_timeout', () => { + const r = validateGroupImpactParams({ + name: 'g', + repo: 'a', + target: 't', + direction: 'downstream', + }); + expect(r.ok).toBe(true); + if (r.ok) expect(r.timeoutMs).toBe(DEFAULT_LOCAL_IMPACT_TIMEOUT_MS); + }); + + it('test_collectImpactSymbolUids_respects_service_prefix', () => { + const local = { + target: { id: 'a', filePath: 'services/auth/x.ts' }, + byDepth: { + 1: [{ id: 'b', filePath: 'other/y.ts' }], + }, + }; + const uids = collectImpactSymbolUids(local, 'services/auth').uids; + expect(uids).toContain('a'); + expect(uids).not.toContain('b'); + }); + + it('test_fileMatchesServicePrefix', () => { + expect(fileMatchesServicePrefix('services/auth/a.ts', 'services/auth')).toBe(true); + expect(fileMatchesServicePrefix('services/aut', 'services/auth')).toBe(false); + }); + + it('test_runGroupImpact_local_timeout_returns_truncation', async () => { + const { tmpDir, cleanup } = tmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + let impactCalls = 0; + const port: GroupToolPort = { + resolveRepo: vi.fn(async () => ({ + id: 'be', + name: 'reg-be', + repoPath: '/r', + storagePath: '/r/.gitnexus', + })), + impact: vi.fn(async () => { + impactCalls++; + await new Promise((r) => setTimeout(r, 200)); + return { summary: { direct: 1 }, byDepth: { 1: [{ id: 'x' }] } }; + }), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + timeoutMs: 15, + }, + ); + expect(impactCalls).toBe(1); + expect('error' in r).toBe(false); + if (!('error' in r)) { + expect(r.truncationReason).toBe('timeout'); + expect(r.truncated).toBe(true); + } + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('test_runGroupImpact_local_phase_error_bubbles_as_top_level_error', async () => { + // Regression for #1004: when the local-impact phase returns a structured + // `{ error: ... }` payload, groupImpact MUST surface it as a top-level + // `{ error }` instead of a zero-hit GroupImpactResult. Otherwise callers + // that branch on top-level `error` silently treat a failed analysis as + // "no impact across the group" — a false negative on the failure path + // of a blast-radius tool. + const { tmpDir, cleanup } = tmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const port: GroupToolPort = { + resolveRepo: vi.fn(async () => ({ + id: 'be', + name: 'reg-be', + repoPath: '/r', + storagePath: '/r/.gitnexus', + })), + impact: vi.fn(async () => ({ error: 'symbol not found: Sym' })), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in r).toBe(true); + if ('error' in r) { + expect(r.error).toContain('symbol not found: Sym'); + expect(r.error).toContain('app/backend'); + } + // And ensure we didn't silently fall back to a zero-hit success payload. + expect((r as { summary?: unknown }).summary).toBeUndefined(); + expect((r as { cross?: unknown }).cross).toBeUndefined(); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('test_runGroupImpact_local_phase_thrown_exception_bubbles_as_top_level_error', async () => { + // Companion to the #1004 regression: safeLocalImpact wraps thrown + // exceptions from port.impact() as `{ error }` payloads. Those must + // bubble to the caller as top-level errors too, not be swallowed into + // an empty success payload. + const { tmpDir, cleanup } = tmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const port: GroupToolPort = { + resolveRepo: vi.fn(async () => ({ + id: 'be', + name: 'reg-be', + repoPath: '/r', + storagePath: '/r/.gitnexus', + })), + impact: vi.fn(async () => { + throw new Error('graph-load failure: .gitnexus missing'); + }), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in r).toBe(true); + if ('error' in r) { + expect(r.error).toContain('graph-load failure'); + } + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('test_runGroupImpact_bridge_schema_mismatch_returns_error', async () => { + const { tmpDir, groupDir, cleanup } = tmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + await writeBridgeMeta(groupDir, { + version: BRIDGE_SCHEMA_VERSION + 9, + generatedAt: new Date().toISOString(), + missingRepos: [], + }); + try { + const port: GroupToolPort = { + resolveRepo: vi.fn(async () => ({ + id: 'be', + name: 'reg-be', + repoPath: '/r', + storagePath: '/r/.gitnexus', + })), + impact: vi.fn(async () => ({ + target: { id: 'u1', filePath: 'src/a.ts' }, + summary: { direct: 1, processes_affected: 0, modules_affected: 0 }, + byDepth: { 1: [{ id: 'u1', filePath: 'src/a.ts' }] }, + risk: 'LOW', + })), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in r).toBe(true); + if ('error' in r) { + expect(r.error).toContain('schema'); + } + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/group/group-path-utils.test.ts b/gitnexus/test/unit/group/group-path-utils.test.ts new file mode 100644 index 000000000..0f57e0adc --- /dev/null +++ b/gitnexus/test/unit/group/group-path-utils.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from 'vitest'; +import { + fileMatchesServicePrefix, + normalizeServicePrefix, + repoInSubgroup, +} from '../../../src/core/group/group-path-utils.js'; + +describe('group-path-utils', () => { + describe('normalizeServicePrefix', () => { + it('returns undefined for null/undefined/empty', () => { + expect(normalizeServicePrefix(undefined)).toBeUndefined(); + expect(normalizeServicePrefix(null)).toBeUndefined(); + expect(normalizeServicePrefix('')).toBeUndefined(); + expect(normalizeServicePrefix(' ')).toBeUndefined(); + }); + + it('strips trailing slashes', () => { + expect(normalizeServicePrefix('services/auth/')).toBe('services/auth'); + expect(normalizeServicePrefix('services/auth///')).toBe('services/auth'); + }); + + it('normalizes Windows-style backslashes to POSIX', () => { + expect(normalizeServicePrefix('services\\auth')).toBe('services/auth'); + expect(normalizeServicePrefix('app\\backend\\')).toBe('app/backend'); + }); + }); + + describe('fileMatchesServicePrefix', () => { + it('returns true when prefix is empty/undefined', () => { + expect(fileMatchesServicePrefix('any/file.ts', undefined)).toBe(true); + expect(fileMatchesServicePrefix('any/file.ts', '')).toBe(true); + }); + + it('returns false when filePath is missing but prefix is set', () => { + expect(fileMatchesServicePrefix(undefined, 'services/auth')).toBe(false); + }); + + it('matches exact prefix and descendants', () => { + expect(fileMatchesServicePrefix('services/auth', 'services/auth')).toBe(true); + expect(fileMatchesServicePrefix('services/auth/a.ts', 'services/auth')).toBe(true); + }); + + it('rejects partial-segment matches', () => { + expect(fileMatchesServicePrefix('services/aut', 'services/auth')).toBe(false); + expect(fileMatchesServicePrefix('services/authz/a.ts', 'services/auth')).toBe(false); + }); + + it('matches Windows-style file paths against POSIX prefix', () => { + expect(fileMatchesServicePrefix('services\\auth\\a.ts', 'services/auth')).toBe(true); + expect(fileMatchesServicePrefix('services\\authz\\a.ts', 'services/auth')).toBe(false); + }); + }); + + describe('repoInSubgroup', () => { + it('matches every repo when subgroup is empty/undefined', () => { + expect(repoInSubgroup('any/repo', undefined)).toBe(true); + expect(repoInSubgroup('any/repo', '')).toBe(true); + expect(repoInSubgroup('any/repo', ' ')).toBe(true); + }); + + it('matches exact path and descendants by default', () => { + expect(repoInSubgroup('app/backend', 'app/backend')).toBe(true); + expect(repoInSubgroup('app/backend/sub', 'app/backend')).toBe(true); + expect(repoInSubgroup('app/frontend', 'app/backend')).toBe(false); + }); + + it('strips trailing slashes from subgroup', () => { + expect(repoInSubgroup('app/backend', 'app/backend/')).toBe(true); + expect(repoInSubgroup('app/backend/x', 'app/backend///')).toBe(true); + }); + + it('with exact=true matches only the exact repo', () => { + expect(repoInSubgroup('app/backend', 'app/backend', true)).toBe(true); + expect(repoInSubgroup('app/backend/sub', 'app/backend', true)).toBe(false); + }); + + it('rejects partial-segment matches', () => { + expect(repoInSubgroup('app/backendz', 'app/backend')).toBe(false); + }); + + it('normalizes Windows-style paths on both sides', () => { + expect(repoInSubgroup('app\\backend', 'app/backend')).toBe(true); + expect(repoInSubgroup('app/backend/x', 'app\\backend')).toBe(true); + expect(repoInSubgroup('app\\backend\\sub', 'app\\backend', true)).toBe(false); + }); + }); +}); diff --git a/gitnexus/test/unit/group/group-service-group-mode.test.ts b/gitnexus/test/unit/group/group-service-group-mode.test.ts new file mode 100644 index 000000000..ebd0ee0ba --- /dev/null +++ b/gitnexus/test/unit/group/group-service-group-mode.test.ts @@ -0,0 +1,129 @@ +/** + * Documents MCP → GroupService mapping: callers use `name` + concrete params; + * the "@group" string is interpreted only in LocalBackend.callTool (Issue #794). + */ +import { describe, it, expect, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + GroupService, + type GroupToolPort, + type GroupRepoHandle, +} from '../../../src/core/group/service.js'; + +function makeTmpGroup(): { tmpDir: string; cleanup: () => void } { + const tmpDir = path.join(os.tmpdir(), `gitnexus-gmode-${Date.now()}`); + const groupDir = path.join(tmpDir, 'groups', 'test-group'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: test-group +repos: + app/backend: test-backend + app/frontend: test-frontend +`, + ); + return { tmpDir, cleanup: () => fs.rmSync(tmpDir, { recursive: true, force: true }) }; +} + +function makePort(overrides: Partial = {}): GroupToolPort { + return { + resolveRepo: vi.fn( + async (name?: string): Promise => ({ + id: name || 'test', + name: name || 'test', + repoPath: '/tmp/repo', + storagePath: '/tmp/repo/.gitnexus', + }), + ), + impact: vi.fn(async () => ({ target: {}, byDepth: {} })), + query: vi.fn(async () => ({ + processes: [{ id: 'p1', heuristicLabel: 'Proc' }], + process_symbols: [ + { id: 's1', process_id: 'p1', filePath: 'services/auth/a.ts' }, + { id: 's2', process_id: 'p1', filePath: 'other/b.ts' }, + ], + })), + impactByUid: vi.fn(async () => null), + context: vi.fn(async () => ({ + status: 'found', + symbol: { filePath: 'services/auth/x.ts', uid: 'u1', name: 'X' }, + })), + ...overrides, + }; +} + +describe('GroupService group-mode API surface', () => { + it('groupQuery uses name (never @-repo) and optional service filters processes', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const query = vi.fn(async () => ({ + processes: [{ id: 'p1' }], + process_symbols: [ + { id: 's1', process_id: 'p1', filePath: 'services/auth/a.ts' }, + { id: 's2', process_id: 'p1', filePath: 'other/b.ts' }, + ], + })); + const svc = new GroupService(makePort({ query })); + const r = (await svc.groupQuery({ + name: 'test-group', + query: 'oauth', + service: 'services/auth', + })) as { results: Array<{ id?: string }> }; + expect(query).toHaveBeenCalled(); + expect(r.results.every((row) => row.id === 'p1')).toBe(true); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('groupQuery rejects empty service string', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const svc = new GroupService(makePort()); + const r = await svc.groupQuery({ name: 'test-group', query: 'x', service: ' ' }); + expect(r).toEqual({ error: 'service must not be an empty string' }); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('groupContext uses name + target (MCP maps @group to name)', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const svc = new GroupService(makePort()); + const r = await svc.groupContext({ name: 'test-group', target: 'MySym' }); + expect(r.group).toBe('test-group'); + expect(r.results).toHaveLength(2); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('groupImpact with mock port returns structured result without @ in params', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const svc = new GroupService(makePort()); + const r = (await svc.groupImpact({ + name: 'test-group', + repo: 'app/backend', + target: 't', + direction: 'upstream', + })) as { group?: string; error?: string }; + expect(r.error).toBeUndefined(); + expect(r.group).toBe('test-group'); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/group/group-tools.test.ts b/gitnexus/test/unit/group/group-tools.test.ts index e58077442..85ba45f0a 100644 --- a/gitnexus/test/unit/group/group-tools.test.ts +++ b/gitnexus/test/unit/group/group-tools.test.ts @@ -2,16 +2,10 @@ import { describe, it, expect } from 'vitest'; import { GITNEXUS_TOOLS } from '../../../src/mcp/tools.js'; -const GROUP_TOOL_NAMES = [ - 'group_list', - 'group_sync', - 'group_contracts', - 'group_query', - 'group_status', -]; +const GROUP_TOOL_NAMES = ['group_list', 'group_sync']; describe('Group MCP tools', () => { - it('all 5 group tools are registered', () => { + it('group_list and group_sync are registered', () => { for (const name of GROUP_TOOL_NAMES) { const tool = GITNEXUS_TOOLS.find((t) => t.name === name); expect(tool, `tool ${name} should be registered`).toBeDefined(); diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index 8ff914fcc..e81f1566e 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -517,6 +517,86 @@ Route::delete('/users/{id}', [UserController::class, 'destroy']); }); }); + describe('consumer extraction — PHP', () => { + it('extracts Laravel Http facade calls', async () => { + const dir = path.join(tmpDir, 'php-http-facade'); + fs.mkdirSync(path.join(dir, 'app'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'app/Client.php'), + ` c.role === 'consumer'); + + expect(consumers.find((c) => c.contractId === 'http::GET::/api/users')).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::POST::/api/orders/{param}'), + ).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::DELETE::/api/users/{param}'), + ).toBeDefined(); + }); + + it('extracts Guzzle $client->method() calls', async () => { + const dir = path.join(tmpDir, 'php-guzzle'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/ApiClient.php'), + `get('/api/health'); + $client->post('/api/orders/42'); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers.find((c) => c.contractId === 'http::GET::/api/health')).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::POST::/api/orders/{param}'), + ).toBeDefined(); + }); + + it('extracts file_get_contents HTTP calls', async () => { + const dir = path.join(tmpDir, 'php-fgc'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/fetch.php'), + ` c.role === 'consumer'); + + expect(consumers.find((c) => c.contractId === 'http::GET::/api/items/{param}')).toBeDefined(); + // file paths and stream wrappers must not emit consumer contracts + expect(consumers.find((c) => c.meta.path === '/tmp/local-file.txt')).toBeUndefined(); + }); + }); + describe('provider extraction — FastAPI', () => { it('extracts FastAPI @app.get decorator patterns', async () => { const dir = path.join(tmpDir, 'fastapi'); diff --git a/gitnexus/test/unit/group/service.test.ts b/gitnexus/test/unit/group/service.test.ts index e4b10443c..8c5c0ed6d 100644 --- a/gitnexus/test/unit/group/service.test.ts +++ b/gitnexus/test/unit/group/service.test.ts @@ -44,6 +44,10 @@ function makePort(overrides: Partial = {}): GroupToolPort { impact: vi.fn(async () => ({ symbols: [] })), query: vi.fn(async () => ({ processes: [] })), impactByUid: vi.fn(async () => null), + context: vi.fn(async () => ({ + status: 'found', + symbol: { filePath: 'services/auth/x.ts', uid: 'u1', name: 'X' }, + })), ...overrides, }; } @@ -233,6 +237,46 @@ describe('GroupService', () => { cleanup(); } }); + + it('test_groupContracts_skips_corrupt_contract_rows', async () => { + const { groupDir, cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const badJson = `{ + "version": 1, + "generatedAt": "2026-01-01T00:00:00.000Z", + "repoSnapshots": {}, + "missingRepos": [], + "contracts": [ + { "not": "a-contract" }, + { + "contractId": "http::GET::/ok", + "type": "http", + "repo": "app/backend", + "role": "provider", + "symbolUid": "u", + "symbolRef": { "filePath": "a.ts", "name": "f" }, + "symbolName": "f", + "confidence": 1, + "meta": {} + } + ], + "crossLinks": [] + }`; + fs.writeFileSync(path.join(groupDir, 'contracts.json'), badJson, 'utf-8'); + + const svc = new GroupService(makePort()); + const result = (await svc.groupContracts({ name: 'test-group' })) as { + contracts: unknown[]; + skippedCorrupt?: number; + }; + expect(result.contracts).toHaveLength(1); + expect(result.skippedCorrupt).toBe(1); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); }); describe('groupSync', () => { @@ -328,6 +372,135 @@ describe('GroupService', () => { cleanup(); } }); + + it('test_groupQuery_subgroupExact_skips_descendant_member_paths', async () => { + const tmpDir = path.join(os.tmpdir(), `gitnexus-svc-nest-${Date.now()}`); + const groupDir = path.join(tmpDir, 'groups', 'nest-group'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: nest-group +repos: + app/frontend: fe-root + app/frontend/mobile: fe-nested + app/backend: be1 +`, + ); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const query = vi.fn(async () => ({ processes: [{ name: 'p1' }] })); + const port = makePort({ query }); + const svc = new GroupService(port); + + const prefixOnly = (await svc.groupQuery({ + name: 'nest-group', + query: 'x', + subgroup: 'app/frontend', + })) as { per_repo: Array<{ repo: string }> }; + expect(prefixOnly.per_repo.map((r) => r.repo).sort()).toEqual([ + 'app/frontend', + 'app/frontend/mobile', + ]); + + const exact = (await svc.groupQuery({ + name: 'nest-group', + query: 'x', + subgroup: 'app/frontend', + subgroupExact: true, + })) as { per_repo: Array<{ repo: string }> }; + expect(exact.per_repo).toHaveLength(1); + expect(exact.per_repo[0].repo).toBe('app/frontend'); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + }); + + describe('groupImpact', () => { + it('test_groupImpact_returns_validation_error', async () => { + const svc = new GroupService(makePort()); + const r = (await svc.groupImpact({})) as { error: string }; + expect(r.error).toContain('name'); + }); + }); + + describe('groupContext', () => { + it('test_groupContext_requires_target_or_uid', async () => { + const svc = new GroupService(makePort()); + const r = await svc.groupContext({ name: 'test-group' }); + expect(r.error).toContain('target'); + }); + + it('test_groupContext_iterates_repos', async () => { + const { cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const port = makePort(); + const svc = new GroupService(port); + const r = await svc.groupContext({ name: 'test-group', target: 'MySym' }); + expect(r.group).toBe('test-group'); + expect(r.results).toHaveLength(2); + expect(port.context).toHaveBeenCalledTimes(2); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('test_groupContext_subgroupExact_skips_descendant_member_paths', async () => { + const tmpDir = path.join(os.tmpdir(), `gitnexus-ctx-nest-${Date.now()}`); + const groupDir = path.join(tmpDir, 'groups', 'nest-group'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: nest-group +repos: + app/frontend: fe-root + app/frontend/mobile: fe-nested +`, + ); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const port = makePort(); + const svc = new GroupService(port); + await svc.groupContext({ + name: 'nest-group', + target: 'X', + subgroup: 'app/frontend', + subgroupExact: true, + }); + expect(port.context).toHaveBeenCalledTimes(1); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('test_groupContext_service_prefix_filters_payload', async () => { + const { cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const port = makePort({ + context: vi.fn(async () => ({ + status: 'found', + symbol: { filePath: 'other/path/x.ts', uid: 'u1', name: 'X' }, + })), + }); + const svc = new GroupService(port); + const r = await svc.groupContext({ + name: 'test-group', + target: 'MySym', + service: 'services/auth', + }); + expect(r.results.every((x) => Object.keys(x.payload as object).length === 0)).toBe(true); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); }); describe('groupStatus', () => { diff --git a/gitnexus/test/unit/mcp/group-repo-routing.test.ts b/gitnexus/test/unit/mcp/group-repo-routing.test.ts new file mode 100644 index 000000000..ddf7d03a5 --- /dev/null +++ b/gitnexus/test/unit/mcp/group-repo-routing.test.ts @@ -0,0 +1,230 @@ +/** + * LocalBackend.callTool routes impact/query/context to GroupService when repo starts with "@". + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; + +const { lbugMocks } = vi.hoisted(() => ({ + lbugMocks: { + initLbug: vi.fn().mockResolvedValue(undefined), + executeQuery: vi.fn().mockResolvedValue([]), + executeParameterized: vi.fn().mockResolvedValue([]), + closeLbug: vi.fn().mockResolvedValue(undefined), + isLbugReady: vi.fn().mockReturnValue(true), + }, +})); + +vi.mock('../../../src/core/lbug/pool-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../../src/mcp/core/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../../src/storage/repo-manager.js', () => ({ + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), +})); + +vi.mock('../../../src/core/search/bm25-index.js', () => ({ + searchFTSFromLbug: vi.fn().mockResolvedValue([]), +})); + +vi.mock('../../../src/mcp/core/embedder.js', () => ({ + embedQuery: vi.fn().mockResolvedValue([]), + getEmbeddingDims: vi.fn().mockReturnValue(384), +})); + +import { LocalBackend } from '../../../src/mcp/local/local-backend.js'; +import { GroupService } from '../../../src/core/group/service.js'; + +describe('LocalBackend @group repo routing', () => { + let tmpDir: string; + let groupSpyQuery: ReturnType; + let groupSpyImpact: ReturnType; + let groupSpyContext: ReturnType; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-atgrp-')); + const groupDir = path.join(tmpDir, 'groups', 'g1'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +repos: + app/backend: test-backend + app/frontend: test-frontend +`, + ); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + groupSpyQuery = vi + .spyOn(GroupService.prototype, 'groupQuery') + .mockResolvedValue({ via: 'query' }); + groupSpyImpact = vi + .spyOn(GroupService.prototype, 'groupImpact') + .mockResolvedValue({ via: 'impact' }); + groupSpyContext = vi.spyOn(GroupService.prototype, 'groupContext').mockResolvedValue({ + group: 'g1', + results: [], + }); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('routes query to groupQuery with default member path (first sorted repos key)', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('query', { repo: '@g1', query: 'login' }); + expect(out).toEqual({ via: 'query' }); + expect(groupSpyQuery).toHaveBeenCalledWith( + expect.objectContaining({ name: 'g1', query: 'login' }), + ); + const arg = groupSpyQuery.mock.calls[0][0] as Record; + expect(arg).not.toHaveProperty('repo'); + }); + + it('routes query with explicit member path as exact subgroup (no descendant repo bleed)', async () => { + const backend = new LocalBackend(); + await backend.callTool('query', { repo: '@g1/app/frontend', query: 'x' }); + expect(groupSpyQuery).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'g1', + query: 'x', + subgroup: 'app/frontend', + subgroupExact: true, + }), + ); + }); + + it('routes impact to groupImpact with resolved repo member path', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('impact', { + repo: '@g1', + target: 'Sym', + direction: 'upstream', + }); + expect(out).toEqual({ via: 'impact' }); + expect(groupSpyImpact).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + }), + ); + }); + + it('routes context to groupContext', async () => { + const backend = new LocalBackend(); + await backend.callTool('context', { repo: '@g1', target: 'Sym' }); + expect(groupSpyContext).toHaveBeenCalledWith( + expect.objectContaining({ name: 'g1', target: 'Sym' }), + ); + }); + + it('maps MCP symbol name to groupContext target (does not overwrite group name)', async () => { + const backend = new LocalBackend(); + await backend.callTool('context', { repo: '@g1', name: 'MyClass' }); + expect(groupSpyContext).toHaveBeenCalledWith( + expect.objectContaining({ name: 'g1', target: 'MyClass' }), + ); + }); + + it('returns error for unknown group name', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('query', { repo: '@no-such-group', query: 'x' }); + expect(out).toHaveProperty('error'); + expect(String((out as { error: string }).error)).toMatch( + /not found|no such|unknown|exist|ENOENT/i, + ); + }); + + it('returns error for unknown member path', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('query', { repo: '@g1/not-a-member', query: 'x' }); + expect(out).toHaveProperty('error'); + expect(String((out as { error: string }).error)).toMatch(/Unknown member path/i); + }); + + it('rejects empty service without calling group tools', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('query', { repo: '@g1', query: 'x', service: '' }); + expect(out).toEqual({ error: 'service must not be an empty string' }); + expect(groupSpyQuery).not.toHaveBeenCalled(); + }); + + it('unknown group_* tools mention removal', async () => { + const backend = new LocalBackend(); + await expect(backend.callTool('group_query', { name: 'g1', query: 'x' })).rejects.toThrow( + /Removed tools/, + ); + }); + + it('removed group_contracts mentions migration', async () => { + const backend = new LocalBackend(); + await expect(backend.callTool('group_contracts', { name: 'g1' })).rejects.toThrow( + /Removed tools/, + ); + }); + + it('removed group_status mentions migration', async () => { + const backend = new LocalBackend(); + await expect(backend.callTool('group_status', { name: 'g1' })).rejects.toThrow(/Removed tools/); + }); + + describe('Issue #794 manual smoke checklist (automated)', () => { + beforeEach(() => { + const groupDir = path.join(tmpDir, 'groups', 'myproduct'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: myproduct +repos: + app/backend: test-backend + app/frontend: test-frontend +`, + ); + }); + + it.each([ + { + method: 'impact', + params: { repo: '@myproduct', target: 'UserService.login', service: 'app/backend' }, + spy: () => groupSpyImpact, + }, + { + method: 'query', + params: { repo: '@myproduct', query: 'login', service: 'app/backend' }, + spy: () => groupSpyQuery, + }, + { + method: 'context', + params: { repo: '@myproduct', target: 'UserService.login', service: 'app/backend' }, + spy: () => groupSpyContext, + }, + ])( + '$method with repo "@myproduct" routes to GroupService and forwards service', + async ({ method, params, spy }) => { + const backend = new LocalBackend(); + await backend.callTool(method, params); + expect(spy()).toHaveBeenCalledWith( + expect.objectContaining({ name: 'myproduct', service: 'app/backend' }), + ); + const callArg = spy().mock.calls[0][0] as Record; + expect( + typeof callArg.repo === 'string' ? (callArg.repo as string).startsWith('@') : false, + ).toBe(false); + }, + ); + }); +}); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 83827fc15..12c56d67a 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -15,7 +15,14 @@ import { loadCLIConfig, registerRepo, listRegisteredRepos, + resolveRegistryEntry, + canonicalizePath, + assertSafeStoragePath, RegistryNameCollisionError, + RegistryNotFoundError, + RegistryAmbiguousTargetError, + UnsafeStoragePathError, + type RegistryEntry, type RepoMeta, } from '../../src/storage/repo-manager.js'; import { parseRepoNameFromUrl, getInferredRepoName } from '../../src/storage/git.js'; @@ -453,3 +460,334 @@ describe('getInferredRepoName + registerRepo (#979 — git remote inference)', ( } }); }); + +// ─── resolveRegistryEntry (#664 — gitnexus remove ) ────────── +// +// The resolver is a pure function over a `RegistryEntry[]` snapshot, so +// these tests build synthetic entries inline and do NOT touch +// ~/.gitnexus. No GITNEXUS_HOME sandboxing needed. This also means the +// tests are platform-portable on Windows where realpath semantics on +// tmpdirs can diverge between runs (see the #955 CI pivot). + +describe('resolveRegistryEntry (#664)', () => { + // A well-known synthetic registry with two same-name entries (which + // can only exist in reality after `--allow-duplicate-name` — #829) and + // one unique-name entry. Path prefixes differ across platforms so the + // tests stay meaningful regardless of `process.platform`. + const prefix = process.platform === 'win32' ? 'D:\\' : '/tmp/'; + const pathA = `${prefix}projects${path.sep}gnx-a${path.sep}app`; + const pathB = `${prefix}projects${path.sep}gnx-b${path.sep}app`; + const pathW = `${prefix}work${path.sep}website`; + + const entries: RegistryEntry[] = [ + { + name: 'app', + path: pathA, + storagePath: `${pathA}${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'aaaaaaa', + }, + { + name: 'app', + path: pathB, + storagePath: `${pathB}${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'bbbbbbb', + }, + { + name: 'website', + path: pathW, + storagePath: `${pathW}${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'ccccccc', + }, + ]; + + it('resolves by absolute path to the exact entry (path tier beats name tier)', () => { + const hit = resolveRegistryEntry(entries, pathA); + expect(hit).toBe(entries[0]); + expect(hit.path).toBe(pathA); + + const hit2 = resolveRegistryEntry(entries, pathB); + expect(hit2).toBe(entries[1]); + expect(hit2.path).toBe(pathB); + }); + + it('resolves by unique name to the only matching entry', () => { + const hit = resolveRegistryEntry(entries, 'website'); + expect(hit).toBe(entries[2]); + expect(hit.name).toBe('website'); + }); + + it('name match is case-insensitive', () => { + expect(resolveRegistryEntry(entries, 'WEBSITE')).toBe(entries[2]); + expect(resolveRegistryEntry(entries, 'Website')).toBe(entries[2]); + }); + + it('path match is case-insensitive on Windows only', () => { + if (process.platform !== 'win32') { + // On POSIX, a differently-cased path must NOT match. Verify by + // lower-casing a mixed-case copy of pathW and expecting a miss. + const upper = pathW.toUpperCase(); + expect(() => resolveRegistryEntry(entries, upper)).toThrow(RegistryNotFoundError); + return; + } + const upper = pathA.toUpperCase(); + const hit = resolveRegistryEntry(entries, upper); + expect(hit).toBe(entries[0]); + }); + + it('throws RegistryAmbiguousTargetError when name matches multiple entries', () => { + // Two 'app' entries exist only because of --allow-duplicate-name + // (#829). The resolver MUST refuse to guess. + expect(() => resolveRegistryEntry(entries, 'app')).toThrow(RegistryAmbiguousTargetError); + try { + resolveRegistryEntry(entries, 'app'); + } catch (e) { + expect(e).toBeInstanceOf(RegistryAmbiguousTargetError); + const err = e as RegistryAmbiguousTargetError; + expect(err.kind).toBe('RegistryAmbiguousTargetError'); + expect(err.target).toBe('app'); + expect(err.matches).toHaveLength(2); + // Error message must include both paths so the CLI can surface + // them without string-matching on `.message`. + expect(err.message).toContain(pathA); + expect(err.message).toContain(pathB); + } + }); + + it('throws RegistryNotFoundError when no entry matches', () => { + expect(() => resolveRegistryEntry(entries, 'nonexistent')).toThrow(RegistryNotFoundError); + try { + resolveRegistryEntry(entries, 'nonexistent'); + } catch (e) { + expect(e).toBeInstanceOf(RegistryNotFoundError); + const err = e as RegistryNotFoundError; + expect(err.kind).toBe('RegistryNotFoundError'); + expect(err.target).toBe('nonexistent'); + // availableNames is disambiguated: 'app' appears twice, so both + // `app (path)` variants are included; 'website' is unique so it + // stays plain — matches the resolveRepo disambiguation shape. + expect(err.availableNames).toContain('website'); + expect(err.availableNames.some((n) => n.startsWith('app ('))).toBe(true); + // Error message surfaces the hint. + expect(err.message).toContain('website'); + } + }); + + it('throws RegistryNotFoundError with "no repositories registered" hint when registry is empty', () => { + try { + resolveRegistryEntry([], 'anything'); + } catch (e) { + expect(e).toBeInstanceOf(RegistryNotFoundError); + const err = e as RegistryNotFoundError; + expect(err.availableNames).toEqual([]); + expect(err.message).toContain('No repositories are currently registered'); + } + }); + + it('path match wins over name match (never ambiguous)', () => { + // Construct a pathological fixture where a registry entry's NAME + // happens to equal another entry's PATH. The path tier must win + // without triggering ambiguity. + const weird: RegistryEntry[] = [ + { ...entries[2] }, // 'website' at pathW + { + name: pathW, // degenerate: name equals another entry's path + path: `${prefix}elsewhere${path.sep}odd`, + storagePath: `${prefix}elsewhere${path.sep}odd${path.sep}.gitnexus`, + indexedAt: '2026-04-18T00:00:00.000Z', + lastCommit: 'ddddddd', + }, + ]; + const hit = resolveRegistryEntry(weird, pathW); + // Must match the entry whose PATH is pathW, not the one whose NAME + // is pathW — because Tier 1 runs before Tier 2 and finds the path + // match first. + expect(hit.path).toBe(pathW); + expect(hit.name).toBe('website'); + }); +}); + +// ─── canonicalizePath (#1003 review — @evander-wang / @magyargergo) ── +// +// Shields `registerRepo`, `unregisterRepo`, and `resolveRegistryEntry` +// against cross-platform path-form divergence: macOS symlink expansion +// (/var → /private/var) and Windows 8.3 short-name expansion +// (RUNNERA~1 → runneradmin). The helper also underpins backwards +// compatibility with registries written by versions that only ran +// `path.resolve` — by canonicalising the stored entry at compare time, +// both pre- and post-fix entries converge to the same key. +// +// These tests avoid snapshotting a specific realpath value (that would +// be platform-fragile); instead they assert: +// - canonicalizePath is idempotent (f(f(x)) == f(x)) +// - canonicalizePath falls back cleanly when the path doesn't exist +// - resolveRegistryEntry matches a stored entry even when the target +// and the stored value disagree on one-step normalisation (simulated +// via a fixture that stores the de-canonicalised form of a real +// existing path). + +describe('canonicalizePath (#1003)', () => { + it('is idempotent — canonicalizePath(canonicalizePath(x)) === canonicalizePath(x)', async () => { + // Use the vitest project-root as a known-existing path. `os.tmpdir()` + // would work too but process.cwd() is guaranteed to exist for the + // test runner. + const p = process.cwd(); + const once = canonicalizePath(p); + const twice = canonicalizePath(once); + expect(twice).toBe(once); + }); + + it('falls back to path.resolve when the target does not exist', () => { + // Construct a definitely-nonexistent path under tmpdir. Using + // random-ish segments so we don't collide with anything real. + const ghost = path.join(os.tmpdir(), 'gnx-never-exists-____', 'still-not-there'); + const got = canonicalizePath(ghost); + // Must not throw, must not resolve to something weird — should be + // identical to `path.resolve(ghost)` since realpathSync.native will + // have thrown and we swallowed it. + expect(got).toBe(path.resolve(ghost)); + }); + + it('returns an absolute path for relative input even when the path is missing', () => { + // Relative path that does not exist. Must still be absolute + // (fallback path: path.resolve normalises even non-existent inputs). + const rel = './does-not-exist-zzz-' + Date.now(); + const got = canonicalizePath(rel); + expect(path.isAbsolute(got)).toBe(true); + }); +}); + +describe('resolveRegistryEntry backward-compat with non-canonical stored paths (#1003)', () => { + it('matches a stored entry even when the target was passed in canonical form', async () => { + // Simulate the bug-producing scenario without depending on a real + // symlink/8.3 discrepancy (those are platform-specific and flaky to + // set up in CI). We take a REAL path that exists + // (canonicalizePath-stable), store a known-non-canonical copy of it + // in a fake RegistryEntry, then resolve with the canonical form and + // assert the match. + // + // Construct a non-canonical string that resolves to the same real + // path. `path.join` auto-normalises `.` and trailing separators, so + // we build the string by raw concat to keep it string-unequal to + // `realDir` until `canonicalizePath` runs. + const realDir = process.cwd(); + const nonCanonical = realDir + path.sep + '.'; // e.g. /work/gitnexus/. + // Sanity: these are string-unequal before canonicalisation. + expect(nonCanonical).not.toBe(realDir); + + const entries: RegistryEntry[] = [ + { + name: 'stored-under-noncanonical-form', + path: nonCanonical, + storagePath: path.join(nonCanonical, '.gitnexus'), + indexedAt: '2026-04-20T00:00:00.000Z', + lastCommit: 'deadbee', + }, + ]; + + // Pass the canonical form as the target — resolver must still match. + const hit = resolveRegistryEntry(entries, realDir); + expect(hit).toBe(entries[0]); + }); +}); + +// ─── assertSafeStoragePath (#1003 review — @magyargergo) ───────────── +// +// Guard rail against destroying more than the `.gitnexus/` subfolder. +// `~/.gitnexus/registry.json` is user-writable plain text, so a +// corrupted or hand-edited entry could put storagePath anywhere. +// These tests use synthetic `RegistryEntry` fixtures (no disk I/O) +// because the guard is a pure string check — it must not depend on +// the paths existing. + +describe('assertSafeStoragePath (#1003)', () => { + const prefix = process.platform === 'win32' ? 'D:\\' : '/tmp/'; + const repoPath = `${prefix}projects${path.sep}my-repo`; + const base: Omit = { + name: 'my-repo', + path: repoPath, + indexedAt: '2026-04-21T00:00:00.000Z', + lastCommit: 'deadbee', + }; + + it('accepts the canonical /.gitnexus storage path', () => { + const entry: RegistryEntry = { + ...base, + storagePath: path.join(repoPath, '.gitnexus'), + }; + expect(() => assertSafeStoragePath(entry)).not.toThrow(); + }); + + it('rejects when storagePath equals the repo path itself (would delete the code)', () => { + const entry: RegistryEntry = { + ...base, + storagePath: repoPath, // catastrophic: rm the working tree + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath is a parent of the repo path', () => { + const entry: RegistryEntry = { + ...base, + storagePath: path.dirname(repoPath), // also catastrophic + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath is empty (path.resolve falls back to cwd)', () => { + const entry: RegistryEntry = { + ...base, + storagePath: '', // path.resolve('') === process.cwd() — would rm cwd + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath points somewhere totally unrelated', () => { + const entry: RegistryEntry = { + ...base, + storagePath: `${prefix}some${path.sep}other${path.sep}place`, + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('rejects when storagePath is a sibling .gitnexus (right basename, wrong parent)', () => { + const entry: RegistryEntry = { + ...base, + storagePath: path.join(`${prefix}different${path.sep}repo`, '.gitnexus'), + }; + expect(() => assertSafeStoragePath(entry)).toThrow(UnsafeStoragePathError); + }); + + it('UnsafeStoragePathError carries the original entry + expected + actual paths', () => { + const entry: RegistryEntry = { + ...base, + storagePath: `${prefix}evil${path.sep}path`, + }; + try { + assertSafeStoragePath(entry); + } catch (e) { + expect(e).toBeInstanceOf(UnsafeStoragePathError); + const err = e as UnsafeStoragePathError; + expect(err.kind).toBe('UnsafeStoragePathError'); + expect(err.entry).toBe(entry); + // Expected path is the canonical `/.gitnexus`. + expect(err.expectedStoragePath).toBe(path.join(path.resolve(repoPath), '.gitnexus')); + // Actual path is the corrupted value (resolved). + expect(err.actualStoragePath).toBe(path.resolve(entry.storagePath)); + // Message must suggest the recovery action. + expect(err.message).toContain('registry.json'); + } + }); + + it('Windows: storagePath match is case-insensitive to match register/unregister semantics', () => { + if (process.platform !== 'win32') return; + const entry: RegistryEntry = { + ...base, + storagePath: path.join(repoPath.toUpperCase(), '.GITNEXUS'), + }; + // Should accept because Windows paths are case-insensitive. + expect(() => assertSafeStoragePath(entry)).not.toThrow(); + }); +}); diff --git a/gitnexus/test/unit/resources.test.ts b/gitnexus/test/unit/resources.test.ts index 6b203c969..ccd8461f1 100644 --- a/gitnexus/test/unit/resources.test.ts +++ b/gitnexus/test/unit/resources.test.ts @@ -12,6 +12,7 @@ import { describe, it, expect, vi } from 'vitest'; import { getResourceDefinitions, getResourceTemplates, + parseResourceUri, readResource, } from '../../src/mcp/resources.js'; @@ -36,6 +37,12 @@ function createMockBackend(overrides: Partial> = {}): any { queryProcessDetail: vi .fn() .mockResolvedValue(overrides.processDetail ?? { error: 'Not found' }), + readGroupContractsResource: vi + .fn() + .mockResolvedValue(overrides.groupContractsBody ?? 'contracts: []\n'), + readGroupStatusResource: vi + .fn() + .mockResolvedValue(overrides.groupStatusBody ?? 'group: mock\n'), ...overrides, }; } @@ -73,12 +80,12 @@ describe('getResourceDefinitions', () => { }); describe('getResourceTemplates', () => { - it('returns 6 dynamic templates', () => { + it('returns 8 dynamic templates', () => { const templates = getResourceTemplates(); - expect(templates).toHaveLength(6); + expect(templates).toHaveLength(8); }); - it('includes context, clusters, processes, schema, cluster detail, process detail', () => { + it('includes context, clusters, processes, schema, cluster detail, process detail, group contracts/status', () => { const templates = getResourceTemplates(); const uris = templates.map((t) => t.uriTemplate); expect(uris).toContain('gitnexus://repo/{name}/context'); @@ -87,6 +94,8 @@ describe('getResourceTemplates', () => { expect(uris).toContain('gitnexus://repo/{name}/schema'); expect(uris).toContain('gitnexus://repo/{name}/cluster/{clusterName}'); expect(uris).toContain('gitnexus://repo/{name}/process/{processName}'); + expect(uris).toContain('gitnexus://group/{name}/contracts'); + expect(uris).toContain('gitnexus://group/{name}/status'); }); it('each template has uriTemplate, name, description, mimeType', () => { @@ -99,6 +108,61 @@ describe('getResourceTemplates', () => { }); }); +describe('parseResourceUri', () => { + it('parses group contracts without query', () => { + const p = parseResourceUri('gitnexus://group/acme/contracts'); + expect(p).toEqual({ + kind: 'group', + groupName: 'acme', + resourceType: 'contracts', + contractsFilter: {}, + }); + }); + + it('parses nested group name and contracts query params', () => { + const p = parseResourceUri( + 'gitnexus://group/acme/billing/contracts?type=http&repo=app%2Fapi&unmatchedOnly=true', + ); + expect(p.kind).toBe('group'); + if (p.kind !== 'group' || p.resourceType !== 'contracts') throw new Error('unexpected'); + expect(p.groupName).toBe('acme/billing'); + expect(p.contractsFilter).toEqual({ + type: 'http', + repo: 'app/api', + unmatchedOnly: true, + }); + }); + + it('coerces unmatchedOnly false from string', () => { + const p = parseResourceUri('gitnexus://group/g1/contracts?unmatchedOnly=false'); + expect(p.kind).toBe('group'); + if (p.kind !== 'group' || p.resourceType !== 'contracts') throw new Error('unexpected'); + expect(p.contractsFilter.unmatchedOnly).toBe(false); + }); + + it('parses group status', () => { + const p = parseResourceUri('gitnexus://group/my/product/status'); + expect(p).toEqual({ + kind: 'group', + groupName: 'my/product', + resourceType: 'status', + }); + }); + + it('round-trips repo URI like legacy regex', () => { + const p = parseResourceUri('gitnexus://repo/my%20project/schema'); + expect(p).toEqual({ + kind: 'repo', + repoName: 'my project', + resourceType: 'schema', + }); + }); + + it('rejects unknown group resource tail', () => { + expect(() => parseResourceUri('gitnexus://group/foo/bar')).toThrow('Unknown group resource'); + }); +}); + // ─── readResource URI parsing ──────────────────────────────────────── describe('readResource', () => { @@ -149,6 +213,22 @@ describe('readResource', () => { expect(result).toContain('No repositories indexed'); }); + it('routes group contracts resource through backend', async () => { + const backend = createMockBackend(); + const uri = 'gitnexus://group/g1/contracts?type=http&unmatchedOnly=true'; + await readResource(uri, backend); + expect(backend.readGroupContractsResource).toHaveBeenCalledWith('g1', { + type: 'http', + unmatchedOnly: true, + }); + }); + + it('routes group status resource through backend', async () => { + const backend = createMockBackend(); + await readResource('gitnexus://group/acme/status', backend); + expect(backend.readGroupStatusResource).toHaveBeenCalledWith('acme'); + }); + it('routes gitnexus://repo/{name}/context correctly', async () => { const backend = createMockBackend({ context: { diff --git a/gitnexus/test/unit/text-generator.test.ts b/gitnexus/test/unit/text-generator.test.ts index 28e16044d..e411428df 100644 --- a/gitnexus/test/unit/text-generator.test.ts +++ b/gitnexus/test/unit/text-generator.test.ts @@ -148,6 +148,65 @@ describe('text-generator', () => { expect(text).toContain('class Parser {'); expect(text).toContain('parseJSON(text: string) { return JSON.parse(text); }'); }); + + it('generates Struct text with structural metadata', () => { + const node: EmbeddableNode = { + ...baseNode, + label: 'Struct', + name: 'User', + fieldNames: ['name', 'age'], + content: `struct User { + name: String, + age: u32, +}`, + }; + const text = generateEmbeddingText(node, node.content); + expect(text).toContain('Struct: User'); + expect(text).toContain('Properties: name, age'); + expect(text).toContain('Container: struct User {'); + expect(text).toContain('struct User {'); + }); + + it('keeps compact container context on later structural chunks', () => { + const node: EmbeddableNode = { + ...baseNode, + label: 'Class', + name: 'Parser', + methodNames: ['parseJSON', 'validate'], + fieldNames: ['options', 'cache'], + content: `class Parser { + options: ParserOptions; + cache: Map; + parseJSON(text: string) { return JSON.parse(text); } + validate() { return true; } +}`, + }; + const text = generateEmbeddingText( + node, + 'validate() { return true; }', + {}, + 1, + 'parseJSON(text: string) { return JSON.parse(text); }', + ); + expect(text).toContain('Class: Parser'); + expect(text).toContain('Container: class Parser {'); + expect(text).toContain('[preceding context]: ...parseJSON(text: string)'); + expect(text).not.toContain('Methods: parseJSON, validate'); + expect(text).not.toContain('Properties: options, cache'); + }); + + it('adds preceding context to non-structural chunk text', () => { + const text = generateEmbeddingText( + baseNode, + 'return JSON.parse(text);', + {}, + 1, + 'function parseJSON(text: string): Result {', + ); + expect(text).toContain('Function: parseJSON'); + expect(text).toContain('[preceding context]: ...function parseJSON'); + expect(text).toContain('return JSON.parse(text);'); + }); }); describe('Constructor label', () => { diff --git a/gitnexus/test/unit/tool-direct-cli.test.ts b/gitnexus/test/unit/tool-direct-cli.test.ts new file mode 100644 index 000000000..9ede6225b --- /dev/null +++ b/gitnexus/test/unit/tool-direct-cli.test.ts @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const initMock = vi.fn(); +const callToolMock = vi.fn(); +const writeSyncMock = vi.fn(); + +vi.mock('../../src/mcp/local/local-backend.js', () => ({ + LocalBackend: class { + init = initMock; + callTool = callToolMock; + }, +})); + +vi.mock('node:fs', () => ({ + writeSync: writeSyncMock, +})); + +describe('direct CLI tool commands', () => { + beforeEach(() => { + vi.resetModules(); + initMock.mockReset(); + callToolMock.mockReset(); + writeSyncMock.mockReset(); + initMock.mockResolvedValue(true); + }); + + it('dispatches detect_changes with CLI-shaped arguments', async () => { + callToolMock.mockResolvedValue({ + summary: { + changed_files: 1, + changed_count: 2, + affected_count: 1, + risk_level: 'low', + }, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({ + scope: 'compare', + baseRef: 'main', + repo: 'gitnexus', + }); + + expect(callToolMock).toHaveBeenCalledWith('detect_changes', { + scope: 'compare', + base_ref: 'main', + repo: 'gitnexus', + }); + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('Risk level: low')); + }); + + it('prints "No changes detected." when changed_count is 0', async () => { + callToolMock.mockResolvedValue({ + summary: { changed_files: 0, changed_count: 0, affected_count: 0, risk_level: 'low' }, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('No changes detected.')); + }); + + it('prints error message when result contains an error', async () => { + callToolMock.mockResolvedValue({ error: 'index is stale' }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('Error: index is stale')); + }); + + it('truncates changed_symbols list beyond 15 and shows overflow count', async () => { + const symbols = Array.from({ length: 17 }, (_, i) => ({ + type: 'function', + name: `fn${i}`, + filePath: `src/file${i}.ts`, + })); + callToolMock.mockResolvedValue({ + summary: { changed_files: 17, changed_count: 17, affected_count: 0, risk_level: 'low' }, + changed_symbols: symbols, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + const output: string = writeSyncMock.mock.calls[0][1]; + expect(output).toContain('function fn14 → src/file14.ts'); + expect(output).not.toContain('fn15'); + expect(output).toContain('... and 2 more'); + }); + + it('truncates affected_processes list beyond 10', async () => { + const processes = Array.from({ length: 12 }, (_, i) => ({ + name: `proc${i}`, + step_count: 3, + changed_steps: [{ symbol: `sym${i}` }], + })); + callToolMock.mockResolvedValue({ + summary: { changed_files: 1, changed_count: 1, affected_count: 12, risk_level: 'low' }, + affected_processes: processes, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + const output: string = writeSyncMock.mock.calls[0][1]; + expect(output).toContain('proc9'); + expect(output).not.toContain('proc10'); + }); +}); diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 4274716a7..231f55e3c 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -2,7 +2,7 @@ * Unit Tests: MCP Tool Definitions * * Tests: GITNEXUS_TOOLS from tools.ts - * - All 16 tools are defined (per-repo + group_*) + * - All 13 tools are defined (per-repo + group_list/group_sync) * - Each tool has valid name, description, inputSchema * - Required fields are correct * - Optional repo parameter is present on tools that need it @@ -10,17 +10,11 @@ import { describe, it, expect } from 'vitest'; import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js'; -const GROUP_TOOLS = new Set([ - 'group_list', - 'group_sync', - 'group_contracts', - 'group_query', - 'group_status', -]); +const GROUP_TOOLS = new Set(['group_list', 'group_sync']); describe('GITNEXUS_TOOLS', () => { - it('exports all tools (7 base + 3 route/tool/shape + 1 api_impact + 5 group)', () => { - expect(GITNEXUS_TOOLS).toHaveLength(16); + it('exports all tools (7 base + 3 route/tool/shape + 1 api_impact + 2 group)', () => { + expect(GITNEXUS_TOOLS).toHaveLength(13); }); it('contains all expected tool names', () => { @@ -101,23 +95,29 @@ describe('GITNEXUS_TOOLS', () => { } }); - it('group_contracts has optional repo filter', () => { - const groupContracts = GITNEXUS_TOOLS.find((t) => t.name === 'group_contracts')!; - expect(groupContracts.inputSchema.properties).toHaveProperty('repo'); - expect(groupContracts.inputSchema.required).not.toContain('repo'); - }); - it('group tools without backend repo param omit repo property', () => { - for (const name of ['group_list', 'group_status', 'group_sync', 'group_query'] as const) { + for (const name of ['group_list', 'group_sync'] as const) { const tool = GITNEXUS_TOOLS.find((t) => t.name === name)!; expect(tool.inputSchema.properties).not.toHaveProperty('repo'); } }); - it('group_query requires name and query', () => { - const groupQuery = GITNEXUS_TOOLS.find((t) => t.name === 'group_query')!; - expect(groupQuery.inputSchema.required).toContain('name'); - expect(groupQuery.inputSchema.required).toContain('query'); + it('impact, query, and context expose optional service with minLength', () => { + for (const n of ['impact', 'query', 'context'] as const) { + const tool = GITNEXUS_TOOLS.find((t) => t.name === n)!; + const svc = tool.inputSchema.properties.service; + expect(svc, n).toBeDefined(); + expect(svc!.minLength).toBe(1); + } + }); + + it('impact schema bounds match cross-impact validation ranges', () => { + const impact = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!; + expect(impact.inputSchema.properties.maxDepth.minimum).toBe(1); + expect(impact.inputSchema.properties.maxDepth.maximum).toBe(32); + expect(impact.inputSchema.properties.minConfidence.minimum).toBe(0); + expect(impact.inputSchema.properties.minConfidence.maximum).toBe(1); + expect(impact.inputSchema.properties.timeoutMs.maximum).toBe(3600000); }); it('detect_changes scope has correct enum values', () => {