211 commits

Author	SHA1	Message	Date
azizur100389	d268f351d3	fix(group): preserve manifest-only impact crossings (#2784) ... * fix(group): preserve manifest-only impact crossings Keep proven manifest cross-repo hits when the far endpoint has no concrete graph symbol, avoiding a guaranteed failed UID fan-out. * fix(group): verify manifest-only neighbor repos Keep manifest-only crossings from bypassing neighbor repository resolution so unavailable repos still surface as truncated fan-out. * fix(group): distinguish boundary-only impact crossings Keep manifest-only boundaries visible without treating unattempted fan-out as completed impact or escalating risk, and cover service scope, deduplication, and real bridge persistence. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-08-01 20:24:06 +01:00
azizur100389	02ebf8f199	fix(ai-context): emit compact markdown tables	2026-07-27 08:47:38 +01:00
azizur100389	24584297d2	fix(trace): add file disambiguator alias (#2705)	2026-07-27 05:05:14 +01:00
azizur100389	8307e3f01f	fix(setup): preserve existing OpenCode config.jsonc (#2694)	2026-07-26 16:23:34 +01:00
Gergő Magyar	3f1e23ba83	fix: stop misdiagnosing glibc-too-old native loads (#2672) and name the Windows FTS zero-install fix (#2669) (#2689) ... * docs(plans): add glibc-windows-fts-diagnostics plan Implementation plan for #2672 (glibc-too-old native-load misdiagnosis) and #2669 (Windows FTS prerequisites + Git Bash zero-install workaround). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cli): stop prescribing a reinstall when the host glibc is too old (#2672) The LadybugDB prebuilt binary requires GLIBC_2.34 (dlopen/pthread_* at 2.34, fstat64/lstat at 2.33). On an older host the loader reports version `GLIBC_2.34' not found (required by .../lbugjs.node) and checkLbugNative answered with "truncated file, ABI mismatch, or wrong-platform binary" plus instructions to re-run install.js. That advice is actively wrong for this class: every download ships the same prebuilt binary, so the reinstall fails identically and the user loops. Add glibcTooOldMessage: match a GLIBC_<version> token on a "not found" line, report the highest required version (compared numerically, so 2.9 < 2.34) alongside this host's glibc from process.report, state that reinstalling will NOT help, and point at the real options. The branch sits on the arm where the probe actually ran and failed, so an unrunnable probe still fails open (#2441). The glibc read is local rather than analyzer-identity's detectLibcVariant: native-check is the dependency-light startup gate and must not statically pull in a module the CLI reaches through a dynamic import. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lbug): name the Git Bash zero-install fix for Windows FTS load failures (#2669) The Windows error-126 remedy already refuses to prescribe a reinstall and names the VC++ redistributable and the OpenSSL 3 DLLs, but not where those DLLs already exist on the machine. #2669's reporter had the redistributable installed and still failed: the same command failed in PowerShell and succeeded in Git Bash, because Git for Windows puts libssl-3-x64.dll and libcrypto-3-x64.dll on PATH via C:\Program Files\Git\mingw64\bin. Add that hint to the Windows-126 and structural missing-dependency remedies through one shared const, following the VC_REDIST_INSTALL_HINT anti-drift pattern (#2383 F5). Placing it in the builders rather than at a call site is load-bearing: markUnavailable caches the whole diagnosis (#2383 F3) and ftsDegradedWarning replays that cached remedy, so a call-site fix would miss the MCP query and /api/search surfaces. The hint is a fixed system path, never a user-profile one — remedy text is not path-redacted, and fts-degraded-warning.test.ts asserts no C:\Users\ path ever reaches a user. Both touched tests now assert that property directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(readme): document the Linux glibc floor and Windows FTS prerequisites (#2672, #2669) Requirements listed only Node and git, so neither runtime prerequisite that these two issues turn on was discoverable before hitting the failure. - Linux: the LadybugDB prebuilt binary needs glibc 2.34+; name the distro versions that clear it and state plainly that reinstalling does not help. - Windows: full-text search needs the VC++ 2015-2022 x64 redistributable AND OpenSSL 3 on PATH. The redistributable alone is not sufficient (#2669's reporter had it), and Git for Windows already ships the OpenSSL DLLs, so running from Git Bash or prepending mingw64\bin is a zero-install fix. Without them analyze still succeeds but the index carries no search tables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: drop the plan document from version control docs/* is gitignored; the plan was force-added so it would travel with the work. It is working material, not a repository artifact — the code, tests and README carry the reasoning that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cli): stop doctor reporting a present-but-unloadable binary as missing (#2672) doctor printed "✗ lbugjs.node missing" for every failed native check — including the case this PR is about, where the binary is right there and merely fails to load because the host glibc is too old. It then wrote the real detail to stderr directly beneath, so the two lines contradicted each other and the headline sent users to reinstall a file they already had. It said the same for a truncated download and for an entirely absent @ladybugdb/core package. checkLbugNative already knows which of the three it found, so record it: a `kind` discriminator ('package_missing' | 'binary_missing' | 'load_failed') set at each failure return. doctor renders it through a new exported `nativeStatusLine`, following the existing pageSizeDoctorLines/poolSizeDoctorLine pure-helper pattern — which also makes the line testable, where before it had no coverage at all. An unrecognized or absent kind keeps the conservative "missing". Deriving this in doctor with a second existsSync would have re-stat'd a file the check had already inspected, and could disagree with what it actually observed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 10:05:57 +01:00
Gergő Magyar	ad1b9227c4	fix: large-repo analyze OOM and false worker-timeout cascade (#2649) (#2679)	2026-07-25 09:16:17 +01:00
jecanore	a500f70d6f	feat(analyze): add opt-in --self-commit flag for AGENTS.md/CLAUDE.md churn (#2640) ... * feat(analyze): add opt-in --self-commit flag for AGENTS.md/CLAUDE.md churn Adds a new `--self-commit` flag to `gitnexus analyze`. When passed, any AGENTS.md/CLAUDE.md changes the run makes (including first-time creation) are auto-committed, scoped to only those two files (never `git add -A`). No-ops silently if neither exists, neither changed, or the repo has no git identity configured — never fails the surrounding analyze run. Complements #1478 (--no-stats): that flag removes the volatile counts entirely, this one keeps them but eliminates the dangling working-tree diff they otherwise leave behind on every run. Closes #2639. * fix(analyze): log a warning when --self-commit fails to commit Addresses review feedback on #2640: the commit step's catch block was silently swallowing failures (e.g. missing git identity) with no signal to the user. Logs via the existing pino logger (matching the rest of the codebase's convention) with the error and the file list, while still never throwing — analyze must not fail over this. New test forces a real commit failure (missing identity, with useConfigOnly + isolated HOME/XDG_CONFIG_HOME/GIT_CONFIG_NOSYSTEM so no ambient global git config on the CI runner can mask it) and asserts the warning is captured via logger's _captureLogger test hook. * fix(analyze): refuse to sweep pre-existing edits into --self-commit Addresses both state-safety blockers from review round 2 on #2640: 1. selfCommitContextFiles could not distinguish a pre-existing unstaged user edit in AGENTS.md/CLAUDE.md from this run's generated stats refresh — both just showed up as "the file is dirty" — so a user edit sitting in either file got silently swept into the generated commit. Fixed by snapshotting each candidate's cleanliness via the new snapshotSelfCommitSafety() BEFORE analyze writes to it; only files confirmed safe (nonexistent pre-run, i.e. first-time creation, or clean pre-run) are ever added/committed. A file already dirty pre-run is skipped and logged, never touched. 2. On a failed `git commit` (e.g. missing identity), the preceding `git add` had already staged the safe files, and analyze reported nothing happened while silently leaving them staged. Fixed with a `git reset -- <safe files>` in the commit-failure catch, restoring the index to its pre-add state for exactly the files this helper staged. Wired analyze.ts to call snapshotSelfCommitSafety() once before runFullAnalysis (which is where the actual AGENTS.md/CLAUDE.md write happens, on both the fast path and the primary run), threading the result through both existing selfCommitContextFiles() call sites. New tests: a pre-dirty AGENTS.md is skipped while a clean CLAUDE.md still commits normally, and a post-add commit failure leaves nothing staged. Updated all existing selfCommitContextFiles() call sites for the new required safety-map parameter. * i18n(cli): add zh-CN translation for --self-commit help text Addresses magyargergo's follow-up on #2640: --self-commit was missing from the analyze command's OPTION_DESCRIPTION_KEYS map, so its help text never went through localizeCliHelp and always rendered in English regardless of locale. Adds the help.option.analyze.selfCommit key to both en.ts and zh-CN.ts and wires it into help-i18n.ts, matching the existing --no-stats/--skills entries. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-25 06:23:26 +01:00
Gergő Magyar	1e764cd475	fix(analyze): single-writer lock for the index write path (#2658) (#2677)	2026-07-25 05:08:13 +01:00
ArgonarioD	39e9dc8b25	Merge remote-tracking branch 'upstream/main'	2026-07-23 14:50:09 +08:00
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
ArgonarioD	6150a793e8	docs(cli): mention .agents/skills/ mirror in --skip-skills help + test ... Address review finding (LOW — docs/help staleness): the --skip-skills help text and README omitted that skills also mirror to .agents/skills/ when .agents/ exists. - index.ts + i18n (en/zh): --skip-skills now reads "directly under .claude/skills/ and .agents/skills/". - skip-git-cli.test.ts: assert the help text covers .agents/skills/. Co-Authored-By: Claude <noreply@anthropic.com>	2026-07-22 11:24:30 +08:00
ArgonarioD	38d0256474	Merge remote-tracking branch 'upstream/main'	2026-07-22 11:00:10 +08: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
ArgonarioD	1c98e7c6dd	fix(cli): make .agents/ skill mirror best-effort + exclude from dirty check ... Address review findings on PR #2488: - skill-gen.ts: wrap mirror-root mkdir and per-skill mirror writes in try/catch + warn, so a mirror failure (e.g. .agents/skills is a file) no longer aborts canonical community-skill generation or destroys prior output. Mirroring is now a weak side-flow, matching ai-context.ts. - git.ts: exclude .agents/ + .agents/** from isWorkingTreeDirty so a tracked .agents/ dir doesn't permanently defeat the up-to-date fast path. - README + --skip-skills help (en/zh): note skills also mirror to .agents/skills/ when .agents/ exists, and --skip-skills skips both. Tests: +18 covering mirror failure paths (root-is-file, per-skill fail, delete-then-rewrite ordering, namespace-scoped cleanup), dirty-check excludes (real-edit regression, prefix collision, subdir .agents/, non-git/git-missing conservative fallback), gate on file-not-dir, and idempotency. Co-Authored-By: Claude <noreply@anthropic.com>	2026-07-20 16:19:59 +08:00
ArgonarioD	6182231cd4	Merge remote-tracking branch 'upstream/main'	2026-07-20 10:58:08 +08: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
ArgonarioD	7ba5483f61	Merge remote-tracking branch 'upstream/main'	2026-07-17 10:10:34 +08: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
ArgonarioD	fb2910a05c	Merge remote-tracking branch 'upstream/main'	2026-07-16 19:04:08 +08:00
ArgonarioD	a5ec631f09	Merge remote-tracking branch 'upstream/main'	2026-07-16 17:58:48 +08: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
ArgonarioD	2c5b390d96	Merge remote-tracking branch 'upstream/main' ... # Conflicts: # gitnexus/src/cli/ai-context.ts # gitnexus/src/cli/skill-gen.ts	2026-07-16 15:47:14 +08: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
ArgonarioD	c7a9b7efc2	feat(cli): mirror skills to .agents/skills/ when .agents/ exists ... Some agents prefer repo-local .agents/skills/ over the global install. When .agents/ is present, mirror the standard and generated skills written to .claude/skills/ so those agents serve up-to-date copies. Opt-in via .agents/; absent directory leaves the layout untouched. Co-Authored-By: Claude <noreply@anthropic.com>	2026-07-14 16:14:30 +08: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