test: re-pin the teardown-arming invariant at its post-#2084 home

Master's v0.42.41.0 triage wave and the #2084 wave fixed the same
pre-armed-timer bug independently; the merge keeps #2084's shape (arming
inside the shared drainThenDisconnect helper, covering all 8 exit paths).
The structural pin now asserts the same invariant — no pre-try arming;
gated, unref'd, before-drain, cleared — at the helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-12 06:15:18 -07:00
co-authored by Claude Fable 5
parent 87a22ad0aa
commit a5b117a085
+44 -36
View File
@@ -1,15 +1,18 @@
/**
* 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 DISCONNECT_HARD_DEADLINE_MS force-exit timer
* must be armed at TEARDOWN ENTRY, never before the op-dispatch try block.
*
* 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 (fixed independently by master's v0.42.41.0 triage wave AND
* the #2084 wave): 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).
*
* Post-#2084 the arming lives INSIDE the shared `drainThenDisconnect`
* helper — the single owner-disconnect every CLI exit path calls from its
* finally — so it structurally bounds ONLY the teardown window (drain +
* disconnect) at every site, not just the op-dispatch path.
*
* Source-grep is the right tool here (same rationale as
* fix-wave-structural.test.ts): the rule is "this arming must stay at this
@@ -22,35 +25,40 @@ 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', () => {
test('no timer arming between the op-dispatch entry and its try block', () => {
const src = readFileSync('src/cli.ts', 'utf8');
// The op-dispatch local-engine path: from connectEngine to its try, no
// setTimeout call may be armed (a pre-try timer kills slow-but-progressing
// op handlers mid-flight with exit 0 and empty stdout). `setTimeout(`
// matches only a call site; ReturnType<typeof setTimeout> stays allowed.
const entry = src.indexOf('// Local engine path (unchanged behavior');
expect(entry).toBeGreaterThan(-1);
const tryIdx = src.indexOf('try {', entry);
expect(tryIdx).toBeGreaterThan(-1);
expect(src.slice(entry, tryIdx)).not.toContain('setTimeout(');
});
test('the deadline arming lives inside drainThenDisconnect: gated, unref\'d, before the drain, cleared after', () => {
const src = readFileSync('src/cli.ts', 'utf8');
const helper = src.match(/export async function drainThenDisconnect[\s\S]+?^\}/m);
expect(helper).not.toBeNull();
const block = helper![0];
const armIdx = block.indexOf('deadlineTimer = setTimeout');
const drainIdx = block.indexOf('drainAllBackgroundWorkForCliExit');
expect(armIdx).toBeGreaterThan(-1);
expect(drainIdx).toBeGreaterThan(-1);
// Armed at teardown entry, before the drain + disconnect it bounds.
expect(armIdx).toBeLessThan(drainIdx);
// Gated on the daemon-survival guard so `serve` stays alive.
expect(block.slice(0, armIdx)).toMatch(/if \(shouldForceExitAfterMain\(\)\)/);
// Unref'd so the timer itself never keeps the event loop alive.
expect(block).toContain('deadlineTimer.unref?.()');
// Cleared on clean teardown.
expect(block).toContain('if (deadlineTimer) clearTimeout(deadlineTimer)');
// The DISCONNECT_HARD_DEADLINE_MS declaration sits with the helper.
const decl = src.indexOf('const DISCONNECT_HARD_DEADLINE_MS');
expect(decl).toBeGreaterThan(-1);
const tryIdx = src.indexOf('try {', decl);
expect(tryIdx).toBeGreaterThan(-1);
const finallyIdx = src.indexOf('} finally {', tryIdx);
expect(finallyIdx).toBeGreaterThan(-1);
const armIdx = src.indexOf('forceExitTimer = setTimeout', decl);
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<typeof setTimeout>` 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)');
expect(src.indexOf('export async function drainThenDisconnect')).toBeGreaterThan(decl);
});
});