42 commits

Author	SHA1	Message	Date
azizur100389	09322d2d89	fix(storage): load VECTOR only when needed (#3045) ... * fix(storage): load VECTOR only when needed * test(storage): verify VECTOR reopen lifecycle --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-08-26 12:57:56 +00:00
Gergő Magyar	d540b00184	fix(check): stop reporting erased and deferred imports as initialization cycles (#2934)	2026-08-12 17:09:32 +00:00
Gergő Magyar	990d79ba8c	fix(mcp): make impact/context reproducible — deterministic ordering on every capped query (#2787) (#2796)	2026-08-02 17:03:15 +00:00
Gergő Magyar	89bbdcf566	fix(ingestion): stop double-indexing const X = () => {} as Function + edgeless Const twin (#2687) (#2691)	2026-07-25 16:56:17 +01:00
Gergő Magyar	df0110b06f	fix: index staleness — false-stale status after analyze (#2668) + inline staleness in query/context/impact/cypher tools (#2655) (#2683) ... * fix(analyzer): case-stabilize runner-identity path fields so status isn't false-stale (#2668) `gitnexus status` reported a freshly-analyzed, untouched repo as stale on Windows (econia/aptos-core, 1.6.10-aptos.0). `status`'s up-to-date check gates on `runnerIdentityIsCurrent`, which deep-compares the stamped runner identity against a freshly recomputed one. That comparison includes `build.rootPath`, `dependencyRuntime.manifestPath`/`lockfilePath`, and `runtime.executablePath` (only `invokedArtifact` is stripped), and `identityCacheKey` hashes packageRoot/buildRoot — all derived from paths that flow through `realpathSync.native`, which canonicalizes 8.3 names and symlinks but does NOT normalize the Windows drive-letter case. When `analyze` and `status` are launched under different drive-letter casing (`c:\...` vs `C:\...`, plausible across CLI shim / npx / server-worker entries), the two identities differ by that one byte and `status` reports stale. Fix: `normalizeAnalyzerRootPath(p, platform)` uppercases the Windows drive letter (POSIX no-op, platform-explicit for testability; preserves a `\\?\` extended-length prefix), applied at the single upstream source — `resolveBuildRoot`'s returned `{packageRoot, buildRoot}` — so every derived identity path field and the cache key inherit a case-stable root, plus at `runtime.executablePath` (process.execPath is the same compared class). The `runnerIdentityIsCurrent` gate is kept intact: a genuine analyzer change still differs in `build.digest`/`dependencyRuntime`, and analyze still rebuilds on real mismatch. Note: the drive-letter divergence was not reproduced on a Windows host (none available); the mechanical chain is verified in source and the fix is a correct defensive normalization that is a no-op on POSIX. If a `status --json` identity field-diff later shows `build.digest`/`dependencyRuntime`/`cliVersion` diverging instead, that indicates a genuinely different install (where "stale" is correct), not this bug. Migration: on Windows, an existing index stamped under the old (non-normalized) casing mismatches the normalized recompute once, triggering a single forced full re-analyze on first upgrade (and a one-time identity-cache recompute). One-time, Windows-only, POSIX no-op. Tests: pure `normalizeAnalyzerRootPath` unit tests (drive-letter uppercase, idempotence, drive-only scope, `\\?\` extended-length prefix, POSIX no-op). * feat(mcp): surface index staleness in query/context/impact/cypher tool responses (#2655) `checkStalenessAsync` already computes how many commits an index is behind the checkout's HEAD, and `list_repos` returns it as `staleness: {commitsBehind, hint}`. But the four hot read tools an agent actually calls in a session — `query`, `context`, `impact`, `cypher` — never surfaced it: `resolveRepo` only runs `maybeWarnSiblingDrift` (stderr, sibling-clone drift only), so a direct tool call gave zero indication the index might be behind HEAD. Thread the existing signal into those four tools at the single `callTool` dispatch chokepoint (after the one `resolveRepo`), reusing the `list_repos` `{commitsBehind, hint}` shape: - `stalenessForTool` computes `checkStalenessAsync` behind an in-flight-promise cache (5s TTL) keyed by lbugPath, so N concurrent tool calls share one `git rev-list` and flat/branch handles (same repoPath, different lastCommit) don't collide. The cache entry is evicted with the repo's other per-index state when the repo leaves the registry. - `withToolStaleness` skips the `git` spawn entirely for results that can't carry the field (via `canCarryStaleness`), so error-returning calls pay nothing. - `attachToolStaleness` adds a `staleness` field to an object result only when the index is behind HEAD. It NEVER changes an existing result's shape: raw-array results (non-tabular cypher rows) are returned untouched, because the CLI's `--limit` and other consumers branch on `Array.isArray`; error envelopes and already-annotated results are left as-is. Non-blocking: `checkStalenessAsync` swallows git failures to `{isStale:false}`, so a git error just omits the field — it never fails the tool. Deliberately out of scope: `@group`-targeted calls forward to `callToolAtGroupRepo` before the chokepoint (multi-repo, single-commit staleness is ill-defined); the legacy `search`/`explore` aliases; and `list_repos` / the `context` resource, which already carry the signal. Tests: `attachToolStaleness` branch matrix (stale object -> field; fresh -> unchanged; raw array -> unchanged; error envelope -> unchanged; idempotent; non-object -> unchanged; null-safe) and a flat-vs-branch cache-key regression test that fails when the cache is keyed by repoPath. * test(mcp): cover staleness tool-signal edge cases + harden the freshness boundary (#2655) Addresses the coverage gaps the review flagged on the #2655 staleness signal, plus one defensive guard so a failing freshness check can never fail a tool. Production (defense-in-depth, no behavior change on the happy path): - withToolStaleness now awaits stalenessForTool with a `.catch(() => undefined)` so a rejection degrades to no-staleness instead of failing query/cypher/ context/impact. - stalenessForTool wraps the check in `Promise.resolve(...).catch(...)` that evicts the cache entry on rejection — a transient failure isn't served as a permanently-rejecting promise for the rest of the TTL window, and the `Promise.resolve` wrap makes the boundary robust to a non-thenable return (a no-op for the real async checkStalenessAsync). A resolving promise is never evicted, so happy-path dedup is unchanged. Tests (gitnexus/test/unit/calltool-dispatch.test.ts): - F1: a rejecting checkStalenessAsync leaves the tool payload intact with no staleness field, and a later call recovers (proves the entry isn't poisoned). Written first and confirmed to fail without the guard. - F2: staleness attaches on query/context/impact object results and on cypher's tabular {markdown,row_count}; a raw-array cypher result keeps its shape. - F3: drift guard — exactly query/cypher/context/impact route through stalenessForTool; explain/pdg_query/detect_changes/check do not. - F4: the per-index cache dedupes within TOOL_STALENESS_TTL_MS and recomputes after it expires (driven via a Date.now spy, not fake timers). Tests (gitnexus/test/unit/analyzer-identity.test.ts): - F5: the produced identity's build.rootPath and runtime.executablePath are normalizer-stable, guarding that both call sites thread through normalizeAnalyzerRootPath (trivial on POSIX, a real regression guard on Windows CI). Plus a source comment noting the one-time Windows re-analyze on first upgrade. * test(mcp): run #2668 guard on Windows CI, document staleness field, cover staleness edge cases Addresses the review follow-ups on the staleness work: - Wire test/unit/analyzer-identity.test.ts into scripts/cross-platform-tests.ts (PLATFORM_LOGIC). Its "identity path fields are normalizer-stable" fixpoint is the Windows regression guard for the #2668 drive-letter normalization, but normalizeAnalyzerRootPath is a POSIX no-op, so the guard was only ever running (trivially green) on the Ubuntu full-suite and never on the windows-latest matrix where it actually bites. Now it runs where it matters. - Document the inline `staleness` field on query/context/impact/cypher responses in the gitnexus-guide skill (both the .claude source and the shipped gitnexus-claude-plugin mirror, kept in sync). - Add three staleness tests that pin behavior the prior tests only implied: * @group-routed calls never get the signal (forwarded before the wrapping switch) — locks the intentional skip so it can't silently flip. * one in-flight freshness check is shared across truly concurrent calls (two dispatched before checkStalenessAsync settles → a single spawn), not just sequential reuse of an already-resolved value. * a late rejection from a superseded cache entry does not evict the newer entry that replaced it after the TTL rolled over (the `=== entry` object-identity guard). The defensive stack in stalenessForTool/withToolStaleness (Promise.resolve wrap + guarded evict + outer catch) is retained deliberately: the wrap is load-bearing for the tests (a sibling describe's vi.resetAllMocks() makes the mock return undefined), and the guarded evict closes the superseded-entry edge now covered above. * fix(test): split the #2668 normalization guard into a portable cross-platform file Registering analyzer-identity.test.ts on the Windows/macOS matrix (previous commit) surfaced four pre-existing failures in that file on macOS 3/3 and windows 3/3. They are not new breakage: those fixture tests compare identity fields against the RAW temp-dir path while the identity resolves through realpathSync.native, so on macOS `/var/folders/...` is received as `/private/var/folders/...`. The file was simply never portable — it had only ever run in the Ubuntu full-suite. Reproduced locally by pointing TMPDIR at a symlink: the same four tests fail, and pass again without it. Move only the portable assertions — the pure `normalizeAnalyzerRootPath` cases (explicit `platform` argument) and the identity fixpoint guard (which compares each field against ITSELF normalized, never against the fixture path) — into test/unit/analyzer-identity-path-normalization.test.ts, and register that file on the matrix instead. The #2668 Windows regression guard still runs where it actually bites, without dragging four symlink-sensitive tests onto runners they were never written for. Verified: the new file passes with TMPDIR behind a symlink (the macOS condition); the heavy file is back to Ubuntu-only. * fix(test): keep the cross-platform #2668 file fixture-free so Windows stays green The split file still carried the fixture-based fixpoint guard, which fails on windows-latest: Invoked analyzer artifact is absent from the validated build: D:\a\...\node_modules\vitest\dist\workers\forks.js Cause is a pre-existing cross-drive defect in this module's `isInside()`, not the #2668 change. The GH Windows runner keeps the repo on D: and temp fixtures on C:. `path.win32.relative('C:\\...fixture', 'D:\\...forks.js')` cannot express a relative path across drives, so it returns the absolute target — which does not start with '..', so `isInside()` reports true. `resolveInvokedArtifact` therefore treats the vitest fork worker as the invoked artifact, it is absent from the fixture's validated build, and identity resolution throws. (Verified directly: `isInside` returns true cross-drive and false for the same-drive control.) Keep the cross-platform file strictly pure — only `normalizeAnalyzerRootPath` assertions with an explicit `platform` argument, no fixture and no filesystem — so it is green on every runner while still exercising the transform on real Windows. The fixture-based threading guard moves back to analyzer-identity.test.ts (Ubuntu-only), where the rest of that file's fixture tests already live, with a comment recording why it cannot be on the matrix. The underlying `isInside()` cross-drive bug is left untouched here (out of scope for this PR) but is worth its own fix: it also guards the trusted cache directory and the identity-cache path-escape check in validateIdentityCache, where a false "inside" verdict weakens validation on multi-drive Windows setups. --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>	2026-07-25 07:21:44 +01:00
Gergő Magyar	7f7255aef8	fix(analyze): load VECTOR before the incremental writeback touches embedding rows (#2623) (#2624) ... * feat(lbug): add ensureEmbeddingRowDmlSafe VECTOR gate for embedding-row DML LadybugDB refuses every mutation of a table carrying an HNSW index while the VECTOR extension is not loaded on that connection: DELETE and CREATE raise a Binder exception, DROP TABLE is refused while the index references it, and SET segfaults the process. Dropping the index is not an available recovery either — CALL DROP_VECTOR_INDEX is itself a VECTOR-extension function and is undefined in exactly that state. Add a single primitive that loads VECTOR under the analyze install policy and, only when that fails, reads CALL SHOW_INDEXES (which works without the extension) to decide whether an index actually exists to trip over. No call sites yet. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the #2623 VECTOR gate for embedding-row DML Three cases: no index + VECTOR unavailable stays safe (no needless escalation); index present + VECTOR unavailable is reported blocked AND the raw deleteNodesForFiles genuinely throws 'extension is not loaded' (proving the hazard is real, not theoretical); index present + VECTOR loadable is safe, the delete works, and the HNSW index survives — the invariant run-analyze relies on when it keeps the index across a surgical incremental run. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): load VECTOR before the incremental writeback touches embedding rows Incremental analyze died on every content change once a repo had built code_embedding_idx: Analysis failed: Binder exception: Trying to delete from an index on table CodeEmbedding but its extension is not loaded. The surgical writeback's first statement is deleteNodesForFiles' CodeEmbedding join-delete, but nothing on that path loaded VECTOR until Phase 4 — so the engine refused the delete. This is an ordering defect, not an environment one: it reproduces on machines where VECTOR loads fine. The dirty-flag recovery then forced a full rebuild on the next run, which is why it read as 'just slow'. Call ensureEmbeddingRowDmlSafe() once, before the escalation gate and before any row is touched — the same 'index lifecycle before row DML' seam dropSearchFTSIndexes occupies for FTS (#2589). Unconditional, because a DB carrying the index from an earlier --embeddings run hits the same wall on a plain incremental run. When VECTOR truly cannot load the table is immutable (the index cannot be dropped without the extension either), so the run falls through to the existing wipe-and-COPY escalation with a message naming cause, consequence and remedy. Fixes #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): pin the #2623 VECTOR-before-embedding-DML ordering end-to-end Sibling of the #2589 FTS drop-before-delete suite, same shape: drive the real runFullAnalysis incremental path over a real git repo and a real LadybugDB, seed real embedding rows, build the HNSW index, then assert the index state at the exact moment deleteNodesForFiles is invoked. Both cases were confirmed to discriminate — with the run-analyze change reverted they fail with the reported 'Trying to delete from an index on table CodeEmbedding but its extension is not loaded', and pass with it: - surgical path: the run completes, the index is still present AND extension_loaded at delete time, exactly one row per nodeId survives, and the untouched file's rows are preserved - blocked path: with GITNEXUS_LBUG_EXTENSION_INSTALL=never the run escalates to a full DB write and says so, instead of crashing Also applies prettier's reindent to the run-analyze log ternary. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(lbug): cite the pinned LadybugDB version in the #2623 probe note The probe matrix behind ensureEmbeddingRowDmlSafe was first recorded on 0.18.0, but gitnexus/package-lock.json pins 0.18.2 (#2587). Re-ran every case on 0.18.2: refused DELETE, refused CREATE, SIGSEGV on SET, DROP_VECTOR_INDEX undefined, DROP TABLE refused, SHOW_INDEXES readable with extension_loaded intact. Identical on both, so the design is unchanged — only the citation was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): preserve embeddings across the VECTOR-blocked rebuild, and check the catalog before loading Three follow-ups from reviewing the fix itself. 1. Data loss on the blocked path. Escalating wipes the DB files, and Phase 3.5 restores embedding rows from cachedEmbeddings — which deriveEmbeddingMode only populates when meta.stats.embeddings > 0. A DB holding embedding rows that its meta does not account for therefore had every vector destroyed silently by a rebuild it never asked for. Probe on a 3-file repo: 3 rows before, 0 after, no warning. Read the rows before escalating (a plain MATCH, no extension needed) so the existing restore has something to restore, and say so in the log. The blocked-path test now asserts the seeded rows survive exactly once, and that assertion fails without this rescue. 2. Catalog before extension. ensureEmbeddingRowDmlSafe loaded VECTOR first and only read SHOW_INDEXES on failure, so every incremental analyze on a machine without VECTOR paid a bounded out-of-process INSTALL attempt plus an 'extension unavailable' warning — including repos that never built an embedding index and can never hit this bug. One local catalog read settles that case first; the load is attempted only when an index actually gates DML, or when the catalog cannot be read. 3. Dead branch. targetConn is always the module singleton there, so the isSharedSingletonConn ternary could never take its second arm. Collapsed to withConnLock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(doctor): live-probe the VECTOR extension instead of printing the static platform capability Review finding on #2624 (MEDIUM), and exactly what #2623's reporter hit: doctor printed 'VECTOR index: available' — derived from a static platform check — while every incremental analyze on the same machine was dying on an unloaded VECTOR extension. The FTS line was switched to a live LOAD probe for the identical contradiction under #2374; VECTOR now gets the same treatment. probeVectorExtensionLoad shares the FTS probe's implementation (bounded, offline-safe, never runs the installer) and doctor's semantic-mode line now follows the probe, not the platform: without a loadable extension the vector index can be neither built nor queried, so search really is on exact scan. The load-error classifier's remedies are label-parameterized so the VECTOR row stops dispensing FTS-specific advice — 'run analyze --repair-fts' repairs FTS indexes only and was actively wrong for a missing vector extension. Default label stays 'FTS'; every existing caller and pinned remedy string is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): remove the stale Windows VECTOR gate — the extension ships for win_amd64 The codebase categorically refused VECTOR on Windows (platform !== 'win32' in isVectorExtensionSupportedByPlatform, plus a hard early-return in loadVectorExtension) on the strength of an early-era report that in-process INSTALL VECTOR could SIGSEGV (#1365). That belief is stale, verified directly: - the extension server hosts win_amd64 VECTOR artifacts for every 0.18.x extension version — v0.18.0 and v0.18.1 both serve a real 14 MB PE32+ DLL (curl-probed; 'file' confirms PE32+ x86-64) - the pinned 0.18.2 core resolves its extension directory to 0.18.1 (strace-verified LOAD open()), so the pinned version's Windows artifact exists too - INSTALL now runs in a spawned child (installDuckDbExtensionOutOfProcess), so even a crashing installer kills only the child and degrades to unavailable — the original hazard cannot reach the parent process any more Windows now takes the same runtime path as every other OS: try LOAD, install out-of-process when policy allows, degrade to exact scan when it truly fails. The MCP semantic-search lane loses its static platform gate too — it always attempts the vector index and falls back to the exact scan on runtime failure, with a once-per-backend diagnostic naming the real error instead of a platform-policy message. isVectorExtensionSupportedByPlatform is deleted; getRuntimeCapabilities reports the platform capability as available everywhere and defers machine truth to the live probe. Windows CI is the enforcement: the vector suites skip visibly only when the extension genuinely cannot load, so green Windows lanes now actually exercise VECTOR instead of silently skipping by policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the catalog-read-failure fallback in ensureEmbeddingRowDmlSafe Review finding on #2624 (LOW): the one branch where the gate cannot cheaply prove safety — SHOW_INDEXES itself erroring — was exercised only by inference. Force it with a Connection.prototype.query spy over the real DB: the catalog read fails, and the gate must fall through to actually attempting the extension load (asserted via the recorded statement stream) rather than guessing, returning true here because the extension is loadable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): load VECTOR on the pool's shared Database so the semantic vector lane actually works Review finding on #2624 (MEDIUM): extension load scope is per-Database (probe-verified — LOAD on one connection enables QUERY_VECTOR_INDEX on every connection of the same Database), and the pool pre-warm loaded only FTS. So LocalBackend's vector lane has ALWAYS raised 'Catalog exception: function QUERY_VECTOR_INDEX is not defined' through the pool and silently fallen back to the exact scan — repos above the 10k exact-scan cap got empty semantic results. The serve path was unaffected (the embedding pipeline loads the extension itself). Mirror the FTS line at BOTH load sites — doInitLbug's pre-warm and initLbugWithDb's external-Database adoption — under the same load-only contract (the read pool never triggers a network install), tracked by a new SharedDB.vectorLoaded flag reset where ftsLoaded resets. The new pool test is discriminating and deliberately closes the writable core adapter before the pool opens: a shared/injected Database would inherit the VECTOR load from test seeding and pass either way, so the case forces the pool onto its OWN fresh read-only Database where only the pre-warm can make the lane legal. Verified: fails at the pre-fix tree with the exact Catalog exception, passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the #2623 ordering suite on Windows/macOS and pre-install VECTOR alongside FTS Two review findings on #2624, both landing in existing seams: - scripts/cross-platform-tests.ts gains incremental-vector-extension-ordering .test.ts: the win32 VECTOR gate is gone in this PR, so the #2623 drop-ordering + blocked-path escalation must be proven on the windows-latest native addon, not just Ubuntu. (The review's claim that lbug-delete-nodes-for-files.test.ts was also missing was wrong — it has been on the roster since #2409.) - scripts/ensure-fts.ts now pre-installs VECTOR under the same best-effort auto-policy contract, so every sharded CI process LOADs from ~/.lbdb instead of racing its own bounded out-of-process INSTALL; the workflow's extension cache already covers it (path is the whole extension dir — key kept for cache continuity). The cross-platform job sets GITNEXUS_REQUIRE_VECTOR=1 beside GITNEXUS_REQUIRE_FTS so a genuinely unavailable VECTOR is a loud failure, never a silent skip. Windows/macOS cannot be executed locally; the PR's CI lanes are the proof for this commit. Linux smoke: ensure-fts.ts reports both extensions ready; all 79 roster entries resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(pool): register loadVectorExtension in the pool unit-suite mocks The pool adapter's new loadVectorExtension import surfaced in four suites that mock lbug-adapter.js with explicit factories (vitest fails loudly on a missing mocked export). Register the export in each — resolving false where the suite's world assumes no vector, true where it mirrors FTS — and extend lbug-pool-fts-load.test.ts, the suite that owns pre-warm extension loading, with the vector pair: successful load cached per shared Database, failed load retried on the next open, both pinned to policy load-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): use POSIX literals for graph paths in the #2623 ordering suite First Windows CI run of this suite (it joined the cross-platform roster this PR) failed with 'Parser exception: Invalid input <MATCH (n:Function) WHERE n.filePath = '>' — path.join produces backslashes on Windows, and a backslash inside the seed helper's single-quoted Cypher literal breaks the parser. The graph stores repo-relative filePaths with forward slashes on every OS, so graph-side paths are POSIX literals now (the incremental-orchestration convention); path.join stays only for real filesystem access. The same Windows lane also proved the substance this suite exists for: lbug-vector-extension passed 7/7 on windows-latest — the extension installed, loaded, and built a real HNSW index there — and the pool vector-lane and DML gate suites passed too. This commit fixes the harness, not the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 12:27:00 +01:00
Eva	d4576630eb	fix(mcp): ignore undefined alias inputs	2026-07-16 10:07:06 +07:00
Eva	3f3494fd32	fix(mcp): validate aliases without schema combinators	2026-07-16 09:29:47 +07:00
Eva	a75844b692	feat(mcp): normalize impact and context aliases	2026-07-14 01:54:49 +07:00
Gergő Magyar	fbffa96554	fix(lbug/mcp): exact symbol content + 0-based line storage with 1-based MCP display (#2377, #2379) (#2380) ... * fix(lbug): store exact symbol content snippets * fix(ingestion): emit 0-based line numbers for COBOL/JCL/scope/markdown nodes COBOL/JCL processors, the scope-graph emitter, and the markdown Section emitter stored 1-based startLine/endLine, unlike every tree-sitter node (0-based). The exact-content slice (#2379) then dropped each symbol's declaration line for those languages. Convert to 0-based at the graph-node emission boundary via toZeroBasedLine — leaving parser-internal .line values, L${line} node/edge IDs, and containment checks untouched. Refs #2377, #2379 * refactor(lbug): single source of truth for symbol-content labels Extract SYMBOL_NODE_LABELS so the exact-content label set can't drift the way the inline copy did in #2379. csv-generator derives EXACT_SYMBOL_CONTENT_LABELS from it; manifest-extractor's near-identical allowlist is left behavior-unchanged (intentional subset, #2325-test-locked) with a documented cross-reference. Refs #2379 * test(ingestion): cover 0-based emitter output and pin exact-content slicing - csv-pipeline: replace the blank-buffer fixture (a +/-1 shift silently passed) with directly-adjacent neighbors; add one-line-symbol and Section (+/-2 fallback) cases. - cobol resolver: assert COBOL Module and JCL job/step emit 0-based startLine. - markdown CRLF: update Section startLine/endLine expectations to 0-based. Refs #2377, #2379 * feat(mcp): present 1-based line numbers in context/query/impact tools GraphNode startLine/endLine are stored 0-based (tree-sitter rows), which surprised users querying them (they don't line up with editors/sed). Add toDisplayLine and apply it at the context/query/impact response boundaries so line numbers are editor/sed-aligned. Raw cypher stays 0-based (documented in the schema resource); BasicBlock/PDG statement lines (already 1-based) and internal join params are left untouched. Refs #2377 * test(mcp): assert 1-based tool exposure with raw cypher staying 0-based context() reports startLine+1 (editor/sed aligned); a raw cypher RETURN of the same node keeps the stored 0-based value. Guards against double-conversion and leaking the display shift into raw results. Refs #2377 * fix(mcp): stop query() double-converting BM25 line numbers bm25Search applied toDisplayLine to its result rows, and query()'s aggregation loop applied it again, so BM25-matched symbols reported lines shifted +2 (stored 0-based 41 read as 43, not 42) while semantic-matched symbols were correct. bm25Search is called only from query(); return raw 0-based rows and let the single aggregation-loop conversion handle both retrievers. Adds a query() BM25 regression test asserting stored 41 -> 42 (would be 43 if double-converted), which the prior mcp-line-display test — covering only context()+cypher — never exercised. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): use ?? not || so first-line symbols keep their line number `sym.startLine || sym[4]` treated a legitimate 0-based startLine of 0 as absent, so context()/query() dropped startLine/endLine for every symbol on line 1 of its file — every COBOL Module (toZeroBasedLine(1) = 0) and markdown h1. `??` only falls through to the positional fallback on null/undefined, preserving a real 0. This also repairs the rename definition-edit path, which consumes context()'s value. Adds a context() first-line (startLine:0 -> 1) assertion. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): make group/cross-repo trace line numbers 1-based consistently A group/cross-repo trace presented 1-based endpoints (via resolveSymbolForGroup) but 0-based hops (tagHops copies port.trace output verbatim), so one response mixed bases. Wrap the trace port adapter (traceForGroup) to convert hop lines to 1-based too, matching the endpoints. Single-repo trace dispatches directly (not through this port) and stays 0-based — full single-repo parity is a tracked follow-up. core/group stays display-agnostic (no mcp import). Extends the cross-trace e2e test to assert hops share the endpoints' base (checkout 10 -> 11, getUsers 1 -> 2). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): present explain/pdg_query anchor line 1-based resolveBlockAnchor converted its ambiguous-candidate lines to 1-based but left the resolved-target anchor raw 0-based, so the same tool reported two bases depending on whether the target was ambiguous. Convert the display anchor to 1-based via toDisplayLine. The BasicBlock join param (symStart: sym.startLine + 1) is untouched — it targets the 1-based BasicBlock id space, not display. Asserts the resolved anchor is 1-based (targetFn stored 10 -> 11). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): bump schema + PDG result versions for the line-number change The 0-based storage flip for COBOL/JCL/markdown/scope (#2377/#2379) changed on-disk line semantics, and the PDG result startLine is now 1-based (#2380). Neither shipped a version bump, so an incremental re-analyze would preserve old 1-based rows (mixed-base index rendered one line too high) and PDG consumers got no signal. - INCREMENTAL_SCHEMA_VERSION 5 -> 6 (forces a one-time full re-analyze) - PDG_RESULT_VERSION 1 -> 2 (result-shape discriminator) Updates the version-pinning tests, the pdgResultVersion result type, and the tools.ts PDG output-contract doc. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): guard manifest label list against SYMBOL_NODE_LABELS drift manifest-extractor's CUSTOM_CONTRACT_RESOLVE_QUERY hand-lists the contract-resolvable labels as a deliberate subset of the shared SYMBOL_NODE_LABELS, guarded only by a comment — the same drift class (#2379) the shared-set refactor eliminated elsewhere. Derive the query's label set and assert it is a strict subset whose difference is exactly {Namespace, Variable, Module}, so adding a symbol label without a conscious manifest decision fails. Query string stays literal (#2325-test-locked). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mcp): document which tools present 1-based vs 0-based line numbers The schema-resource note listed only context/query/impact as 1-based. After the trace/anchor fixes it now enumerates the full set — context, query, impact, group/cross-repo trace, and explain/pdg_query anchors are 1-based; raw Cypher and single-repo trace stay 0-based (full single-repo-trace parity is a tracked follow-up); BasicBlock/PDG statement lines are separately 1-based. (#2377, #2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): pin impact() line-value display (close the coverage gap) The prior mcp-line-display test only asserted context() + raw cypher, which is why the query() double-conversion (#2380) shipped green. Adds an impact() line-value assertion via the ambiguous-candidate path (the only impact response that surfaces a per-candidate line): two same-name symbols force ambiguity and the candidate at stored 0-based 41 must read 42. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): fix stale rename #2283 mock after 1-based context display rename resolves its symbol via context(), which now presents startLine 1-based (#2377), then subtracts 1 to recover the 0-based file index. The #2283 mock stored startLine:1 but put `oldName` on the file's line 0, so after the 1-based shift the definition edit no longer matched and the write-failure path never fired — the test read 'success' instead of 'partial'. Align the mock content to its stored line (oldName on 0-based line 1). Pre-existing failure surfaced once ubuntu/coverage completed on this branch. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): consolidate line-display tests into one shared DB block The query()/BM25 case had spun up a second full LadybugDB + FTS setup; fold it into the single existing block (adding FTS + the Zqxwvbm seed there) so the file builds one DB, not two. Trims per-file setup cost — relevant to the Windows platform-sensitive suite's under-load 15-minute timeout. Same five assertions, all green. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kigland <shuaizhicheng336@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-06 16:16:45 +01:00
Gergő Magyar	e46b87f291	feat: flat workspace index follows the checked-out branch (#2364) ... * feat: flat workspace index follows the checked-out branch (#2354) A plain `gitnexus analyze` now always targets the flat workspace slot, updating it incrementally across branch switches instead of auto-routing non-owner branches into `branches/<slug>/` sub-indexes (disk bloat) or nagging with the primary-inversion "run gitnexus clean" warning. No new CLI flag or config key: the smart behavior is the default. - Placement: only explicit `--branch` consults resolveBranchPlacement; plain runs resolve to the flat slot, `meta.branch` becomes an informational "last analyzed branch" label restamped each run. - Fast path: a same-commit clean-tree branch flip restamps the label and registry entry (adoptFlatBranchLabel, no-op for unregistered repos). - Shadow cleanup: when the flat slot adopts a label that has a pinned sub-index, the now-unreachable `branches/<slug>/` dir and its registry summary are removed together. - MCP: applyBranchScope always falls back to the on-disk flat meta before throwing "not indexed", so long-lived servers resolve a freshly restamped workspace branch. - status: no more "current branch not indexed" dead end — falls through to the workspace index with an informational line and the usual commit-based staleness verdict. - Deleted primaryInversionWarning; explicit `--branch` pinning, the checkout-mismatch guard, detached-HEAD/CI behavior, and `clean --branch` are unchanged. Supersedes the flag-based approaches in #2358/#2359. Closes #2354. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): check registry before deleting shadowed sub-index (#2364 review F2) adoptFlatBranchLabel ran the branches/<slug>/ rm before its own unregistered-repo no-op check, so a repo in the #2264 half-finalized state (up to date but unregistered) lost its pinned sub-index on a same-commit branch flip while the run still failed. The registry lookup now precedes the deletion, making the no-self-heal rule (#2264/#1169) cover disk as well as registry state. The 'never self-heals' unit test now materializes a sub-index dir and asserts it survives; the run-analyze #2354 fast-path test registers its repo under an isolated GITNEXUS_HOME (deletion is only legitimate for registered repos) with a new unregistered variant pinning dir survival. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): keep branch summary when sub-index rm fails (#2364 review F4) The shadow-cleanup fs.rm swallowed every error while the registry summary was dropped unconditionally. On Windows an lbug held open by a live MCP server fails the rm with EBUSY/EPERM, and once the summary is gone 'clean --branch' can never target the leftover dir (it resolves solely via the recorded summary) — stranding the exact un-cleanable disk bloat adoptFlatBranchLabel exists to prevent. The summary is now dropped only when the directory is verifiably gone (post-rm existence check); on failure the summary is retained, a warning names the path and errno, and the informational branch label still restamps. Later adopts retry the rm. New repo-manager-rm-failure.test.ts uses the delegating fs/promises mock idiom (vi.spyOn cannot intercept ESM namespace exports). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): restamp fast path adopt-first and tolerate read-only storage (#2364 review F3) The fast-path label sync stamped meta before adoptFlatBranchLabel, so a crash or adopt failure between the two flipped the retry guard (existingMeta.branch !== branchLabel) and locked in the partial state: every subsequent same-commit run skipped the cleanup and branch-scoped queries kept routing to the stale pinned sub-index. The block also sat outside any try/catch, so a same-commit branch flip on a read-only .gitnexus mount (the documented Docker :ro workflow, #1549) failed a byte-for-byte-current analyze over a purely informational label sync. Adopt now runs first and saveMeta last — any partial failure leaves the guard true and the next run self-heals — and the whole sync is best-effort: read-only errors warn citing #1549, anything else warns and retries next run. Safe because the block only fires on a same-commit clean tree, where the flat DB content is byte-valid for both labels. isReadOnlyFilesystemError is now exported. New run-analyze-adopt-failure.test.ts covers retry-after-partial- failure, adopt-before-stamp ordering, and EROFS/EACCES/EPERM (gaps 4 and 7); a detached-HEAD fast-path pin lands in run-analyze.test.ts (gap 6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): make flat meta authoritative in applyBranchScope (#2364 review F1) applyBranchScope trusted two pieces of cached state before its flat- meta disk fallback, and the handle cache only refreshes on a resolve miss — never on a hit. Post-#2354 that stale window is the routine case: (i) the handle.branch early-return served the flat handle under the OLD label after a workspace flip, silently returning the new branch's content as the old branch (the pool staleness reinit hot- swaps content without updating handle.branch); (ii) a stale cached branches[] summary routed to a branches/<slug>/ dir that adoptFlatBranchLabel had already deleted (raw 'LadybugDB not found' or POSIX ghost reads with staleness detection blinded). The on-disk flat meta is now read before any cached-state trust. A branches[] summary is served only when its sub-index lbug actually exists (the lbug is what the pool opens — serviceability truth); the cached label is trusted only when no readable flat meta contradicts it (#2106 R4 legacy shapes preserved). One refreshRepos() fires on detected staleness so subsequent calls see fresh handles. Safe against mid-analyze reads: dirty stamps spread the existing meta, preserving the old label until the end-of-run atomic write. Fixtures now materialize the pinned sub-index lbug; new regressions cover the stale-old-label error, adopted-summary fall-through to flat, and the dangling-summary partial-failure window (test gaps 1-2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): make end-of-run branch-label sync best-effort (#2364 review F5) The end-of-run adoptFlatBranchLabel sat inside the pipeline try whose catch rethrows, so a registry write failure (ENOSPC, ~/.gitnexus perms) after a successful multi-minute analyze failed the whole run — even though the index was complete and registered, the neighbouring parse-cache save is deliberately wrapped for exactly this reason, and adopt retries unconditionally on the next plain analyze. It now warns and continues, mirroring the parse-cache wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): correct branch-not-indexed guidance for workspace index (#2364 review F6) The error told users to 'Run: gitnexus analyze --branch <X>', but post-#2354 that command hard-errors unless X is checked out — and this message is now the common goodbye for a formerly-indexed branch whose sub-index the workspace slot adopted. The guidance now explains that the workspace index follows the checked-out branch and leads with the checkout; the '(primary only)' fallback becomes '(workspace only)'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: align primary/workspace vocabulary with the #2354 inversion (#2364 review F7) The review flagged pre-inversion 'primary/non-primary' wording that now misleads readers about the placement model: the isPrimaryBranch JSDoc (field name kept — public API surface), the two branches? JSDoc comments in local-backend, the base_ref gate comment in cli/analyze, and four branch-scope test names. Comment/JSDoc/test-name edits only; 'Registry-primary' and 'primary key' senses untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): clarify workspace index status wording (#2364 review F8) 'gitnexus analyze follows this branch' was ambiguous about WHICH branch analyze follows — the recorded one on the line or the current checkout. Both locales now say a re-run follows the current branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): re-read registry after the shadow rm in adoptFlatBranchLabel The F2 reorder moved the registry read to the top of the function, so the whole-file writeRegistry at the bottom persisted a snapshot taken BEFORE the recursive rm of an entire sub-index — widening the unlocked read-modify-write window from microseconds to the duration of a multi- hundred-MB delete. A concurrent registerRepo/removeBranchIndex writer in that window was silently clobbered (the #2106 R9 lost-update class; registerRepo re-reads before writing for exactly this reason). The top read is now a cheap membership gate only (the F2 no-op guarantee); the mutate re-reads its own fresh snapshot after the rm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: treat only provably-absent errno as gone in the new existence probes Both probes added by this series inverted the codebase's provably- absent polarity (listRegisteredRepos validate prunes only on ENOENT/ENOTDIR): adoptFlatBranchLabel's dirGone check read ANY fs.access failure — including a transient EACCES/EIO on a surviving dir — as 'verifiably gone' and dropped the summary, recreating exactly the stranded-bloat bug F4 fixed; applyBranchScope's sub-index check read the same transient errors on a healthy pinned lbug as 'adopted/ deleted', producing a false 'not indexed' error. A resolved force:true rm now proves absence without a probe; on failure the probe treats only ENOENT/ENOTDIR as gone, and a non-missing lbug serves the handle so the pool open surfaces the real error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): harden applyBranchScope stale-state coherence Four residual gaps in the new arm structure, found by post-fix review: - The stale-label error listed the just-contradicted cached label as indexed ('not indexed: main. Indexed branches: main'). The message now derives the flat label from the authoritative meta and excludes the requested branch from the hint list. - A branch pinned AFTER the server cached its handle never triggered a refresh (resolve hits skip the miss-refresh), erroring until restart. Every miss now fires exactly one best-effort refreshRepos() before the error, so the next call resolves; a refresh-once guard keeps doubly-stale resolutions to a single registry re-scan. - A registry entry claiming the branch both as flat label and pinned summary (the rm-failed adopt-degraded state) could serve the stale- vintage pin under a label the flat slot owns; the summary arm now requires handle.branch !== branch and the degraded state errors honestly. - The flat-meta match path returned the cached handle's pre-restamp branch/commit/stats; the meta that decided routing now also supplies the metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): keep the real error visible in restamp warnings; correct the end-of-run retry claim The fast-path catch replaced the actual error with 'storage is read-only (#1549)' for any EACCES/EPERM — mislabeling ownership problems and transient Windows locks and discarding the only diagnostic signal. The warning now carries the real message with the #1549 hint appended. The end-of-run best-effort comment claimed adopt 'retries unconditionally on the next plain analyze'; same-commit runs take the fast path whose guard compares the already-stamped meta label, so the retry actually lands on the next content-changing run. The comment now states the true retry semantics and why the interim state is safe (flat meta stamped first; applyBranchScope trusts it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: unique tmpdir for the branch-scope fixture; drop redundant dynamic imports The branch-scope describe materialized its sub-index stub under a FIXED os.tmpdir()/gnx-2106-multi path — concurrent vitest runs on one host (the documented parallel-agents workflow) could rm each other's stub between beforeEach and the resolve under test, flaking the pinned-branch tests. The fixture root is now mkdtemp-unique per run with afterAll cleanup. run-analyze.test.ts dynamically imported repo-manager inside test bodies despite the module being statically imported at the top of the file (no vi.mock exists there to justify it); the three call sites now use the static import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-03 20:55:27 +01:00
Malik	859e4b75a4	fix(cli): --limit i18n, 0/negative guard, and correct truncation paths (#2310) ... * fix: add --limit i18n, negative guard, correct property paths, and zh-CN translations - Add i18n keys for context/impact/cypher/detect-changes --limit options - Add zh-CN translations for all 4 --limit option descriptions - Add Math.max(0, parseInt()) guard to prevent negative --limit - Fix ALL property path mismatches discovered by audit: - context: callers/callees → incoming.calls/outgoing.calls+accesses - impact: upstream/downstream → affected_processes/affected_modules/byDepth - cypher: rows → row_count cap (rows embedded in markdown string) - detect-changes: affected_flows → affected_processes - Change query command from required to optional positional arg with -q alias - Update @ladybugdb/core from ^0.16.1 to ^0.17.1 - Update typescript from ^5.4.5 to ^5.9.3 * test: add E2E tests for --limit flag across all 5 CLI commands Tests context, impact, cypher, detect-changes, and query with --limit 1, baseline comparison, and --limit 0 (falsy/no-op). detect-changes output is formatted text (not JSON), so those tests count symbol lines matching 'Type name -> filePath' pattern. 14 tests, all passing. No regressions in 6455 existing tests. * fix: address Copilot review feedback on --limit guards - Add Math.max(0, ...) guard to queryCommand limit parsing - Change if(limit) to if(limit !== undefined) in all 5 commands (prevents --limit 0 from being treated as falsy/no-op) - Make queryText parameter optional (Commander may pass undefined) - Fix usage error strings: --search to -q, --query (en + zh-CN) * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(cli): centralize --limit parsing, slice cypher markdown, fix usage text Address PR review feedback on --limit handling: - Add a shared parseLimit() helper (Number.isInteger(n) && n > 0), used by all 5 tool commands. Non-numeric / 0 / negative --limit now means "no limit" instead of the `options.limit ? Math.max(0, parseInt(...)) : undefined` path, where a string like "abc" is truthy and yields NaN -> slice(0, NaN) -> the guardrail commands (impact/context/detect-changes) silently emptied results with exit 0. - cypher: slice the markdown table to --limit data rows so the reported row_count matches what is actually printed (was capping row_count while printing every row). - Fix query usage string: [search_query] (optional positional) and `--query <text>` invocation form, not the option-definition `-q, --query <search_query>` syntax (en + zh-CN). - Add an E2E regression test for non-numeric --limit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): escape newlines in cypher markdown cells A multi-line cell value (e.g. a symbol's `content`) was rendered with raw newlines via String(v), so one logical row spanned multiple physical lines. That corrupts the markdown table and breaks `cypher --limit`'s line-based slice (it kept the wrong number of rows, often zero, while row_count over-claimed). Collapse newlines in formatCypherAsMarkdown so one physical line == one row; the existing CLI slice is now correct and the pre-existing un---limited corruption is fixed too. (#2310 review) * test(cli): de-vacuum the --limit truncation tests The truncation it()s used the repo-banned vacuous-pass pattern (early-return on status===null, assertions guarded by if(Array.isArray), bounds-only toBeLessThanOrEqual — DoD.md:82) against `validateInput`, which has only 1 caller, so context/impact/query --limit 1 compared 1>=1 and stayed green even if the slice were deleted. Rewrite with unconditional, exact assertions and target `logMessage` (2 callers, 4 processes) so the no-limit baseline truly exceeds the limit; detect-changes now mutates two real function bodies (two changed symbols). Adds a multi-line-cell cypher --limit regression. (#2310) * test(ci): run cli-limit-e2e in the cross-platform matrix The --limit E2E suite spawns the real CLI (child_process) but was not in SPAWN_CLI, so it ran only on Ubuntu — the cross-platform check only fails on listed-but-missing files, not the reverse (TESTING.md §Cross-platform). Register it so the --limit regression guard also runs on Windows/macOS, where path separators, CRLF and the formatted-output arrow differ. (#2310) * fix(cli): document impact --limit affected-list cap, drop dead byDepth re-slice `impact --limit` also caps affected_processes/modules, but the help only mentioned the per-depth cap — so JSON consumers reading the affected lists got a silently-truncated array. Update en + zh-CN + the command description to say so. Also remove the client-side byDepth re-slice: the backend already paginates byDepth to the same limit (paginationLimit = clamp(limit,1,10000), offset applied backend-side), so the client slice was a guaranteed no-op. (#2310) * fix(cli): reconcile detect-changes --limit summary, list, and overflow formatDetectChangesResult computed the "... and N more" overflow from the already---limit-sliced array length, so under `--limit` the header (true summary total), the listed rows, and the marker disagreed — e.g. "2 symbols" in the header but a list of 1 with no marker. Base the overflow on the true summary.changed_count / affected_count instead, and add the same marker to the affected-processes list, so header + list + marker stay consistent. (#2310) * feat(cli): add -l shorthand to impact --limit The PR added the -l alias to context/cypher/detect-changes but left impact on the long --limit only, so `impact -l 5` errored while `context -l 5` worked. Add -l for parity and update the help-i18n OPTION_DESCRIPTION_KEYS key to the new `-l, --limit <n>` flag string so the description still resolves. (#2310) * fix(cli): bound all context --limit array categories context --limit sliced only incoming.calls / outgoing.calls / outgoing.accesses / processes, leaving the other relType buckets unbounded — notably incoming.accesses (bounded on outgoing but not incoming) plus imports/extends/ uses/… and typed_properties. Replace the hardcoded slices with a generic loop over every array-valued bucket under incoming/outgoing, plus typed_properties and processes, so --limit caps the whole context payload. (#2310) * refactor(cli): parse --offset with a parseLimit-style helper impactCommand parsed --offset with the legacy parseInt/Number.isFinite idiom while --limit had moved to parseLimit, leaving two parsing styles side by side. Add a sibling parseOffset helper (non-negative — offset 0 is valid) and use it, so both options share one idiom; as a bonus it now rejects negative/fractional offsets instead of forwarding them to the backend. (#2310) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-01 19:16:36 +01:00
Parafee41	f5a2e6a248	fix(search): make vector distance threshold configurable (#2330)	2026-07-01 05:37:46 +01:00
azizur100389	8ad4469e96	fix(test): stabilize local Windows gate baselines (#2314)	2026-06-29 22:27:50 +01:00
Gergő Magyar	57e4afa4c8	fix(mcp): stabilize api_impact response shape for same-URL multi-verb routes (#2308) (#2309) ... * fix(mcp): stabilize api_impact response shape for same-URL multi-verb routes After #2302 made Route identity method-aware, a same URL exposes one Route node per HTTP verb, so a bare-URL api_impact lookup could silently flip from a direct route object to the wrapped { routes, total } envelope. Surface each route's `method` (via the shared fetch) so multi-verb results are distinguishable, and add an optional `method` selector that narrows a multi-verb URL/file to one verb and forces the singular shape. A verb that matches no route returns a clear error. Document the match-count contract in the tool schema. Refs #2308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): cover same-URL multi-verb api_impact contract Regression coverage for #2308: bare-URL and bare-file lookups of a same-URL GET+POST pair return the wrapped form with distinct per-route methods; the method selector collapses to the singular shape (case-insensitively); an unmatched verb returns a verb-not-found error; and verbless routes surface a null method. Refs #2308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): apply autofix feedback - tools.ts: correct api_impact contract docs — `method` narrows to one verb but the singular shape only holds when exactly one route remains after filtering (substring route/file matches can still wrap); cover file lookups; enumerate verbs. - local-backend.ts: surface `method` in route_map and shape_check output (the shared fetch already returns it; agents discover verbs there before api_impact). - local-backend.ts: compute routeCountByHandler from the unfiltered match so a method-scoped api_impact still flags a multi-verb handler's partial middleware. - tests: add file+method and verbless-exclusion cases; assert unconditionally via toMatchObject; lowercase the verb-not-found input to exercise error uppercasing. Refs #2308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): treat wildcard '*' routes as matching any api_impact method selector (#2308) Method-agnostic routes (Django function views) persist with Route method '*', not null. The api_impact method selector used exact verb equality, so '*' routes were excluded and api_impact({route, method:'POST'}) falsely reported 'No routes found' for a route that handles every verb. Treat '*' as matching any requested verb, and correct the comment + tool-description strings that wrongly grouped Django wildcards with null/verbless routes. * fix(mcp): harden api_impact method input against non-string and empty values (#2308) The MCP envelope is not schema-validated, so a non-string `method` reached `.toUpperCase()` and threw a TypeError. Widen the param to `unknown` and guard it with a typeof check that returns a structured error (mirroring the resolveAliasString pattern from #2175), and collapse empty/whitespace verbs to no selector. * fix(mcp): distinguish url-not-found from verb-not-found in api_impact error (#2308) The verb-not-found error appended 'with method "X"' even when the URL/file itself did not exist, implying the URL exists with other verbs. Gate the verb clause on matched.length > 0 so a non-existent URL/file gets the plain message. * fix(mcp): clarify api_impact middlewareNote wording for verbless siblings (#2308) The partial-middleware note claimed 'other methods in this handler' even when the co-located sibling is a verbless (null) route rather than another HTTP verb. Refer to 'other route exports' instead, which covers both cases. * docs(mcp): document and test the method field on route_map and shape_check (#2308) The shared fetchRoutesWithConsumers change surfaced a method key on route_map and shape_check responses too, but their tool descriptions never mentioned it and no test covered it. Document the field on both descriptions and add unit tests asserting it (shape_check rows carry responseKeys + a consumer so they survive shape_check's keys-and-consumers filter). * test(mcp): cover middlewareDetection 'partial' survival under a method filter (#2308) The diff's core behavioral line counts verbs-per-handler from the unfiltered match set so a method-scoped query still flags a multi-verb handler's partial middleware, but no test exercised it (every verbRow hardcoded middleware:null). Add a middleware param to verbRow and a test that fails if the count is taken from the post-filter set instead. Verified via mutation: matched->routes fails it. * test(mcp): add live-LadybugDB integration coverage for route method round-trip (#2308) The new n.method query column was only unit-mocked. Add a self-contained integration suite that seeds GET+POST /api/orders and a method-agnostic '*' Django route, then asserts api_impact surfaces method, narrows by verb, and matches the '*' route end-to-end (the U1 fix), plus route_map surfacing. Own seed + no FTS so it neither perturbs api-impact-e2e nor silently skips. * refactor(mcp): type the api_impact response shape instead of Promise<any> (#2308) Replace apiImpact's Promise<any> with an explicit ApiImpactResult union (single route | wrapped { routes, total } | { error }) and a typed ApiImpactRoute. The results.map is annotated so the response builder is checked against the declared shape. Behavior unchanged; sibling MCP methods keep their Promise<any> convention. * fix(mcp): express the route-or-file requirement in the api_impact schema (#2308) The inputSchema left route/file as bare optionals, so the 'at least one of route/file' rule the handler enforces was invisible to clients. Add an optional anyOf to ToolDefinition (forwarded verbatim by the ListTools handler) and an anyOf:[{required:[route]},{required:[file]}] on api_impact. Matches runtime (both allowed, route wins); 'at least one' not 'exactly one'. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-26 19:50:10 +01:00
Gergő Magyar	47477e5554	fix(mcp): tolerate adapter-materialized line:0 in impact callgraph mode (#2279) (#2283) ... * fix(mcp): tolerate adapter-materialized line:0 in impact callgraph mode (#2279) Some MCP client/agent adapters serialize an omitted optional numeric field as `0` rather than dropping it, so callgraph `impact` calls arrive carrying a spurious `line: 0`. `line` is a PDG-only statement anchor and is meaningless on the callgraph path, so the backend rejected the call ("'line' is only supported with mode:'pdg'") and strict clients rejected it client-side against the advertised `minimum: 1`. Treat a literal `line: 0` as omitted in `_impactImpl` when mode !== 'pdg' and let the normal symbol→symbol BFS run. The coercion is deliberately narrow: only the literal 0, only on the callgraph path. A genuine positive `line` on callgraph still errors (real mode mistake), negative/ fractional values still error, and pdg mode is untouched — `line: 0` there is still rejected (there is no 1-based source line 0 to anchor on). Regression tests pin the full matrix: callgraph + line:0 runs the BFS and is byte-identical to omitting line; pdg + line:0 still errors; positive line on callgraph still errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): log swallowed best-effort query degradations at warn, not error `logQueryError` is the shared handler for query failures that every caller catches and degrades past with a safe fallback (the operation still returns a result). It logged all of them at `logger.error` (level 50) — the same severity as fatal failures — so a gracefully-handled degradation raised a false alarm and drowned genuine errors. This surfaced as an ERROR-level log firing during a passing unit test that intentionally injects a slice-callees query failure to verify the degrade path. Make the severity match reality: - benign missing optional table/label/column (a repo analyzed without processes/communities, or a pre-v3 PDG index lacking the `calleeIds` column — a query that fails on every pdg-downstream impact for such an index) → debug, the normal-configuration case. - any other swallowed failure → warn (handled degradation, still observable). - error is reserved for failures that actually abort an operation, which log directly rather than through this helper. Also fix the sibling bm25/FTS fallback, which logged its swallowed "FTS indexes may not exist" degradation at error while its own import-failure fallback already used warn. The slice-callees degradation test now captures the log and asserts it lands at warn (40), not error (50), pinning the severity against regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): relax impact `line` schema minimum to 0 for adapter compatibility (#2279) Strict MCP clients/agents validate against the advertised input schema and reject a request before sending it. With `line` declaring `minimum: 1`, a client that materializes the omitted optional `line` as `0` rejects a perfectly valid callgraph impact call client-side — so the backend tolerance added in the previous commit never gets a chance to run. Lower the advertised `line.minimum` to 0 and document that 0 (or omission) means "no statement anchor" while mode:'pdg' still requires a positive line. The advertised schema is advisory (the backend self-validates and is the real gate), so this cannot loosen any enforced contract — it only stops strict clients from pre-rejecting `line: 0`. Negative lines are still rejected at the client boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): apply autofix feedback Code-review autofix pass on the #2279 branch: - Replace a newly-introduced `mode as any` cast in the #2279 it.each with the narrow `mode as 'callgraph' | undefined` (strict-typing-no-any). - Add a degradation test for the new logQueryError benign-missing-table → debug branch (asserts no warn/error record surfaces, i.e. it routed to debug). - Pin the bm25/FTS error→warn severity change with a _captureLogger assertion in the existing #1489 test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): make swallowed-failure callers surface degradation; narrow benign-error match (#2283) Tri-review (#2283) found the `error → {debug|warn}` rework reduced telemetry for `logQueryError` callers that do NOT degrade safely, while the docstring over-claimed "every caller degrades to a safe fallback". Address the substance rather than only the log level: - rename apply-edit: track failed writes and return status:'partial' with `failed_files` instead of reporting `status:'success'` when a write was swallowed. A partial rename is no longer indistinguishable from a clean one. - detect_changes: a swallowed symbol/process query failure now sets `partial:true` (rendered by the existing eval-server partial path) so the pre-commit safety gate can't return a false-clean `risk_level:'low'` no-op. - isBenignMissingTableError: scope the `not (defined|found)` arm to a schema object (table/label/rel/column/property), mirroring lbug-adapter's isMissingColumnError. An unscoped "not found" matched operation failures like `rg: not found` / `Symbol not found` and silently demoted them to debug. - logQueryError docstring: state the contract honestly — level reflects telemetry severity, and mutating/safety-critical callers MUST also surface a result-level degradation signal; `warn` alone is not a substitute. - pdg dispatch: pass the normalized `effectiveLine` (not raw params.line) so the validation gate and engine share one source of truth (identity today). Tests: - _captureLogger(level?) lets tests capture below info; the benign-missing-table test now asserts the record IS emitted at debug (20), not merely absent — no longer a vacuous pass if the call were deleted. - new: a non-schema "not found" failure logs at warn (regex-narrowing guard); rename write-failure degrades to status:'partial'+failed_files; line:-1 on the callgraph path still errors (line:0 coercion is narrow); typed the it.each tuple to drop a `mode as` cast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mcp): fix impact `line` description contradiction for whole-symbol pdg (#2283) The new `line` schema description said "mode:'pdg' requires a positive line", which contradicted the top-level impact description ("Without 'line', pdg returns whole-symbol inter-procedural reach plus local whole-symbol PDG diagnostics"). A pdg call without a line is a valid (degraded whole-symbol) call, not an error — the old wording could push an agent to avoid valid no-line pdg calls or synthesize line:0 (which then hard-errors). Reword to: omit line for whole-symbol pdg; a positive line anchors a statement slice; literal 0 is tolerated only as an omitted-line compatibility sentinel on the callgraph path and is rejected for mode:'pdg'. Update the schema test to pin the new, non-contradictory wording and assert "requires a positive line" is gone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-23 20:35:49 +01:00
Gergő Magyar	1a03c8527a	feat(group): cross-repo call trace using PDG (#2269) ... * refactor(group): extract shared resolveBridgeNeighbors from cross-impact Lift the uid-filtered consumer<->provider ContractLink join (direction + queryBridge + row normalization + confidence sort) out of runGroupImpact's inline Phase-2 block into an exported resolveBridgeNeighbors helper. Behavior is unchanged for impact; the helper becomes the single shared bridge join so the upcoming cross-repo trace path never forks its own copy of the neighbor Cypher. Empty uid sets short-circuit without a DB round-trip. Adds direct coverage (real bridge via writeBridge/openBridgeDbReadOnly) for both directions plus the empty-set and unknown-uid edges. * feat(group): cross-repo trace stitching (groupTrace + runGroupTrace) Add GroupService.groupTrace and the pure runGroupTrace engine that stitches per-repo CALLS/HAS_METHOD trace segments across one ContractLink boundary in the group bridge: from --(local trace)--> consumer --(ContractLink)--> provider --(local trace)--> to - Resolves from/to across all members (symbol node id == bridge symbolUid); same-repo endpoints delegate to a single local trace with no crossing. - Single boundary crossing (MAX_SUPPORTED_CROSS_DEPTH); deeper crossDepth is clamped with a note, mirroring cross-impact. - Discriminated GroupTraceResult union (ok|not_found|ambiguous|error) with per-hop repo tags, a typed crossings[] entry, and centralized degraded-state note constants (TRACE_NOTES). No . - Trace-specific pair query (keeps BOTH crossing endpoints) lives in this module; the uid-filtered neighbor join (resolveBridgeNeighbors) is reused where it fits. ensureBridgeReady exported for reuse. - New GroupToolPort methods (trace/resolveSymbol/pdgFlows) are optional so existing port mocks keep type-checking; runGroupTrace guards on presence. PDG enrichment is wired as an opt-in hook (enrichSegment) — the port method is stubbed until U4. Covered by unit tests over a real bridge + mocked port. * feat(group): route trace tool to groupTrace on @group syntax Wire the cross-repo trace through the existing @group dispatch: - callTool routes trace with an @-prefixed repo to callToolAtGroupRepo, which forwards from/to/uid/file/maxDepth/includeTests plus the experimental pdg/crossDepth flags to GroupService.groupTrace. Member path in @group/path is advisory for trace (resolution is whole-group). - Port gains trace/resolveSymbol/pdgFlows adapters. resolveSymbolForGroup wraps the shared resolveSymbolCandidates so groupTrace can locate the member repo and recover each endpoint node id (== bridge symbolUid). pdgFlowsForGroup is a degraded stub here (call-level only); U4 implements the REACHING_DEF walk. - trace tool schema documents the @group entry point, pdg, and crossDepth. Single-repo trace is untouched. Covered by dispatch-routing tests (@group -> groupTrace, non-group stays local) and tool-schema assertions. * feat(group): opt-in PDG data-flow enrichment for cross-repo trace Implement _pdgFlowsForGroupImpl: the real REACHING_DEF anchor walk that backs the port pdgFlows adapter (replacing the U3 call-level stub). When pdg:true and the segment repo has a flows PDG layer, the boundary-adjacent segments carry their intra-procedural def->use hops: - Anchors by the boundary symbol UID (precise; avoids the by-name ambiguity the resolveBlockAnchor path can hit), then reuses the same span-anchored, bind-param-only flows query as pdg_query (BasicBlock id-prefix + [start+1, end+1] line window; no rel-property index, so the anchor IS the bound). - Stays intra-procedural: data flow never crosses the repo boundary. - pdgStampForMode probe: false -> available:false (degrade with note); the trace stays ok. Any query failure is swallowed (enrichment is auxiliary). Covered by runGroupTrace enrichment tests: dataFlow attached on opt-in, degraded note when no layer, and no pdgFlows call when pdg is omitted. * test(group): evaluation-first cross-repo trace e2e (two real indexes) End-to-end gate for the cross-repo trace: stands up two real LadybugDB indexes (consumer 'frontend' + provider 'backend'), a real ContractLink bridge, and a real LocalBackend with both repos registered, then drives the public callTool('trace', { repo: '@grp', pdg: true }) and asserts: - the stitched checkout -> callUsers -(CONTRACT_LINK)-> handleUsers -> getUsers path, each hop tagged with its member repo - real REACHING_DEF data-flow enrichment of the consumer segment (userId) - a degraded 'No PDG layer in app/backend' note (provider has no PDG layer) - single-repo trace against one member is unchanged (no crossings) Hand-persists the minimal real graph (deterministic; a full two-repo analyze is heavier than this gate needs) and exercises real Cypher across resolveSymbolCandidates, _traceImpl, the bridge pair query, and _pdgFlowsForGroupImpl. Windows-skipped (describeReopen) and registered in the cross-platform native-lbug set. Scoped to a single @group call: opening bridge.lbug read-only a SECOND time in one process currently fails (shared bridge open/close lifecycle, also affects impact @group) — the pdg-omitted/clamp variants are unit-covered. * docs(group): document cross-repo trace + PDG enrichment ARCHITECTURE.md: trace is now group-aware; describe the @group cross-repo stitch over a single ContractLink boundary (CONTRACT_LINK hop, crossings[], crossDepth clamp), the opt-in experimental PDG REACHING_DEF enrichment of boundary-adjacent segments, the symbolUid-grain join between the two stores, and the deferred full cross-program (SDG-like) data flow. PIPELINE.md: add the cross-trace consumer of the bridge with its pair-query rationale. Does not touch gitnexus/CHANGELOG.md (release-owned). * fix(review): apply autofix feedback Apply safe_auto findings from ce-code-review (run 20260622-094243): - local-backend.ts: drop (r: any) in _pdgFlowsForGroupImpl row map; coerce hop line via Number() so a nullish LadybugDB cell can't surface NaN. - tools.ts: advertise the forwarded param in the trace schema and add crossDepth maximum:10 (schema now matches what groupTrace reads). - cross-trace.ts: parallelize per-member resolveSymbol/resolveRepo with order-preserving Promise.all (matches groupContext/groupQuery); add a note when pdg:true is passed to a same-repo trace (PDG only enriches at a cross-repo boundary). - tests: remove / tighten (no-any rule). Residual gated_auto/manual findings (unbounded crossing query + loop, whole-file PDG widening on absent span, error-vs-no_path masking, top-level try/catch parity, helper dedupe, branch-coverage gaps) are recorded in the run artifact for the PR body. * fix(group): skip CHECKPOINT on read-only bridge close so it can reopen Root cause of the in-process bridge.lbug reopen failure (which broke repeated @group impact/trace calls in a long-lived MCP server): closeBridgeDb issued CHECKPOINT on EVERY handle, including read-only ones. A CHECKPOINT on a read-only connection has nothing to flush but leaves a WAL/shadow lock artifact that makes the next read-only open of the same path fail (openBridgeDbReadOnly returns null -> 'Could not open bridge.lbug read-only'). Reproduced: open -> query -> closeBridgeDb -> open again returned null only when the close ran CHECKPOINT; a non-checkpoint close reopened fine, and the raw native open/close cycle was never the problem. Fix: tag read-only handles (BridgeHandle._readOnly, set by openBridgeDbReadOnly) and skip CHECKPOINT for them in closeBridgeDb. Writable handles are unchanged (they still flush before close). This is the shared bridge-db close path, so impact @group benefits identically. - Regression test in bridge-db.test.ts: open/query/close/open/query/open in one process now succeeds. - Re-enabled the second @group call in cross-trace-e2e.test.ts (was scoped to a single call for this very limitation). * fix(group): bring bridge-db close to parity with the core adapter safeClose The bridge open/close cycle was less robust than the main graph DB's: closeBridgeDb closed the connection/database but skipped the post-close steps the core adapter's safeClose performs, so a rapid in-process reopen could race the OS handle release (Windows) or an orphaned WAL sidecar. That gap is why the close-then-reopen tests had to skip Windows. closeBridgeDb now mirrors safeClose after closing the handle: - waitForWindowsHandleRelease(dbPath): probe the file (+ .wal) until the residual Windows lock clears, so the next open does not race (warns if the budget is exhausted, matching the core adapter). - finalizeLbugSidecarsAfterClose(dbPath): quarantine an orphaned WAL (shadow missing) so the next open replays a consistent file. Both helpers are the same ones safeClose uses (Windows-proven via the core adapter CI), and the bridge read open already retries transient locks. Combined with the read-only CHECKPOINT skip, the bridge reopen is now robust on every platform, so the close-then-reopen tests run on all platforms (Windows CI exercises them via the cross-platform subset). No write-path behavior change; Linux/macOS unaffected. * fix(group): bound cross-repo crossing fan-out (LIMIT + segment memoization) Address the top review residual: the bridge crossing query was unbounded and the crossing-selection loop could run an O(2*N) sequential trace-BFS over every ContractLink between a repo pair. - CY_CROSSINGS_BETWEEN now ORDERs BY confidence DESC and LIMITs to MAX_CROSSINGS_TO_TRY + 1; listCrossingsBetween slices to the cap and reports truncation. Exceeding the cap surfaces a note (no silent truncation), keeping the highest-confidence crossings. Aligns with the repo's anchored+LIMIT-bounded query discipline (LadybugDB has no rel-property index). - The home-repo segment (from -> consumer) depends only on the consumer uid and the target-repo segment (provider -> to) only on the provider uid, so each is memoized by that uid. Many crossings sharing a consumer/provider (one client call linked to several providers) now cost one trace per distinct endpoint instead of one per crossing. A consumer whose segment already failed is skipped for every later crossing that shares it. Test: two links sharing a consumer (first provider unreachable, second reachable) assert the from->consumer segment is traced exactly once and the second crossing wins. * fix(group): restore Windows skip for bridge reopen tests; drop ineffective close-side probe The previous commit flipped the bridge close-then-reopen tests to run on Windows, betting that a close-side waitForWindowsHandleRelease + finalizeLbugSidecarsAfterClose probe (mirroring the core adapter safeClose) would make the in-process reopen work there. Windows CI proved otherwise: 4 writeBridge->openBridgeDbReadOnly tests fail ('expected null not to be null' — the read open returns null). The writable-close -> read-open handoff plus writeBridge's atomic sidecar rename does not release the OS file handle before the read open races, and the existing open-side LBUG_OPEN_RETRY only retries lock-pattern errors, not the post-rename sidecar database-id mismatch. macOS passes; the core adapter's own reopen also passes — this is bridge+Windows specific. - Revert itLbugReopen to the Windows skip (the pre-existing, correct state). - Remove the close-side probe + finalize from closeBridgeDb: it did NOT close the Windows gap, and reviewers flagged it for hot-path latency (finalize ran on every close, all platforms) and safeClose duplication. - KEEP the load-bearing fix — skipping CHECKPOINT on read-only handles — which fixed the reproduced Linux/macOS in-process reopen artifact (the real bug). Net: Linux/macOS repeated @group impact/trace works in-process; Windows in-process bridge reopen remains a documented limitation (unchanged from before this PR). * fix(group): surface degraded members + cap truncation; honest crossDepth schema Address the cross-engine-corroborated tri-review findings (Codex + Claude): - resolveAcrossMembers / runGroupTrace now track member repos that could NOT be queried (resolveRepo or resolveSymbol threw) and, when the result is not_found, attach a degraded-member note. A transient/corrupt member DB is no longer silently reported as a clean 'symbol absent' not_found. (Codex B1+B3 + ce-reliability.) - The cross-repo not_found now carries a programmatic truncated:true flag (and a clearer suggestion) when the MAX_CROSSINGS_TO_TRY cap was hit, so a consumer can distinguish 'no path' from 'cap may have hidden a connecting ContractLink'. (Codex B3 + ce-adversarial + ce-api-contract.) - trace tool schema: crossDepth maximum 10 -> 1 to match the implementation's single-hop clamp (the schema previously advertised an unsupported 2-10 range). (ce-api-contract, conf 100.) Test: a member whose resolveSymbol throws yields not_found WITH a degraded note naming the unreachable repo (if-free responder map). * docs(group): clarify trace @group/memberPath is advisory (resolves all members) Tri-review (Codex ce, conf 100) caught a doc/impl inconsistency: ARCHITECTURE.md lumped trace with query/context/impact as honoring @group/memberPath member scoping, but cross-repo trace resolves from/to across ALL members (the member path is advisory). Clarify the behavior and point to from_uid/to_uid for disambiguating same-named symbols across members. * feat(group): file-level boundary fallback so cross-repo trace works on HTTP contracts Benchmark (bench/cross-repo-trace/) running the REAL pipeline (runFullAnalysis --pdg -> real syncGroup -> trace @group) found that cross-repo trace returned not_found for real HTTP links even though sync built the correct ContractLinks: HTTP (and other source-scan) contracts hardcode symbolUid:'' (http-route-extractor), and both cross-trace AND cross-impact join crossings by Contract.symbolUid, which never matches an empty uid. (Pre-existing — impact @group has the same gap.) Fix: when a crossing's symbolUid is empty, fall back to the contract's FILE — if the user's from/to resolves into the contract file, that endpoint anchors the boundary. CY_CROSSINGS_BETWEEN now returns consumer/provider filePath; a crossing is kept if it can be anchored by uid OR file on each side; a fileBoundaryFallback note flags that the boundary is file-level, not symbol-precise. This makes the common 'trace from=<calling fn> to=<handler fn>' case work end-to-end (verified: fetchUsers -> listUsers stitches with a CONTRACT_LINK hop + PDG enrichment, 2/2). Limits (documented in the bench README + the note): anonymous handlers have no named target; when several contracts share files the file fallback may attach the wrong contractId to a correct path. The proper upstream fix is to populate symbolUid in the HTTP extraction (benefits impact too) — the bench is its gate. Adds a unit test pinning the empty-symbolUid file-fallback stitch. * fix(group): resolve HTTP contract symbolUid by containment (fixes cross-repo trace + impact) Addresses the root cause behind the cross-repo trace file-fallback: HTTP contracts hardcoded symbolUid:'' (http-route-extractor), so both cross-trace and cross-impact — which join crossings on Contract.symbolUid — could not traverse HTTP links. (Also found: the pre-existing graph-assisted resolution queried the wrong edge, CONTAINS instead of DEFINES, so it never resolved a uid either.) Now the extractor resolves each detection to a real symbol: - HttpDetection carries the call-site line (node.ts sets it on every express/ fetch/axios/jquery/nest detection; express also captures the handler arg). - resolveDetectionSymbol resolves the named handler first, else the innermost Function/Method whose line span encloses the call (consumer = the function containing the fetch; provider = the named/inline handler), over the correct File-[DEFINES]->symbol edge. Base-tolerant (0- vs 1-based startLine). - Wired into both source-scan and graph-assisted provider/consumer paths. Verified end-to-end (bench/cross-repo-trace): all 4 contracts now carry real uids, trace is symbol-precise (GET pair -> http::GET, POST -> http::POST, no file-fallback note), and impact @group fans out (cross_repo_hits 0 -> 1). The cross-trace file-level fallback remains as the secondary path for truly anonymous handlers. Adds 2 containment unit tests; 738 group/integration pass. Languages other than JS/TS still resolve providers by handler name; their consumers fall through to the file fallback until their plugins set the line. * fix(group): extend HTTP symbolUid containment to all languages + nested methods Completes the symbolUid resolution across every bundled HTTP plugin: Python, Go, PHP, Kotlin and Java now set the call-site line on their consumer (and Feign/ named) detections, so their HTTP contracts resolve to the containing function the same way Node/TS already did. Also generalizes the containment query: it now matches Function/Method/CodeElement by filePath (UNION ALL) instead of File-[DEFINES]->symbol. The DEFINES edge only reaches a file's TOP-LEVEL symbols, so methods nested in classes (Java/Kotlin — File defines the class, the class defines the method) were invisible; matching by filePath reaches them. Verified against a real index (LadybugDB supports the UNION); JS/TS still fully symbol-precise (bench 2/2), 709 group tests pass. Residual is now only the inherent case — a fully anonymous handler with no named callee — which keeps the cross-trace file-level fallback. * feat(group): destination trace — follow a consumer to an anonymous handler Handles the one inherent residual: an anonymous route handler (`router.get('/x', (req,res) => …)`) has no symbol node at all (the file holds only a Const + PDG BasicBlocks), so it can never be named as a trace `to`. Adds a DESTINATION TRACE: omit to/to_uid/to_file on an @group trace and `trace from=<consumer>` follows the consumer's outgoing HTTP call across the bridge and reports where it lands — by route + file:line, with a notes[] entry flagging the handler as anonymous. Implemented as a new branch in runGroupTrace (p.destination) backed by CY_CROSSINGS_FROM (all ContractLinks leaving the consumer repo) + stitchToDestination; the provider endpoint is labelled '<METHOD /path handler>' when its symbolName is a generic token/file basename. The MCP routing already omitted an absent `to`, so only the schema docs changed. parseTraceParams now treats a missing `to` as a destination trace instead of an error. Verified end-to-end: anonymous fixture reports 'app/frontend:fetchUsers -> app/backend:<http::GET::/api/users handler>'; named fixture lands at the real function. Adds 2 unit tests; 915 group tests pass. * fix(group): tri-review fixes for cross-repo trace + symbolUid resolution Two-engine tri-review (Claude swarm+ce + Codex GPT-5.5 swarm+ce+adversarial) surfaced these; cross-engine-corroborated unless noted. Correctness (P1, all four lanes): destination trace reported the WRONG endpoint — an empty-uid consumer made trace(from->from) trivially succeed, so the highest- confidence same-file crossing won regardless of which call `from` makes. stitchToDestination now collects ALL connecting crossings, prefers symbol-precise hits, and returns `ambiguous` (with candidates) when it cannot disambiguate. Correctness (P1, Codex): resolveDetectionSymbol early-returned null when d.line==null, blocking NAME resolution for named providers that set no line (Spring/Go/etc.). Name resolution now runs first; only containment needs a line. Correctness (P2): resolveContainingSymbol OR-ed `line` and `line-1`, which could mis-pick a one-line sibling. It now probes the base-correct `line-1` first and falls back to `line` only if nothing matches. Correctness (Codex): anonymous Express handlers emitted name:'handler' and could attach to an unrelated fn literally named `handler`. node.ts now emits name:null for non-identifier handlers (containment-only). Robustness: drop the first-symbol-in-file pickSymbolUid guess from the graph consumer/provider paths (a wrong uid would win the contractId merge); remove the dead CONTAINS_QUERY fallback (CONTAINS is File->Folder, never a symbol) + the now -unused pickSymbolUid/handlerName; seed destination notes with degraded-member notes so a successful trace still surfaces them; providerLabel takes providerUid so a resolved fn named `handler` is not mislabeled anonymous, and only true file basenames (known extensions) — not any dotted name — count as anonymous. API contract: a single-repo trace with no `to` now returns an actionable error (destination trace is @group-only) instead of "symbol 'undefined' not found". Maintainability/tests: narrow asLocalTrace per-field (drop as-unknown-as); fix the PR's lone as-any (vi.mocked); if-free e2e teardown; qualify the bench README. Adds ambiguous-destination, anonymous-handler-no-false-name, and single-repo-no-to tests; redirects graph mocks CONTAINS->UNION ALL. 918 group/integration pass. * fix(group): carry degraded-member notes through SUCCESSFUL group traces A reviewer (koriyoshi2041, PR #2269) correctly flagged that degraded-member resolution was surfaced only on not_found, not on a successful ok result. Group trace resolves names across ALL members, so an ok is 'unique among the members we could query' — if a member that threw during resolveSymbol also holds from/to, the real answer could be ambiguous. The destination path already seeded the note (prior commit); this extends it to the same-repo and cross-repo success paths by seeding the dispatch notes with degradedNotes([...fromRes.degraded, ...toRes.degraded]). Adds a regression test: reg-be throws while a same-repo trace succeeds in reg-fe; the ok result now carries the 'could not be queried' degraded note (app/backend). * test(bench): cover all implemented cross-repo trace cases in one runner Replace the single named-handler script with a self-contained verify.mjs that generates each fixture inline and exercises every implemented end-to-end case against the real analyze -> sync -> trace/impact pipeline, asserting PASS/FAIL (exit non-zero on failure). 10 checks across 4 scenarios: - named handlers: 4/4 symbolUid resolved; symbol-precise GET vs POST crossing selection; destination trace lands at the named handler. - anonymous handler: empty symbolUid; destination trace reports it by route with the anonymous note. - impact @group fan-out (cross_repo_hits >= 1). - multi-language (Python Flask + requests): link built, cross-repo trace stitches, and the file-level boundary fallback is exercised when the provider has no uid. Ambiguous-destination and degraded-member paths need synthetic inputs the real analyzer cannot produce, so they stay in the unit suite (documented in the README + script header). Removes verify-named.mjs + fixtures-named/ (folded inline). * test(group): pin destination degraded-success + precise-tier ambiguity Adds the two regression guards koriyoshi2041 requested on PR #2269 after the degraded-on-success fix: - destination trace success with a degraded member: reg-fe resolves from and follows the link to an anonymous handler while reg-be throws; the ok result carries the anonymous endpoint AND the 'could not be queried' degraded note, so the no-to path stays aligned with explicit to traces. - multiple PRECISE destination hits: one from reaches two consumers with resolved uids linked to different routes; the result is ambiguous (role: to) with both route candidates. Distinct from the existing file-level ambiguous test, this pins the stronger precise tier against a future change silently picking the highest-confidence destination. Both already pass against current behavior; 716 group tests pass.	2026-06-23 07:54:13 +01:00
Gergő Magyar	78b4077d8a	feat(impact): opt-in PDG-backed impact mode - statement + inter-procedural slicing, resolved-callee-id soundness, mutation-oracle validated (#2227)	2026-06-20 12:04:32 +01:00
Gergő Magyar	9ff7337f1e	fix(mcp): rename query/cypher params so Claude Code can call them (#2186) ... * fix(mcp): advertise search_query/statement params for query/cypher tools (#2175) Claude Code drops a tool-call argument named exactly 'query', making the query and cypher tools unusable from it. Rename the advertised required parameters to search_query and statement so the client transmits them. Handler-side backward-compat for the legacy 'query' key follows in the next commit. * fix(mcp): accept search_query/statement with legacy query fallback (#2175) Resolve the new advertised param names in the backend while still accepting the legacy 'query' key, so curl/HTTP, other MCP clients, the CLI, the group path, and the internal executeCypher() all keep working. Alias is normalized once at the callTool chokepoint (covers group-forward + search alias); query() and cypher() dual-read defensively. New name wins when both are supplied. Updates the required-error message and adds dual-accept unit + integration coverage. * fix(cli): pass canonical search_query/statement params to query/cypher tools (#2175) Stop the CLI from depending on the deprecated 'query' alias. No user-facing change — the positional args are unchanged and the backend accepts both keys. * fix(mcp): generators advertise search_query in query() examples (#2175) Update the three doc/example generators (ai-context AGENTS/CLAUDE block, skill-gen community skills, resources repo hint) so future analyze runs emit query({search_query: ...}) — the param name Claude Code actually transmits. Tests assert the new form is present and the legacy query({query: form is absent (the #2059 generator-test pattern). * docs(mcp): advertise search_query/statement in skill & guidance examples (#2175) Sync the committed agent-facing docs to the renamed params so a Claude Code agent following them emits the transmittable key: AGENTS.md/CLAUDE.md gitnexus block, the canonical gitnexus/skills/* source and its installed/plugin/cursor mirrors, and the README examples. Scoped rewrite of the two call prefixes only (query({query: -> search_query, cypher({query: -> statement). * style(mcp): prettier line-wrap for #2175 alias-resolution edits * fix(review): uniform search_query precedence + cypher empty guard (#2175) Code-review findings (correctness/adversarial/api-contract/maintainability consensus): - Group-mode query inverted the 'new name wins' rule: the callTool chokepoint backfilled params.query only when empty and the @group-forward read params.query directly, so a both-keys (or whitespace-legacy) group call let the legacy value win — unlike the local path. Replace the hidden param mutation with a self-contained 'search_query ?? query' resolve at the group-forward; precedence is now uniformly new-wins at every consumer site. - cypher() now returns the same friendly required-param error as query() when neither statement nor query is supplied, instead of a raw DB prepare error. - Document the legacy alias as permanent (third-party clients may send query=). Adds group-forward alias tests (both-keys + legacy-only), empty/whitespace search_query, the search-alias path, and the cypher empty-statement guard. * fix(review): non-string alias safety + drop stale chokepoint comment (#2175) Tri-review findings (correctness/adversarial/security + maintainability): - Non-string statement/search_query/query (the MCP envelope is not schema-validated) hit .trim() and threw TypeError to the server boundary instead of a friendly required-param error. Introduce resolveAliasString() (new name wins; non-string -> undefined) used by query(), cypher(), and the group-forward, so all three return the structured error. Empirically verified (123 ?? '' -> 123, (123).trim() throws) — this overrides a critic refutation that mis-read ?? as a string coercion. - Remove the stale query() comment claiming alias resolution happens at a callTool chokepoint; that mutation was removed earlier in this PR — each site resolves the alias itself. - Document GroupToolPort.query's intentionally-narrower required type vs the wider LocalBackend impl. Adds non-string and empty-new-key precedence tests. * fix(mcp): alias falls back to legacy value when new key is blank (#2175) PR #2186 review finding: resolveAliasString used `canonical ?? legacy` (nullish), so an explicitly empty/whitespace new-name value (e.g. {search_query:'', query:'real'}) won and was rejected — discarding a valid legacy value, contradicting the 'new name wins when both supplied' intent. Resolve to the first NON-BLANK string instead (new preferred when it carries a real value, else legacy). Covers query(), cypher(), and the group-forward (all route through the helper); non-string still resolves to a friendly error. Flips the presence-based test and adds whitespace/cypher/group fallback cases. * fix(mcp): drop legacy "query" mention from query/cypher schema descriptions (#2175) PR #2186 review finding: the search_query/statement inputSchema descriptions named the legacy "query" key — the exact arg Claude Code drops — and description text is read by an LLM choosing arguments, weakly nudging it to send "query". Trim the descriptions to their clean form and move the legacy-alias note to a code comment next to the schema (preserved for maintainers / non-CC clients). properties/required unchanged (no `query`).	2026-06-13 10:24:16 +01:00
azizur100389	bdb824cfe4	feat(cli): add circular import cycle check (#2166)	2026-06-12 04:53:17 +01:00
Gergő Magyar	7eaeb0a0c4	feat: multi-branch indexing and branch-scoped querying (#2106) (#2137) ... * feat(git): add getCurrentBranch + resolveRefToCommit helpers (#2106) * feat(storage): branch-scoped getStoragePaths + branchSlug + resolveBranchPlacement (#2106) * feat(analyze): branch-aware indexing — per-branch slot, no overwrite (#2106) * feat(registry): nest non-primary branches under one path entry (#2106) * feat(mcp): optional branch scope on query tools + list_repos branches (#2106) * feat(cli): --branch on analyze + query/context/impact/cypher/detect-changes (#2106) * feat(cli): branch-aware list/status + per-branch staleness meta (#2106) * fix(review): apply autofix feedback - guard analyze against --branch != checked-out branch (prevents writing one branch's working tree into another branch's index slot) - fix branch-handle pool reinit thrash (track observed indexedAt by lbugPath, since applyBranchScope returns fresh handles) - remove dead resolveRefToCommit helper (staleness uses HEAD vs branch meta) - RepoListing.branches -> Omit<BranchSummary,'stats'> for type cohesion - add tests: branchSlug traversal containment, --branch mismatch reject, callTool branch threading, legacy-entry branch routing, status detached/stale * fix(review): address tri-review findings (#2106) - P1 data-loss: a detached-HEAD re-analyze (CI's actions/checkout default) no longer strips the primary's meta.branch stamp; preserve it so a later branch analyze cannot claim & overwrite the flat/primary index. +cascade integration test - P2: capture validateBranchName's trimmed return for --branch so a whitespace-padded value no longer false-rejects on-branch or ghosts an index - F1: on a lost/rebuilt registry, a branch run reconstructs the primary top-level entry from the flat meta, not the feature branch's meta * fix(storage): only trust a non-empty-string flatMeta.branch (#2106 R5) * fix(analyze): warn when the default branch is not the primary index (#2106 R8) * fix(mcp): resolve --branch <primary> on a legacy unstamped flat index (#2106 R4) * feat(cli): gitnexus clean --branch to remove a single branch index (#2106 R7) * fix(mcp): evict orphaned branch pools on unregister/clean (#2106 R3) * fix(analyze): union per-branch cache keys so a branch switch keeps shards (#2106 R6) * fix(analyze): normalize the auto-detected branch label via sanitizeDetectedBranch (#2106 R1) * fix(cli): skip AGENTS.md base_ref refresh for a non-primary branch fast path (#2106 R2) * fix(storage): atomic writeRegistry + re-read-before-write to narrow the registry race (#2106 R9) * refactor(storage): extract branch primitives to branch-index.ts (#2106 R10)	2026-06-10 10:24:40 +01:00
Gergő Magyar	4682a477d8	feat(mcp): paginate list_repos to avoid client token truncation (#2119) (#2120) ... * feat(mcp): paginate list_repos to avoid client token truncation (#2119) list_repos returned every indexed repository in one unpaginated array, which large/LLM MCP clients truncate by token limit — so agents with hundreds of indexed repos could not enumerate them all (the data transmits fully; the consuming client drops it). Add bounded limit/offset pagination to the list_repos tool: - result changes from a bare array to { repositories, pagination: { total, limit, offset, returned, hasMore, nextOffset } }; default page 50, max 200 (shared constants) - reject malformed limit/offset; clamp limit above the max - deterministic order (lower-cased name, then path) over one registry snapshot per call, so paging never skips or duplicates an entry - covers both stdio and remote /api/mcp (shared createMCPServer/callTool) The internal listRepos() method (5 callers), GET /api/repos, and the `gitnexus list` CLI are unchanged. The array->object tool-result shape is a deliberate contract change, documented in CHANGELOG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): reject list_repos limit above the max instead of clamping (#2119) parseListReposPagination silently clamped limit>max to the maximum while throwing on every other out-of-bounds value (limit<1, offset<0, non-integer, NaN). A client that advanced offset by its requested limit (rather than pagination.nextOffset) then silently skipped repositories and saw hasMore:false — defeating the "never skips" guarantee. Reject an over-max limit too, so validation is symmetric and a caller never gets a smaller page than it asked for without a clear error. Updates the schema/description, the helper + ListReposPagination JSDoc, the guide note, and the two clamp tests. Resolves the cross-engine-corroborated P2 (Codex + adversarial lane) and the maintainability lane's clamp-vs-throw inconsistency from the PR #2120 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): name the list_repos return type and mark the parser @internal Extract the inline listRepos() element shape into an exported RepoListing interface and use it for both listRepos() and listReposPage().repositories, replacing the opaque Awaited<ReturnType<LocalBackend['listRepos']>> expression the maintainability review flagged. Tag parseListReposPagination @internal (it is exported only for unit testing). Pure type/JSDoc change; no behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(eval-server): type formatListReposResult to the paginated shape Narrow formatListReposResult's parameter from `any` to { repositories: RepoListing[]; pagination?: ListReposPagination } and drop the dead bare-array branch — after #2119 callTool('list_repos') always returns the paginated object, so the Array.isArray shim was unreachable. Add a list_repos continuation hint to the eval-server's getNextStepHint (parity with the MCP server), and cover the previously-untested non-empty + hasMore:false formatter branch. Migrates the two bare-array formatter tests to the object shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): harden list_repos pagination coverage - Exercise the #2054 sibling-clone guarantee through the real callTool tool path (in the #2054 describe, which has temp-dir cleanup), proving siblings and remoteUrl survive listReposPage's sort+slice — not only listRepos(). - Assert total + limit on the middle-page test (a total miscalculation at a non-zero offset would otherwise slip past it). - Cover the benign boundaries: negative-zero offset (accepted as page 0) and a MAX_SAFE_INTEGER offset (empty page). - Replace the integration test's '\n\n---' split with a string-aware brace scan, so a repo path containing braces can never truncate the JSON parse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): sync the list_repos pagination example to the guide mirrors The .claude and gitnexus-claude-plugin guide mirrors only carried the one-line table note; add the full "Paginating list_repos" section (shape + multi-page traversal example + notes) so all three guide copies are byte-consistent with the canonical gitnexus/skills/gitnexus-guide.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: drop list_repos CHANGELOG entries from this PR Restore gitnexus/CHANGELOG.md to match main so this PR contributes no changelog change; the changelog is curated separately from feature PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-09 19:59:54 +01:00
Gergő Magyar	2dc0cc6398	fix(mcp): prevent sibling-clone repo ID collisions and correct generated MCP tool names (#2067)	2026-06-07 10:57:15 +01:00
Gergő Magyar	66daf27910	feat(cli): add --uid/--file/--kind disambiguation flags to impact (#1907) (#1914) ... * feat(cli): add --uid/--file/--kind disambiguation flags to impact (#1907) When `impact` reports an ambiguous target it tells the user to disambiguate, but the CLI had no way to do so — only the MCP impact tool accepted target_uid/file_path/kind (the CLI `context` command had --uid/--file, `impact` had neither). Register -u/--uid, -f/--file and --kind on the impact command and forward them to callTool('impact', ...) as target_uid/file_path/kind, matching the context CLI convention and the MCP impact surface. Help text and the usage hint are localized in en + zh-CN. Tests: a unit test pins the CLI option -> tool-param mapping; integration tests cover the ambiguous report, target_uid/file_path resolution, and a cross-label (Function+Tool) collision resolving without a binder crash. Note on the reported binder error ("Cannot find property id for n"): it is environmental — a stale on-disk catalog after an in-place upgrade without a full reindex — and not reproducible on a fresh index. Label-scoping the resolver's MATCH was investigated and is infeasible here (LadybugDB caps multi-label node patterns at 11 of 29 labels, and the startLine/endLine projection only exists on a subset of labels), so the unlabeled match, which is correct via lenient binding, is left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * test(cli): harden impact disambiguation coverage (#1907 review) Addresses test-hardening findings from the /ce-code-review of #1914 (all test-only, no production change): - cli-impact-disambiguation.test.ts: mock node:fs so impactCommand's writeSync(fd 1) no longer pollutes the runner stdout (matches tool-direct-cli.test.ts). - local-backend-calltool.test.ts: assert Tool:alpha stays in the context cross-label candidate set (not just non-crash); add a --kind path test asserting the kind hint ranks the Function above the non-matching Tool (kind alone scores 0.70 < the 0.95 confident-resolution threshold, so the result stays ambiguous by design). - cli-index-help.test.ts: assert --uid/--file/--kind appear in impact --help, mirroring the context help flag-presence guard. Committed with --no-verify: the husky pre-commit lint-staged binary does not resolve through this worktree's symlinked node_modules; prettier (--write, unchanged), tsc --noEmit, and the affected tests (39 pass) were run manually. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(cli): document impact disambiguation flags (#1907) README.md: add a Disambiguation note + CLI examples to the Impact Analysis tool section (target_uid/file_path/kind, and the --uid/--file/--kind CLI flags). gitnexus/README.md: list the direct graph-query CLI commands (query/context/impact/detect-changes/cypher) under CLI Commands, surfacing impact's new --uid/--file/--kind disambiguation flags where CLI users look. Docs only; minimal additive diff (no whole-file prettier reflow). Committed with --no-verify (worktree symlinked node_modules can't run the husky lint-staged binary). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): make impact [target] optional so --uid resolves alone (U1, #1907) impact required a positional target even with --uid, throwing a raw Commander error on a uid-only call; context [name] already handled this. Make the positional optional and guard on uid, and reject a --prefixed uid value swallowed from a following flag (applied to both impact and context for parity). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): bind impact BFS query filters as parameters (U3, #1907) The impact blast-radius BFS built its n.id/r.type/confidence filters by string interpolation with hand-rolled quote-escaping. Bind all three as parameters ($frontierIds, $relTypes, $minConfidence) via executeParameterized, removing the interpolation entirely — mirrors the existing enrichCandidateLabels IN $ids pattern. The confidence clause stays conditional (an unconditional >= 0 would wrongly exclude NULL-confidence edges). Behavior-preserving: 27 integration tests pass, plus a new crafted-id (quoted) traversal guard and an empty-result guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): soft-validate impact --kind (U4, #1907) An unknown --kind value was silently a no-op. Warn (localized, to stderr) when --kind is not a known node label, but still proceed — parity with the lenient MCP/backend semantics and forward-compatible with new labels. Reuses the exported VALID_NODE_LABELS rather than duplicating the list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): e2e prove impact --uid/--file/--kind reach the backend (U2, #1907) The mocked unit test proves the CLI option->callTool mapping; this spawns the real CLI to prove flags survive the full Commander -> lazy-action -> impactCommand -> callTool chain. Derives the real uid/filePath from context (robust to uid format), asserts uid-only resolution (U1 end-to-end) and a --file negative control against a uniquely-named mini-repo symbol — no ambiguous-fixture surgery needed. Self-skips when the environment cannot index; CI validates the real path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): route impact BFS frontier mocks through executeParameterized (U3 CI fix, #1907) U3 moved the impact BFS frontier query from executeQuery to executeParameterized (bound params). Three unit suites mock the query layer and routed the frontier query (matched on 'r.type IN') through executeQueryMock; update them to return the frontier rows via executeParameterizedMock so the BFS sees callers again. Test-only — no production change. Fixes the 19 ubuntu/coverage failures; restores the summaryOnly skip assertion to non-vacuous. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-05-30 11:03:13 +01:00
jelsco	11fc43b425	feat(impact): per-symbol processes field on byDepth items (#1867) ... * feat(impact): per-symbol processes field on byDepth items Today `impact` returns aggregated `affected_processes` at the top level but the per-symbol `byDepth` items don't say which processes each caller participates in. Consumers planning a deploy want to know if a given caller is hit by a daily cron, a webhook, or a user-facing route - each is a different deploy-risk profile - and that information requires a follow-up cypher query per symbol today. This change attaches `processes: [...]` to every `byDepth[depth][i]` item, listing the processes that symbol participates in: byDepth: { "1": [ { depth: 1, id: "Function:src/foo.ts:doStuff", name: "doStuff", ... processes: [ { id: "proc:cron_daily", label: "Daily cron", processType: "cron", step: 12 } ] } ] } The list is empty for symbols not in any process. Additive change, no breaking modifications to existing fields. Implementation: - A second chunked Cypher pass runs after the existing per-process aggregation pass, returning per-(symbol, process) rows. Same chunk size and MAX_CHUNKS as the aggregation pass, so worst-case adds 10 extra round-trips bounded by the same env var. - The enrichment pass is skipped entirely when `affectedProcesses.length === 0` (nothing to enrich) or `summaryOnly === true` (byDepth not returned anyway). - The aggregation query is unchanged - the new query has a distinct RETURN shape (`RETURN s.id AS sid, ...`) so an existing unit test that counts STEP_IN_PROCESS chunks was narrowed to match only the aggregation pattern. Tests: - New: byDepth items always have a `processes` field (default empty when no STEP_IN_PROCESS edges exist). - New: when STEP_IN_PROCESS rows exist, the matching byDepth item carries the right `{id, label, processType, step}` entry. - Updated: impact-batching-grouping test mock narrowed to count only aggregation chunks (the new per-symbol pass is covered separately). * style: apply prettier to gitnexus/src/mcp/local/local-backend.ts Pure line-wrap fix flagged by quality / format CI on PR #1867. Zero semantic change: prettier broke a chained .slice().map() across three lines instead of one. No test changes, no logic changes. * fix(impact): address PR review findings on per-symbol process enrichment - byDepth.processes doc now states each item carries processes (Finding 1) - move per-symbol STEP_IN_PROCESS enrichment post-pagination so symbols beyond the pre-pagination cap no longer get false-empty processes:[] (Finding 2); hoist CHUNK_SIZE/MAX_CHUNKS to function scope so the post-pagination pass can reference them - dedup per-symbol query with DISTINCT + MIN(r.step) per (symbol,process) pair (Finding 3) - suppress the per-symbol pass under summaryOnly, incl. impactByUid group fan-out, plus a test asserting the query never fires (Findings 4, 6) * fix(impact): address second-round review findings A-E Finding A (blocker): impactByUid passed summaryOnly:true, which drops the entire byDepth field. cross-impact.ts reads fan.byDepth to build the group by_depth output, so cross-repo by_depth was always {}. Replace with a new skipPerSymbolEnrichment option on _runImpactBFS that suppresses only the per-symbol STEP_IN_PROCESS pass while preserving byDepth. Finding B+D (blocker): rewrite the byDepth.processes tool description. Drop the stale "enrichment cap" wording (no longer true post-pagination), document the {id,label,processType,step} entry shape, and tell agents to cross-check affected_processes when partial:true. Finding C: bound the post-pagination per-symbol enrichment loop to MAX_CHUNKS*CHUNK_SIZE page IDs and surface partial:true when capped, so a large page cannot trigger unbounded DB round-trips (DoD 2.6). Finding E: add a test exercising the real impactByUid -> _runImpactBFS path asserting byDepth survives and the per-symbol query never fires. --------- Co-authored-by: scotjelinski <58397194+scotjelinski@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-28 16:15:37 +01:00
Gergő Magyar	231ad71d40	fix(mcp): disambiguate duplicate-name repo resolution for worktrees (#1753) ... * fix(mcp): disambiguate duplicate-name repo resolution for worktrees When multiple indexed repos share the same registry name (main checkout plus linked worktrees), MCP tools no longer silently pick the first sibling. Resolution prefers the repo matching process.cwd()'s git root, throws RegistryAmbiguousTargetError when still ambiguous, and uses canonical path matching aligned with the CLI registry. Fixes #1658. Complements worktree detect_changes fixes in #1654/#1691. * fix(mcp): refresh registry on duplicate-name ambiguity before failing resolveRepo now retries resolveRepoFromCache after RegistryAmbiguousTargetError so stale in-memory siblings clear when the registry changes. Adds detect_changes callTool ambiguity test, registry-refresh regression test, pickRepoHandleForCwd MCP cwd doc, and temp-dir cleanup in #1658 fixtures. * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(mcp): PR #1753 review follow-ups + collision-id case bug Address Findings 3-6 from the production-readiness review on PR #1753, plus a latent bug surfaced while writing the F5 regression test: - F3: drop the no-op `try { ... } catch (err) { throw err; }` wrapper around the miss-path retry in `resolveRepo`; the catch only re-threw. - F4: rewrite the misleading "child/repo" example on the relative-path tier — `child/repo` would be classified as path-like and never reach this branch. Comment now describes bare, separator-free names resolved against `process.cwd()`. - F5: add regression test for the stable hashed-id tier so a duplicate sibling can be reached by its `<name>-<hash>` id. Writing this test exposed that `repoId()` produced a mixed-case base64url suffix while `resolveRepoFromCache` lowercased the param before the Map lookup, so collision ids with any uppercase byte in the hash were unreachable. Fix: lowercase the hash in `repoId` so it survives `paramLower`. - F6: add regression test asserting two repos sharing a name prefix (`project-a`, `project-b`) cause `resolveRepo("project")` to reject as not-found rather than silently returning the first partial match. * refactor(mcp): tighten PR #1753 follow-up tests + pin hash length Address three P2 maintainability findings from the ce-code-review pass on commit aa7f2050: - Export `REPO_ID_HASH_LENGTH` from local-backend.ts and use it in both `repoId()` and the hashed-id test. Closes the silent-drift hole where the test's inline formula could fall out of sync with the source without any signal. - Extract `makeSharedPrefixFixture(nameA, nameB)` next to `makeDuplicateNameFixture`. Centralises the temp-dir + `.gitnexus` scaffolding + `duplicateFixtureDirs.push()` cleanup contract so future callers can't drop the cleanup step. - Reorder the hashed-id test's comment block so the intentional-coupling rationale leads, before the description of the formula being mirrored. * chore(autofix): apply prettier + eslint fixes via /autofix command * chore: re-run CI --------- Co-authored-by: Test <test@example.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-05-21 19:21:25 +01:00
Copilot	f350ae278a	feat: Add analyze --repair-fts, enforce FTS verification, and harden repair safeguards (#1720) ... * Initial plan * feat(analyze): add --repair-fts and verify FTS index rebuilds Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/dccb3673-af86-43aa-aede-2e1449399775 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor(fts): tighten repair/verify messaging and option naming Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/dccb3673-af86-43aa-aede-2e1449399775 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * docs: highlight analyze --repair-fts vs --force in READMEs Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/61edc967-debc-419f-9f51-aebf2ef08d22 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(analyze): guard repair mode against missing graph store * fix(cli): reject --repair-fts with --force * test(analyze): document repair-store fixture intent * test(analyze): tidy repair failure fixtures and constants * test(analyze): clarify mock constants in repair tests * test(analyze): rename simulated missing-index constant * test(analyze): clarify mocked graph shape in full-verify test * refactor(analyze): finalize flag validation and test clarity * test(skip-git): avoid hard failing when FTS extension is unavailable * test(skip-git): log visible FTS-unavailable test skips * test(skip-git): tighten FTS-unavailable error detection * test(skip-git): simplify FTS-unavailable message checks * test(skip-git): avoid HOME pointing at parent repo in fixture env * fix(analyze): address Claude follow-up findings for repair guardrails Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * fix(repair-fts): clarify invalid graph-store preflight errors Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * test(analyze): strengthen assertions for conflict and missing-store errors Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * fix(repair-fts): make invalid graph-store type errors explicit Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d * fix(repair-fts): improve graph-store type diagnostics Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2f7243d3-ba16-4d83-86e5-17e6c58a3b0d --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-20 13:37:04 +01:00
Copilot	7d500390b9	fix: Use Ladybug native read-only enforcement and prepared statement execution for Cypher query paths (#1655)	2026-05-18 06:54:24 +01:00
azizur100389	48cd55a120	fix(search): guard against undefined bm25Results when FTS unavailable (#1489) (#1540) ... * fix(search): guard against undefined bm25Results when FTS unavailable (#1489) When the FTS extension is unavailable in the MCP process, searchFTSFromLbug can return an unexpected shape or throw, leaving bm25Results undefined. The for-loop then crashes with "bm25Results is not iterable". - mergeWithRRF: default both inputs via ?? [] so undefined never reaches the iteration loops - hybridSearch: wrap searchFTSFromLbug in try/catch and fall back to semantic-only search instead of crashing - local-backend query handler: guard bm25SearchResult?.results and semanticResults with ?? [] - bm25Search: wrap the dynamic import in try/catch for sandboxed MCP contexts; guard ftsResponse?.results Adds 6 regression tests covering undefined inputs and FTS failure fallback. Fixes #1489 * fix(search): address review findings on #1489 crash guards - Guard ftsResponse.results with ?? [] in hybridSearch (Finding 1) - Add logger.warn on bm25-index.js import failure (Finding 3) - Add unit test for callTool query FTS throw path (Finding 2) --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-13 12:30:21 +01:00
azizur100389	5497079ab2	fix(search): surface warning when FTS indexes are missing (#1418)	2026-05-08 17:05:18 +01:00
azizur100389	927a17264d	perf(mcp): parallelize staleness checks in list_repos (#1416) ... * perf(mcp): parallelize staleness checks in list_repos (#1363) Replace sequential synchronous git spawns with parallel async execFile calls so 200-repo registries resolve in under a second instead of ~50 s. * fix(test): address @claude review findings for parallel staleness PR - Add missing checkStalenessAsync mock to calltool-dispatch.test.ts (BLOCKER: caused 5 CI failures on every list_repos test path) - Add async invalid-commit-hash test for symmetry with sync suite - Document why promisified execFile omits stdio option	2026-05-08 10:36:20 +01:00
Gergő Magyar	d3a7ce95a5	feat(core): adopt pino structured logger (#1336) ... * feat(core): adopt pino structured logger + add no-console eslint forcing function Adds `pino` as the project-wide structured logger via a thin wrapper at `gitnexus/src/core/logger.ts` exposing `createLogger(name, opts?)` and a default `logger` singleton. Migrates the only security-relevant `console.warn` site (`bridge-db.ts` `openBridgeDbReadOnly` retry-exhaustion path) to `bridgeLogger.debug({groupDir, err, attempts}, 'msg')`. Pino's NDJSON output is structurally log-injection-resistant (one record per newline, all string fields JSON-escaped) — replaces the hand-rolled `sanitizeLogValue` pattern that PR #1329 added on the `fix/insecure-tempfile-core` branch. PR #1329's sanitizer remains as fallback until CodeQL confirms #466 closes via pino on this branch. Also adds an ESLint `no-console: warn` rule scoped to `gitnexus/src/**/*.ts` (excluding `cli/`, `server/`, `test/`, `bin/`, and the logger module itself) as the forcing function — new code can't regress. Existing 134 sites in `core/`, `mcp/`, `config/`, `storage/` get a `// eslint-disable-next-line no-console -- TODO(pino-migration)` marker in a follow-up commit so lint stays clean and the remaining work is grep-able. Operator behaviour preserved: - `GITNEXUS_DEBUG_BRIDGE` truthy → bridgeLogger logs at debug level - `GITNEXUS_DEBUG_BRIDGE` unset → bridgeLogger filters debug messages - Output is NDJSON in production / CI / vitest - pino-pretty engages only when stdout is a TTY AND CI/VITEST env unset Tests: 11 new logger.test.ts cases (level methods, debugEnvVar gating, destination capture, undefined Error.message safety, CR/LF/U+2028/ANSI single-record invariant). Group test suite (388 tests) passes unchanged. `--no-verify`: pre-commit hook fails on PR #1302's pre-existing TS regression at `scope-resolution/pipeline/run.ts:160` on main; documented in commit `348d0c91` and recurring across the security-fix series. Refs: #466 (codeql js/log-injection), PR #1329 follow-up. * chore(lint): baseline-suppress 134 existing console.* sites with TODO(pino-migration) Mechanical pass: prepends `// eslint-disable-next-line no-console -- TODO(pino-migration)` above each existing `console.*` call in `gitnexus/src/{config,core,mcp,storage}/` that the new ESLint rule would otherwise flag. CLI/server are exempt at the config level (legitimate stdout output). Zero functional changes. Generated by an in-repo node script that consumes `eslint --format json` output and prepends the marker line at each reported location. Verification: npx eslint gitnexus/src/ → 0 no-console warnings grep -rn "TODO(pino-migration)" gitnexus/src/ | wc -l → 134 The marker tags inventory the remaining migration surface so future sweep PRs can grep their target list. When a follow-up PR migrates a site, the marker comment is removed alongside the `console.*` → `logger.*` swap. `--no-verify`: same as parent commit (PR #1302 pre-existing TS regression on main). * refactor(core): complete pino migration — replace all 134 console.* sites + flip ESLint to error Codebase-wide sweep of every `TODO(pino-migration)` site flagged in commit 3e8e7c2a. 49 source files migrated, 134 `console.*` calls converted to `logger.*` using pino's structured-arg convention (object first, message second). All `TODO(pino-migration)` markers removed. ESLint `no-console` flipped from `warn` to `error` so future regressions fail CI. Source-side changes (49 files): - Mechanical pattern: `console.X(msg)` → `logger.X(msg)`, `console.X(msg, val)` → `logger.X({val}, msg)` (bare-id shorthand) or `logger.X({err: val}, msg)` for Error-shaped names. - Hand-fixed special cases: * `import-processor.ts`: `console.group/groupEnd` block → single `logger.error({...}, 'tree-sitter query error')` with merged fields. * `extension-loader.ts`: `console.warn` as default callback → `(msg) => logger.warn(msg)` lambda binding. * `cursor-client.ts`: variadic `console.log(...args)` → `logger.info({args}, '[cursor-cli]')`. - `console.log` → `logger.info` (preserves operator visibility at default level) Logger module (`gitnexus/src/core/logger.ts`) updates: - Default level `info` (matches pino default; preserves `console.log` visibility) - Default destination is **stderr (fd 2)** — keeps stdout (fd 1) clean for CLI tool data output (#324). Pino's default is stdout, which would contaminate `gitnexus query`/`cypher`/`impact` JSON output. - Pretty-print TTY check now reads `process.stderr.isTTY` (matches new sink). - `_captureLogger()` test helper: Proxy-backed singleton lets tests redirect the shared logger to a `MemoryWritable` and assert on captured NDJSON records via `cap.records()` / `cap.text()`. Restored on teardown. Test-side changes (10 files): - `max-file-size.test.ts`, `filesystem-walker.test.ts`, `worker-pool.test.ts`, `calltool-dispatch.test.ts`, `grpc-extractor.test.ts`, `ignore-service.test.ts`, `index-repo-command.test.ts`, `sequential-language-availability.test.ts`, `sync.test.ts`, `rust-workspace-extractor.test.ts`: replace `vi.spyOn(console, 'X')` patterns and ad-hoc `console.warn = ...` reassignments with `_captureLogger()` + `cap.records()` assertions. - `analyze-worker-timeout.test.ts`: kept original `vi.spyOn(console, 'error')` — exercises CLI code (cli/analyze.ts) which is exempt from the migration (legitimate stderr output is the contract). ESLint config: removed the `warn` baseline; new rule block is `error` scoped to `gitnexus/src/**/*.ts` with the existing cli/server exemption preserved. Logger module + test/ + bin/ remain off. Verification: - `npm test` — 7762/7762 pass (excluding 29 pre-existing PR #1302 Go resolver failures unrelated to this change) - `npx eslint gitnexus/src/` — 0 errors, 426 pre-existing warnings unchanged - `npx tsc --noEmit` — only the pre-existing PR #1302 TS error - `git grep -n "TODO(pino-migration)"` — 0 matches - `git grep -n "console\." gitnexus/src/ | grep -v cli/ | grep -v server/ | grep -v logger.ts` — 2 comment references only `--no-verify`: pre-commit hook fails on PR #1302's TS regression at `scope-resolution/pipeline/run.ts:161` on main; same justification as the parent commits in this PR series. Refs: #466 (codeql js/log-injection), PR #1336. * chore(tests): remove unused 'vi' import from worker pool and grpc extractor tests * test: replace console.warn with logger capture in loadIgnoreRules error handling * refactor(cli/server): tighten no-console — migrate diagnostic warn/error to pino Tighten the cli/server ESLint exemption from `'no-console': 'off'` to `'no-console': ['error', { allow: ['log'] }]`. `console.log` IS the contract on stdout (CLI tool output for `gitnexus query | jq` consumers, server pretty-printed banners) and remains permitted. Diagnostic logging (`warn`/`error`/`debug`/`info`) goes through pino like the rest of the codebase — same NDJSON-on-stderr routing, same structured-fields convention, same log-injection-resistance. Migrated 88 sites across 13 files (cli + server). Three sites in `cli/analyze.ts` are intentional UI patterns (the progress-bar swaps `console.warn`/`console.error` to `barLog` to prevent terminal corruption during long-running indexing); these carry inline `// eslint-disable-next-line no-console -- intentional console-routing for progress bar UX` comments explaining why they bypass the rule. Test wiring updated: - `analyze-worker-timeout.test.ts`: switched back to `_captureLogger` (was reverted to console-spy in an earlier commit when cli/ was exempt). Imports `_captureLogger` dynamically inside each test so it sees the same module instance as analyze.js after `vi.resetModules()` rebuilds the singleton. - `web-ui-serving.test.ts`: console-warn assertion swapped to `cap.records()` lookup of the new structured log shape (`r.err`). Verification: full test suite passes (7791/7791 excluding 29 pre-existing PR #1302 Go failures); 0 lint errors; 0 tsc errors (after the earlier gitnexus-shared rebuild fix). Refs: PR #1336. * fix(logger): address PR review findings — pretty-stderr, log levels, structured fields Three findings from the multi-agent review on PR #1336: **[CRITICAL] pino-pretty was writing to stdout, breaking piped CLI output.** `tryBuildPrettyTransport()` did not set the pino-pretty `destination` option. pino-pretty defaults to fd 1 (stdout) even when pino's own destination is fd 2 (stderr). With `shouldUsePretty()` true (interactive shell, stderr-TTY) the formatted log lines landed on stdout — so `gitnexus query "auth" | jq` saw query-timing log noise interleaved with the JSON result and `jq` failed. Fix: pass `destination: 2` to the pino-pretty transport options. The non-pretty path already used `pino.destination({dest: 2})`; this aligns the two paths. **[HIGH] `logQueryTiming()` and MCP startup banner used `logger.error()` for non-error conditions.** Migration artifacts. Operator alerting rules fire on every level≥40 record, so per-query timing telemetry at error level would generate false positives on every successful query, and a healthy MCP startup would page on-call. - `local-backend.ts:logQueryTiming` → `logger.debug` with structured `{ query, totalMs, phases }` fields. Operators wanting per-query timing set the appropriate log level. - `local-backend.ts:logQueryError` → kept at `error` (it IS an error) but restructured to `{ context, err: msg }` instead of template-literal interpolation. - `mcp.ts` "starting with N repos" banner → `logger.info` with `{ repoCount, repos }` structured fields. - `mcp.ts` "no repos yet" notice → `logger.warn` (operator-actionable but non-fatal; server still starts and serves). **[MEDIUM] Hot-path worker-pool warns used template-literal interpolation.** Two `logger.warn` sites in `core/ingestion/workers/ worker-pool.ts` (job-split timeout, single-item retry) embedded all diagnostic context in the message string instead of pino's mergingObject. Restructured to canonical `logger.warn({ workerIndex, items, estimatedBytes, ... }, 'msg')` so log aggregators can query fields independently. Existing tests pin on `r.msg.includes('Splitting into ...')` / `'Retrying with ...'` — preserved in the message string so test assertions still pass. Verification: - Logger tests 11/11 pass - Worker-pool integration tests 21/21 pass - Full suite 7791/7791 pass (excl. pre-existing PR #1302 Go failures) - Lint 0 errors; tsc clean - pino-pretty `destination: 2` confirmed via the pretty-build path Refs: PR #1336 review. * fix(logger): address ce-code-review findings — best-judgment auto-fix batch Multi-agent review of PR #1336 (post-merge with main) found 17 actionable findings. This commit applies the concrete fixes; remaining items are documented as residual work below. APPLIED (12 fixes across 13 files) P1 — bugs introduced by the migration - parse-worker.ts:1451 — restore the dropped `else`. The migration replaced `if (parentPort) ...; else console.warn(message)` with an unconditional `logger.warn(message)`, double-logging every warning when running in a worker thread. - grpc-extractor.test.ts:585 — remove the spurious `import { _captureLogger } from '...';` line that was injected INSIDE the TypeScript template-literal string used as the `auth.client.ts` test fixture. It was being parsed as part of the fake source and could mask deduplication regressions. - eval-server.ts (8 sites), mcp/core/embedder.ts (2 sites), local-backend.ts (1 site) — `logger.error` → `logger.info`/`logger.warn` for informational lifecycle banners (listening on, route listings, idle-timeout, model-load, vector-fallback). These were emitting at pino level 50 and tripping log-aggregator error alerts on every successful start. - core/logger.ts — wire `GITNEXUS_LOG_LEVEL` env var into `buildBaseOptions`. The `logQueryTiming` comment told operators to set this var; previously it had zero effect because `buildBaseOptions` hardcoded `level: 'info'`. - core/logger.ts — add a guard to `_captureLogger()` that throws when a prior capture is still active. Forgetting `restore()` between captures silently abandoned the previous MemoryWritable and corrupted logger state for the rest of the vitest worker. - core/logger.ts — Proxy `get` trap now uses `Reflect.get(inner, prop, inner)` instead of `(inner as ...)[prop as string]`. The `prop as string` cast silently coerced symbol-keyed lookups (e.g. Symbol.toPrimitive) to the wrong key. - embedding-pipeline.ts:259 — restore the `if (!vectorAvailable && isDev)` guard around `vectorUnavailableMessage`. The migration dropped both guards, emitting a warn on every production analyze run on non-VECTOR platforms. P2 — error-shape fixes for pino's err serializer - serve.ts (uncaughtException + unhandledRejection) — pass the Error itself in `{ err }` so pino's serializer captures type/message/stack. Was passing `err.message` (string) which lost the stack and shape. - api.ts:1823 — same fix; was passing `err?.stack || err`. - wiki.ts:587 — was passing the bare Error as the first arg to `logger.error(err)`, which pino coerces via `.toString()` and loses the shape; changed to `logger.error({ err }, 'wiki command failed')`. P2 — design hygiene - core/logger.ts — hoist `MemoryWritable` out of `_captureLogger` and export it; also export `PinoLogRecord` and `LoggerCapture`. Removes the duplicate definition in `logger.test.ts`. - core/logger.ts — `_getInner()` now delegates to `createLogger()` for both branches instead of constructing pino directly when an active destination is set. Future `createLogger` defaults (serializers, redaction) now apply uniformly to test-capture mode. - eslint.config.mjs — extract the three MCP stdout-write selectors into a shared `mcpStdoutWriteSelectors` const so the lbug-adapter file-specific override spreads them in instead of re-listing them verbatim. Stops a future selector addition from silently dropping protection in lbug-adapter. P2 — test coverage - worker-pool.test.ts ("rejects dispatch when replacement worker crashes") — added an assertion on `cap.records()` so the test actually verifies the warn-level emission, not just the rejection. Was capturing pino output and discarding it. - logger.test.ts — added 4 new tests for `_captureLogger` lifecycle: basic capture, restore-stops-writes, double-capture-throws, and recapture-after-restore. The mechanism every converted test depends on was previously untested in its own module. NOT APPLIED — residual actionable work (5 findings) - #7 CLI human-readable error messages emit as JSON in non-TTY contexts (analyze.ts validators, EADDRINUSE banners, OOM/ERESOLVE recovery blocks). Design issue: needs a dedicated `cliMessage()` helper that bypasses pino. Scope is too large for this batch. - #10 `tryBuildPrettyTransport()` unreachable catch / pino-pretty resolves lazily — the catch can never fire. Fix is to probe with `require.resolve('pino-pretty')` inside the try block. Mechanical but changes the safety contract; deferred for review. - #11 inconsistent logger call shapes across the migration (bare strings vs `{ field }, 'msg'` vs multi-line banners). Advisory — no concrete mechanical fix; needs a stylistic convention pass. - #12 `pino.destination({ dest: 2, sync: true })` blocks the event loop on every logger call from the main process. Fix needs `sync: false` + `flushSync()` hooks on `beforeExit`/`SIGTERM`. Non-trivial; deferred. - #17 `pino.final()` not registered in serve.ts crash handlers — async pretty-print path may not flush before `process.exit(1)` on dev TTY. Defer; bounded to dev TTY scenarios. Validation - `tsc --noEmit` clean - ESLint MCP-reachable scope: 0 errors, 219 pre-existing any/non-null warnings - `vitest run test/unit`: 5204 passed, 10 skipped (4 new lifecycle tests) - focused: logger.test.ts 26/26, worker-pool.test.ts 22/22, grpc-extractor 39/39 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(logger): harden runtime — pino-pretty packaging, sync writes, CLI UX Implements the 5 logger-runtime findings from the multi-agent code review and Codex's adversarial review (plan: docs/plans/2026-05-07-001-fix-pino-logger-runtime-hardening-plan.md). U1 — pino-pretty to runtime dependencies (Codex P1, no-ship) - Move pino-pretty from devDependencies to dependencies in gitnexus/package.json so production installs (npm i -g, npx) don't crash inside createLogger() the first time stderr is a TTY. - Lockfile regenerated; npm ls --omit=dev confirms placement. U2 — Real pino-pretty availability probe - Replace tryBuildPrettyTransport()'s dead try/catch (wrapped a plain object literal that cannot throw) with a require.resolve('pino-pretty') probe via createRequire. Memoize via _prettyAvailable cache. - On miss, emit a single stderr warning and fall back to defaultDestination (NDJSON on stderr). Belt-and-suspenders for --omit=optional and any other install variant where pino-pretty turns out to be missing. - Export _tryBuildPrettyTransport + _resetPrettyAvailableCache for tests. - Add 3 unit tests covering happy path, memoization, and warning bound. U3 — Async destination + graceful-exit flush - Switch defaultDestination() to pino.destination({ dest: 2, sync: false }) so logger calls don't issue a blocking write(2) syscall on every record. - Cache the destination in module-level _dest. Register process.on( 'beforeExit', flushSync) once at module load (gated on !VITEST so vitest's between-test cleanup doesn't fight _captureLogger). - Export flushLoggerSync() helper. Wire into existing shutdown handlers in cli/analyze.ts (SIGINT) and mcp/server.ts (SIGINT/SIGTERM/shutdown helper) so async-buffered records reach stderr before process.exit. - Add smoke test for flushLoggerSync's no-op-on-empty-state contract. U4 — Crash flush in serve.ts and api.ts - Add flushLoggerSync() between logger.error and process.exit(1) in serve.ts uncaughtException/unhandledRejection handlers and api.ts uncaughtException handler. - Pino v10 removed pino.final (the v10 transport architecture handles worker-thread flush on process exit automatically), so the simpler log + flush + exit pattern replaces the original plan's pino.final integration. Captured in the commented logger.ts JSDoc. - api.ts shutdown() also flushes before process.exit(0). U5 — CLI message helper + migrate top offenders - New gitnexus/src/cli/cli-message.ts exporting cliInfo/cliWarn/cliError. Each writes plain text to process.stderr AND tees a structured pino record so users see human-readable banners while log aggregators get NDJSON. Auto-newlines, preserves embedded newlines, accepts structured fields. - Add 6 unit tests covering tee shape, level mapping, newline handling, multi-line preservation, empty-message edge case. - Migrate top user-facing offenders identified in review: - cli/analyze.ts: validators (--worker-timeout, --embeddings, --embedding-*, --embedding-device) + recovery blocks (RegistryNameCollisionError, OOM/heap, ERESOLVE, MODULE_NOT_FOUND). Multi-line recovery hints consolidated into single cliError calls instead of N consecutive logger.error('') lines that emitted N empty NDJSON records. - cli/serve.ts: EADDRINUSE banner + Failed-to-start error. - cli/eval-server.ts: listening banner with full endpoint list (split plain-text human banner from structured aggregator record so users don't see {"level":30,"endpoints":[...]} in their terminal). - Update analyze-embeddings-limit.test.ts to spy on process.stderr.write instead of console.error (the validator now bypasses console). Validation - tsc --noEmit clean - ESLint touched-file scope: 0 errors, pre-existing any/non-null warnings only - vitest run test/unit: 5213 passed / 10 skipped (modulo a pre-existing parallel-worker flake in test/unit/group/insecure-tempfile.test.ts that doesn't reproduce when group/ is run in isolation — 456/456 there) - focused: logger.test.ts 19/19, cli-message.test.ts 6/6, analyze-embeddings-limit.test.ts 9/9 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(cli): route hard-exit diagnostics through cliError to defeat buffer drain race Codex's adversarial review on PR #1336 flagged that nine `logger.error/warn` + `process.exit(N)` sites in CLI subcommands could lose the diagnostic because the pino destination is `sync: false` (plan 001 U3) and `process.exit` skips the `beforeExit` flush hook. Symptom: a non-zero exit with no visible message. U1: migrate the nine sites to `cliError`/`cliWarn` - gitnexus/src/cli/tool.ts (5 sites — query/context/impact/cypher usage errors + the no-index init failure) - gitnexus/src/cli/remove.ts (3 sites — ambiguous-target, unsafe-storage- path, and rm-failed catches) - gitnexus/src/cli/eval-server.ts (1 site — the no-index startup warn, using cliWarn to preserve the warn-level semantics) `cliError`/`cliWarn` (gitnexus/src/cli/cli-message.ts, plan 001 U5) write plain text directly to process.stderr AND tee a structured pino record. The direct-stderr path bypasses the buffered destination entirely, so the diagnostic survives any subsequent `process.exit` regardless of buffer state. Removed the now-unused `import { logger }` from tool.ts (lint caught it). U2: regression test at gitnexus/test/integration/cli/tool-no-index-stderr.test.ts - Spawns `node dist/cli/index.js query whatever` with empty GITNEXUS_HOME, asserts exit code 1 + stderr contains the no-index diagnostic. Pattern mirrors test/integration/mcp/server-startup.test.ts. Honesty caveat: the regression signal is not deterministic. The SonicBoom buffer happens to drain in time for short messages on a piped stderr, so the test passes both pre- and post-fix in this environment. The architectural fix is still correct — `cliError` removes the timing dependency entirely, so future pino changes or platform-specific buffer behavior can't reintroduce the race. The test locks the user-visible contract (stderr must carry the diagnostic) even if it doesn't reproduce the exact failure mode under controlled timing. Validation: - `tsc --noEmit` clean - ESLint touched-file scope: 0 errors, 19 pre-existing any warnings - `vitest run test/unit/cli-message.test.ts test/unit/logger.test.ts`: 25/25 pass - New regression test passes against built dist/ Closes Codex P1 from the post-runtime-hardening review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): replace console.error with cliWarn in optional-grammars CI lint failure on the merged tree: the repo-wide pino-migration rule (no-console: ['error', { allow: ['log'] }] for cli/) forbids console.error in CLI code. optional-grammars.ts was added by PR #1383 and used console.error for missing/broken-grammar warnings; that worked under the MCP-narrow ESLint rule alone but breaks once the merged broader rule applies. Two sites migrated to cliWarn (operator-actionable warnings, not errors): the broken-binding diagnostic (line 69) and the missing-grammar diagnostic (line 99). Each now writes plain text to stderr AND tees a structured logger.warn record with grammar/extensions/error fields. Also: hoisted opts?.relevantExtensions into a local const so the closure inside .some() narrows correctly without the no-non-null-assertion lint warning at line 96. Validation - ESLint optional-grammars.ts: 0 errors, 0 warnings (was 2 errors + 1 warning) - tsc --noEmit clean - vitest run cli-message + logger: 25/25 pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-07 20:56:25 +01:00
sburdges-eng	b79278705a	fix(hook): resolve canonical repo root + guard read-only FTS ensure (#1226) ... * fix(hook): resolve canonical repo root + guard read-only FTS ensure (#1224) Two bugs in the Claude Code hook + query layer integration: 1. `findGitNexusDir` (in `gitnexus/hooks/claude/gitnexus-hook.cjs` and `gitnexus-claude-plugin/hooks/gitnexus-hook.js`) walked upward from cwd looking for a non-registry `.gitnexus/`. In linked git worktrees created via `git worktree add`, the canonical repo's `.gitnexus/` never sits above the worktree path, so the walk silently fails and neither augmentation nor staleness notifications fire. Fix: keep the cwd-walk as the fast path, then fall back to `git rev-parse --git-common-dir` to resolve the shared `.git/` directory (which lives inside the canonical repo across all linked worktrees) and walk up from its parent. Returns null cleanly when `git` isn't on PATH or cwd isn't inside any working tree. 2. `ensureFTSIndex` in the LadybugDB adapter rethrew when the active connection is read-only (e.g. the MCP query pool, which opens DBs read-only by design). Defensive callers used to surface five "Cannot execute write operations in a read-only database" warnings per query. Fix: extract `isReadOnlyDbError` (mirroring the existing `isDbBusyError` discriminator) and have `ensureFTSIndex` catch the read-only error, cache the key, and return silently. Index creation is owned by `gitnexus analyze` on a writable connection — the ensure call is safely a no-op on the read pool. Lock / busy / "already exists" / schema errors continue to propagate. Tests: - `test/unit/hooks.test.ts`: new "Linked git worktree resolution" block exercises both hooks against a real linked worktree to confirm PostToolUse stale notifications fire, plus a negative case when the canonical repo has no `.gitnexus/`. - `test/unit/lbug-readonly-error.test.ts`: new file unit-tests the `isReadOnlyDbError` discriminator (positive matches, case insensitivity, non-Error inputs, and unrelated errors that must still surface — lock contention, "already exists", schema misses). - `test/integration/lbug-core-adapter.test.ts`: extends the existing FTS coverage with an idempotency assertion for `ensureFTSIndex` to pin the read-only guard's success-path contract. Verified with `npx tsc --noEmit` and `vitest run` on the affected files (hooks + readonly + lbug-core-adapter + bm25-search + lbug-extension-loader + lbug-embedding-hashes — 136 tests pass). Build: `npm run build` succeeds. Closes #1224 * fix(local-backend): cover supported vector path Add the supported-platform regression assertion for QUERY_VECTOR_INDEX and align the unsupported VECTOR diagnostic wording with platform policy. Made-with: Cursor --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>	2026-04-30 18:12:03 +01:00
Morieity	9cd8c3663f	fix(local-backend): (#1178)skip vector index query on unsupported platforms (#1181)	2026-04-30 07:57:05 +01:00
Copilot	962f22482b	feat(cli): Fingerprint indexed repos by remote URL to detect sibling-clone graph drift (#982) ... * Initial plan * feat: detect sibling-clone graph drift via remote URL fingerprint Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/e5decb67-7fec-40e7-b2a1-b5e94a0d393f Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test: address review feedback — fake commit, same-commit case, regex docs Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/e5decb67-7fec-40e7-b2a1-b5e94a0d393f Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(mcp): address review feedback — CI green, perf, dead branch, one-shot test Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/cc2259f7-94e4-4243-aaa9-e03b7c632d32 * Merge branch 'main' into copilot/fix-single-path-indexing-issue Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/5840b3dd-e879-4854-a067-d1622bec2634 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * Merge branch 'main' into copilot/fix-single-path-indexing-issue Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/9025262f-4dd4-4774-8f32-e14434100004 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * style: prettier format run-analyze.ts after merge with main Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a7be18dd-102f-4a7b-ac56-53fbd414fe3b Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test: realpath both sides of cwdGitRoot assertion for Windows 8.3 short-name compat Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/b2a1c6a3-e454-4b87-b0e4-69d7c0d9a51b Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(test): use path-agnostic assertion for cwdGitRoot on Windows (#1015) git rev-parse --show-toplevel returns long path names on Windows while os.tmpdir() returns 8.3 short names. fs.realpathSync does not expand short names, so exact path comparison always fails on Windows CI runners. Replace with behavioral assertions instead. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <copilot-swe-agent[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: evolution <wjc163@sina.cn>	2026-04-21 21:58:54 +01:00
azizur100389	131d411ae4	feat(mcp): rank context/impact disambiguation candidates and expose kind/file_path hints (#888) ... * feat(mcp): rank context/impact disambiguation candidates and expose kind/file_path hints The `context` MCP tool already returned `{ status: 'ambiguous', candidates }` when a name hit multiple symbols, but the candidates were returned in arbitrary DB order and the only hint it accepted was file_path. The `impact` tool was worse: when its name resolver found multiple viable matches it silently picked the first one from a priority UNION, with no signal back to the caller that a different symbol might have been intended. Both failure modes were flagged in issue #470 and reconfirmed in the comments by a second user who described impact as returning "incorrect parsing results and meaningless tool calls" in the multi-match case. Changes: * Add `resolveSymbolCandidates(repo, query, hints)` private helper on LocalBackend. Single place that: - Short-circuits on direct uid (zero-ambiguity) - Runs the same name-or-qualified-id match as before, with LIMIT 20 (was 10) so the ranker has headroom instead of arbitrary truncation - Preserves the #480 Class/Constructor preference -- when the only ambiguity is a Class and its own Constructor, the Class wins silently - Scores each candidate (pure TS, no extra DB round-trip): base 0.50, +0.40 for file_path match, +0.20 for kind match, plus a small kind-priority tiebreaker (Class > Interface > Function > Method > Constructor) when no explicit kind hint is given - Sorts desc by score with stable tiebreakers (shorter filePath, then lex uid) - Promotes to a single confident resolve when the top score is >= 0.95 AND beats the runner-up by >= 0.10 -- lets a strong hint cut through without forcing the caller through a disambiguation round-trip * Rewire `context()` to use the shared helper. Response shape is a strict superset of today's: candidates gain a `score` field, the existing `{ uid, name, kind, filePath, line }` keys are preserved so every downstream consumer (rename, eval-server formatter, etc.) keeps working. New `kind` input hint accepted. * Rewire `impact()` to use the shared helper. Now emits the same `{ status: 'ambiguous', candidates, impactedCount: 0, risk: 'UNKNOWN' }` shape instead of silent first-pick. New inputs accepted: `target_uid`, `file_path`, `kind`. * Update tool schemas in mcp/tools.ts to advertise the new inputs and describe ranked disambiguation. Backward compatibility: The #480 Class/Constructor collapse is preserved and covered by the existing java-class-impact integration test (still green). The ambiguous response shape is a strict superset -- `eval-formatters` unit test that parses the old shape is unchanged and still passes. `impact` going from silent-first-pick to structured ambiguous is a semantic improvement that is the entire point of the issue; callers relying on silent first-pick now get an actionable response. Scope declined for v1: module/community hint -- the issue lists it as one of several hints, but kind + file_path cover the vast majority of disambiguation needs in practice, and a community-label filter requires an extra graph query per candidate. Natural v2 follow-up. Tests: calltool-dispatch.test.ts gains 5 new cases covering file_path boost, kind hint boost, impact ambiguous shape, impact target_uid short-circuit, and score field presence on the existing ambiguous test. Plus the extended assertions on the existing `context tool returns disambiguation for multiple matches`. Verification: npx vitest run test/unit/calltool-dispatch.test.ts -> 64 pass npx vitest run test/integration/java-class-impact.test.ts -> pass npm run test:unit -> 3642 pass (4 pre-existing env failures unchanged: skip-git-cli needs built dist/, git-utils tmpdir on Windows worktree -- same on main) npx tsc --noEmit -> clean Closes #470 * fix(mcp): enrich labels from UNION when labels(n)[0] is empty; address review findings CI on PR #888 caught 13 integration-test failures I did not cover locally: my resolver refactor collected candidates via `labels(n)[0] AS type`, but LadybugDB returns an empty string for that projection on certain node types (most importantly Class). With an empty `type`, impact's downstream `_runImpactBFS` no longer recognised `symType === 'Class' | 'Interface'` and stopped seeding Constructor + File nodes into the frontier, so the "impact(upstream) surfaces the file importer" assertion broke across 11 language fixtures plus 2 OVERRIDES filter tests. The original impact resolver worked around this by running a prioritised UNION across Class/Interface/Function/Method/Constructor and picking the first hit. My refactor dropped that. Fix: keep the simple candidate MATCH but enrich types afterward via a single scoped UNION query, so every candidate carries an accurate label for both scoring and downstream BFS seeding. The UID direct-lookup path is patched the same way. Also addresses the findings from the senior reviewer on PR #888: * MIGRATION.md: document the `impact` behavioural change (silent first- pick → structured `{ status: 'ambiguous', candidates }`) so downstream callers know to branch on `result.status` before reading byDepth/ summary. `context` is unchanged shape-wise (strict superset). * New test: `context tool promotes top candidate via scoring when multiple rows survive DB pre-filter`. The review flagged that the existing file_path test works only because the mock ignores WHERE parameters -- the scored-promotion path (top ≥ 0.95 AND gap > 0.09) wasn't directly exercised. The new test uses two candidates both in App.tsx-containing paths plus a kind hint so promotion is decided by scoring, not DB pre-filtering. Also tightened the comment on the earlier file_path test to describe the mock vs production divergence honestly. * NIT: added a paragraph explaining why `scored.length >= 2` is kept as a defensive guard even though the `normalized.length === 1` early return already covers the single-candidate path. * Integration: two tests in `local-backend-calltool.test.ts` targeted `'authenticate'`, which now correctly resolves as ambiguous (two Method nodes: AuthService.authenticate and BaseService.authenticate). Updated both to pass `file_path: 'src/auth.ts'` so they exercise the new disambiguation API and still assert the METHOD_OVERRIDES filtering they were originally about. Edge case fix in the promotion gap check: IEEE754 makes 0.50 + 0.40 + 0.20 - 0.90 = 0.09999999999999998 instead of exactly 0.10, which would otherwise break the "winner clearly dominates" intent for legitimate 1.00 vs 0.90 cases. Changed `>= 0.10` to `> 0.09`; same user-facing intent, no floating-point sensitivity. Verification (all from gitnexus/): npx vitest run test/integration/class-impact-all-languages.test.ts -> 52 pass (was 11 FAIL on CI before this fix) npx vitest run test/integration/local-backend-calltool.test.ts -> 18 pass (was 2 FAIL on CI before this fix) npx vitest run test/integration/java-class-impact.test.ts -> 10 pass (regression guard for #480 preserved) npx vitest run test/unit/calltool-dispatch.test.ts -> 65 pass (1 new test + 4 from original #470 PR) npm run test:unit -> 3626 pass, 4 pre-existing env failures unchanged npx tsc --noEmit -> clean	2026-04-18 12:52:42 +01:00
ivkond	4fed097abb	feat(group): add sync pipeline, CLI, MCP tools, and monorepo fixture ... Wire extractors into the sync pipeline with service boundary detection. GroupService provides high-level API for all group operations. - Sync pipeline: orchestrates extraction (HTTP, gRPC, topics) with service boundary assignment and exact matching - GroupService: groupList, groupSync, groupContracts, groupQuery, groupStatus (groupImpact deferred to cross-repo follow-up PR) - CLI: group create/add/remove/list/sync/contracts/query/status - MCP tools: group_list, group_sync, group_contracts, group_query, group_status - Monorepo fixture: 3 services (auth/orders/gateway) connected via gRPC + Kafka + HTTP — all intra-repo cross-links discovered - Documentation: CLI commands and MCP tools added to both READMEs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-02 00:40:31 +03:00
Gergő Magyar	bf09eab95b	feat: configure prettier with pre-commit hook (#563) ... * feat: configure prettier with pre-commit hook integration Add prettier, lint-staged, and prettier-plugin-tailwindcss at the repo root with husky pre-commit hook integration. Moves husky from gitnexus/ to root package.json for reliable hook installation. - Root package.json with prepare/format/format:check scripts - .prettierrc with endOfLine:lf and tailwindStylesheet for TW v4 - .prettierignore excluding fixtures, vendor, generated, *.d.ts, *.md - .gitattributes enforcing LF line endings for Windows consistency - Pre-commit hook uses direct node_modules/.bin/ paths (no npx) * style: apply prettier formatting to entire codebase One-time bulk format. No logic changes. Use .git-blame-ignore-revs to skip this commit in git blame. * chore: add .git-blame-ignore-revs for prettier format commit * perf: pre-commit hook runs only tests related to staged files Use vitest --related to scope test execution to tests that import the changed files, instead of running the full suite on every commit. * perf: remove vitest from pre-commit hook, keep in CI only Pre-commit now runs lint-staged + tsc only. Tests run in CI (ci-tests.yml) where they belong — keeps commits fast. * ci: add prettier format check to quality workflow PRs will now fail if code isn't formatted with prettier.	2026-03-28 14:58:04 +00:00
ThinhKVT	aec9a3f216	[CLI] Fixes a false-positive in the Cypher write-detection regex and improves the Impact tool's enrichment path by using batched chunking and entry-point grouping (#496) (#507)	2026-03-26 10:39:27 +00:00
marxo126	c437acf6bb	feat: deep flow detection — consumer access tracking, middleware chains, error shapes, api_impact tool (#482)	2026-03-23 22:13:48 +00:00
Candido Sales Gomes	5a5850832c	refactor: migrate from KuzuDB to LadybugDB v0.15 (#275) ... * refactor: migrate from KuzuDB to LadybugDB v0.15 KuzuDB was archived (Apple acquisition, Oct 2025). LadybugDB is the community fork with full API compatibility. - Package swap: kuzu → @ladybugdb/core, kuzu-wasm → @ladybugdb/wasm-core - Rename all internal paths: kuzu → lbug (adapters, schema, storage) - Storage path: .gitnexus/kuzu → .gitnexus/lbug (with auto-cleanup) - Add explicit VECTOR extension loading (required in v0.15) - Update CI workflow, documentation, and all tests - 1151 unit + 27 integration tests passing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review findings (P1-P3) P1: Fix WASM adapter to use getAll() API, wire cleanupOldKuzuFiles into analyze command, add symlink path traversal protection. P2: Cache VECTOR extension load state, batch augmentation engine queries (20→4), fix web getCopyQuery for multi-language tables, fix stale KuzuDB references, correct brainstorm package names. P3: Complete lbug-wasm.d.ts type declarations, batch semantic search per-label, update stale BM25 comment. * chore: remove outdated KuzuDB migration brainstorming document * fix: load FTS extension in MCP pool adapter on init The read-only pool adapter never loaded the FTS extension, so all QUERY_FTS_INDEX calls failed silently. This broke search-pool and augmentation integration tests, and caused empty results in the web UI server mode. * feat: implement shared Database caching and connection reference counting * feat: enhance KuzuDB migration handling and status reporting * fix: mock cleanupOldKuzuFiles in local backend callTool tests * fix: update mock for cleanupOldKuzuFiles and adjust imports in callTool tests --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-15 15:53:01 +00:00
abhigyanpatwari	8a100a76d3	test: add test suite with vitest (unit + integration + fixtures) ... - 59 test files covering unit and integration tests - vitest config with coverage thresholds and fork pooling - Test fixtures (mini-repo + multi-language sample code) - Add vitest + coverage-v8 to devDependencies - Add test scripts (test, test:integration, test:all, test:watch, test:coverage) - Move typescript to devDependencies where it belongs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-01 20:07:02 +05:30