diff --git a/CHANGELOG.md b/CHANGELOG.md index a620e693d..a6972e9a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ All notable changes to GBrain will be documented in this file. +## [0.42.42.0] - 2026-06-12 + +**`gbrain query` no longer pays a flat 10-second exit tax on managed Postgres behind a transaction-mode pooler — and CLI exit codes finally tell the truth on PGLite.** On deployments where the pooler holds sockets open past the bounded pool drain (gbrain#2084, a residual of gbrain#1972), every query printed its results and then sat for 10 seconds until the force-exit banner fired. The cause was two-layered: the hard-deadline timer was armed *before* the operation handler, so a multi-second search on a large brain burned the teardown budget (and any operation slower than 10 seconds was silently killed mid-run with exit 0 and truncated output); and the CLI never exited explicitly on success — it waited for Bun's event loop to drain, which a stuck pooler socket can hold open forever. + +The teardown contract now lives in one place: every cli.ts disconnect site runs a bounded background-work drain and a bounded disconnect under a backstop whose deadline is computed from the bounds it guards (so it fires only when something violated its own bound), then the process exits explicitly — after fencing stdout/stderr and holding a short aliveness window so piped output is delivered (Bun queues pipe writes in a native buffer that only drains while the process is alive). The most-used command in the CLI now exits in milliseconds-to-a-couple-seconds instead of ten. + +Along the way the wave fixed a deeper, silent bug: PGLite's WASM runtime writes its own status into `process.exitCode` at arbitrary points mid-run, which meant **every error exit on PGLite-engine brains has been reporting success (exit 0)** — scripts and agents keying on exit codes never saw failures. The CLI verdict now lives in a gbrain-owned channel that the WASM runtime cannot touch. + +### Fixed +- **The flat 10s teardown tax + force-exit banner on transaction-mode poolers (gbrain#2084).** Queries exit promptly; the banner now appears only when a teardown component genuinely violated its own bound. +- **Slow operations are no longer killed mid-run with a false success.** The teardown deadline starts at teardown, never before the operation handler — a 30-second sync or a deep query runs to completion. +- **Error exits on PGLite report exit 1, not 0.** Failed operations (e.g. `gbrain get `) now exit non-zero on every engine; the exit code reports the operation, not the cleanup. +- **Piped output survives the exit.** Output is fenced and given a delivery window before the process exits, on every routed exit path including the backstop (the truncation class from gbrain#1959). +- **`gbrain doctor` no longer leaks its connection pool when DB checks throw**, and `dream`, `doctor`, `ze-switch`, and the search dashboards route their dispatcher teardown through the same bounded path (closing a long-standing drain gap on the overnight-cron path). +- **Daemon safety with space-separated global flags.** `gbrain --timeout 30s serve` is recognized as the daemon it is — the exit gate resolves the command exactly the way dispatch does. + +### Added +- **`GBRAIN_TEARDOWN_DEADLINE_MS`** — env override for the teardown backstop deadline (incident escape hatch; the default is computed from the drain and pool bounds). +- **`GBRAIN_FLUSH_GRACE_MS`** — env override for the pre-exit output-delivery window (default 250ms on pipes, 0 on TTYs). Raise it when piping very large payloads into slow consumers; lower it for high-frequency scripted invocations that capture to files. + +### To take advantage of v0.42.42.0 +`gbrain upgrade`. No configuration needed. If your `gbrain query` has been printing results and then hanging ~10 seconds before a `force-exiting` banner, this release removes both the wait and the banner. If your scripts check gbrain exit codes on a PGLite brain, they will start seeing real failures — previously masked as exit 0 — so a wrapper that suddenly reports errors is the fix working, not a regression. ## [0.42.41.0] - 2026-06-11 **A correctness-and-reliability wave: your conversation facts survive a cycle, write-through stops polluting other repos, autopilot rides out a DB blip instead of crash-looping, and concurrent PGLite processes stop corrupting each other.** A triage of open reports surfaced six bugs with no fix yet plus a batch of community PRs; this ships them together, each with a regression test. @@ -16422,8 +16444,7 @@ The OAuth provider in `src/core/oauth-provider.ts` got a parallel hardening pass Smaller hardening: admin cookies set `Secure` when behind HTTPS or a public-URL proxy (F9), magic-link nonces are bounded by an LRU cap (F10), `/mcp` wraps `transport.handleRequest` in try/catch so SDK throws hit a JSON-RPC 500 instead of express's default HTML error page (F14), and OperationError + unexpected exceptions both route through the unified `buildError`/`serializeError` envelope (F15). DCR disable became a constructor option on the provider rather than a serve-http monkey-patch (F12 — cleanup, not security). To take advantage of v0.26.9 -============================ - +===================== `gbrain upgrade` is a one-step upgrade. There is no migration; all changes are application-layer. 1. **Upgrade.** `gbrain upgrade`. Confirm `gbrain --version` shows `0.26.9`. @@ -16557,8 +16578,7 @@ Both run at `--max-concurrency=1` after the parallel pass, same as the existing Wallclock observed: 74s on a Mac dev box (running `bun run test` with the new quarantines). Already at the v0.26.9 informational target. The full intra-file marker flip (with codemod + per-file `test.concurrent()`) lands in v0.26.9 and aims for the same ≤60s with pinned config. To take advantage of v0.26.7 -============================ - +===================== `gbrain upgrade` does nothing functional in this release — it ships test infrastructure, not user-facing code. But if you contribute tests: 1. **Run `bun run verify` before pushing.** The new `check-test-isolation.sh` runs alongside the privacy + jsonb + progress checks. Catches new env-mutation, mock.module, and PGLite-pattern violations before CI does. diff --git a/TODOS.md b/TODOS.md index cf8baa784..92c22f146 100644 --- a/TODOS.md +++ b/TODOS.md @@ -239,17 +239,41 @@ GSTACK REVIEW REPORT at can't desync per-engine, bounded against CLI-hang by a top-level forced cleanup. Do this BEFORE introducing any concurrent module-engine connect path. -- [ ] **P3 — `dream` + CLI_ONLY fall-through paths don't drain the facts / - last-retrieved queues before the owner disconnect.** The op-dispatch path - (`cli.ts:~282-314`) drains `getFactsQueue().drainPending()` + - `awaitPendingLastRetrievedWrites()` before `engine.disconnect()`; the `dream` - owner-disconnect (`cli.ts:~1164`) and the fall-through owner-disconnect - (`cli.ts:~1785`) do not. If the dream cycle ever enqueues a facts:absorb / - last-retrieved write that's still in flight at disconnect, the owner nulls the - singleton and the write throws "No database connection". Pre-existing (not - introduced by the #1471 ownership fix), surfaced by the Claude adversarial - review (F5). Fix: hoist the same drain-before-disconnect block the op-dispatch - path uses into a shared helper and call it on all three owner-disconnect sites. +- [x] **P3 — `dream` + CLI_ONLY fall-through paths don't drain the facts / + last-retrieved queues before the owner disconnect.** DONE in the #2084 fix: + `finishCliTeardown` (`src/core/cli-force-exit.ts`) is exactly the shared + drain-before-disconnect helper this item asked for, and ALL NINE cli.ts + disconnect sites route through it (op-dispatch, fall-through, dream, doctor + ×3, ze-switch, search dashboard, read-only timeout path). Structural guard: + no bare `await engine.disconnect()` remains in cli.ts + (`test/fix-wave-structural.test.ts` `#2084` describe). + +- [ ] **P2 — command-module `process.exit` sites bypass the #2084 teardown + contract.** Several CLI_ONLY command modules exit directly on their normal + paths (`doctor.ts` ~10 sites incl. its verdict exit, `dream.ts` ~23, + `ze-switch.ts` ~9, plus friction/claw-test/eval verdict exits in cli.ts) — + those exits preempt the call-site `finally`, so the background-work drain, + bounded disconnect, and `flushThenExit` grace are all skipped on those paths + (pre-existing class, NOT introduced by #2084; pre-fix the same exits skipped + the inline drains too). Consequences: `gbrain doctor --json | ` + keeps the #1959 truncation exposure; a dream path that exits mid-cycle + discards in-flight facts/search-cache writes. Fix shape: convert in-command + `process.exit(n)` to `setCliExitVerdict(n)` + return (the central seam + exits), or route them through a shared `exitCommand(n)` helper that runs + teardown first. Surfaced by the #2084 cross-model adversarial review (F2). + +- [ ] **P3 — opt-in whole-command wallclock cap (`GBRAIN_COMMAND_DEADLINE_MS`), + build ONLY on a real wedged-handler incident.** The #2084 fix deliberately + removed the blanket pre-handler 10s force-exit (it killed slow-legit ops with + exit 0 and truncated output); per-op deadlines (query-embed deadline, + `withTimeout` on read-only commands) own handler wallclock now, and + `connectEngine` hangs — the historically observed zombie class — were never + covered by the old timer anyway. If production ever shows a genuinely wedged + handler (trigger: a non-`serve` command alive >30min with no progress + output), add an opt-in env cap that exits NON-ZERO with a truthful banner. + Attach point: the `GBRAIN_TEARDOWN_DEADLINE_MS` / `computeTeardownDeadlineMs` + plumbing in `src/core/cli-force-exit.ts`. Do not build speculatively — + follow-up from the #2084 eng review (decision D2/D14). ## v0.42.x AI SDK v6 tool-schema fix follow-ups (#1782/#1764) Surfaced by the codex outside-voice pass during `/plan-eng-review` and diff --git a/VERSION b/VERSION index 06c56e14a..b7fef61ef 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.41.0 +0.42.42.0 \ No newline at end of file diff --git a/docs/TESTING.md b/docs/TESTING.md index a02ee8750..d39384b98 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -10,16 +10,16 @@ Seven test command tiers, each with a clear scope: | Command | What it runs | Wallclock | When to use | |---|---|---|---| | `bun run test` | Parallel unit-test fast loop. 8-shard fan-out via `scripts/run-unit-parallel.sh`, then a serial pass over `*.serial.test.ts`. Excludes `*.slow.test.ts` and `test/e2e/*`. No pre-checks, no typecheck. | ~85s on a Mac dev box (3650+ tests) | Inner edit loop. Default. | -| `bun run verify` | CI's authoritative pre-test gate set: `check:privacy && check:jsonb && check:progress && check:wasm && bun run typecheck`. The 4 checks `.github/workflows/test.yml` runs on shard 1 + typecheck. Single source of truth — CI literally calls `bun run verify`. | ~12s (wasm-compile dominates) | Before pushing; before `/ship`. | +| `bun run verify` | CI's authoritative pre-test gate set, fanned out in parallel by `scripts/run-verify-parallel.sh`: the full `check:*` battery (~30 checks — privacy, jsonb, progress, source-id, test-isolation, wasm, …) plus `bun run typecheck`. The `CHECKS` array in that script is the single source of truth — CI literally calls `bun run verify` in a dedicated job. | ~16s (parallel; typecheck dominates) | Before pushing; before `/ship`. | | `bun run test:full` | `verify && bun run test && bun run test:slow && [smart e2e]`. The local equivalent of "everything CI runs." Smart e2e: runs e2e only when `DATABASE_URL` is set; else loud skip notice to stderr. | ~3-5min depending on slow + e2e | Pre-merge sanity, before opening a PR. | | `bun run test:slow` | Just the `*.slow.test.ts` set (intentional cold-path correctness checks). | seconds-to-minutes | When touching slow-path code. | -| `bun run test:serial` | Just the `*.serial.test.ts` set (cross-file-contention quarantine; runs at `--max-concurrency=1`). | ~1s per quarantined file | Debugging a specific quarantined file. | +| `bun run test:serial` | Just the `*.serial.test.ts` set (cross-file-contention quarantine; one bun process per file for true module-registry isolation). | ~1s per quarantined file | Debugging a specific quarantined file. | | `bun run test:e2e` | Real Postgres E2E. Requires Docker + `DATABASE_URL`. Sequential. | ~5-10min | Pre-ship; nightly. | | `bun run check:all` | All 7 historical pre-checks (privacy + jsonb + progress + no-legacy-getconnection + trailing-newline + wasm + exports-count). Superset of `verify`. | ~10s | Local-only sweep. The 4 not in `verify` are nice-to-haves. | ### CI vs local: intentionally divergent file sets -- **CI matrix** (`.github/workflows/test.yml`) runs `scripts/test-shard.sh` 4-way, which uses FNV-1a hash bucketing and INCLUDES `*.slow.test.ts`. CI EXCLUDES `*.serial.test.ts` from the hash buckets and runs them on shard 1 via `bun run test:serial` at `--max-concurrency=1` — keeping serial files out of the hash buckets is what preserves the `mock.module` quarantine (top-level mocks in serial files would otherwise leak into the parallel files they share a shard process with). CI is the ground truth for "did everything pass." +- **CI matrix** (`.github/workflows/test.yml`) runs `scripts/test-shard.sh` across 10 matrix shards partitioned by weight-aware LPT bin-packing (`scripts/sharding.ts`) and INCLUDES `*.slow.test.ts` (the two outlier slow files run as dedicated jobs alongside the matrix). CI EXCLUDES `*.serial.test.ts` from the shards and runs them in a dedicated job via `bun run test:serial`, one bun process per file — keeping serial files out of the shard processes is what preserves the `mock.module` quarantine (a top-level mock in one file leaks into every other file sharing its process). `bun run verify` gets its own job too. CI is the ground truth for "did everything pass." - **Local fast loop** (`scripts/run-unit-shard.sh` via the parallel wrapper) uses round-robin-by-index sharding and EXCLUDES `*.slow.test.ts` AND `*.serial.test.ts`. Local trades coverage for inner-loop speed; CI catches what local skips. This divergence is intentional. Don't try to make them equal — the two scripts deliberately solve different problems. The regression test at `test/scripts/run-unit-shard.test.ts` pins what the local fast loop should and shouldn't include. @@ -39,7 +39,7 @@ If a shard wedges (per-shard `GBRAIN_TEST_SHARD_TIMEOUT` cap, default 600s), the - `*.test.ts` → fast loop (parallel 8-shard fan-out). - `*.slow.test.ts` → run via `bun run test:slow` only (intentional cold-path tests; would dominate the fast loop's wallclock). -- `*.serial.test.ts` → run via `bun run test:serial` after the parallel pass completes; uses `--max-concurrency=1`. Quarantine for tests that share file-wide state and race when run alongside other files in the same `bun test` process. Currently: `test/brain-registry.serial.test.ts`, `test/reconcile-links.serial.test.ts`, `test/core/cycle.serial.test.ts`, `test/embed.serial.test.ts` (the latter two use `mock.module(...)` which leaks across files in the shard process). **Do not put the parallelism back on a serial file unless you've fixed the contention root cause** (it just re-introduces the flake). +- `*.serial.test.ts` → run via `bun run test:serial` after the parallel pass completes; one bun process per file (`--max-concurrency=1` within a shared process is not enough — the module registry still leaks `mock.module`). Quarantine for tests that share file-wide state and race when run alongside other files in the same `bun test` process. Several dozen files, discovered by the `*.serial.test.ts` glob — no list to maintain. Typical residents: `mock.module(...)` users (top-level mocks leak across files in a shard process, e.g. `test/embed.serial.test.ts`), env-coupled files (e.g. `test/brain-registry.serial.test.ts`), and process-lifecycle suites that assert on `process.exitCode` (e.g. `test/pglite-engine-disconnect.serial.test.ts`). **Do not put the parallelism back on a serial file unless you've fixed the contention root cause** (it just re-introduces the flake). - `test/e2e/*.test.ts` → real-Postgres E2E. Skipped when `DATABASE_URL` is unset. - `tests/heavy/*.sh` → ops-shape shell scripts. Cost minutes per run; NOT in default `bun test`. Run via `bun run test:heavy` or scheduled nightly via `.github/workflows/heavy-tests.yml`. Examples: pg_upgrade matrix (boot legacy brain → walk to head), RSS budget gate (measure peak worker RSS vs committed baseline), read-latency-under-sync (p50/p95/p99 under concurrent writer load), sync lock regression (N concurrent syncs assert 1 winner + N-1 lock-busy + zero leaked `gbrain_cycle_locks` rows). See `tests/heavy/README.md` for when to add a script here vs `*.slow.test.ts`. Files prefixed with `_` (e.g. `tests/heavy/_build_legacy_fixtures.sh`) are helpers/libs invoked by sibling tests — the runner skips them. - `test/fuzz/*.test.ts` → property-based fuzz harness. Pure-validator targets in `pure-validators.test.ts` are guarded by `scripts/check-fuzz-purity.sh` (in `bun run verify`), which `bun build --target=bun` bundles each target and greps the resulting bundle for banned transitive imports (`node:fs`, `node:child_process`, engine modules). Anything that fails the guard moves to `mixed-validators.test.ts` (still property-tested, but no purity guarantee) or `filesystem-validators.test.ts` (fs-backed, uses temp dirs). Fuzz tests run in the default `bun test` loop because they're fast (~3s for ~12 properties × 1000 runs each). @@ -111,7 +111,7 @@ Rename to `*.serial.test.ts` when: - The file is genuinely env-coupled (e.g. `gbrain-home-isolation.test.ts`, `claw-test-cli.test.ts`) — module-load env readers + ESM caching defeat dynamic-import-after-env tricks. - The file's tests intentionally share state across `it()` boundaries. -Quarantine count cap: 10 (informational). Beyond that, push back on the design. +The quarantine has grown to dozens of files — treat it as debt: every addition needs a reason from the list above, and prefer fixing the contention root cause when one exists. ### Unit test inventory @@ -123,6 +123,9 @@ Unit tests and what they cover: - `test/chunkers/recursive.test.ts` — chunking. - `test/parity.test.ts` — operations contract parity. - `test/cli.test.ts` — CLI structure. +- `test/cli-finish-teardown.test.ts` — the #2084 teardown contract: `computeTeardownDeadlineMs` formula/floor/live-registry scaling + `GBRAIN_TEARDOWN_DEADLINE_MS` override (garbage/zero/negative values fall back to the formula); `finishCliTeardown` clean path (drain BEFORE disconnect, no exit, no warn), backstop on hung drain or disconnect (honors an errored op's exit code), throwing drain/disconnect warned + swallowed; the gbrain-owned verdict channel is immune to PGLite WASM `process.exitCode` writes; `flushThenExit` unit coverage with mocked streams (exits once after both stream callbacks, non-TTY aliveness grace, blocked-pipe guard, EPIPE-safe, `GBRAIN_FLUSH_GRACE_MS` override). +- `test/flush-then-exit-harness.test.ts` — real spawned-Bun pipe semantics for `flushThenExit` (fixture: `test/fixtures/flush-then-exit-harness.ts`): a 4MB piped stdout payload arrives byte-complete with the exit code even with a late reader, small output survives exit with a concurrent reader, and the fence resolves promptly (wall time well under the guard + grace ceiling). +- `test/cli-should-force-exit.test.ts` — `shouldForceExitAfterMain` daemon-survival gate: `serve` (stdio and `--http`) never force-exits, including with preceding global flags; op commands / empty / flag-only argv do; the #2084 case that space-separated global-flag VALUES can't fake a command (`--timeout 30s serve` resolves to the `serve` daemon, not a `30s` command). - `test/config.test.ts` — config redaction. - `test/files.test.ts` — MIME/hash. - `test/import-file.test.ts` — import pipeline. @@ -219,6 +222,7 @@ E2E tests live in `test/e2e/` and run against real Postgres+pgvector (require `D - `test/e2e/sync.test.ts` — `--skip-failed` failure-loop test alongside happy-path tests: broken file → `performSync` returns `blocked_by_failures` with grouped breakdown → `performSync({skipFailed: true})` advances bookmark and returns `AcknowledgeResult` with code summary → second broken file → second cycle. Saves and restores the user's real `~/.gbrain/sync-failures.jsonl` so the test is hermetic. Asserts bookmark gating, JSONL state, dedup across paths, summary aggregation, and the literal doctor-rendering string format. - `test/e2e/upgrade.test.ts` — check-update against real GitHub API (network required). - `test/e2e/minions-shell-pglite.test.ts` — PGLite `--follow` inline shell-job path (in-memory, no `DATABASE_URL` required) — the path the minion-orchestrator skill documents for dev use. +- `test/e2e/pglite-cli-exit.serial.test.ts` — real spawned-CLI exit behavior on PGLite (in-memory, no `DATABASE_URL`): read commands (`search`/`get`/`query`) exit 0 promptly; CLI_ONLY `capture` exits clean and frees the single-writer lock; the `#2084` describes pin every swept disconnect site — a failed op exits 1 with the error on stderr, and the dashboard, read-only-timeout, doctor, and `dream --dry-run` paths all exit with no force-exit banner. - `test/e2e/openclaw-reference-compat.test.ts` — `check-resolvable` + `skillpack install` against a minimal AGENTS.md workspace fixture (`test/fixtures/openclaw-reference-minimal/`), regression guard for the OpenClaw deployment shape. - `test/e2e/search-swamp.test.ts` — reproduces the source-swamp case. Seeds a curated `originals/talks/article-outline-fat-code` page against two `/chat/` pages stuffed with the same multi-word phrase. Asserts the article wins keyword AND vector ranking, that `detail=high` lets the chat swamp re-surface, and that `source_id` passes through the two-stage CTE intact. PGLite in-memory. - `test/e2e/search-exclude.test.ts` — `test/` + `archive/` pages hidden by default, `include_slug_prefixes` opts back in, caller-supplied `exclude_slug_prefixes` adds to defaults. Both keyword and vector search paths. diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 3491e3d5c..667217659 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -11,7 +11,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/operations.ts` — Contract-first operation definitions (the foundation). Exports upload validators `validateUploadPath`, `validatePageSlug`, `validateFilename`, plus `matchesSlugAllowList(slug, prefixes)` (glob matcher: `/*` matches recursive children; bare `` matches exact only). `OperationContext.remote` is a REQUIRED field flagging untrusted callers; `OperationContext.allowedSlugPrefixes` is the trusted-workspace allow-list set by the dream cycle; `OperationContext.auth?: AuthInfo` is threaded through HTTP dispatch for scope enforcement in `serve-http.ts` before the op runs. `put_page` enforces: when `viaSubagent` and `allowedSlugPrefixes` is set, slug must match the allow-list; else the legacy `wiki/agents//...` namespace check applies. Auto-link skipped only when `remote=true && !trustedWorkspace`. Every `Operation` carries `scope?: 'read' | 'write' | 'admin'` + `localOnly?: boolean`; `sync_brain`, `file_upload`, `file_list`, `file_url` are `admin + localOnly` (rejected over HTTP). Four trust-boundary call sites (`put_page` allowlist, file_upload trust-narrowing, submit_job protected-name guard, auto-link skip) use FAIL-CLOSED semantics: `ctx.remote === false` for trusted-only sites, `ctx.remote !== false` for "untrust unless explicit-false" — anything not strictly `false` is treated as remote (closes the HTTP MCP shell-job RCE where a read+write OAuth token could submit `shell` jobs). `sourceScopeOpts(ctx)` encodes the source-scoped read precedence ladder — federated array (`ctx.auth.allowedSources`) wins over scalar (`ctx.sourceId`/`ctx.auth.sourceId`) over nothing; every read-side op handler routes through it so a source-bound OAuth client can't see neighboring sources via `search`/`query`/`list_pages`/`get_page`/`find_experts`/query's image path. `put_page`'s inline disk write-through is the shared `writePageThrough` helper (`src/core/write-through.ts`), ATOMIC via temp-sibling + rename so a crash or concurrent `gbrain sync` can't read a half-written `.md`; same helper backs `gbrain brainstorm/lsd --save`. Link provenance surface (#1941): `add_link` (`gbrain link`/`link-add`) + `remove_link` (`gbrain unlink`/`link-rm`) expose `link_source`/`link_type`; `add_link` rejects the reconciliation-managed built-ins via `MANAGED_LINK_SOURCES` (`markdown`/`frontmatter`/`mentions`/`wikilink-resolved`) and defaults omitted provenance to `'manual'` (the engine's own default stays `'markdown'` for internal callers); `list_link_sources` (`gbrain link-sources`, read) lists provenances via `sourceScopeOpts`. CLI aliases register through `cliHints.aliases` (collision-guarded in `src/cli.ts`). - `src/core/engine.ts` — Pluggable engine interface (BrainEngine). `clampSearchLimit(limit, default, cap)` takes an explicit cap so per-operation caps can be tighter than `MAX_SEARCH_LIMIT`. Exports `LinkBatchInput`/`TimelineBatchInput` for the bulk-insert API (`addLinksBatch`/`addTimelineEntriesBatch`). `readonly kind: 'postgres' | 'pglite'` discriminator lets `src/core/migrate.ts` and others branch without `instanceof` + dynamic imports. Methods: `batchLoadEmotionalInputs(slugs?)` (CTE-shaped read with per-table aggregates so page × N tags × M takes never produces N×M rows), `setEmotionalWeightBatch(rows)` (`UPDATE FROM unnest($1::text[],$2::text[],$3::real[])` composite-keyed on `(slug, source_id)`), `getRecentSalience(opts)`, `findAnomalies(opts)`. `PageFilters` has `sort?: 'updated_desc'|'updated_asc'|'created_desc'|'slug'` + `PAGE_SORT_SQL` whitelist consumed by both engines. `listAllPageRefs(): Promise>` ordered by `(source_id, slug)` — cheap cross-source enumeration replacing the `getAllSlugs()→getPage(slug)` N+1 (which silently defaulted to `source_id='default'`); parity across postgres-engine.ts + pglite-engine.ts; Pinned by `test/e2e/multi-source-bug-class.test.ts`. `SearchOpts`+`PageFilters` add `sourceIds?: string[]` (federated read axis; both engines apply `WHERE source_id = ANY($N::text[])` when set, preserve scalar `sourceId` fast path when unset); `traverseGraph(slug, depth, opts?)` and `traversePaths(slug, opts?)` accept `opts.sourceId`/`opts.sourceIds`. `traverseGraph` opts has `frontierCap?: number` (per-iteration recursive-CTE cap, approx per-BFS-layer); return type `Promise` for MCP wire stability; export `TraverseGraphOpts`; Postgres uses parenthesized `LIMIT N ORDER BY (slug, id)` inside the recursive term, PGLite mirrors with positional params; Pinned by `test/regressions/v0_36_frontier_cap.test.ts`. Phantom-redirect methods: `refreshPageBody(slug, sourceId, compiled_truth, timeline, content_hash)` narrow-UPDATEs three columns + updated_at, skipping soft-deleted rows (content_hash refresh required so `gbrain sync` sees the canonical as unchanged after fence merge); `migrateFactsToCanonical(phantomSlug, canonicalSlug, sourceId)` UPDATEs `entity_slug`+`source_markdown_slug` on every active fact row keyed on the phantom, preserving embedding/validUntil/kind/status/source_session/confidence; parity at `test/phantom-redirect-engine-parity.test.ts`. `getAdjacencyBoosts(pageIds): Promise>` powers the per-query graph-signals stage — one SQL query returning inbound-link counts among top-K plus a cross-source count (links from differing `source_id`); `COALESCE(p.source_id,'default')` null safety, `HAVING >= 1`, cross-source CASE-WHEN excludes the target's own source; parity SQL across both engines; `SearchResult` gains optional `base_score`, `backlink_boost`, `salience_boost`, `recency_boost`, `exact_match_boost`, `graph_adjacency_boost`, `graph_cross_source_boost`, `session_demote_factor`, `reranker_delta` + internal staging fields; Pinned by `test/e2e/graph-signals-engine.test.ts`. Two REQUIRED methods: `deletePages(slugs, {sourceId}): Promise` (single-batch primitive returning slugs actually deleted) and `resolveSlugsByPaths(paths, {sourceId}): Promise>` (batch path→slug lookup); `sourceId` REQUIRED on both at the type level (asymmetric with single-row `deletePage` which keeps optional/'default'); both short-circuit on empty input and throw when `> DELETE_BATCH_SIZE`. Embedding-signature stale-detection quartet: `countStaleChunks(opts?)` gains optional `signature?: string` widening the stale predicate from `embedding IS NULL` to ALSO include chunks whose JOINed page `embedding_signature IS NOT NULL AND <> $signature` (NULL signature is GRANDFATHERED, never counted; omit `signature` for the legacy NULL-only count); `sumStaleChunkChars(opts?: {sourceId?, signature?}): Promise` = `SUM(LENGTH(chunk_text))` over stale chunks (same predicate + embed_skip filter + optional sourceId scope), used by `gbrain sync --all` cost preview via `estimateCostFromChars`; `setPageEmbeddingSignature(slug, {sourceId?, signature})` stamps `pages.embedding_signature` after a page's chunks (re)embed, idempotent no-op when page absent; `invalidateStaleSignatureEmbeddings({signature, sourceId?}): Promise` NULLs `embedding`+`embedded_at` on every chunk whose page signature is set AND differs, returning the count, called BEFORE `listStaleChunks` so signature-drift pages flow through the NULL-embedding keyset cursor unchanged (NULL never invalidated). Widens `findOrphanPages(opts?: {sourceId?, sourceIds?})` (candidate-side scoping only; inbound links counted from any source). Pinned by `test/sum-stale-chunk-chars.test.ts`, `test/embedding-signature-stale.test.ts`, `test/e2e/engine-parity.test.ts`. Free-text alias layer: `resolveAliases(aliasNorms, opts?): Promise>>` (READ; maps each normalized alias to declaring `(slug, source_id)` pairs, source-scoped) and `setPageAliases(slug, sourceId, aliasNorms)` (WRITE; replaces the full alias set, delete-then-insert, empty clears, idempotent on the unique triple), called by the `importFromContent` ingest projection and the `reindex --aliases` backfill; parity across both engines, Pinned by `test/search/page-aliases-engine.test.ts`. `searchVector` in both engines injects the shared `buildBestPerPagePoolCte` per-page max-pool so a page surfaces on its strongest chunk. `executeRawDirect(sql, params?, opts?)` is the lock-hot-path sibling of `executeRaw`: same single-statement contract, but routes to the direct session-mode pool when dual-pool is active (Postgres/Supabase port 5432) so a long-held lock heartbeat survives the transaction pooler's per-transaction connection recycling; PGLite delegates straight to `executeRaw` (no pooler). Both engines implement it; the Minion lock path (`claim`/`renewLock`) is the consumer. `reconnect(ctx?: {error?})` is a REQUIRED lifecycle method on BOTH engines: it recovers a dropped connection using the config captured at the last `connect()`, so callers (autopilot health probe, `batchRetry`) never `disconnect()` + bare `connect()` (which loses the config and throws `database_url undefined` forever, and opens a null-connection window). PostgresEngine rebuilds its pool with a `_reconnecting` reentrancy guard and emits pool-recovery audit; PGLite is single-writer in-process so it just restores the saved data dir for parity. - `src/core/engine-constants.ts` — single source of truth for engine batch-sizing constants. Exports `DELETE_BATCH_SIZE = 500` consumed by both engines' `deletePages` + `resolveSlugsByPaths` and by the sync delete + rename loops. Lives outside `engine.ts` (the interface module) to avoid circular-import worry — bounded per-statement work for predictable lock hold time + write amplification. -- `src/core/background-work.ts` (#1762/#1745/#1775) — process background-work registry: the single owner of "drain every fire-and-forget DB-write sink before the CLI disconnects." `registerBackgroundWorkDrainer({name, order, drain(timeoutMs), abort?})` + `drainAllBackgroundWorkForCliExit({timeoutMs})` over a `Map` (idempotent registration by name; `__registerDrainerForTest` returns an unregister handle). Drains in explicit `(order, name)` order — facts FIRST (order 0) so its abort-path DB `logIngest` runs against the freshest live engine — and AWAITS `abort()` only when `drain()` reports `unfinished>0`. Best-effort per drainer: one sink's failure never blocks the others or the disconnect. FOUR sinks register at module import (rule-of-four): `facts/queue.ts` (order 0; `abort`=`shutdown()` cancels a hung facts:absorb Haiku via internalAbort), `last-retrieved.ts` (order 1), `search/hybrid.ts` (order 2; `awaitPendingSearchCacheWrites` bounded via `Promise.race`), `eval-capture.ts` (order 3; `captureEvalCandidate` self-tracks its promise via `awaitPendingEvalCaptures`). Both CLI-exit paths (`src/cli.ts` op-dispatch finally + `handleCliOnly` finally) call it before `engine.disconnect()` — closing the PGLite busy-loop where `db.close()` raced an in-flight job and pinned the single-writer lock (#1762). CLI-EXIT-ONLY: the facts `shutdown()` abort is permanent process state, never call in a long-lived `gbrain serve`. Companion changes: `src/core/ai/gateway.ts` `withDefaultTimeout(caller, ms)` bounds every outbound AI call (chat 300s, embed+multimodal 60s; env `GBRAIN_AI_{CHAT,EMBED,MULTIMODAL}_TIMEOUT_MS`; composed with caller signals via `AbortSignal.any`) and the op-dispatch + handleCliOnly force-exit timers `process.exit(process.exitCode ?? 0)` so a hung disconnect can't mask an errored op as success; `src/core/postgres-engine.ts` `reconnect()` module-mode branch re-establishes via idempotent `db.connect()` + `connectionManager.setReadPool` refresh instead of `db.disconnect()` (no null window for concurrent ops; fail-loud on real connect failure — #1745); `src/core/search/hybrid.ts` `embedQueryBounded` + a shared `QueryEmbedDeadline` (6s, floored 2s per embed via `MIN_QUERY_EMBED_BUDGET_MS`; env `GBRAIN_QUERY_EMBED_TIMEOUT_MS`) bounds the cache-lookup AND inner query embeds so a stalled provider falls back to keyword instead of hanging past the 10s force-exit (#1775). Incorporates + hardens PR #1763 (@ElliotDrel). Pinned by `test/core/background-work.test.ts`, `test/search/query-embed-deadline.test.ts`, `test/eval-capture-drain.test.ts`, `test/e2e/postgres-reconnect-singleton.test.ts`, `test/e2e/pglite-cli-exit.serial.test.ts`, `test/fix-wave-structural.test.ts`. +- `src/core/background-work.ts` (#1762/#1745/#1775) — process background-work registry: the single owner of "drain every fire-and-forget DB-write sink before the CLI disconnects." `registerBackgroundWorkDrainer({name, order, drain(timeoutMs), abort?})` + `drainAllBackgroundWorkForCliExit({timeoutMs})` over a `Map` (idempotent registration by name; `__registerDrainerForTest` returns an unregister handle). Drains in explicit `(order, name)` order — facts FIRST (order 0) so its abort-path DB `logIngest` runs against the freshest live engine — and AWAITS `abort()` only when `drain()` reports `unfinished>0`. Best-effort per drainer: one sink's failure never blocks the others or the disconnect. FOUR sinks register at module import (rule-of-four): `facts/queue.ts` (order 0; `abort`=`shutdown()` cancels a hung facts:absorb Haiku via internalAbort), `last-retrieved.ts` (order 1), `search/hybrid.ts` (order 2; `awaitPendingSearchCacheWrites` bounded via `Promise.race`), `eval-capture.ts` (order 3; `captureEvalCandidate` self-tracks its promise via `awaitPendingEvalCaptures`). Every cli.ts teardown site reaches it through `finishCliTeardown` (`src/core/cli-force-exit.ts`), which drains the registry before `engine.disconnect()` — closing the PGLite busy-loop where `db.close()` raced an in-flight job and pinned the single-writer lock (#1762). Exports `backgroundWorkSinkCount()` so the teardown helper computes its backstop deadline from the registered sink count. CLI-EXIT-ONLY: the facts `shutdown()` abort is permanent process state, never call in a long-lived `gbrain serve`. Companion changes: `src/core/ai/gateway.ts` `withDefaultTimeout(caller, ms)` bounds every outbound AI call (chat 300s, embed+multimodal 60s; env `GBRAIN_AI_{CHAT,EMBED,MULTIMODAL}_TIMEOUT_MS`; composed with caller signals via `AbortSignal.any`) and the teardown backstop honors an errored op's exit code so a hung disconnect can't mask failure as success (see `cli-force-exit.ts`); `src/core/postgres-engine.ts` `reconnect()` module-mode branch re-establishes via idempotent `db.connect()` + `connectionManager.setReadPool` refresh instead of `db.disconnect()` (no null window for concurrent ops; fail-loud on real connect failure — #1745); `src/core/search/hybrid.ts` `embedQueryBounded` + a shared `QueryEmbedDeadline` (6s, floored 2s per embed via `MIN_QUERY_EMBED_BUDGET_MS`; env `GBRAIN_QUERY_EMBED_TIMEOUT_MS`) bounds the cache-lookup AND inner query embeds so a stalled provider falls back to keyword instead of stalling the whole op (#1775). Incorporates + hardens PR #1763 (@ElliotDrel). Pinned by `test/core/background-work.test.ts`, `test/search/query-embed-deadline.test.ts`, `test/eval-capture-drain.test.ts`, `test/e2e/postgres-reconnect-singleton.test.ts`, `test/e2e/pglite-cli-exit.serial.test.ts`, `test/fix-wave-structural.test.ts`. - `src/core/search/graph-signals.ts` — per-query graph-signals helper. `applyGraphSignals(results, engine, opts)` runs as the 4th post-fusion stage (after backlink/salience/recency). Three boosts: `ADJACENCY_BOOST=1.05` (page linked from 2+ OTHER top-K results — local hub for THIS query), `CROSS_SOURCE_BOOST=1.10` (page linked from 2+ DIFFERENT sources — corroborated across team brains, dormant in single-source brains), `SESSION_DEMOTE=0.95` (3+ results from same chat session — keep the highest-scoring at full score, demote the rest). All three inherit the floor-ratio gate preventing weak pages from being boosted past strong ones via popularity. `computeScoreDistribution(results)` emits min/p25/p50/p75/p95/max + `reorder_band_width`. `sessionPrefix(slug)` extracts the chat-session anchor (`chat/2026-05-15-...`). Pure `pairedBootstrapPValue(deltas, resamples, rng)` exported for eval gates. Test seam via `adjacencyFn` DI. Fail-open: any error logs via `logGraphSignalsFailure` (JSONL audit via `audit-writer`) and returns the input array unchanged. Pinned by `test/search/graph-signals.test.ts` (incl. the IRON-RULE floor-gate regression). - `src/core/search/hybrid.ts` extension — `runPostFusionStages` has a 4th stage (`graphSignalsEnabled`, `onGraphMeta`, `onScoreDistribution`). `base_score` stamped at function entry idempotently (captured ONCE before any boost stage mutates `score`). Each post-fusion stage stamps its multiplier: `applyBacklinkBoost`→`backlink_boost`, `applySalienceBoost`→`salience_boost`, `applyRecencyBoost`→`recency_boost`. `applyReranker` (earlier in the pipeline) stamps `reranker_delta` as a rank delta (positive = improved). `applyExactMatchBoost` in `src/core/search/intent-weights.ts` stamps `exact_match_boost` when fired. Per-stage attribution powers `gbrain search --explain` — every boost surface carries its own field so `formatResultsExplain` reads them all without coupling to internal stage ordering. - `src/core/search/explain-formatter.ts` — renders `SearchResult[]` as a multi-line per-result breakdown for `gbrain search --explain`. Reads every boost-stamping field. Handles the "no boosts applied" empty path. 4-decimal precision with trailing-zero strip. Pinned by `test/search/explain-formatter.test.ts`. @@ -20,11 +20,13 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/context/` — Retrieval Reflex (Layer 1, issue #1981). `entity-salience.ts`: pure, zero-LLM, precision-biased `extractCandidates(text)` (capitalized runs + `@handles`, STOPWORDS + soft COMMON_WORDS + sentence-start guard, deterministic, capped). `retrieval-reflex.ts`: `resolveEntitiesToPointers(engine, sourceId, candidates, opts)` — alias arm (`resolveAliases`, caught per-arm for pre-v110 brains) + exact title/slug-suffix arm (the recall fix: real slugs are namespaced `people/x` but `slugify` drops the prefix); synopsis runs through `stripTakesFence`/`stripFactsFence` (the same privacy boundary `get_page` applies) so private facts never reach the prompt; suppression scans `priorContextText` only; capped at `MAX_POINTERS`. `reflex.ts`: the orchestrator + engine-aware resolver ladder (host `resolveEntities` → PGLite serve IPC → Postgres cached process-singleton → disabled), zero-candidate fast path, fail-open + timeout, heartbeat write for the doctor check, `reflexEnabled(cfg)` (file/env gate, default ON; DB-plane does NOT gate — `assemble()` is sync). `resolve-ipc.ts`: local unix-socket resolve protocol (client + server) so PGLite resolves through the single connection `gbrain serve` holds (a second opener would hit the exclusive lock; a subprocess would force-steal it past the 5-min staleness window and crash). Wired into `src/mcp/server.ts` (serve binds `/.gbrain-resolve.sock` on PGLite, cleaned up on shutdown). Doctor surface: `retrieval_reflex_health` in `src/commands/doctor.ts` (reads the heartbeat for truthful runtime status; categorized in `doctor-categories.ts`). Config: `retrieval_reflex` + `retrieval_reflex_max_pointers` in `src/core/config.ts`. Policy layer ships as the `retrieval-reflex` recipe (`recipes/retrieval-reflex/`). Pinned by `test/context/entity-salience.test.ts`, `test/retrieval-reflex.test.ts`, `test/context/resolve-ipc.test.ts`, `test/doctor-retrieval-reflex.test.ts`. - `src/commands/integrations.ts` — recipe install. The resolver-row install fence is keyed by `manifest.recipe` (`gbrain::resolver-rows`), so a second `copy-into-host-repo` recipe no longer writes a block mislabeled with the first recipe's name. Pinned by `test/integrations-install.test.ts`. - `src/core/audit/audit-writer.ts` — shared JSONL audit primitive consolidating the hand-rolled audit modules. Exports `createAuditWriter({kind, recordSchema})` returning `{log, readRecent}` plus shared helpers `computeIsoWeekFilename(kind, now?)` and `resolveAuditDir()` (honors `GBRAIN_AUDIT_DIR`). ISO-week file rotation; best-effort writes (stderr warn on failure, never throws); read-path scans current-week + previous-week files for boundary spans. Refactored onto it for parity: `src/core/rerank-audit.ts`, `src/core/audit-slug-fallback.ts`, `src/core/minions/handlers/shell-audit.ts`, `src/core/minions/handlers/supervisor-audit.ts`, `src/core/facts/phantom-audit.ts` (each module's public API preserved bit-for-bit). The `graph-signals-failures` audit (`logGraphSignalsFailure`) uses the same primitive. One hand-rolled audit remains at `src/core/skillpack/audit.ts`. Pinned by `test/audit/audit-writer.test.ts`. +- `src/core/cli-force-exit.ts` (#2084) — single owner of one-shot CLI exit + teardown, designed as a PAIR with the `import.meta.main` seam at the bottom of `src/cli.ts`. `finishCliTeardown({engine, drainTimeoutMs?})` is teardown-ONLY (never exits on the clean path): arms a REF'D backstop (unref'd would let a hung teardown exit naturally, skipping the flush and exiting with whatever PGLite scribbled into `process.exitCode`) whose deadline is COMPUTED from the bounds it guards (`computeTeardownDeadlineMs` = sinks × drainTimeoutMs + facts-abort grace + 2 × pool-end bound + slack, floor 10s; `GBRAIN_TEARDOWN_DEADLINE_MS` env override is the incident escape hatch), drains every background-work sink, disconnects the engine (a throw is warned + swallowed — the exit code reports the OPERATION, not the cleanup), then returns. The exit VERDICT lives in a gbrain-owned channel (`setCliExitVerdict`/`currentExitCode`; mirror-writes `process.exitCode` but NEVER reads it back) because PGLite's Emscripten runtime scribbles its own status into `process.exitCode` at arbitrary points mid-run — every writer that means to set the CLI exit code (op-dispatch catch, reindex, frontmatter, transcripts, brainstorm, autopilot) calls `setCliExitVerdict`. The deadline arms at TEARDOWN start, never before the op handler (the pre-#2084 placement measured handler + teardown combined, so PgBouncer deployments paid a flat 10s force-exit tax on every query and any >10s op was killed mid-run with exit 0). All nine cli.ts disconnect sites route through it; the ONE process exit happens in cli.ts's `main().then/catch` via `flushThenExit(currentExitCode())`, gated by `shouldForceExitAfterMain()` (daemon list: `serve`) — the CLI never waits for Bun's event loop to drain, because `endPoolBounded` deliberately races past stuck PgBouncer sockets that would keep it alive. `flushThenExit(code)` fences stdout+stderr (`write('', cb)` raced with an unref'd guard, EPIPE-safe both sync and async) then holds a REF'D aliveness grace for non-TTY stdio before `process.exit` — Bun delivers queued pipe writes only while the process is alive (no flush API reaches `process.stdout`'s native queue; write callbacks fire on accept, not delivery), so the grace IS the flush (#1959 truncation class). Scope claim is deliberately cli.ts-only: command modules' mid-run engine lifecycles stay local (process-exit semantics inside them would be wrong) and are absorbed by the final explicit exit. Pinned by `test/cli-finish-teardown.test.ts`, `test/flush-then-exit-harness.test.ts` (real spawned-Bun pipe semantics), `test/cli-should-force-exit.test.ts`, the `#2084` describes in `test/fix-wave-structural.test.ts` + `test/e2e/pglite-cli-exit.serial.test.ts`. + - `src/core/cli-options.ts` extension — `CliOptions` gains `explain: boolean`. `parseGlobalFlags` recognizes `--explain` anywhere in argv (stripped before command dispatch). `src/cli.ts` `formatResult` for `search` + `query` cases routes to `formatResultsExplain` from `src/core/search/explain-formatter.ts` when `CliOptions.explain` is set; falls through to the existing JSON / human formatters otherwise. - `src/commands/search.ts:gbrain search stats` extension — `graph_signals` section (enabled/source/failures_count/failures_by_reason). JSON envelope adds a `graph_signals` sibling property; `_meta.metric_glossary` adds `graph_signals.enabled` + `graph_signals.failures_by_reason`. Human output prints the section after the existing block. Reads `search.graph_signals` config first, falls back to the mode default. Pinned by `test/search/search-stats-graph-signals.test.ts`. - `src/commands/doctor.ts` extension — `graph_signals_coverage` check wired into both `runDoctor` (local) and `doctorReportRemote` (HTTP/JSON thin-client path). Reads `search.graph_signals` config first, falls back to mode default; silent `ok` when disabled. Computes inbound link coverage on the page set; warns at <10% with `gbrain extract all` fix hint; `ok` at ≥30% ("fire on most queries") and 10-29% ("fire occasionally"), each with the percentage embedded. Pinned by cases in `test/doctor.test.ts`. - `src/core/engine-factory.ts` — Engine factory with dynamic imports (`'pglite'` | `'postgres'`). -- `src/core/pglite-engine.ts` — PGLite (embedded Postgres 17.5 via WASM) implementation, all BrainEngine methods. `listLinkSources({sourceId?, sourceIds?})` returns distinct `link_source` provenances + counts (`ORDER BY count DESC, link_source ASC NULLS LAST`; scalar + federated scoped; parity with postgres-engine.ts) powering `gbrain link-sources`. `addLinksBatch`/`addTimelineEntriesBatch`/`addTakesBatch` pass the whole batch as one JSONB document via `jsonb_to_recordset(($1::jsonb)->'rows')` (bound through `executeRawJsonb` with a `{ rows }` wrapper; rows built by the shared `src/core/batch-rows.ts` helpers, NUL-stripped), and are `batchRetry`-wrapped. `connect()` wraps `PGlite.create()` in a try/catch that emits an actionable error (macOS 26.3 WASM bug #223, points at `gbrain doctor`); the lock is released on failure so the next process can retry cleanly. `searchKeyword`/`searchKeywordChunks` multiply `ts_rank` by the source-factor CASE at chunk grain; `searchVector` is a two-stage CTE — inner CTE keeps `ORDER BY cc.embedding <=> vec` so HNSW stays usable, outer SELECT re-ranks by `raw_score * source_factor`, inner LIMIT scales with offset to preserve pagination. `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL — probes for forward-referenced state the embedded blob needs (`pages.source_id`, `links.link_source`, `links.origin_page_id`, `content_chunks.symbol_name`, `content_chunks.language`, `sources` FK target, plus `files.source_id`, `files.page_id`, `oauth_clients.source_id`, `oauth_clients.federated_read`, `sources.archived`, `sources.archived_at`, `sources.archive_expires_at`) and adds only what's missing; threads the DDL connection from `initSchema` so probes run inside the advisory-lock scope; no-op on fresh installs and modern brains (closes the upgrade-wedge bug class #239/#243/#266/#357/#366/#374/#375/#378/#395/#396/#1018/#974/#820). `getBrainScore` returns 100/100 with full breakdown (35/25/15/15/10) when `pageCount === 0` (vacuous truth — empty brain has no coverage problem); Pinned by `test/brain-score-breakdown.test.ts` empty-brain assertion + `test/doctor-report-remote.serial.test.ts`. `disconnect()` uses snapshot+early-null (snapshot `_db`/`_lock`, null instance fields BEFORE any `await` so a concurrent `connect()` can't see a partial mid-close state) wrapped in try/finally guaranteeing lock-release even if `db.close()` throws; KEEPS close-then-release order (release-then-close was rejected: it would widen the window where a sibling process connects to a still-closing brain); Pinned by `test/pglite-engine-disconnect.serial.test.ts`. Exports `classifyPgliteInitError(message): 'bunfs' | 'macos-26-3' | 'unknown'` + `buildPgliteInitErrorMessage(verdict, original)` routing the catch-block hint by failure shape (`bunfs` matches literal `$$bunfs` OR `ENOENT[\s\S]*pglite\.data` co-occurrence, surfaces a paste-ready `bun upgrade` + Node fallback; `macos-26-3` keeps the #223 link; `unknown` falls through); Pinned by `test/pglite-init-classifier.test.ts`. Implements `deletePages(slugs, {sourceId})` + `resolveSlugsByPaths(paths, {sourceId})` via `slug = ANY($1::text[])` array-param binding, caller-chunking primitive throwing when input exceeds `DELETE_BATCH_SIZE`, `deletePages` returns `RETURNING slug` rows so callers filter `pagesAffected` to confirmed deletes. Implements the embedding-signature stale-detection quartet — `sumStaleChunkChars({sourceId?, signature?})`, `setPageEmbeddingSignature(slug, {sourceId?, signature})`, `invalidateStaleSignatureEmbeddings({signature, sourceId?})`, widened `countStaleChunks({sourceId?, signature?})` (the `signature` opt widens via `JOIN pages p ... WHERE cc.embedding IS NULL OR (p.embedding_signature IS NOT NULL AND p.embedding_signature <> $signature)`, NULL grandfathered); parity SQL with postgres-engine.ts. PGLite-specific DDL (pgvector, pg_trgm, triggers). +- `src/core/pglite-engine.ts` — PGLite (embedded Postgres 17.5 via WASM) implementation, all BrainEngine methods. `listLinkSources({sourceId?, sourceIds?})` returns distinct `link_source` provenances + counts (`ORDER BY count DESC, link_source ASC NULLS LAST`; scalar + federated scoped; parity with postgres-engine.ts) powering `gbrain link-sources`. `addLinksBatch`/`addTimelineEntriesBatch`/`addTakesBatch` pass the whole batch as one JSONB document via `jsonb_to_recordset(($1::jsonb)->'rows')` (bound through `executeRawJsonb` with a `{ rows }` wrapper; rows built by the shared `src/core/batch-rows.ts` helpers, NUL-stripped), and are `batchRetry`-wrapped. `connect()` wraps `PGlite.create()` in a try/catch that emits an actionable error (macOS 26.3 WASM bug #223, points at `gbrain doctor`); the lock is released on failure so the next process can retry cleanly. `searchKeyword`/`searchKeywordChunks` multiply `ts_rank` by the source-factor CASE at chunk grain; `searchVector` is a two-stage CTE — inner CTE keeps `ORDER BY cc.embedding <=> vec` so HNSW stays usable, outer SELECT re-ranks by `raw_score * source_factor`, inner LIMIT scales with offset to preserve pagination. `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL — probes for forward-referenced state the embedded blob needs (`pages.source_id`, `links.link_source`, `links.origin_page_id`, `content_chunks.symbol_name`, `content_chunks.language`, `sources` FK target, plus `files.source_id`, `files.page_id`, `oauth_clients.source_id`, `oauth_clients.federated_read`, `sources.archived`, `sources.archived_at`, `sources.archive_expires_at`) and adds only what's missing; threads the DDL connection from `initSchema` so probes run inside the advisory-lock scope; no-op on fresh installs and modern brains (closes the upgrade-wedge bug class #239/#243/#266/#357/#366/#374/#375/#378/#395/#396/#1018/#974/#820). `getBrainScore` returns 100/100 with full breakdown (35/25/15/15/10) when `pageCount === 0` (vacuous truth — empty brain has no coverage problem); Pinned by `test/brain-score-breakdown.test.ts` empty-brain assertion + `test/doctor-report-remote.serial.test.ts`. `disconnect()` uses snapshot+early-null (snapshot `_db`/`_lock`, null instance fields BEFORE any `await` so a concurrent `connect()` can't see a partial mid-close state) wrapped in try/finally guaranteeing lock-release even if `db.close()` throws; KEEPS close-then-release order (release-then-close was rejected: it would widen the window where a sibling process connects to a still-closing brain); Pinned by `test/pglite-engine-disconnect.serial.test.ts`. `PGlite.create()` runs inside `preservingProcessExitCode` (#2084): PGLite's Emscripten runtime writes its own status into `process.exitCode` (99 at create; in-memory brains run initdb whose status lands on a later tick; the exit status at close — and assigning `undefined` cannot clear a prior value), which would otherwise silently clobber an errored op's exit 1 back to 0. The wrapper keeps the global tidy for external readers; `db.close()` stays unwrapped (its 0-write is baseline behavior test runners depend on). The CLI's exit verdict is immune either way — it lives in the gbrain-owned channel in `cli-force-exit.ts` and never reads `process.exitCode` back. Exports `classifyPgliteInitError(message): 'bunfs' | 'macos-26-3' | 'unknown'` + `buildPgliteInitErrorMessage(verdict, original)` routing the catch-block hint by failure shape (`bunfs` matches literal `$$bunfs` OR `ENOENT[\s\S]*pglite\.data` co-occurrence, surfaces a paste-ready `bun upgrade` + Node fallback; `macos-26-3` keeps the #223 link; `unknown` falls through); Pinned by `test/pglite-init-classifier.test.ts`. Implements `deletePages(slugs, {sourceId})` + `resolveSlugsByPaths(paths, {sourceId})` via `slug = ANY($1::text[])` array-param binding, caller-chunking primitive throwing when input exceeds `DELETE_BATCH_SIZE`, `deletePages` returns `RETURNING slug` rows so callers filter `pagesAffected` to confirmed deletes. Implements the embedding-signature stale-detection quartet — `sumStaleChunkChars({sourceId?, signature?})`, `setPageEmbeddingSignature(slug, {sourceId?, signature})`, `invalidateStaleSignatureEmbeddings({signature, sourceId?})`, widened `countStaleChunks({sourceId?, signature?})` (the `signature` opt widens via `JOIN pages p ... WHERE cc.embedding IS NULL OR (p.embedding_signature IS NOT NULL AND p.embedding_signature <> $signature)`, NULL grandfathered); parity SQL with postgres-engine.ts. PGLite-specific DDL (pgvector, pg_trgm, triggers). - `src/core/pglite-lock.ts` (#2058) — advisory data-dir lock so two processes never open the same embedded PGLite (single-connection WASM Postgres) at once. Atomic `mkdir` of `.gbrain-lock/` + a `lock` file carrying `{pid, acquired_at, refreshed_at}`. A held lock HEARTBEATS its `refreshed_at` every 30s (`.unref()`ed timer) so a long-running but LIVE holder (an `embed` job can run for minutes) is never mistaken for stale. A waiting acquirer reaps the holder only when it is dead (PID gone) OR has stopped refreshing past the steal grace (`GBRAIN_PGLITE_LOCK_STEAL_GRACE_SECONDS`, default 600s) — pairing PID liveness with heartbeat freshness defeats BOTH the WAL-corruption bug (stealing a live writer) and the PID-reuse false positive (a recycled PID reading as alive). Each holder carries an ownership token (`:`); the heartbeat and `releaseLock` verify the on-disk lock is STILL theirs before touching it, so a stalled-then-resumed holder that was already reaped + replaced can't clobber the new owner. In-memory engines take no lock (no file, no concurrent access). Pinned by `test/pglite-lock.test.ts`. - `src/core/postgres-engine.ts` — Postgres + pgvector implementation (Supabase / self-hosted). `addLinksBatch`/`addTimelineEntriesBatch`/`addTakesBatch` pass the batch as one JSONB document — `INSERT ... SELECT FROM jsonb_to_recordset(($1::jsonb)->'rows') AS v(...) JOIN pages ...` bound through `executeRawJsonb({ rows })` — which encodes arbitrary free text safely (the old `unnest(${arr}::text[])` array-literal path crashed Postgres with "malformed array literal" on calendar/Zoom context, gbrain#1861) and sidesteps the 65535-parameter cap; takes declares native recordset column types (`page_id int, weight real, active boolean, …`) so no per-element casts; all three are `batchRetry`-wrapped. `searchKeyword`/`searchVector` scope `statement_timeout` via `sql.begin` + `SET LOCAL` so the GUC dies with the transaction instead of leaking across the pooled postgres.js connection. `getEmbeddingsByChunkIds` uses `tryParseEmbedding` so one corrupt row skips+warns instead of killing the query. `searchKeyword`/`searchKeywordChunks`/`searchVector` apply source-aware ranking by inlining the source-factor CASE and `NOT (col LIKE …)` hard-exclude from `src/core/search/sql-ranking.ts`; `searchVector` is a two-stage CTE (HNSW-safe inner ORDER BY, source-boost re-rank in outer SELECT) carrying `p.source_id` inner→outer. `_savedConfig` retains the connect config; `reconnect()` tears down + recreates the pool from saved config (called by supervisor watchdog after 3 consecutive health-check failures, and by `batchRetry` on a retryable connection error). Concurrent callers share one in-flight `_reconnectPromise` (they await the single reconnect rather than racing a half-rebuilt pool); ownership re-samples through the atomic `db.connect()` token on the connect leg. `reconnect(ctx?)` accepts the triggering error and records a pool-recovery audit event (`reap_detected`/`reconnect_other`/`reconnect_succeeded`/`reconnect_failed`) for the `pool_reap_health` doctor check. `executeRaw` is a single-statement passthrough — no per-call retry (unsound for non-idempotent statements; recovery is supervisor-driven). `connect()` applies `resolveSessionTimeouts()` from `db.ts` as connection-time startup parameters (`statement_timeout`, `idle_in_transaction_session_timeout`) so orphan pgbouncer backends can't hold locks for hours. `countStaleChunks()`+`listStaleChunks()` server-side-filter on `embedding IS NULL` for `embed --stale` (eliminates ~76 MB/call client-side pull); `upsertChunks()` resets both `embedding` AND `embedded_at` to NULL when chunk_text changes without a new embedding. `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL on the same probe set as PGLite (extended for column-only forward-reference cases: `files.source_id`, `files.page_id`, `oauth_clients.source_id`, `oauth_clients.federated_read`, `sources.archived`/`archived_at`/`archive_expires_at`); the entire probe path runs on the DDL connection threaded from `initSchema` (closing a concurrent-bootstrap race for Supabase pooler users); closes #1018/#974/#820. `disconnect()` is idempotent — `_connectionStyle` tracks whether the engine owns its pool (worker engines) or shares the module-level singleton; second call on an instance-pool engine is a no-op rather than clobbering the singleton; and a module-style engine only calls `db.disconnect()` when it owns the singleton (`_ownsModuleSingleton`, set from the `db.connect()` creation token), so a borrower probe engine's teardown leaves the cycle owner's connection intact. Pinned by `test/e2e/postgres-engine-disconnect-idempotency.test.ts` + `test/postgres-engine-singleton-ownership.test.ts`. `getBrainScore` empty-brain parity with PGLite — 100/100 with breakdown 35/25/15/15/10 when `pageCount === 0` (both engines must agree to keep `doctor-report-remote.serial.test.ts` deterministic). Implements `deletePages(slugs, {sourceId}): Promise` via `DELETE FROM pages WHERE slug = ANY($1::text[]) AND source_id = $2 RETURNING slug` (single round-trip; caller chunks); `resolveSlugsByPaths` does `SELECT slug, source_path FROM pages WHERE source_path = ANY($1::text[]) AND source_id = $2`; FK cascades through `content_chunks`/`links`/`tags`/`raw_data`/`timeline_entries`/`page_versions`, `files.page_id`+`links.origin_page_id` go SET NULL; throws when input exceeds `DELETE_BATCH_SIZE` (from `src/core/engine-constants.ts`); both short-circuit on empty input. Implements the embedding-signature stale-detection quartet (`sumStaleChunkChars`, `setPageEmbeddingSignature`, `invalidateStaleSignatureEmbeddings`, widened `countStaleChunks`, all accept optional `signature` extending "stale" to model/dims-swap drift via the `pages.embedding_signature` JOIN, NULL grandfathered; the `embedding IS NULL` server-side filter is preserved as the no-signature fast path); Pinned by `test/e2e/engine-parity.test.ts`. - `src/core/cjk.ts` — Single source of truth for CJK detection. Exports `CJK_RANGES_REGEX`, `CJK_SLUG_CHARS` (character-class fragment for embedding inside other regexes), `CJK_SENTENCE_DELIMITERS` (`。！？`), `CJK_CLAUSE_DELIMITERS` (`；：，、`), `CJK_DENSITY_THRESHOLD = 0.30`, `hasCJK(s)`, `countCJKAwareWords(s)` (30% density threshold — English docs with one Japanese term stay whitespace-tokenized; Chinese-dominant docs get char-counted), and `escapeLikePattern(s)` (escapes `%`, `_`, `\\` for `ILIKE ... ESCAPE '\\'`). BMP-only ranges (Han / Hiragana / Katakana / Hangul Syllables). Consumers: `expansion.ts`, `sync.ts:slugifySegment`, `operations.ts:validatePageSlug + validateFilename`, `chunkers/recursive.ts:countWords + DELIMITERS`, `pglite-engine.ts:searchKeyword + searchKeywordChunks`. @@ -36,7 +38,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/commands/sync.ts:resolveSlugByPathOrSourcePath` — Resolves a slug by `pages.source_path` first (returns the stored slug for frontmatter-fallback pages whose path doesn't derive a slug), then falls back to `resolveSlugForPath(path)`. Threaded into all 4 delete/rename call sites (`performSync`'s un-syncable cleanup at ~:531, deletes at ~:603, rename oldSlug at ~:622). Without this, emoji-only / Thai / Arabic filenames whose slug came from frontmatter would orphan on delete/rename (the delete path would compute the wrong path-derived slug). Best-effort query — pre-migration brains fall through to the legacy path. - `src/core/sources-ops.ts` — Multi-source registration + clone-lifecycle ops (`addSource`, `recloneIfMissing`, `defaultCloneDir`, `isOwnedClone`, `unownedHint`). **Reclone-ownership invariant (must-never-violate): gbrain may only delete/re-clone a clone it created, NEVER a user working tree.** `recloneIfMissing` deletes `local_path`, so it gates on `isOwnedClone(src)` and throws a `SourceOpError('unmanaged_path', ...)` BEFORE any filesystem op when ownership is unprovable — fail-closed. Ownership is proven by `config.managed_clone === true` (written by `addSource`'s `--url` path, covering default-location and `--clone-dir` clones) OR `local_path === defaultCloneDir(id)` (back-compat for pre-marker clones, via exact normalized-path equality, symlink-free). A row with `remote_url` + an unowned `local_path` (a user-registered working tree, e.g. `sources add --path`) is refused untouched; re-add with `--url` to regain auto-reclone. The reclone is EXDEV-safe: clone into a SIBLING temp of `local_path` (not the shared `clones/.tmp`, which may sit on a different mount than a `--clone-dir` target), then swap (move old aside → move new in → drop old) so `local_path` is never left missing-and-unrecoverable; on swap failure the original is restored, and if restore fails the error names the `aside` path so it's never reflexively deleted. A TOCTOU re-check re-confirms ownership immediately before the destructive move and rejects a symlink leaf swapped in after the entry check (`symlink_escape`). `unownedHint(src, state)` is the shared recovery message used by both the core error and the `gbrain sync --source` CLI error; `gbrain sources restore` special-cases `unmanaged_path` to print "DB row restored; gbrain syncs this path read-only" instead of the misleading "try sync to recover" guidance. `SourceOpErrorCode` includes `unmanaged_path`. Pinned by `test/sources-ops.test.ts`, `test/sources-resync-recovery.test.ts`. - `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness). `isUndefinedColumnError(err)` predicate — pattern-matches Postgres SQLSTATE 42703 / "column ... does not exist" with engine-driver shape variation tolerated; replaces bare `catch {}` blocks in `oauth-provider.ts` so genuine errors (lock timeout, network blip, permission denied) propagate while column-missing falls through to the legacy fallback. `validateSourceId(id)` throws on anything outside `^[a-z0-9_-]+$`, used by the per-source disk-layout fix in patterns.ts/synthesize.ts before any `join(brainDir, '.sources', source_id, slug+'.md')` so source_id can't traverse out of brainDir. `rowToPage` populates the required `Page.source_id` from the SELECT projection (`scripts/check-source-id-projection.sh` enforces every projection feeding `rowToPage` includes the column). -- `src/core/db.ts` — Connection management, schema initialization. `resolveSessionTimeouts()` returns `statement_timeout` + `idle_in_transaction_session_timeout` (defaults 5min each, env-overridable via `GBRAIN_STATEMENT_TIMEOUT`/`GBRAIN_IDLE_TX_TIMEOUT`/`GBRAIN_CLIENT_CHECK_INTERVAL`). Both `connect()` (module singleton) and `PostgresEngine.connect()` (worker pool) consume the result via postgres.js's `connection` option, sending GUCs as startup parameters that survive PgBouncer transaction mode (`setSessionDefaults` kept as a back-compat no-op shim). `connect()` returns `Promise` — `true` iff THIS call created the module singleton, `false` if it joined an existing one; the decision is atomic (no `await` between the `if (sql)` null-check and the synchronous `sql = postgres(...)` assignment), so two concurrent module connects can't both claim creation. `PostgresEngine` stores the return as its `_ownsModuleSingleton` token and only the creating engine may `db.disconnect()` the singleton — a borrower probe engine (lint/doctor config-lift) no-ops its disconnect, so its teardown can't null the connection the long-lived cycle owner is still using (the dream-cycle "connect() has not been called" failure). The module `sql` is only ever nulled by `db.disconnect()` (postgres.js auto-reconnects its own internal pool and never touches our reference). `disconnect()` snapshots + nulls `sql` before awaiting the pool end so a concurrent connect can't join a pool that's already closing. The end routes through `endPoolBounded(pool)` (#1972) — a gbrain-owned `Promise.race` of `pool.end({ timeout: POOL_END_TIMEOUT_SECONDS })` against a hard timer — so a PgBouncer transaction-mode drain that never settles can't block teardown until the CLI's 10s force-exit fires and truncates stdout (#1959); it never throws. `connection-manager.ts` ends its direct + read pools concurrently through the same helper so the per-pool bounds don't stack. +- `src/core/db.ts` — Connection management, schema initialization. `resolveSessionTimeouts()` returns `statement_timeout` + `idle_in_transaction_session_timeout` (defaults 5min each, env-overridable via `GBRAIN_STATEMENT_TIMEOUT`/`GBRAIN_IDLE_TX_TIMEOUT`/`GBRAIN_CLIENT_CHECK_INTERVAL`). Both `connect()` (module singleton) and `PostgresEngine.connect()` (worker pool) consume the result via postgres.js's `connection` option, sending GUCs as startup parameters that survive PgBouncer transaction mode (`setSessionDefaults` kept as a back-compat no-op shim). `connect()` returns `Promise` — `true` iff THIS call created the module singleton, `false` if it joined an existing one; the decision is atomic (no `await` between the `if (sql)` null-check and the synchronous `sql = postgres(...)` assignment), so two concurrent module connects can't both claim creation. `PostgresEngine` stores the return as its `_ownsModuleSingleton` token and only the creating engine may `db.disconnect()` the singleton — a borrower probe engine (lint/doctor config-lift) no-ops its disconnect, so its teardown can't null the connection the long-lived cycle owner is still using (the dream-cycle "connect() has not been called" failure). The module `sql` is only ever nulled by `db.disconnect()` (postgres.js auto-reconnects its own internal pool and never touches our reference). `disconnect()` snapshots + nulls `sql` before awaiting the pool end so a concurrent connect can't join a pool that's already closing. The end routes through `endPoolBounded(pool)` (#1972) — a gbrain-owned `Promise.race` of `pool.end({ timeout: POOL_END_TIMEOUT_SECONDS })` against a hard timer — so a PgBouncer transaction-mode drain that never settles can't hang teardown — the #2084 contract (finishCliTeardown's computed-deadline backstop + flushThenExit's fence-and-grace exit in cli-force-exit.ts) bounds it and delivers pending stdout before exit. `connection-manager.ts` ends its direct + read pools concurrently through the same helper so the per-pool bounds don't stack. - `src/commands/migrate-engine.ts` — Bidirectional engine migration (`gbrain migrate --to supabase/pglite`). - `src/core/import-file.ts` — importFromFile + importFromContent (chunk + embed + tags). `importFromContent` and `importCodeFile` stamp `pages.embedding_signature` via `setPageEmbeddingSignature(slug, {sourceId, signature: currentEmbeddingSignature()})` when the import actually embedded (not `--no-embed`) so a model/dims swap is detectable as stale; `importCodeFile` only stamps when every chunk was freshly embedded this call (`needsEmbedIndexes.length === chunks.length`), mixed reuse-by-hash pages stay unstamped (`reindex --code --force` / `embed --stale` handle those). `importFromContent`'s tag reconciliation is ADD-ONLY: it only `addTag` (idempotent, ON CONFLICT DO NOTHING). The `tags` table has no provenance column and frontmatter tags are stripped from stored `pages.frontmatter` (markdown.ts:118), so a frontmatter-origin tag can't be distinguished from a DB-enrichment tag (auto-tag / dream synthesize / signal-detector) at re-import — deletion is unsafe (would wipe enrichment under `gbrain reindex --markdown`). Accepted trade-off: removing a tag from frontmatter no longer removes it from the DB on next sync (needs a `tag_source` provenance column). Pinned by `test/reindex-preserve-tags.test.ts` + `test/import-file.test.ts`. - `src/core/sync.ts` — Pure sync functions (manifest parsing, filtering, slug conversion). Exported `pruneDir(name: string): boolean` is the single source of truth for descent-time directory exclusion across walkers — blocks `node_modules` (no leading dot, so naive walkers slipped through and inflated MISSING_OPEN counts via vendor packages), dot-prefix dirs, `ops/`, and `*.raw` sidecars; `isSyncable` applies it per path segment, and `walkMarkdownFiles` in `src/commands/extract.ts` + `listTextFiles` in `src/core/cycle/transcript-discovery.ts` consult it BEFORE recursing to save the IO of walking thousands of vendor files (closes #923 + #202). `manageGitignore` worktree discriminator matches the gitdir path segment (`/modules/` = submodule, `/worktrees/` = worktree, per Git's documented layout) so Conductor worktrees (first-class repos) get `.gitignore` management for storage-tiering (closes #889). The sync-failure ledger (failure store, error classifier, the shared bookmark gate, and the doctor severity rule) lives in `src/core/sync-failure-ledger.ts`; `sync.ts` re-exports `classifyErrorCode`, `summarizeFailuresByCode`, `loadSyncFailures`, `unacknowledgedSyncFailures`, `acknowledgeSyncFailures`, `recordSyncFailures`, `decideSyncFailureSeverity`, `applySyncFailureGate`, and the `SyncFailure` type for backward-compatible imports — see its entry below. diff --git a/package.json b/package.json index cde5c8fda..a345afb96 100644 --- a/package.json +++ b/package.json @@ -143,5 +143,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.41.0" + "version": "0.42.42.0" } diff --git a/src/cli.ts b/src/cli.ts index e80d7e4b8..82b7e22a6 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -25,8 +25,7 @@ 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 { drainAllBackgroundWorkForCliExit } from './core/background-work.ts'; -import { shouldForceExitAfterMain } from './core/cli-force-exit.ts'; +import { shouldForceExitAfterMain, finishCliTeardown, flushThenExit, currentExitCode, setCliExitVerdict } 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'; @@ -260,7 +259,11 @@ async function main() { await withTimeout(runSearch(engine, subArgs), timeoutMs, label); } } finally { - await engine.disconnect(); + // #2084: `search diagnose` runs real hybrid retrieval (arms search-cache + // writes) — route through the shared bounded teardown like every other + // one-shot path. The connect-timeout process.exit(124) above is reviewed + // and intentionally unchanged: no engine exists at that point. + await finishCliTeardown({ engine }); } return; } @@ -350,44 +353,26 @@ 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). - // The unref'd hard-exit fallback is armed inside the `finally` below, so it - // bounds ONLY the teardown phase (drain + disconnect) — the same placement - // as the fall-through owner-disconnect site later in this file. It used to - // be armed HERE, before the try, which silently killed any op whose BODY ran - // past the deadline: on a slow Postgres pooler (6-10s per fresh connection) - // a healthy `gbrain search` was force-exited mid-handler with code 0 and - // ZERO stdout — an empty "success" indistinguishable from no results. The - // exitCode honor (v0.42.20.0) can't help there: a mid-op kill fires before - // any error path sets exitCode. Op-body wallclock bounds are the read-scope - // withTimeout wrap inside the try below, not this teardown backstop. - // Daemons (`serve`) are excluded so they stay alive. - const DISCONNECT_HARD_DEADLINE_MS = 10_000; - // Wallclock bound for READ-scope op handlers. With the hard-deadline timer - // correctly scoped to teardown, a genuinely WEDGED read handler (hung pooler - // connection mid-query) would otherwise hang the CLI forever — the #1633 - // zombie class the old (buggy) pre-try timer accidentally bounded at 10s. - // 180s sits far above any healthy slow-pooler run (6-10s/connection); - // --timeout=Ns overrides. Writes/admin stay unbounded: a long import/embed - // must never be killed by a default deadline. + // #2084: the teardown contract (bounded drain of every background-work sink, + // bounded disconnect, computed-deadline backstop) lives in finishCliTeardown + // — see src/core/cli-force-exit.ts for the full design. The hard-deadline + // timer arms at TEARDOWN start inside the helper, never before the handler: + // the pre-#2084 placement here measured handler + teardown combined, so a + // slow-but-healthy query burned the teardown budget (the flat-10s-banner + // bug) and any >10s op was force-killed mid-run with exit 0. The explicit + // process exit happens once, in the import.meta.main seam at the bottom of + // this file — NOT here. + + // v0.42.41.0 (merged): wallclock bound for READ-scope op handlers. With the + // teardown backstop correctly scoped to teardown, a genuinely WEDGED read + // handler (hung pooler connection mid-query) would otherwise hang the CLI + // forever — the #1633 zombie class the old pre-try timer accidentally + // bounded at 10s. 180s sits far above any healthy slow-pooler run + // (6-10s/connection); --timeout=Ns overrides. Writes/admin stay unbounded: + // a long import/embed must never be killed by a default deadline. On + // timeout the abandoned handler may hold ref'd sockets — harmless here, + // because the import.meta.main seam exits explicitly on every one-shot path. const READ_OP_TIMEOUT_MS = 180_000; - let forceExitTimer: ReturnType | undefined; - // Set when a wallclock bound fired. The abandoned (timed-out but still - // running) handler can hold ref'd sockets/timers that keep Bun's event loop - // alive after main() returns — so the finally must hard-exit after teardown - // on this path, or the timeout print is followed by an immortal process: - // the same zombie class, resurrected through the timeout door (adversarial - // review finding). - let wallclockTimedOut = false; try { const { withTimeout, OperationTimeoutError } = await import('./core/timeout.ts'); @@ -397,8 +382,10 @@ async function main() { ? '' : ` (default ${e.ms}ms; pass --timeout=Ns to override)`; console.error(`${e.label} timed out${hint}.`); - process.exitCode = 124; - wallclockTimedOut = true; + // 124 = timeout convention (matches the read-only dispatch path). Set + // through the verdict channel — a raw process.exitCode write is invisible + // to the exit seam and PGLite's WASM runtime can scribble over it. + setCliExitVerdict(124); }; // Context build does DB I/O (resolveSourceId) and runs for EVERY op — @@ -414,7 +401,7 @@ async function main() { } catch (e: unknown) { if (e instanceof OperationTimeoutError) { onWallclockTimeout(e); - return; // the finally below still drains + disconnects, then exits + return; // the finally drains + disconnects; the import.meta.main seam exits } throw e; } @@ -430,7 +417,7 @@ async function main() { } catch (e: unknown) { if (e instanceof OperationTimeoutError) { onWallclockTimeout(e); - return; // the finally below still drains + disconnects, then exits + return; // the finally drains + disconnects; the import.meta.main seam exits } throw e; } @@ -457,45 +444,12 @@ async function main() { } else { console.error(e instanceof Error ? e.message : String(e)); } - process.exitCode = 1; + setCliExitVerdict(1); } finally { - // v0.42.20.0 — drain ALL fire-and-forget sinks (facts, last-retrieved, - // search-cache, eval-capture) via the background-work registry BEFORE - // disconnect, so a PGLite db.close() can't race in-flight work into the - // re-pump busy-loop (#1762). facts drains first (order 0) so its abort-path - // DB logIngest gets the freshest live-engine window. 1s per-sink timeout: - // read paths with no pending work pay the ~0ms fast path; capture/import - // that DO enqueue pay up to 1s (+ facts shutdown grace) while in-flight - // Haiku finishes. The unref'd hard-deadline timer armed here is the - // backstop if disconnect or a lingering socket keeps Bun's loop alive — - // armed at teardown entry (NOT before the op body; see the C13 comment - // above) so a slow-but-progressing op handler is never killed mid-flight. - if (shouldForceExitAfterMain()) { - forceExitTimer = setTimeout(() => { - console.warn( - `[cli] engine.disconnect() did not return within ${DISCONNECT_HARD_DEADLINE_MS}ms — force-exiting`, - ); - // v0.42.20.0 (codex): honor an exit code an errored op already set — - // a bare process.exit(0) here would mask a failed op as success if the - // drain/disconnect then hangs. - process.exit(process.exitCode ?? 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?.(); - } - await drainAllBackgroundWorkForCliExit({ timeoutMs: 1000 }); - await engine.disconnect(); - if (forceExitTimer) clearTimeout(forceExitTimer); - // Wallclock-timeout path: teardown is done, but the ABANDONED handler - // (withTimeout races, it does not cancel) can still hold ref'd sockets / - // SDK retry timers that keep Bun's event loop alive indefinitely. With - // the hard-deadline timer just cleared, nothing else bounds that — exit - // explicitly. Safe: drain + disconnect completed on the lines above. - if (wallclockTimedOut) { - process.exit(process.exitCode ?? 124); - } + // 1s per-sink drain budget: read paths with no pending work pay the ~0ms + // fast path; capture/import that DO enqueue pay up to 1s (+ facts shutdown + // grace) while in-flight Haiku finishes (#1762 drain-before-disconnect). + await finishCliTeardown({ engine, drainTimeoutMs: 1000 }); } } @@ -1252,13 +1206,13 @@ async function handleCliOnly(command: string, args: string[]) { if (args.includes('--remediation-plan')) { const { runRemediationPlan } = await import('./commands/doctor.ts'); const eng = await connectEngine(); - try { await runRemediationPlan(eng, args); } finally { await eng.disconnect(); } + try { await runRemediationPlan(eng, args); } finally { await finishCliTeardown({ engine: eng }); } return; } if (args.includes('--remediate')) { const { runRemediate } = await import('./commands/doctor.ts'); const eng = await connectEngine(); - try { await runRemediate(eng, args); } finally { await eng.disconnect(); } + try { await runRemediate(eng, args); } finally { await finishCliTeardown({ engine: eng }); } return; } @@ -1271,13 +1225,21 @@ async function handleCliOnly(command: string, args: string[]) { // "user chose --fast while config is present". await runDoctor(null, args, getDbUrlSource()); } else { + // #2084: both failure kinds (connect throw, runDoctor(eng) throw) still + // fall back to filesystem-only checks — identical to the prior shape. + // The finally closes the gap where a runDoctor(eng) throw used to skip + // the in-try disconnect. NOTE: runDoctor normally calls process.exit + // itself, which preempts this finally — in-command exit sites bypassing + // teardown are a pre-existing class, tracked as a TODOS.md follow-up. + let eng: BrainEngine | null = null; try { - const eng = await connectEngine(); + eng = await connectEngine(); await runDoctor(eng, args); - await eng.disconnect(); } catch { // DB unavailable — still run filesystem checks await runDoctor(null, args, getDbUrlSource()); + } finally { + if (eng) await finishCliTeardown({ engine: eng }); } } return; @@ -1291,7 +1253,7 @@ async function handleCliOnly(command: string, args: string[]) { try { await runZeSwitch(args, eng); } finally { - await eng.disconnect(); + await finishCliTeardown({ engine: eng }); } return; } @@ -1336,12 +1298,15 @@ async function handleCliOnly(command: string, args: string[]) { await runDream(eng, args); } finally { // #1471 invariant tripwire (the dream-cycle owner): `eng` created the - // module singleton (first module connector) and is disconnected LAST, + // module singleton (first module connector) and is torn down LAST, // here, after the whole cycle. The ownership fix relies on this owner's // lifetime strictly dominating every borrower (lint/doctor probe engines - // created mid-cycle). Do NOT disconnect `eng` before runDream returns, or + // created mid-cycle). Do NOT tear down `eng` before runDream returns, or // a borrower could outlive the owner and lose the shared singleton. - if (eng) await eng.disconnect(); + // #2084: routed through the shared bounded teardown — dream runs as an + // overnight cron, where a lingering-socket hang is a silent zombie + // (closes the TODOS.md drain-before-owner-disconnect item). + if (eng) await finishCliTeardown({ engine: eng }); } return; } @@ -1517,7 +1482,7 @@ async function handleCliOnly(command: string, args: string[]) { } throw e; } finally { - try { await engine.disconnect(); } catch { /* best-effort */ } + await finishCliTeardown({ engine }); } return; } @@ -1569,7 +1534,7 @@ async function handleCliOnly(command: string, args: string[]) { // so wrappers (sync, CI scripts, `&& gbrain doctor`) propagate. const importResult = await runImport(engine, args); if (importResult.errors > 0) { - process.exitCode = 1; + setCliExitVerdict(1); } break; } @@ -1992,31 +1957,16 @@ async function handleCliOnly(command: string, args: string[]) { } } finally { syncWatchdog?.dispose(); // #1633: tear down the hard-deadline watchdog on clean exit - // v0.42.20.0 (#1762) — the CLI_ONLY path (which owns `gbrain capture`) - // lacked the op-dispatch drain-before-disconnect contract. `put_page` fires - // a fire-and-forget facts:absorb job AFTER printing the receipt; on a - // multi-chunk page that job is in flight when this finally tears the engine - // down, and `engine.disconnect()` nulling PGLite's _db mid-job spins - // db.close() into a 100%-CPU busy-loop that pins the single-writer lock. - // Drain every background-work sink first (facts shutdown() abort cancels a - // hung Haiku), THEN disconnect. The drain-before-disconnect is the causal - // fix; the force-exit defense below is secondary (it CANNOT preempt a WASM - // busy-loop on a pinned JS thread — that's exactly why the drain matters). - // #1471: this is also the fall-through OWNER-disconnect — the owner is torn - // down LAST (after the drain), so module-singleton borrowers never outlive it. + // #2084 — the CLI_ONLY fall-through teardown (drain every background-work + // sink, THEN disconnect, under a computed-deadline backstop) lives in + // finishCliTeardown. `gbrain capture`'s fire-and-forget facts:absorb job + // gets its drain window before PGLite's db.close() can race it into the + // re-pump busy-loop (#1762). #1471: this is also the fall-through + // OWNER-disconnect — the owner is torn down LAST (after the drain), so + // module-singleton borrowers never outlive it. `serve` skips teardown + // entirely: the daemon owns its lifecycle. if (command !== 'serve') { - const forceExit = shouldForceExitAfterMain(); - let hardExitTimer: ReturnType | undefined; - if (forceExit) { - hardExitTimer = setTimeout(() => { - console.warn('[cli] engine.disconnect() did not return within 10000ms — force-exiting'); - process.exit(process.exitCode ?? 0); - }, 10_000); - hardExitTimer.unref?.(); - } - await drainAllBackgroundWorkForCliExit(); - await engine.disconnect(); - if (hardExitTimer) clearTimeout(hardExitTimer); + await finishCliTeardown({ engine }); } } } @@ -2328,9 +2278,25 @@ Run gbrain --help for command-specific help. // Only auto-run when invoked as the entry point (the compiled binary or // `bun src/cli.ts`). Guarded so tests can import cliAliases / printOpHelp // without triggering argv parsing + main(). v114 (#1941). +// +// #2084 — the ONE process-exit seam for one-shot commands. Every teardown site +// routes through finishCliTeardown (which returns); the exit itself happens +// here, after main() settles, so the CLI never waits on Bun's event loop to +// drain (stuck PgBouncer sockets kept it alive — endPoolBounded races PAST a +// stuck pool.end() by design). flushThenExit fences stdout/stderr and holds a +// short aliveness grace so piped output is delivered before exit (#1959). +// Daemons (`serve`) are excluded by shouldForceExitAfterMain and keep the +// pre-#2084 behavior: main() resolves and the server's own work keeps the +// process alive. A fatal error still exits 1 for every command, daemons +// included (matches the prior unconditional process.exit(1) on rejection). if (import.meta.main) { - main().catch(e => { - console.error(e.message || e); - process.exit(1); - }); + main().then( + () => { + if (shouldForceExitAfterMain()) flushThenExit(currentExitCode()); + }, + (e) => { + console.error(e.message || e); + flushThenExit(1); + }, + ); } diff --git a/src/commands/autopilot.ts b/src/commands/autopilot.ts index 7ac05cfbc..fd232eb6f 100644 --- a/src/commands/autopilot.ts +++ b/src/commands/autopilot.ts @@ -18,6 +18,7 @@ */ import { existsSync, readFileSync, writeFileSync, mkdirSync, appendFileSync, utimesSync, unlinkSync } from 'fs'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; import { join } from 'path'; import { execSync } from 'child_process'; import type { BrainEngine } from '../core/engine.ts'; @@ -547,7 +548,7 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) { `Exiting so launchd ThrottleInterval can apply backoff.`, ); stopping = true; - process.exitCode = 1; + setCliExitVerdict(1); break; } if (autopilotReconnectFails >= AUTOPILOT_MAX_RECONNECT_FAILS) { @@ -556,7 +557,7 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) { `Last error: ${(e as Error).message ?? 'unknown'}. Exiting.`, ); stopping = true; - process.exitCode = 1; + setCliExitVerdict(1); break; } } diff --git a/src/commands/brainstorm.ts b/src/commands/brainstorm.ts index 253bd4ad9..3f1e0b865 100644 --- a/src/commands/brainstorm.ts +++ b/src/commands/brainstorm.ts @@ -13,6 +13,7 @@ */ import type { BrainEngine } from '../core/engine.ts'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; import { runBrainstorm, formatBrainstormMarkdown, @@ -322,7 +323,7 @@ async function runBrainstormCli( const msg = formatSaveOutcome(outcome, { profileLabel: profile.label, slug }); if (msg.stdout) console.log(msg.stdout); for (const line of msg.stderr) console.error(line); - if (msg.exitCode) process.exitCode = msg.exitCode; + if (msg.exitCode) setCliExitVerdict(msg.exitCode); } } diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 028a8ab4d..f27772ac8 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,4 +1,5 @@ import type { BrainEngine } from '../core/engine.ts'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; import * as db from '../core/db.ts'; import { LATEST_VERSION, getIdleBlockers } from '../core/migrate.ts'; import { checkResolvable } from '../core/check-resolvable.ts'; @@ -7228,7 +7229,7 @@ export async function runDoctor( // Use process.exitCode instead of process.exit() so cleanup handlers // (e.g. Bun unload events, open database connections) still run before // the process terminates. process.exit() is a hard kill that bypasses them. - process.exitCode = hasFail ? 1 : 0; + setCliExitVerdict(hasFail ? 1 : 0); } // --------------------------------------------------------------------------- diff --git a/src/commands/extract.ts b/src/commands/extract.ts index e6b5a4077..01f5dbe43 100644 --- a/src/commands/extract.ts +++ b/src/commands/extract.ts @@ -29,6 +29,7 @@ */ import { readFileSync, readdirSync, lstatSync, existsSync } from 'fs'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; import { join, relative, dirname } from 'path'; import type { BrainEngine, LinkBatchInput, TimelineBatchInput } from '../core/engine.ts'; import type { PageType } from '../core/types.ts'; @@ -864,7 +865,7 @@ Status (v0.42): (r.first_batch_error ? ` (first error: ${r.first_batch_error})` : '') + ` — timeline is incomplete.`, ); - process.exitCode = 1; + setCliExitVerdict(1); } } else if (byMention || ner) { // v0.41.18.0 (T7): combined --by-mention + --ner walk shares one diff --git a/src/commands/frontmatter.ts b/src/commands/frontmatter.ts index 77a8fba4c..4e5295a9d 100644 --- a/src/commands/frontmatter.ts +++ b/src/commands/frontmatter.ts @@ -16,6 +16,7 @@ */ import { readFileSync, writeFileSync, existsSync, lstatSync, readdirSync } from 'fs'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; import { join, relative, resolve } from 'path'; import type { BrainEngine } from '../core/engine.ts'; import { loadConfig, toEngineConfig } from '../core/config.ts'; @@ -63,7 +64,7 @@ export async function runFrontmatter(args: string[]): Promise { } console.error(`Unknown frontmatter subcommand: ${sub}\n`); printHelp(); - process.exitCode = 1; + setCliExitVerdict(1); } async function connectEngineForAudit(): Promise { @@ -164,14 +165,14 @@ async function runValidate(rest: string[]): Promise { } if (!target) { console.error('error: gbrain frontmatter validate requires a argument'); - process.exitCode = 1; + setCliExitVerdict(1); return; } const resolved = resolve(target); if (!existsSync(resolved)) { console.error(`error: path not found: ${target}`); - process.exitCode = 1; + setCliExitVerdict(1); return; } @@ -242,7 +243,7 @@ async function runValidate(rest: string[]): Promise { } } - process.exitCode = totalErrors > 0 && !flags.fix ? 1 : 0; + setCliExitVerdict(totalErrors > 0 && !flags.fix ? 1 : 0); } /** @@ -378,7 +379,7 @@ async function runGenerate(args: string[]): Promise { if (!targetPath) { console.error('error: gbrain frontmatter generate requires a argument'); console.error('usage: gbrain frontmatter generate [--fix] [--dry-run] [--json]'); - process.exitCode = 1; + setCliExitVerdict(1); return; } diff --git a/src/commands/reindex.ts b/src/commands/reindex.ts index 6e5245ac4..77afe1c3f 100644 --- a/src/commands/reindex.ts +++ b/src/commands/reindex.ts @@ -22,6 +22,7 @@ */ import type { BrainEngine } from '../core/engine.ts'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; import { MARKDOWN_CHUNKER_VERSION } from '../core/chunkers/recursive.ts'; import { importFromContent, importFromFile } from '../core/import-file.ts'; import { serializeMarkdown } from '../core/markdown.ts'; @@ -150,7 +151,7 @@ export async function runReindex(engine: BrainEngine, args: string[]): Promise10s op mid-run with exit 0 and + * truncated output). + * + * Daemons: `serve` is excluded at both layers — its command never reaches a + * finishCliTeardown call site, and the central exit is gated by + * `shouldForceExitAfterMain`. The helper itself has NO daemon flag: the drain + * it runs is CLI-exit-only (it can permanently shut down process-level sinks), + * so a long-lived process must simply never call it. + * + * This module stays importable without cli.ts side effects so tests can drive + * every path directly (cli.ts is a script entrypoint). */ +import { drainAllBackgroundWorkForCliExit, backgroundWorkSinkCount } from './background-work.ts'; +import { POOL_END_TIMEOUT_SECONDS } from './db.ts'; +import { parseGlobalFlags } from './cli-options.ts'; + const DAEMON_COMMANDS: ReadonlySet = new Set(['serve']); export function shouldForceExitAfterMain( argv: string[] = process.argv.slice(2), ): boolean { - const command = argv.find((arg) => !arg.startsWith('-')); + // Resolve the command the same way main() does — parseGlobalFlags strips + // global flags INCLUDING space-separated values (`--timeout 30s`), so the + // command here always matches the dispatched one. The old first-non-dash + // heuristic saw `30s` as the command for `gbrain --timeout 30s serve` and + // (post-#2084, where this gates an unconditional process.exit) would have + // killed the daemon ~250ms after boot. Cross-model adversarial finding. + let command: string | undefined; + try { + command = parseGlobalFlags(argv).rest[0]; + } catch { + command = argv.find((arg) => !arg.startsWith('-')); + } if (!command) return true; return !DAEMON_COMMANDS.has(command); } + +/** Floor for the computed backstop deadline (the historical hard deadline). */ +export const TEARDOWN_DEADLINE_FLOOR_MS = 10_000; +/** Allowance for the facts sink's awaited abort() (shutdown of an in-flight job). */ +const FACTS_ABORT_GRACE_MS = 2_000; +/** Headroom over the sum of the guarded bounds so timer jitter can't false-fire. */ +const TEARDOWN_SLACK_MS = 2_000; +/** Max wait for the stdio flush fence before exiting anyway (blocked pipe). */ +const FLUSH_GUARD_MS = 2_000; +/** + * Aliveness grace between the fence and process.exit when stdio is NOT a TTY. + * Empirically verified (#2084 probes): Bun's process.stdout queues pipe writes + * in a native writer that only pushes to the fd on event-loop turns WHILE THE + * PROCESS IS ALIVE — process.exit discards the queue, natural event-loop exit + * discards it too, and no API reaches it (write callbacks fire on accept, not + * delivery; writableLength/bytesWritten read 0 throughout; + * Bun.stdout.writer().flush() is a different writer; fs.writeSync(1) is also + * queued). Staying alive briefly is the ONLY flush. TTY writes are synchronous + * — no grace needed there. + */ +const FLUSH_GRACE_PIPE_MS = 250; + +/** + * Resolve the non-TTY aliveness grace: `GBRAIN_FLUSH_GRACE_MS` env override + * (incident/batch escape hatch, same env-only pattern as + * GBRAIN_TEARDOWN_DEADLINE_MS) over the 250ms default. Consumers piping LARGE + * payloads into slow readers (a reader that attaches later than the grace + * loses the tail — Bun gives no delivery signal to wait on) can raise it; + * high-frequency agent loops capturing to files can lower it. + */ +function resolveFlushGraceMs(): number { + const env = Number(process.env.GBRAIN_FLUSH_GRACE_MS); + if (Number.isFinite(env) && env >= 0) return env; + return FLUSH_GRACE_PIPE_MS; +} +/** Default per-sink drain budget (matches drainAllBackgroundWorkForCliExit). */ +const DEFAULT_DRAIN_TIMEOUT_MS = 2_000; + +/** + * Backstop deadline for drain + disconnect COMBINED, computed from the bounds + * it guards so it fires only when a component violated its own bound (#2084 + * eng-review D9 — a static 10s fired on healthy-but-slow bounded teardown: + * 4 sinks × 2s + facts grace + 2 × ~2.5s pool ends ≈ 13s). + * `GBRAIN_TEARDOWN_DEADLINE_MS` overrides the formula (incident escape hatch, + * same env-only pattern as the GBRAIN_SYNC_* knobs). + */ +export function computeTeardownDeadlineMs(opts: { + sinkCount: number; + drainTimeoutMs: number; +}): number { + const env = Number(process.env.GBRAIN_TEARDOWN_DEADLINE_MS); + if (Number.isFinite(env) && env > 0) return env; + // +500 mirrors endPoolBounded's slack over the postgres.js hint (db.ts); + // ×2 budgets the worst case of two sequential pool ends (direct + read). + const poolEndBoundMs = POOL_END_TIMEOUT_SECONDS * 1000 + 500; + const computed = + opts.sinkCount * opts.drainTimeoutMs + + FACTS_ABORT_GRACE_MS + + 2 * poolEndBoundMs + + TEARDOWN_SLACK_MS; + return Math.max(TEARDOWN_DEADLINE_FLOOR_MS, computed); +} + +/** + * Minimal writable surface for the flush fence — process.stdout/stderr satisfy + * it; tests inject fakes. + */ +export interface MinimalWritable { + write(chunk: string, cb?: (err?: Error | null) => void): boolean; + once?(event: string, listener: (...args: unknown[]) => void): unknown; +} + +/** + * #2084 — the CLI's exit verdict lives in a gbrain-OWNED variable, never read + * back from `process.exitCode`. PGLite's Emscripten runtime writes its own + * status into `process.exitCode` at arbitrary points DURING a run (99 at + * create; in-memory brains run initdb whose exit status, e.g. 100, lands on a + * later event-loop turn — after any point-in-time snapshot), so the global is + * unreadable as a verdict channel on PGLite. Writers call `setCliExitVerdict` + * (which mirrors into `process.exitCode` for anything external that reads the + * global); the exit seam reads `currentExitCode()`, which trusts only the + * owned variable. No verdict set ⇒ 0. + */ +let cliVerdict: number | null = null; + +export function setCliExitVerdict(code: number): void { + cliVerdict = code; + process.exitCode = code; // best-effort mirror; never read back +} + +export function currentExitCode(): number { + return cliVerdict ?? 0; +} + +/** Test seam — clears the verdict so each test starts clean. */ +export function _resetCliExitVerdictForTests(): void { + cliVerdict = null; +} + +export interface FlushThenExitOpts { + exit?: (code: number) => void; + stdout?: MinimalWritable; + stderr?: MinimalWritable; + guardMs?: number; + /** + * Aliveness window between the fence and exit. Default: 0 when BOTH stdio + * streams are TTYs (synchronous writes), FLUSH_GRACE_PIPE_MS otherwise. + * The grace timer is deliberately ref'd — keeping the loop alive is the + * only thing that delivers Bun's queued pipe writes (see module constant). + */ + graceMs?: number; +} + +/** + * Flush stdout + stderr, then exit with `code` — exactly once. + * + * Two stages, both bounded: + * 1. Fence: an empty `write('', cb)` per stream serializes behind the accept + * queue; an unref'd guard bounds a stream whose callback never fires. + * (In Bun the callback fires on ACCEPT, not delivery — the fence alone is + * NOT sufficient; verified in the #2084 probes.) + * 2. Aliveness grace: a REF'D timer keeps the process alive `graceMs` so + * Bun's native writer can push the queued bytes to the fd / a consuming + * reader (#1959 truncation class). TTY stdio skips this (sync writes). + * + * A reader that consumes nothing for longer than guard+grace loses the tail — + * unavoidable without waiting forever; strictly better than the pre-#2084 + * behavior (immediate process.exit discarded everything still queued). + * + * `process.exitCode` is set up front so that even a stubbed `exit` (tests) or + * a natural event-loop exit keeps the right code. + */ +/** Process-level guard: the REAL process.exit fires at most once even if both + * the backstop and the central seam reach flushThenExit (test-injected exit + * fns are exempt so unit tests stay independent). */ +let realExitInitiated = false; + +export function flushThenExit(code: number, opts: FlushThenExitOpts = {}): void { + if (!opts.exit) { + if (realExitInitiated) return; + realExitInitiated = true; + } + const exit = opts.exit ?? ((c: number) => process.exit(c)); + const streams: MinimalWritable[] = [ + opts.stdout ?? process.stdout, + opts.stderr ?? process.stderr, + ]; + const guardMs = opts.guardMs ?? FLUSH_GUARD_MS; + const bothTty = streams.every((s) => (s as { isTTY?: boolean }).isTTY === true); + const graceMs = opts.graceMs ?? (bothTty ? 0 : resolveFlushGraceMs()); + process.exitCode = code; + let fenced = false; + let guard: ReturnType | undefined; + const finish = () => { + if (fenced) return; + fenced = true; + if (guard) clearTimeout(guard); + if (graceMs <= 0) { + exit(code); + return; + } + // Ref'd on purpose: aliveness IS the flush (Bun pipe-write semantics). + setTimeout(() => exit(code), graceMs); + }; + let pending = streams.length; + const done = () => { + pending -= 1; + if (pending <= 0) finish(); + }; + guard = setTimeout(finish, guardMs); + guard.unref?.(); + for (const s of streams) { + try { + // EPIPE on a closed pipe surfaces as an async 'error' event; swallow it — + // the guard or the other stream's callback still drives the exit. + s.once?.('error', () => {}); + s.write('', () => done()); + } catch { + done(); // sync EPIPE / destroyed stream + } + } +} + +export interface FinishCliTeardownOpts { + /** Engine to disconnect. A disconnect throw is warned + swallowed (D3). */ + engine: { disconnect(): Promise }; + /** Per-sink drain budget. Default 2000 (the registry default). */ + drainTimeoutMs?: number; + /** Test seam — wins over the env override and the computed formula. */ + deadlineMs?: number; + /** Forwarded to flushThenExit on the backstop path (test seam). */ + graceMs?: number; + // ---- test seams (default to the real thing) ---- + exit?: (code: number) => void; + warn?: (msg: string) => void; + drain?: (opts: { timeoutMs: number }) => Promise; + stdout?: MinimalWritable; + stderr?: MinimalWritable; +} + +/** + * CLI-EXIT-ONLY teardown: bounded drain of every background-work sink, then + * bounded engine disconnect, under a computed-deadline backstop. Returns to + * the caller — the explicit process exit happens once, in cli.ts's + * import.meta.main seam (see module header). The backstop timer is the ONLY + * exit in here, and it means a component violated its own bound. + */ +export async function finishCliTeardown(opts: FinishCliTeardownOpts): Promise { + const drainTimeoutMs = opts.drainTimeoutMs ?? DEFAULT_DRAIN_TIMEOUT_MS; + const warn = opts.warn ?? ((m: string) => console.warn(m)); + const drain = opts.drain ?? drainAllBackgroundWorkForCliExit; + const deadlineMs = + opts.deadlineMs ?? + computeTeardownDeadlineMs({ sinkCount: backgroundWorkSinkCount(), drainTimeoutMs }); + + const backstop = setTimeout(() => { + warn( + `[cli] teardown (background-work drain + engine.disconnect()) did not return within ${deadlineMs}ms — force-exiting`, + ); + // currentExitCode() reads the gbrain-owned verdict channel — an errored + // op's setCliExitVerdict(1) is honored even when PGLite has scribbled over + // process.exitCode; a bare exit(0) would mask the failure. + flushThenExit(currentExitCode(), opts); + }, deadlineMs); + // Deliberately REF'D (adversarial F3): if teardown hangs while nothing else + // keeps Bun's loop alive, an unref'd timer would let the process exit + // NATURALLY — skipping the flush and exiting with whatever PGLite scribbled + // into process.exitCode. The ref'd timer costs nothing on the clean path + // (cleared in the finally as soon as teardown returns). + + try { + try { + await drain({ timeoutMs: drainTimeoutMs }); + } catch (e) { + // The registry is contractually non-throwing, but a throw here must not + // skip the disconnect or escape a caller's finally (it would replace a + // successful op's completion). Same D3 posture as the disconnect guard. + warn( + `[cli] background-work drain failed during teardown: ${e instanceof Error ? e.message : String(e)} — continuing to disconnect`, + ); + } + try { + await opts.engine.disconnect(); + } catch (e) { + // D3: the exit code reports the operation, not the cleanup. Matches the + // non-throwing posture of endPoolBounded (db.ts). + warn( + `[cli] engine.disconnect() failed during teardown: ${e instanceof Error ? e.message : String(e)} — continuing to exit`, + ); + } + } finally { + clearTimeout(backstop); + } +} diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index e2cf4693a..8808efec5 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -195,6 +195,32 @@ export function buildPgliteInitErrorMessage( return `${header}\n${hint}\n Original error: ${original}`; } +/** + * #2084 — PGLite's Emscripten runtime hijacks `process.exitCode` as ITS status + * channel: instantiation REPLACES the property with an accessor whose getter + * falls back to the WASM runtime status (99 while alive, the exit status after + * close) whenever no explicit value was assigned — and assigning `undefined` + * resets to that fallback, so "unset" cannot be restored. Pre-fix, every clean + * PGLite run carried a bogus 99 until close zeroed it, and an errored op's + * exit 1 survived only by accident of write ordering. + * + * Containment: around PGlite.create(), snapshot the pre-call value and restore + * it — pinning an explicit 0 when nothing was set, because restoring + * `undefined` would surface the WASM fallback instead. This keeps the GLOBAL + * tidy for external readers; the CLI's own verdict never reads it (it lives in + * the owned channel: setCliExitVerdict/currentExitCode, cli-force-exit.ts — + * in-memory brains run initdb whose status lands on a later tick, past any + * snapshot). db.close() stays unwrapped (see the comment at the close site). + */ +async function preservingProcessExitCode(fn: () => Promise): Promise { + const pre = process.exitCode; + try { + return await fn(); + } finally { + process.exitCode = typeof pre === 'number' || typeof pre === 'string' ? pre : 0; + } +} + export class PGLiteEngine implements BrainEngine { readonly kind = 'pglite' as const; private _db: PGLiteDB | null = null; @@ -239,11 +265,13 @@ export class PGLiteEngine implements BrainEngine { } try { - this._db = await PGlite.create({ - dataDir, - loadDataDir, - extensions: { vector, pg_trgm }, - }); + this._db = await preservingProcessExitCode(() => + PGlite.create({ + dataDir, + loadDataDir, + extensions: { vector, pg_trgm }, + }), + ); } catch (err) { // 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 @@ -283,6 +311,12 @@ export class PGLiteEngine implements BrainEngine { this._lock = null; try { if (db) { + // Deliberately NOT wrapped in preservingProcessExitCode: close's + // status write (0) is long-standing baseline behavior that test-runner + // processes depend on (wrapping it flipped bun test's own exit code — + // #2084 implementation note), and the CLI's exit verdict doesn't read + // process.exitCode at all — it lives in the gbrain-owned channel + // (setCliExitVerdict/currentExitCode in cli-force-exit.ts). await db.close(); } } finally { diff --git a/test/cli-finish-teardown.test.ts b/test/cli-finish-teardown.test.ts new file mode 100644 index 000000000..fa1b4e4df --- /dev/null +++ b/test/cli-finish-teardown.test.ts @@ -0,0 +1,510 @@ +/** + * #2084 — unit tests for the one-shot CLI teardown + exit contract in + * src/core/cli-force-exit.ts: finishCliTeardown (teardown-only, computed + * backstop deadline), flushThenExit (write-fence + guard + EPIPE + once-latch), + * and computeTeardownDeadlineMs (formula / floor / env override). + * + * Real short timers, no fake clocks. Every test that touches process.exitCode + * or GBRAIN_TEARDOWN_DEADLINE_MS restores it in a finally so the suite stays + * order-independent. + */ + +import { describe, test, expect } from 'bun:test'; +import { + finishCliTeardown, + flushThenExit, + computeTeardownDeadlineMs, + TEARDOWN_DEADLINE_FLOOR_MS, + setCliExitVerdict, + currentExitCode, + _resetCliExitVerdictForTests, + type MinimalWritable, +} from '../src/core/cli-force-exit.ts'; +import { POOL_END_TIMEOUT_SECONDS } from '../src/core/db.ts'; +import { + backgroundWorkSinkCount, + __registerDrainerForTest, +} from '../src/core/background-work.ts'; +import { withEnv } from './helpers/with-env.ts'; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +function fakeStream(): MinimalWritable & { writes: string[] } { + const writes: string[] = []; + return { + writes, + write(chunk: string, cb?: (err?: Error | null) => void) { + writes.push(chunk); + if (cb) queueMicrotask(() => cb()); + return true; + }, + once() { + return this; + }, + }; +} + +describe('computeTeardownDeadlineMs', () => { + test('formula: sinks × drain + facts grace + 2 × pool bound + slack', () => { + const poolEndBoundMs = POOL_END_TIMEOUT_SECONDS * 1000 + 500; + // 4 sinks × 2000 + 2000 + 2×poolEnd + 2000 — the Site B worst case that + // falsified the old static 10s (eng-review D9). + const got = computeTeardownDeadlineMs({ sinkCount: 4, drainTimeoutMs: 2000 }); + expect(got).toBe(4 * 2000 + 2000 + 2 * poolEndBoundMs + 2000); + expect(got).toBeGreaterThan(10_000); // the codex-found arithmetic bug, pinned + }); + + test('floors at TEARDOWN_DEADLINE_FLOOR_MS for small budgets', () => { + const got = computeTeardownDeadlineMs({ sinkCount: 1, drainTimeoutMs: 100 }); + expect(got).toBe(TEARDOWN_DEADLINE_FLOOR_MS); + }); + + test('GBRAIN_TEARDOWN_DEADLINE_MS env override wins over the formula', async () => { + await withEnv({ GBRAIN_TEARDOWN_DEADLINE_MS: '1234' }, async () => { + expect(computeTeardownDeadlineMs({ sinkCount: 4, drainTimeoutMs: 2000 })).toBe(1234); + }); + }); + + test('garbage env values fall back to the formula', async () => { + await withEnv({ GBRAIN_TEARDOWN_DEADLINE_MS: 'banana' }, async () => { + expect( + computeTeardownDeadlineMs({ sinkCount: 1, drainTimeoutMs: 100 }), + ).toBe(TEARDOWN_DEADLINE_FLOOR_MS); + }); + }); + + test('zero and negative env values fall back to the formula (not "fire immediately")', async () => { + await withEnv({ GBRAIN_TEARDOWN_DEADLINE_MS: '0' }, async () => { + expect(computeTeardownDeadlineMs({ sinkCount: 1, drainTimeoutMs: 100 })).toBe( + TEARDOWN_DEADLINE_FLOOR_MS, + ); + }); + await withEnv({ GBRAIN_TEARDOWN_DEADLINE_MS: '-5' }, async () => { + expect(computeTeardownDeadlineMs({ sinkCount: 1, drainTimeoutMs: 100 })).toBe( + TEARDOWN_DEADLINE_FLOOR_MS, + ); + }); + }); + + test('a newly registered sink widens the computed deadline (D9: formula reads the live registry)', () => { + // Register two sinks and compare between them: in a bare unit-test process + // no production sinks are loaded, so the zero-sink baseline sits below the + // 10s floor and would mask the first sink's delta. + const mkSink = (name: string) => + __registerDrainerForTest({ name, order: 99, drain: async () => ({ unfinished: 0 }) }); + const un1 = mkSink('test-2084-sink-a'); + try { + const withOne = computeTeardownDeadlineMs({ + sinkCount: backgroundWorkSinkCount(), + drainTimeoutMs: 5000, + }); + const un2 = mkSink('test-2084-sink-b'); + try { + const withTwo = computeTeardownDeadlineMs({ + sinkCount: backgroundWorkSinkCount(), + drainTimeoutMs: 5000, + }); + expect(withOne).toBeGreaterThan(TEARDOWN_DEADLINE_FLOOR_MS); // above the floor — delta is visible + expect(withTwo).toBe(withOne + 5000); + } finally { + un2(); + } + } finally { + un1(); + } + }); +}); + +describe('finishCliTeardown — clean path', () => { + test('drains with the injected budget, disconnects, returns; no exit, no warn', async () => { + const calls: string[] = []; + let drainBudget = -1; + const exits: number[] = []; + const warns: string[] = []; + await finishCliTeardown({ + engine: { disconnect: async () => void calls.push('disconnect') }, + drainTimeoutMs: 777, + deadlineMs: 250, + drain: async ({ timeoutMs }) => { + drainBudget = timeoutMs; + calls.push('drain'); + }, + exit: (c) => void exits.push(c), + warn: (m) => void warns.push(m), + stdout: fakeStream(), + stderr: fakeStream(), + }); + // Past the 250ms deadline: a leaked backstop would fire here. + await sleep(400); + expect(calls).toEqual(['drain', 'disconnect']); + expect(drainBudget).toBe(777); + expect(exits).toEqual([]); + expect(warns).toEqual([]); + }); + + test('drain runs BEFORE disconnect (live-engine window for sinks)', async () => { + const order: string[] = []; + await finishCliTeardown({ + engine: { disconnect: async () => void order.push('disconnect') }, + deadlineMs: 1000, + drain: async () => { + await sleep(20); + order.push('drain'); + }, + exit: () => {}, + warn: () => {}, + }); + expect(order).toEqual(['drain', 'disconnect']); + }); +}); + +describe('finishCliTeardown — backstop on hung teardown', () => { + test('hung disconnect fires the banner and exits with current exitCode', async () => { + const prevCode = process.exitCode; + try { + _resetCliExitVerdictForTests(); // no verdict set ⇒ currentExitCode() === 0 + const exits: number[] = []; + const warns: string[] = []; + let resolveHang!: () => void; + const teardown = finishCliTeardown({ + engine: { disconnect: () => new Promise((r) => (resolveHang = r)) }, + deadlineMs: 100, + drain: async () => {}, + exit: (c) => void exits.push(c), + warn: (m) => void warns.push(m), + stdout: fakeStream(), + stderr: fakeStream(), + graceMs: 0, + }); + await sleep(300); + expect(warns.length).toBe(1); + expect(warns[0]).toContain('did not return within'); + expect(warns[0]).toContain('100ms'); + expect(exits).toEqual([0]); + resolveHang(); // unhang so the promise settles + await teardown; + } finally { + _resetCliExitVerdictForTests(); + process.exitCode = prevCode; + } + }); + + test('backstop honors an exit code the errored op already set', async () => { + const prevCode = process.exitCode; + try { + setCliExitVerdict(1); // what the op-dispatch catch does + const exits: number[] = []; + let resolveHang!: () => void; + const teardown = finishCliTeardown({ + engine: { disconnect: () => new Promise((r) => (resolveHang = r)) }, + deadlineMs: 100, + drain: async () => {}, + exit: (c) => void exits.push(c), + warn: () => {}, + stdout: fakeStream(), + stderr: fakeStream(), + graceMs: 0, + }); + await sleep(300); + expect(exits).toEqual([1]); + resolveHang(); + await teardown; + } finally { + _resetCliExitVerdictForTests(); + process.exitCode = prevCode; + } + }); + + test('hung DRAIN (not just disconnect) also trips the backstop', async () => { + const prevCode = process.exitCode; + try { + _resetCliExitVerdictForTests(); + const exits: number[] = []; + const warns: string[] = []; + let resolveHang!: () => void; + const teardown = finishCliTeardown({ + engine: { disconnect: async () => {} }, + deadlineMs: 100, + drain: () => new Promise((r) => (resolveHang = r)), + exit: (c) => void exits.push(c), + warn: (m) => void warns.push(m), + stdout: fakeStream(), + stderr: fakeStream(), + graceMs: 0, + }); + await sleep(300); + expect(warns.length).toBe(1); + expect(exits).toEqual([0]); + resolveHang(); + await teardown; + } finally { + process.exitCode = prevCode; + } + }); +}); + +describe('verdict channel — immune to PGLite WASM process.exitCode writes', () => { + test('engine teardown that rewrites process.exitCode does not change the verdict', async () => { + // PGLite's Emscripten runtime writes its own status into process.exitCode + // at arbitrary points (99 at create, initdb status on a later tick for + // in-memory brains, 0 at close) — pre-#2084 this clobbered an errored + // op's exit 1 back to 0 on every PGLite error path. The verdict lives in + // the gbrain-owned channel and never reads the global back. + const prevCode = process.exitCode; + try { + setCliExitVerdict(1); // the op errored + await finishCliTeardown({ + engine: { + disconnect: async () => { + process.exitCode = 0; // what PGLite's WASM shutdown does + }, + }, + deadlineMs: 1000, + drain: async () => {}, + exit: () => {}, + warn: () => {}, + }); + expect(currentExitCode()).toBe(1); + } finally { + _resetCliExitVerdictForTests(); + process.exitCode = prevCode; + } + }); + + test('mid-run WASM write (in-memory initdb status) cannot fake a verdict', () => { + _resetCliExitVerdictForTests(); + try { + process.exitCode = 100; // what in-memory PGLite's initdb does mid-run + expect(currentExitCode()).toBe(0); // no gbrain verdict was ever set + setCliExitVerdict(2); + expect(currentExitCode()).toBe(2); + // The mirror write exists for EXTERNAL readers of the global. + expect(process.exitCode).toBe(2); + } finally { + _resetCliExitVerdictForTests(); + process.exitCode = 0; + } + }); +}); + +describe('finishCliTeardown — disconnect failure (D3: exit code reports the op)', () => { + test('a throwing drain is warned, disconnect still runs, helper resolves', async () => { + // The registry is contractually non-throwing; this pins the defense-in-depth + // guard — a drain rejection must not skip disconnect or escape the caller's + // finally (it would replace a successful op's completion). + const calls: string[] = []; + const warns: string[] = []; + await finishCliTeardown({ + engine: { disconnect: async () => void calls.push('disconnect') }, + deadlineMs: 1000, + drain: async () => { + throw new Error('sink registry blew up'); + }, + exit: () => {}, + warn: (m) => void warns.push(m), + }); + expect(calls).toEqual(['disconnect']); + expect(warns.length).toBe(1); + expect(warns[0]).toContain('sink registry blew up'); + }); + + test('disconnect throw is warned and swallowed; helper resolves', async () => { + const warns: string[] = []; + const exits: number[] = []; + await finishCliTeardown({ + engine: { + disconnect: async () => { + throw new Error('pool already dead'); + }, + }, + deadlineMs: 1000, + drain: async () => {}, + exit: (c) => void exits.push(c), + warn: (m) => void warns.push(m), + }); + expect(warns.length).toBe(1); + expect(warns[0]).toContain('pool already dead'); + expect(exits).toEqual([]); // helper never exits on the non-backstop path + }); +}); + +describe('flushThenExit', () => { + test('exits after BOTH stream callbacks fire, exactly once, with the code', async () => { + const prevCode = process.exitCode; + try { + const events: string[] = []; + const exits: number[] = []; + const slowStream = (name: string): MinimalWritable => ({ + write(_c: string, cb?: (err?: Error | null) => void) { + setTimeout(() => { + events.push(`${name}-flushed`); + cb?.(); + }, 50); + return true; + }, + once() { + return this; + }, + }); + flushThenExit(3, { + exit: (c) => { + events.push('exit'); + exits.push(c); + }, + stdout: slowStream('stdout'), + stderr: slowStream('stderr'), + guardMs: 2000, + graceMs: 0, + }); + await sleep(200); + expect(events).toEqual(['stdout-flushed', 'stderr-flushed', 'exit']); + expect(exits).toEqual([3]); + expect(process.exitCode).toBe(3); // belt-and-braces for natural exit + } finally { + process.exitCode = prevCode; + } + }); + + test('non-TTY default: exit waits the aliveness grace AFTER the fence', async () => { + const prevCode = process.exitCode; + try { + const exits: number[] = []; + const t0 = Date.now(); + let fencedAt = -1; + const stream: MinimalWritable = { + write(_c: string, cb?: (err?: Error | null) => void) { + fencedAt = Date.now() - t0; + if (cb) queueMicrotask(() => cb()); + return true; + }, + once() { + return this; + }, + }; + flushThenExit(0, { + exit: (c) => void exits.push(c), + stdout: stream, + stderr: stream, + guardMs: 2000, + graceMs: 120, // fakes are non-TTY; explicit grace keeps the test tight + }); + await sleep(60); + expect(exits).toEqual([]); // fence done, still inside the grace window + await sleep(150); + expect(exits).toEqual([0]); + expect(fencedAt).toBeGreaterThanOrEqual(0); + } finally { + process.exitCode = prevCode; + } + }); + + test('guard fires when a callback never arrives (blocked pipe)', async () => { + const prevCode = process.exitCode; + try { + const exits: number[] = []; + const blockedStream: MinimalWritable = { + write() { + return false; // never calls cb — reader stopped consuming + }, + once() { + return this; + }, + }; + const t0 = Date.now(); + flushThenExit(0, { + exit: (c) => void exits.push(c), + stdout: blockedStream, + stderr: blockedStream, + guardMs: 100, + graceMs: 0, + }); + await sleep(300); + expect(exits).toEqual([0]); + expect(Date.now() - t0).toBeGreaterThanOrEqual(100); + } finally { + process.exitCode = prevCode; + } + }); + + test('sync write throw (EPIPE) still exits', async () => { + const prevCode = process.exitCode; + try { + const exits: number[] = []; + const epipeStream: MinimalWritable = { + write() { + throw Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); + }, + once() { + return this; + }, + }; + flushThenExit(0, { + exit: (c) => void exits.push(c), + stdout: epipeStream, + stderr: epipeStream, + guardMs: 2000, + graceMs: 0, + }); + await sleep(50); + expect(exits).toEqual([0]); + } finally { + process.exitCode = prevCode; + } + }); + + test('GBRAIN_FLUSH_GRACE_MS env override is honored (batch/incident knob)', async () => { + const prevCode = process.exitCode; + try { + await withEnv({ GBRAIN_FLUSH_GRACE_MS: '0' }, async () => { + const exits: number[] = []; + flushThenExit(0, { + exit: (c) => void exits.push(c), + stdout: fakeStream(), + stderr: fakeStream(), + guardMs: 2000, + // no graceMs → resolves through the env override (fakes are non-TTY) + }); + await sleep(60); + expect(exits).toEqual([0]); // grace 0: exit right after the fence + }); + } finally { + process.exitCode = prevCode; + } + }); + + test('once-latch: guard + late callbacks cannot double-exit', async () => { + const prevCode = process.exitCode; + try { + const exits: number[] = []; + // stdout flushes late (after the guard), stderr never — both race finish(). + const lateStream: MinimalWritable = { + write(_c: string, cb?: (err?: Error | null) => void) { + setTimeout(() => cb?.(), 150); + return true; + }, + once() { + return this; + }, + }; + const neverStream: MinimalWritable = { + write() { + return false; + }, + once() { + return this; + }, + }; + flushThenExit(0, { + exit: (c) => void exits.push(c), + stdout: lateStream, + stderr: neverStream, + guardMs: 80, + graceMs: 0, + }); + await sleep(400); + expect(exits).toEqual([0]); // exactly one exit despite guard + late cb + } finally { + process.exitCode = prevCode; + } + }); +}); diff --git a/test/cli-force-exit-teardown-arming.test.ts b/test/cli-force-exit-teardown-arming.test.ts index ff9743e30..5d33b8602 100644 --- a/test/cli-force-exit-teardown-arming.test.ts +++ b/test/cli-force-exit-teardown-arming.test.ts @@ -1,56 +1,55 @@ /** - * Structural regression — the DISCONNECT_HARD_DEADLINE_MS force-exit timer in - * cli.ts main() must be armed at TEARDOWN ENTRY (inside the finally, before - * the drain + disconnect), never before the op-dispatch try block. + * Structural regression — the teardown hard-deadline must be armed at + * TEARDOWN ENTRY, never before the op-dispatch body. * - * Pre-fix bug: the 10s unref'd setTimeout was armed BEFORE the try, so any op - * whose handler ran past 10s wall-clock was killed mid-flight with - * process.exit(0) and ZERO stdout — an empty "success" indistinguishable from - * no results (a healthy `gbrain search` on a slow Postgres pooler hit this on - * every run). Armed in the finally, the timer still bounds a hung - * drain/disconnect (the C13 contract) but can no longer kill a - * slow-but-progressing op body. + * Pre-fix bug (closed independently by v0.42.41.0 and the #2084 wave, merged): + * a 10s unref'd setTimeout armed BEFORE the try killed any op whose handler + * ran past 10s wall-clock with process.exit(0) and ZERO stdout — an empty + * "success" indistinguishable from no results. * - * Source-grep is the right tool here (same rationale as - * fix-wave-structural.test.ts): the rule is "this arming must stay at this - * location". A behavioral test would need >10s of real wall-clock plus a - * deliberately slow op handler in a spawned CLI — slow and flaky by - * construction. + * Post-merge shape (#2084): the deadline lives inside `finishCliTeardown` + * (src/core/cli-force-exit.ts), armed as the helper's first act — i.e. at + * teardown entry, because every cli.ts call site invokes the helper from a + * `finally`. The op body's wallclock is bounded separately by the read-scope + * withTimeout wrap (v0.42.41.0). Source-grep is the right tool here (same + * rationale as fix-wave-structural.test.ts): a behavioral test would need + * >10s of real wall-clock in a spawned CLI. */ import { describe, test, expect } from 'bun:test'; import { readFileSync } from 'fs'; describe('cli.ts — disconnect hard-deadline armed at teardown entry, not before the op body', () => { - test('forceExitTimer setTimeout lives inside the finally, gated on the daemon guard, before the drain', () => { - const src = readFileSync('src/cli.ts', 'utf8'); + test('no timer arming exists between op-dispatch setup and the try; the deadline arms inside finishCliTeardown before the drain', () => { + const cli = readFileSync('src/cli.ts', 'utf8'); - const decl = src.indexOf('const DISCONNECT_HARD_DEADLINE_MS'); - expect(decl).toBeGreaterThan(-1); - const tryIdx = src.indexOf('try {', decl); + // The old pre-try arming constant must stay gone (its return is the + // kill-slow-ops-with-exit-0 regression). + expect(cli).not.toContain('DISCONNECT_HARD_DEADLINE_MS'); + + // Between the op-dispatch engine connect and the try there is no + // setTimeout call site (`setTimeout(` matches calls only; the + // ReturnType annotation stays allowed). + const connectIdx = cli.indexOf('// Local engine path (unchanged behavior for local installs).'); + expect(connectIdx).toBeGreaterThan(-1); + const tryIdx = cli.indexOf('try {', connectIdx); expect(tryIdx).toBeGreaterThan(-1); - const finallyIdx = src.indexOf('} finally {', tryIdx); + expect(cli.slice(connectIdx, tryIdx)).not.toContain('setTimeout('); + + // The op-dispatch finally routes through the shared teardown helper. + const finallyIdx = cli.indexOf('} finally {', tryIdx); expect(finallyIdx).toBeGreaterThan(-1); - const armIdx = src.indexOf('forceExitTimer = setTimeout', decl); + const teardownCallIdx = cli.indexOf('finishCliTeardown({ engine, drainTimeoutMs: 1000 })', finallyIdx); + expect(teardownCallIdx).toBeGreaterThan(finallyIdx); + + // Inside the helper, the backstop arms BEFORE the drain runs — teardown + // entry, bounding drain + disconnect and nothing else. + const helper = readFileSync('src/core/cli-force-exit.ts', 'utf8'); + const armIdx = helper.indexOf('const backstop = setTimeout('); expect(armIdx).toBeGreaterThan(-1); - const drainIdx = src.indexOf('drainAllBackgroundWorkForCliExit', finallyIdx); - expect(drainIdx).toBeGreaterThan(-1); - - // NO arming between the deadline declaration and the op-body try — a - // pre-try timer kills slow-but-progressing op handlers mid-flight with - // exit 0 and empty stdout. (`setTimeout(` matches only a call site; the - // `ReturnType` type annotation stays allowed.) - expect(src.slice(decl, tryIdx)).not.toContain('setTimeout('); - - // The arming sits AFTER the finally opens (teardown entry) and BEFORE the - // drain + disconnect it exists to bound. - expect(armIdx).toBeGreaterThan(finallyIdx); - expect(armIdx).toBeLessThan(drainIdx); - - // Still gated on the daemon-survival guard so `serve` stays alive, and - // still unref'd + cleared on clean teardown. - expect(src.slice(finallyIdx, drainIdx)).toMatch(/if \(shouldForceExitAfterMain\(\)\)/); - expect(src.slice(finallyIdx, drainIdx)).toContain('forceExitTimer.unref?.()'); - expect(src.slice(drainIdx)).toContain('if (forceExitTimer) clearTimeout(forceExitTimer)'); + const drainIdx = helper.indexOf('await drain({ timeoutMs: drainTimeoutMs })', armIdx); + expect(drainIdx).toBeGreaterThan(armIdx); + // Cleared on clean teardown. + expect(helper.indexOf('clearTimeout(backstop)', drainIdx)).toBeGreaterThan(drainIdx); }); }); diff --git a/test/cli-should-force-exit.test.ts b/test/cli-should-force-exit.test.ts index d060c7fde..9831c7325 100644 --- a/test/cli-should-force-exit.test.ts +++ b/test/cli-should-force-exit.test.ts @@ -38,6 +38,18 @@ describe('shouldForceExitAfterMain — daemon survival gate', () => { expect(shouldForceExitAfterMain(['get', 'people/alice'])).toBe(true); }); + test('#2084 cross-model finding: space-separated global flag values cannot fake a command', () => { + // `--timeout 30s serve` — the old first-non-dash heuristic resolved the + // command as `30s` → true → the central exit seam would process.exit the + // freshly started daemon ~250ms after boot, exit 0, no error. The gate now + // resolves the command through parseGlobalFlags, matching main()'s dispatch. + expect(shouldForceExitAfterMain(['--timeout', '30s', 'serve'])).toBe(false); + expect(shouldForceExitAfterMain(['--timeout', '30s', 'serve', '--http'])).toBe(false); + expect(shouldForceExitAfterMain(['--progress-interval', '500', 'serve'])).toBe(false); + // ...and the same shape before a one-shot command still force-exits. + expect(shouldForceExitAfterMain(['--timeout', '30s', 'query', 'x'])).toBe(true); + }); + test('returns true for non-daemon CLI commands', () => { expect(shouldForceExitAfterMain(['stats'])).toBe(true); expect(shouldForceExitAfterMain(['doctor'])).toBe(true); diff --git a/test/e2e/pglite-cli-exit.serial.test.ts b/test/e2e/pglite-cli-exit.serial.test.ts index d7b293aa9..9291aee37 100644 --- a/test/e2e/pglite-cli-exit.serial.test.ts +++ b/test/e2e/pglite-cli-exit.serial.test.ts @@ -34,6 +34,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { spawn, spawnSync } from 'child_process'; import { + cpSync, mkdirSync, mkdtempSync, rmSync, @@ -152,12 +153,13 @@ afterAll(() => { function runWithTimeout( args: string[], timeoutMs: number, + envOverride?: Record, ): 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, + env: envOverride ? { ...runEnv, ...envOverride } : runEnv, }); let stdout = ''; let stderr = ''; @@ -173,6 +175,13 @@ function runWithTimeout( }); } +/** + * #2084: the teardown backstop banner must NEVER appear on a healthy run — + * it now means a teardown component violated its own bound, not "this + * command was slower than 10s end-to-end" (the pre-#2084 misfire). + */ +const TEARDOWN_BANNER = 'did not return within'; + 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( @@ -185,6 +194,7 @@ describe('v0.41.8.0 — PGLite CLI read commands exit cleanly (#1247/#1269/#1290 `STDOUT:\n${stdout}\nSTDERR:\n${stderr}`, ); } + expect(stderr).not.toContain(TEARDOWN_BANNER); expect(code).toBe(0); // Must have actually returned a hit — else bumpLastRetrievedAt // would have early-returned on empty pageIds and the bug wouldn't @@ -203,6 +213,7 @@ describe('v0.41.8.0 — PGLite CLI read commands exit cleanly (#1247/#1269/#1290 `STDOUT:\n${stdout}\nSTDERR:\n${stderr}`, ); } + expect(stderr).not.toContain(TEARDOWN_BANNER); expect(code).toBe(0); expect(stdout).toContain('foxtrot'); }, 30_000); @@ -258,6 +269,7 @@ describe('v0.42.20.0 — gbrain capture (CLI_ONLY) exits cleanly + frees the loc // The real lock-pin symptom: the NEXT command times out waiting for the // PGLite lock. Assert a subsequent read runs cleanly and quickly. + expect(cap.stderr).not.toContain(TEARDOWN_BANNER); const next = await runWithTimeout(['get', 'meetings/capture-test'], 15_000); expect(next.durationMs).toBeLessThan(15_000); expect(next.stderr).not.toContain('Timed out waiting for PGLite lock'); @@ -267,6 +279,136 @@ describe('v0.42.20.0 — gbrain capture (CLI_ONLY) exits cleanly + frees the loc }, 60_000); }); +describe('#2084 — explicit-exit teardown: every swept site exits clean, exit codes report the op', () => { + // D6C hardening: mutating commands run against a throwaway COPY of the + // seeded GBRAIN_HOME so a remediation/dream pass can't contaminate the + // brain the other tests share. + function copyBrainHome(label: string): string { + const copy = mkdtempSync(join(tmpdir(), `gbrain-2084-${label}-`)); + cpSync(tmpHome, copy, { recursive: true }); + return copy; + } + + test('D5: failed op exits 1 with the error on stderr (exit code = op outcome)', async () => { + const { code, stderr, durationMs } = await runWithTimeout( + ['get', 'nonexistent-slug-2084'], + 15_000, + ); + expect(durationMs).toBeLessThan(15_000); + expect(code).toBe(1); + expect(stderr.length).toBeGreaterThan(0); + expect(stderr).not.toContain(TEARDOWN_BANNER); + }, 30_000); + + test('search stats (dashboard path, Site C) exits 0, no banner', async () => { + const { code, stdout, stderr } = await runWithTimeout(['search', 'stats'], 20_000); + expect(code).toBe(0); + expect(stdout.length).toBeGreaterThan(0); + expect(stderr).not.toContain(TEARDOWN_BANNER); + }, 30_000); + + test('sources list (read-only timeout path, Site D) exits 0, no banner', async () => { + const { code, stderr } = await runWithTimeout(['sources', 'list'], 20_000); + expect(code).toBe(0); + expect(stderr).not.toContain(TEARDOWN_BANNER); + }, 30_000); + + test('doctor (Site G — leak-fix shape) exits without hanging, no banner', async () => { + const { code, durationMs, stderr } = await runWithTimeout(['doctor'], 45_000); + expect(durationMs).toBeLessThan(45_000); + expect(stderr).not.toContain(TEARDOWN_BANNER); + // Keyless CI may surface advisory findings; doctor's exit code reflects + // brain health, not teardown health. The pin is: exits, no banner. + expect(code).not.toBeNull(); + }, 60_000); + + test('doctor --remediation-plan (Site F) exits without hanging, no banner', async () => { + const { code, durationMs, stderr } = await runWithTimeout( + ['doctor', '--remediation-plan'], + 30_000, + ); + expect(durationMs).toBeLessThan(30_000); + expect(stderr).not.toContain(TEARDOWN_BANNER); + expect(code).not.toBeNull(); + }, 45_000); + + test('doctor --remediate (Site F, mutating) runs on a brain copy, exits, no banner', async () => { + const copy = copyBrainHome('remediate'); + try { + const { code, durationMs, stderr } = await runWithTimeout( + ['doctor', '--remediate'], + 45_000, + { GBRAIN_HOME: copy }, + ); + expect(durationMs).toBeLessThan(45_000); + expect(stderr).not.toContain(TEARDOWN_BANNER); + expect(code).not.toBeNull(); + } finally { + rmSync(copy, { recursive: true, force: true }); + } + }, 60_000); + + test('dream --dry-run (Site E — the overnight-cron TODO site) exits, no banner', async () => { + const copy = copyBrainHome('dream'); + try { + const { code, durationMs, stderr } = await runWithTimeout( + ['dream', '--dry-run'], + 60_000, + { GBRAIN_HOME: copy }, + ); + // Keyless CI: LLM-dependent phases degrade; the IRON rule is exits + no + // banner. A hang here is the silent-overnight-zombie regression. + expect(durationMs).toBeLessThan(60_000); + expect(stderr).not.toContain(TEARDOWN_BANNER); + expect(code).not.toBeNull(); + } finally { + rmSync(copy, { recursive: true, force: true }); + } + }, 90_000); + + test('ze-switch --dry-run (Site H) exits without hanging, no banner', async () => { + const { code, durationMs, stderr } = await runWithTimeout( + ['ze-switch', '--dry-run'], + 30_000, + ); + expect(durationMs).toBeLessThan(30_000); + expect(stderr).not.toContain(TEARDOWN_BANNER); + expect(code).not.toBeNull(); + }, 45_000); + + test('D11: teardown deadline does NOT cover handler time (slow-handler regression)', async () => { + // Post-fix the deadline arms at teardown start, so a 500ms deadline cannot + // touch the handler: results print in full regardless. NOTE the falsification + // story is forward-looking, not historical — pre-#2084 code had no env knob + // (a static 10s constant), so this spawn would pass there too; the guard + // against re-hoisting the timer above the handler is the structural pin on + // DISCONNECT_HARD_DEADLINE_MS absence in fix-wave-structural.test.ts. This + // test pins that the env override is honored AND output survives a deadline + // far smaller than handler time. + const { code, stdout, durationMs } = await runWithTimeout( + ['search', 'foxtrot', '--limit', '3'], + 15_000, + { GBRAIN_TEARDOWN_DEADLINE_MS: '500' }, + ); + expect(durationMs).toBeLessThan(15_000); + expect(code).toBe(0); + expect(stdout.length).toBeGreaterThan(0); // output intact = handler wasn't killed + }, 30_000); + + test('D10: piped --json output parses complete (no exit truncation)', async () => { + // `search stats --json` emits a pure JSON document (the shared-op search + // path renders human format regardless of --json). A truncated-by-exit + // pipe fails to parse — the #1959 class, end-to-end. + const { code, stdout, stderr } = await runWithTimeout( + ['search', 'stats', '--json'], + 20_000, + ); + expect(code).toBe(0); + expect(stderr).not.toContain(TEARDOWN_BANNER); + expect(() => JSON.parse(stdout)).not.toThrow(); + }, 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 diff --git a/test/eval-capture-drain.test.ts b/test/eval-capture-drain.test.ts index 2823027a3..7ab615d40 100644 --- a/test/eval-capture-drain.test.ts +++ b/test/eval-capture-drain.test.ts @@ -72,6 +72,9 @@ describe('awaitPendingEvalCaptures', () => { const r = await awaitPendingEvalCaptures(150); const elapsed = Date.now() - start; expect(r.unfinished).toBe(1); - expect(elapsed).toBeLessThan(1000); + // Bound proves "bounded, not a hang" — the alternative is infinite. 2s + // (13x the 150ms budget) absorbs CI shard-load timer jitter; the old 1s + // bound flaked at 1023ms on a loaded GitHub runner. + expect(elapsed).toBeLessThan(2000); }); }); diff --git a/test/fix-wave-structural.test.ts b/test/fix-wave-structural.test.ts index 16c9ccb75..e2b668e70 100644 --- a/test/fix-wave-structural.test.ts +++ b/test/fix-wave-structural.test.ts @@ -125,15 +125,15 @@ describe('v0.36.1.x #1124 — query --no-expand actually negates expand', () => describe('v0.42.20.0 — background-work registry drains every sink before disconnect', () => { // Supersedes the v0.41.8.0 #1247/#1269/#1290 per-call last-retrieved drain: - // last-retrieved is now one of four registry sinks; cli.ts drains the whole - // registry (drainAllBackgroundWorkForCliExit) before disconnect on BOTH the - // op-dispatch path AND the CLI_ONLY path (the latter closes #1762 for capture). - test('cli.ts imports + uses drainAllBackgroundWorkForCliExit', () => { - const src = readFileSync('src/cli.ts', 'utf8'); - expect(src).toMatch(/import\s+\{\s*drainAllBackgroundWorkForCliExit\s*\}\s*from\s+['"]\.\/core\/background-work\.ts['"]/); - // Two call sites: op-dispatch finally + handleCliOnly finally. - const calls = src.match(/await\s+drainAllBackgroundWorkForCliExit\s*\(/g) ?? []; - expect(calls.length).toBeGreaterThanOrEqual(2); + // last-retrieved is one of four registry sinks. #2084 moved the registry + // drain out of cli.ts's inline finallys into finishCliTeardown + // (cli-force-exit.ts), which every cli.ts teardown site routes through — + // the drain-before-disconnect invariant is pinned there (and behaviorally + // by test/cli-finish-teardown.test.ts). + test('cli-force-exit.ts imports + drains the registry inside finishCliTeardown', () => { + const src = readFileSync('src/core/cli-force-exit.ts', 'utf8'); + expect(src).toMatch(/import\s+\{\s*drainAllBackgroundWorkForCliExit[\s\S]*?\}\s*from\s+['"]\.\/background-work\.ts['"]/); + expect(src).toMatch(/export async function finishCliTeardown/); }); test('last-retrieved.ts still exports the bounded drain + registers a drainer', () => { @@ -155,17 +155,17 @@ describe('v0.42.20.0 — background-work registry drains every sink before disco .toMatch(/name:\s*'eval-capture'/); }); - test('cli.ts behavioral positioning: registry drain appears BEFORE engine.disconnect (op-dispatch)', () => { - const src = readFileSync('src/cli.ts', 'utf8'); - const localPath = src.match(/\/\/ Local engine path \(unchanged behavior[\s\S]+?^\}/m); - expect(localPath).not.toBeNull(); - const block = localPath![0]; - const drainCallRe = /await\s+drainAllBackgroundWorkForCliExit\s*\(/; - const disconnectCallRe = /await\s+engine\.disconnect\s*\(/; - expect(block).toMatch(drainCallRe); - expect(block).toMatch(disconnectCallRe); - const drainIdx = block.indexOf(block.match(drainCallRe)![0]); - const disconnectIdx = block.indexOf(block.match(disconnectCallRe)![0]); + test('finishCliTeardown positioning: registry drain appears BEFORE engine disconnect', () => { + // #2084: the invariant moved from cli.ts's inline finallys into the shared + // helper. The drain must run against a live engine (facts abort-path + // logIngest, #1762) before disconnect tears the pools down. + const src = readFileSync('src/core/cli-force-exit.ts', 'utf8'); + const drainCallRe = /await\s+drain\s*\(\s*\{\s*timeoutMs:\s*drainTimeoutMs\s*\}\s*\)/; + const disconnectCallRe = /await\s+opts\.engine\.disconnect\s*\(/; + expect(src).toMatch(drainCallRe); + expect(src).toMatch(disconnectCallRe); + const drainIdx = src.indexOf(src.match(drainCallRe)![0]); + const disconnectIdx = src.indexOf(src.match(disconnectCallRe)![0]); expect(drainIdx).toBeLessThan(disconnectIdx); }); @@ -184,6 +184,56 @@ describe('v0.42.20.0 — background-work registry drains every sink before disco }); }); +describe('#2084 — cli.ts owns process-exit teardown via finishCliTeardown', () => { + test('no bare awaited engine disconnects remain in cli.ts', () => { + // The awaited forms are the call-site contract (comments never use the + // awaited literal, so this is comment-proof — eng-review D13.2). A bare + // disconnect skips the bounded drain + computed-deadline backstop and + // reopens the lingering-socket hang class. + const src = readFileSync('src/cli.ts', 'utf8'); + expect(src).not.toContain('await engine.disconnect()'); + expect(src).not.toContain('await eng.disconnect()'); + }); + + test('the pre-handler hard-deadline timer is gone (handler time is not teardown budget)', () => { + // Pre-#2084 the op-dispatch timer armed BEFORE the op handler, so any op + // slower than 10s was force-killed mid-run with exit 0 and truncated + // output. The deadline now arms inside finishCliTeardown, at teardown + // start only. + const src = readFileSync('src/cli.ts', 'utf8'); + expect(src).not.toContain('DISCONNECT_HARD_DEADLINE_MS'); + }); + + test('all nine swept sites route through finishCliTeardown; one exit seam', () => { + const src = readFileSync('src/cli.ts', 'utf8'); + const calls = src.match(/await finishCliTeardown\(/g) ?? []; + expect(calls.length).toBeGreaterThanOrEqual(9); + // The single process-exit seam: flushThenExit in the import.meta.main + // block, fed by currentExitCode(). + expect(src).toMatch(/import\.meta\.main/); + expect(src).toMatch(/flushThenExit\(currentExitCode\(\)\)/); + }); + + test('pglite-engine contains the Emscripten process.exitCode hijack', () => { + // PGLite's WASM runtime writes its own status into process.exitCode (99 + // alive / exit status on close) and ignores `undefined` assignment. The + // create call runs inside preservingProcessExitCode to keep the global + // tidy; close is deliberately unwrapped (see below) — the CLI's verdict + // is immune either way via the owned channel. + const src = readFileSync('src/core/pglite-engine.ts', 'utf8'); + expect(src).toMatch(/preservingProcessExitCode\(\(\)\s*=>\s*\n?\s*PGlite\.create/); + // close stays UNWRAPPED by design: its status write is baseline behavior + // test runners depend on; the CLI's verdict is immune because it lives in + // the gbrain-owned channel, never read back from process.exitCode. + const helper = readFileSync('src/core/cli-force-exit.ts', 'utf8'); + expect(helper).toMatch(/let cliVerdict: number \| null = null/); + expect(helper).toMatch(/return cliVerdict \?\? 0/); + // The op-dispatch catch must set the verdict through the owned channel. + const cli = readFileSync('src/cli.ts', 'utf8'); + expect(cli).toMatch(/setCliExitVerdict\(1\);/); + }); +}); + 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'); diff --git a/test/fixtures/flush-then-exit-harness.ts b/test/fixtures/flush-then-exit-harness.ts new file mode 100644 index 000000000..96a843241 --- /dev/null +++ b/test/fixtures/flush-then-exit-harness.ts @@ -0,0 +1,27 @@ +/** + * #2084 (D10) — spawned harness proving flushThenExit against REAL Bun pipe + * semantics: writes HARNESS_BYTES of 'x' to stdout, then flushThenExit with + * HARNESS_EXIT_CODE. The parent test pipes stdout to a slow-attaching reader + * and asserts byte-complete output + the exit code — the exact scenario the + * pre-#2084 force-exit truncated (#1959). + */ + +import { flushThenExit } from '../../src/core/cli-force-exit.ts'; + +const size = Number(process.env.HARNESS_BYTES ?? 4_000_000); +const code = Number(process.env.HARNESS_EXIT_CODE ?? 7); +const guardMs = Number(process.env.HARNESS_GUARD_MS ?? 2_000); +const graceEnv = process.env.HARNESS_GRACE_MS; + +const chunk = 'x'.repeat(65_536); +let written = 0; +while (written < size) { + const n = Math.min(chunk.length, size - written); + process.stdout.write(n === chunk.length ? chunk : chunk.slice(0, n)); + written += n; +} + +flushThenExit(code, { + guardMs, + ...(graceEnv !== undefined ? { graceMs: Number(graceEnv) } : {}), +}); diff --git a/test/flush-then-exit-harness.test.ts b/test/flush-then-exit-harness.test.ts new file mode 100644 index 000000000..800e41976 --- /dev/null +++ b/test/flush-then-exit-harness.test.ts @@ -0,0 +1,97 @@ +/** + * #2084 (D10) — flushThenExit proven on a real spawned Bun process. + * + * The unit tests in cli-finish-teardown.test.ts inject fake streams; they + * prove the helper's logic but not Bun's actual pipe behavior (does an empty + * write('', cb) really fence all prior buffered chunks?). These tests spawn + * test/fixtures/flush-then-exit-harness.ts and assert: + * 1. multi-MB piped stdout arrives byte-complete with the right exit code + * even when the reader attaches late (#1959 truncation regression pin); + * 2. with empty buffers the fence resolves promptly — the process does NOT + * sit out the flush guard (canary for Bun eliding empty-write callbacks). + */ + +import { describe, test, expect } from 'bun:test'; +import { spawn } from 'child_process'; +import { resolve } from 'path'; + +const HARNESS = resolve(import.meta.dir, 'fixtures', 'flush-then-exit-harness.ts'); + +function runHarness(env: Record, readerDelayMs: number): Promise<{ + bytes: number; + code: number | null; + durationMs: number; +}> { + return new Promise((resolveOut, reject) => { + const t0 = Date.now(); + const child = spawn('bun', ['run', HARNESS], { + env: { ...process.env, ...env }, + stdio: ['ignore', 'pipe', 'inherit'], + }); + let bytes = 0; + child.stdout.pause(); + setTimeout(() => { + child.stdout.on('data', (d: Buffer) => (bytes += d.length)); + child.stdout.resume(); + }, readerDelayMs); + const killer = setTimeout(() => { + child.kill('SIGKILL'); + reject(new Error(`harness did not exit (bytes so far: ${bytes})`)); + }, 30_000); + child.on('close', (code) => { + clearTimeout(killer); + resolveOut({ bytes, code, durationMs: Date.now() - t0 }); + }); + }); +} + +describe('flushThenExit on a real Bun process (D10)', () => { + test('4MB piped stdout arrives byte-complete with the exit code, late reader', async () => { + // Bun delivers queued pipe writes only while the process is alive (see the + // cli-force-exit.ts module header). The reader attaches 200ms late; the + // 1500ms aliveness grace must cover attach + transfer. Pre-#2084 (immediate + // process.exit, no grace) this received 0 of the 4MB — verified during + // implementation; that is the #1959 truncation class. + const SIZE = 4_000_000; + const { bytes, code } = await runHarness( + { + HARNESS_BYTES: String(SIZE), + HARNESS_EXIT_CODE: '7', + HARNESS_GUARD_MS: '2000', + HARNESS_GRACE_MS: '1500', + }, + 200, + ); + expect(bytes).toBe(SIZE); + expect(code).toBe(7); + }, 40_000); + + test('default grace: small output survives exit with a concurrent reader', async () => { + // No HARNESS_GRACE_MS → production default (non-TTY grace). Immediate + // reader, 100-byte output: must arrive complete. Pre-#2084 even this case + // lost ALL bytes when exit fired before a loop turn. + const { bytes, code } = await runHarness( + { HARNESS_BYTES: '100', HARNESS_EXIT_CODE: '0', HARNESS_GUARD_MS: '2000' }, + 0, + ); + expect(bytes).toBe(100); + expect(code).toBe(0); + }, 40_000); + + test('fence resolves promptly — wall time well under guard + grace ceiling', async () => { + // Guard 8s, grace 250ms: if Bun ever elides the empty-write callback, the + // process sits out the full guard and wall time exceeds it. A working + // fence exits in startup time + grace (~1-2s). + const { code, durationMs } = await runHarness( + { + HARNESS_BYTES: '100', + HARNESS_EXIT_CODE: '0', + HARNESS_GUARD_MS: '8000', + HARNESS_GRACE_MS: '250', + }, + 0, + ); + expect(code).toBe(0); + expect(durationMs).toBeLessThan(6_000); + }, 40_000); +}); diff --git a/test/pglite-engine-disconnect.serial.test.ts b/test/pglite-engine-disconnect.serial.test.ts index 3c2fd75c7..d4ec84e4c 100644 --- a/test/pglite-engine-disconnect.serial.test.ts +++ b/test/pglite-engine-disconnect.serial.test.ts @@ -219,3 +219,38 @@ describe('PGLiteEngine.disconnect() — v0.41.8.0 lifecycle invariants', () => { } }); }); + +// ───────────────────────────────────────────────────────────────── +// #2084 — preservingProcessExitCode behavioral containment +// ───────────────────────────────────────────────────────────────── +describe('PGLiteEngine: Emscripten process.exitCode containment (#2084)', () => { + test('connect() leaves process.exitCode pinned at 0, not the Emscripten 99', async () => { + const prev = process.exitCode; + const eng = new PGLiteEngine(); + try { + await eng.connect({ engine: 'pglite' }); + // Emscripten writes 99 during create; the wrapper pins explicit 0 when + // nothing was set before (undefined cannot be restored — the accessor + // falls back to the WASM status). + expect(Number(process.exitCode)).toBe(0); + } finally { + await eng.disconnect(); + process.exitCode = prev; + } + }, 60_000); + + test('a pre-call verdict survives the create-throw path (finally restores)', async () => { + const prev = process.exitCode; + const eng = new PGLiteEngine(); + try { + process.exitCode = 3; + // A dataDir under a regular FILE cannot be created — PGlite.create rejects. + await expect( + eng.connect({ engine: 'pglite', database_path: '/dev/null/nope/brain' }), + ).rejects.toThrow(); + expect(Number(process.exitCode)).toBe(3); + } finally { + process.exitCode = prev; + } + }, 60_000); +}); diff --git a/test/pglite-engine.test.ts b/test/pglite-engine.test.ts index cc1244a6d..9ba2732d3 100644 --- a/test/pglite-engine.test.ts +++ b/test/pglite-engine.test.ts @@ -1311,7 +1311,10 @@ describe('PGLiteEngine: v0.13.1 error-wrap on connect() (#223)', () => { // issue and suggest gbrain doctor. Must NOT suggest "missing migrations" // as a cause (that was conflating #218 and #223 — migrations run AFTER // create()). - expect(src).toContain('this._db = await PGlite.create'); + // #2084 wrapped the create call in preservingProcessExitCode (Emscripten + // exitCode containment); the try/catch + error wrap around it is unchanged. + expect(src).toContain('this._db = await preservingProcessExitCode(() =>'); + expect(src).toContain('PGlite.create({'); expect(src).toContain('https://github.com/garrytan/gbrain/issues/223'); expect(src).toContain('gbrain doctor'); expect(src).toContain('Original error:');