diff --git a/tests/helpers/native-profile-startup-child.ts b/tests/helpers/native-profile-startup-child.ts index b0b1e6f49..e60adffdb 100644 --- a/tests/helpers/native-profile-startup-child.ts +++ b/tests/helpers/native-profile-startup-child.ts @@ -3,7 +3,7 @@ import { appendFileSync, existsSync, writeFileSync } from "node:fs"; import { NativeProfileManager } from "../../src/codex/native-profile-manager"; import { isCodexAccountUsable } from "../../src/codex/account-usability"; import { isMainAccountTokenLive, MAIN_CODEX_ACCOUNT_ID } from "../../src/codex/main-account"; -import { loadConfig } from "../../src/config"; +import { atomicWriteFile, loadConfig } from "../../src/config"; import { nativeMainStartupGateSnapshot, waitForNativeMainStartupGate, @@ -68,17 +68,23 @@ const server = startServer(0, { }); writeFileSync(portPath, String(server.port)); +// #1061: the parent parses this file as soon as it exists, so a partial write +// surfaces as `Unexpected EOF`. atomicWriteFile publishes through a rename, so a +// reader sees either nothing or the whole document. void waitForNativeMainStartupGate().then(() => { - writeFileSync(settledPath, JSON.stringify({ + atomicWriteFile(settledPath, JSON.stringify({ gate: nativeMainStartupGateSnapshot(), mainTokenLive: isMainAccountTokenLive(), mainUsable: isCodexAccountUsable(loadConfig(), MAIN_CODEX_ACCOUNT_ID), })); }).catch((error: unknown) => { - writeFileSync(settledPath, JSON.stringify({ + atomicWriteFile(settledPath, JSON.stringify({ error: error instanceof Error ? `${error.message}\n${error.stack ?? ""}` : String(error), })); }); while (!existsSync(stopPath)) await Bun.sleep(10); +// Test-only stall, opt-in. It exists so the parent's bounded teardown can be shown +// firing (#1061) — without it the timeout branch is present but never exercised. +if (process.env.OCX_TEST_STALL_ON_STOP === "1") await new Promise(() => {}); await server.stop(true); diff --git a/tests/native-profile-crash-boundaries.test.ts b/tests/native-profile-crash-boundaries.test.ts index 4c61fc2a3..66bc30893 100644 --- a/tests/native-profile-crash-boundaries.test.ts +++ b/tests/native-profile-crash-boundaries.test.ts @@ -9,6 +9,7 @@ import { NativeProfileManager, type NativeProfileSwitchBoundary } from "../src/c import { readNativeProfileJournal, readNativeProfileVault } from "../src/codex/native-profile-store"; import type { NativeProfileKey, NativeProfileKeyProvider } from "../src/codex/native-profile-types"; import type { OcxConfig } from "../src/types"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; const roots: string[] = []; const oldOcx = process.env.OPENCODEX_HOME; @@ -83,6 +84,59 @@ async function waitFor(path: string, timeout = 10_000): Promise { if (!existsSync(path)) throw new Error(`timed out waiting for ${path}`); } +/* + * #1061. `waitFor` proves a file EXISTS, which is not the precondition a caller + * that immediately parses it actually needs — a partially written document + * satisfies the wait and then throws `Unexpected EOF`. This waits for the real + * precondition instead. + */ +async function waitForJson(path: string, timeout = 10_000): Promise { + const deadline = Date.now() + timeout; + let lastError: unknown; + while (Date.now() < deadline) { + if (existsSync(path)) { + try { + return JSON.parse(readFileSync(path, "utf8")) as T; + } catch (error) { + lastError = error; + } + } + await Bun.sleep(10); + } + throw new Error(`timed out waiting for parseable JSON in ${path}`, { cause: lastError }); +} + +const KILL_GRACE_MS = 2_000; + +/* + * #1061. The teardown used to `await child.exited` with no deadline, so a child + * stalled in `server.stop(true)` hung the run until CI killed the job — a + * 30-minute wait for a test that had already done its work. Every wait here is + * bounded, including the ones after a signal: an ignored SIGTERM would otherwise + * reproduce the same hang one layer down. + */ +async function stopStartup( + child: Bun.Subprocess, + paths: { release: string; stop: string }, + timeoutMs: number = INTERNAL_DEADLINE_MS, +): Promise { + writeFileSync(paths.release, "recover"); + writeFileSync(paths.stop, "stop"); + const exit = await Promise.race([child.exited, Bun.sleep(timeoutMs).then(() => null)]); + if (exit === null) { + child.kill(); + const killed = await Promise.race([child.exited, Bun.sleep(KILL_GRACE_MS).then(() => null)]); + if (killed === null) { + child.kill("SIGKILL"); + // Observe the escalation before throwing, so a caller asserting on + // `exitCode` is not racing the reap. + await Promise.race([child.exited, Bun.sleep(KILL_GRACE_MS).then(() => null)]); + } + throw new Error("startup child did not stop"); + } + if (exit !== 0) throw new Error(`startup child exited ${exit}`); +} + function spawnSwitch(f: Awaited>, options: { boundary?: NativeProfileSwitchBoundary; marker?: string; release?: string; contention?: string; result: string }) { return Bun.spawn([process.execPath, join(import.meta.dir, "helpers", "native-profile-switch-child.ts")], { cwd: join(import.meta.dir, ".."), @@ -109,7 +163,11 @@ function startupPaths(f: Awaited>) { return { port: join(f.root, "port"), release: join(f.root, "recover"), settled: join(f.root, "settled"), upstream: join(f.root, "upstream"), stop: join(f.root, "stop") }; } -function spawnStartup(f: Awaited>, p: ReturnType) { +function spawnStartup( + f: Awaited>, + p: ReturnType, + extraEnv: Record = {}, +) { return Bun.spawn([process.execPath, join(import.meta.dir, "helpers", "native-profile-startup-child.ts")], { cwd: join(import.meta.dir, ".."), env: { @@ -119,6 +177,7 @@ function spawnStartup(f: Awaited>, p: ReturnType { expect(existsSync(p.upstream)).toBe(false); writeFileSync(p.release, "recover"); } - await waitFor(p.settled); - expect(JSON.parse(readFileSync(p.settled, "utf8"))).toMatchObject({ gate: { status: "ready" } }); + expect(await waitForJson(p.settled)).toMatchObject({ gate: { status: "ready" } }); expect((await mainRequest(port)).status).toBe(200); await waitFor(p.upstream); const receipt = JSON.parse(readFileSync(p.upstream, "utf8").trim().split("\n").at(-1)!); @@ -192,9 +250,7 @@ describe("native profile OpenCodex process-exit phases", () => { expect(recoveredVault.revision).toBe(f.initialRevision + (finalTarget ? 1 : 0)); expect(readNativeProfileJournal(f.manager.context)).toBeNull(); } finally { - writeFileSync(p.release, "recover"); - writeFileSync(p.stop, "stop"); - expect(await restart.exited).toBe(0); + await stopStartup(restart, p); } } }, 90_000); @@ -230,4 +286,51 @@ describe("native profile OpenCodex process-exit phases", () => { if (second) await second.exited; } }, 20_000); + + /* + * #1061 activation evidence for the teardown deadline. A green suite says nothing + * about a timeout branch nobody drives, so this drives it: the child is told to + * stall exactly where the reported hang occurred (before `server.stop(true)`), + * and the teardown must give up and reap it instead of waiting forever. + * + * The reap assertion is the part that matters — it proves cleanup happened, not + * merely that a deadline was noticed. A signalled child reports `signalCode` + * rather than `exitCode`, so this checks that the process settled either way. + */ + test("a stalled startup child is killed by the bounded teardown instead of hanging", async () => { + const f = await fixture(); + const p = startupPaths(f); + const child = spawnStartup(f, p, { OCX_TEST_STALL_ON_STOP: "1" }); + try { + await waitFor(p.port); + await expect(stopStartup(child, p, 1_000)).rejects.toThrow("startup child did not stop"); + expect(child.killed).toBe(true); + expect(child.exitCode ?? child.signalCode).not.toBeNull(); + } finally { + if (child.exitCode === null) { + child.kill("SIGKILL"); + await child.exited; + } + } + }, 30_000); + + /* + * #1061 the other half: the settled file is parsed the moment it appears, so a + * reader that only checks existence sees a partial document. This drives that + * exact sequence — partial content first, then the real document — and asserts + * the wait holds out for something parseable. + */ + test("waitForJson holds out for a complete document instead of parsing a partial write", async () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-settled-race-")); + const target = join(dir, "settled.json"); + try { + writeFileSync(target, "{\"gate\":"); // what a half-finished write looks like + const pending = waitForJson<{ gate: { status: string } }>(target, 5_000); + await Bun.sleep(50); + writeFileSync(target, JSON.stringify({ gate: { status: "ready" } })); + expect(await pending).toMatchObject({ gate: { status: "ready" } }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, 15_000); });