Files
gbrain/src/core/cli-force-exit.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

333 lines
14 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
/**
* One-shot CLI exit + teardown contract (#2084, supersedes the narrower
* v0.41.8.0 drain-timeout-only force-exit).
*
* The CLI must never rely on Bun's event loop draining to exit: on PgBouncer
* transaction-mode, `endPoolBounded` (db.ts) deliberately races PAST a stuck
* `pool.end()`, so the promise resolves while the stuck sockets stay open and
* keep the loop alive (#2084's flat 10s teardown tax). Per the doctrine in
* timeout.ts, `process.exit` is the real resource-release mechanism for
* one-shot commands — the kernel reclaims sockets.
*
* The contract is a PAIR (documented together in KEY_FILES.md):
*
* op handler returns / throws (catch sets the verdict: setCliExitVerdict(1))
* │
* ▼ (per call site, in its finally — nine sites in cli.ts)
* finishCliTeardown({ engine, drainTimeoutMs? }) ← teardown ONLY, never exits*
* │
* ├─ arm ref'd backstop timer; deadline COMPUTED from the bounds
* │ it guards (sinks × drainTimeoutMs + facts-abort grace
* │ + 2 × pool-end bound + slack, floor 10s). The backstop fires
* │ ONLY if a component violated its own bound; on fire it prints
* │ a truthful banner and *flushThenExit(currentExitCode()).
* │ GBRAIN_TEARDOWN_DEADLINE_MS overrides (incident escape hatch).
* ▼
* drain background sinks (bounded per-sink; CLI-exit-only contract)
* ▼
* engine.disconnect() — a throw is warned + swallowed: the exit code
* │ reports the OPERATION, not the cleanup
* ▼
* clear backstop, RETURN to caller
* │
* ▼ (exactly ONE place: cli.ts import.meta.main main().then/catch)
* shouldForceExitAfterMain() && flushThenExit(currentExitCode())
* — fence stdout+stderr (write-fence raced with an unref'd guard,
* EPIPE-safe), hold a short REF'D aliveness grace for non-TTY stdio
* (Bun only delivers queued pipe writes while alive), then
* process.exit. Stuck sockets become irrelevant.
*
* The hard-deadline timer is armed at TEARDOWN start, never before the op
* handler — a slow-but-healthy handler must not erode the teardown budget
* (the pre-#2084 bug force-killed any >10s 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<string> = new Set(['serve']);
export function shouldForceExitAfterMain(
argv: string[] = process.argv.slice(2),
): boolean {
// 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<typeof setTimeout> | 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<void> };
/** 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<void>;
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<void> {
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);
}
}