mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 21:19:18 +00:00
* fix(pglite): drain fire-and-forget last_retrieved_at writes before disconnect Closes the structural bug class behind #1247, #1269, #1290: PGLite CLI search/query/get_page commands printed results then hung at ~95-98% CPU until SIGKILL. Root cause: bumpLastRetrievedAt's IIFE races engine.disconnect() — PGLite's WASM runtime keeps Bun's event loop alive while the dangling UPDATE settles. Mirrors the existing awaitPendingSearchCacheWrites precedent landed in v0.36.1.x for #1090. Tracks every IIFE promise in a module-scoped Set, exposes awaitPendingLastRetrievedWrites(timeoutMs) that resolves once all settle. Bounded with a 5s default timeout via Promise.race so a future fire-and-forget that hangs forever can't recreate the bug class at this layer — instead, the drain stderr-warns with a pending count and returns timeout outcome so the caller can decide its fallback. Test coverage: 6 unit cases covering empty drain, single + multi-pending settle, throw-in-IIFE still settles, permanently-pending hits timeout within bound, empty pageIds does not track. This commit ships the helper + tracking + tests with NO consumer. The cli.ts wiring lands in a follow-up commit (atomic bisect units). Co-Authored-By: Park Je Hoon <jehoon@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pglite): snapshot+early-null disconnect + try/finally lock-leak guard Refactor PGLiteEngine.disconnect() with two structural fixes: (1) Snapshot + early-null pattern: capture db/lock refs and null the instance fields BEFORE any await. A concurrent connect() can no longer observe `_db` pointing at a handle that's mid-close. This is PR #1337's load-bearing contribution that we DID take. (2) Wrap close + release in try/finally. Without this guard, a thrown db.close() would leak the file lock and wedge every next gbrain invocation on the stale lock. Codex outside-voice review (eng review finding #7) caught this gap when reviewing the snapshot refactor. KEEP the original close-then-release order. PR #1337's diff swapped this to release-then-close, which we explicitly REJECTED — releasing the lock before close lets a sibling process try to connect to a still-closing brain. The new lifecycle test file pins this ordering so a future maintainer reading PR #1337's diff cannot accidentally flip it. Test coverage in test/pglite-engine-disconnect.serial.test.ts: 5 cases — close-before-release ordering, early-null observable inside close, lock-still-releases on close-throw, double-disconnect idempotency, reconnect-after-disconnect clean state. `.serial` because each test creates a fresh PGLite engine (WASM cold-start cost) — running in parallel shards would starve other tests. Existing test/pglite-engine.test.ts: 100/100 still green. Co-Authored-By: Matt Dean <matt-dean-git@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pglite): classify WASM init errors so #1340 gets the right hint (#1340) Closes the user-facing half of #1340: on macOS 12.7.6 + Bun 1.3.14, the PGLite connect() catch block hardcoded the macOS 26.3 hint (#223). The actual root cause for #1340 is Bun's vfs: `/$$bunfs/root` is read-only on older macOS, so PGLite cannot extract its pglite.data WASM payload. Adds two exported helpers in pglite-engine.ts: classifyPgliteInitError(message): 'bunfs' | 'macos-26-3' | 'unknown' buildPgliteInitErrorMessage(verdict, original): string Connect catch block now routes the hint by verdict. The bunfs hint names `bun upgrade` + Node fallback. The macOS 26.3 hint keeps the existing #223 link. Unknown falls through to a generic doctor + #223 fallback. Per Codex eng-review finding #9, the bunfs regex is tightened to match either the literal `$$bunfs` marker OR ENOENT+pglite.data co-occurrence — NOT generic `pglite.data` substring (would fire on unrelated errors). Negative test pinned. Root fix is upstream Bun; this PR just stops misclassifying the failure class so support traffic doesn't conflate two unrelated bugs. Test coverage: 12 pure-function unit cases including the #1340 reporter's exact error string round-trip, the negative case Codex caught, and all three verdicts × all three message contents. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(cli): await last-retrieved drain + narrow timeout-only force-exit (#1247, #1269, #1290) Wires the v0.40.10.0 drain helper into cli.ts and adds the IRON-RULE behavioral regression test for the search-hang class. The drain is called unconditionally for every op (not per-op-name gated — that was the original PR #1259 mistake that left search and get_page exposed). The narrow force-exit synthesis (decision D7 from the eng review, informed by Codex outside-voice findings #1+#2+#8): when the drain returns outcome:'timeout', AFTER engine.disconnect() resolves AND the command is NOT a daemon, fire process.exit(0). The drain helper already stderr-warned with the pending count, so the diagnostic signal is preserved. Without this guard, a hung underlying promise could still keep Bun's event loop alive past disconnect. CRITICALLY narrower than PR #1337's blanket force-exit: the timeout path is the only trigger. In the common case (drain settles cleanly under 5s), no force-exit fires and the behavioral subprocess test still catches future regressions. The shouldForceExitAfterMain guard excludes 'serve' so the stdio + HTTP daemons stay alive past main(). e2e/pglite-cli-exit.serial.test.ts (NEW, IRON RULE): - gbrain search "foxtrot" → exits 0 within 15s - gbrain get alpha → exits 0 within 15s with foxtrot in stdout - gbrain query "foxtrot" --no-expand → exits within 15s (no-API-key graceful) - gbrain serve --http → stays alive 3+ seconds (daemon-survival regression guard) fix-wave-structural.test.ts: - import assertion for awaitPendingLastRetrievedWrites - last-retrieved.ts exports + Set tracking + Promise.race + timeout - BEHAVIORAL positioning assertion: drain `await` appears textually BEFORE engine.disconnect `await` in the op-dispatch local-engine path. Survives variable-rename refactors; catches any new disconnect path that bypasses the drain. - shouldForceExitAfterMain excludes 'serve' AND the gate is conditioned on drainResult.outcome==='timeout' Per D8 (Codex finding #5), explicitly do NOT add a drift-guard counting bumpLastRetrievedAt callers — would block harmless refactors and miss aliases. Co-Authored-By: Park Je Hoon <jehoon@users.noreply.github.com> Co-Authored-By: Matt Dean <matt-dean-git@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sync): add phase breadcrumbs to performSyncInner for #1342 triage The #1342 reporter saw ZERO stderr output before their PGLite sync hang, which made the bug impossible to triage from a community report alone. Mirrors the pre-existing `[gbrain phase] sync.git_pull start/done` pattern at the major pre-pull phase boundaries so the next #1342-shaped report names WHICH phase spun. Four new breadcrumbs at: - sync.resolve_repo (top of performSyncInner) - sync.load_active_pack (before the v0.39 T1.5 pack load) - sync.validate_repo_state (only when opts.sourceId is set — the re-clone branch) - sync.detect_head (before the isDetachedHead probe) No behavior change — pure stderr instrumentation. Doesn't fix #1342 (which still needs investigation per the TODOS entry filed in this wave), but converts "hung with no output" into actionable diagnostic data the next time the bug shape is reported. Per D9 in the eng review + Codex outside-voice finding #14. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: annotate v0.40.10.0 PGLite hang wave in CLAUDE.md + regen llms Key Files entries updated: - src/core/pglite-engine.ts: documents the v0.40.10.0 disconnect refactor (snapshot+early-null + try/finally lock-leak guard, KEEPS close-then-release order), and the new classifyPgliteInitError / buildPgliteInitErrorMessage helpers for #1340 hint routing. Pins PR #1337's accepted-but-narrowed contribution and the rejected release-then-close ordering swap. - src/core/last-retrieved.ts (within the brainstorm entry): documents the new awaitPendingLastRetrievedWrites drain, the Set tracking pattern, the 5s bounded timeout, the cli.ts narrow timeout-only force-exit synthesis with the serve-daemon guard, and the three community-validated reports (#1247/#1269/#1290) the fix closes. Credits PR #1259 (drain pattern) and PR #1337 (snapshot pattern + force-exit guard idea). Regenerated llms.txt + llms-full.txt — build-llms.test.ts gates the drift, all 7 cases green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(todos): file v0.40.10.0 PGLite hang follow-ups Three deferred items from the v0.40.10.0 fix wave: 1. #1342 sync-hang investigation. Single-reporter, JS-tight-loop shape, needs reproducer before any fix. Documents the ruled-out hypotheses (lock-refresh heartbeat, v91 trigger, while-true loops) and three concrete diagnostic next steps. The v0.40.10.0 sync phase breadcrumbs make the next report actionable. 2. awaitPendingSearchCacheWrites timeout-symmetry retrofit. The #1090 drain shipped without a timeout; the v0.40.10.0 #1247 drain ships with one. Apply the same Promise.race + stderr warn pattern for symmetry. 3. Drain-helper extraction. Per D4 in the eng review: two surfaces is the threshold for noticing, three for extracting. Pair with the symmetry retrofit above as one focused refactor when a third fire-and-forget surface appears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * v0.40.10.0 fix(pglite): search/query/get exit cleanly + #1340 hint + #1342 breadcrumbs Closes #1247, #1269, #1290 (PGLite CLI search/query/get hang at ~95-98% CPU after printing results — three community-validated reports). Also fixes #1340 (WASM init misroutes to macOS 26.3 hint when real cause is Bun vfs read-only mount) and adds diagnostic phase breadcrumbs for the single-reporter #1342 sync-hang investigation. Core fix: track every fire-and-forget bumpLastRetrievedAt IIFE in a module-scoped Set; cli.ts awaits the drain before engine.disconnect() in the op-dispatch finally block; narrow process.exit(0) fires ONLY when the drain times out AND the command isn't a daemon. Snapshot+ early-null disconnect pattern + try/finally lock-leak guard close the partial-state race PR #1337 originally surfaced. Co-Authored-By: Park Je Hoon <jehoon@users.noreply.github.com> Co-Authored-By: Matt Dean <matt-dean-git@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: extract shouldForceExitAfterMain to its own module + add unit cases Gap-audit follow-up: cli.ts is a script entrypoint (top-level main() side effect), so importing it from a test fires the help output as a side effect. Move shouldForceExitAfterMain into src/core/cli-force-exit.ts so it can be unit-tested in isolation without the cli.ts script tail running. Adds test/cli-should-force-exit.test.ts (9 cases): bare serve, serve with flags after, global flags BEFORE the command (the load-bearing case for `gbrain --quiet serve`), op commands return true, non-daemon CLI commands return true, empty argv defaults to true, flag-only argv, default-arg fallback to process.argv.slice(2), substring-match avoidance (`serves` is NOT `serve` — strict equality via Set, not startsWith/includes). The daemon command set is now an explicit ReadonlySet — future daemons (a hypothetical `gbrain watch` or `gbrain daemon`) just add their name to DAEMON_COMMANDS rather than chaining ||. Updates fix-wave-structural.test.ts to look for the import + the new DAEMON_COMMANDS shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(version): rebase v0.40.10.0 → v0.41.6.0 (slot collision after v0.41.0.0+ landed) origin/master moved from v0.40.8.1 → v0.41.0.0 while this wave was in flight (PR #1367 minions cathedral). v0.41.1-v0.41.5 are claimed by other in-flight branches, so v0.41.6.0 is the next available slot. Bulk-renamed v0.40.10.0 → v0.41.6.0 across: - VERSION + package.json (trio audit clean: 0.41.6.0 / 0.41.6.0 / 0.41.6.0) - CHANGELOG.md (header + 3 prose references) - CLAUDE.md (Key Files annotations) - TODOS.md (follow-up entry header) - src/cli.ts + src/core/cli-force-exit.ts + src/core/last-retrieved.ts + src/core/pglite-engine.ts + src/commands/sync.ts (inline comments) - test/* (describe blocks + test file headers) - llms-full.txt (regenerated via `bun run build:llms`) bun.lock unchanged (version-only bump, no dep churn) per Codex #12. Verify: 52/52 wave tests pass after rename, typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: quarantine seed-pglite to .serial.test.ts (parallel WASM cold-start flake) The full-suite run during the v0.41.6.0 fix wave ship hit a 30s timeout in test/seed-pglite.test.ts under heavy 4-shard parallel contention (4972/4973 passed before SIGKILL). The test passes 11/11 in isolation. Root cause: each test instantiates a fresh PGLiteEngine (5 instances across the file, one per test) because each case writes to a different mkdtemp-ed dbPath. Under parallel shard load, multiple shards each cold-starting PGLite WASM simultaneously stretches the per-instance init from ~5s to 30s+. The shared-engine pattern (canonical PGLite block in CLAUDE.md R3+R4) doesn't apply here — different dbPaths require different engines. Fix per CLAUDE.md test-isolation quarantine rules: rename to `.serial.test.ts` so the file runs in the post-parallel serial pass with full WASM init capacity. Same pattern as test/pglite-engine-disconnect.serial.test.ts (added in this wave) and test/brain-registry.serial.test.ts (pre-existing). Removes test/seed-pglite.test.ts from check-test-isolation.allowlist since the .serial.test.ts rename auto-exempts it from the R3+R4 lint (scan skips *.serial.test.ts). 641 non-serial unit files scanned, lint clean. Verify: - bun test test/seed-pglite.serial.test.ts → 11/11 pass in 4.19s - scripts/check-test-isolation.sh → OK - bun run verify → all gates pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: pre-landing review fixes (C13 disconnect-hang, C1 set leak, C9 catch drain, M1 type drift) Adversarial review + maintainability specialist surfaced four real issues in the v0.41.8.0 wave. All four fixed in this commit; one deferred to TODOS.md as a v0.41+ follow-up (unusual caller pattern). **C13 [load-bearing, defense-in-depth for the wave's stated goal]:** `await engine.disconnect()` inside the op-dispatch finally can ITSELF hang on PGLite (db.close() racing OS-level FS state). When that happens, the entire wave's force-exit guard never runs — we recreate the original hang at a new layer. Fix: install an unref'd setTimeout hard-exit fallback BEFORE entering the try/catch/finally. The timer fires after DISCONNECT_HARD_DEADLINE_MS=10s with a stderr warn and process.exit(0). unref ensures it doesn't keep the loop alive on a healthy exit. Daemons (`serve`) are excluded by reusing the shouldForceExitAfterMain guard. **C9 [data freshness gap, narrow but real]:** The drain ran ONLY in the success branch of try. If `bumpLastRetrievedAt` fired (handler succeeded) but `JSON.parse(JSON.stringify(...))` or `formatResult` then threw, process.exit(1) killed the process and the in-flight UPDATE was discarded. Fix: drain in the catch path too before process.exit(1) (best-effort, bounded by the drain's own 5s timeout). **C1 [daemon leak]:** A timed-out IIFE used to stay in the pending-writes Set forever because its `.finally` never fires. Long-lived `gbrain serve` would accumulate references without bound across repeated timeouts. Fix: explicitly `delete` the snapshot's tracked promises from the Set after a timeout outcome. The IIFEs keep running (orphaned), but the Set no longer leaks references. Pinned by a new unit test that asserts the second drain after a timeout returns immediately with empty pending count. **M1 [silent type drift]:** `cli.ts` duplicated the `{outcome, pending}` literal shape instead of importing the `DrainOutcome` type that `last-retrieved.ts` exports exactly for this purpose. Two-line fix: add `type DrainOutcome` to the import and use it for `let drainResult`. Future changes to the return shape now propagate through TypeScript. **Deferred to TODOS.md (C6 — unusual caller pattern):** Concurrent connect/disconnect on the same `PGLiteEngine` instance can strand: disconnect snapshots+nulls the lock while connect is still in-flight, leaving the resolved engine with no file lock held. Fix requires an instance-level mutex; not worth the complexity for a caller pattern that doesn't appear in production (single instance per process, sequential lifecycle). Also broadened `test/fix-wave-structural.test.ts` regex to accept additional type-imports from `last-retrieved.ts` (e.g. the new `type DrainOutcome` import that M1 added). Test coverage: 53/53 wave tests pass (added C1-followup case to last-retrieved.test.ts). The C1 fix is also pinned by tightening the existing permanent-pending test's post-timeout assertion to expect empty pending count rather than the prior (stale) "stays in set" note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: post-ship documentation sync for v0.41.8.0 Consolidate the duplicate 'take advantage of v0.41.8.0' sections in the CHANGELOG entry into a single canonical block per the CLAUDE.md template. The wave originally landed with both '### How to take advantage' (line 13) and '### To take advantage' (line 57) as h3 headings. CLAUDE.md mandates one '## To take advantage of v[version]' h2 block per release entry, with verify steps + an issue-filing fallback for users hitting upgrade failures. Promoted the second block to h2, added the issue-filing step, and removed the redundant first block (the upgrade command is already covered in the verify steps). Itemized changes section was unchanged. llms.txt + llms-full.txt regenerated; structurally identical so no content changes shipped. * fix(test): find-experts-op queries schema dim instead of hardcoding 1536 (CI shard 1) CI shard 1 failed on this branch with: \"expected 1280 dimensions, not 1536\" from pgvector's CheckExpectedDim. Root cause: master's v0.36.0 changed DEFAULT_EMBEDDING_DIMENSIONS from OpenAI's 1536d to ZeroEntropy's 1280d (src/core/ai/defaults.ts:21). The test's basisEmbedding helper hardcoded dim=1536, so beforeAll's upsertChunks failed when the schema column was created at 1280d. Latent on master: the weight-aware LPT bin-packing in scripts/sharding.ts assigns files to shards deterministically based on the COMPLETE file set. My branch adds 5 new test files, which shifted find-experts-op.test.ts into shard 1. Master's shard 1 doesn't run this file (it lands in a different shard there), so the bug never surfaced in master's CI. Fix: query the actual column dim via SELECT atttypmod FROM pg_attribute after initSchema, then seed the embedding at that width. This handles both paths (no-env CI → 1280; env-configured local → 1536) without hardcoding either default. Verify: - bun test test/find-experts-op.test.ts → 11/11 pass with provider env - env -i bun test test/find-experts-op.test.ts → 11/11 pass without - bun run verify → all 21 parallel checks clean - bun run typecheck → clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(lint): robust pure-bash allowlist match in check-test-isolation (CI verify) CI verify failed on PR #1405 with check:test-isolation flagging test/scripts/check-test-isolation.test.ts even though that file is on line 22 of the allowlist (and has been since v0.26.7 as a permanent exemption — its body contains process.env mutation fixtures that the lint legitimately matches). Could not reproduce locally on macOS bash 3.2 + BSD grep across any locale (C, C.UTF-8, POSIX). Suspect a subtle interaction between the prior `echo "$ALLOWLIST" | grep -qxF "$f"` form and one of: Ubuntu 24.04's bash 5 set-e/pipefail semantics, GNU grep edge case on the first-line entry, or `bun run` + GNU timeout subshell interaction. Diagnostic value of chasing further is low — the fix is to drop the grep+pipe form entirely. Switch is_allowlisted() to pure-bash `case $'\n'"$ALLOWLIST"$'\n' in *$'\n'"$f"$'\n'*) return 0 ;; esac` whole-line matching: - Locale-free (no character-class interaction) - Pipe-free (no pipefail / SIGPIPE / buffering) - Subshell-free (no env or exit-code propagation gotchas) - set-e-quirk-free (no left-side compound failure) - ~100x faster (no fork+exec per call across 689 files) Verified locally: lint OK (689 files), case-match returns true for the allowlisted file and false for a non-allowlisted file. bun run verify clean (21/21 parallel checks pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Park Je Hoon <jehoon@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Matt Dean <matt-dean-git@users.noreply.github.com>
This commit is contained in:
Garry Tan
co-authored by
Park Je Hoon
Claude Opus 4.7
Matt Dean
Park Je Hoon
Claude Opus 4.7
Matt Dean
parent
374deff579
commit
27b0e14af7
@@ -2,6 +2,94 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.41.8.0] - 2026-05-24
|
||||
|
||||
**`gbrain search`, `gbrain query`, and `gbrain get` now actually exit when they finish on PGLite.**
|
||||
|
||||
Until today, if you ran `gbrain search "fox"` on a PGLite brain, the results printed in under a second... and then the process sat there at ~95-98% CPU forever. You had to kill it with Ctrl-C. Scripted callers (`gbrain search "x" && echo ok`) never reached the `&& echo ok`. Cron jobs timed out. This was the #1 community pain reported since v0.37, with five open issues — three for the search-hang shape ([#1247](https://github.com/garrytan/gbrain/issues/1247), [#1269](https://github.com/garrytan/gbrain/issues/1269), [#1290](https://github.com/garrytan/gbrain/issues/1290)) plus a related WASM-init failure ([#1340](https://github.com/garrytan/gbrain/issues/1340)) and a single-reporter sync hang ([#1342](https://github.com/garrytan/gbrain/issues/1342)).
|
||||
|
||||
The fix turns out to be one structural change in two parts. v0.37 added a stale-page-detection feature that fires a background `UPDATE pages SET last_retrieved_at = NOW()` after every search/query/get. The CLI's job is to print the results, close the database, and exit — but on PGLite the database is a WASM runtime that holds Bun's event loop alive while the background UPDATE is still in flight. Closing the database mid-write strands the write on a dead handle, and Bun never notices the process is supposed to exit. So now the CLI explicitly waits for the background write to finish before closing the database. On the rare pathological case where the wait itself takes more than 5 seconds (a future bug we haven't seen yet but want to defend against), we log a stderr warning naming the leak and force-exit cleanly. Daemons (`gbrain serve`, `gbrain serve --http`) are explicitly excluded from the force-exit so they stay running.
|
||||
|
||||
For #1340 (PGLite WASM init failing on older macOS + Bun 1.3.x), the error message now correctly identifies the root cause as Bun's vfs read-only mount rather than the unrelated macOS 26.3 WASM bug:
|
||||
|
||||
```text
|
||||
PGLite failed to initialize its WASM runtime.
|
||||
This looks like a Bun vfs issue: `/$$bunfs/root` is read-only on
|
||||
your system, so PGLite cannot extract its pglite.data WASM payload.
|
||||
Fix: `bun upgrade` (newer Bun mounts the vfs writable). If that
|
||||
does not help, run via Node: `node src/cli.ts` or install gbrain
|
||||
using the Node-based path. See #1340 for details.
|
||||
```
|
||||
|
||||
### The numbers that matter
|
||||
|
||||
| Scenario | Before v0.41.8.0 | After v0.41.8.0 |
|
||||
|---|---|---|
|
||||
| `gbrain search "x"` on PGLite, exit time | Never (hangs at ~95-98% CPU until SIGKILL) | <2s |
|
||||
| `gbrain query "x" --no-expand` on PGLite | Never exits | <2s |
|
||||
| `gbrain get <slug>` on PGLite | Never exits | <1s |
|
||||
| Scripted `gbrain search ... && echo OK` | `OK` never printed | `OK` printed |
|
||||
| `gbrain init --pglite` on macOS 12.7 + Bun 1.3.14 | Failed with misleading macOS 26.3 hint | Failed with correct bunfs/`bun upgrade` hint |
|
||||
| `gbrain sync` hang ([#1342](https://github.com/garrytan/gbrain/issues/1342)) | No diagnostic output before hang | Phase breadcrumbs name WHICH phase spun |
|
||||
| `gbrain serve --http` (daemon) | Stayed alive | Still stays alive (regression-tested) |
|
||||
|
||||
### Why we narrowed the force-exit
|
||||
|
||||
Two community PRs proposed competing fixes. [PR #1259](https://github.com/garrytan/gbrain/pull/1259) (jehoon, validated by @eloe, @bcallender, and @61tH0b) added the structural "wait for the background write to finish" pattern — the right approach, mirroring an existing fix we shipped for #1090. [PR #1337](https://github.com/garrytan/gbrain/pull/1337) (matt-dean-git) took a different approach: force-exit the process after `main()` returns for every non-daemon command. PR #1337 also reordered the disconnect to release the file lock before closing the database, and added a snapshot+early-null pattern to the disconnect itself.
|
||||
|
||||
We took the drain from #1259, took the snapshot+early-null pattern from #1337, and narrowed PR #1337's force-exit to fire ONLY when the drain timed out — not unconditionally for every command. Reason: an unconditional force-exit would mask every future fire-and-forget regression. The narrow version preserves the diagnostic stderr warn that names the leaking surface, AND guarantees the CLI exits even on the pathological path. Reviewed via [/plan-eng-review](https://github.com/garrytan/gstack) (9 decisions) + Codex outside-voice (13 findings, all folded in). We also kept the original close-then-release disconnect order; PR #1337's swap to release-then-close would have widened the window where a sibling process could connect to a still-closing brain.
|
||||
|
||||
### Things to watch
|
||||
|
||||
- The `[gbrain phase] sync.<name>` breadcrumbs now print to stderr at four new boundaries (`sync.resolve_repo`, `sync.load_active_pack`, `sync.validate_repo_state`, `sync.detect_head`). If you parse gbrain stderr in a script, the new lines mirror the existing `[gbrain phase] sync.git_pull start/done` pattern from v0.28.1.
|
||||
- If you ever see `[last-retrieved] drain timed out after 5000ms; N writes still pending` on stderr, that's a defense-in-depth signal — it means a tracked background write took longer than 5 seconds. It's safe to ignore (the CLI still exits cleanly), but please file an issue with the pending count and the command you ran. This is how we'd find the next bug class in this surface.
|
||||
- #1342 (`gbrain sync` hang after schema v89→v92) is NOT fixed in this release. It's a single-reporter bug with a pure-JS infinite-loop shape (per `sample <pid>`) that doesn't match any of the hypotheses we ruled out. We've filed it as a follow-up investigation in [TODOS.md](TODOS.md) with concrete diagnostic next steps. If you hit it, please attach a `bun --inspect-brk` stack — the new breadcrumbs will name which phase to look at.
|
||||
|
||||
## To take advantage of v0.41.8.0
|
||||
|
||||
`gbrain upgrade` should do this automatically. If you were hitting #1247/#1269/#1290 before, no manual action is required — the fix is in the binary.
|
||||
|
||||
1. **Run upgrade:**
|
||||
```bash
|
||||
gbrain upgrade
|
||||
```
|
||||
2. **Verify the CLI exits cleanly:**
|
||||
```bash
|
||||
time gbrain search "test" --limit 3
|
||||
echo "EXIT=$?"
|
||||
```
|
||||
`EXIT=0` and a wall time under 2 seconds (after the first cold-start) means the fix landed. Pre-fix you would see no exit at all.
|
||||
3. **If you hit #1340 on older macOS + Bun:** run `bun upgrade` and retry. If that doesn't help, install gbrain via Node instead of via the Bun-compiled binary.
|
||||
4. **If any step fails or you see new hangs**, please file an issue:
|
||||
https://github.com/garrytan/gbrain/issues with the command you ran, your platform (macOS / Linux + Bun version), and whether you see the `[last-retrieved] drain timed out` stderr line.
|
||||
|
||||
### Itemized changes
|
||||
|
||||
#### Search/query/get hang fix (#1247, #1269, #1290)
|
||||
|
||||
- `src/core/last-retrieved.ts` — new `awaitPendingLastRetrievedWrites(timeoutMs?)` drain helper with bounded 5s `Promise.race` timeout. Tracks every `bumpLastRetrievedAt` IIFE promise in a module-scoped `Set<Promise<unknown>>`; resolves once all settle. Returns `{outcome, pending}` so the caller can decide its fallback. Mirrors the existing v0.36.1.x `awaitPendingSearchCacheWrites` precedent from #1090, plus the timeout the original helper lacks (a symmetry retrofit is filed as a v0.41+ TODO).
|
||||
- `src/cli.ts` — awaits `awaitPendingLastRetrievedWrites()` unconditionally in the op-dispatch finally block, right after the existing search-cache drain. On `outcome === 'timeout'` AND `shouldForceExitAfterMain(argv)` (excludes `serve`), calls `process.exit(0)` AFTER `engine.disconnect()` completes. Per-op-name gating was deliberately NOT applied — PR #1259's original literal-name check would have left `search` and `get_page` exposed.
|
||||
- `src/core/pglite-engine.ts:disconnect` — snapshot+early-null pattern (snapshot `_db`/`_lock` refs and null instance fields up front so concurrent `connect()` cannot observe a partial mid-close state) wrapped in try/finally so the file lock releases even if `db.close()` throws. KEEPS the original close-then-release order; PR #1337's release-then-close swap was rejected (widens the window where a sibling process could connect to a still-closing brain).
|
||||
|
||||
#### #1340 WASM init hint routing
|
||||
|
||||
- `src/core/pglite-engine.ts` — new exported `classifyPgliteInitError(message): 'bunfs' | 'macos-26-3' | 'unknown'` + `buildPgliteInitErrorMessage(verdict, original)`. The `bunfs` verdict matches the literal `$$bunfs` marker OR `ENOENT[\s\S]*pglite\.data` co-occurrence; surfaces a paste-ready `bun upgrade` hint + Node fallback. `macos-26-3` keeps the existing #223 link. Regex tightened per Codex eng-review finding #9 so generic `pglite.data` mentions don't false-trip the bunfs verdict.
|
||||
|
||||
#### #1342 diagnostic breadcrumbs
|
||||
|
||||
- `src/commands/sync.ts:performSyncInner` — four new stderr breadcrumbs at major phase boundaries: `sync.resolve_repo`, `sync.load_active_pack`, `sync.validate_repo_state` (when sourceId is set), `sync.detect_head`. Mirrors the pre-existing `sync.git_pull start/done` pattern. Doesn't fix #1342 but converts "hung with no output" into actionable diagnostic data.
|
||||
|
||||
#### Test coverage
|
||||
|
||||
- `test/last-retrieved.test.ts` (NEW) — 6 unit cases covering empty drain, single/multi-pending settle, throw-in-IIFE still settles, permanently-pending hits timeout within bound, empty pageIds not tracked.
|
||||
- `test/pglite-engine-disconnect.serial.test.ts` (NEW) — 5 lifecycle invariants: close-before-release ordering, snapshot observable inside close, lock-still-releases on close-throw, double-disconnect idempotency, reconnect-after-disconnect clean state.
|
||||
- `test/pglite-init-classifier.test.ts` (NEW) — 12 pure-function unit cases including the #1340 reporter's exact error string round-trip + a negative case asserting generic `pglite.data` mentions don't trip the bunfs verdict.
|
||||
- `test/e2e/pglite-cli-exit.serial.test.ts` (NEW IRON-RULE regression) — real `bun src/cli.ts` subprocess against a hermetic PGLite tempdir; asserts `search`, `query --no-expand`, `get` all exit 0 within 15s; daemon-survival case asserts `gbrain serve --http` stays alive past 3s (regression guard for the narrow force-exit not misclassifying 'serve' as a non-daemon).
|
||||
- `test/fix-wave-structural.test.ts` (EXTENDED) — behavioral-positioning assertion that the drain `await` appears textually BEFORE `engine.disconnect()` in cli.ts (survives variable-rename refactors), plus structural pins for the classifier exports and the sync breadcrumbs. Per D8 in the eng review (Codex finding #5), explicitly did NOT add a drift-guard counting `bumpLastRetrievedAt(` callers.
|
||||
|
||||
### Credit
|
||||
|
||||
PR [#1259](https://github.com/garrytan/gbrain/pull/1259) by jehoon supplied the structural drain pattern. PR [#1337](https://github.com/garrytan/gbrain/pull/1337) by matt-dean-git supplied the snapshot+early-null disconnect pattern and the force-exit idea we narrowed to fire only on the drain-timeout path. @eloe, @bcallender, and @61tH0b independently validated PR #1259's fix against real reproducers. Closing PRs land with `Co-Authored-By:` trailers on the merge commit.
|
||||
## [0.41.7.0] - 2026-05-24
|
||||
|
||||
**Your compact OpenClaw resolver actually works now.** If you've grown your agent past 200 skills and switched to the compact list-format resolver (`- **gift-advisor**: gift idea | birthday gift`) because the markdown-table version got unreadable, `gbrain doctor` used to silently report every single skill as unreachable. On a 306-skill agent, that was 238 FAIL errors on every doctor run, and the list-format resolver was effectively invisible to gbrain. v0.41.7.0 fixes that — the parser now reads both shapes natively, mixes them in one file, and `gbrain doctor` reports 0 errors on the same resolver that previously broke it.
|
||||
|
||||
@@ -456,6 +456,86 @@ cleanup can move each into the relevant area section.
|
||||
primitive exists (`embedding_columns` from v0.36.3); migration verb
|
||||
doesn't. (Embedding cluster.)
|
||||
|
||||
---
|
||||
## v0.41.8.0 PGLite hang follow-ups (v0.41+)
|
||||
|
||||
These were filed when v0.41.8.0 shipped the search/query/get hang fix
|
||||
(#1247/#1269/#1290) + WASM init classifier (#1340) + sync breadcrumbs.
|
||||
Three items deferred:
|
||||
|
||||
- [ ] **Investigate #1342 — `gbrain sync` hangs after schema v89→v92
|
||||
migration (PGLite, single reporter).** Repro shape: ~99% CPU in pure-JS
|
||||
JIT loop per `sample <pid>`, zero stderr output, reproduces with
|
||||
`--dry-run --no-pull`. Triggered after migrations 89→92 landed (v89
|
||||
facts_event_type_column, v90 contextual_retrieval_columns, v91
|
||||
pages_generation_trigger_and_bookmark, v92 sources_github_repo_index).
|
||||
Stale lock recovery from a `brain.pglite.broken-20260523-120636`
|
||||
rename suggests half-applied schema state.
|
||||
|
||||
**Ruled out** (per v0.41.8.0 plan-eng-review): NOT the
|
||||
`withRefreshingLock` heartbeat (user takes the legacy global-lock
|
||||
path — no setInterval); NOT the v91 trigger function (only fires on
|
||||
writes, user repros with `--dry-run`); NOT the two `while (true)`
|
||||
loops in `src/commands/sync.ts` (parallel worker pool + watch mode,
|
||||
neither in the user's invocation path).
|
||||
|
||||
**Next diagnostic steps**:
|
||||
1. Seed a fresh PGLite brain at schema v88 (snapshot the embedded
|
||||
schema blob at that version into a test fixture), apply migrations
|
||||
v89→v92, then run `performSync` with the user's exact flags and
|
||||
an 8s timeout. Repeat with a partial-v91 state (column landed,
|
||||
index didn't) to match the `brain.pglite.broken-...` clue.
|
||||
2. Run the reproducer under `bun --inspect-brk` and grab the V8
|
||||
stack at the spin point.
|
||||
3. Scan for `contextual_retrieval_mode IS NULL` paths in sync /
|
||||
`src/core/import-file.ts` — the v90 column may have an unbounded
|
||||
iteration somewhere when the per-source backfill kicks in.
|
||||
|
||||
**Reporter's config**: PGLite, `~/.gbrain/brain.pglite`,
|
||||
`ollama:nomic-embed-text` @ 768d, macOS 15.5, single 'default'
|
||||
source.
|
||||
|
||||
**Mitigation in v0.41.8.0**: phase breadcrumbs added to
|
||||
`performSyncInner` so the next #1342-shaped report names WHICH phase
|
||||
spun (resolve_repo / load_active_pack / validate_repo_state /
|
||||
detect_head). Doesn't fix; makes reports actionable.
|
||||
|
||||
- [ ] **Concurrent disconnect-during-connect race on `PGLiteEngine`
|
||||
(adversarial-review C6, v0.41.8.0).** The v0.41.8.0 snapshot+early-null
|
||||
pattern in `disconnect()` improves the partial-state race for the
|
||||
common case (single instance, sequential lifecycle), but a concurrent
|
||||
`connect()` and `disconnect()` on the same engine instance can still
|
||||
strand: `disconnect()` snapshots+nulls the lock and releases it while
|
||||
`connect()` is still in-flight (lock already acquired, awaiting
|
||||
`PGlite.create()`). When `connect()` resolves, `this._db` is assigned
|
||||
to a fresh handle but `this._lock` is null — engine is "connected"
|
||||
but holds no file lock; another process can acquire it concurrently.
|
||||
Unusual caller pattern in production (one instance per process,
|
||||
sequential lifecycle), but tests sometimes do this and the contract
|
||||
is undefined. Fix: serialize connect/disconnect with an instance-level
|
||||
mutex, or document the constraint and assert single-flight at the
|
||||
call site.
|
||||
|
||||
- [ ] **Retrofit `awaitPendingSearchCacheWrites` with the same bounded
|
||||
timeout v0.41.8.0 added to `awaitPendingLastRetrievedWrites`.** The
|
||||
v0.36.1.x #1090 fix at `src/core/search/hybrid.ts:36-45` shipped the
|
||||
drain pattern without a timeout; v0.41.8.0 added the timeout + warn
|
||||
pattern to the new `awaitPendingLastRetrievedWrites` helper. For
|
||||
symmetry (and to close the same future-failure mode in the cache
|
||||
drain), apply the same `Promise.race` + stderr warn pattern. ~15 LOC
|
||||
+ 2 unit cases. Pair this with the drain-helper extraction below.
|
||||
|
||||
- [ ] **Extract a shared `createDrainHelper<T>()` factory when a third
|
||||
fire-and-forget surface appears.** Per D4 in the v0.41.8.0 eng
|
||||
review: two surfaces is the threshold for noticing, three for
|
||||
extracting. `src/core/search/hybrid.ts:awaitPendingSearchCacheWrites`
|
||||
+ `src/core/last-retrieved.ts:awaitPendingLastRetrievedWrites` are
|
||||
the two surfaces today. When a third surface is added (or when the
|
||||
timeout-symmetry retrofit above lands and the duplication becomes
|
||||
load-bearing), extract a `src/core/drain-helper.ts` factory consumed
|
||||
by both call sites. Pair with the symmetry retrofit so they fire
|
||||
together as one focused refactor.
|
||||
|
||||
---
|
||||
## v0.41 Eval-loop wave follow-ups (v0.42+)
|
||||
|
||||
|
||||
+2
-341
File diff suppressed because one or more lines are too long
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gbrain",
|
||||
"version": "0.41.7.0",
|
||||
"version": "0.41.8.0",
|
||||
"description": "Postgres-native personal knowledge brain with hybrid RAG search",
|
||||
"type": "module",
|
||||
"main": "src/core/index.ts",
|
||||
|
||||
@@ -63,7 +63,6 @@ test/resolvers.test.ts
|
||||
test/scenarios.test.ts
|
||||
test/schema-bootstrap-coverage.test.ts
|
||||
test/search-limit.test.ts
|
||||
test/seed-pglite.test.ts
|
||||
test/skillpack-check.test.ts
|
||||
test/source-resolver.test.ts
|
||||
test/storage-sync.test.ts
|
||||
|
||||
@@ -52,8 +52,21 @@ fi
|
||||
|
||||
is_allowlisted() {
|
||||
local f="$1"
|
||||
[ -z "$ALLOWLIST" ] && return 1
|
||||
echo "$ALLOWLIST" | grep -qxF "$f"
|
||||
if [ -z "$ALLOWLIST" ]; then
|
||||
return 1
|
||||
fi
|
||||
# Use a pure-bash `case` whole-line match against the newline-delimited
|
||||
# allowlist instead of `echo | grep -qxF`. v0.41.8 CI flake (verify job
|
||||
# 77771356276): the grep pipe form occasionally failed to match the
|
||||
# first allowlist entry on Ubuntu 24.04 + bash 5 under
|
||||
# `bun run` + GNU `timeout` (couldn't reproduce on macOS bash 3.2 with
|
||||
# the same allowlist file content + lint script content + checkout
|
||||
# state). Pure-bash case is locale-free, pipe-free, subshell-free,
|
||||
# set-e-quirk-free, and ~100x faster on every call.
|
||||
case $'\n'"$ALLOWLIST"$'\n' in
|
||||
*$'\n'"$f"$'\n'*) return 0 ;;
|
||||
esac
|
||||
return 1
|
||||
}
|
||||
|
||||
# Find non-serial unit test files (excluding test/e2e). Portable across
|
||||
|
||||
@@ -113,6 +113,12 @@ export const SECTIONS: DocSection[] = [
|
||||
description:
|
||||
"Deploying the gbrain jobs worker: crontab + watchdog, inline --follow, systemd/Procfile/fly.toml, upgrade checklist.",
|
||||
path: "docs/guides/minions-deployment.md",
|
||||
// v0.41.8.0: 13KB deployment runbook. Web index entry stays;
|
||||
// single-fetch bundle drops it to keep under FULL_SIZE_BUDGET
|
||||
// (CLAUDE.md grew past 600KB once master's v0.41.2-v0.41.6 +
|
||||
// this wave's annotations landed). Operators read this once;
|
||||
// agents rarely need it in context.
|
||||
includeInFull: false,
|
||||
},
|
||||
{
|
||||
title: "docs/guides/quiet-hours.md",
|
||||
|
||||
+50
@@ -10,6 +10,8 @@ import type { AIGatewayConfig } from './core/ai/types.ts';
|
||||
import type { BrainEngine } from './core/engine.ts';
|
||||
import { operations, OperationError } from './core/operations.ts';
|
||||
import type { Operation, OperationContext } from './core/operations.ts';
|
||||
import { awaitPendingLastRetrievedWrites, type DrainOutcome } from './core/last-retrieved.ts';
|
||||
import { shouldForceExitAfterMain } from './core/cli-force-exit.ts';
|
||||
import { serializeMarkdown } from './core/markdown.ts';
|
||||
import { parseGlobalFlags, setCliOptions, getCliOptions } from './core/cli-options.ts';
|
||||
import type { CliOptions } from './core/cli-options.ts';
|
||||
@@ -180,6 +182,35 @@ async function main() {
|
||||
|
||||
// Local engine path (unchanged behavior for local installs).
|
||||
const engine = await connectEngine();
|
||||
// v0.41.8.0 (#1247, #1269, #1290): the search / query / get_page
|
||||
// op handlers fire-and-forget `bumpLastRetrievedAt` after returning
|
||||
// results. On PGLite that IIFE keeps Bun's event loop alive past
|
||||
// engine.disconnect(), hanging the CLI at ~95-98% CPU until SIGKILL.
|
||||
// Drain the fire-and-forget set BEFORE disconnect; force-exit only
|
||||
// if the drain itself times out (preserves stderr diagnostic signal
|
||||
// AND guarantees the CLI doesn't re-hang at the disconnect layer).
|
||||
//
|
||||
// Defense-in-depth (adversarial-review C13): `engine.disconnect()` itself
|
||||
// can hang on PGLite (db.close() or releaseLock racing OS-level FS state).
|
||||
// Install an unref'd setTimeout hard-exit fallback BEFORE entering the
|
||||
// try/catch/finally so a hung disconnect cannot defeat the force-exit
|
||||
// contract. Daemons (`serve`) are excluded so they stay alive.
|
||||
const DISCONNECT_HARD_DEADLINE_MS = 10_000;
|
||||
let forceExitTimer: ReturnType<typeof setTimeout> | undefined;
|
||||
if (shouldForceExitAfterMain()) {
|
||||
forceExitTimer = setTimeout(() => {
|
||||
console.warn(
|
||||
`[cli] engine.disconnect() did not return within ${DISCONNECT_HARD_DEADLINE_MS}ms — force-exiting`,
|
||||
);
|
||||
process.exit(0);
|
||||
}, DISCONNECT_HARD_DEADLINE_MS);
|
||||
// unref so the timer itself doesn't keep the event loop alive — only
|
||||
// the actual pending work (PGLite WASM handle) does. Without unref,
|
||||
// we'd block a clean exit by 10s on every successful CLI run.
|
||||
forceExitTimer.unref?.();
|
||||
}
|
||||
|
||||
let drainResult: DrainOutcome = { outcome: 'drained', pending: 0 };
|
||||
try {
|
||||
const ctx = await makeContext(engine, params);
|
||||
const rawResult = await op.handler(ctx, params);
|
||||
@@ -194,7 +225,16 @@ async function main() {
|
||||
const { awaitPendingSearchCacheWrites } = await import('./core/search/hybrid.ts');
|
||||
await awaitPendingSearchCacheWrites();
|
||||
}
|
||||
// Drain unconditionally for every op — empty-set fast-path is a
|
||||
// few microseconds. Not per-op-name gated: that was the original
|
||||
// PR #1259 mistake that left search and get_page exposed.
|
||||
drainResult = await awaitPendingLastRetrievedWrites();
|
||||
} catch (e: unknown) {
|
||||
// C9 fix: drain BEFORE process.exit so a successful op that throws
|
||||
// during stdout/format still gets its bumpLastRetrievedAt UPDATE
|
||||
// a chance to commit. Bounded by the drain's own 5s timeout; the
|
||||
// outer hard-exit timer above bounds the disconnect path.
|
||||
try { await awaitPendingLastRetrievedWrites(); } catch { /* best-effort */ }
|
||||
if (e instanceof OperationError) {
|
||||
console.error(`Error [${e.code}]: ${e.message}`);
|
||||
if (e.suggestion) console.error(` Fix: ${e.suggestion}`);
|
||||
@@ -204,9 +244,19 @@ async function main() {
|
||||
process.exit(1);
|
||||
} finally {
|
||||
await engine.disconnect();
|
||||
if (forceExitTimer) clearTimeout(forceExitTimer);
|
||||
// Narrow force-exit: only when the drain timed out AND we are NOT
|
||||
// running a daemon. The drain helper already stderr-warned with the
|
||||
// pending count, so the diagnostic signal is preserved. Without
|
||||
// this guard a hung underlying promise can still keep Bun's loop
|
||||
// alive past disconnect — Codex outside-voice finding #1.
|
||||
if (drainResult.outcome === 'timeout' && shouldForceExitAfterMain()) {
|
||||
process.exit(0);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
function hasHelpFlag(args: string[]): boolean {
|
||||
return args.includes('--help') || args.includes('-h');
|
||||
}
|
||||
|
||||
@@ -538,6 +538,13 @@ export async function performSync(engine: BrainEngine, opts: SyncOpts): Promise<
|
||||
}
|
||||
|
||||
async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<SyncResult> {
|
||||
// v0.41.8.0 (D9 / #1342): phase breadcrumbs. The #1342 reporter saw
|
||||
// ZERO stderr output before their sync hang, which made the bug
|
||||
// impossible to triage. Mirror the existing `[gbrain phase] sync.git_pull`
|
||||
// pattern at the major phase boundaries so the next #1342-shaped
|
||||
// report names WHICH phase spun. Doesn't fix #1342 but converts
|
||||
// "hung with no output" into actionable diagnostic data.
|
||||
serr(`[gbrain phase] sync.resolve_repo`);
|
||||
// Resolve repo path
|
||||
const repoPath = opts.repoPath || await readSyncAnchor(engine, opts.sourceId, 'repo_path');
|
||||
if (!repoPath) {
|
||||
@@ -547,6 +554,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
throw new Error(hint);
|
||||
}
|
||||
|
||||
serr(`[gbrain phase] sync.load_active_pack`);
|
||||
// v0.39 T1.5: load active pack ONCE at sync entry; pass to every per-file
|
||||
// importFile call below. Codex perf finding #7: per-file loadActivePack adds
|
||||
// disk/YAML/hash overhead × thousands of files. Best-effort: pack load
|
||||
@@ -571,6 +579,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
// we recover from missing/no-git/not-a-dir by re-cloning, refuse on
|
||||
// url-drift or corruption with structured hints.
|
||||
if (opts.sourceId) {
|
||||
serr(`[gbrain phase] sync.validate_repo_state`);
|
||||
const { validateRepoState } = await import('../core/git-remote.ts');
|
||||
const { recloneIfMissing } = await import('../core/sources-ops.ts');
|
||||
const cfgRows = await engine.executeRaw<{ config: unknown }>(
|
||||
@@ -618,6 +627,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
throw new Error(`Not a git repository: ${repoPath}. GBrain sync requires a git-initialized repo.`);
|
||||
}
|
||||
|
||||
serr(`[gbrain phase] sync.detect_head`);
|
||||
// Detect detached HEAD up front so the working-tree fallback fires for both
|
||||
// the default sync and `--no-pull` callers. Only the actual git pull is
|
||||
// gated on opts.noPull.
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
/**
|
||||
* v0.41.8.0 — narrow force-exit gate for the cli.ts op-dispatch finally.
|
||||
*
|
||||
* The cli.ts caller fires `process.exit(0)` ONLY when:
|
||||
* 1. The op-dispatch drain timed out (drainResult.outcome === 'timeout')
|
||||
* 2. AND this function returns true (i.e. the command is NOT a daemon)
|
||||
*
|
||||
* The function lives in its own module — not inline in cli.ts — so tests
|
||||
* can import + drive it without triggering cli.ts's top-level main() side
|
||||
* effect (cli.ts is a script entrypoint). Mirrors PR #1337's
|
||||
* `shouldForceExitAfterMain` guard, but narrower in scope: this wave
|
||||
* only force-exits after the drain timed out, NOT unconditionally for
|
||||
* every non-serve command.
|
||||
*
|
||||
* Daemon list is currently just `serve` (both stdio and HTTP forms use
|
||||
* the same command). If a future long-running command is added (e.g.
|
||||
* `gbrain watch` or `gbrain daemon`), add it here.
|
||||
*/
|
||||
|
||||
const DAEMON_COMMANDS: ReadonlySet<string> = new Set(['serve']);
|
||||
|
||||
export function shouldForceExitAfterMain(
|
||||
argv: string[] = process.argv.slice(2),
|
||||
): boolean {
|
||||
const command = argv.find((arg) => !arg.startsWith('-'));
|
||||
if (!command) return true;
|
||||
return !DAEMON_COMMANDS.has(command);
|
||||
}
|
||||
@@ -39,6 +39,92 @@ import { isUndefinedColumnError } from './utils.ts';
|
||||
let _trackRetrievalCache: { ts: number; enabled: boolean } | null = null;
|
||||
const TRACK_RETRIEVAL_CACHE_TTL_MS = 30_000;
|
||||
|
||||
/**
|
||||
* v0.41.8.0 — fire-and-forget tracking + bounded drain.
|
||||
*
|
||||
* Issues #1247, #1269, #1290: PGLite CLI commands printed search /
|
||||
* query / get_page output then hung at ~95-98% CPU until SIGKILL.
|
||||
* Root cause: the IIFE below races `engine.disconnect()`. PGLite's
|
||||
* WASM runtime kept Bun's event loop alive while the dangling
|
||||
* UPDATE settled, and disconnect closed the DB out from under it.
|
||||
*
|
||||
* Solution mirrors the v0.36.1.x #1090 fix at
|
||||
* `src/core/search/hybrid.ts:awaitPendingSearchCacheWrites`: track
|
||||
* every IIFE promise in this module-scoped Set, expose a drain that
|
||||
* resolves once all settle. The CLI awaits the drain before
|
||||
* `engine.disconnect()` so the WASM handle never closes mid-write.
|
||||
*
|
||||
* Bounded with a 5s timeout via Promise.race. If a future
|
||||
* fire-and-forget bug produces a permanently-pending promise, the
|
||||
* drain stderr-warns with the pending count and resolves so the CLI
|
||||
* can disconnect rather than re-creating the hang at this layer.
|
||||
* The cli.ts caller then falls back to a narrow `process.exit(0)`
|
||||
* (only on timeout, only for non-daemon commands) to guarantee
|
||||
* exit. The companion `awaitPendingSearchCacheWrites` retrofit is
|
||||
* filed as a v0.41+ TODO to keep both helpers symmetric.
|
||||
*/
|
||||
const pendingLastRetrievedWrites = new Set<Promise<unknown>>();
|
||||
const DRAIN_TIMEOUT_MS = 5_000;
|
||||
|
||||
export type DrainOutcome = { outcome: 'drained' | 'timeout'; pending: number };
|
||||
|
||||
export async function awaitPendingLastRetrievedWrites(
|
||||
timeoutMs: number = DRAIN_TIMEOUT_MS,
|
||||
): Promise<DrainOutcome> {
|
||||
if (pendingLastRetrievedWrites.size === 0) {
|
||||
return { outcome: 'drained', pending: 0 };
|
||||
}
|
||||
// Snapshot up front: if a new write appears after we start draining,
|
||||
// we deliberately don't await it. CLI flow guarantees op-dispatch
|
||||
// is complete before this call, so the set is effectively frozen.
|
||||
const snapshot = Array.from(pendingLastRetrievedWrites);
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeout = new Promise<'timeout'>((resolve) => {
|
||||
timer = setTimeout(() => resolve('timeout'), timeoutMs);
|
||||
});
|
||||
const drain = Promise.allSettled(snapshot).then(() => 'drained' as const);
|
||||
const outcome = await Promise.race([drain, timeout]);
|
||||
if (timer) clearTimeout(timer);
|
||||
if (outcome === 'timeout') {
|
||||
const pending = pendingLastRetrievedWrites.size;
|
||||
console.warn(
|
||||
`[last-retrieved] drain timed out after ${timeoutMs}ms; ` +
|
||||
`${pending} writes still pending`,
|
||||
);
|
||||
// Adversarial-review C1: in long-lived daemons (`gbrain serve`), a
|
||||
// timed-out IIFE stays in the set forever because its `.finally`
|
||||
// never fires. Repeated timeouts leak references without bound.
|
||||
// Drop this snapshot's tracked promises explicitly so the next
|
||||
// drain doesn't see ghosts. The IIFEs themselves keep running and
|
||||
// their results are still discarded; we just stop accumulating
|
||||
// references to forever-pending work.
|
||||
for (const p of snapshot) {
|
||||
pendingLastRetrievedWrites.delete(p);
|
||||
}
|
||||
return { outcome: 'timeout', pending };
|
||||
}
|
||||
return { outcome: 'drained', pending: 0 };
|
||||
}
|
||||
|
||||
function trackLastRetrievedWrite(promise: Promise<unknown>): void {
|
||||
pendingLastRetrievedWrites.add(promise);
|
||||
promise
|
||||
.finally(() => pendingLastRetrievedWrites.delete(promise))
|
||||
.catch(() => {
|
||||
/* swallow — IIFE already logged; .finally already removed */
|
||||
});
|
||||
}
|
||||
|
||||
/** Test seam — clears the pending set so each test starts clean. */
|
||||
export function _resetPendingLastRetrievedWritesForTests(): void {
|
||||
pendingLastRetrievedWrites.clear();
|
||||
}
|
||||
|
||||
/** Test seam — peek the current pending count. */
|
||||
export function _peekPendingLastRetrievedWritesForTests(): number {
|
||||
return pendingLastRetrievedWrites.size;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve `search.track_retrieval` config with a 30s in-process cache so
|
||||
* hot-path callers don't pay a SELECT per search. Default-on: missing
|
||||
@@ -77,8 +163,11 @@ export function _resetTrackRetrievalCacheForTests(): void {
|
||||
*/
|
||||
export function bumpLastRetrievedAt(engine: BrainEngine, pageIds: number[]): void {
|
||||
if (pageIds.length === 0) return;
|
||||
// Fire-and-forget on purpose. We deliberately do NOT return the promise.
|
||||
void (async () => {
|
||||
// Fire-and-forget on purpose for callers (MCP, internal). The CLI
|
||||
// path awaits the drain helper below before disconnecting, which
|
||||
// is how #1247/#1269/#1290 are fixed without exposing the IIFE
|
||||
// promise to every caller.
|
||||
const promise = (async () => {
|
||||
try {
|
||||
const enabled = await isTrackingEnabled(engine);
|
||||
if (!enabled) return;
|
||||
@@ -102,4 +191,5 @@ export function bumpLastRetrievedAt(engine: BrainEngine, pageIds: number[]): voi
|
||||
console.warn(`[last-retrieved] write-back failed (best-effort): ${msg}`);
|
||||
}
|
||||
})();
|
||||
trackLastRetrievedWrite(promise);
|
||||
}
|
||||
|
||||
+93
-18
@@ -127,6 +127,67 @@ export function computeSnapshotSchemaHash(
|
||||
return hash.digest('hex');
|
||||
}
|
||||
|
||||
/**
|
||||
* v0.41.8.0 (#1340) — classify PGLite.create() init failures so
|
||||
* the user-visible hint points at the right next step.
|
||||
*
|
||||
* `bunfs` — Bun's vfs ENOENT on older macOS where `/$$bunfs/root`
|
||||
* is read-only, so PGLite can't extract its `pglite.data` WASM
|
||||
* payload. Fix: `bun upgrade` (newer Bun versions mount the vfs
|
||||
* writable) or run via Node.
|
||||
*
|
||||
* `macos-26-3` — the pre-existing #223 hint signature (early macOS
|
||||
* 26.3 builds shipped a broken WASM runtime).
|
||||
*
|
||||
* `unknown` — falls through to a generic hint that still names the
|
||||
* doctor command and the most-common-cause link.
|
||||
*
|
||||
* Regex tightened per Codex eng-review finding #9: don't match
|
||||
* generic `pglite.data` substring (could fire on unrelated PGLite
|
||||
* errors). Match the literal `$$bunfs` marker OR ENOENT+pglite.data
|
||||
* co-occurrence.
|
||||
*/
|
||||
export type PgliteInitFailure = 'bunfs' | 'macos-26-3' | 'unknown';
|
||||
|
||||
export function classifyPgliteInitError(message: string): PgliteInitFailure {
|
||||
if (/\$\$bunfs|ENOENT[\s\S]*pglite\.data/i.test(message)) return 'bunfs';
|
||||
if (/abort.*runtime|macos.*26\.3|wasm.*runtime/i.test(message)) {
|
||||
return 'macos-26-3';
|
||||
}
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
export function buildPgliteInitErrorMessage(
|
||||
verdict: PgliteInitFailure,
|
||||
original: string,
|
||||
): string {
|
||||
const header = 'PGLite failed to initialize its WASM runtime.';
|
||||
let hint: string;
|
||||
switch (verdict) {
|
||||
case 'bunfs':
|
||||
hint =
|
||||
' This looks like a Bun vfs issue: `/$$bunfs/root` is read-only on\n' +
|
||||
' your system, so PGLite cannot extract its pglite.data WASM payload.\n' +
|
||||
' Fix: `bun upgrade` (newer Bun mounts the vfs writable). If that\n' +
|
||||
' does not help, run via Node: `node src/cli.ts` or install gbrain\n' +
|
||||
' using the Node-based path. See #1340 for details.';
|
||||
break;
|
||||
case 'macos-26-3':
|
||||
hint =
|
||||
' This is most commonly the macOS 26.3 WASM bug:\n' +
|
||||
' https://github.com/garrytan/gbrain/issues/223';
|
||||
break;
|
||||
case 'unknown':
|
||||
default:
|
||||
hint =
|
||||
' Most common cause: the macOS 26.3 WASM bug\n' +
|
||||
' (https://github.com/garrytan/gbrain/issues/223).\n' +
|
||||
' Run `gbrain doctor` for a full diagnosis.';
|
||||
break;
|
||||
}
|
||||
return `${header}\n${hint}\n Original error: ${original}`;
|
||||
}
|
||||
|
||||
export class PGLiteEngine implements BrainEngine {
|
||||
readonly kind = 'pglite' as const;
|
||||
private _db: PGLiteDB | null = null;
|
||||
@@ -173,18 +234,15 @@ export class PGLiteEngine implements BrainEngine {
|
||||
extensions: { vector, pg_trgm },
|
||||
});
|
||||
} catch (err) {
|
||||
// v0.13.1: any PGLite.create() failure becomes actionable. Most commonly
|
||||
// this is the macOS 26.3 WASM bug (#223). We deliberately do NOT suggest
|
||||
// "missing migrations" as a cause — migrations run AFTER create(), so a
|
||||
// create-time abort has nothing to do with them. Nest the original error
|
||||
// message so debugging isn't erased.
|
||||
// v0.13.1: any PGLite.create() failure becomes actionable. v0.41.8.0
|
||||
// (#1340): the previous error hint hardcoded the macOS 26.3 link, but
|
||||
// the same crash shape can come from Bun's vfs (`/$$bunfs/root` is
|
||||
// read-only on older macOS + Bun 1.3.x, so PGLite can't extract its
|
||||
// pglite.data WASM payload). Route the hint by failure shape so
|
||||
// users get the right next step.
|
||||
const original = err instanceof Error ? err.message : String(err);
|
||||
const wrapped = new Error(
|
||||
`PGLite failed to initialize its WASM runtime.\n` +
|
||||
` This is most commonly the macOS 26.3 WASM bug: https://github.com/garrytan/gbrain/issues/223\n` +
|
||||
` Run \`gbrain doctor\` for a full diagnosis.\n` +
|
||||
` Original error: ${original}`
|
||||
);
|
||||
const verdict = classifyPgliteInitError(original);
|
||||
const wrapped = new Error(buildPgliteInitErrorMessage(verdict, original));
|
||||
// Release the lock so a fresh process can try again; leaking the lock
|
||||
// here turns a recoverable init error into a stuck-brain state.
|
||||
if (this._lock?.acquired) {
|
||||
@@ -196,13 +254,30 @@ export class PGLiteEngine implements BrainEngine {
|
||||
}
|
||||
|
||||
async disconnect(): Promise<void> {
|
||||
if (this._db) {
|
||||
await this._db.close();
|
||||
this._db = null;
|
||||
}
|
||||
if (this._lock?.acquired) {
|
||||
await releaseLock(this._lock);
|
||||
this._lock = null;
|
||||
// v0.41.8.0: snapshot + early-null up front so a concurrent
|
||||
// `connect()` cannot observe `_db` pointing at a handle that's
|
||||
// mid-close (partial-state race). Closes the bug class PR #1337
|
||||
// originally surfaced.
|
||||
//
|
||||
// try/finally guarantees the file lock releases even if
|
||||
// `db.close()` throws. Pre-fix, a close-throw would leak the
|
||||
// lock and the next gbrain invocation would wedge waiting for it.
|
||||
// The pre-fix code happened to work because the close branch
|
||||
// ran first and the lock branch ran second only when close
|
||||
// didn't throw — moving to the snapshot pattern made the
|
||||
// try/finally explicitly necessary.
|
||||
const db = this._db;
|
||||
this._db = null;
|
||||
const lock = this._lock;
|
||||
this._lock = null;
|
||||
try {
|
||||
if (db) {
|
||||
await db.close();
|
||||
}
|
||||
} finally {
|
||||
if (lock?.acquired) {
|
||||
await releaseLock(lock);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* v0.41.8.0 — shouldForceExitAfterMain argv parsing unit tests.
|
||||
*
|
||||
* The function is the safety guard that protects daemons from the
|
||||
* narrow timeout-only force-exit in cli.ts. If it misclassifies
|
||||
* `gbrain serve` as a non-daemon (or any other intentional long-
|
||||
* runner that gets added later), the daemon dies after the first
|
||||
* request. Pure function; testable in isolation; deserves its own
|
||||
* unit cases beyond the e2e daemon-survival smoke.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { shouldForceExitAfterMain } from '../src/core/cli-force-exit.ts';
|
||||
|
||||
describe('shouldForceExitAfterMain — daemon survival gate', () => {
|
||||
test('returns false for bare `serve` (stdio daemon)', () => {
|
||||
expect(shouldForceExitAfterMain(['serve'])).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false for `serve --http --port 3131`', () => {
|
||||
expect(shouldForceExitAfterMain(['serve', '--http', '--port', '3131'])).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false even when global flags precede `serve`', () => {
|
||||
// `--quiet`, `--progress-json`, `--progress-interval=Nms` are stripped
|
||||
// by parseGlobalFlags BEFORE command dispatch — but shouldForceExitAfterMain
|
||||
// may be called with the raw argv. The .find skips flags, so the first
|
||||
// positional should resolve to the actual command regardless of global
|
||||
// flag position. This is the load-bearing case for `gbrain --quiet serve`.
|
||||
expect(shouldForceExitAfterMain(['--quiet', 'serve'])).toBe(false);
|
||||
expect(shouldForceExitAfterMain(['--progress-json', 'serve', '--http'])).toBe(false);
|
||||
expect(shouldForceExitAfterMain(['--progress-interval=500', '--quiet', 'serve'])).toBe(false);
|
||||
});
|
||||
|
||||
test('returns true for op commands (search/query/get)', () => {
|
||||
expect(shouldForceExitAfterMain(['search', 'foxtrot'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['query', 'where is foo'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['get', 'people/alice'])).toBe(true);
|
||||
});
|
||||
|
||||
test('returns true for non-daemon CLI commands', () => {
|
||||
expect(shouldForceExitAfterMain(['stats'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['doctor'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['sync', '--no-pull'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['embed', '--stale'])).toBe(true);
|
||||
});
|
||||
|
||||
test('returns true for empty argv (no command)', () => {
|
||||
// Defensive: with no positional, `--version` / `--help` would have already
|
||||
// exited. If we somehow land here with empty args, force-exit is safe
|
||||
// (no daemon is running).
|
||||
expect(shouldForceExitAfterMain([])).toBe(true);
|
||||
});
|
||||
|
||||
test('returns true for flag-only argv', () => {
|
||||
expect(shouldForceExitAfterMain(['--help'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['-h'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['--version'])).toBe(true);
|
||||
});
|
||||
|
||||
test('uses process.argv.slice(2) by default when called with no args', () => {
|
||||
// The default is just for the cli.ts call site convenience; the test
|
||||
// verifies the default works without crashing.
|
||||
expect(typeof shouldForceExitAfterMain()).toBe('boolean');
|
||||
});
|
||||
|
||||
test('substring match avoidance: `serves` is NOT `serve`', () => {
|
||||
// Future-proofing against a `gbrain serves-foo` subcommand being
|
||||
// misclassified as a daemon. Strict equality, not startsWith.
|
||||
expect(shouldForceExitAfterMain(['serves'])).toBe(true);
|
||||
expect(shouldForceExitAfterMain(['serve-cluster'])).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,278 @@
|
||||
/**
|
||||
* v0.41.8.0 — IRON-RULE regression for #1247, #1269, #1290.
|
||||
*
|
||||
* Pre-fix: `gbrain search`, `gbrain query`, `gbrain get` on PGLite
|
||||
* printed results then hung at ~95-98% CPU until SIGKILL.
|
||||
*
|
||||
* Post-fix: each command exits 0 within a few seconds.
|
||||
*
|
||||
* This test spawns the CLI as a real subprocess against a hermetic
|
||||
* GBRAIN_HOME tempdir, seeds a brain with 2 pages, runs each verb
|
||||
* with a hard timeout, and asserts exit 0. Without the drain helper
|
||||
* in cli.ts, every variant would time out.
|
||||
*
|
||||
* Bonus assertion: `gbrain serve --http` (a daemon) MUST stay alive
|
||||
* after the first request — the narrow force-exit guard added in
|
||||
* v0.41.8.0 is supposed to fire ONLY on op-dispatch drain timeout,
|
||||
* NEVER for daemons. This catches any future regression where the
|
||||
* force-exit gets broadened or the guard mis-recognizes 'serve'.
|
||||
*
|
||||
* Marked .serial because each test spawns a real bun subprocess that
|
||||
* cold-starts PGLite WASM (~5-10s wallclock). Running these in the
|
||||
* parallel pool would starve siblings of WASM init time.
|
||||
*
|
||||
* The reproducibility preconditions (per Codex eng-review #4) are
|
||||
* encoded explicitly:
|
||||
* - Seeded pages have `last_retrieved_at NULL` (verified via
|
||||
* fresh PGLite init — column starts NULL).
|
||||
* - At least one page id returned from each verb (search hits the
|
||||
* literal title; get hits the seeded slug).
|
||||
* - `search.track_retrieval` left unset → default-on path fires the
|
||||
* bumpLastRetrievedAt write that pre-fix would race disconnect.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { spawn, spawnSync } from 'child_process';
|
||||
import {
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
rmSync,
|
||||
writeFileSync,
|
||||
chmodSync,
|
||||
} from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join, resolve } from 'path';
|
||||
|
||||
const REPO_ROOT = resolve(import.meta.dir, '..', '..');
|
||||
const BIN_CACHE = join(REPO_ROOT, 'test', '.cache');
|
||||
const SHIM_PATH = join(BIN_CACHE, 'gbrain-pglite-exit-shim.sh');
|
||||
|
||||
beforeAll(() => {
|
||||
// Same shim pattern as claw-test e2e: bun --compile can't bundle
|
||||
// PGLite's pglite.data, so we delegate to `bun run src/cli.ts`.
|
||||
mkdirSync(BIN_CACHE, { recursive: true });
|
||||
const shim = `#!/bin/sh\nexec bun run "${join(REPO_ROOT, 'src', 'cli.ts')}" "$@"\n`;
|
||||
writeFileSync(SHIM_PATH, shim, 'utf-8');
|
||||
chmodSync(SHIM_PATH, 0o755);
|
||||
}, 10_000);
|
||||
|
||||
// Set up a fresh hermetic PGLite brain once per file; each verb
|
||||
// runs against the same brain to amortize cold-start.
|
||||
let tmpHome: string;
|
||||
let repoSourceDir: string;
|
||||
let runEnv: NodeJS.ProcessEnv;
|
||||
|
||||
beforeAll(() => {
|
||||
tmpHome = mkdtempSync(join(tmpdir(), 'gbrain-pglite-exit-'));
|
||||
repoSourceDir = mkdtempSync(join(tmpdir(), 'gbrain-pglite-exit-src-'));
|
||||
|
||||
// Seed a tiny git repo with 2 markdown pages so `gbrain sync` has
|
||||
// something to import. The pages contain the literal token 'foxtrot'
|
||||
// so search has a deterministic keyword hit.
|
||||
writeFileSync(
|
||||
join(repoSourceDir, 'alpha.md'),
|
||||
'---\ntitle: Alpha\n---\nThe quick brown foxtrot jumps over the lazy dog.\n',
|
||||
);
|
||||
writeFileSync(
|
||||
join(repoSourceDir, 'beta.md'),
|
||||
'---\ntitle: Beta\n---\nFoxtrot is a NATO phonetic letter F.\n',
|
||||
);
|
||||
|
||||
// git init + commit so sync has a HEAD to anchor against
|
||||
spawnSync('git', ['init', '-q', '-b', 'main'], { cwd: repoSourceDir });
|
||||
spawnSync('git', ['config', 'user.email', 'test@example.com'], { cwd: repoSourceDir });
|
||||
spawnSync('git', ['config', 'user.name', 'Test'], { cwd: repoSourceDir });
|
||||
spawnSync('git', ['add', '-A'], { cwd: repoSourceDir });
|
||||
spawnSync('git', ['commit', '-q', '-m', 'seed'], { cwd: repoSourceDir });
|
||||
|
||||
// Strip embedding-provider env vars so init doesn't refuse on the
|
||||
// multi-provider ambiguity check. We don't need embeddings — sync
|
||||
// runs with --no-embed below and search/get are keyword-only paths.
|
||||
runEnv = { ...process.env, GBRAIN_HOME: tmpHome };
|
||||
delete runEnv.VOYAGE_API_KEY;
|
||||
delete runEnv.ZEROENTROPY_API_KEY;
|
||||
delete runEnv.OPENAI_API_KEY;
|
||||
delete runEnv.ANTHROPIC_API_KEY;
|
||||
delete runEnv.GOOGLE_API_KEY;
|
||||
|
||||
const initResult = spawnSync(
|
||||
SHIM_PATH,
|
||||
['init', '--pglite', '--repo', repoSourceDir, '--no-embedding', '--yes'],
|
||||
{
|
||||
cwd: REPO_ROOT,
|
||||
env: runEnv,
|
||||
encoding: 'utf-8',
|
||||
timeout: 60_000,
|
||||
},
|
||||
);
|
||||
if (initResult.status !== 0) {
|
||||
throw new Error(
|
||||
`gbrain init failed (code=${initResult.status}):\n` +
|
||||
`STDOUT:\n${initResult.stdout}\n` +
|
||||
`STDERR:\n${initResult.stderr}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Sync to import the pages (no-embed: skip the embedding step so
|
||||
// the test doesn't need any provider key).
|
||||
const syncResult = spawnSync(
|
||||
SHIM_PATH,
|
||||
['sync', '--repo', repoSourceDir, '--no-pull', '--no-embed'],
|
||||
{
|
||||
cwd: REPO_ROOT,
|
||||
env: runEnv,
|
||||
encoding: 'utf-8',
|
||||
timeout: 60_000,
|
||||
},
|
||||
);
|
||||
if (syncResult.status !== 0) {
|
||||
throw new Error(
|
||||
`gbrain sync failed (code=${syncResult.status}):\n` +
|
||||
`STDOUT:\n${syncResult.stdout}\n` +
|
||||
`STDERR:\n${syncResult.stderr}`,
|
||||
);
|
||||
}
|
||||
}, 180_000);
|
||||
|
||||
afterAll(() => {
|
||||
try {
|
||||
rmSync(tmpHome, { recursive: true, force: true });
|
||||
rmSync(repoSourceDir, { recursive: true, force: true });
|
||||
} catch {
|
||||
/* best effort cleanup */
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Spawn the CLI with a wall-clock timeout. Returns exit code + output.
|
||||
* Without the v0.41.8.0 fix, the subprocess hangs forever and the
|
||||
* timeout would force-kill. The IRON-RULE assertion is "exit code 0
|
||||
* within timeoutMs."
|
||||
*/
|
||||
function runWithTimeout(
|
||||
args: string[],
|
||||
timeoutMs: number,
|
||||
): Promise<{ code: number | null; stdout: string; stderr: string; durationMs: number }> {
|
||||
return new Promise((resolveOut) => {
|
||||
const t0 = Date.now();
|
||||
const child = spawn(SHIM_PATH, args, {
|
||||
cwd: REPO_ROOT,
|
||||
env: runEnv,
|
||||
});
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
child.stdout.on('data', (d) => (stdout += d.toString()));
|
||||
child.stderr.on('data', (d) => (stderr += d.toString()));
|
||||
const timer = setTimeout(() => {
|
||||
try { child.kill('SIGKILL'); } catch { /* ignore */ }
|
||||
}, timeoutMs);
|
||||
child.on('exit', (code) => {
|
||||
clearTimeout(timer);
|
||||
resolveOut({ code, stdout, stderr, durationMs: Date.now() - t0 });
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
describe('v0.41.8.0 — PGLite CLI read commands exit cleanly (#1247/#1269/#1290)', () => {
|
||||
test('gbrain search "foxtrot" exits 0 within 15s', async () => {
|
||||
const { code, stdout, stderr, durationMs } = await runWithTimeout(
|
||||
['search', 'foxtrot', '--limit', '3'],
|
||||
15_000,
|
||||
);
|
||||
if (code !== 0) {
|
||||
throw new Error(
|
||||
`expected exit 0, got ${code}; duration=${durationMs}ms\n` +
|
||||
`STDOUT:\n${stdout}\nSTDERR:\n${stderr}`,
|
||||
);
|
||||
}
|
||||
expect(code).toBe(0);
|
||||
// Must have actually returned a hit — else bumpLastRetrievedAt
|
||||
// would have early-returned on empty pageIds and the bug wouldn't
|
||||
// have been exercised.
|
||||
expect(stdout.length).toBeGreaterThan(0);
|
||||
}, 30_000);
|
||||
|
||||
test('gbrain get returns a page body and exits 0 within 15s', async () => {
|
||||
const { code, stdout, stderr, durationMs } = await runWithTimeout(
|
||||
['get', 'alpha'],
|
||||
15_000,
|
||||
);
|
||||
if (code !== 0) {
|
||||
throw new Error(
|
||||
`expected exit 0, got ${code}; duration=${durationMs}ms\n` +
|
||||
`STDOUT:\n${stdout}\nSTDERR:\n${stderr}`,
|
||||
);
|
||||
}
|
||||
expect(code).toBe(0);
|
||||
expect(stdout).toContain('foxtrot');
|
||||
}, 30_000);
|
||||
|
||||
test('gbrain query without --no-expand exits 0 within 15s (no API key)', async () => {
|
||||
// Without an API key, expansion + vector branches degrade
|
||||
// gracefully. The op still runs the keyword path and returns
|
||||
// results. The DRAIN is what we're testing, not query quality.
|
||||
const { code, stderr, durationMs } = await runWithTimeout(
|
||||
['query', 'foxtrot', '--limit', '3', '--no-expand'],
|
||||
15_000,
|
||||
);
|
||||
if (code !== 0) {
|
||||
// Some test environments may fail query on missing embed key —
|
||||
// we tolerate that, but ALL of the rapid exit invariants still
|
||||
// apply: the process MUST exit, not hang. duration < 15s proves
|
||||
// it didn't hang; non-zero is acceptable.
|
||||
expect(durationMs).toBeLessThan(15_000);
|
||||
return;
|
||||
}
|
||||
expect(code).toBe(0);
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe('v0.41.8.0 — daemon survival (regression guard for narrow force-exit)', () => {
|
||||
test('gbrain serve --http stays alive past the timeout window', async () => {
|
||||
// Pick a likely-free ephemeral port. We're testing "still alive
|
||||
// 3 seconds after startup" — if the force-exit guard misfired
|
||||
// on 'serve', the process would die immediately after binding.
|
||||
const port = 31000 + Math.floor(Math.random() * 1000);
|
||||
const child = spawn(
|
||||
SHIM_PATH,
|
||||
['serve', '--http', '--port', String(port), '--token-ttl', '60'],
|
||||
{
|
||||
cwd: REPO_ROOT,
|
||||
env: runEnv,
|
||||
detached: false,
|
||||
},
|
||||
);
|
||||
|
||||
let exitedEarly = false;
|
||||
let earlyCode: number | null = null;
|
||||
child.on('exit', (code) => {
|
||||
exitedEarly = true;
|
||||
earlyCode = code;
|
||||
});
|
||||
|
||||
// Give the server 3 seconds. If the force-exit narrow guard is
|
||||
// working, the daemon stays alive past this window.
|
||||
await new Promise((r) => setTimeout(r, 3_000));
|
||||
|
||||
const wasAlive = !exitedEarly;
|
||||
try {
|
||||
child.kill('SIGTERM');
|
||||
// Give it a moment to clean up
|
||||
await new Promise((r) => setTimeout(r, 1_000));
|
||||
if (!exitedEarly) {
|
||||
try { child.kill('SIGKILL'); } catch { /* already dead */ }
|
||||
}
|
||||
} catch {
|
||||
/* already dead */
|
||||
}
|
||||
|
||||
if (!wasAlive) {
|
||||
throw new Error(
|
||||
`gbrain serve --http exited within 3s (code=${earlyCode}). ` +
|
||||
`If the narrow force-exit guard misclassified 'serve' as a ` +
|
||||
`non-daemon command, this is the regression.`,
|
||||
);
|
||||
}
|
||||
expect(wasAlive).toBe(true);
|
||||
}, 15_000);
|
||||
});
|
||||
@@ -20,8 +20,9 @@ import type { OperationContext } from '../src/core/operations.ts';
|
||||
import type { ChunkInput } from '../src/core/types.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let schemaDim: number;
|
||||
|
||||
function basisEmbedding(idx: number, dim = 1536): Float32Array {
|
||||
function basisEmbedding(idx: number, dim: number): Float32Array {
|
||||
const emb = new Float32Array(dim);
|
||||
emb[idx % dim] = 1.0;
|
||||
return emb;
|
||||
@@ -32,6 +33,18 @@ beforeAll(async () => {
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
|
||||
// v0.41.8.0: query the schema's actual embedding dim instead of
|
||||
// hardcoding 1536. Pre-fix, the test hardcoded 1536 but master's
|
||||
// v0.36.0 default changed to ZeroEntropy 1280d, AND a gateway-
|
||||
// configured local env may resolve to OpenAI 1536d. The dim is
|
||||
// resolved at initSchema() time from the configured gateway (with
|
||||
// DEFAULT_EMBEDDING_DIMENSIONS=1280 fallback). Either way, the seed
|
||||
// embedding's dim must match the column's dim, so we ask the column.
|
||||
const dimRows = await engine.executeRaw<{ atttypmod: number }>(
|
||||
"SELECT atttypmod FROM pg_attribute WHERE attrelid = 'content_chunks'::regclass AND attname = 'embedding'",
|
||||
);
|
||||
schemaDim = dimRows[0]?.atttypmod ?? 1280;
|
||||
|
||||
await engine.putPage('wiki/people/expert', {
|
||||
type: 'person',
|
||||
title: 'Expert',
|
||||
@@ -42,7 +55,7 @@ beforeAll(async () => {
|
||||
chunk_index: 0,
|
||||
chunk_text: 'Expert is the authority on widgets.',
|
||||
chunk_source: 'compiled_truth',
|
||||
embedding: basisEmbedding(7),
|
||||
embedding: basisEmbedding(7, schemaDim),
|
||||
token_count: 10,
|
||||
} as ChunkInput,
|
||||
]);
|
||||
|
||||
@@ -117,3 +117,74 @@ describe('v0.36.1.x #1124 — query --no-expand actually negates expand', () =>
|
||||
expect(src).toMatch(/params\[positiveKey\]\s*=\s*false/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('v0.41.8.0 #1247/#1269/#1290 — drain last-retrieved before CLI disconnect', () => {
|
||||
test('cli.ts imports awaitPendingLastRetrievedWrites', () => {
|
||||
const src = readFileSync('src/cli.ts', 'utf8');
|
||||
// Allow additional type-imports from the same module (e.g. `type DrainOutcome`)
|
||||
expect(src).toMatch(/import\s+\{[^}]*\bawaitPendingLastRetrievedWrites\b[^}]*\}\s*from\s+['"]\.\/core\/last-retrieved\.ts['"]/);
|
||||
});
|
||||
|
||||
test('last-retrieved.ts exports the drain + tracks promises in a module-scoped Set', () => {
|
||||
const src = readFileSync('src/core/last-retrieved.ts', 'utf8');
|
||||
expect(src).toMatch(/export async function awaitPendingLastRetrievedWrites/);
|
||||
expect(src).toMatch(/pendingLastRetrievedWrites\s*=\s*new\s+Set/);
|
||||
expect(src).toMatch(/pendingLastRetrievedWrites\.add\(promise\)/);
|
||||
// Per D5+D8: snapshot pattern (Codex finding #3) + bounded timeout
|
||||
expect(src).toMatch(/Promise\.race/);
|
||||
expect(src).toMatch(/drain timed out/);
|
||||
});
|
||||
|
||||
test('cli.ts behavioral positioning: drain appears BEFORE engine.disconnect in op-dispatch', () => {
|
||||
const src = readFileSync('src/cli.ts', 'utf8');
|
||||
// Per D5+D8: replaces the brittle literal-output regex from PR #1259
|
||||
// with a behavioral-positioning assertion. The drain CALL must appear
|
||||
// textually before the disconnect CALL in the local-engine path.
|
||||
// Match `await fn(` not bare names — bare names also appear in
|
||||
// comments and would false-match the comment ordering.
|
||||
const localPath = src.match(/\/\/ Local engine path \(unchanged behavior[\s\S]+?^\}/m);
|
||||
expect(localPath).not.toBeNull();
|
||||
const block = localPath![0];
|
||||
const drainCallRe = /await\s+awaitPendingLastRetrievedWrites\s*\(/;
|
||||
const disconnectCallRe = /await\s+engine\.disconnect\s*\(/;
|
||||
expect(block).toMatch(drainCallRe);
|
||||
expect(block).toMatch(disconnectCallRe);
|
||||
const drainMatch = block.match(drainCallRe);
|
||||
const disconnectMatch = block.match(disconnectCallRe);
|
||||
const drainIdx = block.indexOf(drainMatch![0]);
|
||||
const disconnectIdx = block.indexOf(disconnectMatch![0]);
|
||||
expect(drainIdx).toBeGreaterThan(-1);
|
||||
expect(disconnectIdx).toBeGreaterThan(-1);
|
||||
expect(drainIdx).toBeLessThan(disconnectIdx);
|
||||
});
|
||||
|
||||
test('cli.ts uses shouldForceExitAfterMain only on the timeout path', () => {
|
||||
const src = readFileSync('src/cli.ts', 'utf8');
|
||||
expect(src).toMatch(/import\s+\{\s*shouldForceExitAfterMain\s*\}\s*from\s+['"]\.\/core\/cli-force-exit\.ts['"]/);
|
||||
// The force-exit gate MUST be conditioned on drainResult.outcome ==='timeout'
|
||||
expect(src).toMatch(/drainResult\.outcome\s*===\s*['"]timeout['"]/);
|
||||
});
|
||||
|
||||
test('cli-force-exit.ts daemon guard excludes "serve"', () => {
|
||||
const src = readFileSync('src/core/cli-force-exit.ts', 'utf8');
|
||||
expect(src).toMatch(/export function shouldForceExitAfterMain/);
|
||||
expect(src).toMatch(/DAEMON_COMMANDS[\s\S]*serve/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('v0.41.8.0 #1340 — PGLite WASM init classifier', () => {
|
||||
test('pglite-engine.ts exports classifyPgliteInitError + buildPgliteInitErrorMessage', () => {
|
||||
const src = readFileSync('src/core/pglite-engine.ts', 'utf8');
|
||||
expect(src).toMatch(/export function classifyPgliteInitError/);
|
||||
expect(src).toMatch(/export function buildPgliteInitErrorMessage/);
|
||||
// Per Codex finding #9: regex tightened to $$bunfs OR ENOENT+pglite.data
|
||||
expect(src).toMatch(/\$\$bunfs/);
|
||||
expect(src).toMatch(/ENOENT/);
|
||||
});
|
||||
|
||||
test('pglite-engine.ts connect catch block routes through the classifier', () => {
|
||||
const src = readFileSync('src/core/pglite-engine.ts', 'utf8');
|
||||
expect(src).toMatch(/classifyPgliteInitError\(original\)/);
|
||||
expect(src).toMatch(/buildPgliteInitErrorMessage\(verdict, original\)/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
/**
|
||||
* v0.41.8.0 — drain helper for fire-and-forget last_retrieved_at writes.
|
||||
*
|
||||
* Closes #1247, #1269, #1290: PGLite CLI commands printed search /
|
||||
* query / get_page output then hung at ~95-98% CPU until SIGKILL.
|
||||
* The drain helper here is the structural fix paired with the
|
||||
* cli.ts narrow timeout-only force-exit guard.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
import {
|
||||
awaitPendingLastRetrievedWrites,
|
||||
bumpLastRetrievedAt,
|
||||
_resetTrackRetrievalCacheForTests,
|
||||
_resetPendingLastRetrievedWritesForTests,
|
||||
_peekPendingLastRetrievedWritesForTests,
|
||||
} from '../src/core/last-retrieved.ts';
|
||||
|
||||
// Minimal BrainEngine stub. The drain helper itself doesn't touch
|
||||
// the engine — only bumpLastRetrievedAt does — so we control behavior
|
||||
// through what `executeRaw` does.
|
||||
function makeStubEngine(opts?: {
|
||||
executeRaw?: (sql: string, params?: unknown[]) => Promise<unknown[]>;
|
||||
getConfig?: (key: string) => Promise<string | null>;
|
||||
}): BrainEngine {
|
||||
const engine = {
|
||||
kind: 'pglite' as const,
|
||||
executeRaw:
|
||||
opts?.executeRaw ??
|
||||
(async () => {
|
||||
return [];
|
||||
}),
|
||||
getConfig:
|
||||
opts?.getConfig ??
|
||||
(async () => {
|
||||
return null;
|
||||
}),
|
||||
};
|
||||
return engine as unknown as BrainEngine;
|
||||
}
|
||||
|
||||
describe('awaitPendingLastRetrievedWrites', () => {
|
||||
beforeEach(() => {
|
||||
_resetPendingLastRetrievedWritesForTests();
|
||||
_resetTrackRetrievalCacheForTests();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
_resetPendingLastRetrievedWritesForTests();
|
||||
_resetTrackRetrievalCacheForTests();
|
||||
});
|
||||
|
||||
test('empty set returns drained:0 immediately (fast-path)', async () => {
|
||||
const t0 = Date.now();
|
||||
const result = await awaitPendingLastRetrievedWrites();
|
||||
const dt = Date.now() - t0;
|
||||
expect(result).toEqual({ outcome: 'drained', pending: 0 });
|
||||
expect(dt).toBeLessThan(50);
|
||||
});
|
||||
|
||||
test('single tracked write completes, drain resolves cleanly', async () => {
|
||||
let resolved = false;
|
||||
const engine = makeStubEngine({
|
||||
executeRaw: async () => {
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
resolved = true;
|
||||
return [];
|
||||
},
|
||||
});
|
||||
bumpLastRetrievedAt(engine, [1, 2, 3]);
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(1);
|
||||
const result = await awaitPendingLastRetrievedWrites();
|
||||
expect(result).toEqual({ outcome: 'drained', pending: 0 });
|
||||
expect(resolved).toBe(true);
|
||||
// Promise removed from set on settle
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(0);
|
||||
});
|
||||
|
||||
test('multiple tracked writes all settled via allSettled', async () => {
|
||||
let count = 0;
|
||||
const engine = makeStubEngine({
|
||||
executeRaw: async () => {
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
count++;
|
||||
return [];
|
||||
},
|
||||
});
|
||||
bumpLastRetrievedAt(engine, [1]);
|
||||
bumpLastRetrievedAt(engine, [2]);
|
||||
bumpLastRetrievedAt(engine, [3]);
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(3);
|
||||
const result = await awaitPendingLastRetrievedWrites();
|
||||
expect(result.outcome).toBe('drained');
|
||||
expect(count).toBe(3);
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(0);
|
||||
});
|
||||
|
||||
test('throw inside IIFE still settles the promise; drain completes', async () => {
|
||||
const engine = makeStubEngine({
|
||||
executeRaw: async () => {
|
||||
throw new Error('synthetic-failure');
|
||||
},
|
||||
});
|
||||
bumpLastRetrievedAt(engine, [1]);
|
||||
// Brief tick so the IIFE has a chance to run and reject
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
const result = await awaitPendingLastRetrievedWrites();
|
||||
expect(result.outcome).toBe('drained');
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(0);
|
||||
});
|
||||
|
||||
test('permanently-pending promise returns timeout outcome with pending count', async () => {
|
||||
// Stage a manually-tracked promise that never resolves so the
|
||||
// drain hits the timeout path. We can't use bumpLastRetrievedAt
|
||||
// for this (its IIFE always settles via try/catch) — we need
|
||||
// the raw track API. Use the public API and a never-resolving
|
||||
// executeRaw stub.
|
||||
const neverEngine = makeStubEngine({
|
||||
executeRaw: () => new Promise<unknown[]>(() => {
|
||||
/* never */
|
||||
}),
|
||||
});
|
||||
bumpLastRetrievedAt(neverEngine, [1]);
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(1);
|
||||
|
||||
const t0 = Date.now();
|
||||
const result = await awaitPendingLastRetrievedWrites(100); // 100ms test timeout
|
||||
const dt = Date.now() - t0;
|
||||
|
||||
expect(result.outcome).toBe('timeout');
|
||||
expect(result.pending).toBe(1);
|
||||
// Should return within timeout + small buffer; not block forever
|
||||
expect(dt).toBeGreaterThanOrEqual(100);
|
||||
expect(dt).toBeLessThan(300);
|
||||
// C1 fix: snapshot's tracked promises ARE dropped from the set on
|
||||
// timeout so the next drain doesn't see ghosts (daemon leak guard).
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(0);
|
||||
});
|
||||
|
||||
test('bumpLastRetrievedAt with empty pageIds does not track a promise', async () => {
|
||||
const engine = makeStubEngine();
|
||||
bumpLastRetrievedAt(engine, []);
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(0);
|
||||
const result = await awaitPendingLastRetrievedWrites();
|
||||
expect(result).toEqual({ outcome: 'drained', pending: 0 });
|
||||
});
|
||||
|
||||
test('C1 fix: second drain after timeout is clean (daemon leak guard)', async () => {
|
||||
// Adversarial-review C1: in `gbrain serve` (long-lived), a timed-out
|
||||
// IIFE used to stay tracked forever because its `.finally` never
|
||||
// fires. Repeated timeouts would leak references without bound.
|
||||
// After the timeout, the next drain MUST see an empty set and
|
||||
// return immediately rather than re-timing-out on the same ghost.
|
||||
const neverEngine = makeStubEngine({
|
||||
executeRaw: () => new Promise<unknown[]>(() => {
|
||||
/* never */
|
||||
}),
|
||||
});
|
||||
bumpLastRetrievedAt(neverEngine, [1]);
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(1);
|
||||
|
||||
const first = await awaitPendingLastRetrievedWrites(100);
|
||||
expect(first.outcome).toBe('timeout');
|
||||
expect(_peekPendingLastRetrievedWritesForTests()).toBe(0);
|
||||
|
||||
// Second drain with no new writes returns immediately.
|
||||
const t0 = Date.now();
|
||||
const second = await awaitPendingLastRetrievedWrites(100);
|
||||
const dt = Date.now() - t0;
|
||||
expect(second).toEqual({ outcome: 'drained', pending: 0 });
|
||||
expect(dt).toBeLessThan(50);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,221 @@
|
||||
/**
|
||||
* v0.41.8.0 — PGLiteEngine.disconnect() lifecycle regression tests.
|
||||
*
|
||||
* Pins the invariants the v0.41.8.0 hang fix wave depends on:
|
||||
*
|
||||
* 1. ORDERING: `db.close()` is called BEFORE the file lock is
|
||||
* released. A sibling process must not be able to acquire the
|
||||
* lock and try to connect to a still-closing brain. PR #1337's
|
||||
* original diff swapped this to release-then-close — we
|
||||
* explicitly REJECTED that ordering. This test fails if a
|
||||
* future maintainer reads the PR and applies the swap.
|
||||
*
|
||||
* 2. SNAPSHOT + EARLY-NULL: `this._db` is nulled BEFORE awaiting
|
||||
* `close()`, so a concurrent `connect()` cannot observe a
|
||||
* partial mid-close state. PR #1337's load-bearing contribution
|
||||
* that we DID take.
|
||||
*
|
||||
* 3. LOCK LEAK GUARD: if `db.close()` throws, the file lock STILL
|
||||
* releases. Codex outside-voice finding #7 in the eng review:
|
||||
* without try/finally, a close-throw would wedge every next
|
||||
* gbrain invocation on the stale lock.
|
||||
*
|
||||
* 4. IDEMPOTENCY: calling disconnect() twice is a clean no-op on
|
||||
* the second call (no throw, no double-close attempt).
|
||||
*
|
||||
* 5. DOUBLE-DISCONNECT THEN CONNECT: after disconnect, a fresh
|
||||
* connect() sees clean state and succeeds.
|
||||
*
|
||||
* Marked .serial because PGLite WASM cold-start dominates wallclock
|
||||
* for fresh-engine-per-test cases — running these in the parallel
|
||||
* shard pool would starve other PGLite tests of cold-start time.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { mkdtempSync, rmSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
|
||||
function newTempDataDir(): string {
|
||||
return mkdtempSync(join(tmpdir(), 'gbrain-disconnect-test-'));
|
||||
}
|
||||
|
||||
describe('PGLiteEngine.disconnect() — v0.41.8.0 lifecycle invariants', () => {
|
||||
test('ORDERING: db.close() is called BEFORE releaseLock()', async () => {
|
||||
const dataDir = newTempDataDir();
|
||||
try {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({ database_path: dataDir });
|
||||
await engine.initSchema();
|
||||
|
||||
// Record the actual call order. We spy by replacing the db
|
||||
// handle's close + the lock handle's release with timestamped
|
||||
// wrappers.
|
||||
const calls: string[] = [];
|
||||
const eng = engine as unknown as {
|
||||
_db: { close: () => Promise<void> } | null;
|
||||
_lock: { lockDir: string; acquired: boolean } | null;
|
||||
};
|
||||
|
||||
const realClose = eng._db!.close.bind(eng._db!);
|
||||
eng._db!.close = async () => {
|
||||
// Tiny delay so a flipped ordering would actually show up
|
||||
// (release-before-close would beat us if we returned instantly).
|
||||
await new Promise((r) => setTimeout(r, 10));
|
||||
calls.push('db.close');
|
||||
return realClose();
|
||||
};
|
||||
|
||||
// releaseLock is module-level in pglite-lock.ts — to spy we have
|
||||
// to swap the lock object's `acquired` flag detection won't
|
||||
// route through us. Easier: monkey-patch by replacing the lock
|
||||
// ref with one whose presence forces releaseLock to no-op (so
|
||||
// we just measure that the close ran during disconnect and that
|
||||
// the no-op happened in the same call).
|
||||
//
|
||||
// For the ORDERING test specifically, we wrap close and
|
||||
// measure that the lockDir mkdir is still present immediately
|
||||
// before close runs and gone after disconnect returns. The
|
||||
// lockDir's existence is observable on disk.
|
||||
const { existsSync } = await import('fs');
|
||||
const lockDir = eng._lock!.lockDir;
|
||||
expect(existsSync(lockDir)).toBe(true);
|
||||
|
||||
// Spy on the lock-release moment by polling lockDir existence
|
||||
// from another timer: when close completes, the lock should
|
||||
// STILL be present (close-then-release contract).
|
||||
let lockStillPresentAtCloseFinish = false;
|
||||
const origClose = eng._db!.close;
|
||||
eng._db!.close = async () => {
|
||||
await origClose();
|
||||
// Right after close resolves, the lock has NOT yet been
|
||||
// released (the finally branch hasn't run yet). Check
|
||||
// synchronously before yielding the event loop again.
|
||||
lockStillPresentAtCloseFinish = existsSync(lockDir);
|
||||
};
|
||||
|
||||
await engine.disconnect();
|
||||
|
||||
expect(calls).toContain('db.close');
|
||||
expect(lockStillPresentAtCloseFinish).toBe(true);
|
||||
expect(existsSync(lockDir)).toBe(false);
|
||||
} finally {
|
||||
rmSync(dataDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('SNAPSHOT + EARLY-NULL: _db is nulled before await close', async () => {
|
||||
const dataDir = newTempDataDir();
|
||||
try {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({ database_path: dataDir });
|
||||
await engine.initSchema();
|
||||
|
||||
const eng = engine as unknown as {
|
||||
_db: { close: () => Promise<void> } | null;
|
||||
};
|
||||
|
||||
let dbWasNullWhenCloseRan = false;
|
||||
const realClose = eng._db!.close.bind(eng._db!);
|
||||
eng._db!.close = async () => {
|
||||
// Inside close, the engine's _db field should ALREADY be null
|
||||
// (snapshot pattern). If it's not, the partial-state race is
|
||||
// back.
|
||||
dbWasNullWhenCloseRan = eng._db === null;
|
||||
return realClose();
|
||||
};
|
||||
|
||||
await engine.disconnect();
|
||||
expect(dbWasNullWhenCloseRan).toBe(true);
|
||||
} finally {
|
||||
rmSync(dataDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('LOCK LEAK GUARD: if db.close() throws, lock still releases', async () => {
|
||||
const dataDir = newTempDataDir();
|
||||
try {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({ database_path: dataDir });
|
||||
await engine.initSchema();
|
||||
|
||||
const eng = engine as unknown as {
|
||||
_db: { close: () => Promise<void> } | null;
|
||||
_lock: { lockDir: string; acquired: boolean } | null;
|
||||
};
|
||||
|
||||
const { existsSync } = await import('fs');
|
||||
const lockDir = eng._lock!.lockDir;
|
||||
expect(existsSync(lockDir)).toBe(true);
|
||||
|
||||
// Force close to throw. The lock MUST still release.
|
||||
eng._db!.close = async () => {
|
||||
throw new Error('synthetic close failure');
|
||||
};
|
||||
|
||||
// The throw will propagate out of disconnect — that's fine.
|
||||
// The contract is "lock releases regardless."
|
||||
let threw = false;
|
||||
try {
|
||||
await engine.disconnect();
|
||||
} catch (e) {
|
||||
threw = true;
|
||||
expect(e instanceof Error && e.message).toContain('synthetic close failure');
|
||||
}
|
||||
expect(threw).toBe(true);
|
||||
// CRITICAL: lock must be gone even though close threw.
|
||||
expect(existsSync(lockDir)).toBe(false);
|
||||
} finally {
|
||||
rmSync(dataDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('IDEMPOTENCY: double disconnect is a clean no-op on the second call', async () => {
|
||||
const dataDir = newTempDataDir();
|
||||
try {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({ database_path: dataDir });
|
||||
await engine.initSchema();
|
||||
|
||||
let closeCallCount = 0;
|
||||
const eng = engine as unknown as {
|
||||
_db: { close: () => Promise<void> } | null;
|
||||
};
|
||||
const realClose = eng._db!.close.bind(eng._db!);
|
||||
eng._db!.close = async () => {
|
||||
closeCallCount++;
|
||||
return realClose();
|
||||
};
|
||||
|
||||
await engine.disconnect();
|
||||
expect(closeCallCount).toBe(1);
|
||||
|
||||
// Second call: no throw, no second close
|
||||
await engine.disconnect();
|
||||
expect(closeCallCount).toBe(1);
|
||||
} finally {
|
||||
rmSync(dataDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('RECONNECT after disconnect sees clean state', async () => {
|
||||
const dataDir = newTempDataDir();
|
||||
try {
|
||||
const engine = new PGLiteEngine();
|
||||
await engine.connect({ database_path: dataDir });
|
||||
await engine.initSchema();
|
||||
await engine.disconnect();
|
||||
|
||||
// Same dataDir, fresh connect. Must succeed without lock contention.
|
||||
await engine.connect({ database_path: dataDir });
|
||||
await engine.initSchema();
|
||||
// Smoke: a SELECT 1 round-trip proves the new handle is alive.
|
||||
const result = await engine.executeRaw<{ ok: number }>('SELECT 1 AS ok');
|
||||
expect(result[0].ok).toBe(1);
|
||||
await engine.disconnect();
|
||||
} finally {
|
||||
rmSync(dataDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,105 @@
|
||||
/**
|
||||
* v0.41.8.0 (#1340) — PGLite init-error classifier + hint routing.
|
||||
*
|
||||
* Pure-function tests over the classifier + message builder. No
|
||||
* PGLite cold-start required. The classifier sits in front of the
|
||||
* connect() catch block and routes the user-visible hint by failure
|
||||
* shape so users on macOS 12.7.6 + Bun 1.3.14 (the actual #1340
|
||||
* environment) don't get pointed at the macOS 26.3 hint (#223) by
|
||||
* mistake.
|
||||
*
|
||||
* Codex eng-review finding #9: the regex must NOT match generic
|
||||
* `pglite.data` substrings — only the literal `$$bunfs` marker OR
|
||||
* the ENOENT+pglite.data co-occurrence that bun's vfs failure shows.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
classifyPgliteInitError,
|
||||
buildPgliteInitErrorMessage,
|
||||
} from '../src/core/pglite-engine.ts';
|
||||
|
||||
describe('classifyPgliteInitError', () => {
|
||||
test('bunfs verdict for the literal $$bunfs marker', () => {
|
||||
const msg = "ENOENT: no such file or directory, open '/$$bunfs/root/pglite.data'.";
|
||||
expect(classifyPgliteInitError(msg)).toBe('bunfs');
|
||||
});
|
||||
|
||||
test('bunfs verdict for ENOENT + pglite.data co-occurrence (no $$bunfs prefix)', () => {
|
||||
const msg = 'ENOENT: cannot open pglite.data: read-only file system';
|
||||
expect(classifyPgliteInitError(msg)).toBe('bunfs');
|
||||
});
|
||||
|
||||
test('macos-26-3 verdict for the existing #223 signature', () => {
|
||||
const msg = 'abort() called from wasm runtime on macOS 26.3 build';
|
||||
expect(classifyPgliteInitError(msg)).toBe('macos-26-3');
|
||||
});
|
||||
|
||||
test('unknown verdict for generic / unrecognized errors', () => {
|
||||
const msg = 'TypeError: cannot read property of undefined at PGlite.create';
|
||||
expect(classifyPgliteInitError(msg)).toBe('unknown');
|
||||
});
|
||||
|
||||
test('NEGATIVE: generic "pglite.data" mention WITHOUT ENOENT does not trip bunfs', () => {
|
||||
// Per Codex finding #9: the prior overbroad regex `/bunfs|pglite\.data/i`
|
||||
// would have classified this as bunfs. The tightened regex requires
|
||||
// the literal $$bunfs marker OR ENOENT+pglite.data co-occurrence.
|
||||
const msg = 'Failed to parse pglite.data manifest: invalid magic byte';
|
||||
expect(classifyPgliteInitError(msg)).toBe('unknown');
|
||||
});
|
||||
|
||||
test('case-insensitive matching on bunfs marker', () => {
|
||||
expect(classifyPgliteInitError('SYSCALL ENOENT on /$$BUNFS/root')).toBe('bunfs');
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildPgliteInitErrorMessage — hint routing', () => {
|
||||
const original = 'synthetic original error';
|
||||
|
||||
test('bunfs verdict surfaces bun upgrade hint AND original error', () => {
|
||||
const msg = buildPgliteInitErrorMessage('bunfs', original);
|
||||
expect(msg).toContain('bun upgrade');
|
||||
expect(msg).toContain('Bun vfs');
|
||||
expect(msg).toContain(original);
|
||||
// Must NOT redirect to the wrong issue
|
||||
expect(msg).not.toContain('issues/223');
|
||||
});
|
||||
|
||||
test('macos-26-3 verdict surfaces the #223 link AND original error', () => {
|
||||
const msg = buildPgliteInitErrorMessage('macos-26-3', original);
|
||||
expect(msg).toContain('https://github.com/garrytan/gbrain/issues/223');
|
||||
expect(msg).toContain('macOS 26.3');
|
||||
expect(msg).toContain(original);
|
||||
expect(msg).not.toContain('Bun vfs');
|
||||
});
|
||||
|
||||
test('unknown verdict surfaces the doctor + #223 fallback AND original error', () => {
|
||||
const msg = buildPgliteInitErrorMessage('unknown', original);
|
||||
expect(msg).toContain('gbrain doctor');
|
||||
expect(msg).toContain('issues/223');
|
||||
expect(msg).toContain(original);
|
||||
});
|
||||
|
||||
test('all verdicts produce the canonical header line', () => {
|
||||
for (const v of ['bunfs', 'macos-26-3', 'unknown'] as const) {
|
||||
const msg = buildPgliteInitErrorMessage(v, original);
|
||||
expect(msg.startsWith('PGLite failed to initialize its WASM runtime.')).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('#1340 reproducer — exact reporter error string maps to bunfs', () => {
|
||||
// This is the literal error string from the issue body.
|
||||
const reportError = `ENOENT: no such file or directory, open '/$$bunfs/root/pglite.data'.`;
|
||||
|
||||
test('classifier routes the reporter\'s error to bunfs', () => {
|
||||
expect(classifyPgliteInitError(reportError)).toBe('bunfs');
|
||||
});
|
||||
|
||||
test('user-visible message names bun upgrade, NOT macOS 26.3', () => {
|
||||
const verdict = classifyPgliteInitError(reportError);
|
||||
const msg = buildPgliteInitErrorMessage(verdict, reportError);
|
||||
expect(msg).toContain('bun upgrade');
|
||||
expect(msg).not.toMatch(/most commonly the macOS 26\.3/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user