mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-08 03:08:13 +00:00
5 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
561f913a32
|
fix(embeddings): retry unparseable 200 responses and survive partial embedding failures (#2790) (#2795)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (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
Skill copy sync / shipped skills drift guard (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* fix(embeddings): retry unparseable 200 responses and survive partial embedding failures (#2790) A long-running embedding job against an OpenAI-compatible endpoint could lose hours of work to a single transient glitch, then refuse to recover on the next run. Four defects compounded: 1. An HTTP 200 carrying a truncated or non-JSON body was never retried. `classifyOutcome` treats any 2xx as success, and the `resp.json()` parse ran after `resilientFetch` had already returned, so the parse failure surfaced as a terminal error. Measured: a 503 got 3 attempts, a garbage 200 got 1. The parse and the response-shape check now run inside the `fetchImpl` callback, so a bad body is classified as a retryable failure and gets the same backoff as a 5xx. This also stops a garbage 200 from calling the circuit breaker's `recordSuccess()`, which previously erased accumulated failures and meant an endpoint alternating 5xx and garbage-200 could never trip it. 2. One failed `embedBatch` sub-batch aborted the entire pipeline. Failures are now tolerated: the sub-batch's node ids are collected and all of their embedding rows are deleted, so those nodes hold zero rows and are re-embedded later. Deleting rather than keeping partial rows is deliberate — chunk arrays are flat over a 16-node batch and sliced by 8, so a node's chunks can straddle a sub-batch boundary, and surviving rows carry the current content hash. The hash maps collapse per-chunk rows last-row-wins, so a partially embedded node would read as fresh forever and never regenerate its missing chunks. A run that fails 5 sub-batches in a row still aborts, and rethrows the first error of the streak rather than the last: after 3 failures the circuit breaker opens, so later errors degrade into "circuit open, retry in 30s" while the first still names the real defect. 3. The Phase 5 `embeddingCount === 0` fail-fast could not tell "wrote nothing" from "could not ask" — the count query's catch was silent. The count is now tri-state and only a known zero after real work is fatal. A non-numeric count previously bypassed the gate entirely, because `Number()` returns NaN and `NaN === 0` is false, and then serialized as `embeddings: null`. An unverified count no longer certifies `capabilities.vectorSearch.status`. 4. `saveEmbeddingCheckpoint` wrote a completion-shaped meta: it advanced `lastCommit`, wrote the new `fileHashes` and cleared `incrementalInProgress`. The first checkpoint window fires before a single embedding exists, and on a full rebuild the graph is still in a staging database that a crash discards. The next run then diffed against the advanced hashes, saw no changes and preserved the old graph — the "skipping wipe" symptom in the report. It now re-reads meta and replaces only the checkpoint, matching what the server endpoint already did. A partially failed run keeps its checkpoint with the failed ids in `pendingNodeIds`, so the next plain `analyze` regenerates them through the existing resume path. Clearing it would have been silent data loss: a plain run derives `shouldGenerateEmbeddings: false` once embeddings exist, so the pipeline would never have run again. The old crash-and-abort self-healed only by accident, via the checkpoint its crash left behind. `gitnexus status` reports the index incomplete until the nodes recover, and `--drop-embeddings` still abandons them. `POST /api/embed` is the pipeline's other caller and was discarding the result, reporting "Embeddings complete" for a partial run. It now persists the pending ids and reports the run as failed with the underlying endpoint error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(embeddings): abort a run whose sub-batch failure ratio is too high (#2790) The consecutive-failure ceiling only catches a total outage, because any successful sub-batch resets it. An endpoint under load shedding that alternates success and failure never trips it, so the run walks the whole corpus, deletes every failed node's rows and exits 0 having dropped a large fraction of the index. The retained checkpoint made that visible in `gitnexus status`, but a run that drops a quarter of the corpus should tell the operator to fix their endpoint, not leave them to notice a status flag. Adds a cumulative guard: abort once more than 25% of attempted sub-batches have failed, evaluated as the run progresses and gated behind a floor of 20 attempted sub-batches. The shape follows Resilience4j's circuit breaker (failure rate plus a minimum-sample floor) because it is the only one of the surveyed designs that answers the small-repo case — a three node repo can fail one sub-batch and never accumulate enough sample for a ratio to mean anything. The rate sits below a live traffic breaker's 50% because a batch indexer's job is to index the whole corpus rather than serve degraded traffic, and above Hadoop's single-digit `failures.maxpercent` because tolerating transient hiccups is the point of the change this follows. The guard reuses the existing break-then-cleanup path, so the failed batch's DELETE still runs before the rethrow, and it wraps the retained first-error-of- streak rather than inventing a new one, so the message names both the ratio and the underlying endpoint failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(server): record the embedding count after /api/embed so the next analyze cannot wipe it `POST /api/embed` generated embeddings and wrote them to the database but never wrote `stats.embeddings` into meta.json. Its checkpoint writer replaced only `embeddingCheckpoint`, and the finalize write folded in nothing else. So a repo embedded purely through the server kept whatever count the last CLI `analyze` stamped, which is 0 for a repo analyzed without embeddings. The next CLI run read `existingEmbeddingCount = 0`, `deriveEmbeddingMode` returned `shouldLoadCache: false`, and `gitnexus analyze --force` wiped the database with no cache load. Every server generated embedding was silently destroyed, with no warning — the user just lost semantic search. The route now measures the live count with the same query the CLI uses and folds it into both meta writes. The measurement is tri-state and deliberately never falls back to 0: an unverified count is written as absent rather than as zero, because a wrong-low value is exactly what arms the wipe. It is taken after `flushWAL()` and inside `withLbugDb`, so it describes durable rows and the connection is still open. A partial run records its honest count too, alongside the retained checkpoint, so the next CLI run preserves the partial index instead of discarding it. Found while working #2790; not part of that issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(embeddings): retry short 200 bodies and stop laundering body-phase timeouts Two gaps in the #2790 retry fix, both found by review. A 200 carrying `{"data": []}` or fewer vectors than inputs passed the in-`fetchImpl` shape check, because `every(isEmbeddingItem)` is vacuously true for an empty array. `resilientFetch` then classified it `success` and called `recordSuccess()`, erasing the outage signal, and the cardinality check in `httpEmbed` threw terminally one attempt later. That is exactly the pair of properties #2790 was filed about, still broken for this body shape — and worse than before the fix, since the pipeline now tolerates the error by deleting those nodes' rows instead of aborting loudly. The count check moves inside the retried callback; the outer one stays as a backstop. The `.json()` catch also swallowed every rejection, not just parse errors. `AbortSignal.any([caller, timeout])` is wired to the body stream, so a stalled body rejects with a DOMException — which, wrapped in a plain Error, defeated `classifyOutcome`'s terminal-network test. Measured: the same TimeoutError got 3 attempts and "unparseable response" when raised during the body read, but 1 attempt and "timed out after 180000ms" when raised by fetch itself, and three such sub-batches opened the process-global breaker that `recordNeutral()` exists to protect. Abort-like DOMExceptions are now re-raised unchanged. The dimension check stays outside the loop deliberately: it validates against `config.dimensions ?? DEFAULT_DIMS`, not the request-dimensions argument, and a width mismatch is a configuration error where retrying only triples latency and books failures against a healthy endpoint. Adds the negative assertion the review found missing: response body text must never reach the user-facing error string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(embeddings): scale the sub-batch failure-ratio floor to the run The cumulative guard needed 20 attempted sub-batches before a failure rate could abort anything — roughly 160 chunks, or ~80 embeddable nodes at the default subBatchSize of 8. A 50-node repo whose endpoint sheds every other sub-batch fails half of them and still exits 0: the ratio guard is below its floor, and every intervening success resets the consecutive ceiling. The floor was a good choice for a first run over a small repo, where one failure out of one sub-batch is 100% and means nothing. The defect is that every resume run has that shape by construction — its node set is only the pending ids — so the guard was structurally off in the one run whose entire purpose is retrying against the endpoint that already failed. The floor is now sized to the run: clamp(ceil(totalNodes / 16), 5, 20). The lower bound keeps the case the flat floor protected; the upper bound preserves today's behavior above 320 nodes and avoids a proportional-only floor perversely weakening the guard at scale, where a sixteenth of a 20k-node repo would be 1250 sub-batches of damage before a rate could fire. Resilience4j can use a constant minimumNumberOfCalls because a breaker sits on an unbounded call stream; a batch indexer has a finite budget, so a constant can exceed the whole run. The ratio is still evaluated only inside the catch. That is already its local maximum — both counters have just incremented — so sampling more often would only ever observe lower ratios. Also: a failing cleanup DELETE no longer swallows the abort, which was discarding the retained first-error-of-the-streak that names the real endpoint fault; `ceilingError` is renamed `abortError` since it carries the ratio abort too; and three `{ error }` log keys become `{ err }` (#2114 — an arbitrary key serializes to `{}`, losing message and stack). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analyze): one tri-state embedding counter, and stop partial runs wedging later runs The tri-state count doctrine this branch introduced was applied at two of its three CLI sites, and the two implementations that were meant to mirror each other had already drifted. `measurePersistedEmbeddingCount` moves to `core/embedding-count.ts` — beside `embedding-mode.ts`, with the same no-native-imports property, and outside `core/embeddings/` so the lazy-embeddings convention (#2370) still holds. All three call sites now share it. - The mid-run `onCheckpoint` counter ran the query bare. A throw there — DB busy, connection closed, read-only, the VECTOR DML lock (#2623) — rejected the callback out of `runEmbeddingPipeline` and killed the analyze before Phase 5 could apply the tri-state that exists for exactly this case. A non-numeric cell wrote `stats.embeddings: null` to disk mid-run. - Phase 5 used `?? 0` while the server used `?? Number.NaN`, under a comment asserting both measured the field the same way. `Number.isFinite(0)` is true, so a no-row answer became a *measured* zero and hard-failed a run whose embeddings had all persisted. - The unknown-count fallback read `existingMeta`, assigned once at run start, so it republished the pre-run figure over the fresher count the terminal checkpoint had already written. With a prior count of 0 that armed the wipe chain: hasExisting false, shouldLoadCache false, and the next --force discards live embeddings. It now re-reads the latest on-disk meta, and an unverifiable count retains a recovery marker instead of clearing it. A completed-but-partial run also planted a landmine. Its checkpoint is stamped with the run's embedding identity, so a later plain `gitnexus analyze` from a hook, a CI job, or a shell without GITNEXUS_EMBEDDING_URL resolved provider 'local' and threw before any phase ran — after an exit-0 run, where previously only a visible crash left that state. `--force` did not help: the resume gate inspected only `--drop-embeddings`. `RepoMeta.embeddingCheckpoint` gains `kind` to tell the two situations apart. An 'interrupted' marker (or one with no kind, so markers already on disk keep the stricter path) still fails closed — its nodes may be half-written, and resuming under a foreign model would mix vector spaces. A 'partial' marker names nodes the pipeline already deleted to zero rows, so nothing is at risk: an identity mismatch drops the pending set with a warning and continues. `--force` now discards a checkpoint, and `attempts` bounds the retry at EMBEDDING_RESUME_MAX_ATTEMPTS (3, matching the HTTP embedder's and the WAL driver's existing per-operation budgets) so a node the endpoint deterministically rejects converges instead of keeping the repo incomplete forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(server): close the SSE stream on terminal job status, not a progress phase A tolerated partial run reached SSE clients as a clean success — a regression in this branch's own claim that /api/embed reports a partial run as failed. The pipeline emits `phase:'ready'` unconditionally before returning, including when it dropped nodes. The route mapped that to `'complete'`, and `mountSSEProgress` treated a terminal-looking *progress phase* as terminal: write the event, `res.end()`, `unsubscribe()`. The route's own `updateJob({status:'failed'})` then fired into a stream with no listener, and the web app had already shown "ready". Before this branch the pipeline threw, which produced `phase:'error'` and did reach the client. Pollers on GET /api/embed/:jobId were unaffected, so the two consumers disagreed. Terminality is a property of the job, so the relay now asks the job. Remapping `ready` alone would have left the trap armed: the `error -> 'failed'` mapping has the identical shape and would emit `event: failed` with `error: undefined` before the catch block fills the message in. `ready` is additionally remapped to `finalizing` so a poller no longer sees `status:'analyzing'` next to `progress.phase:'complete'`. The single-terminal-event property (#2264) is preserved on both the clean and partial paths, and /api/analyze is unaffected — its terminal progress phase is 'done', never 'complete'. `AnalyzeJob` gains an optional `partial` payload so a client can tell a partial run from a total failure without a new status member; it is absent on every other job, so existing payloads stay byte-identical. Consuming it in gitnexus-web is left to that app's owner — today it renders both as the same red retry chip. `resolveEmbedRunOutcome` moves to `embed-run-outcome.ts` and `mountSSEProgress` to `sse-progress.ts`, both free of Express/LadybugDB/MCP imports, and the local count copy is replaced by the shared `core/embedding-count.ts`. Reaching three pure functions previously meant importing the whole server: measured at ~20s against a 30s test timeout, with one observed timeout failure. That file is now 1.6s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: document the partial embedding index and its recovery A run can now finish exit 0 with a partial embedding index, which neither operator doc described. GUARDRAILS' "Embeddings vanished after analyze" Sign keys its trigger on `stats.embeddings` being 0 and lists "the only ways to end up at zero". A partial run stamps an honest non-zero count and sets `embeddingCheckpoint`, so the operator's actual symptom is `incompleteReasons: ["embedding-checkpoint-pending"]` — a state that Sign cannot match. Adds a Sign for it and drops the exhaustive framing from the existing one. RUNBOOK gains the recovery path: a plain `gitnexus analyze` is correct and needs no flag, because a retained checkpoint forces generation for the pending nodes regardless of flags. Also corrects two stale claims — that `stats.embeddings` is always freshly measured (it can carry forward when the count query cannot answer, which is why `capabilities.vectorSearch.status` is the certified read), and that later analyzes must always pass `--embeddings` or lose their vectors, which contradicts Non-negotiable 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(embeddings): one owner for the checkpoint record and the abort predicate Cleanup pass over the #2790 review fixes. No behavior change except where noted; the two exceptions are both cases where the code was lying to the operator or to the other half of itself. The previous pass extracted `core/embedding-count.ts` because two hand-copied bodies of "measure the embedding count" had drifted inside a single change. It then created a second pair of hand-copied publishers — of `RepoMeta.embeddingCheckpoint` — and those had drifted too: the CLI armed the attempt counter only after clearing its identity gate, the server derived it from the resumed marker alone. Only one of the two READERS implemented `kind` at all, so a 'partial' marker written by `gitnexus analyze` and resumed through POST /api/embed still hit the permanent wedge `kind` exists to remove. `core/embedding-checkpoint.ts` now owns the record: `checkpointKind` (the one home for absent-means-interrupted), the three minters, `nextAttemptCount`, and `decideEmbeddingResume`, which both gates route through. Five mint sites and two resume gates become one implementation each. `resilient-fetch.ts` exports `isTerminalNetworkError` and `classifyOutcome` calls it, replacing a caller-side copy of the same DOMException test whose docstring promised it "mirrors classifyOutcome exactly" — an invariant enforced by prose, where a divergence silently reverts body-phase timeouts to being retried three times and charged to the shared breaker. The ratio-guard floor now divides by the run's actual `subBatchSize` instead of a constant 16 that assumed the default of 8. At `subBatchSize: 32` the old formula demanded more sub-batches than the run contains, leaving the guard structurally off — the exact failure the scaled floor was introduced to fix, and sub-batch size is tuned mainly for the flaky endpoints it protects. Two operator-facing corrections: - The count-recovery marker was stamped `kind: 'partial'` with an empty pending set, so `gitnexus status` reported "N node(s) lost their embeddings" where N is zero. It gets its own kind and its own incomplete reason. - `decideEmbeddingResume` initially keyed its skip-the-identity-gate branch on an empty pending set, assuming that meant the count-recovery marker. It does not: `onCheckpoint` mints an 'interrupted' marker with no pending nodes after every post-window save. That silently cleared an interrupted marker under a foreign provider instead of failing closed. Keyed on `kind` now, with a regression test. Also: `isTerminalJobStatus` adopted at the seven sites that still hand-copied it, including the one gating the single-terminal-event emit; `mountSSEProgress` re-export dropped and `server-sse-payload.test.ts` repointed at the extracted module, which takes it from 24.60s to 0.408s — the test that motivated the extraction was still paying the cost it was meant to remove; the count-mismatch message and the SSE test harness deduplicated; per-batch error strings made lazy (~75k needless `new URL()` per large run); `retryable: true` dropped as a field that can never be false; ~110 lines of restated rationale reduced to pointers at their canonical home. 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> |
||
|
|
711ff8721d | fix(embeddings): make HTTP generation resumable | ||
|
|
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 (
|
||
|
|
bf09eab95b
|
feat: configure prettier with pre-commit hook (#563)
* feat: configure prettier with pre-commit hook integration Add prettier, lint-staged, and prettier-plugin-tailwindcss at the repo root with husky pre-commit hook integration. Moves husky from gitnexus/ to root package.json for reliable hook installation. - Root package.json with prepare/format/format:check scripts - .prettierrc with endOfLine:lf and tailwindStylesheet for TW v4 - .prettierignore excluding fixtures, vendor, generated, *.d.ts, *.md - .gitattributes enforcing LF line endings for Windows consistency - Pre-commit hook uses direct node_modules/.bin/ paths (no npx) * style: apply prettier formatting to entire codebase One-time bulk format. No logic changes. Use .git-blame-ignore-revs to skip this commit in git blame. * chore: add .git-blame-ignore-revs for prettier format commit * perf: pre-commit hook runs only tests related to staged files Use vitest --related to scope test execution to tests that import the changed files, instead of running the full suite on every commit. * perf: remove vitest from pre-commit hook, keep in CI only Pre-commit now runs lint-staged + tsc only. Tests run in CI (ci-tests.yml) where they belong — keeps commits fast. * ci: add prettier format check to quality workflow PRs will now fail if code isn't formatted with prettier. |
||
|
|
fd7fb5bf1f
|
feat: unify web and cli ingestion pipeline (#536)
* feat: add server-side ingestion API (POST /api/analyze, SSE progress)
Extract core analysis orchestration from CLI into shared run-analyze.ts
module. Add server-side analyze endpoints so the web app can trigger
ingestion via HTTP instead of running the full pipeline in-browser.
New files:
- src/core/run-analyze.ts — shared runFullAnalysis() orchestrator
- src/server/analyze-job.ts — job manager (single-slot, dedup, SSE events)
- src/server/analyze-worker.ts — forked child process (8GB heap, IPC)
- src/server/git-clone.ts — shallow clone/pull with SSRF protection
API endpoints:
- POST /api/analyze — start analysis (returns 202 + jobId)
- GET /api/analyze/:jobId — poll job status
- GET /api/analyze/:jobId/progress — SSE progress stream
Security: URL validation blocks private IPs and non-HTTP schemes.
Path validation requires absolute paths. Git stderr not leaked to API.
* feat(web): add server-side analyze UI (Phase 2)
Add "Analyze on Server" flow to the web app's Server tab so users
can trigger server-side ingestion from the browser. On completion,
the graph is automatically loaded via the existing connectToServer flow.
New files:
- AnalyzeProgress.tsx — progress bar with phase label, elapsed time, cancel
Modified files:
- backend.ts — startAnalyze(), streamAnalyzeProgress() SSE client
- DropZone.tsx — analyze URL input + button below Connect section
- App.tsx — onServerAnalyze handler wires analyze -> connect flow
* feat: add job cancellation, timeout, and child process tracking (Phase 3)
- DELETE /api/analyze/:jobId — cancel running analysis (SIGTERM to worker)
- 30-minute timeout kills long-running workers automatically
- Child process refs tracked in JobManager for cleanup on shutdown
- dispose() kills all active children on SIGINT/SIGTERM
- Web cancel button now calls server DELETE endpoint
- cancelAnalyze() added to web backend client
* refactor(web): remove browser ingestion pipeline (Phase 4)
Delete 16 duplicated ingestion files, 2 unused service files
(git-clone, zip), and tree-sitter parser-loader from gitnexus-web.
All ingestion now runs server-side via POST /api/analyze.
Deleted (18 files, ~5,000 lines):
- core/ingestion/*.ts (16 pipeline processors)
- core/tree-sitter/parser-loader.ts (WASM tree-sitter loader)
- services/git-clone.ts (isomorphic-git client-side clone)
- services/zip.ts (JSZip extraction)
Simplified:
- DropZone.tsx — server-only (removed ZIP/GitHub tabs)
- ingestion.worker.ts — removed runPipeline/runPipelineFromFiles
- useAppState.tsx — removed pipeline callbacks
- App.tsx — removed handleFileSelect/handleGitClone
- main.tsx — removed Buffer polyfill for isomorphic-git
- types/pipeline.ts — removed PipelineResult/serialize helpers
Kept: cluster-enricher.ts (LLM enrichment, still used by worker)
Dependencies now removable: web-tree-sitter, isomorphic-git,
@isomorphic-git/lightning-fs, jszip (estimated 3-4MB bundle savings)
* refactor(web): sync graph schema from CLI + delete WASM grammars
Sync graph/types.ts and lbug/schema.ts from the CLI (source of truth)
to the web module so the browser LadybugDB can handle all node and
relationship types the server pipeline produces.
Synced types: Route, Tool, Section node labels; HANDLES_ROUTE, FETCHES,
HANDLES_TOOL, ENTRY_POINT_OF, WRAPS, QUERIES relationship types;
description fields on Function/Class/Interface/Method/CodeElement.
Deleted: public/wasm/ directory (14 tree-sitter WASM grammars + core).
Removed deps: web-tree-sitter, isomorphic-git, @isomorphic-git/lightning-fs,
jszip, buffer, @types/jszip (~3-4MB bundle savings).
* feat: create gitnexus-shared package for unified type definitions
Create a new gitnexus-shared package that is the single source of truth
for types shared between the CLI and web modules:
- SupportedLanguages enum (15 languages)
- Graph types: NodeLabel, NodeProperties, RelationshipType, GraphNode, GraphRelationship
- Schema constants: NODE_TABLES, REL_TYPES, REL_TABLE_NAME, EMBEDDING_TABLE_NAME
- Pipeline types: PipelinePhase, PipelineProgress
Both gitnexus (CLI) and gitnexus-web import from gitnexus-shared via
file: dependency. Each package re-exports and extends with platform-specific
additions (CLI: KnowledgeGraph with mutation methods; Web: simpler KnowledgeGraph).
This ensures types can never drift between packages — adding a new
language, node type, or relationship type in gitnexus-shared automatically
propagates to both consumers.
* refactor: import shared types directly from gitnexus-shared at call sites
Replace all re-export patterns with direct imports from gitnexus-shared.
72 files updated across CLI and web:
- SupportedLanguages: 49 CLI files now import from 'gitnexus-shared'
instead of '../config/supported-languages.js'
- GraphNode, GraphRelationship, NodeLabel: 22 CLI + 10 web files now
import from 'gitnexus-shared' instead of local re-export wrappers
- NODE_TABLES: api.ts imports from 'gitnexus-shared'
- PipelineProgress: useAppState.tsx imports from 'gitnexus-shared'
Local types.ts files now only define platform-specific KnowledgeGraph
(CLI has mutation methods, web has add-only). No more re-exports.
* fix: update lock files for gitnexus-shared, remove stale vite polyfills
Add gitnexus-shared@1.0.0 to lock files so npm ci succeeds in CI.
Remove buffer polyfill and global define from vite.config.ts (isomorphic-git was removed).
* fix(security): add write guard to HTTP /api/query, fix CORS proxy bypass
- Add isWriteQuery() check to POST /api/query handler — blocks CREATE,
DELETE, SET, MERGE, DROP, etc. via HTTP API (guard was only in MCP
pool adapter and browser-side, not the HTTP server path)
- Extend CYPHER_WRITE_RE with CALL, INSTALL, LOAD keywords
- Fix CORS proxy subdomain bypass: endsWith('github.com') allowed
'evil-github.com'. Now requires exact match or '.github.com' suffix
* feat(server): enhance /api/search with enrichment, add /api/grep, strip graph content
- POST /api/search: add mode param (hybrid|semantic|bm25), server-side
enrichment returns connections/cluster/processes per result in one call
(collapses 31 sequential HTTP calls to 1 for the agent search tool)
- GET /api/grep: regex search across indexed file contents, eliminates
need to transfer all file contents to browser
- GET /api/graph: strip content field by default (80-95% payload
reduction). Use ?includeContent=true for backward compat
- Add LRU cache invalidation hook point for future caching
* feat(server): add /api/embed endpoint for server-side embedding generation
- POST /api/embed: triggers embedding pipeline via onnxruntime-node
with JobManager for single-slot concurrency, timeout, and dedup
- GET /api/embed/:jobId: poll job status
- GET /api/embed/:jobId/progress: SSE stream with heartbeat, event IDs,
and X-Accel-Buffering:no header for proxy compatibility
- DELETE /api/embed/:jobId: cancel running embedding job
- Maps embedding pipeline phases (ready→complete, error→failed) to
JobManager status conventions
* feat(web): create consolidated BackendClient module
Single HTTP client replacing backend.ts, server-connection.ts, and
worker HTTP helpers. Includes:
- Typed methods: runQuery, search (enriched), grep, readFile, connect
- Generic streamSSE<T> utility extracted from analyze progress pattern
- BackendError with discriminated code field (network/server/client/timeout)
- Embed API: startEmbeddings, streamEmbeddingProgress, cancelEmbeddings
- Search with mode param (hybrid|semantic|bm25) and enrichment
* refactor(web): rewrite Graph RAG tools for backend-only HTTP queries
- Search tool: uses enriched /api/search (1 call replaces 31 sequential queries)
- Cypher tool: removes browser-side embedding; {{QUERY_VECTOR}} routes to
/api/search with mode:'semantic' instead of local transformers.js
- Grep tool: uses /api/grep instead of in-memory fileContents map
- Read tool: uses /api/file instead of fileContents map lookup
- Impact tool: getCallSiteSnippet now async via /api/file
- createGraphRAGTools now accepts GraphRAGBackend interface instead of
7 separate function params + fileContents map
- createGraphRAGAgent simplified to (config, backend, context?)
- Removed imports: embedder, lbug/schema (replaced with gitnexus-shared)
- Net: -205 lines
* refactor(web): delete WASM infrastructure, remove 7 packages (-5242 lines)
Delete browser-side LadybugDB, embeddings, search, and worker:
- gitnexus-web/src/core/lbug/ (adapter, csv-generator, schema, query-result)
- gitnexus-web/src/core/embeddings/ (embedder, pipeline, text-gen, types)
- gitnexus-web/src/core/search/ (bm25-index, hybrid-search)
- gitnexus-web/src/workers/ingestion.worker.ts (828 lines)
- gitnexus-web/src/services/server-connection.ts (merged into backend-client)
- gitnexus-web/src/types/lbug-wasm.d.ts
Remove packages: @ladybugdb/wasm-core, @huggingface/transformers,
comlink, minisearch, vite-plugin-wasm, vite-plugin-top-level-await,
vite-plugin-static-copy
Update vite.config.ts: remove WASM plugins, COOP/COEP headers,
worker config, optimizeDeps exclude
Update imports: App.tsx, DropZone, Header, AnalyzeProgress,
BackendRepoSelector, useBackend → backend-client
* refactor(web): replace Worker/Comlink with direct BackendClient calls
- useAppState: remove Worker instantiation, Comlink.wrap, apiRef.
All queries now go through BackendClient HTTP functions directly.
- Agent runs on main thread (I/O-bound LLM streaming, not CPU-bound)
- initializeAgent: creates GraphRAGAgent with GraphRAGBackend interface
bound to BackendClient methods (runQuery, search, grep, readFile)
- startEmbeddings: calls POST /api/embed + SSE progress instead of
running browser-side transformers.js pipeline
- switchRepo: no longer loads graph into WASM DB or extracts fileContents
- App.tsx: handleServerConnect simplified (no fileContents, no loadServerGraph)
- Delete old backend.ts (replaced by backend-client.ts)
- Net: -396 lines
* fix(web): fix await-in-map build error in agent streaming
Move dynamic import of AIMessage outside .map() callback to avoid
"await can only be used inside an async function" build error.
* fix(web): remove stale apiRef references that broke chat functionality
sendChatMessage referenced apiRef.current (deleted Worker ref) which
would throw TypeError. Replaced with agentRef.current guard since agent
now runs on main thread.
* fix(server): dispose embedJobManager on shutdown, fix job mutation
- Add embedJobManager.dispose() to shutdown handler (was missing,
causing cleanup timer to keep Node process alive)
- Replace direct job.repoName/status mutation with updateJob() to
ensure SSE event emission for initial status change
* fix(server): parameterize Cypher, harden grep, unify SSE endpoints
- Search enrichment: replace string interpolation with executePrepared()
using $nid parameter binding to prevent Cypher injection
- Add executePrepared() to core lbug-adapter (prepare/execute pattern)
- /api/grep: add 200-char pattern length limit (ReDoS protection),
search files on disk instead of loading entire corpus into memory
(constant memory usage regardless of repo size)
- Extract mountSSEProgress() shared helper for SSE streaming — both
analyze and embed endpoints now have consistent heartbeat (30s),
event IDs (reconnection support), and X-Accel-Buffering header
* refactor(web): remove dead code from Worker-era architecture
- Remove loadServerGraph no-op function, interface member, and all consumers
- Remove testArrayParams stub and interface member
- Remove fileContents state from GraphStateProvider (never populated in
server-side architecture)
- Remove forceDevice parameter from startEmbeddings (server-side, no device choice)
- Replace phantom EmbeddingProgress type with inline { phase, percent }
- Replace resolvePathFromContents (needed fileContents Map) with graph-based
file path resolution using filePathIndex built from graph nodes
- Fix: AI citation grounding ([[file.ts:10]]) now works via graph node lookup
instead of broken fileContents-based resolution
* fix(web): use streamAgentResponse for full tool_call/reasoning streaming
Replace naive agent.stream() loop that only handled content chunks with
streamAgentResponse() generator from agent.ts. This properly routes:
- reasoning tokens (before/between tool calls)
- tool_call events (name, args, status)
- tool_result events (completed tool output)
- content tokens (final answer after all tools done)
Previously the onChunk handler for tool_call/tool_result/reasoning was
dead code since the streaming loop only emitted content events.
* fix(web): resolve CI type errors from dead code removal
- Import GraphNode/GraphRelationship from gitnexus-shared in graph.ts
(not re-exported from local types.ts)
- Add Route, Tool entries to NODE_COLORS and NODE_SIZES constants
- Add PipelineResult type to web types/pipeline.ts
- Remove fileContents from CodeReferencesPanel and RightPanel
- Remove testArrayParams and forceDevice from EmbeddingStatus
- Remove forceDevice args from startEmbeddings() calls in App.tsx
- Fix embeddingProgress property accesses for simplified type
* fix(ci): add setup-gitnexus-web action, build shared once per job
- Remove prepare script from gitnexus-shared (tsc not available during
npm ci of consuming packages)
- Create .github/actions/setup-gitnexus-web composite action: builds
gitnexus-shared then runs npm ci for gitnexus-web
- setup-gitnexus action: already builds gitnexus-shared for CLI jobs
- ci-quality typecheck-web: uses setup-gitnexus-web (DRY)
- ci-e2e: uses setup-gitnexus-web (DRY)
- ci-tests: gitnexus-shared already built by setup-gitnexus, just
install web deps without rebuilding
* fix(ci): use prepare script so gitnexus-shared builds during npm ci
Move typescript from devDependencies to dependencies in gitnexus-shared
so the prepare script (tsc) works when npm resolves file: deps during
npm ci. No GHA modifications needed — npm handles the build lifecycle
automatically.
Remove manual gitnexus-shared build steps from setup-gitnexus and
setup-gitnexus-web actions.
* fix(ci): build gitnexus-shared explicitly in setup actions
The file: dependency protocol doesn't reliably run prepare scripts
because devDependencies aren't installed first. Instead of fragile
lifecycle hacks, build gitnexus-shared explicitly in both setup actions:
- setup-gitnexus: npm install && npm run build in gitnexus-shared/
- setup-gitnexus-web: same, before npm ci in gitnexus-web/
- ci-tests: shared already built by setup-gitnexus, web just npm ci
No prepare script, no dist in git, no typescript as a prod dependency.
* fix: remove CALL from CYPHER_WRITE_RE — breaks FTS and vector search
CALL is used by read-only procedures: CALL QUERY_FTS_INDEX(...) and
CALL QUERY_VECTOR_INDEX(...). Adding it to the write guard blocked all
FTS search, causing 3 test failures. The database is opened in read-only
mode as defense-in-depth against write procedures via CALL.
Keep INSTALL and LOAD in the blocklist (genuinely dangerous).
* fix(web): update vercel.json for gitnexus-shared, remove COOP/COEP
- Add installCommand that builds gitnexus-shared before installing
web deps (Vercel doesn't know about the monorepo file: dependency)
- Remove Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy
headers (no longer needed — WASM LadybugDB removed)
* fix(web): update tests for deleted modules
- Delete csv-generator.test.ts (tests deleted WASM-only csv-generator)
- Update security-guards.test.ts: import NODE_TABLES/REL_TYPES from
gitnexus-shared instead of deleted src/core/lbug/schema
- Update server-connection.test.ts: import normalizeServerUrl from
backend-client, remove extractFileContents tests (function deleted)
* fix(e2e): remove Server tab click — UI is now server-only
The DropZone no longer has ZIP/GitHub/Server tabs (browser ingestion
was removed). The server URL input is directly visible on the landing
page. Update e2e test to skip the tab click and go straight to input.
All 5 e2e tests pass locally.
* refactor: use gitnexus-shared for PipelinePhase/PipelineProgress types
CLI was duplicating PipelinePhase and PipelineProgress locally instead
of importing from gitnexus-shared. Updated all consumers to import
directly. Also removed dead code: SerializablePipelineResult,
serializePipelineResult(), deserializePipelineResult().
* fix(server): address PR #536 review — security, race conditions, dead code
- Fix path traversal in POST /api/analyze: split into isAbsolute + normalize check
- Add shared repo lock (activeRepoPaths) preventing concurrent analyze+embed on same repo
- Fix 202 response returning actual job.status instead of hardcoded 'queued'
- Add 30-minute timeout for embedding jobs (was missing unlike analyze jobs)
- Fix DropZone calling startAnalyze without setting backend URL first
- Add SSE reconnect with exponential backoff (3 retries) and Last-Event-ID
- Fix normalizeServerUrl to return base URL (no /api suffix) — clear contract
- Delete dead code: proxy.ts, server-graph-hydration.ts, pipeline.ts re-export barrel
- Update LoadingOverlay to import PipelineProgress directly from gitnexus-shared
* fix(server): fix repo lock key mismatch and embed cancel race
- Use getStoragePath(targetPath) as lock key in analyze handler to match
embed handler's entry.storagePath — keys now always align
- Guard embed completion: don't overwrite 'failed' with 'complete' when
job was cancelled while pipeline was still running
- Remove unused jobType parameter from acquireRepoLock
- Log backend.init() errors instead of silently swallowing
* fix: add gitnexus-shared as a local dependency in package-lock.json
* refactor: move language detection to gitnexus-shared, add syntax highlighting for all 15 languages
Move getLanguageFromFilename() from CLI to gitnexus-shared with COBOL
support added. Add getSyntaxLanguageFromFilename() for Prism-compatible
syntax highlighting covering all 15 code languages plus auxiliary
formats (json, yaml, markdown, html, css, bash, sql, xml).
Refactor CodeReferencesPanel to use shared function instead of a local
30-line switch. Delete dead gitnexus-web/src/config/supported-languages.ts
(web already imports SupportedLanguages from gitnexus-shared).
* feat(web): add first-time user onboarding with auto server detection
Replace the manual "Connect to Server" panel with an automatic onboarding
flow that guides first-time users through starting the GitNexus server.
Server detection:
- useBackend hook polls via setTimeout chain (3s, no overlap)
- Page Visibility API pauses polling when tab is hidden
- SSE heartbeat (/api/heartbeat) for instant disconnect detection
Onboarding UI (OnboardingGuide.tsx):
- Step-by-step flow: copy command → run → auto-connect
- Smart command: shows `gitnexus serve` in dev, `npx gitnexus@latest serve` in prod
- Node.js version auto-detected from package.json via Vite define
- Faux terminal windows with copy-to-clipboard, platform tabs, polling indicator
Transitions (DropZone.tsx):
- Crossfade wrapper with snapshot pattern for smooth phase transitions
- Three phases: onboarding → success (1.2s hold) → loading → graph
- Auto-recovery: falls back to onboarding if server dies or connect fails
Server changes:
- GET /api/heartbeat: SSE endpoint for liveness detection
- GET /api/info: version, launch context, Node.js version
- npm run serve script for local development
- app.disable('x-powered-by') hardening
* feat(web): add repo analysis UI, SSE heartbeat, and review fixes
Repo analysis:
- AnalyzeOnboarding: empty-state card when server has zero repos
- RepoAnalyzer: GitHub URL + Local Folder tabs with browse button
- Header repo dropdown: click project badge to switch repos or analyze new
- DropZone 'analyze' phase integrated into Crossfade transitions
Reliability fixes from 5-agent review:
- Polling: stop scheduling timers when tab hidden, restart on visibility return
- Heartbeat: exponential backoff (1s/2s/4s, 3 retries) prevents graph loss on blip
- RepoAnalyzer: completion timer tracked in ref, cleaned up on unmount
- DropZone: standardized card padding (p-7), heading sizes (text-lg)
Accessibility:
- prefers-reduced-motion global CSS rule (WCAG 2.3.3)
- focus-visible rings on CopyButton
- cursor-pointer on all Header buttons
- Consistent rounded-xl on all dropdowns
Cleanup:
- Deleted dead AnalyzeSheet.tsx (219 LOC) and BackendRepoSelector.tsx (89 LOC)
- Fixed AnalyzeProgress lucide import (lucide-react → @/lib/lucide-icons)
* fix(server): resolve analyze worker fork crash in dev mode
The forked analyze worker was crashing immediately with exit code 1
when running via `npm run serve` (tsx). Two issues:
1. Worker path resolved to `analyze-worker.js` but only `.ts` exists
in the source directory — the `.js` file is only in `dist/`.
2. On Windows, bare `--import tsx` in execArgv fails because Node's
ESM resolver for --import uses the child's CWD, not the parent's
node_modules. Windows also rejects raw paths as `d:` is not a
valid URL scheme.
Fix: detect dev vs prod via `import.meta.url` extension. In dev mode,
resolve `tsx/esm` to an absolute `file://` URL via `pathToFileURL()`
anchored to the parent's `createRequire` context. This works on all
platforms and doesn't depend on the child's CWD or PATH.
Also captures child stderr for better crash diagnostics.
Verified: `POST /api/analyze` with GitHub URL completes successfully
in dev mode (tsx) — status goes from cloning → analyzing → complete.
* fix(server): add worker auto-retry, error handling, and crash diagnostics
Worker resilience:
- Auto-retry up to 2 times with exponential backoff (1s, 2s) on crash
- SSE progress shows "Retrying after crash (1/2)..." during retry
- Captures child stderr for crash diagnostics in failure message
- AnalyzeJob tracks retryCount per job
Server error handling:
- app.listen wrapped in Promise so EADDRINUSE/EACCES propagate cleanly
- serve.ts catches startup errors with friendly messages and exit code 1
- EADDRINUSE gets actionable guidance (stop other process or --port flag)
- Global uncaughtException/unhandledRejection handlers prevent silent exits
- DEBUG=1 env var shows full stack traces
* feat: add e2e tests for onboarding flows, worker retry, and error handling
E2E tests (onboarding.spec.ts — 11 tests):
- Flow 1: OnboardingGuide shown when server unreachable (6 tests)
- Flow 2: Auto-connect with success card, analyze phase for zero repos
- Flow 3: Analyze form — GitHub URL validation, Local Folder tab, tab switching
- Flow 4: Repo dropdown in exploring view (skipped without live server)
Updated server-connect.spec.ts:
- Replaced manual Connect button flow with auto-connect waitForGraphLoaded
Server resilience:
- Worker auto-retry (2 attempts with exponential backoff) on crash
- Friendly error messages for serve startup failures (EADDRINUSE etc.)
- Global uncaughtException/unhandledRejection handlers prevent silent exits
- app.listen wrapped in Promise for proper error propagation
* refactor(shared): enforce exhaustive language coverage via Record types
Replace the if/else chain in getLanguageFromFilename with two exhaustive
Record<SupportedLanguages, ...> maps:
- EXTENSION_MAP: every language → its file extensions
- SYNTAX_MAP: every language → its Prism syntax identifier
Adding a new member to the SupportedLanguages enum without adding it to
both maps now produces a TypeScript compile error:
Property '[SupportedLanguages.NewLang]' is missing in type...
This matches the existing pattern in languages/index.ts (providers table)
which already uses `satisfies Record<SupportedLanguages, LanguageProvider>`.
Three compile-time enforcement points now exist:
1. EXTENSION_MAP in language-detection.ts (file extensions)
2. SYNTAX_MAP in language-detection.ts (Prism syntax identifiers)
3. providers in languages/index.ts (LanguageProvider instances)
* feat(web): load source code from server and scroll to selected line
CodeReferencesPanel now fetches file content via GET /api/file when a
node is selected, instead of showing "Code not available in memory".
- Fetches via readFile() from backend-client when selectedFilePath changes
- Shows loading spinner while fetching
- After content loads, auto-scrolls to the selected node's startLine
- Highlights the selected line range with a cyan left border
- Cancels in-flight fetch if selection changes before it completes
Also: refactored language-detection.ts to use exhaustive Record types
(EXTENSION_MAP and SYNTAX_MAP) so adding a new SupportedLanguages enum
member without implementing extensions/syntax is a compile error.
* feat: buffered file reading for Code Inspector
Server: GET /api/file now supports ?startLine=N&endLine=M for reading
a line range instead of the entire file. Returns { content, startLine,
endLine, totalLines }.
Client: readFile() returns ReadFileResult with metadata. When selecting
a symbol (function, class, method), fetches only ±50 lines around the
symbol's startLine/endLine instead of the full file. File nodes still
fetch the entire file.
SyntaxHighlighter startingLineNumber set from the buffer offset so line
numbers are correct even for partial reads.
* fix: adapt readFile callers to new ReadFileResult return type
tools.ts: readFile comes from GraphRAGBackend interface which returns
Promise<string> (the adapter in useAppState extracts .content), so
revert the { content } destructuring back to plain string assignment.
useAppState.tsx: wrap backendReadFile with { repo } options object
and extract .content to satisfy the GraphRAGBackend interface.
* fix(web): ensure new repos appear in list immediately after analysis
Two fixes:
1. DropZone: handleAnalyzeComplete now passes the repoName through to
connectToServer so the specific newly-analyzed repo loads — not the
server's default first repo.
2. App.tsx: fetchRepos() is now awaited BEFORE handleServerConnect in
both the DropZone and Header flows. This ensures the repo list is
populated before the exploring view renders, so the new repo appears
in the header dropdown immediately without a page reload.
* feat: delete repos, re-analyze with force, select after analysis
Server — DELETE /api/repo:
- Acquires repo lock first (409 if analyze/embed in flight)
- Closes LadybugDB, deletes index + clone dir, unregisters, re-inits
- Lock released in finally block
Server — analyze complete:
- backend.init() must succeed before SSE complete fires
- If backend.init() fails, job is marked failed (not complete)
Web — Header repo dropdown:
- Re-analyze: calls POST /api/analyze with force=true, shows spinning
icon + inline progress bar via SSE
- Delete: acquires lock, aborts any running re-analysis SSE for same
repo, refreshes list, switches to next repo
- After analysis completes: refreshes repo list, connects to the
specific repo by name, loads graph, shows in explorer
- Retry with 1.5s backoff on 404 (server may still be reinitializing)
Type safety:
- err: any → err: unknown + instanceof BackendError in retry loop
- Added missing BackendRepo + BackendError imports in App.tsx
|