mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-10 03:27:59 +00:00
7 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gergő Magyar
|
21a52af1d4
|
fix(lbug): ship FTS per-platform and recover in-place native aborts (#3274)
* fix(lbug): pin Ladybug core so Dependabot cannot ship a skewed FTS artifact The extension version is a separate upstream constant. Ignore daily core bumps and fail the pairing gate when the committed manifest does not name the installed core. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): make doctor and CI FTS gates resolve the packaged artifact Doctor and the REQUIRE_FTS file gates still treated an empty ~/.lbdb as unavailable, which would turn three CI jobs red once analyze stops installing into that tree. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): name native-abort and tuple-missing so analyze cannot mis-advise The CLI summary's trailing else treated every unknown skip reason as a missing extension. New crash and platform causes must get their own remedies, not a network-install hint. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): delete the dead read-path FTS index create ensureFTSIndex had no production callers and swallowed read-only CREATE_FTS_INDEX failures, which hid the only signal that a reader tried to write. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): vendor per-platform FTS artifacts so analyze needs no host install Keyword search depended on a CDN fetch into ~/.lbdb. Shipping the five published tuples inside the package makes air-gapped and ignore-scripts installs load the same artifact the publish gate checksums. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): load the packaged FTS artifact before any network install Analyze still required a CDN fetch into ~/.lbdb even when the package already shipped the file. FTS now path-loads the vendored tuple first and records source labels so a later truncated home copy cannot steal the diagnosis. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): diagnose a core/extension version skew instead of a missing runtime A structurally valid FTS artifact whose path version disagrees with the packaged pin must name both versions, not prescribe VC++ or OpenSSL. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): stamp an FTS phase so repair stays usable after an in-place abort A native CREATE_FTS_INDEX abort leaves no skip reason; the next run infers it from the dirty flag, and --repair-fts must not treat that phase as a half-written graph. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): park an in-place FTS crash WAL without wiping the graph An FTS abort after a successful checkpoint must reopen the live index on macOS, Windows, and Linux. Staging never parks the live WAL; readers keep today's large-WAL refusal. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): refuse read-only opens of an FTS-poisoned WAL MCP and serve cannot repair a leftover in-place abort. Fail before the native open and name --repair-fts, on macOS, Windows, and Linux. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): name a vendor-neutral Windows OpenSSL prerequisite OQ1 is unanswered here so GitNexus does not ship OpenSSL DLLs. Windows FTS now asks for a system OpenSSL 3 runtime instead of Git Bash PATH. Co-authored-by: Cursor <cursoragent@cursor.com> * test(lbug): inject the FTS vendor root and redact it on HTTP and MCP Path-loaded artifacts no longer vary with HOME. Tests pass an injected vendor tree and assert search warnings never leak a filesystem path. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(lbug): document load-only as the global FTS install default Analyze still overrides to auto. Packaged per-platform artifacts load before any network install on macOS, Windows, and Linux. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(lbug): format the FTS install-policy README table Prettier does not run on Markdown in pre-commit, so the U10 table wrap needs its own formatting commit. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): skip FTS CREATE after a persisted native abort A recovered analyze run was retrying CREATE_FTS_INDEX from skipReason alone. Keep that skip until --repair-fts, fail closed on unsupported tuples, and honor the checkpoint warrant for park/repair. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): honor checkpoint flushed warrant and align FTS tests with packaged vendor A no-op CHECKPOINT must not satisfy the FTS park warrant, and CI still asserted HOME-only FTS isolation after analyze started path-LOADing the packaged artifact. Co-authored-by: Cursor <cursoragent@cursor.com> * test(lbug): accept a nonempty incremental write set in the #2790 recovery check FTS-phase recovery can incremental-add files (changed=0, added=1). That is not the #2790 empty-diff wipe skip. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): compare FTS home versions to the core pin and tighten the publish filename gate Ladybug's ~/.lbdb/extension directory is the runtime/core version; treating it as the artifact version false-diagnosed skew. The publish guard now rejects a path-escaping filename the same way the fetch script does. Co-authored-by: Cursor <cursoragent@cursor.com> * test(lbug): seed FTS e2e fixtures from the packaged vendor artifact A machine with no ~/.lbdb copy should still run the vendor-survivorship cases; the seed no longer depends on HOME or a network install. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3274) Keep in-place FTS abort evidence after persist so a second CREATE abort cannot fail-open readers, and close the CLI, loader, embed, and e2e gaps the review called out. Note: full npm test hit Ladybug worker-pool startup failures under memory pressure; tsc and 180 targeted unit tests passed. Co-authored-by: Cursor <cursoragent@cursor.com> * Address PR review feedback (#3274) Run the vendored-path symlink guard on the OS matrix, put e2e HOME fixtures on Ladybug's real extension layout, pin the embed crash-WAL gate before the writable open, and let analyze writers park through missing-shadow recovery. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(lbug): keep --repair-fts CI green after vendored-first FTS Never-installed warning fixtures must not inspect a packaged vendor binary, and a failed dirty restamp must not abort an otherwise successful --repair-fts run. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): give the #1169 analyze e2e the same 90s Windows budget as its sibling The first #1169 persist-meta case was still on a 60s spawn/it budget and was killed banner-only on windows-latest after the FTS warning fixture no longer failed the shard first. Co-authored-by: Cursor <cursoragent@cursor.com> * test(ci): reweight Windows shards after the FTS e2e grew Vendored-first HOME fixtures pushed fts-extension-e2e to ~6 minutes on windows-latest, so the old 146s weight packed it with skills-e2e and blew the 20-minute watchdog. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
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:
|
||
|
MyShining
|
de84ad6297
|
feat(spring): index @Bean factories and @Resource injection (#2740)
* feat(spring): index Bean factories and Resource injection * fix(spring): address Bean and Resource review findings * refactor(lbug): keep relation pair parsing in router * test(lbug): preserve schema exports in WAL mocks * test(cache): align schema bump pin --------- Co-authored-by: Shining <xuenning@qiyi.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
Gergő Magyar
|
cdbdf219dc
|
fix(lbug): reclaim missing-shadow WAL quarantine files on write-path init (#2638)
Some checks are pending
Gitleaks / gitleaks (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
|
||
|
Gergő Magyar
|
b98f6e458f
|
fix(lbug): recognize Windows missing-shadow error so serve repo-switch recovers (#2382) (#2387) | ||
|
ChamHerry
|
a9fef2c68d
|
fix(lbug): keep serve stable when sidecars are missing (#1747)
* fix(lbug): keep serve stable when sidecars are missing Shared missing-shadow WAL recovery prevents repeated read-only open warnings when LadybugDB sidecars are absent, while the Express preflight fix keeps `gitnexus serve` compatible with Express 5 route parsing. Constraint: LadybugDB read-only replay can require a `.shadow` sidecar that may be absent after interrupted writes or checkpoint edge cases. Rejected: keep reactive WARN-only quarantine in each adapter | it leaves repeated user-visible warnings and duplicate recovery behavior. Confidence: high Scope-risk: broad Directive: Do not silently delete large orphan WALs; only quarantine tiny orphan WALs before open and keep large WALs for explicit recovery. Tested: cd gitnexus && npx vitest run test/unit/sidecar-recovery.test.ts test/unit/lbug-adapter-wal-schema.test.ts test/unit/pool-wal-recovery.test.ts test/unit/web-ui-serving.test.ts && npx tsc --noEmit Not-tested: full npm test in this split branch; full unit suite passed on the source branch before PR split. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(lbug): pool-caller ENOENT guard, symmetric size gate, permission-aware errors (PR #1747 review) Addresses the production-readiness review of PR #1747 (Findings 1, 2, 3 of 6). Findings 4, 5, 6 are deferred to follow-ups per the plan. 1. ENOENT-tolerance scoped to pool-adapter callers only - `quarantineWalForMissingShadow` stays strict in `sidecar-recovery.ts`. The direct adapter calls it inside `acquireInitLock` (cross-process file lock) — ENOENT there means the file vanished under lock and remains a real bug to surface. - New `tryQuarantineForMissingShadow` local helper in `pool-adapter.ts` returns a discriminated union { kind: 'quarantined', path } | { kind: 'peer-handled' }. Catches ENOENT, re-verifies via statIfExists, and converts to 'peer-handled' only when WAL really is gone. Defensive: if ENOENT but WAL still present, throws as classified error rather than silently returning success. 2. Symmetric WAL-size gate on both recovery paths - `refuseLargeWalQuarantine` applied in both `reopenReadOnlyAfterMissingShadow` and `reopenWritableAfterMissingShadow`. Closes the read-only data-loss vector (large orphan WAL silently discarded would never be replayed by a later writable open). 3. Permission-aware error classifier - New `renameFailureMessage` and `isPermissionRenameError` in `sidecar-recovery.ts`. EACCES / EPERM / EBUSY now surface a permission-specific message pointing at ACLs, AV exclusions, and file-locks. Other codes (ENOSPC, EROFS, EIO, ENOENT) fall through to `shadowSidecarRecoveryMessage`. - Used at both pool-adapter and direct-adapter caller catches around `quarantineWalForMissingShadow`. - `doInitLbug`'s pass-through classifier extended to include the new permission message. The lock-retry substring match tightened so "file-lock error" in the permission message is not mistaken for a LadybugDB lock-retry trigger. Tests - sidecar-recovery.test.ts: 7 new tests for `renameFailureMessage` and `isPermissionRenameError`. - pool-wal-recovery.test.ts: 6 new tests covering ENOENT race, EACCES/EPERM/EBUSY classification, ENOSPC fallthrough, and the defensive "WAL still present after ENOENT" branch. - lbug-adapter-wal-schema.test.ts: 5 new tests covering the symmetric size gate on both recovery paths, including the boundary at exactly TINY_ORPHAN_WAL_BYTES (4096) and the off-by-one at 4097. Deferred (tracked as follow-up work) - Brittle LadybugDB error-string matching (Finding 4). - PNA header end-to-end coverage gap (Finding 5). - warnedKeys module-global persistence (Finding 6). - Cross-process init lock for pool-adapter. * fix(lbug): dedup shadow-replay predicate + counter-based warn anti-spam (PR #1747 review, Findings 4 & 6) Smallest viable response to the two remaining non-blocking findings from the production-readiness review of PR #1747. An earlier-revision plan proposed regex widening + a near-miss detector + per-dbPath warn scoping; an adversarial doc-review found those defended against hypothetical strings LadybugDB does not produce, added observability theater with no recovery behavior change, and did not actually fix the long-running gitnexus serve case for hot dbPaths (where finalizeLbugSidecarsAfterClose rarely fires). Scope shrunk to dedup + counter-based — strictly behavior-changing and fully testable. Finding 4 — dedup + version-coupling markers - `isReadOnlyShadowReplayError` was inlined in both `lbug-adapter.ts:451` and `pool-adapter.ts:317`. Centralized as an export from `sidecar-recovery.ts`. The two local copies are removed; both adapters now import from the shared module. - Both LadybugDB-coupled predicates (`isMissingShadowSidecarError` and `isReadOnlyShadowReplayError`) gain a `// LADYBUGDB-CONTRACT:` marker comment citing `@ladybugdb/core ^0.16.1`. When bumping LadybugDB, `git grep "LADYBUGDB-CONTRACT"` enumerates every version-coupled spot. - Strict matcher unchanged — when LadybugDB actually changes the error format, the failure mode stays loud (raw native error propagates) and the markers make every affected predicate trivially greppable. Finding 6 — counter-based warn anti-spam - `warnedKeys: Set<string>` → `warnedKeyCounts: Map<string, number>`. `warnOnce` keeps its signature `(logger, key, message)` and keying convention unchanged — the swap is internal. - `WARN_MILESTONES = [1, 10, 100, 1000, 10000]`. Logarithmic spacing gives O(log N) warns for a condition that fires N times. Past the first occurrence the warn message is suffixed with "(Nth occurrence of this condition)" so persistence is visible in the log line itself. - Solves the long-running serve case: a hot dbPath hitting the same condition 100 times now fires 3 warns (occurrences 1, 10, 100) instead of 1 warn + 99 silent debug lines. Tests (10 new in sidecar-recovery.test.ts, all green) - Centralized isReadOnlyShadowReplayError: positive match, false-positive guard, structural assertion that the duplicate regex is gone from both adapter files, LADYBUGDB-CONTRACT marker count. - Counter-based warnOnce: milestone-at-10 with suffix, milestone-at-100, key isolation across dbPaths, reset zeroes the counter, first-occurrence message does NOT carry the suffix. Deferred (tracked separately) - Finding 5 — PNA header end-to-end coverage gap (CORS boundary is sound). - LadybugDB structured error codes (if/when the library exposes them). - Per-call milestone configurability — re-open if tuning is needed. * chore(autofix): apply prettier + eslint fixes via /autofix command * ci: trigger CI rebuild --------- Co-authored-by: wangxc <wangxc_a_bj@si-tech.com.cn> Co-authored-by: OmX <omx@oh-my-codex.dev> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Nilotpal Kashyap
|
dfbe68ad24
|
fix(lbug): issue #1647, detect WAL corruption in schema init and surface recovery (#1650) |