diff --git a/tests/codex-integration/codex-write-lock.test.ts b/tests/codex-integration/codex-write-lock.test.ts index 71d0631f53..8fcd2c034d 100644 --- a/tests/codex-integration/codex-write-lock.test.ts +++ b/tests/codex-integration/codex-write-lock.test.ts @@ -331,13 +331,34 @@ describe("two real processes contend for one lock", () => { // literal expired first on run 33930757649 ("case 0", 10.67 s). INTERNAL_DEADLINE_MS is // the named bound for an in-test wait and stays under the enclosing SPAWN_BUDGET_MS so // this helper's "timed out waiting for" diagnostic is what gets reported, not Bun's. - async function waitFor(path: string, timeoutMs = INTERNAL_DEADLINE_MS): Promise { + // + // The CHILD is watched here, not only the file. Until it was, a child that died before + // publishing produced the same "timed out waiting for" line as one that was merely slow on a + // loaded shard, so nothing in CI could tell those apart -- and the two want opposite fixes. + // Racing the exit reports the dead child immediately, with its code and stderr, instead of + // spending the rest of the deadline to say nothing (run 35211904734, windows 3/9). + async function waitFor( + path: string, + child: ReturnType, + timeoutMs = INTERNAL_DEADLINE_MS, + ): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (Bun.file(path).size > 0) return; + if (child.exitCode !== null || child.signalCode !== null) { + // The marker write and the exit can land in the same 10 ms gap, so look once more + // before calling it a death: a holder that published and then exited is not a failure. + if (Bun.file(path).size > 0) return; + throw new Error( + `child exited (code=${child.exitCode}, signal=${child.signalCode}) before publishing ` + + `${path}; stderr=${await new Response(child.stderr).text()}`, + ); + } await Bun.sleep(10); } - throw new Error(`timed out waiting for ${path}`); + // Still running, so this one really is a slow boot rather than a crash. Say which, because + // the previous message was true of both. + throw new Error(`timed out waiting for ${path} after ${timeoutMs}ms; the child is still running`); } test("a second process is excluded while the first holds, and succeeds after it releases", async () => { @@ -345,7 +366,7 @@ describe("two real processes contend for one lock", () => { const releaseMarker = join(root, "release"); const holder = spawnChild({ holdMarker, releaseMarker, timeoutMs: 0, holdMs: 20_000 }); - await waitFor(holdMarker); + await waitFor(holdMarker, holder); // The lock is genuinely held by another process right now. const blocked = await withCodexWriteLock(options({ timeoutMs: 0 }), publishing("parent")); @@ -379,13 +400,13 @@ describe("two real processes contend for one lock", () => { const releaseMarker = join(root, "release-2"); const waitMarker = join(root, "waiting-2"); const holder = spawnChild({ holdMarker, releaseMarker, timeoutMs: 0, holdMs: 20_000 }); - await waitFor(holdMarker); + await waitFor(holdMarker, holder); const waiter = spawnChild({ timeoutMs: 5_000, waitMarker }); // The waiter writes this only after withCodexWriteLock has returned its // pending promise. Because the holder is still held, that means the waiter // has attempted N and reached the retry wait rather than failing fast. - await waitFor(waitMarker); + await waitFor(waitMarker, waiter); writeFileSync(releaseMarker, "go"); const [waited, holderResult] = await Promise.all([childResult(waiter), childResult(holder)]); @@ -456,7 +477,7 @@ describe("two real processes contend for one lock", () => { // to outlast the contender's process boot, which took >4 s on windows-latest in run // 33603770447 and made the default 3 s hold expire first (read as 'acquired'). const holder = spawnChildWithEnv({ holdMarker, releaseMarker, timeoutMs: 0, holdMs: 20_000 }, { ...a }); - await waitFor(holdMarker); + await waitFor(holdMarker, holder); // Fail-fast: if the two environments produced different lock files this // would acquire instead of reporting contention. diff --git a/tests/codex-integration/native-main-owner-lifetime.test.ts b/tests/codex-integration/native-main-owner-lifetime.test.ts index 49d42dcae6..1bca364f8b 100644 --- a/tests/codex-integration/native-main-owner-lifetime.test.ts +++ b/tests/codex-integration/native-main-owner-lifetime.test.ts @@ -211,8 +211,29 @@ class ChildHarness { for (;;) { const found = this.events.find(predicate); if (found) return found; + // A dead child and a slow one used to report identically. On run 35210400258 + // (windows 7/9) the first wait of a case failed with `events=[] stderr=` -- and because + // that stderr promise only resolves at EOF, its emptiness proves the child had already + // exited, silently, rather than that it was still booting. The message never said so. + // Report the exit the moment it happens, with the code, instead of spending the deadline. + if (this.child.exitCode !== null || this.child.signalCode !== null) { + // The event and the exit can land in the same wake, so re-check before blaming death. + const settled = this.events.find(predicate); + if (settled) return settled; + throw new Error( + `child exited (code=${this.child.exitCode}, signal=${this.child.signalCode}) before the ` + + `awaited event; events=${JSON.stringify(this.events)} stderr=${await this.stderr}`, + ); + } if (Date.now() >= deadline) { - throw new Error(`child event timeout; events=${JSON.stringify(this.events)} stderr=${await this.stderr}`); + // Do NOT await `this.stderr` unguarded here. It resolves at EOF, so for the case this + // branch now describes -- a child still running -- it would never settle, and the + // timeout would hang until the enclosing budget killed the test with a worse message. + const stderr = await Promise.race([this.stderr, Bun.sleep(1_000).then(() => "")]); + throw new Error( + `child event timeout after ${timeoutMs}ms; the child is still running; ` + + `events=${JSON.stringify(this.events)} stderr=${stderr}`, + ); } await Promise.race([ new Promise(resolve => this.waiters.add(resolve)),