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.
2026-07-28 06:23:01 +00:00 · 2026-06-11 23:15:23 -07:00
parent 803b5a713d
commit 4e049efeb6
2 changed files with 208 additions and 21 deletions
@@ -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<string, string>,
 ): 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,132 @@ 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 () => {
+    // Pre-#2084 the hard-deadline timer armed BEFORE the op handler, so with a
+    // 500ms deadline this spawn would force-exit mid-handler with exit 0 and
+    // empty stdout. Post-fix the timer arms at teardown start: the handler runs
+    // to completion and the results print regardless of the tiny deadline.
+    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
@@ -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,55 @@ 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. Both
+    // lifecycle calls must run inside preservingProcessExitCode or PGLite
+    // error exits silently report success again.
+    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');