diff --git a/README.md b/README.md index bfcca5117..a761716e7 100644 --- a/README.md +++ b/README.md @@ -616,6 +616,7 @@ Most `analyze` knobs are also CLI flags (`--workers`, `--worker-timeout`, `--max | `GITNEXUS_VERBOSE` | unset | When `1`, enables verbose ingestion logs (skipped-file warnings, per-chunk throughput, parse-cache stats). Equivalent to `--verbose`. | Debugging an analyze that "completed" but seems to have missed files; tuning `--workers` / chunk concurrency against observable throughput. | | `GITNEXUS_EMBEDDING_RETRY_TIMEOUTS` | unset | When truthy (`1`/`true`/`yes`), per-attempt HTTP embedding timeouts (`TimeoutError` on fetch or body read) go through the bounded `GITNEXUS_EMBEDDING_MAX_ATTEMPTS` retry loop instead of failing the job. Any other value leaves it off, so cloud/default timeouts remain terminal. | Local accelerators that drop a device lock when the client disconnects and succeed on the next request (observed with FastFlowLM on Ryzen AI). | | `GITNEXUS_EMBEDDING_SIDECAR_TIMEOUT_MS` | `180000` (3 minutes) | Per-request IPC timeout for local embedding sidecar embed batches. On overrun the parent SIGKILLs the sidecar child and rejects the batch. Init still uses the HF download budget (`HF_DOWNLOAD_TIMEOUT_MS` × attempts), not this knob. | Large embed batches or slow local ONNX inference cause sidecar request timeouts during `analyze --embeddings`, `embeddings sync`, serve, or MCP. | +| `GITNEXUS_VECTOR_MAX_DISTANCE` | `0.6` for CLI/MCP `query`; `0.5` for standalone semantic search | Maximum cosine distance for vector hits, applied to both indexed search and exact-scan fallback. Hits must have `distance < cutoff`. Unset/blank uses the default; non-numeric, non-finite, zero, or negative values warn and use the default; finite values above `2` warn and clamp to `2`. | Paraphrased queries have weak semantic recall with your embedding model. See [Vector cutoff tuning](#vector-cutoff-tuning). | | `GITNEXUS_ANALYZER_IDENTITY_IN_PROCESS_GUARDS` | unset | When truthy (`1`/`true`/`yes`), forces in-process cache-guard validation once a batch has ≥128 requests. In-process mode also auto-selects when `packageRoot`/`buildRoot` fail `W_OK` with `EACCES`/`EROFS`. Otherwise those large batches use a Node subprocess probe. Batches under 128 always stay in-process. | Trusted or read-only installs where two identity subprocess spawns per analyze dominate wall time; leave unset to keep the default isolation path on writable trees. | | `GITNEXUS_RESOLVE_DEF_GRAPH_ID_MEMO` | on (unset) | Memoizes `resolveDefGraphId` per `nodeLookup` instance (WeakMap). Enabled by default. Set to `0`/`false`/`off`/`no` to disable and recompute on every call (debug / bisect memo bugs). | Suspecting stale graph-id resolution after a lookup rebuild, or comparing memo vs uncached cost on a large index. | | `GITNEXUS_AUTH_TOKEN` | unset | Bearer token required when `eval-server` binds beyond loopback. May also be read from `.env.local` or `.env`; shell values take precedence. | Exposing the evaluation HTTP tools to a container, VM, or LAN. | @@ -655,6 +656,22 @@ Most `analyze` knobs are also CLI flags (`--workers`, `--worker-timeout`, `--max | `GITNEXUS_PUBLIC_ORIGIN` | unset | The single browser origin `serve` is reached through, added to the CORS allowlist and to the write-route origin guard. A wildcard bind (`0.0.0.0`) has no host identity, so without this the server's own UI is refused. **Setting it currently refuses to start:** `serve` has no authentication, requests carrying no `Origin` header already reach `POST /api/analyze` and `DELETE /api/repo`, and this is the setting that would admit browser writes on top of that. Matching rules for when the gate lifts: the hostname must match exactly, and so must the scheme. A value with no scheme (`app.example.com`) means `https`, since a bare host comes from platform service discovery and those terminate TLS; spell out `http://app.example.com` for plain HTTP. An explicit port must match; with no port, any port on that hostname is accepted. Anything that is not one reachable host (a list, `*`, a bare port number, a `:0` port, a trailing dot) warns at startup and allows nothing. | `gitnexus serve` runs behind a reverse proxy or on a wildcard bind, and the UI's index/delete requests return `origin_not_allowed`. | | `GITNEXUS_TRUST_PROXY` | `loopback, linklocal, uniquelocal` | Express `trust proxy` value — which upstream hops may set `X-Forwarded-*`, and so what the per-IP rate limiter reads as the client IP. Set it to the exact number of proxies you control. Every hop past that is one more entry of the chain the caller gets to write. `false`/`no`/`off` (and a `0` hop count) trust no hop; a proxy list Express can compile (`loopback`, `10.0.0.0/8, 127.0.0.1`) names them instead. `true`/`yes`/`on` is **rejected**: it reads the client-controlled leftmost `X-Forwarded-For` entry, so a spoofed chain earns a fresh rate-limit key per request, and express-rate-limit rejects it too (`ERR_ERL_PERMISSIVE_TRUST_PROXY`). Counts above `16` are rejected as well, as a sanity ceiling rather than a safety boundary. Any invalid value warns and falls back to the default. Bind non-loopback with this unset and `serve` warns: a load balancer outside the private ranges is untrusted, so every request keys to the balancer and the per-IP limit becomes one shared limit. | `serve` sits behind a load balancer outside the private ranges (AWS ALB, Cloudflare, CGNAT), where every request otherwise collapses to the proxy hop and rate limiting goes global. | +### Vector cutoff tuning + +`GITNEXUS_VECTOR_MAX_DISTANCE` controls which vector candidates enter hybrid search. Cosine distance is lower for more similar vectors; the appropriate cutoff depends on the embedding model and repository. A higher cutoff can recover useful semantic matches, but can also admit less relevant hits and change the fused ranking. The default is `0.6` for CLI and MCP `query`, and `0.5` for the standalone semantic-search function. + +For an index that already has embeddings, compare the default with `0.8` and `1.0` on representative queries, including paraphrases, exact symbol names, and queries that should have no relevant result: + +```bash +GITNEXUS_VECTOR_MAX_DISTANCE=0.8 gitnexus query "how are expired sessions removed" --limit 10 +``` + +Check both whether the expected symbols appear and where they rank. In [#3457](https://github.com/abhigyanpatwari/GitNexus/issues/3457), the reporter's `voyage-code-4` sample found 20 of 32 paraphrased targets at `0.6`, 25 at `0.8`, and 28 at `1.0`. Those values are a tuning starting point for that model, not universal recommendations: one paraphrased target fell from first to twelfth at `1.0`. The sample covered eight repositories, recorded no raw distances, used agent-written queries after reading the code, and did not send Voyage's `input_type`. + +Set the variable in the process that runs the search. For MCP, add it to the server's launch environment and restart the server; for `gitnexus serve`, set it in that process's environment and restart it. Setting it only during `analyze`, or exporting it in another shell, does not configure an existing server. Changing the cutoff requires no reindex. + +The cutoff only filters available vector candidates. It cannot add missing embeddings or repair a mismatch between the indexing and query-time embedding configuration; see the [embeddings runbook](RUNBOOK.md#embeddings). A cutoff of `2` is very permissive, but still rejects distance exactly `2` and retains the search candidate limits. +
diff --git a/RUNBOOK.md b/RUNBOOK.md index 2f2c06f14..e2e1de94c 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -87,6 +87,14 @@ npx gitnexus analyze --force If it recurs, the cause is almost always environmental rather than a code defect: check free disk space on the volume holding `.gitnexus/`, make sure no second `analyze` is running against the same repo (both use `.gitnexus/csv` for staging), then run `npx gitnexus doctor`. The check compares in-memory relationship totals (including streamed rows) against what the DB hands back, and is deliberately skipped on incremental runs, where the two counts are not comparable. +**Weak semantic recall despite existing embeddings:** If paraphrased queries look like keyword-only search, the distance cutoff may be too strict for the embedding model. CLI and MCP `query` default to `0.6`. From the indexed repository, try a broader cutoff: + +```bash +GITNEXUS_VECTOR_MAX_DISTANCE=0.8 npx gitnexus query "how are expired sessions removed" --limit 10 +``` + +Compare target inclusion and ranking against the default; a broader cutoff also admits less relevant hits. Set the variable in the MCP/serve launch environment and restart that process when tuning a server. A cutoff change needs no reindex and setting it only during `analyze` does not persist it. If the index has no vectors, generate embeddings first; if the embedding model or dimensions differ between indexing and querying, align that configuration and regenerate vectors. See [Vector cutoff tuning](README.md#vector-cutoff-tuning) for validation rules and the limited `voyage-code-4` evidence from #3457. + **Large repos:** Analyze may skip or limit embedding work when node counts are very high; watch CLI output. --- diff --git a/gitnexus/bench/scope-capture/baselines.json b/gitnexus/bench/scope-capture/baselines.json index 4845f23fb..60ffca32d 100644 --- a/gitnexus/bench/scope-capture/baselines.json +++ b/gitnexus/bench/scope-capture/baselines.json @@ -119,7 +119,8 @@ "_rebaselined_3354_callable_alternatives": "#3354 callable alternatives: a callable chosen by a value-selecting source now flows every branch it can yield (`a ?? b`, `a || b`, `a or b`, `c ? a : b`, statement `if`, elvis), and an operator branch (`x == f || g`) stays opaque instead of seeding a qualified name. Verified by running the BASE (merge-base 233ca2849) and HEAD emitters over the SAME HEAD fixture corpus: every added or removed match is an `@callable-flow.*` match on one of those sources, and the pre-existing corpus is byte-identical (all other languages: zero delta). The rest of the drift is corpus growth from this PR's regression fixture, which this bench globs. Corpus growth: ruby-callable-alternatives/app.rb (+1 file, +58 groups). Emitter delta: +5 / -0: seeds for single-statement `if` / `elsif` branches (run_then, run_else, run_sweep, run_a, run_b); the multi-statement branch contributes nothing. capture_groups_fp 1358 -> 1416, fixture_count 91 -> 92; synthetic counts unchanged; scaling 1.04 < 1.5. Prior 1c8c9c4b54036fa24c2a81e39ea530e938645c856d369075e5f437da78218c57 -> 45e65d9fa8a9e5e905ddb596b179b77c86ed13ab5139c6b17ccaaf7315dfe46a." }, "swift": { - "fingerprint": "d56406c2645637042899cfcc8dc73f603d77caef0a9a2bac179ffbec848258a3", + "fingerprint": "2507ac75ba6fb47c272a5caa97b5e8b9f6ebc5098c15295d05c7bff3cabc321a", + "_rebaselined_3425_injected_closure": "Swift capture fixture growth: 83 to 85 fixtures, 1436 to 1553 capture groups; scaling budget unchanged.", "_rebaselined_3355_xcode_and_import_fixtures": "#3355 follow-up: new swift-xcode-targets fixture (four sources) and two sources added to swift-nested-packages (a Docs/Net folder decoy and an `import Net` caller). The capture query is unchanged; this is fixture-corpus growth only. capture_groups_fp 1393 -> 1436 and fixture_count 77 -> 83; synthetic scale counts remain 5012/16012. Prior aea33bf12f57561be7e3125929fb98cec7aed5a681e25aad435c7bb558747853 -> d56406c2645637042899cfcc8dc73f603d77caef0a9a2bac179ffbec848258a3; measured scaling 1.036 < 1.5.", "_rebaselined_3355_nested_packages": "#3355: new swift-nested-packages fixture (three nested Package.swift manifests, six sources) for nested-package module grouping. The capture query is unchanged; this is fixture-corpus growth only. capture_groups_fp 1333 -> 1393 and fixture_count 68 -> 77; synthetic scale counts remain 5012/16012. Prior c9fc553662f0db18027fba882e3ff730744f6153cc46eca7b00667fabd6a0df8 -> aea33bf12f57561be7e3125929fb98cec7aed5a681e25aad435c7bb558747853; measured scaling 1.023 < 1.5.", "scaling_budget": 1.5, @@ -187,7 +188,8 @@ "capture_groups_fp": 680 }, "typescript": { - "fingerprint": "77c9b4ea654123a64972db8348190ec250b669472b150db65ea8c7f20b355467", + "fingerprint": "a7972d87abd253583dafa37443bee8ddf5c635d41b4dfbb0c831efda71d82b47", + "_rebaselined_3446_mcp_tools_fixture": "#3446 adds typescript-mcp-tools/src/{handlers,server,tools}.ts: corpus growth only, with fixture_count 171 -> 174 and capture_groups_fp 2753 -> 2894. Excluding only that fixture directory restores the prior fingerprint 77c9b4ea654123a64972db8348190ec250b669472b150db65ea8c7f20b355467 exactly. The scope emitter, synthetic capture counts (4503/14403), scaling budget, and other language baselines are unchanged.", "_rebaselined_3190": "Capture matches now retain explicit ESM export/private evidence, including synthesized default HOCs; CommonJS surfaces remain undecided. Capture group counts unchanged. Scaling budget unchanged.", "scaling_budget": 1.5, "_rebaselined_2934_import_type_only": "#2934: `import-decomposer.ts` attaches a presence-only `@import.type-only` synthetic capture to specifiers `tsc` erases, so `check --cycles` can stop counting type-only edges as initialization cycles. DIGEST DRIFT ONLY, NOT A CAPTURE-SET CHANGE \u2014 the tag is added to import matches that already existed, never a new match, the same shape as the #2747 receiver-chain rebaseline. Every count is unchanged: capture_groups_fp 2414, fixture_count 155, capture_groups_small/large 4503/14403 (those measure the SYNTHETIC scaling source, which has no imports at all). The fingerprint moves because `canonicalizeMatch` in measure.mjs hashes every TAG on every match, synthetics included, so one extra presence-only tag on an existing match rewrites that match's canonical string. Attribution is exact, not inferred: neutralizing ONLY the `m['@import.type-only'] = \u2026` assignment in import-decomposer.ts and re-running returns the fingerprint to c2fbf8a89e5686dd\u2026 byte-for-byte, so nothing else in the TypeScript capture stream moved. All 14 other languages report ok. Scaling 0.997 < 1.5. NOTE ON THE CONTROL: javascript did not move (2026993b\u2026, 43 fixtures), but it is a WEAK control here \u2014 `import type` is TypeScript-only syntax, so a JS corpus cannot express the construct and could not have drifted either way. It evidences no collateral damage, not the correctness of the TS change; the exact-attribution check above is what does that. Prior c2fbf8a89e5686dd1ff3659b20d41d8b05ebcc9790356e3653ee0c8ca5d365c8 -> f719163eb03a447c9e40ca316a905dd76cee82192a75a403df478ebbdc13e98f.", diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index 3b3fea7be..5cc0fcbce 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -854,9 +854,9 @@ } }, "node_modules/@nodable/entities": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@nodable/entities/-/entities-3.0.0.tgz", - "integrity": "sha512-8L9xFeTYKhm49xfIypoe2W5wV1m/3Z58kT+7kR9A8OyFxcPduI4VmxaUMQyKYrRjUoLLSXv6EKKID5Tvj9cUVw==", + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@nodable/entities/-/entities-3.1.0.tgz", + "integrity": "sha512-LsS/DjHr+uDM647Gru/cA8+J3a3HfhttwCKLyuoyN7yXTFCxKBtSgMQBvrW9yNPa2/zDRUNuGPaH5QUFoa4arQ==", "funding": [ { "type": "github", @@ -2733,9 +2733,9 @@ } }, "node_modules/fast-xml-parser": { - "version": "5.11.1", - "resolved": "https://registry.npmjs.org/fast-xml-parser/-/fast-xml-parser-5.11.1.tgz", - "integrity": "sha512-TBw6K/fxoQGGjCmZDw9w/ZwP3uDcnTM4YH/g+PFRWr8sbe5idXtxNN6vITh4+1ruCZaho6uBFurElsA7F0zzgw==", + "version": "5.11.2", + "resolved": "https://registry.npmjs.org/fast-xml-parser/-/fast-xml-parser-5.11.2.tgz", + "integrity": "sha512-R9iDuNrQYeQut46cn2r2wHKn4HYzVDvDm5J1wW+koZewykv0yuO2HChTYeZtrULyHIDM9cj9TUXloCsueCQUog==", "funding": [ { "type": "github", @@ -2744,7 +2744,7 @@ ], "license": "MIT", "dependencies": { - "@nodable/entities": "^3.0.0", + "@nodable/entities": "^3.0.1", "fast-xml-builder": "^1.2.0", "is-unsafe": "^2.0.0", "path-expression-matcher": "^1.6.2", diff --git a/gitnexus/src/cli/group-status-format.ts b/gitnexus/src/cli/group-status-format.ts index f5d09c47e..bd94ccc9a 100644 --- a/gitnexus/src/cli/group-status-format.ts +++ b/gitnexus/src/cli/group-status-format.ts @@ -4,16 +4,18 @@ * against a real group, which is how `STALE (-1 commits behind)` went unnoticed * (#3256). */ +import type { StalenessStatus } from '../core/staleness-status.js'; /** The fields of a `groupStatus` repo row the index column reads. */ export interface GroupRepoIndexRow { indexStale: boolean; commitsBehind?: number; + status?: StalenessStatus; } /** - * The index column of a `group status` row. The output is unchanged except - * for one case: a count that is not a real count renders as `?`. + * The index column of a `group status` row. Diverged indexes differ from HEAD + * without a forward commit count; an unavailable count renders as `?`. * * `group/service.ts` has always reported a repo with no recorded commit as * `{ indexStale: true, commitsBehind: -1 }`. The previous `?? '?'` fallback @@ -21,6 +23,7 @@ export interface GroupRepoIndexRow { */ export const formatIndexStatusCell = (row: GroupRepoIndexRow): string => { if (!row.indexStale) return 'OK '; + if (row.status === 'diverged') return 'STALE (index differs from HEAD)'; const n = row.commitsBehind; const count = typeof n === 'number' && n >= 0 ? String(n) : '?'; return `STALE (${count} commits behind)`; diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 15114ab41..8cedc843f 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -430,5 +430,5 @@ export const en = { 'help.identityCache.environment': '\nAnalyzer identity cache:\n GITNEXUS_ANALYZER_IDENTITY_CACHE_DIR=/absolute/protected/dir\n Operator-trusted persistent cache for warm cross-process status. The directory must pre-exist, be outside the GitNexus package/build roots, and contain no symlink or junction components. Defaults remain fail-closed on platforms without POSIX ownership APIs.', 'help.analyze.environment': - '\nEnvironment variables:\n GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)\n GITNEXUS_MAX_FILE_SIZE=N Override large-file skip threshold (KB). Default 512, max 32768.\n GITNEXUS_STORAGE_PATH=/absolute/index Complete external index directory. Preserves the existing configuration semantics and overrides GITNEXUS_STORAGE_ROOT when both are set.\n GITNEXUS_STORAGE_ROOT=/absolute/root External index root; each repository uses an isolated -/ slot.\n GITNEXUS_CONTENT_RETENTION=full Source-text retention profile: full, symbol, or none. Default full.\n GITNEXUS_ANALYZER_IDENTITY_CACHE_DIR=/absolute/protected/dir Operator-trusted persistent analyzer identity cache; must pre-exist, be outside package/build roots, and contain no symlink/junction components.\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker idle timeout in milliseconds. Default 30000.\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL auto-checkpoint threshold in bytes (default 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB).\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker job byte budget. Default 8388608.\n GITNEXUS_WORKER_POOL_SIZE=N Parse worker count override. Default cores-1 capped at 16.\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N Concurrent in-flight parse chunks. Default 2.\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N Max replacement spawns per slot before drop. Default 3.\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N Total retry wall-time per job. Default 5x sub-batch timeout.\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N Per-slot deaths to trip circuit breaker. Default max(3, poolSize).\n GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS=N Max wait at pool shutdown for a retired worker still inside native code (terminated at its next safe point instead of aborting the process). Default 30000.\n GITNEXUS_CPP_CAPTURE_BUDGET_MS=N Per-file wall-clock budget for C++ capture extraction; on breach the file keeps partial captures with a warning. Default 20000.\n GITNEXUS_EMBEDDING_THREADS=N Limit local ONNX CPU threads for --embeddings.\n GITNEXUS_EMBEDDING_RETRY_TIMEOUTS=1 Retry per-attempt HTTP embedding timeouts through GITNEXUS_EMBEDDING_MAX_ATTEMPTS (default off; timeouts stay terminal).\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N Max embedding chunks for exact-scan fallback. Default 10000.\n GITNEXUS_VECTOR_MAX_DISTANCE=N Max accepted semantic/vector cosine distance (0 < N <= 2; higher values clamp to 2). Default 0.6 for MCP, 0.5 elsewhere.\n GITNEXUS_MAX_PROCESSES=N Process-detection process cap (positive integer). Replaces the dynamic max(20, round(symbols/10)) formula. Distinct from query-time IMPACT_MAX_CHUNKS.\n GITNEXUS_MAX_PROCESS_BRANCHING=N Process-detection per-node branching cap. Default 4.\n GITNEXUS_MAX_PROCESS_TRACE_DEPTH=N Process-detection DFS depth cap. Default 10.\n GITNEXUS_MAX_ENTRY_POINT_CANDIDATES=N Ranked entry-point candidate pool. Default 200. Raise when the warning names this knob; doubling is the usual first raise.\n\nCLI flags take precedence over `.gitnexusrc`, which takes precedence over env vars, which take precedence over built-in defaults.\n\nTip: `.gitnexusignore` supports `.gitignore`-style negation. Add e.g.\n `!__tests__/` to index a directory that is auto-filtered by default (#771).', + '\nEnvironment variables:\n GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)\n GITNEXUS_MAX_FILE_SIZE=N Override large-file skip threshold (KB). Default 512, max 32768.\n GITNEXUS_STORAGE_PATH=/absolute/index Complete external index directory. Preserves the existing configuration semantics and overrides GITNEXUS_STORAGE_ROOT when both are set.\n GITNEXUS_STORAGE_ROOT=/absolute/root External index root; each repository uses an isolated -/ slot.\n GITNEXUS_CONTENT_RETENTION=full Source-text retention profile: full, symbol, or none. Default full.\n GITNEXUS_ANALYZER_IDENTITY_CACHE_DIR=/absolute/protected/dir Operator-trusted persistent analyzer identity cache; must pre-exist, be outside package/build roots, and contain no symlink/junction components.\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker idle timeout in milliseconds. Default 30000.\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL auto-checkpoint threshold in bytes (default 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB).\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker job byte budget. Default 8388608.\n GITNEXUS_WORKER_POOL_SIZE=N Parse worker count override. Default cores-1 capped at 16.\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N Concurrent in-flight parse chunks. Default 2.\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N Max replacement spawns per slot before drop. Default 3.\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N Total retry wall-time per job. Default 5x sub-batch timeout.\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N Per-slot deaths to trip circuit breaker. Default max(3, poolSize).\n GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS=N Max wait at pool shutdown for a retired worker still inside native code (terminated at its next safe point instead of aborting the process). Default 30000.\n GITNEXUS_CPP_CAPTURE_BUDGET_MS=N Per-file wall-clock budget for C++ capture extraction; on breach the file keeps partial captures with a warning. Default 20000.\n GITNEXUS_EMBEDDING_THREADS=N Limit local ONNX CPU threads for --embeddings.\n GITNEXUS_EMBEDDING_RETRY_TIMEOUTS=1 Retry per-attempt HTTP embedding timeouts through GITNEXUS_EMBEDDING_MAX_ATTEMPTS (default off; timeouts stay terminal).\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N Max embedding chunks for exact-scan fallback. Default 10000.\n GITNEXUS_VECTOR_MAX_DISTANCE=N Max accepted semantic/vector cosine distance (0 < N <= 2; higher values clamp to 2). Default 0.6 for CLI/MCP query, 0.5 for standalone semantic search. Tune for your embedding model: higher values can improve recall but may reduce relevance. Set in the query/server environment; no reindex needed.\n GITNEXUS_MAX_PROCESSES=N Process-detection process cap (positive integer). Replaces the dynamic max(20, round(symbols/10)) formula. Distinct from query-time IMPACT_MAX_CHUNKS.\n GITNEXUS_MAX_PROCESS_BRANCHING=N Process-detection per-node branching cap. Default 4.\n GITNEXUS_MAX_PROCESS_TRACE_DEPTH=N Process-detection DFS depth cap. Default 10.\n GITNEXUS_MAX_ENTRY_POINT_CANDIDATES=N Ranked entry-point candidate pool. Default 200. Raise when the warning names this knob; doubling is the usual first raise.\n\nCLI flags take precedence over `.gitnexusrc`, which takes precedence over env vars, which take precedence over built-in defaults.\n\nTip: `.gitnexusignore` supports `.gitignore`-style negation. Add e.g.\n `!__tests__/` to index a directory that is auto-filtered by default (#771).', } as const; diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 9519954d2..f333188b7 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -392,5 +392,5 @@ export const zhCN = { 'help.identityCache.environment': '\n分析器身份缓存：\n GITNEXUS_ANALYZER_IDENTITY_CACHE_DIR=/absolute/protected/dir\n 由操作员明确信任的持久缓存，用于跨进程快速查询状态。目录必须预先存在、位于 GitNexus 包/构建根目录之外，且路径中不得包含符号链接或 junction。缺少 POSIX 所有权 API 的平台默认保持故障关闭。', 'help.analyze.environment': - '\n环境变量：\n GITNEXUS_NO_GITIGNORE=1 跳过 .gitignore 解析（仍读取 .gitnexusignore）\n GITNEXUS_MAX_FILE_SIZE=N 覆盖大文件跳过阈值（KB）。默认 512，最大 32768。\n GITNEXUS_STORAGE_PATH=/absolute/index 完整外部索引目录。保留既有配置语义；与 GITNEXUS_STORAGE_ROOT 同时设置时优先使用。\n GITNEXUS_STORAGE_ROOT=/absolute/root 外部索引根目录；每个仓库使用独立的 <仓库名>-<规范路径哈希>/ 子目录。\n GITNEXUS_CONTENT_RETENTION=full 源码文本保留策略：full、symbol 或 none。默认 full。\n GITNEXUS_ANALYZER_IDENTITY_CACHE_DIR=/absolute/protected/dir 由操作员明确信任的持久分析器身份缓存；目录必须预先存在、位于包/构建根目录之外，且路径中不得包含符号链接或 junction。\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker 空闲超时（毫秒）。默认 30000。\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL 自动 checkpoint 阈值（字节，默认 67108864 = 64 MiB；-1 保持 Ladybug 默认约 16 MiB）。\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker 作业字节预算。默认 8388608。\n GITNEXUS_WORKER_POOL_SIZE=N 解析 worker 数量覆盖值。默认 cores-1，最多 16。\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N 并发进行中的解析分块数。默认 2。\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N 每个 slot 丢弃前允许的最大替换进程数。默认 3。\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N 每个作业的总重试墙钟时间。默认 5 倍子批次超时。\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N 每个 slot 触发熔断的死亡次数。默认 max(3, poolSize)。\n GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS=N 线程池关闭时等待仍在原生代码中的已退役 worker 的最长时间（到达安全点后再终止，避免进程级 abort）。默认 30000。\n GITNEXUS_CPP_CAPTURE_BUDGET_MS=N C++ 捕获提取的每文件墙钟预算；超出后该文件保留部分捕获并输出警告。默认 20000。\n GITNEXUS_EMBEDDING_THREADS=N 限制 --embeddings 的本地 ONNX CPU 线程数。\n GITNEXUS_EMBEDDING_RETRY_TIMEOUTS=1 将单次 HTTP 嵌入超时纳入 GITNEXUS_EMBEDDING_MAX_ATTEMPTS 重试（默认关闭，超时仍为终止错误）。\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N exact-scan 回退的最大嵌入分块数。默认 10000。\n GITNEXUS_VECTOR_MAX_DISTANCE=N 语义/向量搜索接受的最大余弦距离（0 < N <= 2；超出则钳制为 2）。MCP 默认 0.6，其他路径默认 0.5。\n GITNEXUS_MAX_PROCESSES=N 流程检测的流程数量上限（正整数）。覆盖动态的 max(20, round(symbols/10)) 公式。与查询时的 IMPACT_MAX_CHUNKS 无关。\n GITNEXUS_MAX_PROCESS_BRANCHING=N 流程检测的单节点分支上限。默认 4。\n GITNEXUS_MAX_PROCESS_TRACE_DEPTH=N 流程检测的 DFS 深度上限。默认 10。\n GITNEXUS_MAX_ENTRY_POINT_CANDIDATES=N 排序后的入口点候选池。默认 200。仅在警告点名该上限时提高；那时通常先翻倍。\n\nCLI 参数优先于 `.gitnexusrc`，后者优先于环境变量，环境变量优先于内置默认值。\n\n提示：`.gitnexusignore` 支持 `.gitignore` 风格的取反。比如添加\n `!__tests__/` 可以索引默认自动过滤的目录（#771）。', + '\n环境变量：\n GITNEXUS_NO_GITIGNORE=1 跳过 .gitignore 解析（仍读取 .gitnexusignore）\n GITNEXUS_MAX_FILE_SIZE=N 覆盖大文件跳过阈值（KB）。默认 512，最大 32768。\n GITNEXUS_STORAGE_PATH=/absolute/index 完整外部索引目录。保留既有配置语义；与 GITNEXUS_STORAGE_ROOT 同时设置时优先使用。\n GITNEXUS_STORAGE_ROOT=/absolute/root 外部索引根目录；每个仓库使用独立的 <仓库名>-<规范路径哈希>/ 子目录。\n GITNEXUS_CONTENT_RETENTION=full 源码文本保留策略：full、symbol 或 none。默认 full。\n GITNEXUS_ANALYZER_IDENTITY_CACHE_DIR=/absolute/protected/dir 由操作员明确信任的持久分析器身份缓存；目录必须预先存在、位于包/构建根目录之外，且路径中不得包含符号链接或 junction。\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker 空闲超时（毫秒）。默认 30000。\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL 自动 checkpoint 阈值（字节，默认 67108864 = 64 MiB；-1 保持 Ladybug 默认约 16 MiB）。\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker 作业字节预算。默认 8388608。\n GITNEXUS_WORKER_POOL_SIZE=N 解析 worker 数量覆盖值。默认 cores-1，最多 16。\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N 并发进行中的解析分块数。默认 2。\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N 每个 slot 丢弃前允许的最大替换进程数。默认 3。\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N 每个作业的总重试墙钟时间。默认 5 倍子批次超时。\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N 每个 slot 触发熔断的死亡次数。默认 max(3, poolSize)。\n GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS=N 线程池关闭时等待仍在原生代码中的已退役 worker 的最长时间（到达安全点后再终止，避免进程级 abort）。默认 30000。\n GITNEXUS_CPP_CAPTURE_BUDGET_MS=N C++ 捕获提取的每文件墙钟预算；超出后该文件保留部分捕获并输出警告。默认 20000。\n GITNEXUS_EMBEDDING_THREADS=N 限制 --embeddings 的本地 ONNX CPU 线程数。\n GITNEXUS_EMBEDDING_RETRY_TIMEOUTS=1 将单次 HTTP 嵌入超时纳入 GITNEXUS_EMBEDDING_MAX_ATTEMPTS 重试（默认关闭，超时仍为终止错误）。\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N exact-scan 回退的最大嵌入分块数。默认 10000。\n GITNEXUS_VECTOR_MAX_DISTANCE=N 语义/向量搜索接受的最大余弦距离（0 < N <= 2；超出则钳制为 2）。CLI/MCP query 默认 0.6，独立语义搜索默认 0.5。应根据嵌入模型调节：提高阈值可增加召回，但可能降低相关性。在查询/服务器进程的环境中设置，无需重新索引。\n GITNEXUS_MAX_PROCESSES=N 流程检测的流程数量上限（正整数）。覆盖动态的 max(20, round(symbols/10)) 公式。与查询时的 IMPACT_MAX_CHUNKS 无关。\n GITNEXUS_MAX_PROCESS_BRANCHING=N 流程检测的单节点分支上限。默认 4。\n GITNEXUS_MAX_PROCESS_TRACE_DEPTH=N 流程检测的 DFS 深度上限。默认 10。\n GITNEXUS_MAX_ENTRY_POINT_CANDIDATES=N 排序后的入口点候选池。默认 200。仅在警告点名该上限时提高；那时通常先翻倍。\n\nCLI 参数优先于 `.gitnexusrc`，后者优先于环境变量，环境变量优先于内置默认值。\n\n提示：`.gitnexusignore` 支持 `.gitignore` 风格的取反。比如添加\n `!__tests__/` 可以索引默认自动过滤的目录（#771）。', } satisfies EnglishMessages; diff --git a/gitnexus/src/cli/skill-gen.ts b/gitnexus/src/cli/skill-gen.ts index f1dd45c3b..ed7f58232 100644 --- a/gitnexus/src/cli/skill-gen.ts +++ b/gitnexus/src/cli/skill-gen.ts @@ -114,21 +114,21 @@ export const generateSkillFiles = async ( } } - if (!communityResult || !communityResult.memberships.length) { + const memberships = communityResult?.rawMemberships ?? communityResult?.memberships ?? []; + if (!communityResult || !memberships.length) { console.log('\n Skills: no communities detected, skipping skill generation'); return { skills: [], outputPath: outputDir }; } console.log('\n Generating repo-specific skills...'); - // Step 1: Build communities from memberships (not the filtered communities array). - // The community processor skips singletons from its communities array but memberships - // include ALL assignments. For repos with sparse CALLS edges, the communities array - // can be empty while memberships still has useful groupings. + // Step 1: Use raw assignments for the fallback when all communities were + // filtered as singletons. Same-folder aggregation can still produce skills + // for these sparse graphs without emitting dangling MEMBER_OF edges. const communities = communityResult.communities.length > 0 ? communityResult.communities - : buildCommunitiesFromMemberships(communityResult.memberships, graph, repoPath); + : buildCommunitiesFromMemberships(memberships, graph, repoPath); const aggregated = aggregateCommunities(communities); @@ -145,11 +145,8 @@ export const generateSkillFiles = async ( } // Step 3: Build lookup maps - const membershipsByComm = buildMembershipMap(communityResult.memberships); - const nodeIdToCommunityLabel = buildNodeCommunityLabelMap( - communityResult.memberships, - communities, - ); + const membershipsByComm = buildMembershipMap(memberships); + const nodeIdToCommunityLabel = buildNodeCommunityLabelMap(memberships, communities); // Step 4: Ensure the shared project-skill root exists. Never clear it: it // also contains user-authored and standard GitNexus skills. @@ -185,7 +182,7 @@ export const generateSkillFiles = async ( const entryPoints = gatherEntryPoints(members); // Gather execution flows - const flows = gatherFlows(community.rawIds, processResult?.processes || []); + const flows = gatherFlows(community.rawIds, members, processResult?.processes || []); // Gather cross-community connections const connections = gatherCrossConnections( @@ -529,14 +526,26 @@ const gatherEntryPoints = (members: MemberSymbol[]): MemberSymbol[] => { /** * @brief Gather execution flows touching this community * @param {string[]} rawIds - Raw community IDs for this aggregated community + * @param {MemberSymbol[]} members - Member symbols, including raw singleton assignments * @param {ProcessNode[]} processes - All detected processes - * @returns {ProcessNode[]} Processes whose communities intersect rawIds, sorted by stepCount + * @returns {ProcessNode[]} Processes matching the community IDs or member symbols, sorted by stepCount */ -const gatherFlows = (rawIds: string[], processes: ProcessNode[]): ProcessNode[] => { +const gatherFlows = ( + rawIds: string[], + members: MemberSymbol[], + processes: ProcessNode[], +): ProcessNode[] => { const rawIdSet = new Set(rawIds); + const memberIds = new Set(members.map((member) => member.id)); return processes - .filter((proc) => proc.communities.some((cid) => rawIdSet.has(cid))) + .filter( + (proc) => + proc.communities.some((cid) => rawIdSet.has(cid)) || + // Filtered singleton communities are absent from process metadata, + // but their symbols still participate in detected execution traces. + proc.trace.some((nodeId) => memberIds.has(nodeId)), + ) .sort((a, b) => b.stepCount - a.stepCount); }; diff --git a/gitnexus/src/config/ignore-service.ts b/gitnexus/src/config/ignore-service.ts index ac2dc7704..6af0e479a 100644 --- a/gitnexus/src/config/ignore-service.ts +++ b/gitnexus/src/config/ignore-service.ts @@ -1,5 +1,13 @@ import ignore, { type Ignore } from 'ignore'; -import { existsSync } from 'fs'; +import { + closeSync, + constants as fsConstants, + existsSync, + fstatSync, + lstatSync, + openSync, + readFileSync, +} from 'fs'; import fs from 'fs/promises'; import nodePath from 'path'; import type { Path } from 'path-scurry'; @@ -531,9 +539,163 @@ const hasExplicitUnignore = (ig: Ignore, rel: string): boolean => { return false; }; +/** + * Read a nested `.gitignore` only if it is a regular file, not a symlink. + * + * git does not follow a symlinked `.gitignore` in the working tree, and + * reading one could pull rules from outside the repository. Where the + * platform supports it, the file is opened with O_NOFOLLOW (a symlink fails + * with ELOOP). Windows has no O_NOFOLLOW, so there the path is lstat'ed after + * opening and must be the same regular file as the open descriptor. Either + * way the content is read through the descriptor that was checked, never by + * path, so the file cannot be swapped between the check and the read. + * + * The open also passes O_NONBLOCK where it exists. Opening a FIFO for reading + * blocks in open(2) until a writer appears, so a `.gitignore` that is a FIFO + * would hang the scan before the isFile() check could reject it (glob's + * ignore callback is synchronous). The flag makes that open return at once + * and changes nothing for a regular file. Same reasoning as readBoundedFile + * in src/core/ingestion/asyncapi/document.ts. + */ +const readNestedGitignore = (filePath: string): string | null => { + const noFollow = fsConstants.O_NOFOLLOW; + const nonBlock = fsConstants.O_NONBLOCK; + const fd = openSync(filePath, fsConstants.O_RDONLY | (noFollow ?? 0) | (nonBlock ?? 0)); + try { + const stat = fstatSync(fd); + if (!stat.isFile()) return null; + if (noFollow === undefined) { + const link = lstatSync(filePath); + if (!link.isFile() || link.ino !== stat.ino || link.dev !== stat.dev) return null; + } + return readFileSync(fd, 'utf-8'); + } finally { + closeSync(fd); + } +}; + +/** + * Resolve `.gitignore` files below the repository root (#2675). + * + * `loadIgnoreRules` only reads the root `.gitignore`, so a monorepo package + * or checked-out submodule with its own `.gitignore` had its generated + * output indexed anyway. Each nested file is read lazily (glob's filter is + * synchronous) and cached per directory, and its patterns are matched + * against the path relative to that directory, like git does. + * + * Returns the effective decision when nested rules affect the path or an + * ancestor, and `undefined` otherwise. Root rules participate so a directory + * negation does not erase independent root exclusions for its children. + * The caller still gives `.gitnexusignore` its higher precedence. + */ +const createNestedGitignoreMatcher = ( + repoPath: string, + rootRules: Ignore | null, +): ((rel: string, isDirectory: boolean) => boolean | undefined) => { + const rulesFor = (dirRel: string): Ignore | null => { + let rules: Ignore | null = null; + const filePath = nodePath.join(repoPath, dirRel, '.gitignore'); + try { + const content = readNestedGitignore(filePath); + if (content !== null) rules = ignore().add(content); + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException).code; + if (code !== 'ENOENT' && code !== 'ENOTDIR' && code !== 'ELOOP') { + logger.warn(` Warning: could not read ${filePath}: ${(err as Error).message}`); + } + } + return rules; + }; + + interface Scope { + base: string; + rules: Ignore; + } + interface DirectoryContext { + scopes: Scope[]; + ignored: boolean; + nested: boolean; + } + + const relativeTo = (base: string, rel: string): string => + base ? rel.slice(base.length + 1) : rel; + const match = (scopes: Scope[], rel: string, isDirectory: boolean) => { + for (let i = scopes.length - 1; i >= 0; i--) { + const { base, rules } = scopes[i]; + const sub = relativeTo(base, rel); + const result = rules.test(isDirectory ? `${sub}/` : sub); + if (result.ignored || result.unignored) { + return { ignored: result.ignored, nested: base !== '' }; + } + } + return undefined; + }; + + const contexts = new Map([ + [ + '', + { + scopes: rootRules ? [{ base: '', rules: rootRules }] : [], + ignored: false, + nested: false, + }, + ], + ]); + + const contextFor = (dir: string): DirectoryContext => { + const cached = contexts.get(dir); + if (cached) return cached; + const parentDir = nodePath.posix.dirname(dir); + const parent = contextFor(parentDir === '.' ? '' : parentDir); + // A .gitignore inside an excluded directory cannot bring that directory + // back. Do not read rules below a parent that traversal would prune. + if (parent.ignored) { + contexts.set(dir, parent); + return parent; + } + + const result = match(parent.scopes, dir, true); + const context: DirectoryContext = { + scopes: parent.scopes, + ignored: result?.ignored ?? false, + nested: parent.nested || (result?.nested ?? false), + }; + if (!context.ignored) { + context.scopes = parent.scopes.map(({ base, rules }) => { + const sub = relativeTo(base, dir); + if (!rules.test(`${sub}/`).ignored) return { base, rules }; + // A deeper rule let us enter this directory. Clear only its inherited + // exclusion in the shallower layer; child rules must still be tested. + // Keep patterns in their original scope, and escape this literal path. + const literal = sub.replace(/[\\*?\[\]]/g, '\\$&'); + return { + base, + rules: ignore() + .add(rules) + .add({ pattern: `!/${literal}/` }), + }; + }); + const rules = rulesFor(dir); + if (rules) context.scopes.push({ base: dir, rules }); + } + contexts.set(dir, context); + return context; + }; + + return (rel: string, isDirectory: boolean): boolean | undefined => { + const parentDir = nodePath.posix.dirname(rel); + const parent = contextFor(parentDir === '.' ? '' : parentDir); + if (parent.ignored) return parent.nested ? true : undefined; + const result = match(parent.scopes, rel, isDirectory); + if (parent.nested || result?.nested) return result?.ignored ?? false; + return undefined; + }; +}; + /** * Create a glob-compatible ignore filter combining: * - .gitignore / .gitnexusignore patterns (via `ignore` package) + * - nested .gitignore files, scoped to their own directory (#2675) * - Hardcoded DEFAULT_IGNORE_LIST, IGNORED_EXTENSIONS, IGNORED_FILES * * Returns an IgnoreLike object for glob's `ignore` option, @@ -550,6 +712,13 @@ const hasExplicitUnignore = (ig: Ignore, rel: string): boolean => { */ export const createIgnoreFilter = async (repoPath: string, options?: IgnoreOptions) => { const ig = await loadIgnoreRules(repoPath, options); + const skipGitignore = options?.noGitignore ?? !!process.env.GITNEXUS_NO_GITIGNORE; + const nestedIgnores = skipGitignore ? null : createNestedGitignoreMatcher(repoPath, ig); + // A nested negation outranks the root .gitignore, as in git, but not the + // user's .gitnexusignore, so keep a matcher for that file on its own. + const nexusIgnore = nestedIgnores + ? await loadIgnoreRules(repoPath, { ...options, noGitignore: true, noGlobalIgnore: true }) + : null; return { ignored(p: Path): boolean { @@ -557,6 +726,23 @@ export const createIgnoreFilter = async (repoPath: string, options?: IgnoreOptio // native separators on Windows when called through glob. const rel = p.relative().replace(/\\/g, '/'); if (!rel) return false; + // Nested .gitignore files below the root (#2675). .gitnexusignore + // comes first, then the deepest nested .gitignore, which outranks the + // root .gitignore as in git. The nested matcher preserves independent + // root exclusions; a nested negation never rescues a hardcoded default. + // With no nested opinion the original order below applies unchanged. + if (nestedIgnores) { + if (nexusIgnore) { + if (hasExplicitUnignore(nexusIgnore, rel) && !ig?.ignores(rel)) return false; + if (nexusIgnore.ignores(rel)) return true; + } + const nested = nestedIgnores(rel, false); + if (nested === true) return true; + if (nested === false) { + if (ig && hasExplicitUnignore(ig, rel) && !ig.ignores(rel)) return false; + return shouldIgnorePath(rel); + } + } // User's .gitnexusignore negation takes precedence over hardcoded // rules (#771). If any ancestor or the path itself was explicitly // unignored AND no more-specific rule re-ignores this exact path, @@ -576,6 +762,22 @@ export const createIgnoreFilter = async (repoPath: string, options?: IgnoreOptio // list check below is defense-in-depth — do not remove `dot: false` // assuming this covers it. const rel = p.relative().replace(/\\/g, '/'); + // Nested .gitignore files below the root (#2675), same precedence as in + // `ignored` above. + if (nestedIgnores && rel) { + if (nexusIgnore) { + if (hasExplicitUnignore(nexusIgnore, rel) && !ig?.ignores(rel + '/')) { + return false; + } + if (nexusIgnore.ignores(rel + '/')) return true; + } + const nested = nestedIgnores(rel, true); + if (nested === true) return true; + if (nested === false) { + if (ig && hasExplicitUnignore(ig, rel) && !ig.ignores(rel + '/')) return false; + return isHardcodedIgnoredDirectoryAtPath(repoPath, nodePath.join(repoPath, rel)); + } + } // User's .gitnexusignore negation takes precedence (#771) — if the // user explicitly unignored this directory or any ancestor via a // !pattern rule, allow descent even if the directory name is in diff --git a/gitnexus/src/core/git-staleness.ts b/gitnexus/src/core/git-staleness.ts index ece63a87c..e224b2319 100644 --- a/gitnexus/src/core/git-staleness.ts +++ b/gitnexus/src/core/git-staleness.ts @@ -17,8 +17,8 @@ export type { StalenessInfo, StalenessStatus } from './staleness-status.js'; const execFileAsync = promisify(execFile); /** - * Ceiling for one `git rev-list` staleness probe. Generous for the local - * history walk this is, and short enough that an unresponsive working tree + * Per-command ceiling for async `git rev-list` and both HEAD probes. + * Generous for local Git queries, and short enough that an unresponsive working tree * degrades to "not stale" quickly rather than holding a request open. */ const STALENESS_TIMEOUT_MS = 5_000; @@ -33,12 +33,23 @@ const behindHint = (n: number): string => const DIVERGED_HINT = "⚠️ Index is not at HEAD and the commit gap could not be counted — the recorded commit may no longer be in this clone's history. Run analyze tool to update."; +// `rev-list --count lastCommit..HEAD` answering 0 does NOT mean "HEAD is the +// indexed commit" — it means "HEAD has no commits lastCommit lacks", which is +// also true when HEAD is an *ancestor* of lastCommit (the working tree checked +// out an older commit than the one indexed, or switched to a line of history +// behind it). That read a rollback as `current` until this hint existed. +const REGRESSED_HINT = + '⚠️ Index is not at HEAD — the indexed commit is not reachable from the checked-out commit (the working tree may have checked out an older commit, or a different line of history). Run analyze tool to update.'; + const unknown = (): StalenessInfo => ({ isStale: false, commitsBehind: 0, status: 'unknown' }); -const fromCount = (commitsBehind: number): StalenessInfo => - commitsBehind > 0 - ? { isStale: true, commitsBehind, hint: behindHint(commitsBehind), status: 'behind' } - : { isStale: false, commitsBehind: 0, status: 'current' }; +// Called only once a positive HEAD-only count is in hand. +const behind = (commitsBehind: number): StalenessInfo => ({ + isStale: true, + commitsBehind, + hint: behindHint(commitsBehind), + status: 'behind', +}); /** * `rev-list` could not answer. Asking for HEAD alone needs no history walk and @@ -53,6 +64,26 @@ const fromHead = (head: string | null, lastCommit: string): StalenessInfo => { return { isStale: false, commitsBehind: 0, hint: DIVERGED_HINT, status: 'diverged' }; }; +/** + * `rev-list --left-right --count lastCommit...HEAD` measures both sides in + * one process, so a later HEAD change cannot mix two snapshots. The left + * count identifies a rollback even when the HEAD-only (right) count is 0. + * Positive right counts keep the existing `behind` behavior, including when + * both sides have commits (divergent branches or a re-shallowed clone). + */ +const fromCounts = (output: string): StalenessInfo => { + const counts = /^(\d+)\s+(\d+)$/.exec(output.trim()); + if (!counts) return unknown(); + const indexedOnly = Number(counts[1]); + const headOnly = Number(counts[2]); + if (!Number.isSafeInteger(indexedOnly) || !Number.isSafeInteger(headOnly)) return unknown(); + if (headOnly > 0) return behind(headOnly); + if (indexedOnly > 0) { + return { isStale: true, commitsBehind: 0, hint: REGRESSED_HINT, status: 'diverged' }; + } + return { isStale: false, commitsBehind: 0, status: 'current' }; +}; + const readHeadSync = (repoPath: string): string | null => { try { return ( @@ -61,6 +92,7 @@ const readHeadSync = (repoPath: string): string | null => { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true, + timeout: STALENESS_TIMEOUT_MS, }).trim() || null ); } catch { @@ -89,14 +121,18 @@ export function checkStaleness(repoPath: string, lastCommit: string): StalenessI // No recorded commit is not "at HEAD": there is nothing to measure against. if (!lastCommit) return unknown(); try { - const result = execFileSync('git', ['rev-list', '--count', `${lastCommit}..HEAD`], { - cwd: repoPath, - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], - windowsHide: true, - }).trim(); + const result = execFileSync( + 'git', + ['rev-list', '--left-right', '--count', `${lastCommit}...HEAD`], + { + cwd: repoPath, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }, + ); - return fromCount(parseInt(result, 10) || 0); + return fromCounts(result); } catch { return fromHead(readHeadSync(repoPath), lastCommit); } @@ -115,21 +151,25 @@ export async function checkStalenessAsync( try { // Note: promisified execFile captures stdout/stderr by default (no stdio option needed, // unlike the sync variant which requires explicit stdio: ['pipe','pipe','pipe']). - const { stdout } = await execFileAsync('git', ['rev-list', '--count', `${lastCommit}..HEAD`], { - cwd: repoPath, - encoding: 'utf-8', - windowsHide: true, - // The catch below fails closed on every git ERROR, but a hang is not an - // error — it is silence, and without a bound this await never settles. - // A working tree on a disconnected network mount or behind a stuck lock - // does exactly that, and `/api/repos` fans this out once per registered - // repo, so one unreachable mount could hold the whole listing open - // (#3232 review). The timeout kills the child and rejects, and the catch - // below reports it as `unknown` — still the fail-closed `isStale: false`. - timeout: STALENESS_TIMEOUT_MS, - }); + const { stdout } = await execFileAsync( + 'git', + ['rev-list', '--left-right', '--count', `${lastCommit}...HEAD`], + { + cwd: repoPath, + encoding: 'utf-8', + windowsHide: true, + // The catch below fails closed on every git ERROR, but a hang is not an + // error — it is silence, and without a bound this await never settles. + // A working tree on a disconnected network mount or behind a stuck lock + // does exactly that, and `/api/repos` fans this out once per registered + // repo, so one unreachable mount could hold the whole listing open + // (#3232 review). The timeout kills the child and rejects, and the catch + // below reports it as `unknown` — still the fail-closed `isStale: false`. + timeout: STALENESS_TIMEOUT_MS, + }, + ); - return fromCount(parseInt(stdout.trim(), 10) || 0); + return fromCounts(stdout); } catch (err) { // A rev-list that timed out means the working tree is not answering. Asking // it again for HEAD would only double the bound #3232 put on a hung mount. diff --git a/gitnexus/src/core/ingestion/community-processor.ts b/gitnexus/src/core/ingestion/community-processor.ts index 77b66d73b..233326e03 100644 --- a/gitnexus/src/core/ingestion/community-processor.ts +++ b/gitnexus/src/core/ingestion/community-processor.ts @@ -176,7 +176,10 @@ export interface CommunityMembership { export interface CommunityDetectionResult { communities: CommunityNode[]; + /** Assignments to retained communities, safe to emit as MEMBER_OF edges. */ memberships: CommunityMembership[]; + /** All Leiden assignments, including filtered singletons. Optional for legacy producers. */ + rawMemberships?: CommunityMembership[]; stats: { totalCommunities: number; modularity: number; @@ -235,6 +238,7 @@ export const processCommunities = async ( return { communities: [], memberships: [], + rawMemberships: [], stats: { totalCommunities: 0, modularity: 0, @@ -267,13 +271,16 @@ export const processCommunities = async ( onProgress?.('Creating membership edges...', 80); - // Step 4: Create membership mappings + // Step 4: Preserve all assignments for skill generation, but only emit + // memberships for retained communities with a corresponding graph node. + const retainedCommunityIds = new Set(communityNodes.map((community) => community.id)); const memberships: CommunityMembership[] = []; + const rawMemberships: CommunityMembership[] = []; Object.entries(details.communities).forEach(([nodeId, communityNum]) => { - memberships.push({ - nodeId, - communityId: `comm_${communityNum}`, - }); + const communityId = `comm_${communityNum}`; + const membership = { nodeId, communityId }; + rawMemberships.push(membership); + if (retainedCommunityIds.has(communityId)) memberships.push(membership); }); onProgress?.('Community detection complete!', 100); @@ -281,6 +288,7 @@ export const processCommunities = async ( return { communities: communityNodes, memberships, + rawMemberships, stats: { totalCommunities: details.count, modularity: details.modularity, diff --git a/gitnexus/src/core/ingestion/language-provider.ts b/gitnexus/src/core/ingestion/language-provider.ts index 90122aacc..718d8693d 100644 --- a/gitnexus/src/core/ingestion/language-provider.ts +++ b/gitnexus/src/core/ingestion/language-provider.ts @@ -46,7 +46,7 @@ import type { RepoConstants, } from './route-extractors/constant-resolver.js'; import type Parser from 'tree-sitter'; -import type { ExtractedDecoratorRoute } from './workers/parse-worker.js'; +import type { ExtractedDecoratorRoute, ExtractedToolDef } from './workers/parse-worker.js'; import type { SemanticModel } from './model/semantic-model.js'; /** What a provider's {@link LanguageProviderConfig.resolveRouteHandler} can see. */ @@ -546,6 +546,15 @@ interface LanguageProviderConfig { */ readonly extractTextRoutes?: (filePath: string, content: string) => ExtractedRoute[]; + /** Extract tool registrations after captures, using only emitted callable identities. + * The map keys are declaration-name AST node IDs, local to this parsed tree. */ + readonly extractToolDefinitions?: ( + tree: Parser.Tree, + filePath: string, + lineOffset: number, + callableBindings: ReadonlyMap, + ) => ExtractedToolDef[]; + /** * Extract routes that a parsed file declares in its own AST. * diff --git a/gitnexus/src/core/ingestion/languages/swift/callable-visibility.ts b/gitnexus/src/core/ingestion/languages/swift/callable-visibility.ts new file mode 100644 index 000000000..f89b2b639 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/swift/callable-visibility.ts @@ -0,0 +1,78 @@ +import type { ParsedFile, ScopeId, SymbolDefinition, TypeRef } from 'gitnexus-shared'; +import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import { findClassBindingInScope } from '../../scope-resolution/scope/walkers.js'; + +export function swiftIsCallableVisibleFromCaller(ctx: { + readonly candidate: SymbolDefinition; + readonly callerParsed?: ParsedFile; + readonly callArity?: number; + readonly callerScope?: ScopeId; + readonly scopes?: ScopeResolutionIndexes; +}): boolean { + const indexes = ctx.scopes; + if (ctx.callerScope === undefined || indexes === undefined || ctx.callArity !== 0) return true; + + const name = ctx.candidate.qualifiedName?.split('.').at(-1); + if (name === undefined) return true; + + let scopeId: ScopeId | null = ctx.callerScope; + let selfType: TypeRef | undefined; + while (scopeId !== null) { + const scope = indexes.scopeTree.getScope(scopeId); + if (scope === undefined) break; + // A local function selected inside the caller wins before member lookup. + if ( + scope.kind !== 'Class' && + scope.ownedDefs.some((def) => def.nodeId === ctx.candidate.nodeId) + ) + return true; + selfType ??= scope.typeBindings.get('self'); + if (scope.kind === 'Class') { + const classDef = + scope.ownedDefs.find((def) => def.type === 'Class') ?? + (selfType === undefined + ? undefined + : findClassBindingInScope(ctx.callerScope, selfType.rawName, indexes, undefined, { + uniqueQualifiedNameFallback: false, + })); + const propertyOnOwner = (ownerId: string, ownerName: string): boolean => + indexes.qualifiedNames.get(`${ownerName}.${name}`).some((defId) => { + const def = indexes.defs.get(defId); + return def?.type === 'Property' && def.ownerId === ownerId; + }); + const ownProperty = + scope.ownedDefs.some( + (def) => def.type === 'Property' && def.qualifiedName?.split('.').at(-1) === name, + ) || + (classDef !== undefined && + classDef.qualifiedName !== undefined && + propertyOnOwner(classDef.nodeId, classDef.qualifiedName)); + const inheritedProperty = + classDef !== undefined && + indexes.methodDispatch.mroFor(classDef.nodeId).some((ownerId) => { + const ownerName = indexes.defs.get(ownerId)?.qualifiedName; + return ownerName !== undefined && propertyOnOwner(ownerId, ownerName); + }); + if (!ownProperty && !inheritedProperty) return true; + + // Swift's nested function is owned by its own Function scope. A + // same-file sibling must not count as a nearer lexical binding. + const declarationScope = ctx.callerParsed?.scopes.find((candidateScope) => + candidateScope.ownedDefs.some((def) => def.nodeId === ctx.candidate.nodeId), + ); + if ( + declarationScope !== undefined && + declarationScope.kind === 'Function' && + (declarationScope.id === ctx.callerScope || + indexes.scopeTree.getAncestors(declarationScope.id).includes(ctx.callerScope)) + ) + return true; + + // Scope defs do not carry Swift access modifiers. A method positively + // selected on this type must not be vetoed by an uncertain ancestor. + return classDef !== undefined && ctx.candidate.ownerId === classDef.nodeId; + } + scopeId = scope.parent; + } + return true; +} diff --git a/gitnexus/src/core/ingestion/languages/swift/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/swift/scope-resolver.ts index 0822b8cf8..7fe075abf 100644 --- a/gitnexus/src/core/ingestion/languages/swift/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/swift/scope-resolver.ts @@ -70,6 +70,7 @@ import { import { stripSwiftTypePreservingDecoration } from './interpret.js'; import { groupSwiftFilesByModule } from './target-grouping.js'; import { swiftIsGlobalNameFallbackPlausible } from './name-fallback-visibility.js'; +import { swiftIsCallableVisibleFromCaller } from './callable-visibility.js'; const ZERO_RANGE = { startLine: 0, startCol: 0, endLine: 0, endCol: 0 } as const; @@ -155,6 +156,7 @@ const swiftScopeResolver: ScopeResolver = { // no-`new` constructor + cross-file free-call shape). allowGlobalFreeCallFallback: true, isGlobalNameFallbackPlausible: swiftIsGlobalNameFallbackPlausible, + isCallableVisibleFromCaller: swiftIsCallableVisibleFromCaller, // Swift's call graph models `Type(...)` as a reference to the type // itself, not its `init` — both the legacy DAG and this test suite link diff --git a/gitnexus/src/core/ingestion/languages/typescript.ts b/gitnexus/src/core/ingestion/languages/typescript.ts index 5251910cb..c3509ea5e 100644 --- a/gitnexus/src/core/ingestion/languages/typescript.ts +++ b/gitnexus/src/core/ingestion/languages/typescript.ts @@ -130,6 +130,7 @@ import { extractDataRouteTableRoutes } from '../route-extractors/data-route-tabl import { extractNestRoutes } from '../route-extractors/nest.js'; import { extractTrpcRoutes, shouldScanForTrpcRoutes } from '../route-extractors/trpc.js'; import { extractConvexEndpointProperties } from './typescript/convex-endpoint-metadata.js'; +import { extractToolDefinitions } from './typescript/tool-definitions.js'; const extractJsTsRoutes = (...args: Parameters) => [ ...extractDispatchGuardRoutes(...args), @@ -494,6 +495,7 @@ export const typescriptProvider = defineLanguage({ // Content-based (not AST): tRPC procedure routers are scanned from source text. // Path-gate lives here (language provider), not in the shared parse worker. extractTextRoutes: extractJsTsTextRoutes, + extractToolDefinitions, }); export const javascriptProvider = defineLanguage({ @@ -577,4 +579,5 @@ export const javascriptProvider = defineLanguage({ extractDecoratorRoutes: extractJsTsRoutes, // Content-based (not AST): tRPC procedure routers are scanned from source text. extractTextRoutes: extractJsTsTextRoutes, + extractToolDefinitions, }); diff --git a/gitnexus/src/core/ingestion/languages/typescript/tool-definitions.ts b/gitnexus/src/core/ingestion/languages/typescript/tool-definitions.ts new file mode 100644 index 000000000..307998c56 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/typescript/tool-definitions.ts @@ -0,0 +1,365 @@ +import type Parser from 'tree-sitter'; +import type { SyntaxNode } from 'tree-sitter'; +import type { ExtractedToolDef } from '../../workers/parse-worker.js'; +import { plainString, propertyName } from '../../route-extractors/data-route-table.js'; + +interface Scope { + parent?: Scope; + functionScope: boolean; + bindings: Map; +} + +interface Binding { + name: SyntaxNode; + scope: Scope; + kind: 'unknown' | 'sdk' | 'sdk-namespace' | 'variable' | 'parameter' | 'function'; + value?: SyntaxNode; + type?: SyntaxNode; + typeOnly?: boolean; + immutable?: boolean; + invalid?: boolean; +} + +const FUNCTIONS = new Set([ + 'function_declaration', + 'generator_function_declaration', + 'function_expression', + 'generator_function', + 'arrow_function', + 'method_definition', +]); +const BLOCKS = new Set([ + 'statement_block', + 'for_statement', + 'for_in_statement', + 'switch_body', + 'catch_clause', + 'class_body', +]); +const SDK_MODULES = new Set([ + '@modelcontextprotocol/sdk/server/mcp.js', + '@modelcontextprotocol/sdk/server/mcp', +]); + +function lookup(scope: Scope, name: string): Binding | undefined { + for (let current: Scope | undefined = scope; current; current = current.parent) { + const binding = current.bindings.get(name); + if (binding) return binding; + } +} + +/** Only binding/assignment patterns: never descend into keys, types or defaults. + * Member assignment targets are reported separately from binding names. */ +function patternNames(pattern: SyntaxNode, onMember?: (member: SyntaxNode) => void): SyntaxNode[] { + const names: SyntaxNode[] = []; + const pending = [pattern]; + while (pending.length) { + const node = pending.pop()!; + if (node.type === 'identifier' || node.type === 'shorthand_property_identifier_pattern') { + names.push(node); + } else if (node.type === 'member_expression' || node.type === 'subscript_expression') { + onMember?.(node); + } else if (node.type === 'pair_pattern') { + const value = node.childForFieldName('value'); + if (value) pending.push(value); + } else if (node.type === 'assignment_pattern' || node.type === 'object_assignment_pattern') { + const left = node.childForFieldName('left'); + if (left) pending.push(left); + } else if ( + node.type === 'array_pattern' || + node.type === 'object_pattern' || + node.type === 'rest_pattern' + ) { + pending.push(...node.namedChildren); + } + } + return names; +} + +function declare(scope: Scope, name: SyntaxNode, details: Partial = {}): void { + const previous = scope.bindings.get(name.text); + if (previous) { + previous.invalid = true; + } else { + scope.bindings.set(name.text, { name, scope, kind: 'unknown', ...details }); + } +} + +function variableScope(scope: Scope): Scope { + while (!scope.functionScope && scope.parent) scope = scope.parent; + return scope; +} + +function collectBindings(root: SyntaxNode) { + const moduleScope: Scope = { functionScope: true, bindings: new Map() }; + const scopes = new Map(); + const calls: SyntaxNode[] = []; + const writes: SyntaxNode[] = []; + const pending = [{ node: root, scope: moduleScope }]; + while (pending.length) { + const entry = pending.pop()!; + const node = entry.node; + let scope = entry.scope; + const isFunction = FUNCTIONS.has(node.type); + const name = node.childForFieldName('name'); + if (node.type === 'function_declaration' || node.type === 'generator_function_declaration') { + if (name) declare(scope, name, { kind: 'function', value: node, immutable: true }); + } else if ( + [ + 'class_declaration', + 'interface_declaration', + 'type_alias_declaration', + 'enum_declaration', + ].includes(node.type) + ) { + if (name) declare(scope, name); + } + if ( + isFunction || + BLOCKS.has(node.type) || + node.type === 'class' || + node.type === 'class_declaration' + ) { + scope = { parent: scope, functionScope: isFunction, bindings: new Map() }; + } + scopes.set(node.id, scope); + + if (node.type === 'class' && name) declare(scope, name); + + if (isFunction) { + if (name && (node.type === 'function_expression' || node.type === 'generator_function')) { + declare(scope, name, { kind: 'function', value: node, immutable: true }); + } + const parameters = node.childForFieldName('parameters'); + const single = node.childForFieldName('parameter'); + for (const parameter of parameters?.namedChildren ?? (single ? [single] : [])) { + const pattern = parameter.childForFieldName('pattern') ?? parameter; + const type = parameter.childForFieldName('type')?.namedChildren[0]; + for (const bindingName of patternNames(pattern)) { + declare(scope, bindingName, { + kind: 'parameter', + ...(pattern.type === 'identifier' && type ? { type } : {}), + }); + } + } + } else if (node.type === 'import_statement') { + const source = node.childForFieldName('source'); + const sdk = source !== null && SDK_MODULES.has(plainString(source) ?? ''); + const typeOnly = node.children.some((child) => child.type === 'type'); + const clause = node.namedChildren.find((child) => child.type === 'import_clause'); + for (const child of clause?.namedChildren ?? []) { + if (child.type === 'identifier') declare(scope, child); + else if (child.type === 'namespace_import') { + const local = child.namedChildren[0]; + if (local) declare(scope, local, { kind: sdk ? 'sdk-namespace' : 'unknown', typeOnly }); + } else if (child.type === 'named_imports') { + for (const specifier of child.namedChildren) { + const imported = specifier.childForFieldName('name'); + const local = specifier.childForFieldName('alias') ?? imported; + if (local) + declare(scope, local, { + kind: sdk && imported?.text === 'McpServer' ? 'sdk' : 'unknown', + typeOnly: typeOnly || specifier.children.some((part) => part.type === 'type'), + }); + } + } + } + } else if (node.type === 'variable_declarator') { + if (name) { + const target = node.parent?.type === 'variable_declaration' ? variableScope(scope) : scope; + for (const bindingName of patternNames(name)) { + declare(target, bindingName, { + kind: 'variable', + immutable: node.parent?.childForFieldName('kind')?.type === 'const', + ...(name.type === 'identifier' + ? { value: node.childForFieldName('value') ?? undefined } + : {}), + }); + } + } + } else if (node.type === 'catch_clause') { + const parameter = node.childForFieldName('parameter'); + if (parameter) for (const bindingName of patternNames(parameter)) declare(scope, bindingName); + } else if (node.type === 'for_in_statement') { + const left = node.childForFieldName('left'); + const kind = node.childForFieldName('kind'); + if (left && kind) { + const target = kind.type === 'var' ? variableScope(scope) : scope; + for (const bindingName of patternNames(left)) declare(target, bindingName); + } else if (left) writes.push(left); + } else if (node.type === 'type_parameter') { + if (name) declare(scope, name); + } + if (node.type === 'call_expression') calls.push(node); + if (node.type === 'assignment_expression' || node.type === 'augmented_assignment_expression') { + const left = node.childForFieldName('left'); + if (left) writes.push(left); + } else if ( + node.type === 'update_expression' || + (node.type === 'unary_expression' && node.children.some((child) => child.type === 'delete')) + ) { + const argument = node.childForFieldName('argument'); + if (argument) writes.push(argument); + } + for (let index = node.namedChildCount - 1; index >= 0; index--) { + pending.push({ node: node.namedChild(index)!, scope }); + } + } + // Resolve writes after declarations so later declarations also shadow outer names. + while (writes.length) { + let target = writes.pop()!; + const scope = scopes.get(target.id)!; + const members: Array = []; + while (target.type === 'member_expression' || target.type === 'subscript_expression') { + const object = target.childForFieldName('object'); + if (!object) break; + const property = target.childForFieldName('property'); + const index = target.childForFieldName('index'); + members.push(property ? propertyName(property) : index ? plainString(index) : null); + target = object; + } + // Nested member targets must pass the same guard as direct property writes. + for (const name of patternNames(target, (member) => writes.push(member))) { + const binding = lookup(scope, name.text); + // Lifecycle callbacks and other known properties do not replace the receiver + // or its registration methods. Unknown keys and constructor mutations remain unsafe. + if ( + binding?.kind !== 'sdk' && + binding?.kind !== 'sdk-namespace' && + members.length > 0 && + members.every((member) => member !== null) && + !['tool', 'registerTool', '__proto__'].includes(members[members.length - 1]!) + ) + continue; + if (binding) binding.invalid = true; + } + } + return { scopes, calls }; +} + +function sdkBinding(node: SyntaxNode, scope: Scope, forType = false): boolean { + let kind: Binding['kind'] = 'sdk'; + if (node.type === (forType ? 'nested_type_identifier' : 'member_expression')) { + const namespace = node.childForFieldName(forType ? 'module' : 'object'); + const member = node.childForFieldName(forType ? 'name' : 'property'); + if (namespace?.type !== 'identifier' || member?.text !== 'McpServer') return false; + node = namespace; + kind = 'sdk-namespace'; + } + if (node.type !== 'identifier' && node.type !== 'type_identifier') return false; + const binding = lookup(scope, node.text); + return binding?.kind === kind && !binding.invalid && (forType || !binding.typeOnly); +} + +function sdkReceiver(node: SyntaxNode, scope: Scope, scopes: ReadonlyMap): boolean { + if (node.type !== 'identifier') return false; + const binding = lookup(scope, node.text); + if (!binding || binding.invalid) return false; + if (binding.kind === 'parameter' && binding.type) { + return sdkBinding(binding.type, binding.scope, true); + } + const value = binding.value; + if (binding.kind !== 'variable' || value?.type !== 'new_expression') return false; + if (value.endIndex > node.startIndex && variableScope(binding.scope) === variableScope(scope)) + return false; + const constructor = value.childForFieldName('constructor'); + return constructor !== null && sdkBinding(constructor, scopes.get(value.id)!); +} + +/** An unknown later property can replace description; a later explicit property restores proof. */ +function descriptionFromConfig(config: SyntaxNode): string { + let description = ''; + if (config.type !== 'object') return description; + for (const child of config.namedChildren) { + if (child.type === 'comment') continue; + const key = child.childForFieldName('key') ?? child.childForFieldName('name'); + const name = + child.type === 'shorthand_property_identifier' ? child.text : key && propertyName(key); + if (child.type === 'pair' && name === 'description') { + const value = child.childForFieldName('value'); + description = value ? (plainString(value) ?? '') : ''; + } else if (!name || name === 'description') { + description = ''; + } + } + return description; +} + +function handlerNodeId( + node: SyntaxNode, + scope: Scope, + callableBindings: ReadonlyMap | undefined, +): string | undefined { + if (node.type !== 'identifier') return undefined; + const binding = lookup(scope, node.text); + if (!binding || binding.invalid) return undefined; + if (binding.kind === 'variable') { + const value = binding.value; + if ( + !binding.immutable || + !value || + (value.type !== 'arrow_function' && value.type !== 'function_expression') + ) + return undefined; + if (value.endIndex > node.startIndex && variableScope(binding.scope) === variableScope(scope)) + return undefined; + } else if (binding.kind !== 'function') return undefined; + return callableBindings?.get(binding.name.id); +} + +/** Direct SDK registrations only; no wrapper, alias-chain or runtime-value inference. */ +export function extractToolDefinitions( + tree: Parser.Tree, + filePath: string, + lineOffset = 0, + callableBindings?: ReadonlyMap, +): ExtractedToolDef[] { + // Ordinary files need no lexical walk; every supported receiver originates here. + const importsSdk = tree.rootNode.namedChildren.some((node) => { + if (node.type !== 'import_statement') return false; + const source = node.childForFieldName('source'); + return source !== null && SDK_MODULES.has(plainString(source) ?? ''); + }); + if (!importsSdk) return []; + + const { scopes, calls } = collectBindings(tree.rootNode); + const definitions: ExtractedToolDef[] = []; + for (const call of calls) { + const callee = call.childForFieldName('function'); + if (call.hasError || callee?.type !== 'member_expression') continue; + const receiver = callee.childForFieldName('object'); + const method = callee.childForFieldName('property'); + if ( + !receiver || + method?.type !== 'property_identifier' || + (method.text !== 'registerTool' && method.text !== 'tool') || + !sdkReceiver(receiver, scopes.get(call.id)!, scopes) + ) + continue; + const args = + call + .childForFieldName('arguments') + ?.namedChildren.filter((child) => child.type !== 'comment') ?? []; + if (args.some((arg) => arg.type === 'spread_element')) continue; + if (method.text === 'registerTool' ? args.length !== 3 : args.length < 2 || args.length > 5) + continue; + const toolName = plainString(args[0]); + if (toolName === null) continue; + const description = + method.text === 'registerTool' + ? descriptionFromConfig(args[1]) + : args.length > 2 + ? (plainString(args[1]) ?? '') + : ''; + const handler = handlerNodeId(args[args.length - 1], scopes.get(call.id)!, callableBindings); + definitions.push({ + filePath, + toolName, + description, + lineNumber: call.startPosition.row + 1 + lineOffset, + ...(handler !== undefined ? { handlerNodeId: handler } : {}), + allowFileFallback: false, + }); + } + return definitions; +} diff --git a/gitnexus/src/core/ingestion/pipeline-phases/processes.ts b/gitnexus/src/core/ingestion/pipeline-phases/processes.ts index 6be8839c3..75f8a0b48 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/processes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/processes.ts @@ -323,6 +323,7 @@ export const processesPhase: PipelinePhase = { const toolsByHandlerId = new Map(); const toolsWithoutHandlerByFile = new Map(); for (const td of toolDefs) { + if (!td.handlerNodeId && td.allowFileFallback === false) continue; const key = td.handlerNodeId ?? td.filePath; const targetMap = td.handlerNodeId ? toolsByHandlerId : toolsWithoutHandlerByFile; let list = targetMap.get(key); diff --git a/gitnexus/src/core/ingestion/pipeline-phases/tools.ts b/gitnexus/src/core/ingestion/pipeline-phases/tools.ts index 32a0ae71f..296d0a42a 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/tools.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/tools.ts @@ -22,6 +22,7 @@ export interface ToolDef { filePath: string; description: string; handlerNodeId?: string; + allowFileFallback?: false; } export interface ToolsOutput { @@ -51,6 +52,7 @@ export const toolsPhase: PipelinePhase = { filePath: td.filePath, description: td.description, ...(handlerNodeId !== undefined ? { handlerNodeId } : {}), + ...(td.allowFileFallback === false ? { allowFileFallback: false as const } : {}), }); } diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index 46a80060a..22a7a2d41 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -1235,6 +1235,9 @@ export interface ScopeResolver { readonly isCallableVisibleFromCaller?: (ctx: { readonly callerParsed: ParsedFile; readonly candidate: SymbolDefinition; + /** Arity of the actual call, when known. A visibility veto must not + * infer applicability from a name match alone. */ + readonly callArity?: number; /** Caller's enclosing scope id. Languages that gate visibility on * caller scope (e.g. C++ two-phase template lookup) consult it; * others ignore. Optional so existing implementations stay valid. */ diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts index 6693a2bc6..af556d0d3 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts @@ -84,12 +84,7 @@ export function emitFreeCallFallback( * appended to its reason. See `ScopeResolver.markConstructionSites`. */ readonly markConstructionSites?: boolean; readonly isFileLocalDef?: (def: SymbolDefinition) => boolean; - readonly isCallableVisibleFromCaller?: (ctx: { - readonly callerParsed: ParsedFile; - readonly candidate: SymbolDefinition; - readonly callerScope?: ScopeId; - readonly scopes?: ScopeResolutionIndexes; - }) => boolean; + readonly isCallableVisibleFromCaller?: ScopeResolver['isCallableVisibleFromCaller']; readonly resolveAdlCandidates?: ( site: { readonly name: string; @@ -620,6 +615,7 @@ export function emitFreeCallFallback( options.isCallableVisibleFromCaller!({ callerParsed: parsed, candidate, + callArity: site.arity, callerScope: site.inScope, scopes, }) @@ -698,6 +694,7 @@ export function emitFreeCallFallback( !options.isCallableVisibleFromCaller({ callerParsed: parsed, candidate: fnDef, + callArity: site.arity, callerScope: site.inScope, scopes, }) diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index e6333c65d..18b65eba1 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -439,6 +439,8 @@ export interface ExtractedToolDef { description: string; lineNumber: number; handlerNodeId?: string; + /** Unresolved registrations must not inherit unrelated same-file flows. */ + allowFileFallback?: false; } export interface ExtractedORMQuery { @@ -1685,6 +1687,7 @@ const processFileGroup = ( // node id → graph node id for classes THIS file's capture loop materialized. // Keyed by in-memory AST identity (never persisted); filled below. const classOwnersByNodeId = new Map(); + const callableBindings = new Map(); // #2687: ONE pass over `matches` yields both suppression sets — the // definition-name claims by rank (callable > Property > value), so the dedup @@ -3123,6 +3126,11 @@ const processFileGroup = ( }), }); + // Keep actual emitted identities; providers must not reconstruct graph IDs. + if (nameNode && (nodeLabel === 'Function' || nodeLabel === 'Method')) { + callableBindings.set(nameNode.id, nodeId); + } + // enclosingClassId already computed above (before nodeId generation) const ownerId = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId; @@ -3225,6 +3233,23 @@ const processFileGroup = ( } } + if (provider.extractToolDefinitions) { + // Distinct lexical declarations can share a graph ID (for example, sibling + // block-scoped functions). Such IDs cannot prove which handler owns a tool. + const seenCallableIds = new Set(); + const ambiguousCallableIds = new Set(); + for (const nodeId of callableBindings.values()) { + if (seenCallableIds.has(nodeId)) ambiguousCallableIds.add(nodeId); + seenCallableIds.add(nodeId); + } + for (const [bindingId, nodeId] of callableBindings) { + if (ambiguousCallableIds.has(nodeId)) callableBindings.delete(bindingId); + } + result.toolDefs.push( + ...provider.extractToolDefinitions(tree, file.path, lineOffset, callableBindings), + ); + } + // Extract framework routes via provider detection (e.g., Laravel routes.php) if (provider.isRouteFile?.(file.path)) { const extractedRoutes = extractLaravelRoutes(tree, file.path); diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 59f903261..92adff18a 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -4044,6 +4044,7 @@ export const buildFtsQueryCypher = ( * @param query - Search query string * @param limit - Maximum results * @param conjunctive - If true, all terms must match (AND); if false, any term matches (OR) + * @param missingIndex - Preserve the empty-result default, or propagate missing indexes for diagnostics * @returns Array of { node properties, score } */ export const queryFTS = async ( @@ -4052,6 +4053,7 @@ export const queryFTS = async ( query: string, limit: number = 20, conjunctive: boolean = false, + missingIndex: 'empty' | 'throw' = 'empty', ): Promise< Array<{ nodeId: string; name: string; filePath: string; score: number; [key: string]: any }> > => { @@ -4082,7 +4084,7 @@ export const queryFTS = async ( // NEW-6 — this used to be a bare `.includes('does not exist')` check // that could not tell the two apart). const message = e instanceof Error ? e.message : String(e); - if (classifyFtsQueryError(message) === 'missing-index') { + if (missingIndex === 'empty' && classifyFtsQueryError(message) === 'missing-index') { return []; } throw e; diff --git a/gitnexus/src/core/search/bm25-index.ts b/gitnexus/src/core/search/bm25-index.ts index 8aaa279ef..c105a62a3 100644 --- a/gitnexus/src/core/search/bm25-index.ts +++ b/gitnexus/src/core/search/bm25-index.ts @@ -47,6 +47,8 @@ export interface FTSSearchResponse { * which only does so when every table failed). */ nonBenignErrors?: string[]; + /** Configured table.index names that could not be queried because their index is missing. */ + missingIndexes?: string[]; } /** @@ -142,6 +144,7 @@ export const searchFTSFromLbug = async ( const resultsByIndex: any[][] = []; let queriesSucceeded = 0; const nonBenignErrors: string[] = []; + const missingIndexes: string[] = []; const ftsExtension = getExtensionCapabilities().find((c) => c.name === 'fts'); if (ftsExtension && !ftsExtension.loaded) { @@ -171,24 +174,28 @@ export const searchFTSFromLbug = async ( if (outcome.rows) { queriesSucceeded++; resultsByIndex.push(outcome.rows); - } else if (!outcome.benign) { + } else if (outcome.benign) { + missingIndexes.push(`${table}.${indexName}`); + } else { nonBenignErrors.push(redactPaths(outcome.message ?? 'Unknown FTS query error')); } } } else { // Use core lbug adapter (CLI / pipeline context) — also sequential for safety. - // tri-review Residual-1: `queryFTS` itself only swallows a genuinely-missing - // index (via the SAME classifyFtsQueryError this module re-exports); a - // missing-table or real query error rethrows here — track it the same way - // the MCP pool path does instead of a bare `catch {}` that dropped it. + // Opt into missing-index propagation so absent indexes cannot masquerade + // as successful zero-match queries. Keep core/pool classification identical. for (const { table, indexName } of FTS_INDEXES) { try { - const result = await queryFTS(table, indexName, searchQuery, limit, false); + const result = await queryFTS(table, indexName, searchQuery, limit, false, 'throw'); queriesSucceeded++; resultsByIndex.push(result); } catch (e) { const message = e instanceof Error ? e.message : String(e); - nonBenignErrors.push(redactPaths(message)); + if (classifyFtsQueryError(message) === 'missing-index') { + missingIndexes.push(`${table}.${indexName}`); + } else { + nonBenignErrors.push(redactPaths(message)); + } } } } @@ -234,5 +241,6 @@ export const searchFTSFromLbug = async ( })), ftsAvailable, ...(nonBenignErrors.length > 0 && { nonBenignErrors }), + ...(missingIndexes.length > 0 && { missingIndexes }), }; }; diff --git a/gitnexus/src/core/search/fts-indexes.ts b/gitnexus/src/core/search/fts-indexes.ts index 2070fadf3..5f8b59f13 100644 --- a/gitnexus/src/core/search/fts-indexes.ts +++ b/gitnexus/src/core/search/fts-indexes.ts @@ -110,19 +110,21 @@ export const ftsDegradedWarning = ( }; /** - * Warning for when the FTS extension is loaded and indexes exist, but every - * configured table's query failed for a real, non-benign reason (timeout, - * connection reset, native fault) — as opposed to `ftsDegradedWarning`'s - * missing-index case. `--repair-fts` will not fix a query/connection error, - * so this deliberately does NOT suggest it: reusing the missing-index - * message here would reproduce, for this cause, the exact misleading - * "run --repair-fts" guidance #2767 itself was about (tri-review NEW-1). + * Warning when no FTS query succeeded and at least one failed for a real, + * non-benign reason (timeout, connection reset, native fault). `--repair-fts` + * will not fix those errors. If indexes are also missing, the caller composes + * their repair guidance separately; do not deny that additional failure cause. */ -export const ftsQueryFailedWarning = (context: FtsWarningContext): string => +export const ftsQueryFailedWarning = ( + context: FtsWarningContext, + hasMissingIndexes = false, +): string => 'FTS keyword search failed — every configured index query returned an error' + (context.lastErrorRedacted ? ` (${context.lastErrorRedacted})` : '') + - '; results do not include keyword matches. This is not a missing-index ' + - 'condition — see server logs for details.' + + '; results do not include keyword matches. ' + + (hasMissingIndexes + ? 'See server logs for query error details.' + : 'This is not a missing-index condition — see server logs for details.') + ` (resolved: ${formatResolvedSuffix(context)})`; // Stemmers shipped by the LadybugDB FTS extension. Mirrors the lowercase token diff --git a/gitnexus/src/core/staleness-status.ts b/gitnexus/src/core/staleness-status.ts index 97a8cce94..40f9d3b51 100644 --- a/gitnexus/src/core/staleness-status.ts +++ b/gitnexus/src/core/staleness-status.ts @@ -18,21 +18,35 @@ * provably stale index indistinguishable from a fresh one. `status` is the * additive channel that separates them for a caller that wants to act on it: * - * - `current` — the index is at HEAD: `rev-list` answered 0, or it could not - * answer but HEAD alone resolved to the indexed commit. - * - `behind` — `rev-list` answered N > 0; `commitsBehind` is N. - * - `diverged` — `rev-list` could not answer, but HEAD resolved and is not the - * indexed commit. The index is provably not at HEAD; only the count is - * unknown. A branch-pinned `serve` clone reaches this once git prunes the - * commit a failed re-index left behind — the pinned update is a - * `fetch --depth 1`, which orphans it — and a rewritten history reaches it - * directly. It is the rule the Claude hook already applies: - * HEAD !== lastCommit. - * - `unknown` — HEAD could not be resolved at all: not a git repository, git - * timed out, or no commit was recorded. + * The successful probe is `rev-list --left-right --count lastCommit...HEAD`: + * the left count is indexed-only commits, and the right is HEAD-only commits. + * Both counts come from one HEAD snapshot, without a follow-up process. * - * `isStale` and `commitsBehind` keep their historical values in every case, so - * no existing consumer changes behaviour unless it reads `status`. + * - `current` — both counts are 0, or `rev-list` could not answer but a + * fallback `rev-parse HEAD` resolved to the indexed commit. + * - `behind` — the right count is N > 0; `commitsBehind` is N, including + * when the left count is positive too (divergent or shallow history). + * - `diverged` — the index is provably not at HEAD, reached two different ways: + * - The left count is positive and the right is 0: HEAD is an ancestor + * of the indexed commit (#3127). The working tree checked out an older + * commit than the one indexed, or a release branch behind the indexed + * tip. The mismatch is established: `isStale` is `true` and + * `commitsBehind` stays 0 (there is no forward count to report). + * - `rev-list` could not answer at all, but HEAD resolved and is not the + * indexed commit: only the count is unknown. A branch-pinned `serve` + * clone reaches this once git prunes the commit a failed re-index left + * behind — the pinned update is a `fetch --depth 1`, which orphans it — + * and a rewritten history reaches it directly. This arm keeps the + * historical fail-open `isStale: false` (see below). + * - `unknown` — the probe could not establish the relationship: no readable + * HEAD, a timeout, no recorded commit, or malformed count output. + * + * `isStale` and `commitsBehind` keep their historical fail-open values + * (`false` / `0`) whenever the check could not fully answer — every `unknown`, + * and the `rev-list`-failure arm of `diverged` — so no existing consumer + * changes behaviour there unless it reads `status`. The other arm of + * `diverged` (the confirmed rollback) is a successful, computed + * answer rather than a failure, so `isStale` reflects it (`true`) instead. */ export type StalenessStatus = 'current' | 'behind' | 'diverged' | 'unknown'; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 5b26beeea..cb8546543 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -324,6 +324,28 @@ function nonBlankUid(value: unknown): string | undefined { return typeof value === 'string' ? value.trim() || undefined : undefined; } +const SYMBOL_IDENTITY_RECOVERY_SUGGESTION = + 'Run gitnexus analyze --force from the affected repository root to rebuild the index.'; + +class SymbolIdentityError extends Error { + constructor() { + super('The index returned an invalid symbol identity. ' + SYMBOL_IDENTITY_RECOVERY_SUGGESTION); + this.name = 'SymbolIdentityError'; + } +} + +/** Validate database identities before using them as graph traversal anchors. */ +function assertSymbolIdentity(id: unknown, expectedUid?: string): asserts id is string { + if ( + typeof id !== 'string' || + !id.trim() || + id.includes('\0') || + (expectedUid !== undefined && id !== expectedUid) + ) { + throw new SymbolIdentityError(); + } +} + interface StringAliasDefinition { canonical: string; aliases: readonly string[]; @@ -3150,6 +3172,7 @@ export class LocalBackend { // regardless of whether OTHER tables succeeded — previously a real error // on N-1 of N tables while one succeeded left zero diagnostic trail. const ftsQueryErrors = bm25SearchResult?.nonBenignErrors; + const ftsMissingIndexes = bm25SearchResult?.missingIndexes; if (ftsQueryErrors) { // tri-review NEW-5: these strings are already classified non-benign by // classifyFtsQueryError — do NOT route them through logQueryError, @@ -3639,13 +3662,15 @@ export class LocalBackend { branch: repo.branch, indexedAt: this.lastObservedPoolState.get(repo.lbugPath)?.indexedAt ?? repo.indexedAt, }; - // tri-review NEW-1: every table failing for a REAL error (timeout, - // connection reset) is not a missing-index condition — `ftsDegradedWarning`'s - // "run --repair-fts" headline won't fix it. Route to a dedicated message - // instead of burying the real cause as a trailing suffix on bad advice. + // Real errors (timeout, connection reset) need their own diagnosis. + // When some indexes are also missing, preserve both causes and append + // their repair guidance below even though no FTS query succeeded. warnings.push( ftsQueryErrors - ? ftsQueryFailedWarning({ ...warningContext, lastErrorRedacted: ftsQueryErrors[0] }) + ? ftsQueryFailedWarning( + { ...warningContext, lastErrorRedacted: ftsQueryErrors[0] }, + !!ftsMissingIndexes?.length, + ) : ftsDegradedWarning(warningContext, ftsDisabledReason), ); } else if (ftsQueryErrors) { @@ -3658,6 +3683,12 @@ export class LocalBackend { `FTS keyword search partially failed — ${ftsQueryErrors.length} of the configured indexes hit a query error and were skipped; results may be missing matches from those node types (see server logs).`, ); } + if (ftsMissingIndexes?.length && (ftsUsed || ftsQueryErrors)) { + warnings.push( + `FTS keyword search is incomplete: missing configured indexes (${ftsMissingIndexes.join(', ')}). ` + + 'Results may be missing matches from those node types. Run `gitnexus analyze --repair-fts`.', + ); + } // #2331: a CJK query against a server process resolving // GITNEXUS_FTS_CJK_SEGMENTATION to 'none' silently misses sub-phrase // matches with no other signal — this is the only place an agent driving @@ -3789,7 +3820,7 @@ export class LocalBackend { // #2767: a partial FTS failure (some tables ok, one or more real errors) // is as much a "results may be incomplete" signal as enrichmentDegraded — // flag it the same way rather than only via the warning string. - const ftsPartial = ftsUsed && !!ftsQueryErrors; + const ftsPartial = ftsUsed && (!!ftsQueryErrors || !!ftsMissingIndexes?.length); return { processes, @@ -3810,7 +3841,12 @@ export class LocalBackend { query: string, limit: number, disabledReason?: FtsDisabledReason, - ): Promise<{ results: any[]; ftsUsed: boolean; nonBenignErrors?: string[] }> { + ): Promise<{ + results: any[]; + ftsUsed: boolean; + nonBenignErrors?: string[]; + missingIndexes?: string[]; + }> { if (disabledReason) return { results: [], ftsUsed: false }; let searchFTSFromLbug; try { @@ -3844,6 +3880,7 @@ export class LocalBackend { const bm25Results = ftsResponse?.results ?? []; const ftsUsed = ftsResponse?.ftsAvailable ?? false; const nonBenignErrors = ftsResponse?.nonBenignErrors; + const missingIndexes = ftsResponse?.missingIndexes; const results: any[] = []; @@ -3917,7 +3954,12 @@ export class LocalBackend { } } - return { results, ftsUsed, ...(nonBenignErrors && { nonBenignErrors }) }; + return { + results, + ftsUsed, + ...(nonBenignErrors && { nonBenignErrors }), + ...(missingIndexes && { missingIndexes }), + }; } /** @@ -4529,6 +4571,7 @@ export class LocalBackend { endLine: (r.endLine ?? r[5]) as number, ...(include_content ? { content: (r.content ?? r[6]) as string | undefined } : {}), }; + assertSymbolIdentity(symbol.id, uid); // Same LadybugDB label-enrichment as the name-based path: a UID // pointing at a Class must still surface `type: 'Class'` so impact's // Class/Interface BFS seed fires. No-op when type is already set. @@ -4662,6 +4705,9 @@ export class LocalBackend { endLine: (r.endLine ?? r[5]) as number, ...(include_content ? { content: (r.content ?? r[6]) as string | undefined } : {}), })); + // Reject the whole result before narrowing or scoring: dropping a corrupt + // candidate could make an unrelated surviving symbol look unambiguous. + for (const candidate of normalized) assertSymbolIdentity(candidate.id); // An exact File path wins over anchored suffix candidates. Without this, // `lib/a.ts` and `src/lib/a.ts` both score as File candidates and turn an @@ -4815,6 +4861,9 @@ export class LocalBackend { return await this._contextImpl(repo, params); } catch (err: any) { const msg = (err instanceof Error ? err.message : String(err)) || 'Context query failed'; + if (err instanceof SymbolIdentityError) { + return { error: msg, recoverySuggestion: SYMBOL_IDENTITY_RECOVERY_SUGGESTION }; + } if (isWalCorruptionError(err)) { return { error: msg, @@ -7119,8 +7168,16 @@ export class LocalBackend { // Return structured error instead of crashing (#321) const message = (err instanceof Error ? err.message : String(err)) || 'Impact analysis failed'; - const suggestion = 'The graph query failed — try gitnexus context as a fallback'; - const recoverySuggestion = isWalCorruptionError(err) ? WAL_RECOVERY_SUGGESTION : undefined; + const recoverySuggestion = + err instanceof SymbolIdentityError + ? SYMBOL_IDENTITY_RECOVERY_SUGGESTION + : isWalCorruptionError(err) + ? WAL_RECOVERY_SUGGESTION + : undefined; + const suggestion = + err instanceof SymbolIdentityError + ? SYMBOL_IDENTITY_RECOVERY_SUGGESTION + : 'The graph query failed — try gitnexus context as a fallback'; if (params.mode === 'pdg') { // Symbol resolution never reached the catch with a resolved symbol (the // throw can originate before/within resolution), so the envelope carries @@ -9138,6 +9195,7 @@ export class LocalBackend { ]; try { + assertSymbolIdentity(sym.id ?? sym[0], uid); // skipPerSymbolEnrichment suppresses ONLY the per-symbol STEP_IN_PROCESS // enrichment pass while preserving byDepth. Group-mode cross-repo fan-out // may fan across many repos; the per-symbol pass adds up to MAX_CHUNKS diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index 8436629e8..49687fa9b 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -351,7 +351,7 @@ async function getContextResource(backend: LocalBackend, repoName?: string): Pro // Check staleness using the current on-disk lastCommit (not the cached handle) const repoPath = repo.repoPath; - const lastCommit = freshMeta?.lastCommit ?? repo.lastCommit ?? 'HEAD'; + const lastCommit = freshMeta?.lastCommit ?? repo.lastCommit ?? ''; const staleness = repoPath ? checkStaleness(repoPath, lastCommit) : { isStale: false, commitsBehind: 0 }; diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 37eb0baf7..77e37a250 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -822,7 +822,14 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // `handlerReceiver` hint. Warm v123 Go worker results carry no routes. // v125 (#3402): Go route hints now honor lexical declarations and captured writes; // namespace imports retain whether their local name comes from the package clause. -const SCHEMA_BUMP = 125; +// v126 (#3446): SDK positional tool registrations now emit tool definitions, +// exact handler identities, and an opt-out from unrelated file-level flows. +// Warm v125 worker results omit these definitions and must be re-extracted. +// v127 (#3450): Destructured member writes invalidate SDK registration evidence. +// Warm v126 worker results can retain false tools after a method replacement. +// v128 (#3450): SDK namespace imports now prove positional tool receivers. +// Warm v127 worker results omit these definitions and must be re-extracted. +const SCHEMA_BUMP = 128; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/lang-resolution/swift-injected-closure-call/Caller.swift b/gitnexus/test/fixtures/lang-resolution/swift-injected-closure-call/Caller.swift new file mode 100644 index 000000000..dfea6fe9a --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/swift-injected-closure-call/Caller.swift @@ -0,0 +1,54 @@ +import Foundation + +enum Example { + static func runScenario() -> Int { + makeValue(input: 1) + } +} + +final class Service { + private let clock: () -> Date + + init(clock: @escaping () -> Date) { + self.clock = clock + } + + func refreshValue() -> Date { + clock() + } + + func refreshWithLocalClock() -> Int { + func clock() -> Int { 2 } + return clock() + } +} + +class BaseService { + let clock: () -> Date + + init(clock: @escaping () -> Date) { + self.clock = clock + } +} + +final class DerivedService: BaseService { + func refreshInheritedValue() -> Date { + clock() + } +} + +final class LabeledService { + let first: Int = 1 + + func first(where value: Bool) -> Int { + value ? 2 : 0 + } + + func refreshLabeled() -> Int { + first(where: true) + } +} + +class PrivateBase { + private let clock: () -> Int = { 1 } +} diff --git a/gitnexus/test/fixtures/lang-resolution/swift-injected-closure-call/Helpers.swift b/gitnexus/test/fixtures/lang-resolution/swift-injected-closure-call/Helpers.swift new file mode 100644 index 000000000..64c6b6a10 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/swift-injected-closure-call/Helpers.swift @@ -0,0 +1,39 @@ +import Foundation + +extension Example { + static func makeValue(input: Int) -> Int { + input + 1 + } +} + +enum Other { + private static func makeValue(other: String) -> Int { + -1 + } + + static func clock() -> Date { + Date.distantPast + } +} + +extension DerivedService { + func refreshInheritedFromExtension() -> Date { + clock() + } +} + +extension BaseService { + func refreshOwnFromExtension() -> Date { + clock() + } +} + +final class PrivateDerived: PrivateBase { + func clock() -> Int { + 2 + } + + func refreshPrivateAncestor() -> Int { + clock() + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/handlers.ts b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/handlers.ts new file mode 100644 index 000000000..e30cb154d --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/handlers.ts @@ -0,0 +1 @@ +export function importedHandler() { return 'imported'; } diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/server.js b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/server.js new file mode 100644 index 000000000..eedf3488f --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/server.js @@ -0,0 +1,5 @@ +import { McpServer as Server } from '@modelcontextprotocol/sdk/server/mcp.js'; + +const server = new Server({ name: 'javascript', version: '1' }); +function jsPing() { return 'pong'; } +server.registerTool('js_ping', { description: 'Ping JavaScript' }, jsPing); diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/server.ts b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/server.ts new file mode 100644 index 000000000..4a9c954f8 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/server.ts @@ -0,0 +1,52 @@ +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { importedHandler } from './handlers.js'; + +const server = new McpServer({ name: 'fixture', version: '1' }); + +function formatSearch(query: string) { return query; } +function lookupSearch(query: string) { return formatSearch(query); } +export function searchFiles(query: string) { return lookupSearch(query); } + +function formatFile(file: string) { return file; } +function lookupFile(file: string) { return formatFile(file); } +export const readFile = (file: string) => lookupFile(file); + +function formatOther() { return 'other'; } +function lookupOther() { return formatOther(); } +export function unrelatedEntry() { return lookupOther(); } + +server.registerTool('search-files', { description: 'Search files' }, searchFiles); +server.tool('read_file', 'Read a file', {}, readFile); +server.registerTool('inline_callback', {}, async () => 'inline'); +server.tool('imported_callback', importedHandler); + +function install(server: McpServer, searchFiles: () => string) { + server.registerTool('parameter_callback', {}, searchFiles); +} + +let mutable = () => 'mutable'; +server.tool('mutable_callback', mutable); + +function replaced() { return 'before'; } +replaced = () => 'after'; +server.tool('reassigned_callback', replaced); + +{ + const searchFiles = 'not callable'; + server.tool('shadowed_callback', searchFiles); +} + +const alias = readFile; +server.tool('alias_callback', alias); + +const expressionHandler = function () { return 'expression'; }; +server.registerTool('function_expression_tool', {}, expressionHandler); + +{ + const handler = () => lookupSearch('first'); + server.tool('first_block_callback', handler); +} +{ + const handler = () => lookupFile('second'); + server.tool('second_block_callback', handler); +} diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/tools.ts b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/tools.ts new file mode 100644 index 000000000..0013ccfab --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/typescript-mcp-tools/src/tools.ts @@ -0,0 +1,4 @@ +export const tools = [ + { name: 'manifest_tool', description: 'Existing object manifest', inputSchema: {} }, + { name: 'read_file', description: 'Duplicate manifest entry', inputSchema: {} }, +]; diff --git a/gitnexus/test/fixtures/local-backend-seed.ts b/gitnexus/test/fixtures/local-backend-seed.ts index 4878348b2..1f7fd02c9 100644 --- a/gitnexus/test/fixtures/local-backend-seed.ts +++ b/gitnexus/test/fixtures/local-backend-seed.ts @@ -1,4 +1,5 @@ import type { FTSIndexDef } from '../helpers/test-indexed-db.js'; +import { FTS_INDEXES } from '../../src/core/search/fts-schema.js'; export const LOCAL_BACKEND_SEED_DATA = [ // Files @@ -60,9 +61,8 @@ export const LOCAL_BACKEND_SEED_DATA = [ CREATE (c)-[:CodeRelation {type: 'HAS_METHOD', confidence: 1.0, reason: 'class-method', step: 0}]->(m)`, ]; -export const LOCAL_BACKEND_FTS_INDEXES: FTSIndexDef[] = [ - { table: 'Function', indexName: 'function_fts', columns: ['name', 'content', 'description'] }, - { table: 'Class', indexName: 'class_fts', columns: ['name', 'content', 'description'] }, - { table: 'Method', indexName: 'method_fts', columns: ['name', 'content', 'description'] }, - { table: 'File', indexName: 'file_fts', columns: ['name', 'content'] }, -]; +// Healthy-backend fixtures must mirror the complete configured search index set. +// The old four-table subset is now correctly reported as partial FTS coverage. +export const LOCAL_BACKEND_FTS_INDEXES: FTSIndexDef[] = FTS_INDEXES.map( + ({ table, indexName, properties }) => ({ table, indexName, columns: [...properties] }), +); diff --git a/gitnexus/test/fixtures/swift-captures-golden/expected-captures.json b/gitnexus/test/fixtures/swift-captures-golden/expected-captures.json index b6c40d1b1..037d5d01c 100644 --- a/gitnexus/test/fixtures/swift-captures-golden/expected-captures.json +++ b/gitnexus/test/fixtures/swift-captures-golden/expected-captures.json @@ -127,6 +127,14 @@ "captureGroups": 9, "digest": "c4d442b67247406de3b1158b958f6f3e294f11c0bbb360ed32489582769a1187" }, + "swift-injected-closure-call/Caller.swift": { + "captureGroups": 69, + "digest": "6768d62722a6b173dbe953507fec53661ea6c9786a49d8edf7caedb0a3cef0e4" + }, + "swift-injected-closure-call/Helpers.swift": { + "captureGroups": 48, + "digest": "6e6bfe83084bf189a7789702cfccb5fcc5753986b0de3c0d7f81fc3ae59270b2" + }, "swift-member-write-access/App.swift": { "captureGroups": 14, "digest": "7be68b7e510fefac99c94a4ebab3eb96556a0e587155b6c82b7602705e270987" diff --git a/gitnexus/test/integration/context-resource-staleness.test.ts b/gitnexus/test/integration/context-resource-staleness.test.ts index ab94fcab8..c5f58454d 100644 --- a/gitnexus/test/integration/context-resource-staleness.test.ts +++ b/gitnexus/test/integration/context-resource-staleness.test.ts @@ -207,4 +207,26 @@ describe('context resource freshness — out-of-process analyze (#2438)', () => expect(result).toContain('symbols: 333'); expect(result).toContain('processes: 4'); }); + + it('does not claim divergence when disk metadata and the cached handle have no indexed commit', async function missingIndexedCommitDoesNotClaimDivergence() { + writeFileSync(path.join(repoPath, 'a.ts'), 'export const a = 1;\n'); + runGit(repoPath, 'add', 'a.ts'); + runGit(repoPath, 'commit', '-m', 'c1'); + + // A legacy metadata/registry entry can omit lastCommit despite RepoMeta's + // required field; seed that persisted shape rather than a symbolic ref. + await seedIndexedRepo(repoPath, storagePath, { + repoPath, + indexedAt: '2024-01-01T00:00:00Z', + stats: { files: 1, nodes: 1, processes: 0 }, + } as RepoMeta); + + const backend = new LocalBackend(); + await backend.init(); + expect((await backend.resolveRepo('test-repo')).lastCommit).toBeUndefined(); + + const result = await readResource('gitnexus://repo/test-repo/context', backend); + expect(result).not.toContain('staleness:'); + expect(result).toContain(' commit: ""'); + }); }); diff --git a/gitnexus/test/integration/fts-repair-warm-session.test.ts b/gitnexus/test/integration/fts-repair-warm-session.test.ts index 2550ba874..ac9ce2713 100644 --- a/gitnexus/test/integration/fts-repair-warm-session.test.ts +++ b/gitnexus/test/integration/fts-repair-warm-session.test.ts @@ -32,6 +32,7 @@ const REQUIRE_FTS = process.env.GITNEXUS_REQUIRE_FTS === '1'; type QueryResult = { error?: unknown; warning?: string; + partial?: boolean; definitions?: Array<{ id: string }>; process_symbols?: Array<{ id: string }>; }; @@ -40,7 +41,8 @@ const matchedIds = (r: QueryResult): string[] => [...(r.process_symbols ?? []), ...(r.definitions ?? [])].map((s) => s.id); const ftsMissing = (r: QueryResult): boolean => - typeof r.warning === 'string' && /FTS indexes missing/i.test(r.warning); + typeof r.warning === 'string' && + /FTS indexes missing|missing configured indexes/i.test(r.warning); /** * Poll the SAME warm `LocalBackend` until it stops reporting FTS-missing, or @@ -90,14 +92,14 @@ describe('warm MCP session observes an in-place --repair-fts rebuild (#2767)', ( await tmpHandle.cleanup(); }); - it( - 'a warm session transitions from FTS-unavailable to FTS-available without restarting, after an out-of-band --repair-fts', + it.each(['all', 'Function'] as const)( + 'a warm session repairs missing %s indexes without restarting', { timeout: 60_000 }, - async (ctx) => { + async (missing, ctx) => { const adapter = await import('../../src/core/lbug/lbug-adapter.js'); const { createSearchFTSIndexes } = await import('../../src/core/search/fts-indexes.js'); - // ── Step 1: build the index WITHOUT FTS (analyzed before repair) ──── + // ── Step 1: build an index with all or just one FTS index missing ── await adapter.initLbug(lbugPath); const ftsAvailable = await adapter.loadFTSExtension(undefined, { @@ -118,6 +120,10 @@ describe('warm MCP session observes an in-place --repair-fts rebuild (#2767)', ( await adapter.executeQuery( `CREATE (n:Function {id: 'func:login', name: 'login', filePath: 'src/auth.ts', startLine: 1, endLine: 3, content: 'function login() { return true; }'})`, ); + if (missing === 'Function') { + await createSearchFTSIndexes(); + await adapter.dropFTSIndex('Function', 'function_fts'); + } await adapter.flushWAL(); await adapter.closeLbug(); @@ -129,7 +135,10 @@ describe('warm MCP session observes an in-place --repair-fts rebuild (#2767)', ( stats: { files: 1, nodes: 1 }, capabilities: { graph: { provider: 'ladybugdb', status: 'available' }, - fts: { provider: 'ladybugdb-fts', status: 'unavailable' }, + fts: { + provider: 'ladybugdb-fts', + status: missing === 'all' ? 'unavailable' : 'available', + }, vectorSearch: { provider: 'exact-scan', status: 'unavailable', exactScanLimit: 0 }, }, }; @@ -142,6 +151,10 @@ describe('warm MCP session observes an in-place --repair-fts rebuild (#2767)', ( const before = await backend.callTool('query', { query: 'login' }); expect(before.error).toBeUndefined(); expect(ftsMissing(before)).toBe(true); + if (missing === 'Function') { + expect(before.partial).toBe(true); + expect(before.warning).toContain('Function.function_fts'); + } // ── Step 3: out-of-band --repair-fts (separate writable session) ──── // Same production functions the repair-fts branch of runFullAnalysis @@ -163,6 +176,7 @@ describe('warm MCP session observes an in-place --repair-fts rebuild (#2767)', ( const after = await waitForFtsRecognized(backend, 'login'); expect(after.error).toBeUndefined(); expect(ftsMissing(after)).toBe(false); + expect(after.partial).toBeUndefined(); expect(matchedIds(after)).toContain('func:login'); }, ); diff --git a/gitnexus/test/integration/local-backend-calltool.test.ts b/gitnexus/test/integration/local-backend-calltool.test.ts index f618a3c5b..35eabbdd4 100644 --- a/gitnexus/test/integration/local-backend-calltool.test.ts +++ b/gitnexus/test/integration/local-backend-calltool.test.ts @@ -956,3 +956,98 @@ withTestLbugDB( }, }, ); + +const PYTHON_METHOD_ID = 'Method:tests/test_supervisor.py:Supervisor.run'; +const PYTHON_CALLER_ID = 'Function:tests/test_supervisor.py:test_run'; +const SWIFT_METHOD_ID = 'Method:Sources/Supervisor.swift:Supervisor.run'; +const SWIFT_CALLER_ID = 'Constructor:Sources/Supervisor.swift:Supervisor.init'; + +withTestLbugDB( + 'symbol-identity-isolation-3424', + (handle) => { + describe('mixed Python/Swift symbol identity isolation (#3424)', () => { + let backend: LocalBackend; + + beforeAll(() => { + backend = (handle as typeof handle & { _backend: LocalBackend })._backend; + }); + + it.each(['name and file', 'UID'])('keeps Python context isolated by %s', async (lookup) => { + const params = + lookup === 'UID' + ? { uid: PYTHON_METHOD_ID } + : { name: 'run', file_path: 'tests/test_supervisor.py' }; + const result = await backend.callTool('context', params); + expect(result).not.toHaveProperty('error'); + expect(result.symbol.uid).toBe(PYTHON_METHOD_ID); + expect(result.incoming.calls.map((caller: { uid: string }) => caller.uid)).toEqual([ + PYTHON_CALLER_ID, + ]); + }); + + it.each(['name and file', 'UID'])('keeps Python impact isolated by %s', async (lookup) => { + const params = + lookup === 'UID' + ? { target_uid: PYTHON_METHOD_ID } + : { target: 'run', file_path: 'tests/test_supervisor.py' }; + const result = await backend.callTool('impact', { + ...params, + direction: 'upstream', + includeTests: true, + }); + expect(result).not.toHaveProperty('error'); + expect(result.target.id).toBe(PYTHON_METHOD_ID); + expect(result.impactedCount).toBe(1); + expect(result.byDepth[1].map((caller: { id: string }) => caller.id)).toEqual([ + PYTHON_CALLER_ID, + ]); + }); + + it('keeps the unrelated Swift constructor queryable', async () => { + const context = await backend.callTool('context', { uid: SWIFT_METHOD_ID }); + expect(context).not.toHaveProperty('error'); + expect(context.symbol.uid).toBe(SWIFT_METHOD_ID); + expect(context.incoming.calls.map((caller: { uid: string }) => caller.uid)).toEqual([ + SWIFT_CALLER_ID, + ]); + const impact = await backend.callTool('impact', { + target_uid: SWIFT_METHOD_ID, + direction: 'upstream', + includeTests: true, + }); + expect(impact).not.toHaveProperty('error'); + expect(impact.target.id).toBe(SWIFT_METHOD_ID); + expect(impact.impactedCount).toBe(1); + expect(impact.byDepth[1].map((caller: { id: string }) => caller.id)).toEqual([ + SWIFT_CALLER_ID, + ]); + }); + }); + }, + { + seed: [ + `CREATE (:Method {id: '${PYTHON_METHOD_ID}', name: 'run', filePath: 'tests/test_supervisor.py', startLine: 3, endLine: 5})`, + `CREATE (:Function {id: '${PYTHON_CALLER_ID}', name: 'test_run', filePath: 'tests/test_supervisor.py', startLine: 7, endLine: 9})`, + `CREATE (:Method {id: '${SWIFT_METHOD_ID}', name: 'run', filePath: 'Sources/Supervisor.swift', startLine: 3, endLine: 5})`, + `CREATE (:Constructor {id: '${SWIFT_CALLER_ID}', name: 'init', filePath: 'Sources/Supervisor.swift', startLine: 7, endLine: 9})`, + `MATCH (a:Function), (b:Method) WHERE a.id = '${PYTHON_CALLER_ID}' AND b.id = '${PYTHON_METHOD_ID}' CREATE (a)-[:CodeRelation {type: 'CALLS', confidence: 1.0, reason: 'direct', step: 0}]->(b)`, + `MATCH (a:Constructor), (b:Method) WHERE a.id = '${SWIFT_CALLER_ID}' AND b.id = '${SWIFT_METHOD_ID}' CREATE (a)-[:CodeRelation {type: 'CALLS', confidence: 1.0, reason: 'direct', step: 0}]->(b)`, + ], + poolAdapter: true, + afterSetup: async (handle) => { + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'mixed-language-repo', + path: '/mixed-language/repo', + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + stats: { files: 2, nodes: 4, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (handle as typeof handle & { _backend: LocalBackend })._backend = backend; + }, + }, +); diff --git a/gitnexus/test/integration/resolvers/swift.test.ts b/gitnexus/test/integration/resolvers/swift.test.ts index 5d3e0bd85..76b25a565 100644 --- a/gitnexus/test/integration/resolvers/swift.test.ts +++ b/gitnexus/test/integration/resolvers/swift.test.ts @@ -359,6 +359,95 @@ describe.skipIf(!swiftAvailable)('Swift protocol-extension implicit self (#3273) }); }); +describe.skipIf(!swiftAvailable)('Swift injected closure property call (#3425)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'swift-injected-closure-call'), + () => {}, + ); + }, 60000); + + it('does not resolve an injected closure call to an unrelated method', () => { + expect( + getNodesByLabelFull(result, 'Property').some( + (node) => node.name === 'clock' && node.properties.filePath === 'Caller.swift', + ), + ).toBe(true); + expect( + getNodesByLabelFull(result, 'Function').some( + (node) => node.name === 'clock' && node.properties.filePath === 'Helpers.swift', + ), + ).toBe(true); + const calls = getRelationships(result, 'CALLS').filter((c) => c.source === 'refreshValue'); + expect(calls.filter((c) => c.target === 'clock')).toEqual([]); + }); + + it('does not resolve an inherited closure property call to the unrelated method', () => { + const extendsEdges = getRelationships(result, 'EXTENDS'); + expect( + extendsEdges.some( + (edge) => edge.source === 'DerivedService' && edge.target === 'BaseService', + ), + ).toBe(true); + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.source === 'refreshInheritedValue', + ); + expect(calls.filter((c) => c.target === 'clock')).toEqual([]); + }); + + it('keeps inherited closure calls in extensions unlinked', () => { + expect( + getNodesByLabelFull(result, 'Function').some( + (node) => + node.name === 'refreshInheritedFromExtension' && + node.properties.filePath === 'Helpers.swift', + ), + ).toBe(true); + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.source === 'refreshInheritedFromExtension', + ); + expect(calls.filter((c) => c.target === 'clock')).toEqual([]); + }); + + it('keeps same-type closure calls in extensions unlinked', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.source === 'refreshOwnFromExtension', + ); + expect(calls.filter((c) => c.target === 'clock')).toEqual([]); + }); + + it('still resolves the concrete-type extension call', () => { + const calls = getRelationships(result, 'CALLS').filter((c) => c.source === 'runScenario'); + expect(calls.map((c) => c.rel.targetId)).toEqual([ + 'Function:Helpers.swift:Example.makeValue#1', + ]); + }); + + it('keeps a nested function that shadows the stored closure', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.source === 'refreshWithLocalClock', + ); + expect(calls.filter((c) => c.target === 'clock')).toHaveLength(1); + expect(calls.find((c) => c.target === 'clock')?.targetFilePath).toBe('Caller.swift'); + }); + + it('keeps a labeled method selected alongside a same-name property', () => { + const calls = getRelationships(result, 'CALLS').filter((c) => c.source === 'refreshLabeled'); + expect(calls.map((c) => c.target)).toContain('first'); + expect(calls.find((c) => c.target === 'first')?.targetFilePath).toBe('Caller.swift'); + }); + + it('keeps a derived method despite an inaccessible ancestor property', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.source === 'refreshPrivateAncestor', + ); + expect(calls.map((c) => c.target)).toContain('clock'); + expect(calls.find((c) => c.target === 'clock')?.targetFilePath).toBe('Helpers.swift'); + }); +}); + // --------------------------------------------------------------------------- // Constructor fallback: Swift constructors look like free function calls // (no `new` keyword). The resolver retries with constructor form when diff --git a/gitnexus/test/integration/resolvers/typescript-mcp-tools.test.ts b/gitnexus/test/integration/resolvers/typescript-mcp-tools.test.ts new file mode 100644 index 000000000..fba040759 --- /dev/null +++ b/gitnexus/test/integration/resolvers/typescript-mcp-tools.test.ts @@ -0,0 +1,283 @@ +import { beforeAll, describe, expect, it } from 'vitest'; +import path from 'node:path'; +import fs from 'node:fs'; +import os from 'node:os'; +import { + loadParseCache, + PARSE_CACHE_VERSION, + pruneCache, + saveParseCache, + type ParseCache, +} from '../../../src/storage/parse-cache.js'; +import { + getDurableParsedFileDir, + pruneAndSaveDurableParsedFileStore, +} from '../../../src/storage/parsedfile-store.js'; +import { + FIXTURES, + findDanglingEdges, + getNodesByLabel, + getNodesByLabelFull, + getRelationships, + runPipelineFromRepo, + type PipelineResult, +} from './helpers.js'; + +describe('JavaScript and TypeScript SDK tool registrations', () => { + let result: PipelineResult; + const unresolved = [ + 'inline_callback', + 'imported_callback', + 'parameter_callback', + 'mutable_callback', + 'reassigned_callback', + 'shadowed_callback', + 'alias_callback', + 'first_block_callback', + 'second_block_callback', + ]; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'typescript-mcp-tools'), () => {}); + }, 60000); + + it.each(['ts', 'js'])( + 'does not emit tools for a destructured method replacement in %s', + async (extension) => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-mcp-tool-write-')); + try { + fs.writeFileSync( + path.join(repo, `server.${extension}`), + ` + import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; + const replaced = new McpServer({ name: 'replaced', version: '1' }); + ({ registerTool: replaced.registerTool } = { registerTool: () => undefined }); + replaced.registerTool('fake', {}, () => ({ content: [] })); + const actual = new McpServer({ name: 'actual', version: '1' }); + actual.registerTool('real', {}, () => ({ content: [] })); + `, + ); + const pipeline = await runPipelineFromRepo(repo, () => {}, { workerPoolSize: 1 }); + expect(getNodesByLabel(pipeline, 'Tool')).toEqual(['real']); + expect(findDanglingEdges(pipeline, ['HANDLES_TOOL', 'ENTRY_POINT_OF'])).toEqual([]); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }, + ); + + it.each(['ts', 'js'])( + 'preserves namespace registrations through cold/warm %s parsing', + async (extension) => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-mcp-namespace-')); + const storageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-mcp-namespace-cache-')); + try { + fs.writeFileSync( + path.join(repo, `server.${extension}`), + ` + import * as SDK from '@modelcontextprotocol/sdk/server/mcp.js'; + const server = new SDK.McpServer({}); + function handleNamespace() { return { content: [] }; } + server.registerTool('namespace', { description: 'Namespace tool' }, handleNamespace); + const replaced = new SDK.McpServer({}); + ({ method: replaced.registerTool } = other); + replaced.registerTool('fake', {}, handleNamespace); + ${extension === 'ts' ? "function install(typed: SDK.McpServer) { typed.tool('typed_namespace', handleNamespace); }" : ''} + `, + ); + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + storagePath: storageDir, + onDiskKeys: new Set(), + }; + const cold = await runPipelineFromRepo(repo, () => {}, { + parseCache: cache, + workerPoolSize: 1, + }); + expect(cold.usedWorkerPool).toBe(true); + pruneCache(cache, cache.usedKeys); + const keys = await saveParseCache(storageDir, cache); + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + new Set(keys), + ); + const warmCache = await loadParseCache(storageDir); + expect(warmCache).not.toBeNull(); + const warm = await runPipelineFromRepo(repo, () => {}, { + parseCache: warmCache!, + workerPoolSize: 1, + }); + expect(warm.usedWorkerPool).toBe(false); + for (const pipeline of [cold, warm]) { + expect(getNodesByLabel(pipeline, 'Tool')).toEqual( + extension === 'ts' ? ['namespace', 'typed_namespace'] : ['namespace'], + ); + expect( + getNodesByLabelFull(pipeline, 'Tool').find((tool) => tool.name === 'namespace') + ?.properties.description, + ).toBe('Namespace tool'); + expect( + getRelationships(pipeline, 'HANDLES_TOOL').filter( + (edge) => edge.target === 'namespace', + ), + ).toMatchObject([{ source: 'handleNamespace', sourceLabel: 'Function' }]); + expect(findDanglingEdges(pipeline, ['HANDLES_TOOL', 'ENTRY_POINT_OF'])).toEqual([]); + } + expect(getNodesByLabelFull(warm, 'Tool')).toEqual(getNodesByLabelFull(cold, 'Tool')); + expect(getRelationships(warm, 'HANDLES_TOOL')).toEqual( + getRelationships(cold, 'HANDLES_TOOL'), + ); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + fs.rmSync(storageDir, { recursive: true, force: true }); + } + }, + 120_000, + ); + + it('discovers ordinary server files alongside deduplicated object manifests', () => { + expect(getNodesByLabel(result, 'Tool')).toEqual( + [ + ...unresolved, + 'search-files', + 'read_file', + 'function_expression_tool', + 'js_ping', + 'manifest_tool', + ].sort(), + ); + const tools = new Map( + getNodesByLabelFull(result, 'Tool').map((tool) => [tool.name, tool.properties]), + ); + expect(tools.get('search-files')).toMatchObject({ + filePath: 'src/server.ts', + description: 'Search files', + }); + expect(tools.get('read_file')).toMatchObject({ + filePath: 'src/server.ts', + description: 'Read a file', + }); + expect(tools.get('js_ping')).toMatchObject({ + filePath: 'src/server.js', + description: 'Ping JavaScript', + }); + expect(tools.get('manifest_tool')).toMatchObject({ + filePath: 'src/tools.ts', + description: 'Existing object manifest', + }); + expect(tools.get('inline_callback')?.description).toBe(''); + }); + + it('uses actual emitted callable nodes for supported local handlers', () => { + const edges = getRelationships(result, 'HANDLES_TOOL'); + for (const [tool, handler] of [ + ['search-files', 'searchFiles'], + ['read_file', 'readFile'], + ['function_expression_tool', 'expressionHandler'], + ['js_ping', 'jsPing'], + ]) { + expect(edges.filter((edge) => edge.target === tool)).toMatchObject([ + { source: handler, sourceLabel: 'Function' }, + ]); + } + expect(findDanglingEdges(result, ['HANDLES_TOOL', 'ENTRY_POINT_OF'])).toEqual([]); + }); + + it('links each same-file named handler only to its own execution flow', () => { + const edges = getRelationships(result, 'ENTRY_POINT_OF').filter( + (edge) => edge.sourceLabel === 'Tool', + ); + for (const [tool, handler] of [ + ['search-files', 'searchFiles'], + ['read_file', 'readFile'], + ]) { + const flows = edges.filter((edge) => edge.source === tool); + expect(flows).toHaveLength(1); + const process = result.graph.getNode(flows[0].rel.targetId)!; + const entry = result.graph.getNode(process.properties.entryPointId as string)!; + expect(entry.properties.name).toBe(handler); + } + }); + + it('keeps unresolved callbacks at file attribution without unrelated same-file flows', () => { + const handles = getRelationships(result, 'HANDLES_TOOL'); + const flows = getRelationships(result, 'ENTRY_POINT_OF'); + expect( + getNodesByLabelFull(result, 'Process').some((process) => { + const entry = result.graph.getNode(process.properties.entryPointId as string); + return entry?.properties.name === 'unrelatedEntry'; + }), + ).toBe(true); + for (const name of unresolved) { + expect(handles.filter((edge) => edge.target === name)).toMatchObject([ + { sourceLabel: 'File', sourceFilePath: 'src/server.ts' }, + ]); + expect(flows.filter((edge) => edge.sourceLabel === 'Tool' && edge.source === name)).toEqual( + [], + ); + } + }); + + it('preserves tool metadata, handler identities, and flow attribution on warm replay', async () => { + const storageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-mcp-tools-cache-')); + try { + const cold: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + storagePath: storageDir, + onDiskKeys: new Set(), + }; + const fixture = path.join(FIXTURES, 'typescript-mcp-tools'); + const initial = await runPipelineFromRepo(fixture, () => {}, { + parseCache: cold, + workerPoolSize: 1, + }); + expect(initial.usedWorkerPool).toBe(true); + pruneCache(cold, cold.usedKeys); + const savedKeys = await saveParseCache(storageDir, cold); + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + new Set(savedKeys), + ); + const warm = await loadParseCache(storageDir); + expect(warm).not.toBeNull(); + const replay = await runPipelineFromRepo(fixture, () => {}, { + parseCache: warm!, + workerPoolSize: 1, + }); + expect(replay.usedWorkerPool).toBe(false); + + const project = (pipeline: PipelineResult) => ({ + tools: getNodesByLabelFull(pipeline, 'Tool'), + edges: [ + ...getRelationships(pipeline, 'HANDLES_TOOL'), + ...getRelationships(pipeline, 'ENTRY_POINT_OF').filter( + (edge) => edge.sourceLabel === 'Tool', + ), + ] + .map(({ rel, source }) => ({ + type: rel.type, + source, + sourceId: rel.sourceId, + targetId: rel.targetId, + })) + .sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b))), + }); + const expected = project(initial); + expect(expected.tools).toHaveLength(unresolved.length + 5); + expect(project(replay)).toEqual(expected); + for (const name of unresolved) { + expect( + expected.edges.filter((edge) => edge.type === 'ENTRY_POINT_OF' && edge.source === name), + ).toEqual([]); + } + } finally { + fs.rmSync(storageDir, { recursive: true, force: true }); + } + }, 120_000); +}); diff --git a/gitnexus/test/unit/bm25-search.test.ts b/gitnexus/test/unit/bm25-search.test.ts index 793cf7aae..08b2fc46a 100644 --- a/gitnexus/test/unit/bm25-search.test.ts +++ b/gitnexus/test/unit/bm25-search.test.ts @@ -647,3 +647,125 @@ describe('BM25 search', () => { }); }); }); + +describe('FTS index completeness', () => { + beforeEach(async () => { + resetExtensionState(); + mockExecuteParameterized.mockReset(); + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS).mockReset(); + }); + + const missing = (table: string, indexName: string) => + new Error(`Binder exception: Table ${table} doesn't have an index with name ${indexName}.`); + + for (const pooled of [false, true]) { + const mode = pooled ? 'pool' : 'core'; + const repo = pooled ? 'completeness-test' : undefined; + + async function useOutcome(outcome: (table: string, indexName: string) => Promise) { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS).mockImplementation((table, indexName) => outcome(table, indexName)); + mockExecuteParameterized.mockImplementation(async (_repo: string, cypher: string) => { + const match = cypher.match(/QUERY_FTS_INDEX\('([^']+)', '([^']+)'/); + return outcome(match![1], match![2]); + }); + } + + it(`${mode}: distinguishes healthy zero matches from missing indexes`, async () => { + await useOutcome(async () => []); + const result = await searchFTSFromLbug('no matches', 5, repo); + expect(result).toEqual({ results: [], ftsAvailable: true }); + }); + + it(`${mode}: reports one missing index while retaining successful matches`, async () => { + await useOutcome(async (table, indexName) => { + if (table === 'Function') throw missing(table, indexName); + if (table !== 'File') return []; + return pooled + ? [{ node: { filePath: 'src/auth.ts', id: 'file:auth' }, score: 3 }] + : [{ filePath: 'src/auth.ts', nodeId: 'file:auth', score: 3 }]; + }); + const result = await searchFTSFromLbug('auth', 5, repo); + expect(result.ftsAvailable).toBe(true); + expect(result.results[0]).toMatchObject({ filePath: 'src/auth.ts', score: 3 }); + expect(result.missingIndexes).toEqual(['Function.function_fts']); + expect(result.nonBenignErrors).toBeUndefined(); + }); + + it(`${mode}: reports all missing indexes without claiming FTS is available`, async () => { + await useOutcome(async (table, indexName) => { + throw missing(table, indexName); + }); + const result = await searchFTSFromLbug('auth', 5, repo); + expect(result.ftsAvailable).toBe(false); + expect(result.results).toEqual([]); + if (!pooled) { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + for (const { table, indexName } of FTS_INDEXES) { + expect(queryFTS).toHaveBeenCalledWith(table, indexName, 'auth', 5, false, 'throw'); + } + } + expect(result.missingIndexes).toEqual( + FTS_INDEXES.map(({ table, indexName }) => `${table}.${indexName}`), + ); + expect(result.nonBenignErrors).toBeUndefined(); + }); + + it(`${mode}: preserves redacted real errors alongside missing-index diagnostics`, async () => { + await useOutcome(async (table, indexName) => { + if (table === 'Function') throw missing(table, indexName); + if (table === 'Class') + throw new Error('connection reset at /home/alice/private/index.lbug'); + return []; + }); + const result = await searchFTSFromLbug('auth', 5, repo); + expect(result.ftsAvailable).toBe(true); + expect(result.missingIndexes).toEqual(['Function.function_fts']); + expect(result.nonBenignErrors).toHaveLength(1); + expect(result.nonBenignErrors![0]).toContain('connection reset'); + expect(JSON.stringify(result)).not.toContain('/home/alice'); + }); + + it(`${mode}: retains both failure causes when no configured index query succeeds`, async () => { + await useOutcome(async (table, indexName) => { + if (table === 'Function') throw missing(table, indexName); + throw new Error('connection reset at /home/alice/private/index.lbug'); + }); + + const result = await searchFTSFromLbug('auth', 5, repo); + + expect(result.ftsAvailable).toBe(false); + expect(result.results).toEqual([]); + expect(result.missingIndexes).toEqual(['Function.function_fts']); + expect(result.nonBenignErrors).toEqual( + Array(FTS_INDEXES.length - 1).fill('connection reset at '), + ); + }); + + it(`${mode}: clears missing-index diagnostics after a repaired query`, async () => { + await useOutcome(async (table, indexName) => { + if (table === 'Function') throw missing(table, indexName); + return []; + }); + expect((await searchFTSFromLbug('auth', 5, repo)).missingIndexes).toEqual([ + 'Function.function_fts', + ]); + await useOutcome(async () => []); + expect(await searchFTSFromLbug('auth', 5, repo)).toEqual({ results: [], ftsAvailable: true }); + }); + + it(`${mode}: explicit opt-out makes no queries and emits no missing-index diagnosis`, async () => { + await useOutcome(async (table, indexName) => { + throw missing(table, indexName); + }); + expect(await searchFTSFromLbug('auth', 5, repo, 'disabled-by-flag')).toEqual({ + results: [], + ftsAvailable: false, + }); + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + expect(queryFTS).not.toHaveBeenCalled(); + expect(mockExecuteParameterized).not.toHaveBeenCalled(); + }); + } +}); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 6a0a98d4a..aeefc94a4 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -624,8 +624,8 @@ describe('LocalBackend.callTool', () => { }); it('reports UNKNOWN instead of a blast radius when the target resolves without a node id (#3354)', async () => { - // Every query returns the same id-less row: the resolver picks it as the - // single match, and the frontier query would answer for no symbol at all. + // Every query returns the same id-less row: resolution must reject it + // before a frontier query could answer for no symbol at all. (executeParameterized as any).mockResolvedValue([{ name: 'runSweep', type: 'Function' }]); const result = await backend.callTool('impact', { target: 'runSweep', direction: 'upstream' }); @@ -635,7 +635,8 @@ describe('LocalBackend.callTool', () => { impactedCount: null, risk: 'UNKNOWN', }); - expect(result.error).toMatch(/without a node id/); + expect(result.error).toMatch(/invalid symbol identity/); + expect(result.recoverySuggestion).toContain('gitnexus analyze --force'); expect(result).not.toHaveProperty('byDepthCounts'); }); @@ -2466,7 +2467,7 @@ describe('LocalBackend.callTool', () => { }, ]); - const result = await backend.impactByUid('test-project', 'uid:main', 'upstream', { + const result = await backend.impactByUid('test-project', 'func:main', 'upstream', { maxDepth: 5, relationTypes: ['CALLS'], minConfidence: 0, diff --git a/gitnexus/test/unit/cli-group-status-format.test.ts b/gitnexus/test/unit/cli-group-status-format.test.ts index 32278dd04..962efd536 100644 --- a/gitnexus/test/unit/cli-group-status-format.test.ts +++ b/gitnexus/test/unit/cli-group-status-format.test.ts @@ -27,6 +27,29 @@ describe('formatIndexStatusCell', () => { ); }); + it.each([0, 3])( + 'renders a stale diverged row without a commits-behind count of %i', + (commitsBehind) => { + const row = { indexStale: true, commitsBehind, status: 'diverged' as const }; + expect(formatIndexStatusCell(row)).toBe('STALE (index differs from HEAD)'); + }, + ); + + it('renders an explicit behind status with its counted gap', () => { + const row = { indexStale: true, commitsBehind: 3, status: 'behind' as const }; + expect(formatIndexStatusCell(row)).toBe('STALE (3 commits behind)'); + }); + + it('renders an explicit current status as OK', () => { + const row = { indexStale: false, commitsBehind: 0, status: 'current' as const }; + expect(formatIndexStatusCell(row)).toBe('OK '); + }); + + it('preserves the fail-open cell when a diverged probe did not confirm staleness', () => { + const row = { indexStale: false, commitsBehind: 0, status: 'diverged' as const }; + expect(formatIndexStatusCell(row)).toBe('OK '); + }); + it('keeps the OK cell byte-identical, padding included', () => { expect(formatIndexStatusCell({ indexStale: false, commitsBehind: 0 })).toBe('OK '); }); diff --git a/gitnexus/test/unit/community-processor.test.ts b/gitnexus/test/unit/community-processor.test.ts index 2e207a22a..e9b3d07aa 100644 --- a/gitnexus/test/unit/community-processor.test.ts +++ b/gitnexus/test/unit/community-processor.test.ts @@ -448,7 +448,76 @@ module.exports = { const second = await processCommunities(graph); expect(second.memberships).toEqual(first.memberships); + expect(second.rawMemberships).toEqual(first.rawMemberships); expect(second.stats.modularity).toBe(first.stats.modularity); }); }); }); + +describe('community membership integrity', () => { + it('returns empty raw and retained memberships for an empty graph', async () => { + const result = await processCommunities(createKnowledgeGraph()); + + expect(result.communities).toEqual([]); + expect(result.memberships).toEqual([]); + expect(result.rawMemberships).toEqual([]); + expect(result.stats).toMatchObject({ totalCommunities: 0, modularity: 0, nodesProcessed: 0 }); + }); + + it('retains connected members without emitting memberships for a filtered singleton', async () => { + const graph = createKnowledgeGraph(); + for (const id of ['fn:a', 'fn:b', 'fn:c', 'fn:singleton']) { + graph.addNode(makeNode(id, id.slice(3))); + } + graph.addNode(makeNode('file:target', 'target', 'File')); + graph.addRelationship(makeRel('rel:ab', 'fn:a', 'fn:b')); + graph.addRelationship(makeRel('rel:bc', 'fn:b', 'fn:c')); + graph.addRelationship(makeRel('rel:ca', 'fn:c', 'fn:a')); + // The symbol enters the projection, but its non-symbol target does not. + // Leiden therefore partitions it into a singleton, which is not emitted. + graph.addRelationship(makeRel('rel:singleton', 'fn:singleton', 'file:target')); + + const result = await processCommunities(graph, undefined, { engine: 'graphology' }); + const communityIds = new Set(result.communities.map((community) => community.id)); + + expect(result.communities).toHaveLength(1); + expect(result.communities[0].symbolCount).toBe(3); + expect(result.stats).toMatchObject({ totalCommunities: 2, nodesProcessed: 4 }); + expect(result.memberships.map((membership) => membership.nodeId).sort()).toEqual([ + 'fn:a', + 'fn:b', + 'fn:c', + ]); + expect(result.memberships.every((membership) => communityIds.has(membership.communityId))).toBe( + true, + ); + expect(result.rawMemberships?.map((membership) => membership.nodeId)).toEqual([ + 'fn:a', + 'fn:b', + 'fn:c', + 'fn:singleton', + ]); + expect( + result.rawMemberships?.filter((membership) => communityIds.has(membership.communityId)), + ).toEqual(result.memberships); + }); + + it('returns no memberships when every detected community is a filtered singleton', async () => { + const graph = createKnowledgeGraph(); + graph.addNode(makeNode('file:target', 'target', 'File')); + for (const id of ['fn:a', 'fn:b']) { + graph.addNode(makeNode(id, id.slice(3))); + graph.addRelationship(makeRel(`rel:${id}`, id, 'file:target')); + } + + const result = await processCommunities(graph, undefined, { engine: 'graphology' }); + + expect(result.communities).toEqual([]); + expect(result.memberships).toEqual([]); + expect(result.rawMemberships).toEqual([ + { nodeId: 'fn:a', communityId: 'comm_0' }, + { nodeId: 'fn:b', communityId: 'comm_1' }, + ]); + expect(result.stats).toMatchObject({ totalCommunities: 2, nodesProcessed: 2 }); + }); +}); diff --git a/gitnexus/test/unit/group/service.test.ts b/gitnexus/test/unit/group/service.test.ts index f56522ef9..770e6889f 100644 --- a/gitnexus/test/unit/group/service.test.ts +++ b/gitnexus/test/unit/group/service.test.ts @@ -2,13 +2,17 @@ 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 { execFileSync } from 'node:child_process'; import { GroupService, type GroupToolPort, type GroupRepoHandle, } from '../../../src/core/group/service.js'; import { writeContractRegistry } from '../../../src/core/group/storage.js'; -import { formatIndexStatusCell } from '../../../src/cli/group-status-format.js'; +import { + formatIndexStatusCell, + type GroupRepoIndexRow, +} from '../../../src/cli/group-status-format.js'; import type { ContractRegistry, StoredContract, CrossLink } from '../../../src/core/group/types.js'; function makeTmpGroup(): { tmpDir: string; groupDir: string; cleanup: () => void } { @@ -717,10 +721,7 @@ repos: const svc = new GroupService(port); const result = (await svc.groupStatus({ name: 'test-group' })) as { - repos: Record< - string, - { indexStale: boolean; commitsBehind?: number; status?: string; missing: boolean } - >; + repos: Record; }; const row = result.repos['app/backend']; @@ -736,5 +737,67 @@ repos: cleanup(); } }); + + it('renders a real rollback as an index that differs from HEAD', async () => { + const { cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const repoPath = path.join(tmpDir, 'rollback'); + fs.mkdirSync(repoPath); + const git = (...args: string[]): string => + execFileSync( + 'git', + [ + '-c', + 'user.email=t@example.com', + '-c', + 'user.name=T', + '-c', + 'commit.gpgsign=false', + ...args, + ], + { cwd: repoPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }, + ).trim(); + git('init', '-q', '--initial-branch=main'); + git('commit', '--allow-empty', '-qm', 'first'); + const firstCommit = git('rev-parse', 'HEAD'); + git('commit', '--allow-empty', '-qm', 'indexed'); + const indexedCommit = git('rev-parse', 'HEAD'); + git('checkout', '-q', '--detach', firstCommit); + + const storagePath = path.join(repoPath, '.gitnexus'); + fs.mkdirSync(storagePath); + fs.writeFileSync( + path.join(storagePath, 'gitnexus.json'), + JSON.stringify({ lastCommit: indexedCommit, indexedAt: '2026-01-01T00:00:00.000Z' }), + ); + const port = makePort({ + resolveRepo: vi.fn( + async (name?: string): Promise => ({ + id: name || 'test', + name: name || 'test', + repoPath, + storagePath, + }), + ), + }); + + const svc = new GroupService(port); + const result = (await svc.groupStatus({ name: 'test-group' })) as { + repos: Record; + }; + const row = result.repos['app/backend']; + expect(row).toMatchObject({ + missing: false, + indexStale: true, + commitsBehind: 0, + status: 'diverged', + }); + expect(formatIndexStatusCell(row)).toBe('STALE (index differs from HEAD)'); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); }); }); diff --git a/gitnexus/test/unit/ignore-service.test.ts b/gitnexus/test/unit/ignore-service.test.ts index 60d6e8f77..307fd5a15 100644 --- a/gitnexus/test/unit/ignore-service.test.ts +++ b/gitnexus/test/unit/ignore-service.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeAll, beforeEach, afterAll, afterEach, vi } import fs from 'fs/promises'; import path from 'path'; import os from 'os'; +import { execFileSync } from 'child_process'; import { shouldIgnorePath, isHardcodedIgnoredDirectory, @@ -687,6 +688,319 @@ describe('createIgnoreFilter', () => { }); }); +describe('createIgnoreFilter with nested .gitignore files (#2675)', () => { + let tmpDir: string; + let originalNoGitignore: string | undefined; + + const asPath = (rel: string) => ({ name: path.basename(rel), relative: () => rel }) as any; + + beforeEach(async () => { + originalNoGitignore = process.env.GITNEXUS_NO_GITIGNORE; + // These tests expect nested rules to apply, so a value inherited from the + // invoking shell must not switch them off. afterEach restores it. + delete process.env.GITNEXUS_NO_GITIGNORE; + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-nested-ignore-test-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + if (originalNoGitignore === undefined) { + delete process.env.GITNEXUS_NO_GITIGNORE; + } else { + process.env.GITNEXUS_NO_GITIGNORE = originalNoGitignore; + } + }); + + it('applies a nested .gitignore relative to its own directory', async () => { + await fs.mkdir(path.join(tmpDir, 'app', 'public', 'generated'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), 'public/generated/\n*.log\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('app/public/generated'))).toBe(true); + expect(filter.ignored(asPath('app/public/generated/bundle.js'))).toBe(true); + expect(filter.ignored(asPath('app/debug.log'))).toBe(true); + expect(filter.ignored(asPath('app/src/deep/debug.log'))).toBe(true); + + // Rules stay scoped to the directory that declares them. + expect(filter.childrenIgnored(asPath('public/generated'))).toBe(false); + expect(filter.ignored(asPath('debug.log'))).toBe(false); + expect(filter.ignored(asPath('other/debug.log'))).toBe(false); + expect(filter.ignored(asPath('app/src/index.ts'))).toBe(false); + }); + + it('preserves independent root exclusions beneath a GitNexus directory negation', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg', 'generated'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), '*.log\n**/pkg/generated/\n'); + await fs.writeFile(path.join(tmpDir, '.gitnexusignore'), '!pkg/\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('pkg'))).toBe(false); + expect(filter.ignored(asPath('pkg/debug.log'))).toBe(true); + expect(filter.childrenIgnored(asPath('pkg/generated'))).toBe(true); + expect(filter.ignored(asPath('pkg/generated/index.ts'))).toBe(true); + expect(filter.ignored(asPath('pkg/index.ts'))).toBe(false); + }); + + it('preserves root inclusions after an unrelated nested directory negation', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg', 'reports', '__tests__'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), '!__tests__/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), '!reports/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', 'reports', '__tests__', 'test.ts'), 'export {};\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('pkg/reports/__tests__'))).toBe(false); + expect(filter.ignored(asPath('pkg/reports/__tests__/test.ts'))).toBe(false); + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + expect((await walkRepositoryPaths(tmpDir)).map((f) => f.path)).toContain( + 'pkg/reports/__tests__/test.ts', + ); + }); + + it('lets a deeper nested negation re-include what an outer nested file ignored', async () => { + await fs.mkdir(path.join(tmpDir, 'app', 'lib'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), '*.gen.ts\n'); + await fs.writeFile(path.join(tmpDir, 'app', 'lib', '.gitignore'), '!keep.gen.ts\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.ignored(asPath('app/lib/keep.gen.ts'))).toBe(false); + expect(filter.ignored(asPath('app/lib/other.gen.ts'))).toBe(true); + }); + + it('lets a nested negation re-include what the root .gitignore ignored', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg', 'reports'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), '*.log\nreports/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), '!keep.log\n!reports/\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.ignored(asPath('pkg/keep.log'))).toBe(false); + expect(filter.ignored(asPath('pkg/other.log'))).toBe(true); + expect(filter.childrenIgnored(asPath('pkg/reports'))).toBe(false); + expect(filter.childrenIgnored(asPath('reports'))).toBe(true); + }); + + it('re-includes the files inside a directory a nested negation un-ignores', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg', 'reports', 'daily'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), 'reports/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), '!reports/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', 'reports', 'summary.ts'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', 'reports', 'daily', 'run.ts'), 'export {};\n'); + await fs.mkdir(path.join(tmpDir, 'reports'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'reports', 'top.ts'), 'export {};\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.ignored(asPath('pkg/reports/summary.ts'))).toBe(false); + expect(filter.childrenIgnored(asPath('pkg/reports/daily'))).toBe(false); + expect(filter.ignored(asPath('pkg/reports/daily/run.ts'))).toBe(false); + expect(filter.ignored(asPath('reports/top.ts'))).toBe(true); + + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + const scanned = (await walkRepositoryPaths(tmpDir)).map((f) => f.path); + + expect(scanned).toContain('pkg/reports/summary.ts'); + expect(scanned).toContain('pkg/reports/daily/run.ts'); + expect(scanned).not.toContain('reports/top.ts'); + }); + + it('keeps independent root exclusions inside a directory re-included by nested rules', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg', 'reports', 'private'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), '*.ts\nreports/\n**/reports/private/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), '!reports/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', 'reports', 'file.ts'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', 'reports', 'keep.js'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', 'reports', 'private', 'secret.js'), 'export {};\n'); + + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + const scanned = (await walkRepositoryPaths(tmpDir)).map((f) => f.path); + + expect(scanned).toContain('pkg/reports/keep.js'); + expect(scanned).not.toContain('pkg/reports/file.ts'); + expect(scanned).not.toContain('pkg/reports/private/secret.js'); + }); + + it('re-includes descendants when a deeper directory negation overrides an outer nested file', async () => { + await fs.mkdir(path.join(tmpDir, 'app', 'pkg', 'reports', 'daily'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), 'reports/\n*.gen.ts\n'); + await fs.writeFile(path.join(tmpDir, 'app', 'pkg', '.gitignore'), '!reports/\n'); + await fs.writeFile(path.join(tmpDir, 'app', 'pkg', 'reports', 'keep.ts'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'app', 'pkg', 'reports', 'drop.gen.ts'), 'export {};\n'); + await fs.writeFile( + path.join(tmpDir, 'app', 'pkg', 'reports', 'daily', 'run.ts'), + 'export {};\n', + ); + + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + const scanned = (await walkRepositoryPaths(tmpDir)).map((f) => f.path); + + expect(scanned).toContain('app/pkg/reports/keep.ts'); + expect(scanned).toContain('app/pkg/reports/daily/run.ts'); + expect(scanned).not.toContain('app/pkg/reports/drop.gen.ts'); + }); + + it.each(['reports [daily]', ...(process.platform === 'win32' ? [] : ['reports\ndaily'])])( + 'keeps re-included directory names literal: %j', + async (directory) => { + await fs.mkdir(path.join(tmpDir, 'pkg', directory), { recursive: true }); + await fs.mkdir(path.join(tmpDir, 'other', directory), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), 'reports*/\n*.ts\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), '!reports*/\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', directory, 'keep.js'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', directory, 'drop.ts'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'other', directory, 'drop.js'), 'export {};\n'); + + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + const scanned = (await walkRepositoryPaths(tmpDir)).map((f) => f.path); + + expect(scanned).toContain(`pkg/${directory}/keep.js`); + expect(scanned).not.toContain(`pkg/${directory}/drop.ts`); + expect(scanned).not.toContain(`other/${directory}/drop.js`); + }, + ); + + it('keeps .gitnexusignore above a nested negation', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), '!keep.log\n'); + await fs.writeFile(path.join(tmpDir, '.gitnexusignore'), 'pkg/keep.log\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.ignored(asPath('pkg/keep.log'))).toBe(true); + }); + + it('keeps an explicit root .gitnexusignore negation in charge', async () => { + await fs.mkdir(path.join(tmpDir, 'app'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), 'generated/\n'); + await fs.writeFile(path.join(tmpDir, '.gitnexusignore'), '!app/generated/\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('app/generated'))).toBe(false); + expect(filter.ignored(asPath('app/generated/schema.ts'))).toBe(false); + }); + + it('lets a nested ignore beat a root .gitignore file negation', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), '*.log\n!pkg/keep.log\n'); + await fs.writeFile(path.join(tmpDir, 'pkg', '.gitignore'), 'keep.log\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.ignored(asPath('pkg/keep.log'))).toBe(true); + }); + + it('lets a nested ignore beat a root .gitignore directory negation', async () => { + await fs.mkdir(path.join(tmpDir, 'app', 'generated'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, '.gitignore'), '!app/generated/\n'); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), 'generated/\n'); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('app/generated'))).toBe(true); + expect(filter.ignored(asPath('app/generated/schema.ts'))).toBe(true); + }); + + it('does not let a nested negation rescue hardcoded defaults', async () => { + await fs.mkdir(path.join(tmpDir, 'pkg', 'node_modules'), { recursive: true }); + await fs.writeFile( + path.join(tmpDir, 'pkg', '.gitignore'), + '!node_modules/\n!package-lock.json\n', + ); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('pkg/node_modules'))).toBe(true); + expect(filter.ignored(asPath('pkg/package-lock.json'))).toBe(true); + }); + + it.skipIf(process.platform === 'win32')('ignores a symlinked nested .gitignore', async () => { + await fs.mkdir(path.join(tmpDir, 'app'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'rules.txt'), '*.log\n'); + await fs.symlink(path.join(tmpDir, 'rules.txt'), path.join(tmpDir, 'app', '.gitignore')); + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.ignored(asPath('app/debug.log'))).toBe(false); + }); + + it('skips nested .gitignore files when GITNEXUS_NO_GITIGNORE is set', async () => { + await fs.mkdir(path.join(tmpDir, 'app'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), 'generated/\n'); + process.env.GITNEXUS_NO_GITIGNORE = '1'; + const filter = await createIgnoreFilter(tmpDir); + + expect(filter.childrenIgnored(asPath('app/generated'))).toBe(false); + }); + + it('prunes nested-ignored files from a real repository walk', async () => { + await fs.mkdir(path.join(tmpDir, 'app', 'src'), { recursive: true }); + await fs.mkdir(path.join(tmpDir, 'app', 'public', 'generated'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'app', '.gitignore'), 'public/generated/\n'); + await fs.writeFile(path.join(tmpDir, 'app', 'src', 'index.ts'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, 'app', 'public', 'generated', 'bundle.js'), 'x;\n'); + + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + const scanned = (await walkRepositoryPaths(tmpDir)).map((f) => f.path); + + expect(scanned).toContain('app/src/index.ts'); + expect(scanned).not.toContain('app/public/generated/bundle.js'); + }); + + it('follows git when a nested file negates a path inside an ignored directory', async () => { + // git cannot re-include a file whose parent directory is excluded, so + // `gen/` + `!gen/keep.ts` leaves keep.ts out, while `gen/*` excludes only + // the contents and lets the negation bring keep.ts back. Root rules behave + // the same way. (`gen`, not `build`: `build` is a hardcoded default.) + const { walkRepositoryPaths } = await import('../../src/core/ingestion/filesystem-walker.js'); + for (const [pkg, rules] of [ + ['dir', 'gen/\n!gen/keep.ts\n'], + ['star', 'gen/*\n!gen/keep.ts\n'], + ]) { + await fs.mkdir(path.join(tmpDir, pkg, 'gen'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, pkg, '.gitignore'), rules); + await fs.writeFile(path.join(tmpDir, pkg, 'gen', 'keep.ts'), 'export {};\n'); + await fs.writeFile(path.join(tmpDir, pkg, 'gen', 'drop.ts'), 'export {};\n'); + } + + const scanned = (await walkRepositoryPaths(tmpDir)).map((f) => f.path); + + expect(scanned).not.toContain('dir/gen/keep.ts'); + expect(scanned).not.toContain('dir/gen/drop.ts'); + expect(scanned).toContain('star/gen/keep.ts'); + expect(scanned).not.toContain('star/gen/drop.ts'); + }); + + it.skipIf(process.platform === 'win32')( + 'does not hang on a nested .gitignore that is a FIFO', + async () => { + // Opening a FIFO for reading blocks until a writer appears. Without + // O_NONBLOCK the walk would stop in open(2), before the regular-file + // check, and never return. + await fs.mkdir(path.join(tmpDir, 'pkg', 'src'), { recursive: true }); + await fs.writeFile(path.join(tmpDir, 'pkg', 'src', 'index.ts'), 'export {};\n'); + execFileSync('mkfifo', [path.join(tmpDir, 'pkg', '.gitignore')]); + + // A timer in this worker cannot interrupt a blocked synchronous open. + // Enforce the deadline outside the process that performs the walk. + const walkerUrl = new URL('../../src/core/ingestion/filesystem-walker.ts', import.meta.url) + .href; + const output = execFileSync( + process.execPath, + [ + '--import', + import.meta.resolve('tsx'), + '--input-type=module', + '--eval', + `import { walkRepositoryPaths } from ${JSON.stringify(walkerUrl)}; + const files = await walkRepositoryPaths(${JSON.stringify(tmpDir)}); + console.log(JSON.stringify(files.map((file) => file.path)));`, + ], + { + encoding: 'utf8', + timeout: 5_000, + killSignal: 'SIGKILL', + env: { ...process.env, GITNEXUS_NO_GLOBAL_IGNORE: '1' }, + }, + ); + + expect(JSON.parse(output)).toContain('pkg/src/index.ts'); + }, + 10_000, + ); +}); + describe('loadIgnoreRules — error handling', () => { let tmpDir: string; diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index b86c19926..fb2916ab4 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -302,8 +302,11 @@ describe('PARSE_CACHE_VERSION', () => { // Moved 121 -> 122 for #3414 restoring helper calls. // Moved 122 -> 123 for #3408 FastAPI nested router-prefix capture fields. // Moved 123 -> 124 for #3402 Go gin/echo decorator routes. - it('pins SCHEMA_BUMP to 125 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354, #3371, #2965, #3390, #3398, #3396, #3394, #3399, #3414, #3408, #3402)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(125); + // Moved 125 -> 126 for #3446: SDK positional tool definitions and attribution. + // Moved 126 -> 127 for #3450: reject destructured SDK registration-method writes. + // Moved 127 -> 128 for #3450: recognize SDK namespace imports. + it('pins SCHEMA_BUMP to 128 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865, #3130, #1432, #3161, #3179, #3219, #3190, #3253, #3273, #3339, #3354, #3371, #2965, #3390, #3398, #3396, #3394, #3399, #3414, #3408, #3402, #3446, #3450)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(128); expect(PARSE_CACHE_BUCKET_COUNT).toBe(128); // The PREVIOUS version must fail the reuse gate, not merely differ from the // current one — a hardcoded number outside the conflict hunk rebases cleanly @@ -313,7 +316,7 @@ describe('PARSE_CACHE_VERSION', () => { 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, - 123, 124, + 123, 124, 125, 126, 127, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); } diff --git a/gitnexus/test/unit/query-degraded-signal.test.ts b/gitnexus/test/unit/query-degraded-signal.test.ts index a7ed807e8..f4c9e763c 100644 --- a/gitnexus/test/unit/query-degraded-signal.test.ts +++ b/gitnexus/test/unit/query-degraded-signal.test.ts @@ -42,6 +42,7 @@ vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { }); import { LocalBackend } from '../../src/mcp/local/local-backend'; +import { resetExtensionState } from '../../src/core/lbug/extension-loader.js'; // A backend whose hybrid search yields exactly one matched symbol, so the // enrichment chunk loop runs and can be made to fail. `ftsUsed` is parameterized @@ -415,3 +416,100 @@ describe('query: degraded-enrichment signal', () => { } }); }); + +describe('query: partial missing FTS indexes', () => { + beforeEach(() => { + vi.clearAllMocks(); + loadMetaMock.mockResolvedValue(null); + executeParameterizedMock.mockResolvedValue([]); + }); + afterEach(() => vi.unstubAllEnvs()); + + it('returns successful symbols with a repair hint and partial flag when another index is missing', async () => { + const b = makeBackend(true) as any; + const resultWithMissing = await b.backend.bm25Search(); + b.backend.bm25Search.mockResolvedValue({ + ...resultWithMissing, + missingIndexes: ['Function.function_fts'], + }); + + const result = await runQuery(b); + + expect(result.definitions.map((d: any) => d.id)).toContain('func:x'); + expect(result.partial).toBe(true); + expect(result.warning).toContain('Function.function_fts'); + expect(result.warning).toContain('repair-fts'); + }); + + it('composes missing indexes with real FTS errors and enrichment failures', async () => { + const b = makeBackend(true, ['connection reset']) as any; + const resultWithMissing = await b.backend.bm25Search(); + b.backend.bm25Search.mockResolvedValue({ + ...resultWithMissing, + missingIndexes: ['Function.function_fts'], + }); + executeParameterizedMock.mockImplementation(async (_repo: string, cypher: string) => { + if (cypher.includes('STEP_IN_PROCESS')) throw new Error('timed out'); + return []; + }); + + const result = await runQuery(b); + + expect(result.partial).toBe(true); + expect(result.warning).toContain('Function.function_fts'); + expect(result.warning).toContain('FTS keyword search partially failed'); + expect(result.warning).toContain('enrichment'); + }); +}); + +it('propagates missing-index diagnostics through the real bm25Search helper into query', async () => { + vi.clearAllMocks(); + loadMetaMock.mockResolvedValue(null); + executeParameterizedMock.mockResolvedValue([]); + const search = await import('../../src/core/search/bm25-index.js'); + const spy = vi.spyOn(search, 'searchFTSFromLbug').mockResolvedValue({ + results: [], + ftsAvailable: true, + missingIndexes: ['Function.function_fts'], + }); + try { + const b = makeBackend(true) as any; + b.backend.bm25Search = (LocalBackend.prototype as any).bm25Search; + const result = await runQuery(b); + expect(result.partial).toBe(true); + expect(result.warning).toContain('Function.function_fts'); + } finally { + spy.mockRestore(); + } +}); + +it('reports missing indexes and real errors when every FTS query fails through the search boundary', async () => { + vi.clearAllMocks(); + resetExtensionState(); + loadMetaMock.mockResolvedValue(null); + executeParameterizedMock.mockImplementation(async (_repo: string, cypher: string) => { + if (cypher.includes("QUERY_FTS_INDEX('Function'")) { + throw new Error( + "Binder exception: Table Function doesn't have an index with name function_fts.", + ); + } + if (cypher.includes('QUERY_FTS_INDEX')) { + throw new Error('connection reset at /home/alice/private/index.lbug'); + } + return []; + }); + const b = makeBackend(false) as any; + b.backend.bm25Search = (LocalBackend.prototype as any).bm25Search; + + const result = await runQuery(b); + + expect(result.definitions).toEqual([]); + expect(result.warning).toContain('FTS keyword search failed'); + expect(result.warning).toContain('connection reset at '); + expect(result.warning).toContain('Function.function_fts'); + expect(result.warning).toContain('gitnexus analyze --repair-fts'); + expect(result.warning).toContain('resolved: repo1'); + expect(result.warning).not.toContain('not a missing-index'); + expect(result.warning).not.toContain('partially failed'); + expect(JSON.stringify(result)).not.toContain('/home/alice'); +}); diff --git a/gitnexus/test/unit/query-fts-missing-index.test.ts b/gitnexus/test/unit/query-fts-missing-index.test.ts new file mode 100644 index 000000000..aa50a50bf --- /dev/null +++ b/gitnexus/test/unit/query-fts-missing-index.test.ts @@ -0,0 +1,114 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +const native = vi.hoisted(() => ({ + error: undefined as string | undefined, + params: [] as unknown[], +})); + +// Control the native statement result, while exercising the real adapter's +// connection lock, prepared-query path and missing-index error handling. +vi.mock('@ladybugdb/core', () => { + const emptyResult = { + getAll: async () => [], + close: async () => {}, + isSuccess: () => true, + getErrorMessage: async () => '', + }; + class Database { + async init() {} + async close() {} + } + class Connection { + async query() { + return emptyResult; + } + async prepare() { + return { + isSuccess: () => native.error === undefined, + getErrorMessage: async () => native.error ?? '', + }; + } + async execute(_statement: unknown, params: unknown) { + native.params.push(params); + return emptyResult; + } + async close() {} + } + const mod = { Database, Connection }; + return { ...mod, default: mod, lbug: mod }; +}); + +import { closeLbug, queryFTS, withLbugDb } from '../../src/core/lbug/lbug-adapter.js'; + +const MISSING = "Binder exception: Table Function doesn't have an index with name function_fts."; + +describe('queryFTS missing-index diagnostics', () => { + let dir: string; + let dbPath: string; + beforeEach(async () => { + native.error = undefined; + native.params = []; + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-test-')); + dbPath = path.join(dir, 'lbug'); + await fs.writeFile(dbPath, ''); + }); + afterEach(async () => { + await closeLbug(); + await fs.rm(dir, { recursive: true, force: true }); + }); + + it('retains the existing empty-result default for a missing index', async () => { + await withLbugDb( + dbPath, + async () => { + native.error = MISSING; + expect(await queryFTS('Function', 'function_fts', 'auth')).toEqual([]); + }, + { readOnly: true, skipFts: true }, + ); + }); + + it('can propagate a missing index so search does not count it as a successful zero-match query', async () => { + await withLbugDb( + dbPath, + async () => { + native.error = MISSING; + await expect( + queryFTS('Function', 'function_fts', 'auth', 5, false, 'throw'), + ).rejects.toThrow(MISSING); + }, + { readOnly: true, skipFts: true }, + ); + }); + + it('preserves prepared query binding and successful empty results in diagnostic mode', async () => { + await withLbugDb( + dbPath, + async () => { + const query = "auth' DELETE n"; + expect(await queryFTS('Function', 'function_fts', query, 5, false, 'throw')).toEqual([]); + expect(native.params).toEqual([{ query }]); + }, + { readOnly: true, skipFts: true }, + ); + }); + + it('still propagates real failures in either missing-index mode', async () => { + await withLbugDb( + dbPath, + async () => { + native.error = 'Runtime exception: connection reset'; + await expect(queryFTS('Function', 'function_fts', 'auth')).rejects.toThrow( + 'connection reset', + ); + await expect( + queryFTS('Function', 'function_fts', 'auth', 5, false, 'throw'), + ).rejects.toThrow('connection reset'); + }, + { readOnly: true, skipFts: true }, + ); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/swift/callable-visibility.test.ts b/gitnexus/test/unit/scope-resolution/swift/callable-visibility.test.ts new file mode 100644 index 000000000..7cc8daed5 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/swift/callable-visibility.test.ts @@ -0,0 +1,267 @@ +import { + buildDefIndex, + buildMethodDispatchIndex, + buildQualifiedNameIndex, + buildScopeTree, + type Scope, + type SymbolDefinition, +} from 'gitnexus-shared'; +import { describe, expect, it } from 'vitest'; +import { swiftIsCallableVisibleFromCaller } from '../../../../src/core/ingestion/languages/swift/callable-visibility.js'; +import type { ScopeResolutionIndexes } from '../../../../src/core/ingestion/model/scope-resolution-indexes.js'; + +const filePath = 'Service.swift'; +const moduleRange = { startLine: 1, startCol: 0, endLine: 12, endCol: 0 }; +const classRange = { startLine: 2, startCol: 0, endLine: 10, endCol: 0 }; +const functionRange = { startLine: 5, startCol: 0, endLine: 8, endCol: 0 }; + +function scope( + id: string, + parent: string | null, + kind: Scope['kind'], + ownedDefs: SymbolDefinition[], + range: Scope['range'], +): Scope { + return { + id, + parent, + kind, + range, + filePath, + ownedDefs, + bindings: new Map(), + imports: [], + typeBindings: new Map(), + }; +} + +const classDef: SymbolDefinition = { + nodeId: 'Service', + filePath, + type: 'Class', + qualifiedName: 'Service', +}; +const property: SymbolDefinition = { + nodeId: 'Service.clock', + filePath, + type: 'Property', + qualifiedName: 'Service.clock', + ownerId: 'Service', +}; +const method: SymbolDefinition = { + nodeId: 'Service.refresh', + filePath, + type: 'Method', + qualifiedName: 'Service.refresh', + ownerId: 'Service', +}; +const scopes = { + scopeTree: buildScopeTree([ + scope('module', null, 'Module', [], moduleRange), + scope('class', 'module', 'Class', [classDef, property], classRange), + scope('function', 'class', 'Function', [method], functionRange), + ]), + defs: buildDefIndex([classDef, property, method]), + qualifiedNames: buildQualifiedNameIndex([classDef, property, method]), + methodDispatch: buildMethodDispatchIndex({ + owners: [classDef.nodeId], + computeMro: () => [], + implementsOf: () => [], + }), +} as ScopeResolutionIndexes; + +describe('Swift caller-side callable visibility', () => { + it('rejects an unrelated same-name method shadowed by a stored property', () => { + expect( + swiftIsCallableVisibleFromCaller({ + candidate: { + nodeId: 'Other.clock', + filePath: 'Other.swift', + type: 'Method', + qualifiedName: 'Other.clock', + }, + callerScope: 'function', + callArity: 0, + scopes, + }), + ).toBe(false); + }); + + it('keeps differently named methods and unknown caller scopes eligible', () => { + const candidate: SymbolDefinition = { + nodeId: 'Other.run', + filePath: 'Other.swift', + type: 'Method', + qualifiedName: 'Other.run', + }; + expect( + swiftIsCallableVisibleFromCaller({ + candidate, + callerScope: 'function', + callArity: 0, + scopes, + }), + ).toBe(true); + expect( + swiftIsCallableVisibleFromCaller({ + candidate: { ...candidate, qualifiedName: 'Other.clock' }, + }), + ).toBe(true); + }); + + it('preserves a selected local function before checking the enclosing property', () => { + const local: SymbolDefinition = { + nodeId: 'local.clock', + filePath, + type: 'Function', + qualifiedName: 'Service.refresh.clock', + }; + const localScopes = { + ...scopes, + scopeTree: buildScopeTree([ + scope('module', null, 'Module', [], moduleRange), + scope('class', 'module', 'Class', [classDef, property], classRange), + scope('function', 'class', 'Function', [method], functionRange), + scope('local', 'function', 'Function', [local], { + startLine: 6, + startCol: 0, + endLine: 7, + endCol: 0, + }), + ]), + } as ScopeResolutionIndexes; + expect( + swiftIsCallableVisibleFromCaller({ + candidate: local, + callerParsed: { + filePath, + moduleScope: 'module', + scopes: [...localScopes.scopeTree.byId.values()], + localDefs: [classDef, property, method, local], + parsedImports: [], + referenceSites: [], + }, + callerScope: 'function', + callArity: 0, + scopes: localScopes, + }), + ).toBe(true); + }); + + it('preserves a selected method owned by the current type', () => { + expect( + swiftIsCallableVisibleFromCaller({ + candidate: { ...method, nodeId: 'Service.clock', qualifiedName: 'Service.clock' }, + callerScope: 'function', + callArity: 0, + scopes, + }), + ).toBe(true); + }); + + it('does not veto a call with arguments or unknown arity', () => { + const candidate: SymbolDefinition = { + nodeId: 'Other.clock', + filePath: 'Other.swift', + type: 'Method', + qualifiedName: 'Other.clock', + }; + expect( + swiftIsCallableVisibleFromCaller({ + candidate, + callerScope: 'function', + callArity: 1, + scopes, + }), + ).toBe(true); + expect(swiftIsCallableVisibleFromCaller({ candidate, callerScope: 'function', scopes })).toBe( + true, + ); + }); + + it('rejects a decoy when a superclass owns the closure property', () => { + const base: SymbolDefinition = { + nodeId: 'BaseService', + filePath, + type: 'Class', + qualifiedName: 'BaseService', + }; + const derived: SymbolDefinition = { + nodeId: 'DerivedService', + filePath, + type: 'Class', + qualifiedName: 'DerivedService', + }; + const inheritedProperty: SymbolDefinition = { + nodeId: 'BaseService.clock', + filePath, + type: 'Property', + qualifiedName: 'BaseService.clock', + ownerId: base.nodeId, + }; + const inheritedScopes = { + scopeTree: buildScopeTree([ + scope('derivedModule', null, 'Module', [], moduleRange), + scope('derivedClass', 'derivedModule', 'Class', [derived], classRange), + scope('derivedFunction', 'derivedClass', 'Function', [method], functionRange), + ]), + defs: buildDefIndex([base, derived, inheritedProperty]), + qualifiedNames: buildQualifiedNameIndex([base, derived, inheritedProperty]), + methodDispatch: buildMethodDispatchIndex({ + owners: [derived.nodeId], + computeMro: () => [base.nodeId], + implementsOf: () => [], + }), + } as ScopeResolutionIndexes; + expect( + swiftIsCallableVisibleFromCaller({ + candidate: { + nodeId: 'Other.clock', + filePath: 'Other.swift', + type: 'Method', + qualifiedName: 'Other.clock', + }, + callerScope: 'derivedFunction', + callArity: 0, + scopes: inheritedScopes, + }), + ).toBe(false); + + const extensionScopes = { + ...inheritedScopes, + bindings: new Map([ + [ + 'extensionModule', + new Map([['DerivedService', [{ def: derived, origin: 'local' as const }]]]), + ], + ]), + bindingAugmentations: new Map(), + scopeTree: buildScopeTree([ + scope('extensionModule', null, 'Module', [], moduleRange), + scope('extensionClass', 'extensionModule', 'Class', [], classRange), + { + ...scope('extensionFunction', 'extensionClass', 'Function', [method], functionRange), + typeBindings: new Map([ + [ + 'self', + { rawName: 'DerivedService', declaredAtScope: 'extensionFunction', source: 'self' }, + ], + ]), + }, + ]), + } as ScopeResolutionIndexes; + expect( + swiftIsCallableVisibleFromCaller({ + candidate: { + nodeId: 'Other.clock', + filePath: 'Other.swift', + type: 'Method', + qualifiedName: 'Other.clock', + }, + callerScope: 'extensionFunction', + callArity: 0, + scopes: extensionScopes, + }), + ).toBe(false); + }); +}); diff --git a/gitnexus/test/unit/skill-gen.test.ts b/gitnexus/test/unit/skill-gen.test.ts index 0ffb60ec0..57167ce20 100644 --- a/gitnexus/test/unit/skill-gen.test.ts +++ b/gitnexus/test/unit/skill-gen.test.ts @@ -22,6 +22,9 @@ import type { ProcessDetectionResult, } from '../../src/core/ingestion/process-processor.js'; import type { PipelineResult } from '../../src/types/pipeline.js'; +import { communitiesPhase } from '../../src/core/ingestion/pipeline-phases/communities.js'; +import { processesPhase } from '../../src/core/ingestion/pipeline-phases/processes.js'; +import type { PhaseResult } from '../../src/core/ingestion/pipeline-phases/types.js'; // ============================================================================ // FIXTURE HELPERS @@ -135,6 +138,25 @@ function buildPipelineResult(opts: { }; } +/** Run the real community phase, including graph node and membership edge emission. */ +async function detectCommunities(graph: KnowledgeGraph, repoPath: string): Promise { + const { communityResult } = await communitiesPhase.execute( + { graph, repoPath, onProgress: () => {}, pipelineStart: Date.now() }, + new Map([['structure', { phaseName: 'structure', output: { totalFiles: 0 }, durationMs: 0 }]]), + ); + return { + graph, + repoPath, + totalFileCount: 0, + communityResult, + resolutionOutcomes: [], + usedWorkerPool: false, + reparsedFileCount: 0, + scopeExtractionFailures: [], + unavailableScopeLanguageFiles: 0, + }; +} + // ============================================================================ // TESTS — RETURN VALUES // ============================================================================ @@ -407,6 +429,179 @@ describe('generateSkillFiles — return values', () => { expect(result.skills[0].label).toBe('Auth'); }); + it('generates a folder skill from real singleton assignments without dangling membership edges', async () => { + const graph = createKnowledgeGraph(); + graph.addNode(makeNode('file:target', 'target', 'File', `${tmpDir}/target.ts`, 1, false)); + for (const name of ['gamma', 'alpha', 'beta']) { + graph.addNode( + makeNode(`fn:${name}`, name, 'Function', `${tmpDir}/src/auth/${name}.ts`, 1, true), + ); + // The File target admits the symbol to the projection, then is excluded + // itself, leaving a singleton in the real Leiden result. + graph.addRelationship(makeRel(`rel:${name}`, `fn:${name}`, 'file:target', 'CALLS')); + } + + const pipeline = await detectCommunities(graph, tmpDir); + const result = await generateSkillFiles(tmpDir, 'TestProject', pipeline); + + expect(result.skills).toHaveLength(1); + expect(result.skills[0]).toMatchObject({ + name: 'gitnexus-area-auth', + label: 'Auth', + symbolCount: 3, + fileCount: 3, + }); + expect(pipeline.communityResult?.communities).toEqual([]); + expect(pipeline.communityResult?.memberships).toEqual([]); + expect(pipeline.communityResult?.rawMemberships).toEqual([ + { nodeId: 'fn:alpha', communityId: 'comm_0' }, + { nodeId: 'fn:beta', communityId: 'comm_1' }, + { nodeId: 'fn:gamma', communityId: 'comm_2' }, + ]); + expect([...graph.iterRelationships()].filter((rel) => rel.type === 'MEMBER_OF')).toEqual([]); + const content = await fs.readFile( + path.join(result.outputPath, result.skills[0].name, 'SKILL.md'), + 'utf-8', + ); + for (const name of ['alpha', 'beta', 'gamma']) { + expect(content).toContain(name); + expect(content).toContain(`src/auth/${name}.ts`); + } + }); + + it('preserves execution flows for real singleton fallback skills', async () => { + const graph = createKnowledgeGraph(); + // Large-graph projection prunes the degree-one endpoints of each chain, + // leaving only its middle function as a singleton community. + for (let i = 0; i < 10_001; i++) { + graph.addNode(makeNode(`fn:unused${i}`, `unused${i}`, 'Function', 'src/unused.ts', 1, false)); + } + for (let i = 0; i < 4; i++) { + const folder = i < 3 ? 'auth' : 'billing'; + for (const role of ['handle', 'middle', 'end']) { + const name = `${role}${i}`; + graph.addNode( + makeNode( + `fn:${name}`, + name, + 'Function', + `${tmpDir}/src/${folder}/${name}.ts`, + 1, + role === 'handle', + ), + ); + } + graph.addRelationship(makeRel(`rel:handle${i}`, `fn:handle${i}`, `fn:middle${i}`, 'CALLS')); + graph.addRelationship(makeRel(`rel:middle${i}`, `fn:middle${i}`, `fn:end${i}`, 'CALLS')); + } + + const pipeline = await detectCommunities(graph, tmpDir); + const { processResult } = await processesPhase.execute( + { graph, repoPath: tmpDir, onProgress: () => {}, pipelineStart: Date.now() }, + new Map>([ + ['structure', { phaseName: 'structure', output: { totalFiles: 0 }, durationMs: 0 }], + [ + 'communities', + { + phaseName: 'communities', + output: { communityResult: pipeline.communityResult }, + durationMs: 0, + }, + ], + ['routes', { phaseName: 'routes', output: { routeRegistry: new Map() }, durationMs: 0 }], + ['tools', { phaseName: 'tools', output: { toolDefs: [] }, durationMs: 0 }], + ]), + ); + pipeline.processResult = processResult; + + expect(pipeline.communityResult?.communities).toEqual([]); + expect(pipeline.communityResult?.memberships).toEqual([]); + expect(pipeline.communityResult?.rawMemberships).toHaveLength(4); + expect(processResult.processes).toHaveLength(4); + expect(processResult.processes.every((process) => process.communities.length === 0)).toBe(true); + expect([...graph.iterRelationships()].filter((rel) => rel.type === 'MEMBER_OF')).toEqual([]); + + const result = await generateSkillFiles(tmpDir, 'TestProject', pipeline); + expect(result.skills).toHaveLength(1); + expect(result.skills[0]).toMatchObject({ label: 'Auth', symbolCount: 3 }); + const content = await fs.readFile( + path.join(result.outputPath, result.skills[0].name, 'SKILL.md'), + 'utf-8', + ); + expect(content).toContain('## Execution Flows'); + for (const process of processResult.processes) { + if (process.entryPointId === 'fn:handle3') { + expect(content).not.toContain(process.heuristicLabel); + } else { + expect(content).toContain(process.heuristicLabel); + } + } + }); + + it.each([0, 2])('skips real singleton fallback below threshold (%i symbols)', async (count) => { + const graph = createKnowledgeGraph(); + if (count > 0) { + graph.addNode(makeNode('file:target', 'target', 'File', `${tmpDir}/target.ts`, 1, false)); + } + for (let i = 0; i < count; i++) { + graph.addNode( + makeNode(`fn:n${i}`, `n${i}`, 'Function', `${tmpDir}/src/auth/f${i}.ts`, 1, true), + ); + graph.addRelationship(makeRel(`rel:${i}`, `fn:n${i}`, 'file:target', 'CALLS')); + } + + const pipeline = await detectCommunities(graph, tmpDir); + const result = await generateSkillFiles(tmpDir, 'TestProject', pipeline); + + expect(result.skills).toEqual([]); + expect(pipeline.communityResult?.rawMemberships).toHaveLength(count); + expect(pipeline.communityResult?.memberships).toEqual([]); + expect([...graph.iterRelationships()].filter((rel) => rel.type === 'MEMBER_OF')).toEqual([]); + }); + + it('keeps real retained skills and membership edges separate from filtered singletons', async () => { + const graph = createKnowledgeGraph(); + for (const name of ['alpha', 'beta', 'gamma', 'singleton']) { + graph.addNode( + makeNode(`fn:${name}`, name, 'Function', `${tmpDir}/src/auth/${name}.ts`, 1, true), + ); + } + graph.addNode(makeNode('file:target', 'target', 'File', `${tmpDir}/target.ts`, 1, false)); + graph.addRelationship(makeRel('rel:ab', 'fn:alpha', 'fn:beta', 'CALLS')); + graph.addRelationship(makeRel('rel:bc', 'fn:beta', 'fn:gamma', 'CALLS')); + graph.addRelationship(makeRel('rel:ca', 'fn:gamma', 'fn:alpha', 'CALLS')); + graph.addRelationship(makeRel('rel:singleton', 'fn:singleton', 'file:target', 'CALLS')); + + const pipeline = await detectCommunities(graph, tmpDir); + const result = await generateSkillFiles(tmpDir, 'TestProject', pipeline); + + expect(result.skills).toHaveLength(1); + expect(result.skills[0]).toMatchObject({ label: 'Auth', symbolCount: 3, fileCount: 3 }); + expect(pipeline.communityResult?.rawMemberships).toHaveLength(4); + expect(pipeline.communityResult?.memberships.map((membership) => membership.nodeId)).toEqual([ + 'fn:alpha', + 'fn:beta', + 'fn:gamma', + ]); + const membershipEdges = [...graph.iterRelationships()].filter( + (rel) => rel.type === 'MEMBER_OF', + ); + expect(membershipEdges).toHaveLength(3); + for (const edge of membershipEdges) { + expect(graph.getNode(edge.sourceId)).toBeDefined(); + expect(graph.getNode(edge.targetId)?.label).toBe('Community'); + expect(edge.sourceId).not.toBe('fn:singleton'); + } + const content = await fs.readFile( + path.join(result.outputPath, result.skills[0].name, 'SKILL.md'), + 'utf-8', + ); + expect(content).not.toContain('singleton'); + for (const name of ['alpha', 'beta', 'gamma']) { + expect(content).toContain(`src/auth/${name}.ts`); + } + }); + /** * When processResult is undefined, the generator should still work * without crashing — it simply has no execution flows. diff --git a/gitnexus/test/unit/staleness-fallback.test.ts b/gitnexus/test/unit/staleness-fallback.test.ts index 427c52b40..baed8e13b 100644 --- a/gitnexus/test/unit/staleness-fallback.test.ts +++ b/gitnexus/test/unit/staleness-fallback.test.ts @@ -3,7 +3,7 @@ * OTHER than a timeout. `staleness.test.ts` reaches `diverged` and `unknown` * against real repositories, but not the third arm of `fromHead`: HEAD still * resolves to the indexed commit, so the index is at HEAD however `rev-list` - * failed. Real git cannot fail `..HEAD` while HEAD prints that same SHA + * failed. Real git cannot fail `...HEAD` while HEAD prints that same SHA * without a corrupted object store, so this drives it through a mock. * * Its own file for the same reason as `staleness-timeout.test.ts`: the mock @@ -12,14 +12,21 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const { plan, spawnedArgs } = vi.hoisted(() => ({ - plan: { head: null as string | null }, + plan: { head: null as string | null, enoent: false }, spawnedArgs: [] as string[][], })); // `rev-list` exits 128 the way git does for a missing object; `rev-parse HEAD` -// answers `plan.head`, or fails when it is null. +// answers `plan.head`, or fails when it is null. `plan.enoent` instead fails +// every invocation the way Node reports a missing executable (git not on +// PATH) — no exit code, `code: 'ENOENT'` — to check that shape of failure is +// caught by the same `catch` as a present-but-failing git (#3127). const answer = (args: readonly string[]): { error: Error | null; stdout: string } => { spawnedArgs.push([...args]); + if (plan.enoent) { + const failure = Object.assign(new Error('spawn git ENOENT'), { code: 'ENOENT' }); + return { error: failure, stdout: '' }; + } if (args[0] === 'rev-parse' && plan.head) return { error: null, stdout: `${plan.head}\n` }; const failure = Object.assign(new Error(`Command failed: git ${args.join(' ')}`), { code: 128, @@ -54,7 +61,7 @@ vi.mock('node:child_process', async (importOriginal) => { import { checkStaleness, checkStalenessAsync } from '../../src/core/git-staleness.js'; const INDEXED_COMMIT = 'a'.repeat(40); -const REV_LIST = ['rev-list', '--count', `${INDEXED_COMMIT}..HEAD`]; +const REV_LIST = ['rev-list', '--left-right', '--count', `${INDEXED_COMMIT}...HEAD`]; const REV_PARSE = ['rev-parse', 'HEAD']; const bothHelpers = { @@ -65,6 +72,7 @@ const bothHelpers = { describe('staleness after a failed (not timed-out) rev-list (#3256)', () => { beforeEach(() => { plan.head = null; + plan.enoent = false; spawnedArgs.length = 0; }); @@ -95,6 +103,19 @@ describe('staleness after a failed (not timed-out) rev-list (#3256)', () => { expect(result).toEqual({ isStale: false, commitsBehind: 0, status: 'unknown' }); expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]); }); + + it('reports unknown — never current — when git itself is not on PATH', async () => { + // nikolai-vysotskyi (#3127): "git not on PATH" specifically, as a + // distinct failure shape (ENOENT, no exit code) from a present git + // that fails against a bad ref. Same requirement either way: it must + // never be silently read as fresh. + plan.enoent = true; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ isStale: false, commitsBehind: 0, status: 'unknown' }); + expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]); + }); }); } }); diff --git a/gitnexus/test/unit/staleness-timeout.test.ts b/gitnexus/test/unit/staleness-timeout.test.ts index 1b80df5b8..6f4670135 100644 --- a/gitnexus/test/unit/staleness-timeout.test.ts +++ b/gitnexus/test/unit/staleness-timeout.test.ts @@ -43,6 +43,8 @@ describe('checkStalenessAsync — timed-out rev-list (#3256)', () => { expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 }); // Exactly one spawn. A `rev-parse HEAD` follow-up here is the regression: // it doubles the hung-mount bound and can flip this answer. - expect(spawnedArgs).toEqual([['rev-list', '--count', `${INDEXED_COMMIT}..HEAD`]]); + expect(spawnedArgs).toEqual([ + ['rev-list', '--left-right', '--count', `${INDEXED_COMMIT}...HEAD`], + ]); }); }); diff --git a/gitnexus/test/unit/staleness-zero-count.test.ts b/gitnexus/test/unit/staleness-zero-count.test.ts new file mode 100644 index 000000000..4e2b510b7 --- /dev/null +++ b/gitnexus/test/unit/staleness-zero-count.test.ts @@ -0,0 +1,162 @@ +/** + * Count both sides in one Git process to distinguish freshness from rollback + * without mixing HEAD snapshots (#3127). + * Keep the child-process mock isolated from tests that use real repositories. + */ +import type { ExecFileOptions } from 'node:child_process'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { plan, invocations } = vi.hoisted(() => ({ + plan: { counts: '0\t0\n' as string | null, head: 'failure', advanceHead: false }, + invocations: [] as { args: string[]; options: ExecFileOptions }[], +})); + +const answer = (args: readonly string[], options: ExecFileOptions): string => { + invocations.push({ args: [...args], options }); + if (args[0] === 'rev-list') { + if (plan.counts === null) { + throw Object.assign(new Error('Command failed: git rev-list'), { code: 128, killed: false }); + } + // Simulate a checkout/commit after Git measured the relationship. A second + // process would see this different HEAD and could misclassify the result. + if (plan.advanceHead) plan.head = 'b'.repeat(40); + return plan.counts; + } + if (plan.head === 'empty') return ' \n'; + if (plan.head === 'timeout') { + throw Object.assign(new Error('Command failed: git rev-parse HEAD'), { + killed: true, + signal: 'SIGTERM', + }); + } + if (plan.head === 'failure') { + throw Object.assign(new Error('Command failed: git rev-parse HEAD'), { + code: 128, + killed: false, + }); + } + return `${plan.head}\n`; +}; + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + const { promisify } = await import('node:util'); + const execFile = ( + _file: string, + args: readonly string[], + options: ExecFileOptions, + callback: (error: Error | null, stdout: string, stderr: string) => void, + ): void => { + try { + callback(null, answer(args, options), ''); + } catch (error) { + callback(error as Error, '', ''); + } + }; + // Real execFile has a custom promisifier returning both output streams. + Object.defineProperty(execFile, promisify.custom, { + value: async (_file: string, args: readonly string[], options: ExecFileOptions) => ({ + stdout: answer(args, options), + stderr: '', + }), + }); + const execFileSync = (_file: string, args: readonly string[], options: ExecFileOptions): string => + answer(args, options); + return { + ...actual, + execFile: execFile as unknown as typeof actual.execFile, + execFileSync: execFileSync as unknown as typeof actual.execFileSync, + }; +}); + +import { checkStaleness, checkStalenessAsync } from '../../src/core/git-staleness.js'; + +const INDEXED_COMMIT = 'a'.repeat(40); +const REV_LIST = ['rev-list', '--left-right', '--count', `${INDEXED_COMMIT}...HEAD`]; +const REV_PARSE = ['rev-parse', 'HEAD']; + +const bothHelpers = { + checkStaleness: async (repo: string, lastCommit: string) => checkStaleness(repo, lastCommit), + checkStalenessAsync, +}; + +describe('staleness from one relationship query (#3127)', () => { + beforeEach(() => { + plan.counts = '0\t0\n'; + plan.head = 'failure'; + plan.advanceHead = false; + invocations.length = 0; + }); + + for (const [name, check] of Object.entries(bothHelpers)) { + describe(name, () => { + it.each([ + { counts: '0\t0\n', status: 'current', isStale: false, commitsBehind: 0 }, + { counts: '2\t0\n', status: 'diverged', isStale: true, commitsBehind: 0 }, + { counts: '0\t3\n', status: 'behind', isStale: true, commitsBehind: 3 }, + { counts: '2\t3\n', status: 'behind', isStale: true, commitsBehind: 3 }, + ])('reports $status from $counts in one process', async ({ counts, ...expected }) => { + plan.counts = counts; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toMatchObject(expected); + expect(invocations.map(({ args }) => args)).toEqual([REV_LIST]); + }); + + it('keeps the count snapshot when HEAD advances after the query', async () => { + plan.head = INDEXED_COMMIT; + plan.advanceHead = true; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(plan.head).not.toBe(INDEXED_COMMIT); + expect(result).toEqual({ status: 'current', isStale: false, commitsBehind: 0 }); + expect(invocations.map(({ args }) => args)).toEqual([REV_LIST]); + }); + + it.each(['', '0\n', '0\tbogus\n'])( + 'reports unknown for malformed counts %j', + async (counts) => { + plan.counts = counts; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 }); + expect(invocations.map(({ args }) => args)).toEqual([REV_LIST]); + }, + ); + + it('reports unknown when the follow-up HEAD command fails', async () => { + plan.counts = null; + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 }); + expect(invocations.map(({ args }) => args)).toEqual([REV_LIST, REV_PARSE]); + }); + + it('reports unknown when the follow-up HEAD command returns no commit', async () => { + plan.counts = null; + plan.head = 'empty'; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 }); + expect(invocations.map(({ args }) => args)).toEqual([REV_LIST, REV_PARSE]); + }); + + it('bounds the HEAD command and reports unknown without retrying after its timeout', async () => { + plan.counts = null; + plan.head = 'timeout'; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 }); + expect(invocations.map(({ args }) => args)).toEqual([REV_LIST, REV_PARSE]); + const timeout = invocations[1].options.timeout; + expect(Number.isFinite(timeout)).toBe(true); + expect(timeout).toBeGreaterThan(0); + }); + }); + } +}); diff --git a/gitnexus/test/unit/staleness.test.ts b/gitnexus/test/unit/staleness.test.ts index ef4a6a517..0e6084d7e 100644 --- a/gitnexus/test/unit/staleness.test.ts +++ b/gitnexus/test/unit/staleness.test.ts @@ -335,3 +335,52 @@ describe('branch-pinned serve clone once git prunes the indexed commit (#3256)', }); }); }); + +// ── #3127: a 0 forward count is not "the index matches this tree" ─────────── +// +// nikolai-vysotskyi (issue #3127, comment on the `--stale-policy` proposal): +// `git rev-list --count lastCommit..HEAD` answers "commits reachable from +// HEAD but not lastCommit", which is also 0 when HEAD is an *ancestor* of +// lastCommit — i.e. the working tree checked out an older commit than the one +// indexed, or a release branch behind the indexed tip. Before this fix that +// read as `current`; a `--stale-policy error`-style caller would exit 0 +// exactly when the index is provably wrong about the checked-out tree. +describe('staleness when the checkout has regressed behind the indexed commit (#3127)', () => { + let root: string; + let fixture: ReturnType; + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'gitnexus-staleness-regressed-')); + fixture = makeRepo(root, 'repo'); + // Roll the working tree back to c1. `rev-list --count c3..HEAD` alone + // answers 0 here (HEAD/c1 has no commits c3 lacks) — exactly the count a + // pre-fix caller read as "index matches HEAD". + git(fixture.repo, 'checkout', '-q', fixture.c1); + }); + afterAll(() => removeTree(root)); + + for (const [name, check] of Object.entries(bothHelpers)) { + describe(name, () => { + it('reports diverged, not current, when HEAD is behind the indexed commit', async () => { + const result = await check(fixture.repo, fixture.c3); + + expect(result.status).toBe('diverged'); + // Established by a positive indexed-only count and a zero HEAD-only + // count (not a `rev-list` failure), so — unlike the failure-path + // `diverged` above — `isStale` reflects the mismatch instead of the + // historical fail-open `false`. + expect(result.isStale).toBe(true); + expect(result.commitsBehind).toBe(0); + expect(result.hint).toContain('not reachable from the checked-out commit'); + }); + + it('still reports current for the commit actually checked out', async () => { + expect(await check(fixture.repo, fixture.c1)).toMatchObject({ + status: 'current', + isStale: false, + commitsBehind: 0, + }); + }); + }); + } +}); diff --git a/gitnexus/test/unit/symbol-identity-validation.test.ts b/gitnexus/test/unit/symbol-identity-validation.test.ts new file mode 100644 index 000000000..7fe380a20 --- /dev/null +++ b/gitnexus/test/unit/symbol-identity-validation.test.ts @@ -0,0 +1,234 @@ +/** Regression for unusable database lookup identities (#3424). */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +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', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + listRegisteredRepos: vi.fn().mockResolvedValue([ + { + name: 'test-project', + path: '/tmp/test-project', + storagePath: '/tmp/.gitnexus/test-project', + indexedAt: '2024-06-01T12:00:00Z', + lastCommit: 'abc123', + stats: { files: 10, nodes: 50, edges: 100, communities: 3, processes: 5 }, + }, + ]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), + }; +}); + +vi.mock('../../src/core/git-staleness.js', () => ({ + checkStaleness: vi.fn().mockReturnValue({ isStale: false, commitsBehind: 0 }), + checkStalenessAsync: vi.fn().mockResolvedValue({ isStale: false, commitsBehind: 0 }), + checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), +})); + +vi.mock('../../src/storage/git.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getGitRoot: vi.fn().mockReturnValue(null) }; +}); + +vi.mock('../../src/core/search/bm25-index.js', () => ({ + searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), +})); + +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 { executeParameterized } from '../../src/mcp/core/lbug-adapter.js'; + +const SYMBOL = { + id: 'Method:tests/test_supervisor.py:Supervisor.test_run', + name: 'test_run', + type: 'Method', + filePath: 'tests/test_supervisor.py', + startLine: 3, + endLine: 5, +}; + +const badIds = [ + ['missing', undefined], + ['null', null], + ['number', 42], + ['empty', ''], + ['blank', ' \t '], + ['NUL-only', '\0\0'], + ['embedded NUL', 'Method:tests/test_supervisor.py:\0test_run'], +] as const; + +function row(id: unknown, shape: string): unknown { + return shape === 'tuple' + ? [id, SYMBOL.name, SYMBOL.type, SYMBOL.filePath, SYMBOL.startLine, SYMBOL.endLine] + : { ...SYMBOL, id }; +} + +const surfaces = [ + { tool: 'context', params: { name: SYMBOL.name, file_path: SYMBOL.filePath } }, + { tool: 'impact', params: { target: SYMBOL.name, direction: 'upstream' } }, + { tool: 'impact', params: { target: SYMBOL.name, direction: 'upstream', mode: 'pdg' } }, +] as const; + +describe('symbol lookup identity validation (#3424)', () => { + let backend: LocalBackend; + + beforeEach(async () => { + vi.clearAllMocks(); + backend = new LocalBackend(); + await backend.init(); + (backend as any).ensureInitialized = vi.fn().mockResolvedValue(undefined); + }); + + function lookupRows(rows: unknown[]) { + vi.mocked(executeParameterized).mockImplementation(async (_db, _query, params) => { + return params?.symName || params?.uid ? rows : []; + }); + } + + function expectNoExpansion() { + const queries = vi.mocked(executeParameterized).mock.calls.map(([, query]) => query); + expect(queries.length).toBeGreaterThan(0); + expect(queries.every((query) => !query.includes('CodeRelation'))).toBe(true); + expect(queries.every((query) => !query.includes('UNION'))).toBe(true); + } + + function expectIdentityError(result: any, tool: string) { + expect(result.error).toMatch(/symbol identity/i); + expect(result.recoverySuggestion).toMatch(/analyze.*--force/); + expect(result.epistemic).not.toBe('exact'); + expect(result).not.toHaveProperty('symbol'); + expect(result).not.toHaveProperty('incoming'); + if (tool === 'impact') { + expect(result.risk).toBe('UNKNOWN'); + expect(result.impactedCount).toBeNull(); + expect(result.target).not.toHaveProperty('id'); + expect(result.suggestion ?? '').not.toContain('context'); + } + expectNoExpansion(); + } + + for (const surface of surfaces) { + describe(`${surface.tool} ${'mode' in surface.params ? surface.params.mode : 'default'}`, () => { + for (const shape of ['object', 'tuple']) { + it.each(badIds)(`rejects %s IDs in ${shape} rows before traversal`, async (_label, id) => { + lookupRows([row(id, shape)]); + const result = await backend.callTool(surface.tool, surface.params); + expectIdentityError(result, surface.tool); + }); + it.each(badIds)( + `rejects %s IDs from exact UID lookups in ${shape} rows`, + async (_label, id) => { + lookupRows([row(id, shape)]); + const uidParam = + surface.tool === 'context' ? { uid: SYMBOL.id } : { target_uid: SYMBOL.id }; + expectIdentityError( + await backend.callTool(surface.tool, { ...surface.params, ...uidParam }), + surface.tool, + ); + }, + ); + } + + it('does not choose a healthy candidate beside a corrupt candidate', async () => { + lookupRows([SYMBOL, { ...SYMBOL, id: '\0', type: '' }]); + expectIdentityError(await backend.callTool(surface.tool, surface.params), surface.tool); + }); + + it('rejects a different identity returned for an exact UID', async () => { + lookupRows([{ ...SYMBOL, id: 'Constructor:Services.swift:Service.init' }]); + const uidParam = + surface.tool === 'context' ? { uid: SYMBOL.id } : { target_uid: SYMBOL.id }; + expectIdentityError( + await backend.callTool(surface.tool, { ...surface.params, ...uidParam }), + surface.tool, + ); + }); + + it('keeps an empty lookup distinct from an invalid identity', async () => { + lookupRows([]); + const result = await backend.callTool(surface.tool, surface.params); + expect(result.error).toMatch(/not found/); + expect(result.recoverySuggestion).toBeUndefined(); + }); + }); + } + + it('validates every row before exact File narrowing', async () => { + lookupRows([ + { ...SYMBOL, id: 'File:tests/test_supervisor.py', name: 'test_supervisor.py' }, + { ...SYMBOL, id: undefined }, + ]); + expectIdentityError(await backend.callTool('context', { name: SYMBOL.filePath }), 'context'); + }); + + for (const shape of ['object', 'tuple']) { + it(`accepts an opaque legacy identity in a healthy ${shape} row`, async () => { + lookupRows([row('func:alpha', shape)]); + const result = await backend.callTool('context', { uid: 'func:alpha' }); + expect(result).not.toHaveProperty('error'); + expect(result.symbol.uid).toBe('func:alpha'); + }); + } + + it('preserves a valid ID byte-for-byte instead of trimming it', async () => { + lookupRows([{ ...SYMBOL, id: 'func:alpha ' }]); + const result = await backend.callTool('context', { name: SYMBOL.name }); + expect(result).not.toHaveProperty('error'); + expect(result.symbol.uid).toBe('func:alpha '); + }); + + describe('group impact UID adapter', () => { + const opts = { maxDepth: 3, relationTypes: ['CALLS'], minConfidence: 0, includeTests: true }; + + it.each(badIds)('rejects a %s persisted ID before BFS', async (_label, id) => { + lookupRows([{ ...SYMBOL, id }]); + const bfs = vi.spyOn(backend as any, '_runImpactBFS').mockResolvedValue({ byDepth: {} }); + const repoId = [...(backend as any).repos.keys()][0]; + expect(await backend.impactByUid(repoId, SYMBOL.id, 'upstream', opts)).toBeNull(); + expect(bfs).not.toHaveBeenCalled(); + }); + + it('rejects an otherwise valid mismatched UID', async () => { + lookupRows([{ ...SYMBOL, id: 'route:other' }]); + const bfs = vi.spyOn(backend as any, '_runImpactBFS').mockResolvedValue({ byDepth: {} }); + const repoId = [...(backend as any).repos.keys()][0]; + expect(await backend.impactByUid(repoId, SYMBOL.id, 'upstream', opts)).toBeNull(); + expect(bfs).not.toHaveBeenCalled(); + }); + + it('accepts a legitimate synthetic identity', async () => { + lookupRows([{ ...SYMBOL, id: 'Route:svc:/health' }]); + const bfs = vi.spyOn(backend as any, '_runImpactBFS').mockResolvedValue({ byDepth: {} }); + const repoId = [...(backend as any).repos.keys()][0]; + expect(await backend.impactByUid(repoId, 'Route:svc:/health', 'upstream', opts)).toEqual({ + byDepth: {}, + }); + expect(bfs).toHaveBeenCalledOnce(); + }); + }); +}); diff --git a/gitnexus/test/unit/tool-process-linking.test.ts b/gitnexus/test/unit/tool-process-linking.test.ts index 6c536bb38..d28dd9329 100644 --- a/gitnexus/test/unit/tool-process-linking.test.ts +++ b/gitnexus/test/unit/tool-process-linking.test.ts @@ -55,41 +55,50 @@ function addCall(graph: KnowledgeGraph, sourceId: string, targetId: string) { } describe('Tool handler and process linking phases', () => { - it('falls back to the file node when a parsed tool handler is missing from the graph', async () => { - const graph = createKnowledgeGraph(); - addNode(graph, 'File:src/tools.py', 'File', 'tools.py', 'src/tools.py'); + it.each([undefined, false] as const)( + 'retains file attribution policy %s when a parsed handler is missing', + async (allowFileFallback) => { + const graph = createKnowledgeGraph(); + addNode(graph, 'File:src/tools.py', 'File', 'tools.py', 'src/tools.py'); - const output = await toolsPhase.execute( - makeCtx(graph), - new Map([ - [ - 'parse', - phaseResult('parse', { - allToolDefs: [ - { - filePath: 'src/tools.py', - toolName: 'stale_tool', - description: 'Stale handler', - lineNumber: 1, - handlerNodeId: 'Function:src/tools.py:missing', - }, - ], - allPaths: [], - }), - ], - ]), - ); + const output = await toolsPhase.execute( + makeCtx(graph), + new Map([ + [ + 'parse', + phaseResult('parse', { + allToolDefs: [ + { + filePath: 'src/tools.py', + toolName: 'stale_tool', + description: 'Stale handler', + lineNumber: 1, + handlerNodeId: 'Function:src/tools.py:missing', + ...(allowFileFallback === false ? { allowFileFallback } : {}), + }, + ], + allPaths: [], + }), + ], + ]), + ); - expect(output.toolDefs).toEqual([ - { name: 'stale_tool', filePath: 'src/tools.py', description: 'Stale handler' }, - ]); + expect(output.toolDefs).toEqual([ + { + name: 'stale_tool', + filePath: 'src/tools.py', + description: 'Stale handler', + ...(allowFileFallback === false ? { allowFileFallback } : {}), + }, + ]); - const edge = graph.relationships.find((rel) => rel.type === 'HANDLES_TOOL'); - expect(edge).toMatchObject({ - sourceId: 'File:src/tools.py', - targetId: 'Tool:stale_tool', - }); - }); + const edge = graph.relationships.find((rel) => rel.type === 'HANDLES_TOOL'); + expect(edge).toMatchObject({ + sourceId: 'File:src/tools.py', + targetId: 'Tool:stale_tool', + }); + }, + ); it('does not attach file-level fallback tools to handler-specific processes', async () => { const graph = createKnowledgeGraph(); @@ -110,6 +119,7 @@ describe('Tool handler and process linking phases', () => { addNode(graph, fileLeaf, 'Function', 'fileLeaf', filePath); addNode(graph, 'Tool:alpha', 'Tool', 'alpha', filePath); addNode(graph, 'Tool:fallback_tool', 'Tool', 'fallback_tool', filePath); + addNode(graph, 'Tool:unresolved_tool', 'Tool', 'unresolved_tool', filePath); addCall(graph, alpha, alphaHelper); addCall(graph, alphaHelper, alphaLeaf); addCall(graph, fileEntry, fileHelper); @@ -127,6 +137,7 @@ describe('Tool handler and process linking phases', () => { toolDefs: [ { name: 'alpha', filePath, description: '', handlerNodeId: alpha }, { name: 'fallback_tool', filePath, description: '' }, + { name: 'unresolved_tool', filePath, description: '', allowFileFallback: false }, ], }), ], @@ -147,5 +158,6 @@ describe('Tool handler and process linking phases', () => { expect(linkedEntriesByTool.get('Tool:alpha')).toEqual([alpha]); expect(linkedEntriesByTool.get('Tool:fallback_tool')).toEqual([fileEntry]); + expect(linkedEntriesByTool.has('Tool:unresolved_tool')).toBe(false); }); }); diff --git a/gitnexus/test/unit/typescript-tool-definitions.test.ts b/gitnexus/test/unit/typescript-tool-definitions.test.ts new file mode 100644 index 000000000..e248a682c --- /dev/null +++ b/gitnexus/test/unit/typescript-tool-definitions.test.ts @@ -0,0 +1,441 @@ +import { describe, expect, it } from 'vitest'; +import Parser from 'tree-sitter'; +import JavaScript from 'tree-sitter-javascript'; +import TypeScript from 'tree-sitter-typescript'; +import { extractToolDefinitions } from '../../src/core/ingestion/languages/typescript/tool-definitions.js'; + +const tsParser = new Parser(); +tsParser.setLanguage(TypeScript.typescript); +const jsParser = new Parser(); +jsParser.setLanguage(JavaScript); + +const sdkImport = `import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';`; +const server = `${sdkImport}\nconst server = new McpServer({ name: 'example', version: '1' });`; +const extract = (source: string, parser = tsParser, filePath = 'src/server.ts', offset = 0) => + extractToolDefinitions(parser.parse(source), filePath, offset); +const metadata = (source: string) => + extract(source).map(({ toolName, description }) => ({ toolName, description })); + +describe('SDK tool registration extraction', () => { + it.each([ + ['TypeScript', tsParser, 'src/server.ts'], + ['JavaScript', jsParser, 'src/server.js'], + ] as const)( + 'extracts modern %s registrations in ordinary server files', + (_language, parser, filePath) => { + expect( + extract( + `${server}\nserver.registerTool('search', { description: 'Search files' }, handler);`, + parser, + filePath, + 10, + ), + ).toEqual([ + { + filePath, + toolName: 'search', + description: 'Search files', + lineNumber: 13, + allowFileFallback: false, + }, + ]); + }, + ); + + it.each([ + ['TypeScript', tsParser, 'src/server.ts'], + ['JavaScript', jsParser, 'src/server.js'], + ] as const)('recognizes namespace imports in %s', (_language, parser, filePath) => { + expect( + extract( + ` + import * as SDK from '@modelcontextprotocol/sdk/server/mcp.js'; + const server = new SDK.McpServer({}); + server.registerTool('modern', { description: 'Namespace tool' }, handler); + server.tool('legacy', handler); + `, + parser, + filePath, + ).map(({ toolName }) => toolName), + ).toEqual(['modern', 'legacy']); + }); + + it.each(['', 'type '])('recognizes %snamespace imports in directly typed helpers', (typeOnly) => { + expect( + metadata(` + import ${typeOnly}* as SDK from '@modelcontextprotocol/sdk/server/mcp'; + function install(server: SDK.McpServer) { server.tool('typed', handler); } + `), + ).toEqual([{ toolName: 'typed', description: '' }]); + }); + + it.each([ + "function install(SDK) { const server = new SDK.McpServer({}); server.tool('fake', handler); }", + "function install(server: SDK.McpServer) { server.tool('fake', handler); }", + "SDK = other; const server = new SDK.McpServer({}); server.tool('fake', handler);", + "SDK.McpServer = other; const server = new SDK.McpServer({}); server.tool('fake', handler);", + "({ value: SDK.McpServer } = other); const server = new SDK.McpServer({}); server.tool('fake', handler);", + "SDK.McpServer.prototype.tool = other; const server = new SDK.McpServer({}); server.tool('fake', handler);", + "SDK[key] = other; const server = new SDK.McpServer({}); server.tool('fake', handler);", + "const server = new SDK.McpServer({}); ({ method: server.tool } = other); server.tool('fake', handler);", + "const alias = SDK; const server = new alias.McpServer({}); server.tool('fake', handler);", + "const server = new SDK.OtherServer({}); server.tool('fake', handler);", + ])('rejects unproven namespace receivers: %s', (source) => { + expect( + metadata(`import * as SDK from '@modelcontextprotocol/sdk/server/mcp.js'; ${source}`), + ).toEqual([]); + }); + + it('rejects type-only namespace construction and unrelated namespace imports', () => { + expect( + metadata(` + import type * as SDK from '@modelcontextprotocol/sdk/server/mcp.js'; + import * as Other from 'unrelated'; + const first = new SDK.McpServer({}); first.tool('type-only', handler); + const second = new Other.McpServer({}); second.tool('unrelated', handler); + `), + ).toEqual([]); + }); + + it('does not require description or inputSchema, and ignores nested descriptions', () => { + expect( + metadata(`${server} + server.registerTool('empty', {}, () => {}); + server.registerTool('nested', { inputSchema: { description: 'Schema decoy' } }, handler); + server.registerTool('dynamic-description', { description: getDescription() }, handler); + `), + ).toEqual([ + { toolName: 'empty', description: '' }, + { toolName: 'nested', description: '' }, + { toolName: 'dynamic-description', description: '' }, + ]); + }); + + it('recognizes legacy callback-last overloads with descriptions, schemas and annotations', () => { + expect( + metadata(`${server} + server.tool('bare', handler); + server.tool('described', 'Human description', handler); + server.tool('schema', { query: z.string().describe('Field decoy') }, handler); + server.tool('annotated', { readOnlyHint: true }, handler); + server.tool('full', 'Full description', { query: z.string() }, { readOnlyHint: true }, handler); + `), + ).toEqual([ + { toolName: 'bare', description: '' }, + { toolName: 'described', description: 'Human description' }, + { toolName: 'schema', description: '' }, + { toolName: 'annotated', description: '' }, + { toolName: 'full', description: 'Full description' }, + ]); + }); + + it('decodes static names and quoted description keys without distance or property-order limits', () => { + expect( + metadata(`${server} + server.registerTool(\`find\\x2ditems!?\`, { + inputSchema: { description: 'Nested decoy', example: '${'x'.repeat(2000)}' }, + 'descr\\u0069ption': 'Line\\nwith \\"quotes\\" and \\u{1F680}', + }, handler); + server.tool('legacy\\u002fname', \`Static description\`, handler); + `), + ).toEqual([ + { toolName: 'find-items!?', description: 'Line\nwith "quotes" and 🚀' }, + { toolName: 'legacy/name', description: 'Static description' }, + ]); + }); + + it('recognizes SDK aliases, locally constructed instances and directly typed helper parameters', () => { + expect( + metadata(` + import { McpServer as Server } from '@modelcontextprotocol/sdk/server/mcp.js'; + function install(server: Server) { server.registerTool('helper', {}, handler); } + const add = (server: Server) => server.tool('arrow-helper', handler); + function start() { + const local = new Server({ name: 'local', version: '1' }); + local.registerTool('local', {}, handler); + } + `), + ).toEqual([ + { toolName: 'helper', description: '' }, + { toolName: 'arrow-helper', description: '' }, + { toolName: 'local', description: '' }, + ]); + }); + + it('ignores dynamic names, comments, string decoys and unrelated receivers', () => { + expect( + metadata(`${server} + // server.registerTool('comment', { description: 'decoy' }, handler); + const decoy = "server.tool('string', handler)"; + server.registerTool(runtimeName, {}, handler); + server.tool(\`dynamic-\${runtimeName}\`, handler); + server.registerTool('prefix' + suffix, {}, handler); + unrelated.registerTool('unrelated', {}, handler); + const alias = server; + alias.tool('alias', handler); + `), + ).toEqual([]); + }); + + it('rejects shadowed SDK names and receivers, including declarations later in the scope', () => { + expect( + metadata(`${server} + function parameter(server) { server.tool('parameter', handler); } + function constructor(McpServer) { + const fake = new McpServer(); + fake.tool('constructor', handler); + } + { + server.registerTool('temporal-shadow', {}, handler); + const server = unrelated; + } + function localType() { + class McpServer {} + function helper(server: McpServer) { server.tool('type-shadow', handler); } + } + `), + ).toEqual([]); + }); + + it('rejects reassigned receivers and SDK constructors', () => { + expect( + metadata(`${sdkImport} + let changed = new McpServer(); + changed = unrelated; + changed.tool('changed', handler); + const instance = new McpServer(); + function replace() { McpServer = OtherServer; } + instance.registerTool('constructor-mutated', {}, handler); + `), + ).toEqual([]); + }); + + it('uses decoded type-only imports for helper parameters, but not construction', () => { + expect( + metadata(` + import type { McpServer as Server } from '@modelcontextprotocol/\\u0073dk/server/mcp.js'; + function install(server: Server) { server.tool('typed', handler); } + const fake = new Server(); + fake.tool('type-only-constructor', handler); + `), + ).toEqual([{ toolName: 'typed', description: '' }]); + }); + + it.each(['before', 'after'])('allows SDK lifecycle configuration %s registration', (when) => { + const configure = `server.server.oninitialized = () => {}; server.server.onerror = () => {};`; + const registration = `server.registerTool('visible', {}, handler);`; + expect( + metadata( + `${server}\n${when === 'before' ? configure + registration : registration + configure}`, + ), + ).toEqual([{ toolName: 'visible', description: '' }]); + }); + + describe.each([ + ['TypeScript', tsParser], + ['JavaScript', jsParser], + ] as const)('%s destructuring writes', (_language, parser) => { + it.each([ + ['object member', `({ registerTool: server.registerTool } = replacement);`], + ['array member', `[server.tool] = replacement;`], + ['nested quoted member', `({ nested: [server['registerTool']] } = replacement);`], + ['defaulted member', `({ registerTool: server.registerTool = fallback } = replacement);`], + ['rest member', `[...server.tool] = replacement;`], + ['computed member', `[server[method]] = replacement;`], + ['loop target', `for ({ registerTool: server.registerTool } of replacements) {}`], + ['constructor member', `[McpServer.prototype.registerTool] = replacement;`], + ])('rejects registrations after a write to an %s target', (_name, write) => { + expect( + extract(`${server}\n${write}\nserver.registerTool('fake', {}, handler);`, parser), + ).toEqual([]); + }); + + it('preserves lifecycle writes and ignores pattern keys and default-value reads', () => { + expect( + extract( + `${server} + ({ oninitialized: server.server.oninitialized } = callbacks); + ({ [server.registerTool]: ignored } = source); + ({ untouched = server.registerTool } = source); + server.registerTool('visible', {}, handler); + `, + parser, + ).map((tool) => tool.toolName), + ).toEqual(['visible']); + }); + }); + + it.each([ + [ + 'different package', + `import { McpServer } from 'unrelated'; const server = new McpServer(); server.tool('fake', h);`, + ], + [ + 'destructured parameter', + `${server} function install({ server }) { server.tool('fake', h); }`, + ], + ['destructured local', `${server} { const { other: server } = obj; server.tool('fake', h); }`], + ['catch parameter', `${server} try {} catch (server) { server.tool('fake', h); }`], + ['loop binding', `${server} for (const server of other) { server.tool('fake', h); }`], + [ + 'hoisted var', + `${server} function install() { server.tool('fake', h); { var server = other; } }`, + ], + [ + 'generic type', + `${sdkImport} function install(server: McpServer) { server.tool('fake', h); }`, + ], + [ + 'named class expression', + `${sdkImport} const Other = class McpServer { install() { const server = new McpServer(); server.tool('fake', h); } };`, + ], + ['method write', `${server} server.tool = unrelated; server.tool('fake', h);`], + ['quoted method write', `${server} server['tool'] = unrelated; server.tool('fake', h);`], + ['computed method write', `${server} server[method] = unrelated; server.tool('fake', h);`], + ['method delete', `${server} delete server.registerTool; server.registerTool('fake', {}, h);`], + ['destructured write', `${server} ({ server } = other); server.tool('fake', h);`], + [ + 'spread arguments', + `${server} server.registerTool('fake', ...args); server.tool('fake', ...args);`, + ], + ])('rejects %s', (_name, source) => { + expect(metadata(source)).toEqual([]); + }); + + it('keeps evidence outside shadowing scopes and in closures declared before the instance', () => { + expect( + metadata(`${sdkImport} + function install() { server.tool('closure', handler); } + const server = new McpServer(); + { const server = unrelated; server.tool('decoy', handler); } + server.registerTool('outer', {}, handler); + `), + ).toEqual([ + { toolName: 'closure', description: '' }, + { toolName: 'outer', description: '' }, + ]); + }); + + it('bounds descriptions to top-level properties and respects property overrides', () => { + expect( + metadata(`${server} + server.registerTool('shorthand', { description: 'Kept', title }, handler); + server.registerTool('spread-after', { description: 'Unproven', ...config }, handler); + server.registerTool('spread-before', { ...config, description: 'Known' }, handler); + server.registerTool('last-wins', { description: 'Old', description: 'New' }, handler); + server.registerTool('comments', /* first */ 'not an object', /* callback */ handler); + `), + ).toEqual([ + { toolName: 'shorthand', description: 'Kept' }, + { toolName: 'spread-after', description: '' }, + { toolName: 'spread-before', description: 'Known' }, + { toolName: 'last-wins', description: 'New' }, + { toolName: 'comments', description: '' }, + ]); + }); + + it('resolves hoisted declarations and immutable callable bindings using supplied graph IDs', () => { + const tree = tsParser.parse(`${server} + server.tool('declared', declaration); + function declaration() {} + const arrow = () => {}; + const expression = function () {}; + server.registerTool('arrow', {}, arrow); + server.tool('expression', expression); + server.tool('inline', () => {}); + `); + const bindings = new Map(); + for (const declaration of tree.rootNode.descendantsOfType([ + 'function_declaration', + 'variable_declarator', + ])) { + const name = declaration.childForFieldName('name')!; + bindings.set(name.id, `existing-graph-id:${name.text}`); + } + expect( + extractToolDefinitions(tree, 'server.ts', 0, bindings).map((tool) => [ + tool.toolName, + tool.handlerNodeId, + ]), + ).toEqual([ + ['declared', 'existing-graph-id:declaration'], + ['arrow', 'existing-graph-id:arrow'], + ['expression', 'existing-graph-id:expression'], + ['inline', undefined], + ]); + expect( + extractToolDefinitions(tree, 'server.ts').every((tool) => tool.handlerNodeId === undefined), + ).toBe(true); + }); + + it('uses the nearest callable binding without conflating same-name declarations', () => { + const tree = tsParser.parse(`${server} + function handler() {} + function install() { + const handler = () => {}; + server.tool('inner', handler); + } + server.tool('outer', handler); + `); + const outer = tree.rootNode + .descendantsOfType('function_declaration')[0] + .childForFieldName('name')!; + const inner = tree.rootNode + .descendantsOfType('variable_declarator') + .find((node) => node.childForFieldName('name')?.text === 'handler')! + .childForFieldName('name')!; + const bindings = new Map([ + [outer.id, 'emitted-outer'], + [inner.id, 'emitted-inner'], + ]); + expect( + extractToolDefinitions(tree, 'server.ts', 0, bindings).map((tool) => [ + tool.toolName, + tool.handlerNodeId, + ]), + ).toEqual([ + ['inner', 'emitted-inner'], + ['outer', 'emitted-outer'], + ]); + }); + + it('keeps ambiguous, shadowed, mutable and noncallable handler bindings unresolved', () => { + const tree = tsParser.parse(`${server} + import { imported } from './handlers'; + function handler() {} + function parameter(handler) { server.tool('parameter', handler); } + { const handler = 42; server.tool('shadowed', handler); } + const alias = handler; + server.tool('alias', alias); + server.tool('imported', imported); + let mutable = () => {}; + server.tool('mutable', mutable); + const reassigned = () => {}; + ({ reassigned } = replacements); + server.tool('reassigned', reassigned); + function duplicate() {} + function duplicate() {} + server.tool('duplicate', duplicate); + server.tool('before-initialization', later); + const later = () => {}; + `); + const bindings = new Map(); + // Even graph nodes sharing these names cannot establish a safe callback binding. + for (const node of tree.rootNode.descendantsOfType('identifier')) + bindings.set(node.id, `emitted:${node.text}`); + const tools = extractToolDefinitions(tree, 'server.ts', 0, bindings); + expect(tools.map((tool) => tool.toolName)).toEqual([ + 'parameter', + 'shadowed', + 'alias', + 'imported', + 'mutable', + 'reassigned', + 'duplicate', + 'before-initialization', + ]); + expect( + tools.every((tool) => tool.handlerNodeId === undefined && tool.allowFileFallback === false), + ).toBe(true); + }); +});