mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 03:12:32 +00:00
* 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>
122 lines
5.4 KiB
TypeScript
122 lines
5.4 KiB
TypeScript
/**
|
|
* v0.42.20.0 (#1762 / #1745 / #1775 reliability wave) — process background-work
|
|
* registry. Single source of truth for "drain every fire-and-forget sink before
|
|
* the CLI exits / disconnects."
|
|
*
|
|
* WHY THIS EXISTS (rule-of-four): four independent fire-and-forget sinks each
|
|
* write to the DB after an op returns its response —
|
|
* - `last-retrieved.ts` UPDATE pages.last_retrieved_at (#1247/#1269/#1290)
|
|
* - `facts/queue.ts` facts:absorb Haiku job + logIngest (#1762)
|
|
* - `search/hybrid.ts` query_cache write
|
|
* - `eval-capture.ts` eval_candidates INSERT
|
|
* On PGLite, if `engine.disconnect()` nulls `_db` while one of these is in
|
|
* flight, the sink's "not connected" error path re-pumps via queueMicrotask and
|
|
* spins `db.close()` into a 100%-CPU busy-loop that pins the single-writer lock
|
|
* (the #1762 incident). The fix is to DRAIN every sink before disconnect. A
|
|
* registry (not a hand-written N-call helper) makes that structural: a future
|
|
* 5th sink that registers is auto-drained, and the drain is invoked from THREE
|
|
* exit points (op-dispatch success finally, op-dispatch error catch, CLI_ONLY
|
|
* finally) without repeating the sink list at each.
|
|
*
|
|
* register (at module import) ─┐
|
|
* last-retrieved (order 1) │
|
|
* facts (order 0) ├─► Map<name, drainer>
|
|
* search-cache (order 2) │
|
|
* eval-capture (order 3) ┘
|
|
* │ CLI exit
|
|
* ▼
|
|
* drainAllBackgroundWorkForCliExit ──► sort by (order, name)
|
|
* for each: await drain(timeoutMs)
|
|
* if unfinished>0 && abort:
|
|
* await abort() ◄─ facts shutdown()
|
|
* ▼
|
|
* engine.disconnect() (caller)
|
|
*
|
|
* Registration MUST live in the enqueue-owning module (so "module not imported
|
|
* ⇒ no work enqueued ⇒ nothing to drain" holds). The Map is keyed by name so a
|
|
* re-import / test mock REPLACES rather than duplicating (an array would
|
|
* double-register).
|
|
*/
|
|
|
|
export interface BackgroundWorkDrainer {
|
|
/** Stable identity; also the Map key (idempotent registration). */
|
|
name: string;
|
|
/**
|
|
* Explicit drain order — lower runs first. Facts is 0 so its abort-path DB
|
|
* `logIngest` gets the freshest live-engine window before the fast
|
|
* last-retrieved / search-cache drains. Ties break by name for determinism.
|
|
*/
|
|
order: number;
|
|
/** Resolve when in-flight work settles OR the bound elapses; report leftovers. */
|
|
drain(timeoutMs: number): Promise<{ unfinished: number }>;
|
|
/**
|
|
* Optional hard-stop for stragglers (facts-queue: `shutdown()`). AWAITED by
|
|
* the registry so the aborted job's DB write settles against a live engine
|
|
* BEFORE the caller disconnects. Only invoked when `drain` reports unfinished.
|
|
*/
|
|
abort?(): Promise<void>;
|
|
}
|
|
|
|
const drainers = new Map<string, BackgroundWorkDrainer>();
|
|
|
|
/** Register (or replace, by name) a fire-and-forget sink drainer. */
|
|
export function registerBackgroundWorkDrainer(d: BackgroundWorkDrainer): void {
|
|
drainers.set(d.name, d);
|
|
}
|
|
|
|
/**
|
|
* Number of registered sinks. Used by `finishCliTeardown` (cli-force-exit.ts)
|
|
* to COMPUTE its backstop deadline from the bounds it guards — a 5th sink
|
|
* registering automatically widens the deadline instead of silently making
|
|
* the worst-case bounded drain exceed a static number (#2084 eng-review D9).
|
|
*/
|
|
export function backgroundWorkSinkCount(): number {
|
|
return drainers.size;
|
|
}
|
|
|
|
/**
|
|
* Test seam — registers a drainer and returns an unregister handle. Preferred
|
|
* over a blunt reset: real sink modules register at import time and won't re-run
|
|
* that top-level side effect on a second import, so a global clear would
|
|
* silently drop the production drainers for the rest of the test process.
|
|
*/
|
|
export function __registerDrainerForTest(d: BackgroundWorkDrainer): () => void {
|
|
drainers.set(d.name, d);
|
|
return () => { drainers.delete(d.name); };
|
|
}
|
|
|
|
/** Test seam — snapshot of registered drainer names (sorted), for assertions. */
|
|
export function __listDrainerNamesForTest(): string[] {
|
|
return [...drainers.keys()].sort();
|
|
}
|
|
|
|
/**
|
|
* CLI-EXIT-ONLY. `abort()` is a permanent process-level state change on a sink
|
|
* (the facts queue's `shutdown()` sets `shuttingDown=true` for the process
|
|
* lifetime). NEVER call this in a long-lived process (`gbrain serve`). Drains
|
|
* every registered sink before `engine.disconnect()` so a PGLite `db.close()`
|
|
* can't race in-flight work into the re-pump busy-loop (#1762).
|
|
*
|
|
* Best-effort and non-throwing: one sink's failure never blocks the others or
|
|
* the subsequent disconnect.
|
|
*/
|
|
export async function drainAllBackgroundWorkForCliExit(opts?: { timeoutMs?: number }): Promise<void> {
|
|
const timeoutMs = opts?.timeoutMs ?? 2000;
|
|
const ordered = [...drainers.values()].sort(
|
|
(a, b) => a.order - b.order || a.name.localeCompare(b.name),
|
|
);
|
|
for (const d of ordered) {
|
|
try {
|
|
const { unfinished } = await d.drain(timeoutMs);
|
|
if (unfinished > 0 && d.abort) {
|
|
// codex #9: AWAIT — the facts:absorb job writes its absorb-log to the
|
|
// DB on settle; the abort must finish against a live engine before the
|
|
// caller disconnects.
|
|
await d.abort();
|
|
}
|
|
} catch {
|
|
/* best-effort; never block disconnect on one sink's failure */
|
|
}
|
|
}
|
|
}
|