193 commits

Author	SHA1	Message	Date
Gergő Magyar	9538be957d	fix(lbug): scale the buffer-pool budget by the OS page-size granule ratio (#2631) (#2636) ... * fix(lbug): scale the buffer-pool budget by the OS-page discard-granule ratio (#2631) LadybugDB bills buffer-pool budget per discard granule, not per 4 KiB frame: the engine's vm_region.cpp sets discardGranuleSize = max(frameSize, osPageSize), claimFrame charges the whole granule when its first frame becomes resident, and releaseFrame refunds only when the granule's last frame leaves — while BufferManager::reserve measures eviction progress in refunded bytes and throws 'The buffer pool is full and no memory could be freed!' after three zero-refund passes. On a 64 KiB-page kernel (Ascend/aarch64 openEuler — the #2631 reporter's host) that is 16 frames per granule: the same COPY bills up to 16× the budget it needs on x86, and whole eviction passes can evict frames yet refund nothing. Apple Silicon macOS (16 KiB pages) is the same mechanism at 4×. Measured with the reporter's exact command and version: vllm-ascend needs a (128, 256] MiB pool on 4 KiB pages — 64/128 MiB reproduce the reporter's byte-identical error, 256 MiB and the 576 MiB adaptive pool succeed — so their 64 KiB host cannot survive on a page-size-blind budget. Scale every derived pool size by granuleRatio = max(1, osPageSize/4096): the per-element estimate, the COPY-safety floor, and the default cap (still bounded by 80% of RAM). 4 KiB hosts are byte-identical to before — proven by pinning the existing sizing tests to an explicit 4096 page size, which also stops them drifting on 16 KiB Apple Silicon runners. GITNEXUS_LBUG_BUFFER_POOL_SIZE keeps absolute precedence and 0 still restores the native default. Also: bufferPoolExhaustionRemedy() gives the exhaustion error an actionable cause→consequence→remedy message; the isLbugPageSizeFrameError comment that called pool exhaustion 'a sizing problem, not a page-size one' is corrected — that framing inverted when #2582 made pool size a function of a page-size-blind estimate. Cannot execute on a 64 KiB kernel here: the scaled path is proven by unit stubs plus the engine-source math above; the env override remains the field escape hatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): actionable pool-exhaustion remedies at the COPY sites and a doctor pool line (#2631) The node-COPY throw and the relationship-COPY warning now append bufferPoolExhaustionRemedy() when the failure is the engine's pool-exhaustion class: the raw binder text gave the operator nothing to act on, and on non-4K-page hosts the pool bills up to pageSize/4KiB × faster than the sizing was calibrated for. The relationship path appends the remedy once per bulk load, not once per failed pair. doctor prints the effective pool size next to the page-size line ('pool size 2048 MiB', with an '(×N page-size scaling)' suffix on non-4K hosts) so support triage sees the sizing inputs at a glance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(lbug): re-anchor getEffectiveBufferPoolSize's placement and reuse granuleRatio in doctor Self-review fixes: the getter's insertion had orphaned resolveBufferManagerSize's doc comment (it read as documenting the wrong function), and doctor's scale note duplicated the granule math with a hardcoded 4096. granuleRatio is now exported (it already carried the test-seam default param) and doctor consumes it. No behavioral change — the sizing suite pins byte-identical outputs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): keep the hintless pool default unscaled and make both remedies visible (#2631) Review fixes: - Scale only the analyze-path cap (scaledAnalyzePoolCap), not defaultBufferPoolSize: the pool is an eager native allocation at DB open (measured, see POOL_BYTES_PER_ELEMENT), so a page-size-scaled hintless default would hand a long-lived MCP process up to 80% of RAM — the #2557 OOM exposure the 2 GiB cap removed. Fix the MAP_NORESERVE claim that contradicted that measurement. - Log the rel-pair pool remedy (loadGraphToLbug returns warnings that no call site reads) and dedup it with a local boolean instead of matching the remedy's own wording. - Label the GITNEXUS_LBUG_BUFFER_POOL_SIZE=0 sentinel as the native 80%-of-RAM default in both the remedy and doctor instead of '0 MiB'. - Extract poolSizeDoctorLine (pageSizeDoctorLines convention): mark env overrides, drop the scaling suffix that misdescribed absolute values. - Fold _resetOsPageSizeCacheForTest into _setOsPageSizeForTests(undefined). - Document the analyze-path scaling in both README env tables. --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 20:09:48 +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
Abhigyan Patwari	9efc6bfcad	fix(lbug/analyze): atomic index swap + read-pool staleness invalidation (#2614) ... * fix(lbug): re-open the read pool when analyze rebuilds the index under it The MCP read pool's initLbug early-returned on an existing pool entry with no freshness check, so after analyze rebuilt or mutated the on-disk index the pool kept serving the old (POSIX: unlinked-but-open) inode until LRU/idle eviction — a silent stale-read window of up to IDLE_TIMEOUT_MS (5 min). Record the file identity {ino, mtimeMs, size} on each PoolEntry at open, and re-stat in initLbug: unchanged → reuse; changed & idle → closeOne + reopen the new file; changed while a query is in flight → serve the current handle (a later idle initLbug reopens, since closing an in-use connection is a native use-after-free). A stat failure (ENOENT during a full rebuild's unlink window) is treated as unchanged so the reader keeps its valid open inode until the new file appears. Mirrors the bridge cache's mtime-invalidation pattern. Step 1 of docs/plans/2026-07-21-...-analyze-atomic-swap-invalidation. The end-to-end reopen-on-swap path is exercised by the reader-during-rebuild integration test in a later step. * fix(analyze): publish a full rebuild via an atomic swap (POSIX) The full-rebuild path wiped the live index (wipeLbugDbFiles(lbugPath)) and rebuilt it in place, so a concurrent MCP reader that opened mid-build could see an empty/half-loaded DB, and a crash between the wipe and the end-of-run left the index destroyed (recoverable only by --force). Build the fresh index at <lbugPath>.new and swap it over the live index in one atomic rename at the end. All DB work flows through the singleton connection, so only initLbug/wipeLbugDbFiles take the temp target; the close already checkpoint-consolidates the build to a single file (verified: no residual .wal/.shadow), so the rename publishes a complete index in one step. A reader opening mid-build only ever sees the previous complete index; a reader holding the old inode keeps a consistent stale snapshot until the pool re-opens onto the new one (the pool staleness invalidation from the prior commit). On failure the swap is skipped, leaving the previous index byte-for-byte intact. POSIX only: the common CLI/serve-worker analyze paths skip the native close (closeLbugBeforeExit, #2264) and leave the build handle open at swap time. POSIX renames an open file cleanly; a same-process open handle blocks the rename on Windows, so Windows keeps the current in-place behavior (buildPath === lbugPath) until that is resolved. The Windows atomic swap and a deterministic concurrent reader-during-rebuild test are deferred follow-ups. Steps 2b + partial 3 of docs/plans/2026-07-21-...-analyze-atomic-swap-invalidation. Integration test asserts the no-temp-leak + inode-swap invariants and the crash-safety guarantee (a load failure leaves the live index untouched). * test(analyze): end-to-end read-pool reopen after an atomic swap Adds the deferred reader-during-rebuild / pool-reopen integration test: analyze v1 -> read pool serves it -> rebuild with a renamed function (atomic swap) -> the same repoId's initLbug detects the swapped inode and re-opens the pool onto the new index. Asserts the pool sees the renamed function and NOT the stale v1 name, exercising #1 (invalidation) and #2 (swap) together end to end. * fix(lbug): bound pooled read queries with setQueryTimeout The read pool relied only on a JS-side Promise.race (QUERY_TIMEOUT_MS) that frees the waiter but leaves the native call running. Set the engine-level setQueryTimeout on every pooled connection so a pathological query is bounded at the source too. * fix(lbug): name the held-open cause for WAL checkpoint failures (#2599) A WAL-checkpoint IO error that also carries a busy/lock signal means another handle (a gitnexus mcp server, or this process's own reader) holds the store open, not a disk fault. Add isLbugCheckpointBusyError (reusing the tested isDbBusyError keyword set) and, when the checkpoint driver exhausts its retry budget on such an error, annotate the surfaced error with the actionable held-open cause instead of a raw IO string. Note: overlaps in-flight work on repro/issue-2599-windows-wal-checkpoint; bundled here at the maintainer's request. * feat(analyze): opt-in atomic incremental + best-effort Windows swap Extends the atomic-swap publish (POSIX full rebuild) to two more cases: - Windows: the swap now applies when a real close is safe to release the build handle before the rename — i.e. non-pdg runs (windowsSwapOk excludes --pdg, the #2264 destructor-crash case), forcing a real close on the swap path. UNVERIFIED on Windows (no Windows runner here); --pdg and any failure fall back to today's in-place behavior, so it can never corrupt. - Incremental (opt-in, GITNEXUS_ATOMIC_INCREMENTAL=1): copies the live index into the temp, applies the incremental delete/writeback to the copy, and swaps at the end. Off by default because the whole-file copy negates incremental's speed premise — kept behind a flag pending a benchmark. The escalation valve also targets the temp so an escalated write stays atomic. Integration test covers the opt-in incremental path end to end (no temp leak, the incremental change is reflected after the swap). * refactor(lbug): centralize the read-pool + bridge open-retry budgets The lbug-config retry registry documented the open/handle-release/query-time budgets but the read pool's LOCK_RETRY_* (pool-adapter) and the bridge's LBUG_OPEN_RETRY_* (group/bridge-db) kept private copies that could drift. Move both into the registry as exported constants (POOL_OPEN_LOCK_RETRY_*, BRIDGE_OPEN_RETRY_*) and alias the local names to them — one tuning surface, no behavior change. * fix: address CI regressions from the bundled follow-ups - setQueryTimeout: guard the call so test doubles that don't model the engine method don't break connection creation. - atomic swap: skip the rename when the build produced no DB at buildPath (an empty repo / mocked pipeline) instead of throwing ENOENT. - #2599: don't wrap the checkpoint error in the driver (it hid the IO signature the CLI's --wal-checkpoint-threshold hint keys on); name the held-open cause at the CLI instead, beside that hint, keeping the original error intact. - retry consolidation: revert to documentation-only — moving the pool/bridge budgets into lbug-config broke every explicit lbug-config test mock. The registry now catalogues all budgets with their in-file locations. - analyze-wal-checkpoint-failure test: block both lbug.wal.checkpoint and lbug.new.wal.checkpoint, since a full rebuild now checkpoints the temp. * fix(analyze): publish the swap before stamping meta; identity-gate the reader (#2614 F1) Review found a HIGH regression: the full-rebuild wrote the freshness stamp (saveMeta, indexedAt=T_new) BEFORE the atomic swap, so a concurrent MCP reader that reinited in the saveMeta->swap window opened the OLD inode, recorded observed=T_new, and then never reinited again (ensureInitialized returns early on 'current') — serving the pre-rebuild graph indefinitely. The build-into-temp change inverted the pre-PR invariant that 'meta shows T_new' implied 'lbugPath holds T_new data'. Two coordinated fixes: - run-analyze: move the final saveMeta AFTER the swap, so meta.indexedAt only becomes visible once lbugPath resolves to the new inode. Verified nothing in the span reads on-disk meta and registerRepo writes only the registry. Leaving the dirty flag set across the swap also improves crash-safety. - local-backend: the reader staleness gate now also compares the lbug file IDENTITY (ino/mtime/size), reiniting on an inode change even when meta.indexedAt is unchanged. This closes the swap-window latch and covers the in-place incremental case — and is what actually makes the pool's dbIdentity net reachable for the MCP reader (the indexedAt gate otherwise bypassed it). * fix(lbug/analyze): WAL-aware incremental, residual-sidecar reconcile, Windows opt-in, #2599 anchor (#2614 F2-F4) Review remediations: - F3: gate atomic incremental on a CLEAN live index (inspectLbugSidecars) — the main-file-only copy would drop an orphan .wal's delta; fall back to in-place. - F4: on the swap, MOVE a residual <buildPath>.wal/.shadow beside the published index (not orphan it) so a swallowed final checkpoint's delta is replayed. - F2: record identity on the shared read-only Database and warn when a cached handle is reused after its on-disk index was rebuilt while another consumer holds it (unreachable via MCP — one consumer per lbugPath; a complete fix needs per-inode handles, documented). - Windows swap: opt-in (GITNEXUS_ATOMIC_WINDOWS_SWAP=1), default off — the forced real close re-bets an unproven #2264 assumption and can't be verified without a Windows runner, so the default Windows path stays in-place. - #2599: anchor isLbugCheckpointBusyError to real held-open wording instead of isDbBusyError's bare .includes('lock') over a message that embeds the DB path (a repo under blockchain-app misclassified a disk fault as held-open). - Docs: corrected retry-catalogue budgets (linear, not exp) and the checkedOut>0 bound comment (load-bounded, not IDLE_TIMEOUT_MS). * test(analyze): cover the production close path in the atomic swap (#2614 F5) Adds a full-rebuild swap test with skipNativeCloseOnExit:true — the close path the CLI and serve-worker actually ship (build handle left open at swap time), distinct from the default real-close the other swap tests exercise. Asserts the POSIX swap still publishes a single consolidated lbug with no .new temp and no orphan sidecar. * test(analyze): give the follow-up git commits an inline identity (CI fix) The end-to-end reopen and atomic-incremental tests' second commits used a bare `git commit`, which fails on CI runners with no global git identity (empty ident name). makeRepo's initial commit already passes -c user.name/-c user.email inline; apply the same to the rename/change commits. No code change. * fix(mcp): route reader reinit through initLbug's active-query guard (#2614 review) Review found an active-query retirement race: LocalBackend.ensureInitialized detected an identity/stamp change and called closeLbug(poolKey) DIRECTLY, but closeOne closes the shared Database at refCount 0 regardless of checked-out connections. So a reader detecting the new generation could close the Database a concurrent query is still executing on — a native use-after-free. This bypassed the checkedOut>0 guard that initLbug itself has. Fix (delegate, not close directly): initLbug now returns whether it actually rolled the pool over; ensureInitialized calls initLbug (which serves the current handle while a query is in flight and reopens only when idle) instead of closeLbug. The observed IDENTITY is advanced only when the pool actually reopened — if a query was in flight, the identity stays divergent and the reopen retries on a later idle check rather than latching on the old handle. The observed STAMP advances regardless so a same-file stamp change can't loop. Old generation now stays alive until its in-flight queries drain (lazy rollover); new requests during the busy window share the old handle until the pool goes idle, then reopen. No parallel open-both-generations, but no UAF and no stale latch. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-21 21:58:00 +01:00
Gergő Magyar	8b5057f325	feat(skills): GitNexus Engineering Tool Kits (#2566) ... * feat(skills): add ce-plan — GitNexus+PDG implementation-planning skill Adds .claude/skills/ce-plan: a planning-only skill that builds implementation-ready plans from GitNexus graph navigation (query/context/ impact/trace), bounded statement-level PDG slices (pdg_query, impact mode:pdg, explain), and targeted source verification, with a context ledger to prevent repeated reads and a machine-readable implementation context pack (stable contract for a future ce-implement). Whitelisted in .gitignore and registered in AGENTS.md and CLAUDE.md outside the auto-managed gitnexus block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(skills): apply ce-plan validation findings (tool contract, consistency, conventions) Tool contract: impact mode:'pdg' shape now includes the schema-required direction param; CDG branch sense documented as the result 'label' field (reason is cypher/raw-edge only); explain caveats corrected to its real false-negative classes (cross-function TAINT_PATH is modeled). Consistency: PDG slice homed in working memory (ledger keeps one-liners); depth knob defined and category-overrides-baseline ordering stated; call_depth (consumed by nothing) and content-hash bookkeeping dropped; Never section folded into Hard rules; Phase 3 deduplicated to a pointer; allowed-repeat escalations defined; budget/discard accounting clarified; verification-commands gathering added to Phase 4; open_questions added to the context pack. From scenario runs: plans now pin the verified-at HEAD commit and index freshness in a header, tag claims [verified]/[graph]/[inferred]/[assumed], quote load-bearing tool output, prefer pre-hook-carrying npm scripts, and support an out:<path> destination override; output path defined as the Phase 1 target repo root. Conventions: AGENTS.md 1.9.0 / CLAUDE.md 1.4.0 changelog rows + metadata bumps; future ce-implement qualified as future. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): rename ce-plan → gitnexus-plan; add cross-CLI (Codex) entrypoints Renames the skill dir, frontmatter, output filename convention, plan H1 (GitNexus Engineering Plan), the future executor handle (gitnexus-implement), the .gitignore whitelist entry, and all AGENTS.md/CLAUDE.md references. Follows the pr-swarm-review cross-CLI pattern: SKILL.md is the canonical CLI-neutral spec, AGENTS.md § Engineering planning is the Codex/any-agent entrypoint, and the README documents the optional user-level ~/.codex/prompts/gitnexus-plan.md slash command plus an invocation matrix. Skill prose de-branded from Claude Code (agent-neutral verification layer). Also fixes two post-review README contradictions: the anti-reread claim now names the ledger's allowed escalations, and 'read-only by contract' is now 'planning-only' (the skill writes exactly one repo file — the plan); the scope-creep rule and template §12 now agree on where deferred follow-ups land. Drops the stale plugin-collision limitation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(skills): document Codex user-level install path for gitnexus-plan Codex discovers SKILL.md skills from ~/.agents/skills (same path the other gitnexus-* skills install to); README now documents the cp install plus the optional ~/.codex/prompts slash-command file, with the prompt body preferring the repo copy and falling back to the user-level install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): gitnexus-plan freshness gate + active PDG-layer refresh Freshness is now a Phase 1 gate, not advisory: under the default freshness:strict, a stale index is refreshed once per planning session via node .gitnexus/run.cjs analyze --index-only (appending --pdg when the task will reach the PDG phase), then the context resource is re-read. A missing PDG layer likewise triggers the one permitted --index-only --pdg refresh and re-probe instead of a passive recommendation. freshness:accept (or a failed/impractical refresh) preserves the old behavior: plan on the stale graph, source-weighted, labelled in the plan header. --index-only is the load-bearing flag choice — it suppresses all file generation, so the planning-only contract holds (only the .gitnexus store changes). Ledger gains an index_refresh record; plan header states fresh / refreshed / refresh-skipped-with-reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): gitnexus-plan runner build check before freshness refresh When the target repo builds the analyzer from its own source (bin → dist/ mapping, as gitnexus/ does), the Phase 1 freshness gate now verifies dist/ is current before running the analyze refresh — rebuilding via the package's build script when any analyzer source file is newer than the built entrypoint — and prefers that freshly built CLI. Otherwise a stale dist re-indexes with outdated extraction logic and the 'fresh' index lies. Rebuilds are recorded in the ledger's index_refresh; the PDG-phase refresh inherits the same check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): add gitnexus-work executor and gitnexus-lfg pipeline gitnexus-work executes a gitnexus-plan as verified atomic commits: consumes the §11 implementation_context pack, drift-checks the plan's evidence pin against HEAD, re-verifies assumptions before relying on them, runs impact before every symbol edit and detect_changes before every commit (repo mandates), builds tests from the plan's scenarios, and routes structural drift back to gitnexus-plan Deepen mode instead of coding around it. gitnexus-lfg is a thin orchestrator: gitnexus-plan → blocking user gate (deepen / proceed / stop, deepen loops allowed) → gitnexus-work → review via the existing gitnexus-pr-review skill (open PR, else branch diff vs default). One bounded fix cycle for review findings; never pushes or opens a PR on its own. gitnexus-plan gains a Deepen mode (re-run freshness gate, escalate to depth:deep, re-verify graph/inferred/assumed claims toward verified, rewrite the same file); its 'future gitnexus-implement' placeholder is retired in favor of gitnexus-work. Registered via .gitignore whitelists, AGENTS.md 1.10.0 (section renamed to Engineering planning & execution), CLAUDE.md 1.5.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(skills): apply cross-skill review findings to the gitnexus skill family Two P1s: gitnexus-plan Deepen mode now re-anchors before re-pinning (diffs the old evidence pin over every [verified]-claim file and re-reads or downgrades before the header moves — moving the pin without this laundered stale claims as verified); the index-refresh budget is stated once in Phase 1 (one --index-only refresh plus at most one Phase 3 --pdg upgrade per session, Deepen = its own session) with ledger and pdg-slice deferring to it. Contract fixes: gitnexus-work's drift check now covers every file the pack cites (not just files_to_modify) and parses the full pack incl. primary/related symbols and acceptance_criteria (walked in Phase 4 alongside §13); a pre-completed check skips §7 steps already landed and Deepen gains a reconcile-execution-state step, closing the mid-execution route-back loop; pack assumptions must name what to check and how. lfg: Lane 4 passes the merge-base to detect_changes compare (two-dot diff misattributes upstream commits when default advanced), branch-diff is the stated normal case, oversized review findings route to the plan gate instead of overflowing direct mode, the one-fix-cycle cap is explicit on re-run, and headless runs end at the plan gate with the plan as deliverable. work: blank mode narrowed to *gitnexus-plan*.md with a re-execution guard, direct-mode discipline spelled out, branch meaningfulness defined against the plan slug, and the plan document is committed as the branch's docs commit (review diff includes it). Planning-only contract now names the dist/ rebuild as the second permitted state change; Phase 5.1 names the four claim tags; stale AGENTS.md anchors fixed. Known latent issue left untouched: gitnexus/gitnexus-pr-review pairs a three-dot example with a two-dot detect_changes compare — that skill is also shipped by the plugin, so fixing it here would drift the copies; lfg compensates by passing the merge-base. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): ship the engineering skill family with the gitnexus package npm i -g gitnexus users now get gitnexus-plan / gitnexus-work / gitnexus-lfg: the three skills are added to gitnexus/skills/ in directory form (SKILL.md + references/), which installSkillsTo already enumerates dynamically and copies recursively to every editor target (~/.agents/skills for Codex, Cursor, OpenCode, Qoder, ...) on gitnexus setup — uninstall enumerates the same root, so removal stays clean. The Claude Code plugin channel (gitnexus-claude-plugin/skills/) carries the same copies plus the standard per-skill mcp.json. Global-install support in the skill text: gitnexus-plan Phase 1 now resolves the analyzer runner explicitly — node .gitnexus/run.cjs analyze when the project has a runner, else gitnexus analyze (installed CLI), else npx gitnexus analyze — and all analyze mentions route through it, satisfying the skills-steering policy (#1939/#1945) which sweeps the plugin copies. New drift guard test/unit/shipped-skills-sync.test.ts asserts the npm and plugin copies stay byte-identical to the canonical .claude/skills/ family (plugin = canonical + mcp.json), same discipline as run.cjs ↔ resolve-invocation.ts. skills-steering + shipped-skills-sync: 11/11 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): workflow_bench — measure the skill workflow's token savings Benchmarks gitnexus-plan → gitnexus-work against a baseline agent (--disallowedTools Skill) on identical tasks, in fresh detached worktrees, using real headless Claude Code sessions; every number comes from the CLI's --output-format json usage report (field names validated against a live 2.1.207 session). Reports per-arm medians (input/cache/output tokens, cost, wall time, turns), a savings row, and resolve status from a per-task verify command — savings on failed tasks are flagged, not celebrated. Per-task setup hook prepares fresh worktrees (deps); --permission-mode bypassPermissions (default) lets sessions run unattended in the throwaway trees. Free-model support: --base-url/--auth-token/--model route headless sessions through any Anthropic-compatible endpoint; free-model.litellm.yaml is a ready litellm-proxy template for OpenRouter :free variants or local Ollama, so benchmarking burns no paid tokens (README documents rate limits and the small-model skill-following caveat). Harness validated end-to-end with a stub CLI (worktree lifecycle, both arms, plan→work chaining, verify, aggregation, report) and 4 pytest units for the pure aggregation/savings/report helpers. AGENTS.md 1.11.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): record first workflow_bench calibration run Trivial-task calibration (add -V alias): both arms resolved; workflow arm ~4.3x baseline cost — the documented overhead-dominated regime, recorded so the regime boundary is empirical rather than asserted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): workflow_bench scenario matrix — arm variants, task classes, churn Ground-base measurement across scenarios: tasks.scenarios.yaml spans four labeled classes (trivial → investigation-bug → investigation-feature → cross-module) with deterministic verifies (prescribed test files). New arms: workflow_direct (gitnexus-work direct mode — the middle option that locates the routing boundary lfg's gate and work's triage encode) and baseline_nomcp (no skills AND no graph tools — separates workflow-discipline value from GitNexus-tool value; off by default). Records now carry task class and diff churn (files/+ins/−del vs the starting commit) as an over-engineering proxy; the report renders a class column and per-arm savings rows vs baseline. 5 pytest units + stub-CLI e2e of the full three-arm matrix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): record workflow_bench ground base; fix churn measurement bias Ground base (3 classes x 3 arms, n=1/cell): every arm resolved every task — pass/fail quality saturates at this difficulty, making the comparison pure cost. Full plan→work never amortized its ~$9-11 fixed cost on tasks a baseline finishes in ≤35 turns (−211% to −333% cost); workflow_direct sits near baseline (−15% to −55%, once faster wall) with more test coverage. Routing implication recorded: direct mode/plain agent below this scale, full workflow for cross-module / multi-session / plan-as-deliverable work. The cross-module cell and multi-run variance are the next measurements. Churn fix: git add --intent-to-add -A before diffing (arms that never commit no longer undercount new files) and :(exclude)docs/plans (the committed plan doc no longer inflates workflow churn); this run's churn numbers predate the fix and are omitted from the recorded table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(skills): cost-optimize the workflow from measured ground base Every optimization targets a measured fixed-cost component (eval/workflow_bench ground base: workflow arm −211% to −333% vs baseline, all tasks resolved): - Plan form is category-priced: compact form (core sections w/ § anchors preserved, ≤80 lines excl. pack, mini-pack subset of the context pack) for narrow/default categories; the full 13 sections only for deep work (refactor/security/performance/concurrency/architecture). A compact plan outgrowing its cap reclassifies to full rather than overflowing. - Freshness gate is category-priced: compact categories default to accept (source-weighted, refresh only when a graph claim becomes load-bearing); strict stays the default for full-plan categories — the rebuild+re-index was the largest single fixed cost. - Turn economy: per-category tool-call budgets (~10 to ~45; architecture uncapped); budget exhaustion routes open questions to §12 instead of more digging. - gitnexus-work fast path: HEAD == evidence pin → skip all citation re-reading (the pin's entire point); mini-pack fields tolerated. - lfg Lane 1 boundary triage: tasks below the measured ~35-turn boundary get offered gitnexus-work direct mode before the plan lane is spent. Copies re-synced (npm skills/, plugin, ~/.agents); steering + sync guards green. Re-measurement of the workflow arm follows to verify the numbers actually improve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): record optimization re-measurement — inv-bug workflow cell −20% cost Same task, same conditions, post-830a0459 skills: $14.56→$11.70 (−20%), 83→72 turns, cache_read −24%; verified in-transcript that the compact form, turn budget, and skipped rebuild/re-index all fired. Wall +15% from a work- session test-debugging tail (n=1 variance). Regime unchanged (~3.5x baseline on this class) — routing rule stands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(eval): per-arm clone isolation — worktree ref-namespace leak contaminated an arm The cross-module workflow_direct cell reported an impossible 28-turn solve with churn byte-identical to the workflow arm: git worktree add shares the repo's ref namespace, so the workflow arm's slug branch (created by gitnexus-work Phase 2) survived worktree removal and the direct arm found and adopted the completed work. Arms now get isolated git clone --shared copies (object store via alternates, refs clone-local — agent branches and stashes die with the clone; origin/<ref> fallback for non-default refs). Leaked branch deleted; baseline arm verified clean (0 branch references in its transcript); cell marked invalidated pending re-run. Records the valid cross-module cells: workflow $18.32 vs baseline $18.03 (premium −1.6%, vs −211%..−333% on smaller classes) — fixed costs amortize at this scale, with a less destructive diff and a plan artifact as bonus; resolve rate still tied. Churn fingerprinting is what caught the contamination — noted in the README as an integrity check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(eval): complete cross-module cell — direct mode wins 47% cost / 56% wall Clean clone-isolated re-run: workflow_direct resolved the hardest class at $9.53/52 turns/15m vs $18.03/98/34m baseline and $18.32/107/37m full workflow. The measured story across all four classes: the execution discipline (gitnexus-work) is the consistent sweet spot and delivers real token savings on hard tasks; the planning pass buys its artifact, not same-session savings. Resolve rate tied everywhere (n=1/cell caveat). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): add trajectory-gated skill evolution (#2431) - Pair prompt candidates with incumbent workflow arms - Gate promotions on pinned-model quality and efficiency - Expire router evidence and document its lifecycle * fix(eval): allow pr-review skill candidates * feat(skills): rename and generalize GitNexus review * feat(eval): external-comparator and review arms for workflow_bench - ce_workflow / ce_workflow_direct: compound-engineering ce-plan/ce-work arms prompted with the same structure as the gitnexus arms - review / ce_review: gitnexus-review vs ce-code-review on an identical diff applied by the task's setup - plan handoff is snapshot-based: committed example plans in docs/plans/ tie on clone mtimes and broke the name-glob pick (executed a stale plan) - verify output tail is recorded per run and the final working-tree patch is kept, so failed rows are diagnosable after the clone is destroyed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(skills,eval): address #2431 review — data-safe rename migration, fail-closed bench evidence - setup: never delete a legacy renamed skill dir — the installer cannot prove ownership (users customize or hand-write skills under these names); warn with the path instead, and the test now asserts survival - workflow_bench: fail closed when a session's --output-format json report is empty, malformed, or missing usage fields — an exit-0 shell with no parseable usage no longer counts as measured evidence (5 parametrized regression tests) - workflow_bench: document the trust model prominently (task setup/verify are shell-executed, sessions run bypassPermissions with the parent env, candidate overlays are prompt injection surface) in README + docstring - free-model.litellm.yaml: master_key from LITELLM_MASTER_KEY env instead of a static token; loopback-binding warning - ci: run the eval workflow_bench pytest suite on ubuntu (pytest+pyyaml only — no full eval stack) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(eval): demand observed foreground verification in headless work-arm prompts In a headless -p session there is no later turn: a work arm backgrounded its slow test run, scheduled wakeups that can never fire, and reported done while two of its tests failed. All four work-arm prompts (both skill families, symmetric) now require verification output to be observed inside the session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): ask plan depth up front instead of offering deepen afterwards gitnexus-plan Phase 0 now asks one blocking question in interactive sessions — quick / standard / deep, mapped onto the existing depth/form/ freshness knobs — when the invocation carries no explicit depth signal. Explicit knobs and headless runs skip the question (category posture unchanged, so benchmarks and automation behave as before). gitnexus-lfg's plan gate slims to proceed/stop: depth was already the user's up-front choice, so deepening is no longer offered by default — an explicit deepen request at the gate and executor route-backs still run Deepen mode, which remains the mechanism for strengthening an existing plan document. All shipped copies resynced (npm skills/, Claude plugin); AGENTS.md 1.13.0 and CLAUDE.md 1.7.0 pointers updated, including the analyzer's regenerated index-stats block at this branch's head. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): taint pass, expert lenses, and post-work index refresh gitnexus-review gains a PDG-backed taint-and-dependence pass (explain + pdg_query, --pdg folded into the stale refresh on trust-boundary diffs) and an Expert lenses section: domain reviewers derived from the graph's clusters plus four cross-cutting lenses (architectural fit, language conformance per the repo's own contract, Definition of Done, simplicity), dispatched once after the evidence-gathering steps and scaled to the diff. gitnexus-work Phase 4 now refreshes the knowledge graph after the DoD walk via the resolved-runner ladder with analyze --index-only, so the lfg review lane and later sessions query the finished work without dirtying the tree. lfg's threshold-governance paragraph moves to its README; eval citations are tagged as measured in the GitNexus repo. All shipped copies re-synced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): remove legacy gitnexus-pr-review on uninstall; cover the rename migration uninstall's removal set now includes LEGACY_SKILL_DIR_NAMES derived from RENAMED_SKILL_DIRS, so a pre-rename install is cleaned up instead of orphaned. The rename warning gains behavioral coverage (fires with a legacy dir present, silent without), and shipped-skills-sync asserts legacy names stay absent from every shipped tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(eval): metric provenance, error-kind rows, skill-invocation verification, gate noise floor The promotion gate defaults to cost_usd (the only metric that includes subagent spend); token metrics carry an explicit main-loop-only warning in the report and promotion.json. Rows are classified by error_kind (session-error / verify-failed / infra-error), excluded from efficiency medians, and the gate requires equal valid-run counts. Each session's transcript is scanned for the expected Skill invocation and fails closed on a verified miss; a one-run resolution edge no longer promotes (noise floor). Per-run timeouts and setup failures record an infra-error row instead of aborting the sweep. Overlays touching skills no candidate arm exercises are rejected up front. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: fix skill routing paths, version headers, and skill rosters Routing tables point at the tracked direct skill paths (matching the post-#2434 generator output), AGENTS.md/CLAUDE.md headers match their latest changelog rows, the 1.12.0 row describes what the migration actually does, package/cursor READMEs list the full shipped skill roster, and the swarm READMEs describe /gitnexus-review's expert lenses instead of calling it single-agent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: drift-guard workflow for skill copies; pin eval pip deps; track docs/plans ci.yml ignores '**.md', so an md-only skill edit would merge without the shipped-skills-sync test running — skill-sync.yml triggers exactly on the guarded trees. The eval job's pip install is version-pinned, and docs/plans/ is unignored so gitnexus-plan output can be committed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): keep the runner-invocation literal in gitnexus-review; add concurrency block to skill-sync skills-steering requires skills with a stale-index hint to carry the exact 'node .gitnexus/run.cjs analyze' form — restore it with the fallback ladder as a parenthetical instead of replacing it. skill-sync.yml gains the top-level concurrency block the workflow-convention check enforces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(skills): token-economy guidance for expert lenses Merge lenses that ground in the same material into one reviewer, and use cheaper model/effort tiers for mechanical lenses where the harness offers them, reserving the strongest engine for adversarial judgment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(eval): isolate transcript home on Windows Ensure workflow_bench transcript tests set USERPROFILE alongside HOME so Path.home() resolves to the temporary test home on Windows. * docs(skills): fold PR #2522 execution learnings into review/work/plan Eight incident-backed hardenings from running the full skill cycle (review -> plan -> work, 28-finding fix series) on PR #2522: gitnexus-review: - Expert lenses execute the code under review on candidate failing shapes (empirical probe outranks source reading — every HIGH the language lenses found came from a probe, not a read). - Step 7 re-runs the exact CI check for refreshed baselines/fingerprints (a stale committed artifact is invisible in the diff; caught a red benchmarks arm). - Step 8 treats version/invalidation constants as review surface (INCREMENTAL_SCHEMA_VERSION class recurred verbatim from #2494). gitnexus-work: - Step 4 proves regression tests discriminate against the pre-fix tree. - Step 5 rebuilds executed build output before every verification run (parse workers load dist/; a correct fix 'failed' until rebuilt). - Step 6 makes stage -> detect_changes -> commit one unbroken sequence. gitnexus-plan: - Phase 0 seeded-evidence mode: plan FROM a completed review's verified findings instead of re-running the graph ladder. - Template §7: fingerprint/golden-guarded output rebaselines once, at the series tip. All distribution copies resynced; shipped-skills-sync + skills-steering 24/24 locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(eval): close the skill-evolution loop with an automated proposer driver workflow_bench.evolve adds the three arrows the README described as manual: a proposer session that turns loser trajectories (results.jsonl rows, transcripts, patches, the learning queue) into ONE bounded candidate overlay, a driver that iterates propose -> paired benchmark -> deterministic gate up to --generations, and an --apply step that copies a promoted overlay onto the canonical skills and shipped mirrors as a working-tree diff. The trust boundary is unchanged: overlays re-validate through candidate_overlay_files before any benchmark or apply consumes them, and committing, CI, and the PR merge stay human. learnings.jsonl is gitignored: it is machine-local evidence, like the session transcripts it complements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(skills): route live-task friction into the evolution learning queue Each family skill gains a short 'Skill feedback' section: on friction with the skill's own instructions, append one JSON line to eval/workflow_bench/learnings.jsonl (GitNexus repo only) — never self-edit the skill from a live task. The proposer in workflow_bench.evolve consumes the queue as hints; a learning reaches a shipped skill only by beating the incumbent on the paired benchmark. All shipped mirrors re-copied byte- identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(tests): run the evolve helper tests in the eval pytest job test_evolve.py needs only pytest+pyyaml, same as the harness tests the job already runs — without this line the new module had no CI coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ci): comment-triggered GitNexus review agent for PRs '@gitnexus review' from a maintainer (OWNER/MEMBER/COLLABORATOR; the action re-validates write access) runs the repo's gitnexus-review skill headlessly against the PR and posts the review as a sticky comment — remote triggering with no local setup. Read-only by construction: contents: read token, Write/Edit and web tools disallowed, Bash allowlisted to git reads and the gitnexus CLI; analyze parses PR code with tree-sitter, never executes it. Requires the ANTHROPIC_API_KEY repository secret; activates once the file is on the default branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ci): dispatch lane + existing OAuth secret for the review agent Align with claude.yml: same action pin and the CLAUDE_CODE_OAUTH_TOKEN secret the repo already carries — no new secret to configure. Add a workflow_dispatch lane (PR number input) so the agent can be triggered from the Actions UI and tested before the issue_comment trigger reaches the default branch. Allowlist gh pr view/diff and gh api, which the review skill uses to pin PR SHAs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): close a fork-PR RCE vector in the review agent's tool allowlist A live headless run of the exact workflow session against PR #2431 (66 turns, full gitnexus-review pass) surfaced a real HIGH-severity confused deputy: .gitnexus/ is gitignored, not blocked — a fork PR can commit its own .gitnexus/run.cjs, issue_comment checks out PR-head content, and the skill's runner ladder tries 'node .gitnexus/run.cjs analyze' first. That would execute fork-controlled JS inside a job holding CLAUDE_CODE_OAUTH_TOKEN and a write-scoped GITHUB_TOKEN — the opposite of the 'PR code is read, never executed' claim in the workflow's own header. Fix: drop the run.cjs allowlist entry so analyze always resolves through npx gitnexus (npm registry, not the checked-out tree); the skill's documented fallback mode covers the resulting graceful degradation. Also drop 'gh api' (not read-only — accepts -X POST/PATCH/DELETE) and downgrade pull-requests: write to read (comment posting only needs issues: write; the prompt already forbids formal review submission). Same session flagged a latent evolve.py bug: select_evidence's cost sort used dict.get's missing-key default, which doesn't cover an explicit JSON null in a foreign --seed-results row and crashes proposer setup with TypeError. Guarded with 'or 0.0' and added a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: harden PR review and evolution trust boundaries * ci: follow workflow concurrency convention * fix(eval): make terminating error paths explicit * fix: unblock hardened review runtime checks * test: make containment canaries deterministic * test: expose Claude canary tool failures * fix: adapt clean shell environment for Claude * fix(eval): accept the runner's transcript source key in evidence preflight The proposer evidence preflight required transcript-artifact metadata to be exactly {path, sha256, bytes}, but the runner stamps a fourth provenance key (source=parent-captured-stream-json). Any --seed-results or generation>=2 run therefore aborted with SandboxError before proposing or promoting. Pin the producer literal as PARENT_EVENT_STREAM_SOURCE and validate it in the metadata check, and round-trip real producer output through sum_sessions into the preflight so the schema can't drift again. * fix(eval): treat an unmeasured session cost as unavailable, not $0 well_formed validated only the nested usage block, so an otherwise-successful session missing total_cost_usd was recorded as cost_usd=0.0 — and cost_usd is the default promotion metric (lower wins), so a cost-less session scored as free and could win promotion it never earned. Extract cost via measured_cost() (None on absent/garbage, a measured 0.0 preserved), propagate None through sum_sessions/aggregate/savings/report, and have the gate refuse to rank on a metric that was not measured on every run in both arms. * fix(eval): warn when ranking on the main-loop-only num_turns metric num_turns comes from the CLI's top-level usage (main-loop session only), like output_tokens, but selecting it emitted no metric_warning — so a subagent-heavy candidate could look artificially efficient. Add num_turns to MAIN_LOOP_ONLY_METRICS and broaden the warning to cover turns. * fix(eval): fail closed when an overlay adds a file with no committed base An overlay adding a new .md under gitnexus-{plan,work} passes the structural overlay checks but has no committed base for committed_destination_base_digests to bind against, so it raised an uncaught ValueError that crashed the evolve driver (and runner --candidate-overlay) mid-run. Catch it at both call sites: evolve reports NOT PROMOTED and exits, runner routes it through parser.error. * feat(eval): circuit-break the runner sweep on a systemic outage A sustained upstream outage used to pay out every remaining --timeout window one session at a time. Track consecutive session/infra/cleanup failures via a pure systemic_outage_streak helper; after --outage-streak (default 5) in a row, stop the sweep, still write report.md/promotion.json from partial evidence, and exit non-zero so evolve.py halts instead of proposing from truncated evidence. A task's own resolved=False never trips the breaker. * fix(cli): report a dirty working tree as stale in gitnexus status status --json (and the human output) computed up-to-date from commit + runner identity + completeness only, so a repo with uncommitted source changes at a matching HEAD was reported up-to-date while analyze would still re-index it. A graph-backed agent gating on that JSON could skip re-analysis on a stale graph. Extract analyze's dirty-tree check into a shared isWorkingTreeDirty() in storage/git and fold it into the status freshness decision. * fix(ci): use single-slash deny globs in the review agent's disallowedTools github.workspace already expands to an absolute path, so Read(/${{ github.workspace }}/**) and Read(//proc/**),(//sys/**),(//dev/**) produced double-slash patterns that a normalizing matcher may not match — silently no-opping the deny layer. Not exploitable (the allowlist is the primary control and never grants those paths), but the globs should be well-formed. Update the pinned test strings. * ci: install gitnexus-shared with npm ci from the committed lockfile The gitnexus-shared build floated its deps via npm install in three workflows (skill-sync, ci-tests, and — most importantly — the release publish.yml) while every other install step uses npm ci. The lockfile is committed and in sync, so switch all three to npm ci for reproducible, locked installs. * test(cli): make the shipped-skills drift guard reject symlinks listFilesRecursive walked with readdirSync and snapshotDir read with readFileSync, both of which follow symlinks — so a mirror file symlinked to the canonical tree passed the byte-compare (and a symlinked mirror dir would be followed too). Reject a symlinked root via lstat and any symlinked entry via Dirent.isSymbolicLink, with negative tests (skipped on Windows). * test(eval): guard the candidate-skill vs mirror-root coverage invariant MIRROR_SKILL_ROOTS omits the Cursor tree, safe only because no candidate skill is cursor-shipped. Pin that invariant: every CANDIDATE_SKILLS entry must exist under canonical + every mirror root and must not ship to Cursor, so adding a cursor-shipped skill to the candidate set (the PR #2488 asymmetric-sync class) fails loudly instead of syncing three of four trees. * docs(ci): describe the review agent's staged post-merge rollout The DoD asked for a dry-run or triggered run before merge, but an issue_comment (or newly added workflow_dispatch) workflow only ever executes the default-branch copy, so it cannot be exercised from the PR that introduces it. Reword the DoD and the activation checklist to a staged rollout: merge registered-but-disabled, validate same-repo and fork execution post-merge, then enable the variable. * fix: pin plugin skill mcp.json to the release version via #2445 tooling The ten plugin skill mcp.json launched `npx -y gitnexus@latest mcp` on every skill connect — non-reproducible and a supply-chain surface, and (unlike the persisted setup config) never pinned. Extend sync-plugin-manifests.mjs with an mcp surface kind that stamps the gitnexus@<version> launch arg, pin all ten to 1.6.9 now, and keep them byte-identical so the drift guard stays green. The release lifecycle + publish.yml --check now re-stamp them like the four manifest surfaces; only READMEs stay on @latest as docs. * test(eval): prove the proposer's built-in file tools are confined The real-Claude canary only exercised Bash + MCP, so it proved process/MCP containment but not that the proposer's built-in file tools stay inside their mounts. Add a canary over the exact PROPOSER_ALLOWED_TOOLS surface and the same read-only /evidence mount as run_proposer (allowlist extracted to a shared constant so it can't drift): Read reaches /evidence, a Write into the read-only evidence mount is denied, and a Write lands in the output tree. * fix(eval): apply the candidate overlay after task setup for fair arms The candidate overlay was applied before the task's untrusted setup ran, so setup could observe candidate prose and the incumbent/candidate arms started from different pre-overlay state. Reorder within the sandbox: capture the base (pre-overlay) skill digest, run setup against the base skills, verify setup did not tamper them, then apply the overlay and capture the post-overlay digest the model must preserve. apply_candidate_overlay stages path-specific overlay files, so setup's uncommitted changes stay out of the baseline and churn is unchanged. Graph freshness for the review arm is handled by the status dirty-tree fix plus the review skill's stale-triggered re-index, not by reordering the cached per-task-sha graph materialization (which is mechanically blocked). * test(eval): end-to-end containment proof of the autonomous proposer Drives the real run_proposer through bubblewrap with a deterministic scripted model (no paid API): it reads the read-only evidence bundle and writes a candidate gitnexus-plan skill edit plus a rationale into the sandbox output tree; run_proposer enforces the trust boundary and copies only the validated overlay + proposal out. This exercises the autonomous-proposal stage of the self-evolution loop end-to-end in the eval/containment CI job (the gate and apply stages are covered by test_workflow_bench_evolution and test_promotion_apply). Env-gated on GITNEXUS_REQUIRE_CLAUDE_CANARY, so it runs only where the pinned Claude binary and user namespaces are available. * fix(eval): let the proposer author its overlay via Bash Running the end-to-end proposer canary in the containment CI job surfaced a real bug: run_proposer starts the session with --bare, which hard-disables the Write/Edit tools ("Write exists but is not enabled in this context"), yet allowlisted Edit/Write and omitted Bash. The proposer therefore had no working way to write its candidate overlay — the self-evolution loop could never produce a candidate. The sandbox settings already pre-authorize Bash (autoAllowBashIfSandboxed) and confine writes to workspace/tmp/home, so switch PROPOSER_ALLOWED_TOOLS to Read/Grep/Glob/Bash and tell the proposer to author files with Bash. The end-to-end test now drives the real run_proposer through bubblewrap and asserts a validated overlay + proposal are produced (this also replaces the earlier file-tool canary, whose Write/Edit premise was moot). * test(eval): author the proposer overlay with newline-free Bash content The nested shell-sandbox prefix mangles embedded newlines, so the multi-line overlay content never landed. Use single-line content for the deterministic proposer canary. * test(eval): drop the unverifiable end-to-end proposer canary The scripted proposer overlay never materialized in the containment job across runs, and the model tool-result content is not visible in CI logs, so the test cannot be finalized without an environment where the sandbox can actually run. Keep the verified production fix (Bash-authoring in run_proposer); the proposer sandbox/containment stays covered by the existing Bash+MCP and process-tree canaries. * test(cli): drop run-analyze.ts from the windowsHide spawn-family list U7 moved run-analyze.ts's only child_process call (the git status --porcelain dirty check) into storage/git.ts (already covered by this test, with windowsHide). run-analyze.ts no longer imports a spawn-family function, so the windowsHide-regression test's 'must have >=1 spawn call' invariant failed for it. Remove it from SRC_FILES. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Zander Raycraft <zanderjraycraft@gmail.com> Co-authored-by: Azizur Rahman <azizur100389@gmail.com>	2026-07-19 15:07:24 +01:00
azizur100389	a333d94a00	feat(wiki): allow explicit HTTP LLM hosts (#2491) ... * feat(wiki): allow explicit HTTP LLM hosts Keep wiki LLM HTTP endpoints fail-closed by default while adding a narrow exact-host opt-in for LAN/self-hosted models. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(wiki): simplify insecure LLM flag name Rename the wiki HTTP opt-in flag to --allow-insecure-connection per review feedback. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(wiki): simplify insecure connection env Rename the wiki HTTP allowlist environment variable and align validation errors with the CLI flag naming. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-16 13:53:58 +01:00
Gergo Magyar	e20e326290	fix(cli): fail every tool command loudly on backend error payloads ... Moves the #2469 guard from cypherCommand into output() so all seven tool commands that print backend results share the exit semantics. Adds query and context regression cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 09:58:14 +00:00
Gergo Magyar	639aab6af0	fix(cli): fail cypher errors loudly (#2470)	2026-07-16 09:56:09 +00:00
Gergo Magyar	48a145a47f	feat(mcp): enforce repository allowlist and default (#2465) ... Merged with the read-only policy: both filters compose in server.ts. Review hardening: assertResourceUri compares the opaque host case insensitively (GITNEXUS://GROUP bypass) and gains the fail-closed fallback for unparseable URIs; the backend proxy now also intercepts queryClusters/queryProcesses/queryClusterDetail/queryProcessDetail; scrubGroupDescription is shared from read-only-policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 08:50:07 +00:00
Gergo Magyar	277dfbbec7	fix(eval): defer env file read errors to binds that need a token ... Loopback binds now warn and start when .env or .env.local exists but cannot be read; non-loopback binds keep the fail-closed error. Help text notes that hostnames resolve to IPv4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 08:25:52 +00:00
Eva	5d1911b2ef	Merge remote-tracking branch 'upstream/main' into upstream/cypher-error-exit-2469	2026-07-16 09:49:16 +07:00
Eva	e0a83f098a	Merge remote-tracking branch 'upstream/main' into upstream/mcp-repository-policy	2026-07-16 09:36:31 +07:00
Eva	c9fdab17f2	fix(eval): address auth configuration feedback	2026-07-16 09:13:34 +07:00
Eva	8682c8a56e	Merge remote-tracking branch 'upstream/main' into upstream/eval-server-auth	2026-07-16 09:06:59 +07:00
Gergő Magyar	a05b501102	fix(cli): make Claude skills discoverable (#2434)	2026-07-15 20:40:20 +05:00
Eva	c35fc27427	fix(cli): fail cypher errors loudly	2026-07-14 03:08:32 +07:00
Eva	181faa858c	feat(mcp): enforce repository allowlist	2026-07-14 02:01:00 +07:00
Eva	5b65f610a9	feat(eval): require bearer auth for remote binding	2026-07-14 01:45:28 +07:00
Gergő Magyar	c6445096eb	fix: stop Napi::Error SIGABRT on analyze — index C++ type lookups, terminate workers only at JS-safe points (#2432) (#2436)	2026-07-11 18:07:08 +01:00
VL	117587d543	fix(cli): actionable diagnostics for non-4K page-size buffer manager failures (#2424)	2026-07-10 19:12:53 +01:00
Gergő Magyar	df1fc36094	fix: make large incremental writebacks commit reliably (#2409) (#2425)	2026-07-10 14:05:23 +01:00
Gergő Magyar	a7a5ea65a6	fix: report custom HTTP embedding endpoint failures instead of huggingface download errors (#2385) (#2386)	2026-07-06 22:06:08 +01:00
Gergő Magyar	76a1c90b02	fix(fts): diagnose Windows FTS missing-dependency load failures (#2374, Phase 1) (#2383) ... * feat(lbug): classify FTS extension load errors with Windows missing-dependency guard (#2374) Add classifyExtensionLoadError() — a pure-string, lbug-free four-way classifier (missing_file / corrupt_file / missing_dependency / unknown). The Windows catch-all guard keys missing_dependency strictly on the error-126 signal, never LadybugDB's generic 'Failed to load library … needed by extension' wrapper, so 127/5/1114 and truncated (193) files route correctly. * feat(fts): surface classified missing-dependency remedy in doctor, repair-fts, and degrade warnings (#2374) Route the FTS load reason through classifyExtensionLoadError at all four surfaces (doctor, --repair-fts error, analyze degrade log, ftsDegradedWarning). For the Windows missing-dependency class, emit the runtime-install remedy (VC++ redist, then OpenSSL) instead of the wrong reinstall-over-network guidance; other classes keep their existing routing. Path redaction preserved on the client-facing warning. * test(fts): assert doctor surfaces the classified remedy end-to-end (#2374) Extend the broken-file e2e: doctor now prints the corrupt-file re-download remedy through the real CLI, and the Windows missing-dependency remedy (VC++/OpenSSL) must not misfire on a corrupt file — the catch-all guard, verified end-to-end. Also assert the repair path does not misfire. * style(fts): apply prettier formatting to #2374 diagnosis files * feat(fts): language-independent hedged fallback for Windows load failures (#2374) The Windows OS-error tail is localized, so matching only en/zh 126 text left other locales on the generic 'run doctor' remedy. lbug's 'Failed to load library' wrapper is English on every platform and present for all load failures, so use it as a fallback: when the localized tail matches no specific class, emit a hedged remedy that points the user at their own OS error and offers both branches (install runtime / --repair-fts) without prescribing the wrong single fix. Precise en/zh 126 keeps its definite remedy. * feat(fts): language-independent structural classifier via binary inspection (#2374) Add diagnoseExtensionLoad: pull the extension's file path out of lbug's own English wrapper and inspect the binary header (PE/ELF/Mach-O magic + arch) directly, so corrupt-vs-valid is decided by the file itself, not the localized OS-error tail. A valid binary that still failed to load ⇒ missing_dependency (runtime dep), decided in any OS display language and on all three platforms. Falls back to the string classifier (with its hedged fallback) when the file can't be read. Wire all four surfaces to it. Event Viewer / GetLastError-via-FFI were dead ends (lbug catches the failure — no crash event; no native FFI dep). * test(fts): exercise the structural classifier on real binaries (#2374) Add an integration suite that runs inspectExtensionBinary/diagnoseExtensionLoad against genuine binaries — the running node executable, the real lbugjs.node addon, and the installed FTS extension (valid); a truncated real binary and a real text file (corrupt). Registered in cross-platform-tests PLATFORM_LOGIC so it runs on the Windows + macOS matrix, proving the PE and Mach-O header parsing on real PE/Mach-O files (ubuntu covers ELF). * fix(fts): honor a corrupt_file verdict over a structurally-valid header (#2374) The structural probe in diagnoseExtensionLoad inspects only the first 4 KB, so a download truncated after its header reads 'valid' and was routed to the "install VC++, reinstalling will NOT help" remedy — the exact loop #2374 exists to kill, for the truncated-download case the module docstring claims it handles. Honor the loader's own corruption report ("file too short" / Windows error 193 "not a valid Win32 application") before defaulting to the dependency remedy; localized corrupt tails stay hedged missing_dependency, preserving language-independence. Addresses PR #2383 review finding F1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(fts): return indeterminate for a PE header beyond the read window (#2374) The structural probe reads only BINARY_HEADER_BYTES (4 KB). A valid PE with a large DOS stub whose e_lfanew points past that window was wrongly called 'corrupt', routing a fine DLL to "re-download". A garbage e_lfanew from a truly corrupt file is indistinguishable from here, so widen the header verdict with 'indeterminate' and return it in that case; the caller then defers to the loader's own report instead of asserting a false verdict. Fat Mach-O stays valid (LadybugDB ships thin per-arch binaries). Also covers the unmapped-arch and garbage-PE-signature branches. Addresses PR #2383 review finding F1-secondary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(fts): drop contradictory reinstall guidance from the analyze degrade log (#2374) For a missing runtime dependency the extension file is present, so appending FTS_UNAVAILABLE_MESSAGE (which tells the user to install it "with network access") to the remedy ("reinstalling will NOT help") produced self-contradictory guidance on the main analyze surface. Lead the missing_dependency degrade log with the class-neutral sentence (FTS_UNAVAILABLE_LEAD) and append only the classified remedy; other classes keep FTS_UNAVAILABLE_MESSAGE unchanged. Addresses PR #2383 review finding F2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(fts): cache the load diagnosis so the degraded warning does no per-request I/O (#2374) ftsDegradedWarning() runs on every degraded /api/search response and MCP query, and it was calling diagnoseExtensionLoad — a synchronous openSync/readSync of the extension file — on every call. Compute the diagnosis once at mark-unavailable time (the single load-failure sink, run per Database not per request), cache it on ExtensionCapability, and have the warning read the cached result (falling back to the pure, no-I/O string classifier if it is absent). Loader capability-shape assertions relax from toEqual to toMatchObject for the new optional field. Addresses PR #2383 review finding F3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(fts): cover the missing_dependency remedy on the --repair-fts path (#2374) The repair-fts error interpolates the classified remedy, but no test reached the missing_dependency branch — only the corrupt/invalid-ELF path. Add a Windows error-126 case asserting the thrown error carries the VC++ redistributable remedy and omits the old "retry the network install" tail, and that no index is dropped. Addresses PR #2383 review finding F6a (--repair-fts surface). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(fts): share the VC++ redistributable install hint (#2374) The Microsoft Visual C++ redistributable name and aka.ms URL were duplicated verbatim in WINDOWS_MISSING_DEPENDENCY_REMEDY and STRUCTURAL_MISSING_DEPENDENCY_REMEDY. Factor a single VC_REDIST_INSTALL_HINT constant so the pointer cannot drift between them; the composed remedy strings are byte-identical (existing exact-text assertions unchanged). Also adds a test covering the previously-unexercised structural remedy branch. Addresses PR #2383 review finding F5a. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(fts): guard FILE_CORRUPTION_SIGNATURES parity with the installer script (#2374) The corruption-signature list is deliberately duplicated between extension-load-error.ts and scripts/install-duckdb-extension.mjs (the .mjs cannot import the .ts), with nothing guarding against drift — a one-sided edit would desync the FORCE-INSTALL verb from remedy classification. Export the array from both and add a parity test that compares regex source + flags element-wise. Addresses PR #2383 review finding F5b. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(test): run extension-binary-real in the sequential lbug-db vitest project (#2374) extension-binary-real.test.ts imports @ladybugdb/core but ran in the parallel `default` project, contrary to TESTING.md's rule that native-LadybugDB tests live in the sequential `lbug-db` project. Add it to the lbug-db include list and the default exclude list; it now runs under lbug-db and no longer under default. Addresses PR #2383 review finding F6c. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(fts): fail loud, not silent-skip, on missing FTS artifacts under REQUIRE_FTS=1 (#2374) The real-binary structural tests gated on raw .skipIf(!lbugNative) / .skipIf(!installedFts), so under GITNEXUS_REQUIRE_FTS=1 a missing artifact would silently vanish from a green CI run (the #2299 trap). These tests inspect the extension file directly and need its path, not a loaded connection — so skipUnlessFtsAvailable (which needs an initialized LadybugDB) does not fit. Add requireFtsResourceOrSkip: skip gracefully offline, throw under REQUIRE_FTS=1. The always-on process.execPath assertion still runs everywhere. Addresses PR #2383 review finding F6d. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(fts): apply prettier formatting to the #2383 fix files (#2374) Line-wrapping only; the quality/format CI check flagged three files. No behavior change. 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-07-06 21:27:37 +01:00
Gergő Magyar	177bbc89c3	fix: surface real FTS extension LOAD errors and self-heal broken extension files (#2374) (#2375)	2026-07-06 06:41:05 +01:00
Gergő Magyar	cdad478c96	fix: proxy-blocked installs survive onnxruntime-node postinstall and self-heal embeddings (#2370) (#2372)	2026-07-05 16:15:10 +01:00
Gergő Magyar	187c162fd8	feat: full Codex support — hooks, plugin marketplace, and setup (#2328, supersedes #1131) (#2369) ... * feat(setup): install Codex PreToolUse/PostToolUse hooks (#2328) Codex CLI supports lifecycle hooks with Claude Code's exact {hooks: {Event: [...]}} JSON schema, stdin payload, and hookSpecificOutput response contract, registered in a dedicated ~/.codex/hooks.json (https://developers.openai.com/codex/hooks). Parameterize installClaudeCodeHooks into installClaudeSchemaHooks (claude | codex): both runtimes share the installer, the bundled gitnexus-hook.cjs adapter, and its helpers. A codex HookTarget in editor-targets.ts makes uninstall and the setup-uninstall round-trip tripwire cover the new surface with no uninstall.ts changes. SessionStart is deliberately not registered: Codex reads AGENTS.md natively, which already carries the GitNexus context block. Closes #2328. Closes #244 (Codex setup support is now complete: MCP + skills + hooks). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(plugin): make the GitNexus plugin installable from Codex (#1131) Codex's plugin system (https://developers.openai.com/codex/plugins/build) reads a .codex-plugin/plugin.json manifest and a repo-root .agents/plugins/marketplace.json registry. The existing gitnexus-claude-plugin/ is already Codex-compatible as-is — Codex sets CLAUDE_PLUGIN_ROOT for hook-command compatibility, loads the same SKILL.md skills, hooks/hooks.json, and .mcp.json — so a second manifest in the same folder replaces PR #1131's duplicated plugin tree with zero copied skills or hooks. The .gitignore .agents/ scratch rule narrows to re-include only the registry file. Install: codex plugin marketplace add abhigyanpatwari/GitNexus Supersedes #1131. Co-authored-by: jublin <1799126+jublin@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: document Codex full support (MCP + skills + hooks + plugin) Promote Codex to Full in both editor tables, document the ~/.codex/hooks.json hook install, and add the Codex plugin marketplace install path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(release): extend the version-lockstep guard to the Codex manifests The always-on drift guard asserted only the Claude plugin manifests against gitnexus/package.json, so a release could ship stale versions in .codex-plugin/plugin.json and .agents/plugins/marketplace.json without CI noticing. Mirror the Claude lockstep test for the two Codex files and extend the CONTRIBUTING §Releases lockstep list to match. Verified guard semantics: a deliberate local version mutation of the Codex marketplace entry turns the new test red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(plugin): quote the hook command path for space-containing plugin roots Both plugin hook commands ran `node ${CLAUDE_PLUGIN_ROOT}/hooks/...` unquoted, which breaks whenever the substituted plugin root contains a space — the common case on Windows user profiles. Both Claude Code and Codex substitute the placeholder before shell execution, and Claude Code's plugin docs mandate the double-quoted form in shell-form hooks. No commandWindows entry: Codex source (codex-rs hooks engine) falls back to `command` on Windows with identical placeholder substitution, so an identical-content override would be pure duplication. Verified: space-in-root smoke test (old form exits 1 MODULE_NOT_FOUND, quoted form exits 0), `claude plugin validate` passes, and a local `codex plugin marketplace add` parses the marketplace + plugin cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(setup): pin fail-closed behavior for unreadable/corrupt Codex hooks.json The non-ENOENT suite covered Claude settings.json (EACCES) and Codex config.toml (EACCES) but not the new ~/.codex/hooks.json surface, and the mergeHooksJsonc "is corrupt" branch had zero coverage for either editor. A future refactor dropping the isEnoent rethrow or the parse gate could silently rewrite a user's hooks.json gitnexus-only with no CI tripwire. Two regression tests: EACCES leaves hooks.json byte-identical and reports "Codex hooks: EACCES"; corrupt content is preserved and reported via "Codex hooks: hooks.json is corrupt". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(readme): add the Codex plugin-marketplace install path to the npm README The root README documents the one-step plugin route but the package README (what npmjs.com renders) only showed the setup-CLI path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(setup): rename claudeHook to hookCfg in installClaudeSchemaHooks The local held a codex HookTarget on the codex branch since the installer was parameterized, so the claude-specific name misled. Pure local rename, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(readme): document Codex SessionStart exclusion, /hooks trust gate, and install-route choice Three behaviors were only recorded in code comments and the PR body: SessionStart is deliberately not registered (Codex reads AGENTS.md natively), setup-installed hooks need one-time /hooks approval in Codex, and the setup CLI and plugin are alternative install routes whose hooks load alongside each other if both are used. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(test): share one logLines helper across setup.test.ts describes The corrupt-hooks.json test inlined the console.log-flattening expression that the non-ENOENT describe already defined locally. Hoist a single file-scope logLines so the two stay in sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-04 13:32:17 +01:00
Gergő Magyar	6252aa745f	feat(setup): add CodeBuddy and Qoder coding-agent integrations (#2368) ... * feat(setup): add CodeBuddy and Qoder coding-agent integrations Adds Tencent CodeBuddy and Alibaba Qoder to gitnexus setup/uninstall, fitted to the editor-targets registry and --coding-agent selection. - CodeBuddy: MCP entry written into the first existing file of its documented priority chain (~/.codebuddy/.mcp.json recommended, ~/.codebuddy/mcp.json deprecated, ~/.codebuddy.json legacy) so a populated deprecated config is never shadowed; skills to ~/.codebuddy/skills/ (https://www.codebuddy.ai/docs/cli/mcp) - Qoder: MCP entry in ~/.qoder.json, skills to ~/.qoder/skills/ (https://docs.qoder.com/cli/using-cli, /extensions/skills) - editor-targets gains optional legacyFiles; uninstall sweeps them - roster strings updated (CLI help, i18n en/zh-CN, READMEs); en/zh-CN setup descriptions were stale (missing Antigravity) and are refreshed Supersedes and credits PR #1030 by @zykai0302, re-fitted to the post-#2168 selective-agent architecture with documented config paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(cli): assert stable zh-CN setup-description fragment * fix(setup): surface non-ENOENT config read/stat failures instead of clobbering * fix(setup): report corrupt legacy MCP files informationally during uninstall * test(setup): cover multi-candidate uninstall sweep combinations * fix(setup): detect CodeBuddy/Qoder installs via existing MCP config files * fix(setup): skip empty and non-file candidates in the MCP config chain * docs: add CodeBuddy and Qoder manual MCP configuration sections * test(ci): run the setup-uninstall round-trip in the cross-platform matrix * fix(setup): never claim "not configured" when uninstall recorded errors * refactor(cli): share the isEnoent predicate via editor-targets * refactor(setup): share chain-file install detection between CodeBuddy and Qoder --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-04 10:54:50 +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
Livio Gamassia	d546fa3cce	fix(storage): rename index metadata to gitnexus.json with dual-write compatibility (#2363)	2026-07-03 19:32:59 +01:00
ACT900	5aada28da5	fix(embeddings): use system-matched onnxruntime-node CUDA build so CUDA 13 hosts use the GPU (#2341) ... * fix(embeddings): use system-matched onnxruntime-node CUDA build so CUDA 13 hosts use the GPU transformers.js exact-pins a CUDA-12 onnxruntime-node while gitnexus' own dep floats to a CUDA-13 build. npm/pnpm cannot dedupe an exact pin against a range, so npm i -g installs two copies and the gitnexus overrides block (root-only) is inert. On a CUDA-13-only host the nested CUDA-12 provider cannot load libcublasLt.so.12, the CUDA EP fails, and embeddings silently fall back to CPU (isCudaAvailable() also only probed .so.12). Add onnxruntime-node-resolver.ts (module.registerHooks redirect to the host-matching build, no-op elsewhere) mirroring onnxruntime-common-resolver.ts; probe libcublasLt .so.12 OR .so.13 against the copy that actually loads; unit test with 12 cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(embeddings): wire CUDA-13 build-match resolver into MCP query embedder The MCP query-time embedder (src/mcp/core/embedder.ts) has its own, separate initEmbedder() used for semantic search — it only called ensureOnnxRuntimeCommonResolvable() before importing transformers.js, so the CUDA-13 build-matching redirect added for the analyze/CLI embedder never applied here. A CUDA-13 host running MCP search with --embedding-device cuda still loaded the mismatched default onnxruntime-node build. Wire ensureOnnxRuntimeNodeMatchesSystem() into the same call site, mirroring the core embedder's ordering (registered after the common-resolver fallback, before the dynamic transformers import). * fix(embeddings): gate CUDA redirect decision on registerHooks availability decide() computed the CUDA-major redirect independent of whether Node's module.registerHooks API actually exists — only ensureOnnxRuntimeNodeMatchesSystem() checked that. On Node 22.0-22.14 (allowed by this package's engines floor; registerHooks needs >=22.15), isCudaAvailable() could therefore report a redirect target that ensureOnnxRuntimeNodeMatchesSystem() then silently failed to install, so transformers.js loaded the mismatched default onnxruntime-node build while the embedder still requested device:'cuda' against it — reintroducing the uncatchable native crash this probe exists to prevent. Move the registerHooks check to the top of decide() so the probe and the loader can never disagree, and skip CUDA-major probing entirely in that case (a redirect could never install anyway). Also fixes a related test-helper bug found while writing this unit's tests: loadResolver's destructuring default (`registerHooks = vi.fn()`) silently substituted a real mock function even when a test passed `registerHooks: undefined` to simulate Node < 22.15 — meaning the existing 'no-ops... when registerHooks is unavailable' test never actually exercised that path. Distinguish 'omitted' from 'explicitly undefined' via an 'in' check. * test(embeddings): drive decide() -> redirect:true and assert the resolve() closure The PR's actual shipped behavior — the installed registerHooks resolve() closure, and the full redirect-active decision path — had zero executed test coverage. All 4 prior ensureOnnxRuntimeNodeMatchesSystem tests avoided driving decide() into redirect:true because require.resolve/createRequire were never mocked, so the two-distinct-directory comparison decide() depends on always resolved against whatever's actually installed in this test's real node_modules (a single real copy, not the PR's two-copy scenario). Extend loadResolver()'s existing node:module mock to also fake createRequire, keyed by call origin, so resolveOurOrtNodeDir/resolveDefaultOrtNodeDir can be driven to two distinct fake directories with distinct CUDA majors — reaching redirect:true without adding any injection points to production code. Then capture the installed resolve() closure (mirroring the sibling onnxruntime-common-resolver.test.ts's captureResolve() pattern) and assert its three branches directly: onnxruntime-node redirect, onnxruntime-common redirect, and passthrough for any other specifier. * fix(embeddings): distinguish ldd detection-failure from no-CUDA-provider ortCudaMajor treated any execFileSync('ldd', ...) failure with no usable stdout (missing ldd binary, permission-denied .so, sandboxed exec) identically to 'CUDA provider genuinely absent'. The pre-PR detection (hasOrtCudaProvider) only used existsSync, never ldd, so this is a regression: a CUDA-12 host that worked fine before this PR can now silently fall back to CPU if ldd itself can't run, even though the provider .so and system CUDA libs are both genuinely present. readSoNeeded now reports whether ldd produced any usable output at all, distinct from 'ldd ran and just found no matching NEEDED entry' (the existing, already-handled '=> not found' case). When detection genuinely fails, log a warning so an operator can tell 'CPU fallback because detection itself failed' apart from 'CPU fallback because no CUDA build shipped' — the return value stays null either way (the type can't distinguish a third state), but the two cases are now observably different via the log. * fix(embeddings): check ourDir independently of whether defaultDir resolved decide()'s ourDir fallback lookup was nested inside 'if (systemMajor != null && defaultDir)', so a null defaultDir (transformers' own onnxruntime-node resolution failing outright, e.g. a partial/broken install) skipped checking ourDir entirely — getEffectiveOnnxRuntimeNodeDir() returned null even when gitnexus' own matching CUDA-13 copy would have resolved fine and worked. defaultDir resolving is not a precondition for the comparison: an unresolvable default already counts as 'the default doesn't match', so the ourDir check now runs whenever systemMajor is known, regardless of whether defaultDir resolved. * fix(embeddings): prefer CUDA 13 globally across the env-var directory scan detectSystemCudaMajor's CUDA_PATH/LD_LIBRARY_PATH scan returned on the first CUDA-major match within a single dir/sub pair, so a stale .so.12 found early (e.g. a leftover CUDA_PATH entry from a prior install) shadowed a genuine .so.13 found later in the search path, even though the scan's own ordering (checking 13 before 12 within each pair) was clearly intended to prefer 13 wherever possible. Keep scanning the full search space once a 12 is found, only returning early once a 13 is found (the best possible answer) or the space is exhausted. * fix(embeddings): have onnxruntime-common-resolver defer to the effective onnxruntime-node dir onnxruntime-common-resolver.ts independently re-derived transformers' default onnxruntime-node dir (its own copy of the 'resolve transformers' main entry, then onnxruntime-node' walk) to compute which onnxruntime-common to pair with — duplicating onnxruntime-node-resolver.ts's own walk, and capable of disagreeing with it: when the CUDA-major redirect is active, this hook would still pair onnxruntime-common with transformers' default (unredirected) onnxruntime-node, not the redirected copy the other hook just switched onnxruntime-node itself to. Have it call the already-exported getEffectiveOnnxRuntimeNodeDir() instead — the same decision the CUDA-major redirect hook uses — so both hooks always agree on which onnxruntime-node they're pairing onnxruntime-common against, and the duplicated resolve-walk is removed entirely rather than merely factored out. * fix(embeddings): cache the effective CUDA major to remove redundant subprocess spawns isCudaAvailable() in embedder.ts re-invoked ortCudaMajor/detectSystemCudaMajor directly even though decide() (via getEffectiveOnnxRuntimeNodeDir) had already computed both to make its redirect decision — a second, wasted ldconfig + up to 2 ldd spawns on every initEmbedder() call. Add effectiveMajor to the memoized Decision, computed once inside decide() alongside effectiveDir/systemMajor, and export a single isEffectiveCudaAvailable() that reads straight from the cached decision. embedder.ts's local isCudaAvailable() wrapper (and its now-unused getEffectiveOnnxRuntimeNodeDir/ortCudaMajor/detectSystemCudaMajor imports) is replaced by this one exported function. * fix(embeddings): surface CUDA redirect state at info level and in doctor A successful CUDA-build redirect logged only at logger.debug (filtered by the default 'info' level), and gitnexus doctor's embeddings section never mentioned the redirect at all — leaving no diagnostic path for 'why is my CUDA-13 host still on CPU' after this PR ships. Log the successful-redirect line at info (no-redirect/failure paths stay at debug, since those are the common, expected case). Add cudaRedirectDoctorStatus(), a pure summary of decide()'s already-computed decision mirroring doctor.ts's existing localEmbeddingDoctorStatus shape, and print it as a new literal (non-i18n) 'CUDA:' line in doctor's embeddings section alongside the existing 'Support:' line, matching that line's established convention. * test(embeddings): register onnxruntime-node-resolver.test.ts in the cross-platform subset The new test file guards on process.platform (linux/darwin cases) but was absent from cross-platform-tests.ts's PLATFORM_LOGIC list, which TESTING.md says platform-sensitive tests should be added to — so it never ran on the Windows/macOS CI matrix, only Ubuntu. Note: the sibling onnxruntime-common-resolver.test.ts has the identical, pre-existing gap (it predates this PR) — left as-is here, since fixing unrelated pre-existing test-registration debt is out of scope for this PR's own follow-up fixes. * test(embeddings): strengthen weak assertions, add garbled-output and CUDA_PATH coverage Three of the four ensureOnnxRuntimeNodeMatchesSystem tests only asserted 'doesn't throw' rather than a concrete outcome — including one literally named 'idempotent' that never asserted a call count on its own spy. Strengthen each to assert real outcomes (module stays functional after a no-op; spy call counts; return-value shape), while keeping the true install-once idempotency proof in the redirect-active test added earlier (this file's no-redirect scenario can't exercise it, since registerHooks is never called either way). Add the missing edge cases flagged in review: a CUDA_PATH-only fallback scan test (mirroring the existing LD_LIBRARY_PATH one), and garbled/ unrecognized ldconfig and ldd output cases for both detectSystemCudaMajor and ortCudaMajor, confirming neither falsely matches a CUDA major on unparseable input. Also parameterize the non-linux platform test across both darwin and win32 rather than darwin alone. Not changed: the process.env reassignment vs. Object.defineProperty 'inconsistency' flagged in review — process.env, unlike process.platform, has no getter-only restriction, so plain reassignment is already correct and switching it to Object.defineProperty would be unnecessary ceremony. * docs(embeddings): note the npm link/symlinked dev-checkout resolution caveat resolveOurOrtNodeDir/resolveDefaultOrtNodeDir anchor to this module's own real (post-symlink) location via import.meta.url, so a linked local dev checkout may resolve against its own node_modules rather than the consuming app's. Narrow, dev-only blast radius (regular npm/pnpm installs are unaffected) — document-only, no structural fix warranted. * fix(test): point the windowsHide spawn-family registry at the file that actually spawns hooks.test.ts's windowsHide regression check still listed gitnexus/src/core/embeddings/embedder.ts as a child_process-spawning file, but this PR itself already moved all execFileSync usage out of embedder.ts and into the new onnxruntime-node-resolver.ts — without updating this registry. The check was silently failing at the PR's own head commit (confirmed: 0 spawn-family calls found in embedder.ts, 'expected 0 to be greater than 0'), a pre-existing gap this fix-pass surfaced via a full-suite run rather than something introduced by any of the preceding follow-up commits. Swap the registry entry to onnxruntime-node-resolver.ts, which does import execFileSync (ldd + ldconfig, both already correctly passing windowsHide: true). * fix(test): make onnxruntime-node-resolver.test.ts path comparisons OS-agnostic Registering this file in cross-platform-tests.ts's PLATFORM_LOGIC (a prior commit in this series) means it now runs on the Windows CI matrix, not just Ubuntu — and several of the fakeDirs-based tests (redirect:true, ourDir-independent, subprocess-count, doctor-status) compared the resolver's real join()/dirname() output against hardcoded forward-slash fixture strings via exact-match or .startsWith(). Node's module is bound to path.win32 (or path.posix) based on the REAL host OS at process start — stubbing process.platform later, as these tests already do for the resolver's own platform branching, has no effect on it. So on a genuine Windows runner, join(effectiveDir, 'package.json') backslash-normalizes even under a faked platform:'linux', silently breaking every forward-slash comparison in this file: the createRequire dispatch would route to the wrong fake require, throw MODULE_NOT_FOUND, get swallowed by ensureOnnxRuntimeNodeMatchesSystem's outer try/catch, and registerHooks would never fire — the redirect-active tests would fail outright on Windows CI. Normalize every comparison point (the createRequire dispatcher, and the shared execFileSync/existsSync mocks) with a single toPosix() helper. Added a forceWin32Path test option (using path.win32's real join/dirname behavior) to prove this holds without needing an actual Windows runner — confirmed by temporarily reverting the fix and observing the new test fail with the exact predicted mismatch before restoring it. * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(embeddings): keep CUDA auto-detect working on Node < 22.15 when the default build already matches The registerHooks guard in decide() returned effectiveMajor: null unconditionally, so on Node 22.0-22.14 / 23.0-23.4 (engines floor is >=22.0.0) isEffectiveCudaAvailable() was always false and a CUDA-12 host whose default onnxruntime-node build already matched — which needs no hook at all to use the GPU — silently regressed from CUDA to CPU on the auto device path (pre-PR isCudaAvailable() behavior). Probe the system and the default copy regardless of registerHooks availability; only the ourDir redirect branch stays gated on it, so the probe still never reports a redirect target that cannot be installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-07-02 07:53:32 +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
Gergő Magyar	aa8c567126	fix(lbug): stop --pdg analyze double-free (skip LadybugDB close-destructor crash) + harden connection serialization (#2264) ... * fix(lbug): serialize singleton connection to stop --pdg analyze double-free LadybugDB is single-writer and its Connection is NOT safe for concurrent query execution. The WAL-checkpoint driver (5s setInterval) issued `conn.query('CHECKPOINT')` on the same module-singleton `conn` the analyze pipeline used for COPY. With --pdg the extra BasicBlock / REACHING_DEF / CDG / POST_DOMINATE / TAINTED / CALL_SUMMARY / TAINT_PATH table COPYs outlast the 5s tick, so a checkpoint executed concurrently with an in-flight COPY on one connection -> two libuv workers mutate shared native state -> heap corruption ("double free or corruption (out)" / SIGABRT, detected at the final "Saving metadata..." free). Fix: add conn-lock.ts (`withConnLock`, a promise-chain mutex) and run every singleton-`conn` helper's full query + result-drain inside it: queryAndDrain (when targetConn === conn), executePrepared, executeWithReusedStatement, flushWAL, tryFlushWAL, getLbugStats, deleteAllInterprocTaintPaths, deleteAllCallSummaries. Add an `if (inflight) return` reentrancy guard to the driver tick so overdue ticks don't stack checkpoints. streamQuery is intentionally NOT wrapped (read path, re-entrant per-row callback). Reproduced the crash with concurrent queries on one raw Connection (serial = stable); verified the fix drives the same overlap through the locked adapter without crashing. Tests: conn-lock serialization (no overlap / FIFO / throw-releases) and driver reentrancy guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): lock deleteAllCommunitiesAndProcesses against the WAL driver (#2264) The count + DETACH DELETE ran raw conn.query on the singleton connection during incremental --pdg writeback while the WAL-checkpoint driver was live — the same concurrent CHECKPOINT-vs-write double-free this branch fixes elsewhere. Wrap the body in withConnLock, mirroring the already-wrapped deleteAllInterprocTaintPaths. Adds test/integration/lbug-conn-serialization.test.ts (call-through withConnLock spy) asserting the helper now acquires the lock, wired into the lbug-db vitest project (and excluded from the default project so it doesn't run twice). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): lock queryImporters against the WAL driver (#2264) queryImporters issued a raw conn.query on the singleton connection inside the importer-BFS loop of incremental --pdg writeback, while the WAL-checkpoint driver could fire a concurrent CHECKPOINT — the same double-free class. Wrap the read (query + getAll + drain) in withConnLock. Extends lbug-conn-serialization.test.ts with a routing assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): lock deleteNodesForFile count query on the singleton path (#2264) The per-table count read used a raw targetConn.query while the sibling DETACH DELETE already routed through the locked queryAndDrain — an asymmetry that left the count racing the WAL-checkpoint driver during incremental --pdg writeback. Gate the count through withConnLock when targetConn === conn (the singleton), matching queryAndDrain; per-query/temp connections stay lock-free. Test asserts the count loop takes the lock once per filePath-bearing node table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): drain DELETE results in the deleteAll* helpers (#2264) deleteAllInterprocTaintPaths, deleteAllCallSummaries, and deleteAllCommunitiesAndProcesses awaited conn.query(...DELETE...) but dropped the returned QueryResult (only the count result was closed), leaking a native result handle and violating the helpers' own "query + drain inside the lock" contract. Close each delete result via closeQueryResults, matching the count handling. Adds a seeded drain test (closeQueryResults fires for the DELETE result). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): make the conn-lock non-reentrancy invariant enforced, not just documented (#2264) A future withConnLock-wrapped helper calling another wrapped helper would await its own holder's tail and hang silently. Add an AsyncLocalStorage-based re-entry guard: withConnLock throws a clear error when invoked from within a holding fn's async context. A boolean flag can't do this — a legitimately-queued top-level caller also runs while the lock is held; only AsyncLocalStorage distinguishes a true nested call from normal contention. Tests: re-entry throws (not deadlocks); sequential and concurrent top-level calls do NOT false-fire; the lock releases after a re-entry throw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * refactor(lbug): rename __resetConnLockForTests to _resetConnLockForTests (#2264) Match the repo's single-underscore test-seam convention (_initLockPathForTest). Pure rename of the @internal export and its sole importer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(lbug): fix stale CHECKPOINT guard regex after the c.query refactor (#2264) lbug-checkpoint.test.ts asserted exactly two CHECKPOINT sites by grepping the literal `conn.query('CHECKPOINT')`. The connection-serialization refactor changed flushWAL/tryFlushWAL to capture `const c = conn` and call `c.query('CHECKPOINT')` inside withConnLock, so the literal grep found 0 and the test failed (expected 2). Make the regex receiver-agnostic (`.query('CHECKPOINT')`) — preserves the guard's intent (exactly two authorized CHECKPOINT sites; a third is a regression) while tolerating the captured-receiver form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): skip native close on CLI exit to dodge LadybugDB destructor double-free (#2264) THE actual fix for the `analyze --pdg` crash. gdb shows the abort is a double-free inside LadybugDB's own destructor during conn.close(): "double free or corruption (out)" -> abort lbug::main::ClientContext::~ClientContext() lbug::main::Connection::~Connection() NodeConnection::Close(...) <- conn.close() from safeClose It reproduces with the WAL driver OFF and with serial load, so it is NOT the checkpoint/COPY concurrency the rest of this branch serialized — it's a native LadybugDB engine bug (@ladybugdb/core 0.17.1, latest stable) triggered by the larger --pdg write set, firing during teardown AFTER a fully-written, checkpointed index. Fix: closeLbug({ skipNativeClose }) CHECKPOINTs for durability (flushWAL) then skips conn.close()/db.close(), leaving the handles referenced so no GC finalizer re-runs the destructor. The CLI analyze command (success, error, and SIGINT paths all process.exit) opts in via skipNativeCloseOnExit; long-lived callers (MCP server, tests) keep the real close. Mirrors the pool adapter's fire-and-forget native close and the ONNX native-cleanup philosophy. Validated end-to-end: `analyze --pdg --force` now exits 0 with a 193,876-node index; re-opening it (no --force) reads clean and reports up-to-date, proving the CHECKPOINT-only persistence is durable without db.close(). Workaround for an upstream LadybugDB bug (ClientContext destructor double-free). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): keep conn.close()/db.close() literals out of the closeLbug comment (#2264 review P1-1) The skipNativeClose comment in closeLbug contained the literal `conn.close()`/ `db.close()`, which the structural guard test (lbug-checkpoint.test.ts:52-53 — "closeLbug must not inline conn.close()/db.close()") greps for and fails on. Reword the comment to describe the native close without the literal tokens; the code already delegates close exclusively to safeClose, so the guard's intent holds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): real close on the analyze error path to avoid a hang under skipNativeClose (#2264 review P1-2) The CLI error handler soft-returns (process.exitCode = 1) instead of forcing exit, relying on the released native handles to let Node terminate. The earlier commit made runFullAnalysis's error-path closeLbug skip the native close, leaving live LadybugDB handles that keep the event loop alive forever — a post-init analyze failure would hang. Only the SUCCESS path (which guarantees a following process.exit) skips the native close; the error path now always closes for real. A late-error close could still abort in the destructor, but that terminates the process — it does not hang. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): skip native close in the analyze worker to avoid the LadybugDB destructor crash (#2264 review P2-3) The forked server analyze worker runs runFullAnalysis then force-exits (process.exit(0)). With a real native close inside runFullAnalysis, the LadybugDB ClientContext destructor can double-free after --pdg writes and abort the worker BEFORE it sends 'complete', failing the parent's analyze. Pass skipNativeCloseOnExit: true so the worker checkpoints for durability and lets its process.exit reclaim the handles — same about-to-exit contract as the CLI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(cli): force exit on a soft error-return when LadybugDB handles are open (#2264 review P1) The full-analysis success path skip-closes LadybugDB (handles left open, reclaimed by process.exit). If a post-finalize step (assertAnalysisFinalized) then throws, the outer catch soft-returns (process.exitCode = 1) — and with native handles open the event loop never drains, so the process HANGS instead of exiting 1. Guard once at the analyzeCommand wrapper, after the try/finally: if isLbugReady() (handles still open) the analyze actually ran and we must force the exit. The success path never reaches here (analyzeCommandImpl process.exit(0)s itself); early-validation errors and unit tests that mock runFullAnalysis never open the DB (isLbugReady() false), so the soft return is preserved. Adds analyze-finalize-failure-exits.test.ts (force-exits when handles open; does NOT when they aren't). The analyze-*.test.ts that mock lbug-adapter now also mock isLbugReady (vitest throws on accessing an undefined export of a mocked module). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): skip the native close on the analyze error path too (#2264 review P2) A real conn.close() on the error path after large --pdg writes can itself hit the LadybugDB ClientContext destructor double-free → SIGABRT, degrading an actionable exit-1 error into a raw native abort. Switch the error-path close to skipNativeClose (mirroring the success path). Safe now that the CLI catch force-exits when isLbugReady() (the prior commit): handles left open are reclaimed by that guaranteed process.exit, so the process terminates without the abort and without hanging. flushWAL keeps the partial index durable. Depends on the prior commit (CLI force-exit guard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): run loadCachedEmbeddings reads under withConnLock (#2264 review P2) loadCachedEmbeddings issued raw conn.query reads on the singleton connection outside withConnLock — safe today only because it runs before the WAL-checkpoint driver starts, an ordering invariant not enforced by code. Wrap the whole read in withConnLock so a future reorder can't race a CHECKPOINT on the connection. Leaf read; no nested wrapped helpers. Adds a routing assertion to lbug-conn-serialization.test.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(lbug): de-brittle the close/CHECKPOINT structural guard (#2264 review P2) lbug-checkpoint.test.ts grepped the adapter SOURCE (comments included) for conn.close()/db.close()/.query('CHECKPOINT') literals, coupling a passing test to comment wording — a prior commit had to reword a comment just to keep it green. Strip comments from the read source before the structural assertions so they reflect code only; the invariant (exactly two CHECKPOINT sites; close calls only in safeClose) is preserved and no longer breaks on a comment edit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * refactor(lbug): rel COPY uses the captured writeConn, matching node COPY (#2264 review P3) The relationship COPY passed the module-level `conn` to copyCsvWithRetry while the node COPY uses the captured `writeConn`. Use `writeConn` for both — one captured reference for the whole bulk load, removing the latent identity dependency. Same object during analyze (`conn` is only reassigned at open/close under the session lock), so the queryAndDrain `targetConn === conn` lock gate still engages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): make the analyze worker's IPC send() failure-safe (#2264 review P3) The worker's send() used `process.send?.(msg)` — the `?.` guards an undefined channel but not a throw from an already-closed one (ERR_IPC_CHANNEL_CLOSED). A throw in the catch-branch send() would escape the message handler and skip the scheduled `setTimeout(process.exit(0))`, stranding the worker (with skip-close leaving native handles open, #2264). Wrap process.send in try/catch so the exit always fires; a vanished child is a failure to the parent regardless. Not unit-tested: send() is module-private and importing the worker registers process signal handlers; the change is a defensive try/catch around one call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(cli): bound the SIGINT cleanup CHECKPOINT so Ctrl-C stays responsive (#2264 review P3) The SIGINT handler calls closeLbug({skipNativeClose:true}), whose flushWAL CHECKPOINT queues behind the connection lock held by an in-flight COPY — so a single Ctrl-C during a long --pdg COPY appeared hung until the COPY released. Race the cleanup against a 2s timeout before process.exit(130); the WAL replays on the next analyze. The double-Ctrl-C escape hatch is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): report analyze-worker errors over IPC, never swallow (#2264 P3) The worker's send() swallowed IPC failures (and a prior pass logged them to stderr). Per review, all worker errors must be reported back to the parent over the existing IPC channel (send({ type: 'error' })) and nothing silently dropped. - send() no longer catches: a dead channel (ERR_IPC_CHANNEL_CLOSED) throws instead of being swallowed. - Every handler (uncaughtException, unhandledRejection, SIGTERM, the analysis message handler) reports its error via send() in try and schedules process.exit in finally, so a throw from send() can no longer skip the exit and wedge the worker — the P3 'schedule the exit so it always fires' fix, without a swallow. - SIGTERM cleanup failures are now reported to the parent instead of an empty catch {}. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(workers): report every caught parse-worker error over IPC (#2264) The parse worker swallowed or only-locally-logged several caught errors, so they never reached the pool: the per-language-group catch was an empty catch {} that silently dropped the whole group on any throw (not just an unavailable grammar), the per-file parse/query-execution catches only logger.warn'd (worker-thread local), and the C++ template-constraint catch swallowed silently. Route all work-path catches through a new reportWarning() helper that posts { type: 'warning', message } to the pool (which logs it on the main thread AND resets the worker idle timer, so a worker grinding through failing files isn't falsely idle-evicted), with a logger.warn fallback for the non-worker path. The existing inline warning sites (query-compilation, the extractParsedFile callback, CFG build) are migrated to the same helper. The 4 optional-grammar module-load guards (Swift/Dart/Kotlin/C) stay silent: they run before the 'ready' handshake and their absence is already surfaced via result.skippedLanguages + the isLanguageAvailable gate. Fatal/group-aborting errors continue to flow through the message handler's { type: 'error', errorStack }. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(cli): harden finalize-failure test against forked-worker death (#2264) analyzeCommand calls installFatalHandlers(), which registers global unhandledRejection/uncaughtException handlers that call the REAL process.exit(1). Across this file's vi.resetModules() reimports they accumulate on `process`, and under CI timing a stray async rejection fired one while no process.exit spy was active — killing the forked vitest worker ("Worker exited unexpectedly"), which only surfaced once the full test lanes finished (they were pending at review time). Keep process.exit spied for the whole file (beforeAll/afterAll) so a fatal handler can never really exit mid-run, strip the handlers installFatalHandlers added in afterAll (preserving vitest's own, snapshotted up front) before restoring the real process.exit, and reset process.exitCode so the worker exits clean. Passes in isolation and grouped; behavior under test is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(analyze): don't take the up-to-date fast path for an unregistered repo (#2264) A prior 'analyze --name X' that hit a registry name collision writes meta.json (meta-save runs before registerRepo) but fails before registering — leaving the index up-to-date but UNREGISTERED. A later 'analyze --name X --allow-duplicate-name' then matched the up-to-date gate and early-returned WITHOUT registering, so the repo stayed invisible to list_repos/MCP and the CLI's assertAnalysisFinalized rejected it. --allow-duplicate-name could never heal it. This was latent on main, masked by the very close-hang this PR fixes: the lingering process pushed the cli-e2e #829 step-3 analyze past its 60s spawn timeout (status===null → the test's vacuous early-return). With the hang gone the analyze exits promptly, exit 1 surfaces, and the bug becomes deterministic on all platforms. Fix: the up-to-date fast path now short-circuits only when the repo is actually registered (new isRepoRegistered helper, sharing assertAnalysisFinalized's exact canonical/case-folded membership check). An indexed-but-unregistered repo falls through to the pipeline, which registers it honoring allowDuplicateName. Already registered repos keep the fast path unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * chore: trigger CI re-run Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(analyze): gate up-to-date self-heal on --allow-duplicate-name (#2264) The prior commit healed every up-to-date-but-unregistered repo by falling through to register it — which broke the #1169 guard: a plain `analyze` of an up-to-date repo whose registry entry is missing MUST fail loudly ("Analysis did not finalize") rather than silently register a possibly half-finalized index. Distinguish the two causes of "unregistered": - collision-rejected + user re-runs with --allow-duplicate-name → explicit intent to register, so fall through to the pipeline and register it (#829). - plain analyze, registry missing/wiped → keep the #1169 fail-loud behavior. So self-heal is gated on options.allowDuplicateName; isRepoRegistered is only read on that opt-in branch, so the common fast path keeps its single-stat cost. Both cli-e2e guards (#1169 fail-loud, #829 heal) now pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): skip the native close on analyze-worker SIGTERM cancellation (#2264 P2) cancelJob() / the 30-min timeout (analyze-job.ts) send SIGTERM to the forked analyze worker, but its SIGTERM handler still did a full `await closeLbug()` (native conn/db teardown) — even though normal completion now skips it via skipNativeCloseOnExit. A cancelled or timed-out --pdg server analyze could therefore still hit the LadybugDB ClientContext destructor double-free, or block behind the in-flight COPY's connection lock before exiting. Mirror the CLI SIGINT path: a best-effort CHECKPOINT with closeLbug({ skipNativeClose: true }) bounded by a 2s Promise.race timeout, then process.exit(0) (which reclaims the handles). A CHECKPOINT failure is reported to the parent over IPC rather than swallowed; the exit is in .finally so it always fires. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(cli): import analyze once in the finalize-failure test (#2264 CI) The test failed deterministically only on the ubuntu coverage lane (2/2 runs) while passing locally and in isolation, incl. with --coverage. Cause: the vi.resetModules() + per-test `await import('analyze.js')` re-instrumented the ENTIRE analyze module graph on every test; under --coverage on the memory-constrained CI runner that OOM/crashed the forked worker ("Worker exited unexpectedly" → the assertion never ran). Import analyzeCommand ONCE and drive the mocks per-test via mockReturnValue (resetModules wasn't needed — the hoisted mocks are controllable per-test). Keeps the whole-file process.exit spy + afterAll fatal-handler strip from the prior pass. Behavior under test is unchanged; passes in isolation and with --coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(cli): pre-set NODE_OPTIONS heap cap so ensureHeap can't re-exec (#2264 CI) Root cause of the ubuntu-coverage-only failure (3/3 CI runs, passing locally): analyzeCommand calls ensureHeap() (analyze.ts:715), which RE-EXECS the process — spawning `node <heap-flags> <argv>` with vitest's argv — unless NODE_OPTIONS already carries --max-old-space-size (analyze.ts:498). That re-exec killed the forked vitest worker ("Worker exited unexpectedly" → the assertion never ran). It only reproduced on the memory-constrained CI runner because locally a high V8 heap-size-limit also short-circuits ensureHeap (analyze.ts:501). Reproduced locally with NODE_OPTIONS="--no-warnings" (no heap cap) → same failure; fixed by pre-setting --max-old-space-size in beforeAll (restored in afterAll), the same workaround cli-e2e uses. Verified: passes under the repro condition, normally, and with --coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): worker asserts finalization before reporting complete (#2264 P2) The forked analyze worker reported {type:'complete'} straight after runFullAnalysis, so a server/web analyze of a half-finalized repo (meta.json written but the global registry entry missing — a prior collision-aborted run, or a wiped registry) was reported successful while the repo stayed unregistered/invisible to list_repos. The CLI already guards this with assertAnalysisFinalized; the worker did not. Extract the run -> finalize -> report contract into a side-effect-free analyze-worker-core seam (the entry module's top-level process.on handlers make it untestable directly) and call assertAnalysisFinalized before sending complete — a failure is reported as {type:'error'} instead of a false success. The seam is dependency-injected and unit-tested with fakes; the entry module wires the real deps and keeps owning process.exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): coordinate worker SIGTERM cancellation with completion (#2264 P3) The worker SIGTERM handler unconditionally sent {type:'error','Analysis cancelled'} and didn't coordinate with the message handler that sends complete, so a cancel near the finish line could report a cancelled job complete, or a late SIGTERM could flip an already-complete job to failed. Add a single terminal-outcome claim (createTerminalClaim) shared by the message handler and the SIGTERM handler: whoever claims it first reports its terminal message; the other skips its terminal send. Single-threaded JS makes the check-and-set atomic. The cleanup + process.exit still run regardless. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(server): make a job's terminal outcome immutable on the parent side (#2264 P3) Defense-in-depth complement to the worker terminal-claim: the launcher's message handler and the job manager's updateJob both lacked a terminal-state guard, so a late worker IPC message (a SIGTERM-driven 'error' after 'complete', or vice versa) could re-release the repo lock and flip the reported status. (Touches parent-side files outside the original PR diff — deliberate, clearly-scoped.) - analyze-job.ts updateJob: drop any update once the job is already terminal (the transition INTO terminal still applies, since status isn't terminal yet then). - analyze-launch.ts message handler: return early when the job is already terminal, mirroring its sibling exit handler. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(lbug): assert deleteAllInterprocTaintPaths + deleteAllCallSummaries route through withConnLock (#2264) The lock-routing suite covered 4 singleton-conn helpers but not these two withConnLock-wrapped delete helpers (lbug-adapter.ts), which also run during the incremental --pdg writeback window — so a revert of either wrapper would have gone uncaught. Add the two routing assertions to complete the coverage the file's header claims (every singleton-conn helper reachable during the WAL-driver window). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(lbug): assert temp-conn deleteNodesForFile skips withConnLock (negative gate, #2264) The positive case (singleton deleteNodesForFile locks each per-table count) was covered, but not the negative branch of the targetConn === conn gate: a per-file/temp connection (dbPath provided) must NOT take the singleton lock, or temp-conn callers would needlessly contend with it. Add the negative-gate assertion so a regression that unconditionally locks is caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * test(cli): assert the force-exit forwards process.exitCode, not a hardcoded 1 (#2264) The existing cases asserted process.exit(1), but since the error catch always sets exitCode=1 they couldn't distinguish forwarding (process.exit(process.exitCode ?? 1)) from a hardcoded 1. Add a case on the alreadyUpToDate path — which returns without setting exitCode or calling process.exit — with a pre-set exitCode=2 and isLbugReady forced true, asserting the wrapper force-exits with 2. Proves the exitCode-forwarding branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * refactor(lbug): replace skipNativeClose flag with a dedicated closeLbugBeforeExit() (#2264) The "skip the native close only when a process.exit is guaranteed to follow" invariant was enforced by convention across ~4 call sites via a boolean option on closeLbug — the exact foot-gun the review flagged. Encode the contract in the name instead: - New closeLbugBeforeExit() (CHECKPOINT via flushWAL, then return without the native close); closeLbug() drops the option and is the plain real-close again. - run-analyze success + error paths: options.skipNativeCloseOnExit ? closeLbugBeforeExit() : closeLbug(). CLI SIGINT + worker SIGTERM call closeLbugBeforeExit() directly. skipNativeCloseOnExit stays on AnalyzeOptions as the caller's "I will exit" signal. - lbug-checkpoint.test: assert closeLbugBeforeExit exists + has no native close, and match `closeLbug =` precisely so it doesn't prefix-match the new function. - Retarget the conn-serialization integration case to closeLbugBeforeExit(); add the new export to the 12 analyze-*.test.ts lbug-adapter mocks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * refactor(lbug): extract isSharedSingletonConn predicate for the lock gate (#2264) The targetConn === conn object-identity gate (decides whether an op takes withConnLock) was duplicated inline in queryAndDrain and deleteNodesForFile with its own explanatory comments. Extract a single isSharedSingletonConn(c) predicate with the rationale in one place; both sites route through it. Behavior unchanged — covered by the lock-routing tests' positive (singleton locks) and negative (temp-conn skips) cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * refactor(lbug): share the bounded checkpoint-then-exit cleanup (SIGINT/SIGTERM) (#2264) The CLI SIGINT handler (analyze.ts) and the worker SIGTERM handler (analyze-worker.ts) had near-identical Promise.race([closeLbugBeforeExit, timeout]).finally(exit) blocks with separately-hardcoded 2s timeouts. Extract boundedCheckpointBeforeExit into a shared shutdown-helpers module — parameterized by exit code, an optional flush-error reporter (worker reports over IPC), and an optional beforeExit hook (CLI flushes the logger). checkpoint + exit are injectable test seams, so it's unit-tested without the real LadybugDB close or process.exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * refactor(storage): extract registryPathEquals for the registry case-fold compare (#2264) The Windows case-insensitive / POSIX case-sensitive registry-path comparison was duplicated across 6 sites (registerRepo dedup, the fresh-merge findIndex, removeRepo/removeBranchIndex local 'matches' helpers, isRepoRegistered, and the path-match lookup). Extract a single registryPathEquals(a, b) predicate so every registry lookup/dedup/finalize check answers identically; route all 6 through it. No behavior change — repo-manager + finalize-invariant suites pass (incl. the Windows case-fold case). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * feat(lbug): runtime-guard streamQuery against the WAL-checkpoint driver (#2264) streamQuery is deliberately not wrapped in withConnLock (its per-row callback can re-enter the adapter), so its unlocked per-row reads could race a CHECKPOINT on the shared connection — the corruption window the lock serializes everything else against. That invariant was comment-only, safe today only because the serve/read path forks analyze workers. Make it enforced: - lbug-adapter: a walDriverActive flag + markWalDriverActive(bool); streamQuery throws an actionable error when the driver is active. - wal-checkpoint-driver: arm the flag on start, disarm in stop() AFTER the in-flight CHECKPOINT drains (clearing earlier would briefly allow a race). A future in-process analyze overlapping a stream now fails loud instead of corrupting native state. (reentrancy test's lbug-adapter mock gains the new export.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * docs(lbug): explain why closeLbugBeforeExit skips finalizeLbugSidecarsAfterClose (#2264) Document the deliberate trade-off: the skip-close path intentionally does NOT run the sidecar-finalize step that safeClose runs after a real close. It's designed for released WAL handles; running it with the connection still open risks a Windows file-lock on the in-use WAL. The CHECKPOINT already made the index durable and the next run's preflightLbugSidecars reconciles residual WAL — the deferral is the accepted cost of skipping the native close to dodge the destructor double-free. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm * fix(lbug): move WAL-driver-active flag to its own module (fix mock ripple, #2264) The streamQuery guard (4449bb4a) put markWalDriverActive in lbug-adapter, and the wal-checkpoint-driver imported it from there. That broke every test mocking lbug-adapter while loading the real driver — CI's ubuntu lane caught run-analyze-fts-repair.test.ts ('No markWalDriverActive export on the mock'). Move the one-bit shared flag to a dedicated wal-driver-state module: the driver toggles markWalDriverActive there, streamQuery reads isWalDriverActive there, and lbug-adapter no longer carries it — so mocking lbug-adapter no longer has to stub the toggle. run-analyze-fts-repair now passes untouched; the reentrancy test's mock addition is reverted (no longer needed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 15:25:56 +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
Goutham Krishna Mandati	4c73b18387	feat(mcp): add trace tool for shortest call path between symbols (#2173) ... * feat(mcp): add trace tool for shortest call path between symbols (#1821) Implement the \ race\ MCP tool and \gitnexus trace\ CLI command that finds the shortest directed call path between two symbols using BFS over CALLS + HAS_METHOD edges. - MCP tool definition in tools.ts with READ_ONLY annotations - Directed BFS in local-backend.ts with parent-map path reconstruction - Symbol resolution via resolveSymbolCandidates (name/UID/file-hint) - Gap reporting with furthest reachable node and depth tracking - CLI wiring: gitnexus trace <from> <to> [--from-uid] [--to-uid] [--depth] - i18n keys in en.ts and zh-CN.ts + help-i18n.ts registration - ARCHITECTURE.md tools table entry - 16 unit tests (11 BFS core + 5 CLI wiring) * test(mcp): account for trace tool in tools.test.ts count The trace tool makes GITNEXUS_TOOLS length 15; update the hardcoded count, add 'trace' to the expected-names list, and refresh the stale "13 tools" comment and it() title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): sanitize trace maxDepth to reject 0/NaN/negative `Math.min(params.maxDepth ?? 10, 30)` had no lower bound and `??` does not recover 0 or NaN, so `--depth 0|-5|abc` made the BFS loop run zero iterations and return a false `no_path`. Clamp at the real boundary with a `Number.isInteger && > 0` guard (the MCP inputSchema minimum is advisory only), and reject a non-numeric `--depth` in the CLI up front rather than forwarding NaN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): check trace target before applying test-file filter The `isTestFilePath` filter ran before the target-equality check, but resolveSymbolCandidates does not exclude test-file symbols. A target (or a required hop) that lives in a test file was therefore skipped under the default includeTests=false and produced a false no_path with a misleading dynamic-dispatch suggestion. Match the explicitly-requested target first; non-target test-file nodes are still filtered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(mcp): bound trace BFS with per-level LIMIT and visited cap The per-level query had no LIMIT and the visited set was uncapped, so a high-fanout hub could materialize an unbounded frontier. Cap per-level rows (interpolated LIMIT — Kuzu does not bind LIMIT) and the total visited set; either cap sets a `truncated` flag so a resulting no_path reports that the search was cut short rather than implying the graph was exhausted. Note: the sibling impact BFS shares the same unbounded pattern; applying the cap there is deferred (out of scope for this PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): clarify trace traverses call + class-member edges trace was advertised as a "shortest call path" but also traverses HAS_METHOD (class→member) containment edges so a class-rooted trace can descend into its methods. Keep that capability (consistent with impact/ context) and make the docs honest: rename EDGE_TYPES→TRAVERSAL_EDGE_TYPES, state the call + class-member traversal in the MCP/CLI/i18n/ARCHITECTURE descriptions, and note each hop's edge type is reported in edges[]. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): set status:'error' on trace failure responses Every trace return path sets a `status` discriminator except the caught-error path, so a consumer switching on `result.status` saw undefined on failure. Add status:'error' to both the backend trace() catch and the CLI traceCommand catch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): return a friendly error for non-string trace from/to A non-string from/to reaching resolveSymbolCandidates surfaced a low-level "x.includes is not a function" via name.includes. Guard the four name/uid params at the top of _traceImpl and return a structured status:'error' with a clear message instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): single row-decode + drop dead field in trace BFS Decode each BFS row once into named locals instead of repeating `(row.x ?? row[N])` across the two parent.set calls and the furthest-tracking. Drop the `type` field from the parent map value (it was written but never read), and rename the internal `deepestInfo` to `lastReached` for accuracy (the output field `furthest` is unchanged). Pure refactor — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): dedicated trace includeTests i18n key + guard coverage `trace|--include-tests` reused the impact help key, so rewording the impact option would silently change trace's help text. Add a dedicated help.option.trace.includeTests key in en + zh-CN and repoint it. Add CLI coverage for the (already symmetric) --from-uid/--to-uid flag-value guard and for --include-tests forwarding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): faithful BFS mock + expand trace coverage Fix makeResolveMock: concatenate neighbors across ALL frontier ids (it returned only the first node's, so a multi-node frontier was unmodelled) and key the UID branch on params.uid (the old query-text match never fired). Add coverage: shortest path through the second frontier node (proves the mock fix), confidence floor fallback, HAS_METHOD traversal with a mixed edge-type chain, no_path furthest:null, and from_file disambiguation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(trace): apply root prettier formatting to trace files The root `quality / format` gate (prettier --check, printWidth 100) runs on the full repo and flagged the trace sources/tests (the local config masks it). Reformat to root style — no behavior change; trace + tools suites and tsc stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): document the trace tool for AI agents Add `trace` to the GitNexus skill docs so agents reach for it instead of hand-chaining context/impact hops. The guide gains a Tools Reference row and a "shortest path between two symbols" subsection (params, result shape, status/furthest/truncated semantics); the debugging skill gains a "how does A reach B?" pattern row and a trace tool example. Mirrored to the .claude and claude-plugin copies (byte-identical) and the cursor copy (compact style). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): drop unused trace test fixtures (CodeQL js/unused-local-variable) CodeQL flagged two unused locals in the trace BFS tests: the top-level SYMBOL_C and a SYMBOL_D inside the maxDepth test (both defined, never referenced). Remove them. No behavior change — 58 trace/tools tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-14 10:43:01 +01:00
Gergő Magyar	7c3d4e6862	feat(pdg): control dependence — post-dominators + CDG (Ferrante) [M5 #2085] (#2188) ... * feat(pdg): add CDG + POST_DOMINATE edge types (M5 #2085) * feat(pdg): post-dominator tree on reverse CFG (M5 #2085) * feat(pdg): Ferrante control-dependence over the post-dom tree (M5 #2085) * feat(pdg): emitFileCdg + optional POST_DOMINATE debug edges (M5 #2085) * feat(pdg): wire CDG emission in-phase + pdgModeMismatch CDG-cap stamp (M5 #2085) * test(pdg): CDG snapshot + end-to-end pipeline answerability (M5 #2085) * fix(review): apply autofix feedback (M5 #2085) * fix(pdg): label CDG edges by controller arm sense, not edge kind (#2188 F1/F2/F4) Tri-review (with Codex as the independent engine) found the CDG 'T'/'F' label was wrong for the commonest control flow: the M1 TS visitor wires a condition's fall-through FALSE arm as `seq`/`loop-back`, but `branchSense` mapped both to 'T', so guard clauses, if-no-else, and loop `break` got 'T' instead of 'F' (F1, P1). The structural CDG edges were correct; only the label — the AC3 "under what condition does X run?" answer — was wrong. - F1: replace edge-kind `branchSense` with controller-arm-sense `labelFor`. An ambiguous fall-through edge (seq/loop-back) takes the COMPLEMENT of its source block's explicit cond-true/cond-false sibling arm. This correctly handles do/while (loop-back = TRUE arm) and inner-if-in-loop (loop-back = FALSE arm) — the ambiguity a kind→label table cannot resolve. Adds real-parser regression tests (the hand-built tests used a fictional cond-false edge and missed it). - F2: correct the false "sound over-approximation that never drops a real dependence" claim in post-dominators.ts — exit-unreachable regions both drop and invent control dependences (latent for the current TS visitor, which keeps EXIT reverse-reachable). Reframe the exit-less-loop test to characterize, not bless, the degenerate behavior. - F4: make the AC2 property-test reference compute post-dominance INDEPENDENTLY (node-removal reachability, no shared code with post-dominators.ts), so a post-dom direction bug can no longer pass both the impl and the reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): root-prettier format + run-analyze pdg stamp gains maxCdgEdgesPerFunction (#2085) Two deterministic CI failures from the M5 CDG work: - quality/format: basicblock-roundtrip.test.ts failed CI's root `prettier --check .` (the pre-commit hook uses the gitnexus-local prettier config, which differs); reformatted with the root config. - tests/ubuntu/coverage: run-analyze.test.ts pinned the resolved RepoMeta.pdg shape (DEFAULTS) and the all-zero cap override without the new maxCdgEdgesPerFunction key (default 5000); added it so resolvePdgConfig toEqual and pdgModeMismatch(DEFAULTS) pass. (The stale-test sweep missed this file in PR #2188 — same trap M2 hit.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(mcp): add pdg_query tool definition (controls/flows modes) [M6 #2086] * feat(mcp): pdg_query backend — controls (CDG) + flows (REACHING_DEF) + e2e test [M6 #2086] * feat(mcp): document PDG edges + pdg_query (schema, cypher, skill, --pdg-gated ai-context) [M6 #2086] * fix(mcp): correct pdg_query symbol-anchor lower bound + harden inputs [PR #2188 review] Tri-review (Codex + adversarial + correctness lanes) of the M6 pdg_query surface found the symbol-anchor window over-includes a neighbor function's block. The upper bound was widened to the 1-based BasicBlock basis (symEnd+1) but the lower bound was left 0-based, so a block on the line directly above the target function leaked into the result. Shift both bounds +1 ([symStart+1, symEnd+1]) so the window is the function's true block span. Also from the same review: - pdg_query no longer throws on a no-arguments MCP call: the dispatch passes raw `params`, so default it to {} → a clean mode-validation error instead of a TypeError. (`explain` shares this latent pattern — pre-existing follow-up.) - tools.ts: the controls-mode description no longer hard-codes the 'F' branch sense for guards — `if (!ok) return;` rides the predicate's 'T' arm; the guard:true flag is label-agnostic (regex on the dependent block text). Tests: a hand-seeded adjacency regression (verified failing without the lower-bound +1) + a no-arguments validation test. Skill doc updated to document the two-sided [symStart+1, symEnd+1] window. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): drop always-true anchor conditional in pdg_query [CodeQL #2188] CodeQL alert 756 flagged `...(anchor ? { anchor } : {})` in _pdgQueryImpl as a useless conditional: `anchor` is unconditionally assigned in both the file-path and symbol branches before the return (the not-found/ambiguous/no-layer paths return earlier), so it is always truthy. Drop `| undefined` from the declaration (TypeScript definite-assignment holds across both branches) and emit `anchor` directly. No runtime change — the `anchor` field was already present on every result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): add hasPdg to the noStats bridge expectation [#2188] The M6 work threaded `hasPdg: options.pdg === true` into the AIContextOptions passed to generateAIContextFiles on the --skills regeneration path, but this test's strict .toEqual expectation predated it (4 keys vs 3 → CI failure). Add `hasPdg: false` (the value on this non---pdg path). The assertion stays strict; the #1477 noStats bridging it guards is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): collapse generateGitNexusContent params to an options bag [#2188] The function had grown to 9 positional params; reaching `hasPdg` meant passing six `undefined`s (the M6 review's maintainability flag). Collapse params 3-9 (generatedSkills, groupNames, noStats, skipSkills, runnerPath, defaultBranch, hasPdg) into a `GitNexusContentOptions` object with the defaults moved to destructuring. The body is unchanged (same local names); the single production caller and the test calls become self-documenting named fields. Pure refactor — generated AGENTS.md/CLAUDE.md content is byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): skip CDG for exit-unreachable CFGs (unsound post-dominance) [#2188] M5 review P2: computePostDominators roots only at cfg.exitIndex and nothing enforced that EXIT is reachable from every block. For an entry-reachable region that cannot reach EXIT (a non-terminating loop, or a multi-terminal CFG a future visitor might emit) the EXIT-rooted reverse walk degenerates — it both drops real control dependences and invents spurious ones. Add a pure precondition predicate `isExitReachableFromAllBlocks` (co-located with the algorithm it guards) and gate it in emitFileCdg: a CFG that violates it is skipped for CDG (counted as skippedUnsoundFunctions + one onWarn), while its CFG and REACHING_DEF projections — which do not depend on post-dominance — are kept. A CDG-specific gate, not a widening of isEmitSafeCfg, so the blast radius is exactly the unsound CDG. The current TS visitor always satisfies the precondition (every loop gets a structural header→loopExit edge), so CDG output for real fixtures is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): bound computeControlDependence materialization (heap parity) [#2188] M5 review P2: unlike computeReachingDefs (maxFacts) and the emit-side edge cap, computeControlDependence materialized the full deduped seen/out before emitFileCdg's per-function cap could trim it — O(edges × post-dom depth) heap for a deeply nested function. Add a `maxEdges` ceiling (default 0 = unbounded) returning {edges, truncated}, mirroring computeReachingDefs's {facts, truncated}. The ceiling is checked before pushing a new unique edge, so `truncated` means a genuine overflow (not merely "reached cap"). emitFileCdg passes a FIXED materialization ceiling (8× the default edge cap) — deliberately NOT derived from the runtime edge cap, because CDG's materialization IS the deduped-edge quantity the cap reports on (deriving it would pre-truncate that set and lose the exact dropped count). A ceiling hit is surfaced via onWarn + the truncated flag — never silent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): share resolveBlockAnchor; fix explain's anchor off-by-one [#2188] M6 review P2 (duplication) + the flagged pre-existing _explainImpl correctness follow-up. _pdgQueryImpl and _explainImpl each carried a near-identical symbol↔block anchor resolver that had DRIFTED: pdg_query used the corrected [symStart+1, symEnd+1] window (BasicBlock startLine is 1-based, the symbol span 0-based) while _explainImpl still used [symStart, symEnd] — dropping a taint source on the function's final line AND leaking a neighbor's block on the line directly above. Extract one `resolveBlockAnchor` helper, used by both, that applies the correct window and a single (bare) clause convention (callers compose their own WHERE). This removes ~50 duplicated lines and fixes explain's anchor in one place. A hand-seeded characterization test (taint-explain Block 4) pins both bounds — verified to FAIL on the pre-fix window (it returned the line-10 neighbor instead of the line-15 final-line source). Existing taint-explain + pdg-query suites are unchanged (their fixtures have interior sources/sinks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): pdg_query reports "status unknown" when the layer can't be confirmed [#2188] M6 review P3 (Codex): when meta is UNREADABLE and the bounded global existence probe returns zero rows of the edge type, _pdgQueryImpl asserted "no PDG layer" — but a genuinely edge-free layer (all-linear functions) is indistinguishable from a missing one via that probe. Soften only that fallback path to an inconclusive "PDG layer status unknown — was this repo indexed with --pdg?" note. The meta-stamped path (stamp present, cap absent ⇒ layer truly missing) keeps the definitive "no PDG layer" wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): cover pdg_query ambiguous / pagination / Windows-path gaps [#2188] M6 review test-gap follow-ups, all hand-seeded with controlled data: - ambiguous symbol name → status:'ambiguous' + ranked candidates shape (uid/name/filePath/score), never a silent guess; - total/truncated page boundary in both directions (limit below the match count sets truncated with the full total; limit above it omits truncated); - a Windows-style filePath containing ':' resolves and fnLineOf decodes the function-line segment correctly (split-from-right past the drive letter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): ship gitnexus-pdg-query skill mirrors + add pdg_query to the guide [#2086] M6 bundled pdg_query into this PR, but the skill shipped only in the canonical gitnexus/skills/ root. Mirror it (byte-identical) to the two hand-maintained roots the sibling taint skill uses — .claude/skills/gitnexus/ and the plugin — so Claude Code + plugin users get it too. Also extend the gitnexus-guide tool reference (all 3 copies, now byte-identical): add a `pdg_query` row + a "Control & data dependence" section mirroring the taint/`explain` section, and reconcile the pre-existing drift where only the .claude copy carried the `check` tool row (a real registered tool) — all three now list it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): refresh CFG/PDG section for the full M1–M6 stack [#2086] The PR body had deferred the "ARCHITECTURE docs refresh" to #2086; now that M6 ships here, do it: - MCP tools table gains `explain` and `pdg_query` (were absent). - "Optional CFG/PDG emission" was M1-only; rewrite to cover the whole opt-in stack — M1 CFG, M2 REACHING_DEF, M3/M4 taint, M5 CDG (Ferrante over CHK post-dominators, with the exit-unreachable skip), M6 read surface (pdg_query + explain, anchored + LIMIT-bounded, shared resolveBlockAnchor) — and note the no-Function→BasicBlock-edge join. - LadybugDB schema notes the `--pdg` additions: the `BasicBlock` node table and the CFG/REACHING_DEF/CDG/TAINTED/SANITIZES/TAINT_PATH relation types, kept out of the default VALID_RELATION_TYPES / web schema. 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-13 18:49:03 +01:00
bluerose	89ffa71a52	feat(cli): add --embeddings-baseurl/-model/-auth-token/-dims flags to analyze (#2140) ... * feat(cli): add --embeddings-baseurl/-model/-auth-token/-dims flags to analyze Add four CLI flags to `gitnexus analyze` that configure a custom OpenAI-compatible HTTP embedding endpoint by setting the GITNEXUS_EMBEDDING_URL / _MODEL / _API_KEY / _DIMS env vars the HTTP embedding client already reads. Flags override env vars; env vars keep working as before. URLs are validated (http/https) and dims must be a positive integer. Prints "Using custom embedding endpoint: <url>" when a URL+model pair is configured, and warns when the flags are passed without --embeddings. The new env keys are added to the analyze snapshot/restore set so programmatic callers don't leak state. The non-secret flags are also accepted from .gitnexusrc; the auth token is intentionally CLI/env-only. * fix(analyze): set GITNEXUS_EMBEDDING_DIMS from CLI flags before module import schema.ts reads EMBEDDING_DIMS at module-load time via the static-import chain (analyze.ts -> run-analyze.ts -> schema.ts). The previous approach of setting the env var inside analyzeCommandImpl ran AFTER schema.ts had already loaded with the default 384, causing "Expected: 384, Actual: 4096" errors when using --embeddings-dims 4096. Fix: use Commander's preAction hook to set GITNEXUS_EMBEDDING_* env vars before the lazy import of analyze.ts triggers the schema.ts module load. * fix(analyze): use hook callback arg instead of this in preAction Commander v14 passes the command as first argument, not as this binding. * refactor(cli): rename --embeddings-* analyze flags to singular --embedding-* Aligns the custom embedding endpoint flags with the existing singular tuning flags (--embedding-threads/--embedding-device): --embedding-base-url, --embedding-model, --embedding-auth-token, --embedding-dims. Renames the derived AnalyzeOptions fields and the .gitnexusrc KEY_SPECS keys to match. Behavior-preserving; the GITNEXUS_EMBEDDING_* env vars are unchanged. Refs #2140 review. * fix(cli): validate and normalize --embedding-dims before module-load reads it The preAction hook wrote GITNEXUS_EMBEDDING_DIMS unvalidated, so an invalid value (abc/0/-5/0x10) threw from schema.ts during the lazy import — surfacing as a raw unhandled rejection on the synchronous program.parse path instead of a friendly error. And '1e3' slipped through: schema.ts parseInt froze the vector column at FLOAT[1] while the impl's Number-based check accepted 1000, so http-client requested 1000-dim vectors against a 1-dim column. Extract a dependency-free normalizeEmbeddingDims helper (strict /^\d+$/ + positive, trim-then-validate, canonicalized) shared by both the hook (CLI path, before module-load) and analyzeCommandImpl (direct-call path). All three readers — schema.ts, http-client, and this helper — now agree on one value, and invalid input gets a clean message instead of a crash or a silent mismatch. Refs #2140 review. * fix(cli): mask credentials in the custom embedding endpoint confirmation A base URL with userinfo (http://user:pass@host/v1) or a query token (?api_key=…) passed the new-URL + http/https validation and was printed verbatim in the 'Using custom embedding endpoint:' line, leaking the secret to terminal scrollback and CI logs. Route it through the existing safeUrl() (now exported from http-client) which strips userinfo + query, keeping protocol/host/path. Single source of truth — no second sanitizer. Refs #2140 review. * fix(cli): drop the ineffective embeddingDims .gitnexusrc key embeddingDims as a .gitnexusrc key silently did nothing: .gitnexusrc loads in analyzeCommandImpl, AFTER the lazy import already ran schema.ts's module-load read of GITNEXUS_EMBEDDING_DIMS, so a config value never sized the vector column. Remove it (config now fails closed on the key, like the auth token); URL/MODEL stay as config keys because they're read lazily at runtime. Dims remains available via --embedding-dims or GITNEXUS_EMBEDDING_DIMS. Refs #2140 review. * refactor(cli): narrow the analyze preAction hook to GITNEXUS_EMBEDDING_DIMS Only DIMS is read at module-load (schema.ts), so only it must be set before the lazy import. URL/MODEL/API_KEY are read lazily at runtime, so analyzeCommandImpl is their sole setter — and because the impl's env snapshot is taken AFTER this hook ran, leaving those three in the hook leaked them past restore. Drop them from the hook (the impl already sets+restores them), and capture/restore the pre-hook DIMS baseline via a postAction hook so a CLI --embedding-dims override no longer leaks into a later in-process program.parseAsync. Refs #2140 review. * fix(cli): gate the custom-endpoint confirmation on the embedding flags The confirmation collapsed into one if/else chain that emits at most one message reflecting the run's intent. Gating on embeddingsEnabled stops the 'Using custom embedding endpoint' line from printing on every analyze run when GITNEXUS_EMBEDDING_URL+MODEL merely happen to be set in the environment, and ordering the '--embeddings absent' note first removes the contradiction where it printed alongside 'Using custom embedding endpoint'. Refs #2140 review. * test(cli): cover the custom embedding endpoint flags Adds direct-call (analyzeCommandImpl path) coverage the original PR lacked: URL validation (empty/invalid/non-http), model/token emptiness, dims validation incl. the 1e3 regression, credential masking in the confirmation line, confirmation gating (absent --embeddings; ambient env must not trigger it), CLI-over-env precedence, and the GITNEXUS_EMBEDDING_* snapshot/restore round-trip. Complements embedding-dims.test.ts and http-client-safe-url.test.ts. Refs #2140 review. * test(cli): e2e-cover the --embedding-dims crash path on the real CLI The dims-validation fix lives in the commander preAction hook, which only fires on the program.parse path; the direct analyzeCommand() unit tests bypass it. Add a subprocess e2e (run via tsx, no build) asserting that invalid --embedding-dims (abc/0/-5/1e3/3.5) produces the friendly flag-named error and exit 1 — NOT the raw schema.ts module-load throw that the original bug surfaced. Cases exit inside the hook (no repo/import/pipeline), so they're deterministic and fast. Updates the unit-suite comment to point at it. Refs #2140 review. --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>	2026-06-13 14:01:12 +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
bluerose	cab63b508e	feat(mcp): add gitnexus mcp --http server with Streamable HTTP and legacy SSE transports (#2141)	2026-06-13 09:58:49 +01:00
azizur100389	50cda61a43	feat(setup): select coding agent integrations (#2168) ... * feat(setup): select coding agent integrations * style: format setup agent selection * fix(setup): validate explicit agent selection --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-06-13 08:06:50 +01:00
Gergő Magyar	14397dd4aa	feat(taint): intra-procedural taint analysis (#2083) (#2164) ... * feat(taint): harvest occurrence-tagged call/member sites on StatementFacts (#2083 U1) Worker-side site harvest in TsHarvester: call/new/member-read records with dotted callee paths, receiver slots, per-argument occurrence tagging with nested-site links, per-declarator resultDefs, spread/template/require-literal markers. hasTaintSafeSites validation seam. The pdg parse-cache chunk-key namespace is versioned (pdg:1 -> pdg:2) instead of a global SCHEMA_BUMP so flag-off users keep warm caches; bench fingerprints re-baselined for the three call-bearing scenarios (straight-line/dense-bindings byte-unchanged). * feat(taint): built-in TS/JS source/sink/sanitizer model + site matcher (#2083 U2) Typed spec (kind taxonomy; sanitizers carry neutralizes-kinds), the canonical Express/Node model, and matchFunctionSites: ESM alias/namespace + require- literal callee resolution, bare-name fallback restricted to true globals, sanitizers module-or-global only (never user-shadowable by name), spread/ template arg-position rules, deterministic taintModelVersion. * feat(taint): pure intra-procedural taint propagation engine (#2083 U3) Two-rule model (statement-local + du-fact worklist) with per-taint neutralized-kind exclusion sets: sanitizers exclude only the sink kinds they neutralize (escape(req.body) suppresses res.send but still fires db.query; exec(path.basename(t)) fires), intersection-over-paths so a bypass occurrence keeps the taint live, kill locality on resultDefs, propagate-through args+receiver with viaCall hops, one path per finding, deterministic caps, coverage-gap statuses. Test-first: 38 scenarios on real harvested CFGs. * feat(taint): thread taint caps + model version through pdg config/meta (#2083 U5) resolvePdgConfig gains maxTaintFindingsPerFunction (200), maxTaintHops (32), and the taintModelVersion digest; RepoMeta.pdg + RunScopeResolutionInput surfaces added. The key-union comparator trips full writeback on M2->M3 upgrade and on model-version change without --force (mode-flip tested). No CLI flags or rc keys (programmatic parity with the other caps). * feat(taint): in-phase taint emit with sparse TAINTED/SANITIZES edges (#2083 U4) run.ts pdg window: match-first fast path (solver only when a function has both a matched source and sink) -> computeReachingDefs with the shared RD fact derivation -> computeTaintFlows -> per-finding TAINTED (versioned hop-encoded reason via the shared path codec, statement-level occurrence identity) + per-kill SANITIZES, dedup-before-budget, truncate-and-warn. All emit counters surfaced (aggregate warn for gaps/drops, debug for volume); PROF gains taint=. Flag-off golden untouched. * feat(mcp): explain tool for persisted taint findings (#2083 U6) Anchorless calls enumerate the sparse TAINTED table (bounded, deterministic, limit-clamped); anchored calls (file or symbol via resolveSymbolCandidates) return full decoded hop detail. sinkKind rides a version-1 codec header (1;<kind>|hops — no other persisted channel exists; U4/U6 ship together). RepoMeta.pdg probe yields a no-taint-layer note instead of an error. TAINTED/SANITIZES pinned OUT of VALID_RELATION_TYPES (KTD9a negative- membership tests); generators + canonical skill docs + mirrors updated. * test(taint): acceptance fixture battery, snapshots, and bench gates (#2083 U7) pdg-repo taint-cases fixtures complete the six plan shapes; committed findings/kills snapshot via a shared pure-path harness that also feeds the AE2 exact-equality assertion (stored TAINTED == pure-path findings, the no-explosion gate). New taint-dense bench scenario with four --check gates: per-function findings pinned AT the cap, absolute reason-byte + site-bytes disk ceilings (the load-bearing R10 gate), zero-match pass < 0.5x match- dense, N-linearity. Pre-existing scenario baselines untouched. * refactor(taint): share one pointKey helper across propagate + emit (#2083 review) Extract pointKey(ProgramPoint) to cfg/reaching-defs.ts (colon-separated, matching the codebase block:stmt id convention) and import it in both propagate.ts and emit.ts, replacing the two divergent locals (':' vs '.'). Edge-id material now uses the colon form; ids are in-memory only and no test asserts the pointKey segment shape. * fix(taint): discriminate taint state by source occurrence (#2083 review) Two distinct sources flowing into one variable at one def point no longer collapse to a single TAINTED edge: the taint-state key gains a root source-occurrence discriminator ({point, siteIndex} — the same fields recordFinding's identity uses, excluding kind). Def->use fact lookup keys on the source-independent (binding, def-point) portion. Same-source multi-path flows still share one state so their exclusion sets intersect (the raw arm soundly wins); termination holds (finite keys, monotone shrink, no cross-source ping-pong). Restores the KTD6 identity contract. * fix(mcp): route dotted symbol names in explain to symbol resolution (#2083 review) The fileish classifier matched any dotted name (UserController.create) as a file via its extension-like suffix, so symbol resolution never ran and the tool returned a silent empty file-anchored result. Tighten the classifier to require a path separator or a real source extension (derived from the resolver's EXTENSIONS list, multi-language), so dotted/bare names route to resolveSymbolCandidates (found / ambiguous / not-found). * fix(mcp): gate explain no-taint-layer note on taintModelVersion (#2083 review) An M1/M2-era --pdg index has meta.pdg defined (BasicBlock/REACHING_DEF recorded) but no taintModelVersion and zero TAINTED rows. The probe keyed on generic meta.pdg presence, so explain returned the generic empty note instead of the actionable 'no taint layer — run analyze' hint. Gate on meta.pdg?.taintModelVersion (the field M3 stamps) so an M2-era index gets the layer hint; a taint-stamped index with no findings still gets the generic note. * fix(taint): sequence-expression value flows only the final operand (#2083 review) A comma expression in value position (exec((log(x), 'safe'))) default- descended, fanning every operand's occurrences into the enclosing sink argument — over-tainting exec's arg 0 with x. Add an explicit walkValue case that records earlier operands' uses with occurrence fan-out suppressed (new FactAccumulator.suppressOccurrences) and routes only the last operand through the value path. Sites-layer only; defs/uses/mayDefs byte-identical (cfg + reaching-defs snapshots unchanged). * perf(taint): FIFO head-cursor worklist + dedup before chainHops (#2083 review) Replace queue.shift() (O(N) dequeue) with a strict-FIFO head cursor plus order-preserving prefix reclamation; FIFO is load-bearing because chainHops reads the live taints map whose parent/source/viaCall are rewritten order-sensitively on monotone shrink, so hop determinism is dequeue-order contingent. Extract findingKey() and dedup-check before chainHops in the justify branch — already-recorded identities discard their hop chain (first write wins), so the ancestry walk was pure waste. The else kill branch is untouched. Findings + hops byte-identical (snapshot unchanged). * perf(taint): O(1) member-read dedup via composite-key set (#2083 review) addMemberRead rescanned the whole per-statement sites array per call to dedup by (object, property, parent) — O(n^2) on member-read-dense statements. Track a composite-key Set alongside sites for O(1) dedup. (The require-literal join is already O(sites) with a no-op body on non-require sites, so no early-exit is needed there.) Behavior identical: harvest + model-match + taint snapshots unchanged. * refactor(taint): drop test-only export; source taint caps via emit.ts (#2083 review) Remove the sanitizerNeutralizes export (its only consumers were two test assertions — inlined to entry.neutralizes membership). Re-export the DEFAULT_PDG_MAX_TAINT_* caps from emit.ts and point run.ts at emit.ts, so the pipeline's taint dependency surface is the single orchestration module rather than reaching into propagate.ts. * test(taint): extract the shared TS CFG/taint test harness (#2083 review) The parse/collectFunctions/cfgOf/cfgsOf/importsFor harness was copied byte-for-byte across four suites (harvest, model-match, propagate, taint-emit). Promote it to test/helpers/ts-cfg-harness.ts and import it. site-safety/reaching-defs carry a structurally different inlined builder and are left as-is. Pure extraction, no assertion changes. * test(mcp): harden explain limit-rejection battery (#2083 review) Add NaN, Infinity, -Infinity, and a numeric string to the out-of-bounds limit cases — a regression fence over the interpolated LIMIT, confirming the Number.isInteger guard rejects every non-integer/non-finite/string input before it reaches the query.	2026-06-12 07:35:09 +01:00
azizur100389	bdb824cfe4	feat(cli): add circular import cycle check (#2166)	2026-06-12 04:53:17 +01:00
Gergő Magyar	5bf8a17cd5	feat(ingestion): add control-flow-graph layer for TS/JS (#2081) (#2099) ... * feat(cfg): language-agnostic CFG construction core (#2081) U1 of M1 (CFG layer). Plain JSON-serializable CFG data model (BasicBlockData/ CfgEdgeData/FunctionCfg — must survive the worker→main boundary + ParsedFile store), a CfgBuilder accumulator (leaders→blocks→edges, synthetic ENTRY/EXIT, idempotent edges), a ControlFlowContext (break/continue/switch + labeled-jump target stacks), and a TraversalResult ({entry, dangling exits}). AST-agnostic and unit-tested on the classic control-flow topologies (if/else, while back-edge, mid-block return, labeled break/continue) the S2 spike validated; reachability helper backs the R9 property test. * feat(ingestion): U2 — TS/JS CFG visitor over tree-sitter AST (#2081) Add the TS/JS CfgVisitor that walks a function's tree-sitter AST and drives the U1 CfgBuilder to produce a serializable FunctionCfg. One visitor covers both languages (shared grammar family). Handles the classic CFG hazards explicitly (R2, R10): - loops allocate a dedicated loop-exit block so `break` has a concrete target before the loop's successor is known; `continue`/back-edge close the loop (while, do-while, C-for with init-once + increment-as-continue-target, for-in, for-of) - switch fallthrough falls out naturally: a non-breaking case yields exits we wire to the next case as `fallthrough`; a breaking case wires to the switch exit via ControlFlowContext - try/catch/finally: normal completion AND exceptional flow both route through finally (post-domination); a conservative exceptional edge models that the protected region may raise to its handler (not just explicit `throw`) - labeled break/continue resolve against the labeled loop's frame - early return/throw wire to EXIT/handler and terminate their block 19 hazard tests (one per construct) + AC1 10-function fixture; all green. No change to the committed U1 core or ControlFlowContext. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ingestion): U3 — worker CFG build + cfgSideChannel + cache coherence (#2081) Run the CFG visitor in the parse worker (where the AST lives), serialize the per-function CFG onto a new ParsedFile.cfgSideChannel, and keep it coherent across the disk-backed store and the warm/durable parse cache (R3, R4). - gitnexus-shared parsed-file.ts: add `cfgSideChannel?: unknown` as a DISTINCT field from captureSideChannel (different producer/consumer/lifecycle; plain JSON data — blocks/edges deliberately lack the `nodeId` the store's interning reviver keys on, so no mis-interning). - cfg/types.ts + visitors/typescript.ts: add CfgVisitor.isFunction so the worker enumerates functions (and applies the line budget) by a cheap node-type test. - cfg/collect.ts (new): collectFunctionCfgs walks the tree, builds one CFG per function (nested included), applies maxFunctionLines (over-cap = skipped). - language-provider.ts: add `cfgVisitor?: CfgVisitor<SyntaxNode>` hook; typescript.ts attaches it to both the TS and JS providers (shared grammar). - parse-worker.ts: read pdg + pdgMaxFunctionLines from workerData (read once at init — the worker never sees PipelineOptions), gate the build, attach cfgSideChannel alongside captureSideChannel. - parse-cache.ts: bump SCHEMA_BUMP 4→5 (ParsedFile shape changed) and fold the pdg flag into computeChunkHash so a pdg-off cached chunk is NOT reused on a --pdg run (the #2038-class warm-cache trap). Default path keeps its keys. - worker-pool.ts + parse-impl.ts + pipeline.ts: thread pdg/pdgMaxFunctionLines PipelineOptions → WorkerPoolOptions → workerData, and into the chunk-hash key. 9 boundary tests: collect contract, JSON round-trip identity (no AST leakage), the pdg cache-key guard, the line-cap skip, and the no-visitor gate. Full CFG suite (U1+U2+U3) green; build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ingestion): U4 — emit BasicBlock + CFG within scope-resolution (#2081) Emit persisted BasicBlock nodes + CFG edges from each ParsedFile's worker-built cfgSideChannel, INSIDE scope-resolution's Phase-4 graph emission — the last point where the worker-built CFGs are loaded (emitParsedFiles carries the channel; the disk store is cleared right after the orchestrator returns). This is the architecture the doc-review corrected to: a standalone post-`mro` phase (the issue's literal subtask) provably reads empty data (KTD1). - cfg/emit.ts (new): pure emitFileCfgs(graph, cfgs, maxEdgesPerFunction, onWarn). BasicBlock id = `BasicBlock:<filePath>:<functionStartLine>:<blockIndex>` (KTD3 — funcStart disambiguates blocks across functions in one file; no `name` column). CFG edge = CodeRelation type 'CFG' with the edge KIND (seq/cond-true/…) in `reason` (kinds can't be their own edge type). Per- function edge cap stops at the cap and warns with the dropped count — no silent truncation (R6/KTD6). - run.ts: pdg-gated emit pass over emitParsedFiles after emitPostResolutionEdges (store still live); RunScopeResolutionInput gains pdg + pdgMaxEdgesPerFunction. - phase.ts: thread ctx.options.pdg / pdgMaxEdgesPerFunction into the call. - pipeline.ts: PipelineOptions.pdgMaxEdgesPerFunction. 6 tests: node/edge shape (KTD3 id, no name, type='CFG', kind in reason), cross-function id uniqueness, AC2 reachability-from-ENTRY property, the edge cap's no-silent-truncation contract, and empty-input no-op. Flag-off byte-identity + full runPipelineFromRepo round-trip land in U7. Build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): U5 — `--pdg` opt-in plumbing (CLI + .gitnexusrc → both sinks) (#2081) Expose the CFG/PDG substrate as an opt-in and thread it from CLI/.gitnexusrc to the single source of truth (PipelineOptions.pdg), which fans out to BOTH sinks already wired in U3/U4: the worker build gate (workerData.pdg) and the scope-resolution emit gate. Off by default (R7). - cli/index.ts: `--pdg` commander flag. - cli/analyze.ts: AnalyzeOptions.pdg + pass `pdg` into runFullAnalysis options. - cli/analyze-config.ts: KEY_SPECS `pdg` (boolean) so `.gitnexusrc { "pdg": true }` normalizes and a non-boolean value fails closed with GitNexusRcError. - core/run-analyze.ts: AnalyzeOptions.pdg → runPipelineFromRepo({ pdg }). (The internal PipelineOptions/WorkerPoolOptions/workerData fields + the parse-cache key fold landed in U3/U4; this unit adds the user-facing surface. The budget knobs stay at internal defaults for M1.) Tests: analyze-config pdg normalization + non-boolean rejection; opt-in.test.ts covers the CLI/file merge precedence and that pdg perturbs the chunk-dispatch key. The full worker-build + main-emit round-trip is the U7 integration test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ingestion): U7 — CFG acceptance fixtures, parity, end-to-end + docs (#2081) Acceptance criteria for the M1 CFG layer: - AC1: a 10-function TS fixture's CFG node/edge set matches a committed snapshot (cfg-snapshot.test.ts). - AC2: every BasicBlock is reachable from its function ENTRY (property test over the emitted graph; the fixture has no dead code). - AC3: hazard fixtures lock the classic-bug coverage — try/throw/finally post-domination + labeled break/continue resolution. - AC4: the existing pipeline-graph-golden test stays byte-identical with --pdg off (verified; no UPDATE_GOLDEN), proving the opt-in adds zero default-run drift. - End-to-end (pipeline-pdg.test.ts): runPipelineFromRepo({ pdg: true }) on a tiny repo emits BasicBlock nodes + CFG edges with both endpoints present — the true both-sinks proof (worker builds → store → scope-resolution emits); the default run emits zero. Docs: CHANGELOG M1 entry, ARCHITECTURE "Optional CFG/PDG emission" subsection (why emit is in-phase, not post-mro), README CFG language-support note. Full CFG suite (U1–U7): 56 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ingestion): drop unused helper in cfg-snapshot test (#2081) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): apply ce-code-review autofix feedback (#2081) Review (10 reviewers) confirmed OFF-path byte-identity (adversarial + golden) and found defects all within the --pdg path. Fixes: - P1 same-line BasicBlock id collision: add a start-column disambiguator to FunctionCfg + the id (`BasicBlock:<file>:<line>:<col>:<idx>`) so two functions sharing a start line no longer collide under first-writer-wins addNode. - P1 worker crash-cascade: per-file try/catch around collectFunctionCfgs so a CFG-build throw cannot escape to the language-group catch and silently drop every remaining file in the group. - P2 edge-cap drop now logs unconditionally (input.onWarn is validator-gated/ silent in prod) — upholds the no-silent-truncation guarantee. - P2 Array.isArray guard before the cfgSideChannel cast in run.ts. - P2 maxFunctionLines default: worker applies DEFAULT_PDG_MAX_FUNCTION_LINES=2000 when unset; caps forwarded through run-analyze AnalyzeOptions (closes the server-path drop). - P3 README duplicate paragraph removed; `0`-vs-default docstrings corrected; CLI --pdg flag made language-neutral; reachableBlocks JSDoc corrected. - Documented the break-through-finally + stacked-label CFG limitations. - Tests: same-line id-collision regression, standalone throw→EXIT, dead-code- after-return, async/generator/method coverage, strengthened labeled-continue. Refuted: the HTTP-500 getNodeQuery finding — M0 already shipped the BasicBlock branch + name-floor (R12/web-safety handled). CFG + analyze-config suites: 95 tests green; golden parity (AC4) byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(ingestion): benchmark CFG construction + O(n) block-text accumulation (#2081) Closes the M1 review's requires_verification perf gap ("no benchmark for collectFunctionCfgs; a wall-time + cfgSideChannel byte-size regression gate would catch the extendBlock concatenation before kernel scale"). - bench/cfg/measure.mjs (new): build-free tsx harness timing collectFunctionCfgs (parse once, reuse the tree) across three scaling scenarios — straight-line (extendBlock path), many-functions (collect walk), branchy (block/edge growth) — at 500→2000. Reports a wall-time scaling ratio AND a cfgSideChannel byte-size ratio, plus an order-independent sha256 over the emitted blocks/edges as the behavior gate. `--check` compares both ratios + the fingerprint against bench/cfg/baselines.json; mirrors the scope-capture / python-scope harnesses. - .github/workflows/ci-tests.yml: run the gate on every test job (build-free, alongside the existing scope-capture guards) so an O(n^2) re-regression fails CI. - cfg-builder.ts: structural fix for the one real hotspot the bench surfaced — accumulate basic-block text as fragments joined once in finish(), instead of concatenating onto a growing string per coalesced statement (O(n^2) → O(n)). Behavior-identical (the CFG fingerprint + the AC1 snapshot are unchanged). Measured (post-fix): time ratios straight-line ~1.3, many-functions ~1.0, branchy ~1.1 (all sub-quadratic; a true O(n^2) would be ~4.0). cfgSideChannel bytes scale linearly (~1.0-1.04). 60 CFG tests green; build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(ingestion): add memory + disk growth gates to the CFG benchmark (#2081) Extend bench/cfg/measure.mjs beyond wall-time to the two other scalability dimensions that matter at kernel scale: - DISK growth: utf8 byte size of the serialized cfgSideChannel — exactly what a --pdg run writes onto every ParsedFile shard (durable store + parse cache). - MEMORY growth: retained JS heap of the cfgSideChannel payload, measured by the release-delta method (heap held minus heap after dropping it) — robust to pre-existing garbage and dead-stable run-to-run. Needs `node --expose-gc`; without it the heap metric is null and its gate is skipped (local runs still work). ci-tests.yml now passes --expose-gc so the heap gate runs in CI. Both gated on linear scaling in baselines.json (disk_bytes_budget / heap_budget 1.2-1.3). Measured: disk ~1.0-1.04, retained heap ~0.87-1.0 — both linear (~1KB/function each; ~2MB heap / 1.6MB disk at 2000 functions, --pdg only). Bumped REPS 7->15 to stabilize the noisier time signal and widened the coarse time tripwire budgets (the disk/heap gates carry the tight regression detection). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ingestion): address tri-review + CFG-expert findings (#2081) Corroborated findings from the tri-review (Codex + CE personas + GitNexus swarm + a CFG/program-analysis domain-expert lane). The OFF-path stays byte-identical; all fixes are within the --pdg path or the benchmark. - [Codex+CFG-expert] Exceptional `throw` edges now wire EVERY block in a try's protected region to the handler, not just the body ENTRY. A branched try body (`try { if (x) { use(t); } } catch`) previously left interior blocks with no path to `catch` — a taint false-negative into the handler for the M2 PDG pass. - [Codex+CFG-expert] An unresolved labeled jump (a stacked outer label or a labeled non-loop block) now routes to the function EXIT instead of leaving a dangling sink — restores the single-exit invariant post-dominator/PDG computation needs. - [Codex] computeChunkHash now folds pdgMaxFunctionLines/pdgMaxEdgesPerFunction into the chunk key (not just the pdg boolean), so a warm cache built under one cap is never served to a run with a different cap (#2038 class, extended to the budgets). Adds PdgCacheKey; boolean form kept for back-compat. - [perf] visitTry resolves catch/finally in a single namedChild pass (the double `namedChildren.find` allocated two throwaway arrays). - [adversarial] The bench `straight-line` scenario now runs at 2000->8000: output is a constant 4 blocks so disk/heap can't see the concat path, and at the old N a genuine O(n²) was masked by V8 cons-strings. Verified at the new N: the array-join impl ~1.0, a rope-optimized `+=` ~1.0 (correctly not flagged), a real O(n²) (re-join-every-append) ~3.8 — budget tightened 2.0->1.5. - [adversarial+Codex] The bench `--check` now FAILS LOUDLY when run without `--expose-gc` instead of silently skipping the retained-heap gate. - Doc: re-labeled the finally-bypass as a SOUNDNESS (false-negative) limitation tracked for M2, not mere "precision." 3 new regression tests (branched-try interior→handler, stacked-label→EXIT, cap-fold key). 99 CFG tests pass; build clean; bench gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(parse-cache): clarify that SCHEMA_BUMP still invalidates caches once (#2099 F6) The computeChunkHash comment claimed pdg-off warm caches "survive this change untouched" — true for the key FORMAT, but misleading as an upgrade-behavior promise: SCHEMA_BUMP 4→5 changes PARSE_CACHE_VERSION and both stores hard-invalidate on it. Separate the two facts so the next cache change isn't reasoned about from a false premise. Review finding F6 (P3) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): correct for-loop back-edge kinds when no increment clause (#2099 F5) A for with a body but no increment emitted an unconditional header→header 'loop-back' self-edge (a path that never executes the body) while the real back-edge body→header was labeled 'seq'. Any consumer identifying loops via reason='loop-back' picked the phantom edge and excluded the body from the natural loop. Gate the self-edge on the body being absent (the one case where the header genuinely re-tests itself) and carry 'loop-back' on the body's exits when they ARE the back-edge, matching visitWhile/visitForIn. Review finding F5 (P3) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): treat an empty catch clause as a real handler (#2099 F2) visitTry keyed handler semantics off the traversal result — null for an empty body, since visitSeq([]) returns null — instead of the syntactic clause. An empty `catch {}` was therefore treated as NO catch: the swallowed exception escaped to the outer handler/EXIT, the no-catch re-propagation misfired past finally, and code after a try whose body always throws became unreachable from ENTRY — a hard false-negative source for the M2 taint pass, on an extremely common pattern. Synthesize one empty block spanning the clause (entry == sole exit) when the catch body traverses to null, before the protected region is walked. Exception flow lands in it and rejoins the normal continuation; all downstream wiring (handler selection, finally routing, the !catchRes re-propagation gate) operates on the syntactically-correct shape. Review finding F2 (P2, reproduced) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): guard CFG emission per element, not just per outer array (#2099 F4) The cfgSideChannel guard checked only Array.isArray before casting to FunctionCfg[] — its own comment promised a wrong-shape value would 'skip emission, not throw a TypeError mid-graph-build', but a malformed ELEMENT sailed through. Worse, the obvious-looking failure shape never throws at all: emitFileCfgs string-templates any edge endpoint into the BasicBlock id and graph inserts are no-throw, so a non-integer endpoint silently became a dangling 'BasicBlock:…:undefined' edge that degrades the DB rel-pair COPY to row-by-row fallback inserts much later. Layered fix matching house precedents (parsedfile-store reviver, worker-side per-file catch): a per-element shape+content predicate (arrays + integer edge endpoints) that warns and skips malformed elements while valid siblings still emit, plus a per-file try/catch backstop for shapes that genuinely throw (e.g. a null inside blocks). Review finding F4 (P3) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(parse-cache): drop emit-time edge cap from the pdg chunk key (#2099 F3) pdgMaxEdgesPerFunction is applied exclusively in emitFileCfgs during scope-resolution on the main thread — the worker never receives it (workerData carries only pdg + pdgMaxFunctionLines), so the cached worker output is byte-identical across cap values. Folding it into the chunk key (added by a prior review round) only converted a free knob into a repo-sized cost: every cap change forced a full re-parse and a durable-store rewrite of unchanged data. Keep pdg + maxFunctionLines (genuinely worker-visible, shape the cached cfgSideChannel) and document the classification test in the PdgCacheKey doc comment so the next option gets sorted deliberately: worker-shard inputs go in this key; persisted-graph-only inputs belong in the RepoMeta pdg stamp (F1). Chunks written under the old ns string miss once and prune — no migration needed. Review finding F3 (P2) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): record pdg config in RepoMeta; force full writeback on mode flip (#2099 F1) Running --pdg against an already-indexed repo silently persisted ~zero CFG: incremental eligibility had no pdg term, RepoMeta recorded no mode, and extractChangedSubgraph keeps only changed-file nodes — on a no-change --pdg re-run every freshly built BasicBlock was dropped from the written subgraph ('Incremental: changed=0', run succeeds, zero rows). The converse flip left zombie mixed-coverage blocks only --force could clean. Worse, a clean-tree flip hit the alreadyUpToDate fast path and never ran the pipeline at all. - RepoMeta gains an additive-optional pdg stamp ({maxFunctionLines, maxEdgesPerFunction}, resolved values; absent ≡ pdg-off, which covers every legacy meta). No INCREMENTAL_SCHEMA_VERSION bump — that would force a one-time full rebuild for everyone. The end-of-run meta is a fresh literal, so omitting the field on a pdg-off run is what clears the stamp after an on→off flip. - pdgModeMismatch (pure, exported) compares the resolved triple; the flip check sits before the fast path and always logs its notice (not gated on options.force — --skills implies force with no message of its own), naming the .gitnexusrc pdg key that pins the mode. - The full-rebuild branch now writes the incrementalInProgress dirty flag (toWriteCount: 0 sentinel) before the wipe whenever a prior meta exists, mirroring the incremental branch. This closes the crash window where a rebuild dying between the bulk load and saveMeta left meta/DB inconsistent and the fast path certified zombie (or missing) CFG rows indefinitely — and incidentally closes the same pre-existing hole for user --force runs. Recovery log reworded accordingly. Tests: pdg-mode-flip.test.ts (real git + LadybugDB; primary assertion is a direct BasicBlock table count — meta.stats aggregates nondeterministic Community/Process rows) covering off→on, steady-state fast path, on→off zombie cleanup, cap-change rebuild, and dirty-flag + flip composition; pure-helper tests for default resolution and the 0=unlimited carve-out. Review finding F1 (P1) of PR #2099 tri-review. 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-10 19:26:45 +01:00
Gergő Magyar	2870aa6248	fix(grammars): load vendored tree-sitter grammars from vendor/ by absolute path (#2111) (#2144) ... * fix(grammars): load vendored tree-sitter grammars from vendor/ by absolute path (#2111) The recurring Windows `EPERM: operation not permitted, symlink` (errno -4048) when adding the MCP server to Antigravity is NOT the #2101/#2110 module-load crash — it is an install-time arborist failure during the `_npx` reify that the MCP client triggers on every `npx gitnexus` launch. Root cause: the `postinstall` materialize step copied each vendored grammar (`vendor/tree-sitter-{c,dart,proto,swift,kotlin}`) into `node_modules/gitnexus/node_modules/tree-sitter-*` as a real package so runtime `require('tree-sitter-dart')` would resolve. Those packages are in no dependency graph, so every subsequent npm/npx reify treats them as **extraneous** and prunes/relocates them — on Windows the relocation goes through `@npmcli/move-file`'s symlink path and throws EPERM (symlinks need Developer Mode/admin), and on every OS the 2nd run silently deletes the grammars. This is the same class as #1728, which the materialize step itself claimed to have fixed. Fix (the prebuildify + node-gyp-build ecosystem pattern): never copy grammars into node_modules. Load each by absolute path from `vendor/<name>` via the new `requireVendoredGrammar` helper — the grammar's own `bindings/node` runs `node-gyp-build(<dir>)` and loads the committed `vendor/<name>/prebuilds/ <platform>-<arch>/…` directly (all 5 ship all 6 tuples). vendor/ is inside the package but not a node_modules subtree, so arborist never sees the grammars and the reify is idempotent — no EPERM, no silent deletion. - new src/core/tree-sitter/vendored-grammars.ts (requireVendoredGrammar / vendoredGrammarDir / VENDORED_GRAMMAR_PACKAGES; VENDOR_ROOT stable in dev+dist) - route all consumers through it: parser-loader, parse-worker, grpc proto, include-extractor (C), http-patterns kotlin, cli optional-grammars probe - postinstall drops the materialize step; build-tree-sitter-grammars.cjs builds in-place under vendor/ (gitignored) and deletes materialize-vendor-grammars.cjs - tests + grammar-introspection helper load grammars from vendor/ too (single source of truth); new vendored-grammars.test.ts guards against reintroducing a bare `require('tree-sitter-<vendored>')` Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(grammars): throw on a non-vendored name in requireVendoredGrammar Drift guard (PR #2144 review, P3): validate the argument against VENDORED_GRAMMAR_PACKAGES and fail loudly on an unknown name, so the three grammar lists (package set / CLI probe / build registry) drifting out of sync surfaces as a clear error instead of a confusing absolute-path require miss. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(grammars): prepack guard against stray vendor/<g>/build/ shadowing prebuilds Publish hygiene (PR #2144 review, P2). Now that build-tree-sitter-grammars.cjs source-builds into vendor/<name>/build/, a stray build dir would ship in the tarball (files:["vendor"] overrides .gitignore/.npmignore) AND shadow the committed prebuild — node-gyp-build resolves build/Release before prebuilds/. assert-publish-grammar-coverage.cjs (prepack) now fails `npm pack` if any vendor/*/build exists (findStrayBuildArtifacts), with a clear `rm -rf` fix hint. Adds unit coverage for the new pure function. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(grammars): harden the #2111 no-bare-require regression guard PR #2144 review (P2). The guard regex missed dynamic import(), side-effect `import 'x'`, /subpath, and backtick loads, and only scanned src/. It now covers every node_modules-forcing form (single/double/backtick quotes, optional subpath), scans test/ too (excluding fixtures and the guard file itself), drops the `//`-substring false-negative (leading-comment-only heuristic), and adds a self-test asserting every load form is caught while prose mentions and tree-sitter-cpp are ignored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(grammars): correct stale vendored-grammar comments PR #2144 review (P3). kotlin/query.ts called tree-sitter-kotlin an "optionalDependency" — it is vendored and loaded from vendor/ by absolute path (#2111). proto.ts now states its remaining `_require` is only for the real `tree-sitter` dependency, not a vendored grammar (which goes through requireVendoredGrammar). Comment-only; no behavior change. 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-10 14:20:42 +01:00
Gergő Magyar	ae5ec94fd9	fix: stop impact()/route_map under-reporting blast radius (#2129, #1858, #1589/#1852) (#2136) ... * fix(query): stop impact()/context() under-reporting blast radius (#2129, #1858) Two read-side fixes to the "run impact before editing" safety workflow, both about the tools rendering "I could not give a single confident answer" as "no impact" — the most dangerous failure mode for a refactor-safety tool. #2129 — ambiguous resolution no longer hides a real caller behind a bare `impactedCount: 0`. When a bare name collides with several symbols, the resolver returns `ambiguous`; previously the payload carried a flat `impactedCount: 0`, so the real caller (which calls a *different* same-name node) was invisible unless the user already knew to disambiguate. The ambiguous branch now runs a bounded, summary-only BFS per candidate (capped at 6) and surfaces each candidate's true count plus the top-level `maxImpactedCount` / `maxRisk`, ranked most-impactful-first. `risk` stays `UNKNOWN` (ambiguity must not read as "safe"), `impactedCount` stays 0 (no single resolved symbol). The BFS and edge storage are unchanged — an empirical repro confirmed they are correct; the bug was purely in how the ambiguous case reported. Disambiguation by uid still returns the exact result. #1858 — impact()/context() now carry an additive `epistemic` field. When the queried symbol sits on an interface / indirection boundary (it implements or extends an interface, or is one) whose consumers bind via a DI container or dynamic dispatch, those callers are not traced to the concrete symbol, so the count is a lower bound. The result is annotated `epistemic: 'lower-bound'` with a human-readable `boundaries[]` note; a fully resolved leaf stays `epistemic: 'exact'`. Aligned to the surviving numeric confidence model (the 0.85 IMPACT_RELATION_CONFIDENCE heritage floor), not the long-deleted TIER_CONFIDENCE enum. Purely additive — no existing field or count changes. Tests: impact-ambiguous-blast-radius (per-candidate surfacing + uid disambiguation) and impact-epistemic-lower-bound (interface boundary → lower-bound, resolved leaf → exact, context parity). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(routes): configurable fetch wrappers + faster consumer scan (#1589/#1852) Closes the residual gap behind the now-merged #1852 (which fixed #1589): the fetch-wrapper consumer scan only traced wrappers the parse phase auto-detected as calling the bare global `fetch()`. A wrapper built on axios / a custom client, or one named outside the built-in convention, was invisible — route_map silently returned `consumers: []` (the exact "named outside convention → silent zero" hole #1858 calls out as needing a backstop). - Configurable wrappers: `.gitnexusrc` gains a `fetchWrappers: [...]` list (validated as identifier/member names, de-duped, capped, regex-safe), threaded AnalyzeOptions → PipelineOptions → routes phase. Configured names are unioned with the auto-detected ones; configured names alone now trigger the scan even when nothing was auto-detected. - Perf (F3 from #1852's review): the cross-file scan built one RegExp per (file × wrapper) — O(files × wrappers). It now builds a single alternation regex per file (O(files)) and reuses file contents already read for handler extraction instead of re-reading them. Tests: configurable-fetch-wrapper (axios-based `doRequest` wrapper — invisible without config, traced with it) + .gitnexusrc `fetchWrappers` validation cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): harden the under-reporting fixes after adversarial review Addresses findings from a reviewer-swarm pass over the two prior commits: - CLI text false-safe (major): `formatImpactResult` (eval-server.ts) had no ambiguous branch, so `gitnexus impact <colliding-name>` printed "No dependencies found. This symbol appears isolated." for an ambiguous target — the exact false-safe #2129 exists to kill, defeating the JSON-layer fix at the text surface. Added an ambiguous branch (per-candidate blast radius + maxImpactedCount/maxRisk) and a lower-bound branch for both the zero-count and non-zero paths, mirroring the context formatter. Covered by new unit tests. - Group fan-out dead work (major): impactByUid now passes skipEpistemic:true — the group cross-impact fan-out consumes only byDepth, so computing the #1858 boundary per neighbor was wasted round-trips on the highest-volume path. - Ambiguous all-UNKNOWN risk (minor): if every per-candidate probe fails, maxRisk now reports 'UNKNOWN' instead of falling to the 'LOW' seed (which would read as "safe"). - Candidate-probe cost (minor): the per-candidate summary BFS now sets skipEnrichment:true, bypassing the process/module aggregation passes it does not use. - Epistemic latency (minor): computeEpistemicBoundary now runs concurrently with the impact BFS instead of as a trailing serial round-trip. - Wrapper over-match (minor): the consumer-scan regex uses a `(?<![.\w$])` lookbehind instead of `\b`, so a bare configured name like `get` matches the free call `get('/x')` but not a member access `client.get(` (and `apiFetch` no longer matches `myApiFetch`). - Boundary wording (nit): correct article ("a class" vs "an interface") and singular/plural ("1 implementation"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lint): drop unused describe import in new impact tests The withTestLbugDB harness wraps describe internally, so the explicit describe import was unused — unused-imports/no-unused-imports is an error (not a warning) in the root eslint config, failing quality/lint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query): flag partialProbe when an ambiguous candidate probe fails (#2129 review F1) The ambiguous-impact branch hoists maxRisk/maxImpactedCount so a colliding name can't read as "isolated". But if a per-candidate BFS throws (e.g. DB pool contention during the ≤6-way fan-out), it was recorded as risk:'UNKNOWN', impactedCount:0 and silently masked by any benign sibling success — maxRisk reduced to the benign tier and maxImpactedCount reflected only successful probes. Track probeFailed and surface partialProbe:true (additive, intentionally distinct from the traversal-interrupted `partial` flag); formatImpactResult prints a lower-bound warning. Covered by a formatter unit test (a natural in-harness probe throw is unreachable — _runImpactBFS is fully self-catching under summaryOnly+skipEpistemic+ skipEnrichment). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query): report the full match count when ambiguous candidates are truncated (#2129 review F11) The ambiguous candidate list is capped at AMBIGUOUS_MAX_CANDIDATES (6), but the CLI headline read the truncated `candidates[]` length — so a name matching 9 symbols printed "6 symbols share this name" while the JSON message stated the true count. Add an additive `totalCandidates` field carrying the full match count, include a "showing N of M" clause in the message when truncated, and have formatImpactResult report the full count. Covered by formatter unit tests for the truncated and non-truncated cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(query): run context() epistemic probe concurrently with methodMetadata (#1858 review F2) impact() overlaps the #1858 boundary probe with its BFS, but _contextImpl awaited computeEpistemicBoundary serially after every other query. Start the probe right after `symKind` is known (the earliest point it can — symKind depends on the incoming/outgoing round-trips) so it runs concurrently with the methodMetadata fetch, and await it at result assembly. Output is unchanged (covered by the existing epistemic context() tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(query): flag a leaf interface as lower-bound in context() (#1858 review F3) context() passed `symKind` to computeEpistemicBoundary, but symKind collapses a single-resolved Interface to 'Class' (resolvedLabel is '' on the single-candidate path), so the `symType === 'Interface'` self-boundary branch never fired and a directly-queried leaf interface (implements nothing, but consumed) was under-reported as 'exact'. Pass an interface-preserving type (`resolvedLabel || sym.type || symKind`) instead — enrichCandidateLabels runs before the single-candidate early return and patches sym.type to 'Interface', mirroring impact()'s derivation. impact() was already unaffected. Covered by a new context()-on-a-leaf-interface test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(query): hoist epistemic relation-type lists + add USES to the allowlist (#1858/#2129 review F4, F5) F4: promote computeEpistemicBoundary's function-local heritage/consumer relation-type lists to module-level readonly constants (EPISTEMIC_HERITAGE_RELATION_TYPES / EPISTEMIC_CONSUMER_RELATION_TYPES) next to VALID_RELATION_TYPES / IMPACT_RELATION_CONFIDENCE, so a future heritage edge type is visible to the probe. Kept as arrays (not Sets) because they bind as Cypher params. F5 (latent bug): USES is emitted (emit-references.ts) and already in the default impact relTypes + context() queries, but was missing from VALID_RELATION_TYPES — so impact({relationTypes:['USES']}) filtered to [] and silently ran the full default traversal. Add it (0.5 confidence fallback, matching FETCHES/WRAPS). Updates the security.test.ts allowlist assertions (size 15→16, USES now valid). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(query): document the _runImpactBFS enrichment skip-flag composition (#1858/#2129 review F6) The three skip-flags (skipPerSymbolEnrichment / skipEpistemic / skipEnrichment) suppress distinct sub-phases and compose implicitly. Add a JSDoc block at the opts type listing what each suppresses, the three real call patterns, and the key interaction (skipEnrichment makes skipPerSymbolEnrichment a no-op). Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): genericize the shared string-array validation messages (#1589/#1852 review F7) The shared `string-array` ValueKind hardcoded fetch-wrapper phrasing in three messages (non-array, identifier-shape, empty-list). Since `source` already names the config key, genericize all three so the shared normalizer carries no fetchWrappers coupling — a future string-array config key gets sensible errors. Test assertions updated to the new wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(query): type the ambiguous candidate summary + epistemicPromise (#1858/#2129 review F8) The ambiguous per-candidate summary was read through `any`, so a rename of _runImpactBFS's return fields would silently zero candidate counts. Name the read shape ({impactedCount, risk, summary?.direct}) at the narrowing site, and type epistemicPromise as the optional-epistemic union (the skip case's `{}` subtype) — keeping computeEpistemicBoundary's own return precise (epistemic required). Type-only; no runtime change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(routes): trust validated fetchWrappers config, drop redundant re-filter (#1589/#1852 review F9) `ctx.options.fetchWrappers` is already trimmed/shape-validated/de-duped/capped in analyze-config.ts, so the routes-phase re-trim/re-typeof pre-pass was redundant. Pass it straight through; the single Set-construction filter remains to guard the auto-detected functionName values (which don't pass through analyze-config). No behavior change — covered by the existing fetch-wrapper route suites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(routes): make the wrapper-call boundary Unicode-aware (#1852 review F10) The consumer-scan lookbehind used ASCII `\w`, so a configured bare wrapper name preceded by a non-ASCII identifier character (`caféget('/x')`) satisfied the boundary and produced a spurious FETCHES edge. Switch to the `u` flag with Unicode property classes (`(?<![.\p{L}\p{N}_$])`). Covered by a fixture consumer (`cafédoRequest('/api/things')`) asserting no spurious edge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(routes): count wrapper-scan line numbers incrementally (#1852 review F12) The wrapper consumer scan computed each match's line number via content.substring(0, match.index).split('\n').length — an O(matchIndex) allocation per match. Matches arrive in ascending index, so accumulate newlines with a running counter instead. 1-based line numbers are byte-identical (covered by the existing fetch-wrapper route suites). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): keep the #1858 epistemic probe from skewing the impact-pagination mock The impact-pagination mock counts every query containing `r.type IN` as a BFS depth level. Once the #1858 epistemic boundary probe was parallelized with the BFS (it fires `MATCH (x)-[r]->(iface) ... r.type IN $heritage` before the frontier loop), that query was miscounted as depth-1, shifting the real depths so multi-depth impactedCount read 50 instead of 200. Short-circuit the epistemic queries (uniquely aliased `iface`) to empty in both mock setups so only frontier queries count. Test-only; production is unaffected (the epistemic query is a separate real query there). 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-10 11:25:48 +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
Nilotpal Kashyap	1716bf7c1e	feat(cli): add gitnexus uninstall to reverse setup (#2060) (#2062) ... * feat(cli): add `gitnexus uninstall` to reverse setup (#2060) `gitnexus uninstall` was documented in #168 but never implemented, so the CLI rejected it with "error: unknown command 'uninstall'" (#2060). Add an `uninstall` command that reverses `gitnexus setup` target-by-target: removes the GitNexus MCP server entries (Cursor, Claude Code, Antigravity, OpenCode, Codex), the installed skill directories, and the Claude Code / Antigravity hook entries plus their bundled hook scripts. Edits are surgical and idempotent — only gitnexus-owned keys/entries/dirs are touched, and JSONC comments/indentation are preserved. Defaults to a dry-run preview; `--force` applies. Per-repo indexes and the global npm package are left alone with printed hints, since both are destructive in ways setup never caused. Adds i18n entries (en + zh-CN), help wiring, README/CHANGELOG docs, and unit tests covering MCP/hook/skill/Codex-TOML removal, dry-run, corrupt-file safety, and the no-op case. * changelog changes * changelog changes * fix(cli): harden uninstall against data-loss edge cases (review #2062) Address review findings on the uninstall command: - Empty derived skill name no longer wipes the whole skills dir: a bare '.md' source file would make basename() return '', resolving to the skills dir itself. Skip empty names in derivation and reject empty/'.'/'..'/separator names in removeSkillsFrom. - Corrupt settings.json no longer orphans the hook: gate the hook-script dir removal on status !== 'corrupt' so we don't delete a script while a still-registered entry points at it (Claude + Antigravity blocks). - Hook removal is now element-granular: delete only the gitnexus command inside an entry's hooks[], removing the whole entry only when it becomes empty. Preserves a user command co-located in the same entry. - Fallback TOML stripper: also remove descendant sub-tables ([mcp_servers.gitnexus.env]), track multiline strings so a bracketed line inside a value isn't treated as a header, and stop reflowing unrelated blank lines. - Set process.exitCode=1 on partial failure; add a 10s timeout to 'codex mcp remove'. Tests expanded 7 -> 17: empty-skill guard, corrupt-settings hook preservation, shared-entry hook removal, OpenCode MCP keyPath, Antigravity MCP + AfterTool hooks, codex-remove success path, TOML sub-table + multiline-string cases, dry-run for hooks/skills, and the directory-layout skill branch. * refactor(cli): share setup/uninstall target map + harden TOML fallback (review #2062) Maintainer review follow-ups: - Extract editor target identities into editor-targets.ts (MCP paths/keyPaths, Codex TOML section, skill dirs, hook settings/events/needles/script dirs, shared detectIndentation). Both setup.ts and uninstall.ts consume it, so a target change updates both sides — killing the silent drift hazard. - Add a setup -> uninstall round-trip integration test that iterates getEditorTargets(): setup writes every target, uninstall removes all of them, and a co-located user MCP server + user hook survive. Drift tripwire in both directions. - Preview now prints the exact paths it would remove; command output + README state skills are matched by bundled gitnexus skill name. (Provenance marker deferred to a tracked follow-up.) Hardening of the hand-rolled Codex TOML fallback (found in code review): - Strip a section header that has a trailing inline comment (was matched as a header but failed the exact classify check -> section left behind while reported removed). - Preserve CRLF line endings instead of rewriting the whole file to LF. - Fix multiline-string scan: a line with an odd count of BOTH """ and ''' no longer mis-picks the delimiter and desyncs the scanner (left->right scan). - removeSkillsFrom guard also rejects absolute names. Regression tests added for each. Full setup/uninstall suite green. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-06-09 10:50:18 +01:00
Gergő Magyar	4de4d205dd	fix(ingestion): lazy-load optional grammars so analyze never crashes when one is missing (#2091, #2093) (#2101)	2026-06-09 04:58:01 +01:00
Gergő Magyar	3963c497dd	fix(parse): correct worker-pool docs drift + surface worker-side stack on crash (#2068) (#2070)	2026-06-08 07:20:12 +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