Files
gbrain/test/pglite-engine-disconnect.serial.test.ts
T
4ee530f3c5 v0.42.42.0 fix(cli): bounded teardown + explicit exit — kill the 10s force-exit tax on txn-mode poolers (#2084) (#2141)
* feat(core): finishCliTeardown + flushThenExit — bounded teardown, owned exit verdict (#2084)

cli-force-exit.ts becomes the single owner of one-shot CLI exit + teardown:
- finishCliTeardown: bounded sink drain -> bounded disconnect under a backstop
  whose deadline is COMPUTED from the bounds it guards (floor 10s;
  GBRAIN_TEARDOWN_DEADLINE_MS env override). Arms at teardown start, never
  before the op handler.
- flushThenExit: stdio write-fence (unref'd guard, EPIPE-safe) + REF'D
  aliveness grace for non-TTY stdio — Bun only delivers queued pipe writes
  while the process is alive (no flush API reaches the native queue).
- setCliExitVerdict/currentExitCode: the exit verdict lives in a gbrain-owned
  channel, never read back from process.exitCode (PGLite's Emscripten runtime
  scribbles its own status there mid-run).
- background-work.ts exports backgroundWorkSinkCount() for the deadline formula.

Unit tests + a spawned-Bun harness proving byte-complete piped output.

* fix(cli): route all nine disconnect sites through finishCliTeardown; one exit seam (#2084)

Deletes the pre-handler 10s force-exit timer (it measured handler + teardown
combined: PgBouncer txn-mode deployments paid a flat 10s banner tax on every
query, and any >10s op was killed mid-run with exit 0 and truncated output).
Sweeps op-dispatch, CLI_ONLY fall-through, search dashboard, read-only timeout
path, dream, doctor x3 (fixing a pre-existing pool leak when DB checks throw),
and ze-switch. The ONE process exit lives in main().then/catch via
flushThenExit(currentExitCode()), gated by shouldForceExitAfterMain().
Exit-code writers (op-dispatch catch, reindex, transcripts, brainstorm,
autopilot, frontmatter) now set the verdict through setCliExitVerdict.

* fix(pglite): contain Emscripten's process.exitCode writes at PGlite.create (#2084)

PGLite's WASM 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) — on PGLite every error exit was silently clobbered.
preservingProcessExitCode wraps create() to keep the global tidy; db.close()
stays unwrapped (its 0-write is baseline behavior test runners depend on).
The CLI verdict itself is immune: it lives in the owned channel.

* test: e2e + structural pins for the #2084 teardown contract

E2E: failed op exits 1; every swept command spawned (brain-copy isolation for
mutators, no-network); slow-handler regression via the deadline env knob;
piped --json parses complete; teardown banner absent on every happy path;
daemon survival untouched. Structural: no bare awaited engine disconnects in
cli.ts; DISCONNECT_HARD_DEADLINE_MS gone; >=9 helper call sites; verdict
channel + create-wrap pins.

* test: fix R1 env-isolation violations in retrieval-reflex tests

Pre-existing on master: both files mutated GBRAIN_RETRIEVAL_REFLEX directly,
failing scripts/check-test-isolation.sh (bun run verify). Converted to the
canonical withEnv() pattern; the reflex describe's beforeEach also never
restored the flag, leaking it across the shard.

* docs: KEY_FILES entries for the teardown contract; close + file TODOS (#2084)

KEY_FILES.md: current-state entry for cli-force-exit.ts (helper + central exit
seam pair, verdict channel, cli.ts-scoped claim); background-work.ts and
pglite-engine.ts entries updated. TODOS.md: the drain-before-owner-disconnect
P3 (filed from #1972) is done by this wave; files the trigger-gated
GBRAIN_COMMAND_DEADLINE_MS follow-up (eng-review D2/D14).

* fix: pre-landing review fixes (#2084)

Review army (testing/maintainability/security/performance, 0 critical):
- drain defense-in-depth: a throwing drain warns and still disconnects
  (cannot escape a caller's finally or skip the engine teardown)
- behavioral tests for preservingProcessExitCode (connect pins 0; create-throw
  restores the pre-call verdict)
- D9 widening test (live-registry sink count feeds the deadline formula),
  env 0/negative boundary cases, verdict mirror-write assertion
- stale comments: header diagram backstop line, structural-test 'both
  lifecycle calls' contradiction, KEY_FILES 10s-force-exit clauses, e2e D11
  falsification story corrected
- named the formula's pool-end literals

* fix: adversarial-review hardening — daemon-safe command resolution, flush knob, ref'd backstop (#2084)

Cross-model adversarial review (Claude subagent + Codex, both P1'd it):
- shouldForceExitAfterMain now resolves the command through parseGlobalFlags —
  the old first-non-dash heuristic read `gbrain --timeout 30s serve` as
  command "30s" and the new exit seam would have killed the daemon ~250ms
  after boot with exit 0 (unit-pinned)
- GBRAIN_FLUSH_GRACE_MS env override for the non-TTY aliveness grace (batch
  consumers piping large payloads to slow readers can raise it; agent loops
  can lower it)
- backstop timer is now REF'D: a hung teardown on an otherwise-empty event
  loop previously exited naturally — skipping the flush and surfacing
  PGLite's scribbled process.exitCode
- flushThenExit: real process.exit latched once per process
- doctor-site comment corrected; in-command process.exit teardown-bypass
  class (pre-existing) filed as a P2 TODO

* chore: bump version and changelog (v0.42.42.0)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test: move #2084 exitCode-containment lifecycle tests to the serial quarantine (R3)

* docs: update project documentation for v0.42.42.0

- docs/TESTING.md: replace the stale 4-file serial-quarantine enumeration
  with a current-state description (the quarantine is glob-discovered, now
  several dozen files incl. the #2084 exitCode-containment suite); add unit
  inventory entries for test/cli-finish-teardown.test.ts and
  test/flush-then-exit-harness.test.ts.
- docs/architecture/KEY_FILES.md: rephrase the pglite-engine exitCode
  containment note to current-state wording (clears the
  check-key-files-current-state prose-history warning).

llms bundles regenerated (byte-identical: both docs are link-only).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: apply cross-model doc-review findings for v0.42.42.0

Codex review of docs-vs-shipped-code found 9 gaps; all verified against
the code before fixing:

- CHANGELOG.md (0.42.42.0 entry, precision narrowing only — no entries
  touched): "every CLI exit path" -> "every cli.ts disconnect site";
  "on every path" -> "on every routed exit path"; dream/doctor/ze-switch
  claim scoped to dispatcher teardown (command-internal process.exit
  sites are tracked in TODOS as the open P2).
- docs/architecture/KEY_FILES.md: the teardown backstop is REF'D, not
  unref'd (matches the F3 adversarial-review decision in the code).
- src/core/cli-force-exit.ts: header diagram comment had the same stale
  unref'd claim + `process.exitCode ?? 0`; now matches the implementation
  (ref'd timer, `currentExitCode()`). Comment-only change.
- docs/TESTING.md: verify is the 30-check parallel battery via
  run-verify-parallel.sh (was described as 4 checks); CI is 10 weighted
  LPT shards + dedicated verify/serial/slow jobs (was "4-way FNV on
  shard 1"); test:serial runs one bun process per file (not
  --max-concurrency=1); dead "cap: 10" line rewritten as debt guidance;
  inventory entries added for test/cli-should-force-exit.test.ts and
  test/e2e/pglite-cli-exit.serial.test.ts.

bun run verify green (30/30); #2084 test files green; llms bundles
regenerated (byte-identical — reference docs are link-only).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: route v0.42.41.0's raw exitCode writers through the verdict channel; reconcile merged structural pins (#2084)

CI fallout from merging the v0.42.41.0 triage wave into the #2084 exit-seam
design — both waves fixed the same timer-placement bug independently:

- doctor.ts + extract.ts set failure exit codes via raw `process.exitCode =`
  writes (v0.42.41.0's process.exit -> exitCode conversion); the #2084 exit
  seam reads only the gbrain-owned verdict channel, so doctor FAILs exited 0
  (Tier 1 RLS e2e + half-migrated-Minions tests). Converted to
  setCliExitVerdict, same as the wallclock-124 site in the merge commit.
- cli-force-exit-teardown-arming.test.ts pinned v0.42.41.0's inline
  finally-armed timer, which the merge replaced with finishCliTeardown;
  rewritten to pin the merged invariant (no pre-try arming in cli.ts; the
  backstop arms inside the helper before the drain).
- eval-capture drain timing bound 1s -> 2s: flaked at 1023ms under CI shard
  load after the new test files shifted LPT shard packing (13x budget slack
  still proves bounded-not-hung).

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-06-12 07:28:13 -07:00

257 lines
9.7 KiB
TypeScript

/**
* v0.41.8.0 — PGLiteEngine.disconnect() lifecycle regression tests.
*
* Pins the invariants the v0.41.8.0 hang fix wave depends on:
*
* 1. ORDERING: `db.close()` is called BEFORE the file lock is
* released. A sibling process must not be able to acquire the
* lock and try to connect to a still-closing brain. PR #1337's
* original diff swapped this to release-then-close — we
* explicitly REJECTED that ordering. This test fails if a
* future maintainer reads the PR and applies the swap.
*
* 2. SNAPSHOT + EARLY-NULL: `this._db` is nulled BEFORE awaiting
* `close()`, so a concurrent `connect()` cannot observe a
* partial mid-close state. PR #1337's load-bearing contribution
* that we DID take.
*
* 3. LOCK LEAK GUARD: if `db.close()` throws, the file lock STILL
* releases. Codex outside-voice finding #7 in the eng review:
* without try/finally, a close-throw would wedge every next
* gbrain invocation on the stale lock.
*
* 4. IDEMPOTENCY: calling disconnect() twice is a clean no-op on
* the second call (no throw, no double-close attempt).
*
* 5. DOUBLE-DISCONNECT THEN CONNECT: after disconnect, a fresh
* connect() sees clean state and succeeds.
*
* Marked .serial because PGLite WASM cold-start dominates wallclock
* for fresh-engine-per-test cases — running these in the parallel
* shard pool would starve other PGLite tests of cold-start time.
*/
import { describe, test, expect } from 'bun:test';
import { mkdtempSync, rmSync } from 'fs';
import { tmpdir } from 'os';
import { join } from 'path';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
function newTempDataDir(): string {
return mkdtempSync(join(tmpdir(), 'gbrain-disconnect-test-'));
}
describe('PGLiteEngine.disconnect() — v0.41.8.0 lifecycle invariants', () => {
test('ORDERING: db.close() is called BEFORE releaseLock()', async () => {
const dataDir = newTempDataDir();
try {
const engine = new PGLiteEngine();
await engine.connect({ database_path: dataDir });
await engine.initSchema();
// Record the actual call order. We spy by replacing the db
// handle's close + the lock handle's release with timestamped
// wrappers.
const calls: string[] = [];
const eng = engine as unknown as {
_db: { close: () => Promise<void> } | null;
_lock: { lockDir: string; acquired: boolean } | null;
};
const realClose = eng._db!.close.bind(eng._db!);
eng._db!.close = async () => {
// Tiny delay so a flipped ordering would actually show up
// (release-before-close would beat us if we returned instantly).
await new Promise((r) => setTimeout(r, 10));
calls.push('db.close');
return realClose();
};
// releaseLock is module-level in pglite-lock.ts — to spy we have
// to swap the lock object's `acquired` flag detection won't
// route through us. Easier: monkey-patch by replacing the lock
// ref with one whose presence forces releaseLock to no-op (so
// we just measure that the close ran during disconnect and that
// the no-op happened in the same call).
//
// For the ORDERING test specifically, we wrap close and
// measure that the lockDir mkdir is still present immediately
// before close runs and gone after disconnect returns. The
// lockDir's existence is observable on disk.
const { existsSync } = await import('fs');
const lockDir = eng._lock!.lockDir;
expect(existsSync(lockDir)).toBe(true);
// Spy on the lock-release moment by polling lockDir existence
// from another timer: when close completes, the lock should
// STILL be present (close-then-release contract).
let lockStillPresentAtCloseFinish = false;
const origClose = eng._db!.close;
eng._db!.close = async () => {
await origClose();
// Right after close resolves, the lock has NOT yet been
// released (the finally branch hasn't run yet). Check
// synchronously before yielding the event loop again.
lockStillPresentAtCloseFinish = existsSync(lockDir);
};
await engine.disconnect();
expect(calls).toContain('db.close');
expect(lockStillPresentAtCloseFinish).toBe(true);
expect(existsSync(lockDir)).toBe(false);
} finally {
rmSync(dataDir, { recursive: true, force: true });
}
});
test('SNAPSHOT + EARLY-NULL: _db is nulled before await close', async () => {
const dataDir = newTempDataDir();
try {
const engine = new PGLiteEngine();
await engine.connect({ database_path: dataDir });
await engine.initSchema();
const eng = engine as unknown as {
_db: { close: () => Promise<void> } | null;
};
let dbWasNullWhenCloseRan = false;
const realClose = eng._db!.close.bind(eng._db!);
eng._db!.close = async () => {
// Inside close, the engine's _db field should ALREADY be null
// (snapshot pattern). If it's not, the partial-state race is
// back.
dbWasNullWhenCloseRan = eng._db === null;
return realClose();
};
await engine.disconnect();
expect(dbWasNullWhenCloseRan).toBe(true);
} finally {
rmSync(dataDir, { recursive: true, force: true });
}
});
test('LOCK LEAK GUARD: if db.close() throws, lock still releases', async () => {
const dataDir = newTempDataDir();
try {
const engine = new PGLiteEngine();
await engine.connect({ database_path: dataDir });
await engine.initSchema();
const eng = engine as unknown as {
_db: { close: () => Promise<void> } | null;
_lock: { lockDir: string; acquired: boolean } | null;
};
const { existsSync } = await import('fs');
const lockDir = eng._lock!.lockDir;
expect(existsSync(lockDir)).toBe(true);
// Force close to throw. The lock MUST still release.
eng._db!.close = async () => {
throw new Error('synthetic close failure');
};
// The throw will propagate out of disconnect — that's fine.
// The contract is "lock releases regardless."
let threw = false;
try {
await engine.disconnect();
} catch (e) {
threw = true;
expect(e instanceof Error && e.message).toContain('synthetic close failure');
}
expect(threw).toBe(true);
// CRITICAL: lock must be gone even though close threw.
expect(existsSync(lockDir)).toBe(false);
} finally {
rmSync(dataDir, { recursive: true, force: true });
}
});
test('IDEMPOTENCY: double disconnect is a clean no-op on the second call', async () => {
const dataDir = newTempDataDir();
try {
const engine = new PGLiteEngine();
await engine.connect({ database_path: dataDir });
await engine.initSchema();
let closeCallCount = 0;
const eng = engine as unknown as {
_db: { close: () => Promise<void> } | null;
};
const realClose = eng._db!.close.bind(eng._db!);
eng._db!.close = async () => {
closeCallCount++;
return realClose();
};
await engine.disconnect();
expect(closeCallCount).toBe(1);
// Second call: no throw, no second close
await engine.disconnect();
expect(closeCallCount).toBe(1);
} finally {
rmSync(dataDir, { recursive: true, force: true });
}
});
test('RECONNECT after disconnect sees clean state', async () => {
const dataDir = newTempDataDir();
try {
const engine = new PGLiteEngine();
await engine.connect({ database_path: dataDir });
await engine.initSchema();
await engine.disconnect();
// Same dataDir, fresh connect. Must succeed without lock contention.
await engine.connect({ database_path: dataDir });
await engine.initSchema();
// Smoke: a SELECT 1 round-trip proves the new handle is alive.
const result = await engine.executeRaw<{ ok: number }>('SELECT 1 AS ok');
expect(result[0].ok).toBe(1);
await engine.disconnect();
} finally {
rmSync(dataDir, { recursive: true, force: true });
}
});
});
// ─────────────────────────────────────────────────────────────────
// #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);
});