From de4c29d037740b36d95c66033c0aa811ed07d05c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 12 Jun 2026 06:41:54 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20doctor's=20FAIL=20verdict=20was=20zeroed?= =?UTF-8?q?=20by=20the=20owned=20exit=20=E2=80=94=20sweep=20stragglers=20+?= =?UTF-8?q?=20class=20pin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merged-state suite caught it: doctor --fast --json reported FAIL but exited 0. Master's v0.42.41.0 brought raw `process.exitCode =` writes (doctor.ts hasFail ternary, extract.ts) that the #2084 verdict-owning exit silently zeroes — getCliExitCode() deliberately never reads ambient process.exitCode (the PGLite-Emscripten pollution defense), so any setter that bypasses setCliExitCode reports success on failure. Swept both sites and added the structural class pin: a test greps src/ for raw `process.exitCode =` outside cli-force-exit.ts, so the next merge that introduces one fails loudly instead of lying about exit codes. Co-Authored-By: Claude Fable 5 --- src/commands/doctor.ts | 11 +++++++---- src/commands/extract.ts | 3 ++- test/cli-flush-exit.test.ts | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 028a8ab4d..d04403b51 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,5 +1,6 @@ import type { BrainEngine } from '../core/engine.ts'; import * as db from '../core/db.ts'; +import { setCliExitCode } from '../core/cli-force-exit.ts'; import { LATEST_VERSION, getIdleBlockers } from '../core/migrate.ts'; import { checkResolvable } from '../core/check-resolvable.ts'; import { autoFixDryViolations, type AutoFixReport, type FixOutcome } from '../core/dry-fix.ts'; @@ -7225,10 +7226,12 @@ export async function runDoctor( } catch { /* best-effort */ } } - // Use process.exitCode instead of process.exit() so cleanup handlers - // (e.g. Bun unload events, open database connections) still run before - // the process terminates. process.exit() is a hard kill that bypasses them. - process.exitCode = hasFail ? 1 : 0; + // Use the owned exit verdict instead of process.exit() so cleanup handlers + // (e.g. Bun unload events, open database connections) still run before the + // process terminates — and so the deliberate flush-exit (#2084) reports + // doctor's verdict: a RAW process.exitCode write is silently zeroed by + // getCliExitCode() at the entrypoint (the PGLite-pollution defense). + setCliExitCode(hasFail ? 1 : 0); } // --------------------------------------------------------------------------- diff --git a/src/commands/extract.ts b/src/commands/extract.ts index e6b5a4077..ac37218b7 100644 --- a/src/commands/extract.ts +++ b/src/commands/extract.ts @@ -28,6 +28,7 @@ * pagination). FS-source preserves the original v0.10.1 walker behavior. */ +import { setCliExitCode } from '../core/cli-force-exit.ts'; import { readFileSync, readdirSync, lstatSync, existsSync } from 'fs'; import { join, relative, dirname } from 'path'; import type { BrainEngine, LinkBatchInput, TimelineBatchInput } from '../core/engine.ts'; @@ -864,7 +865,7 @@ Status (v0.42): (r.first_batch_error ? ` (first error: ${r.first_batch_error})` : '') + ` — timeline is incomplete.`, ); - process.exitCode = 1; + setCliExitCode(1); } } else if (byMention || ner) { // v0.41.18.0 (T7): combined --by-mention + --ner walk shares one diff --git a/test/cli-flush-exit.test.ts b/test/cli-flush-exit.test.ts index cfa96ab81..1a4e7e4b7 100644 --- a/test/cli-flush-exit.test.ts +++ b/test/cli-flush-exit.test.ts @@ -137,3 +137,19 @@ describe('flushStdoutThenExit — deliberate exit after output drains', () => { expect(stream.drainListeners()).toBe(0); }); }); + +describe('exit-verdict ownership — no raw process.exitCode assignments (#2084 class pin)', () => { + test('every exit-code write in src/ routes through setCliExitCode', async () => { + // A RAW `process.exitCode = N` is silently ZEROED by the deliberate + // flush-exit: getCliExitCode() reads only gbrain's owned verdict (the + // PGLite-Emscripten-pollution defense), so a command that bypasses the + // setter reports success on failure. Caught live: doctor's FAIL path + // exited 0 after the v0.42.41.0 merge introduced a raw write. + const { execSync } = await import('child_process'); + const hits = execSync( + `grep -rn "process.exitCode = " src --include='*.ts' | grep -v "core/cli-force-exit.ts" || true`, + { encoding: 'utf-8', cwd: new URL('..', import.meta.url).pathname }, + ).trim(); + expect(hits).toBe(''); + }); +});