From a5b117a08529c9e16de9d3570ac159cb92ad369b Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 12 Jun 2026 06:15:18 -0700 Subject: [PATCH] test: re-pin the teardown-arming invariant at its post-#2084 home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- test/cli-force-exit-teardown-arming.test.ts | 80 +++++++++++---------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/test/cli-force-exit-teardown-arming.test.ts b/test/cli-force-exit-teardown-arming.test.ts index ff9743e30..30d2d1ff0 100644 --- a/test/cli-force-exit-teardown-arming.test.ts +++ b/test/cli-force-exit-teardown-arming.test.ts @@ -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 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` 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); }); });