5 commits

Author	SHA1	Message	Date
pcaceres-hazloagil	a8736a07d0	fix(lbug): checkpoint race in pool-adapter.ts + pin @ladybugdb/core to 0.18.3 (#3189) ... * fix(lbug): await evict-then-reopen so it can't race the checkpoint closeOne() closed the evicted repo's shared Database with a fire-and-forget `db.close().catch(() => {})` (no await). Both call sites that evict-then-reopen — evictLRU() right before doInitLbug opens the new connection, and the "idle & changed" path in initLbug — proceeded to open the next repo's connection immediately after, without waiting for the evicted repo's close (and the checkpoint it triggers) to finish. On the real engine the new open can then collide with that still-in-flight checkpoint, surfacing on any read as: Runtime exception: Cannot open database in read-only mode while checkpoint is in progress. Please retry later. This reproduces reliably once more than MAX_POOL_SIZE (5) distinct repos are queried within a short window (self-hosted deployments with more than a handful of active repos hit it routinely), and gets worse under genuinely concurrent requests for different repos, since nothing serialized pool mutations across callers either. Fix: - closeOne / evictLRU are now async and await their internal work (closeOne's own close() call; evictLRU's call to closeOne), closing the race within a single initLbug call. - The exported initLbug is wrapped in a small async mutex (initLbugInner does the real work) so concurrent initLbug calls for different repos serialize instead of each racing their own evict-then-reopen against the others. - closeLbug's two closeOne() calls are now awaited too — closeOne becoming async meant closeLbug could resolve before pool.delete() had actually run, which a repo-pinning test caught (isLbugReady() briefly still true right after a resolved closeLbug()). - closeOne now deletes the pool entry (and clears its pin, and notifies pool-close listeners) BEFORE the awaited db.close(), not after. Review caught that the previous order left a "zombie" entry reachable via pool.get(repoId) — closed=true, available emptied, but still present — for the duration of that await; a same-repo query/init landing in that window would see isLbugReady() as true and hit a "Connection pool integrity error" in checkout() instead of just reopening. Deleting first removes the entry entirely, so a concurrent caller takes the normal fresh-open path instead. Verified two ways: - Against the compiled bundle (`ghcr.io/abhigyanpatwari/gitnexus`, 1.6.10/1.6.11 — pool-adapter.js is byte-identical between them): an A/B docker build with 7 tiny local repos and genuinely concurrent (parallel, not sequential) /api/graph requests goes from 7/7 failing to 7/7 succeeding on a freshly-analyzed pool. - Unit tests here (mocks @ladybugdb/core the same way as lbug-pool-pinning.test.ts): one asserts the evicted repo's close() completes before the initLbug call that triggered the eviction settles; another asserts closeLbug's own promise doesn't resolve before the underlying close() does. Both gate their mock's close() on a real short delay and were confirmed to fail against code that drops the corresponding await. Note: a second, deeper issue was also observed in the docker A/B setup — repeated rounds of concurrent access show a repo that has gone through one evict+reopen cycle can become permanently unable to reopen for reads, identically with and without this fix. That did not reproduce with mocks and isn't understood yet; filed separately as #3186, which stays open and untouched by this PR — this fix closes a real, root-caused bug on its own but does not resolve #3186 by itself. Second review round caught a follow-up: the idle-timeout sweep calls closeOne(repoId) directly, outside of initLbug's poolLock. Now that closeOne deletes the pool entry before its awaited close(), an unsynchronized idle close racing a same-repo initLbug could let that initLbug treat the repo as absent while the idle close (and its checkpoint) is still in flight — reopening the same class of race this PR exists to close, just via the idle path instead of LRU eviction. Routed the idle sweep's closeOne call through withPoolLock too, so it serializes against initLbug the same way evictLRU already does. (Tried to add a mocked regression test for this specific interleaving; dropped it — the mock's dbCache-reuse path masks the difference regardless of the fix, so it could not be made to discriminate reliably. Fixed by direct code review instead, same as the note below already does for the native-engine-specific checkpoint collision.) Also removed the initLbugInner per-repoId initPromises dedup map: with every initLbug call now serialized through poolLock, a second call for a repoId already being initialized cannot observe a pending promise in initPromises (the first call always fully completes, including its finally-block cleanup, before the lock releases) — the branch was dead code the bot correctly flagged twice. Third review round caught two more follow-ups on the same theme (both introduced by making the idle sweep route through poolLock): - closeLbug()'s no-arg ("close everything") branch still calls closeOne directly in a loop over a snapshotted pool.keys(), without the lock — an initLbug racing that loop could register a fresh entry the snapshot never saw, leaving it resident after a call meant to empty the pool. Wrapped the snapshot+loop in withPoolLock. - The idle timer callback can now sit queued behind an in-progress initLbug before its turn arrives, and that init (or a concurrent touchRepo()) can refresh lastUsed in the meantime — so the pre-lock idleness check taken when the timer fired can be stale by the time it actually runs. Re-check lastUsed/checkedOut again inside the lock, right before closing, instead of trusting the outer snapshot. * fix(deps): pin @ladybugdb/core back to 0.18.3 Bisected the "checkpoint is in progress" symptom (root cause #2, not touched by the pool-adapter.ts fix in the previous commit) down to a single dependency-version-bump commit with zero application code changes: e91ea0ca, "chore(deps): bump @ladybugdb/core in /gitnexus", 0.18.3 -> 0.19.0. Confirmed both ends independently, on the actual official build (Dockerfile.cli), no engine-swapping involved: - v1.6.9 (native 0.18.3, as released): 7 tiny repos, 5 rounds of genuinely concurrent /api/graph requests each — 0/35 failures. - v1.6.10/v1.6.11 (native 0.19.1, as released): same repro — fails every round from round 2 onward. - v1.6.11 completely unmodified (not even this repo's own fix) with ONLY @ladybugdb/core downgraded to 0.18.3 (real `npm install @ladybugdb/core@0.18.3 --save-exact`, full rebuild, no application code touched): 0/56 failures across 8 rounds. That last point isolates this fully: none of GitNexus's own JS changes between 1.6.9 and 1.6.11 (including the dbIdentity/rebuild-detection logic added in #2614, or anything in sidecar-recovery.ts) are load-bearing for this symptom — the regression lives entirely in the native engine, introduced somewhere between 0.18.3 and 0.19.0. Full unit suite green with this pin (14818 passed, same 2 environment-specific flakes present on main regardless of this change — macOS realpath symlink resolution in analyzer-identity.test.ts and a subprocess retry-count assertion in review-agent-workflow.test.ts, neither touches lbug/ladybugdb). This is a pragmatic pin, not a long-term fix: 0.19.0+ presumably ships fixes of its own that 0.18.3 lacks, and the actual regression should still be root-caused and fixed upstream (tracked at LadybugDB/ladybug#919, which a maintainer is already engaging with). Recommend re-evaluating this pin once that's resolved. * fix(lbug): serialize closeLbug's single-repo branch with the pool lock Review on PR #3189 caught the same class of gap as three earlier rounds on the previous PR: closeOne now deletes the pool entry before its awaited db.close() finishes, so an initLbug(repoId, ...) racing this branch could acquire the lock right after the delete, see no cached entry, and start opening a fresh connection while this close's checkpoint is still in flight — reopening the exact race withPoolLock exists to close. The no-arg ("close everything") branch already went through the lock; this makes the single-repoId branch consistent with it. npx tsc --noEmit clean, full lbug/pool unit suite (19 files, 257 tests) green. * fix(lbug): address azizur100389's review findings on PR #3189 - pool-adapter.ts: the idle timer's in-lock recheck already re-verified lastUsed/checkedOut against a fresh snapshot, but not pinnedRepos — pinRepo() can run while the timer callback is queued behind an in-progress initLbug, and the timer would then still close a repo the caller just pinned, dropping that lease entirely (LOW finding). - Storage-version mismatches (opening an index written by a different @ladybugdb/core build, e.g. after downgrading the pinned dependency) surfaced as GitNexus's generic "unavailable, retry later" and were retried LOCK_RETRY_ATTEMPTS times for nothing, since the file's on-disk version never changes between retries (HIGH/blocking finding). Added isStorageVersionMismatchError() to lbug-config.ts and wired it into both places that actually open a LadybugDB connection: pool-adapter.ts's doInitLbug (used by MCP tools/wiki/group-sync) and lbug-adapter.ts's doInitLbug (the separate single-connection path /api/graph and /api/query use via withLbugDb). Both now fail fast with an actionable "run `gitnexus analyze --force`" message instead. In lbug-adapter.ts the check wraps both openLbugConnection and ensureReadOnlyConnectionUsable: the native engine's storage-version check isn't necessarily enforced until the first real query runs (ensureReadOnlyConnectionUsable's own probe), so openLbugConnection alone can succeed on a mismatched file. Verified end-to-end against the real native engine: registered a repo under the pinned 0.18.3 engine, swapped its .gitnexus/lbug file for one written by an unmodified v1.6.11 (0.19.1) image, restarted the server to bypass in-memory connection caching, and queried /api/graph — got the actionable message instead of the generic retry-later error. `npx tsc --noEmit` clean; full lbug/pool unit suite (36 tests, 14 files) green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(test): export isStorageVersionMismatchError from wholesale lbug-config mocks doInitLbug's catch block (both pool-adapter.ts and lbug-adapter.ts) now calls isStorageVersionMismatchError() unconditionally on every open failure, but 5 test files wholesale-mock lbug-config.js without that export — Vitest rejects access to an undeclared mocked export, so any test driving an error through that catch block (e.g. the WAL-recovery and evict-reopen-race suites) breaks (bot finding on PR #3189). Added isStorageVersionMismatchError (stubbed to always return false — none of these suites exercise the storage-version path) and STORAGE_VERSION_MISMATCH_SUGGESTION to each mock, matching the existing isWalCorruptionError/WAL_RECOVERY_SUGGESTION pattern already there. analyze-pagesize-error.test.ts was not affected: it mocks lbug-config.js via importOriginal, so it already re-exports the real function. Verified: the 6 affected files (48 tests) pass; npx tsc --noEmit clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(test): export isStorageVersionMismatchError from remaining lbug-config mocks The previous commit fixed the 5 test files that wholesale-mock lbug-config.js via vi.mock(), but missed 4 more that mock it via vi.doMock() instead (a different Vitest API my earlier grep for vi.mock(...) didn't match): lbug-adapter-wal-schema.test.ts (8 call sites), lbug-checkpoint-lifecycle.test.ts (12 call sites), and basicblock-callee-ids-schema.test.ts / convex-metadata-persistence- contract.test.ts (1 shared mock factory each). All of these exercise lbug-adapter.ts's doInitLbug, which now also calls isStorageVersionMismatchError() unconditionally on every open failure — caught by actually running the full suite rather than trusting the `-t lbug` name filter, which doesn't match these files' test names. Verified: all 10 affected files (97 tests) pass; npx tsc --noEmit clean. Full suite rerun in progress to confirm no other gaps remain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(lbug): fail-fast storage-version mismatch and unlock pool lock-retry Incremental analyze was warning through a version mismatch, and lock-retry sleep held the pool mutex so one analyze-locked repo blocked every other init. Fail immediately with the rebuild hint on both adapters, and sleep outside withPoolLock so other repos can open during backoff. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3189) - Serialize initLbugWithDb on withPoolLock so it cannot attach to a Database closeOne is still checkpointing. - Delete pin leases again after the awaited close so a pin acquired during teardown cannot survive onto the next init. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-09-12 14:49:21 +01:00
Gergő Magyar	cdbdf219dc	fix(lbug): reclaim missing-shadow WAL quarantine files on write-path init (#2638)	2026-07-22 21:30:52 +01:00
Copilot	7d500390b9	fix: Use Ladybug native read-only enforcement and prepared statement execution for Cypher query paths (#1655)	2026-05-18 06:54:24 +01:00
Copilot	a26ac55fb0	fix(lbug): Recover gitnexus analyze from orphan LadybugDB sidecars when main DB file is missing (#1622) ... * Initial plan * fix: recover from orphan lbug sidecars on init Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6e8ea6e8-f9ab-46ff-9c1b-4d2c73a6452c * test: strengthen orphan sidecar recovery coverage Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6e8ea6e8-f9ab-46ff-9c1b-4d2c73a6452c * fix(lbug): only clean orphan sidecars when DB is missing Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): cover no-cleanup path when db file exists Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): use errno-shaped ENOENT mocks for sidecar recovery Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): cover partial sidecar and unlink-failure recovery cases Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * refactor(lbug): tighten ENOENT detection and test naming Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): normalize errno mock helpers across sidecar tests Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * docs(lbug): annotate orphan `.wal.checkpoint` cleanup provenance Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/7edf5156-43e0-412d-87a4-bf4b2934deac * test(lbug): clarify unlink-failure path test intent Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/7edf5156-43e0-412d-87a4-bf4b2934deac * fix(lbug): handle orphan-sidecar cleanup error paths explicitly Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/93294b2c-57f6-459c-8eb2-86e3b8920fb0 * refactor(lbug): extract errno and error-summary helpers Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/93294b2c-57f6-459c-8eb2-86e3b8920fb0 * test(lbug): expand non-ENOENT lstat coverage and remove magic number Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/93294b2c-57f6-459c-8eb2-86e3b8920fb0 * test(lbug): add native integration test for orphan sidecar recovery Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2dd28264-4604-430a-a249-af52afd29245 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(lbug): annotate best-effort catch in integration test cleanup Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2dd28264-4604-430a-a249-af52afd29245 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(lbug): add cross-process init lock for orphan sidecar cleanup with integration tests Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/e4cbcfec-a252-449d-8d65-2f3570a253f8 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor(lbug): use INIT_LOCK_STALE_MS in stale lock detection and address review feedback Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/e4cbcfec-a252-449d-8d65-2f3570a253f8 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * style(lbug): fix Prettier line-length violation in acquireInitLock fs.open call Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a140b567-0e9b-4ec9-a158-9fe6b8685ec2 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(lbug): ensure parent directory exists before creating init lock file acquireInitLock tried to create `${dbPath}.init.lock` using O_CREAT | O_EXCL, but on a fresh repo the parent directory (`.gitnexus/`) doesn't exist yet — the mkdir call was inside the locked section. This caused ENOENT failures on all platforms (Windows, macOS, Ubuntu) during `gitnexus analyze`. Move mkdir to before the lock file creation attempt. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6883dc3c-36eb-4907-bcd8-61d23e2c641a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(lbug): verify acquireInitLock succeeds when parent directory does not exist Adds an integration test proving the fix from the previous commit: acquireInitLock now creates the parent directory before attempting to create the lock file, preventing ENOENT on fresh repos. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6883dc3c-36eb-4907-bcd8-61d23e2c641a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-05-16 11:45:32 +01:00
evolution	0daae93701	fix(lbug): drain checkpoint result before close (#1506) ... * fix(lbug): drain checkpoint result before close * test(lbug): cover checkpoint drain lifecycle * fix(lbug): close query results after reads * fix(lbug): close all stream query results * fix(lbug): harden query result cleanup --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-05-12 14:03:45 +01:00