From 8f1a9344b80367417a2aee728a8874234f325be9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 26 Aug 2026 13:46:58 +0900 Subject: [PATCH] test(update): give the launcher-recovery case a budget that contains its own work This test failed on macOS shards at 46-47s across three unrelated PRs while the product was healthy. Rerunning made it pass, which is how a broken budget disguises itself as an infrastructure blip. The arithmetic never worked. The case spawns `node launcher update` (30s timeout), waits for the recovered proxy, then spawns `node launcher stop` (30s timeout) in its finally block - inside a 60s Bun timeout. When 34ef53966 raised the readiness wait from 15s to 45s to survive a loaded runner, it fixed the wait and left 60s of spawn budget with nowhere to go: 30 + 45 + 30 does not fit in 60, so on any runner where the proxy was slower than about 15s the case could not finish at all. Locally the proxy boots in ~2s and the whole case takes 2.4s, so the ceiling was invisible until CI was loaded enough to need the headroom that was promised but not reserved. The budget is now derived from its parts rather than guessed against them, and a contract test asserts the relationship instead of the numbers - so raising any single timeout later cannot silently recreate an impossible one. Falsified: restoring the 60s value fails that test. The readiness deadline stays 45s deliberately. It exists to stop a hung proxy, not to assert a boot deadline the suite never intended to enforce; a slow-but-live proxy must still pass. bun test tests/update-stop-first.test.ts: 15 pass, 0 fail (was 14). bun x tsc --noEmit: exit 0. --- tests/update-stop-first.test.ts | 53 ++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/tests/update-stop-first.test.ts b/tests/update-stop-first.test.ts index 3cf9dabdbf..140ed3c491 100644 --- a/tests/update-stop-first.test.ts +++ b/tests/update-stop-first.test.ts @@ -20,14 +20,28 @@ function freePort(): Promise { return promise; } +/** + * Budget for the whole recovery case, and the arithmetic that keeps it honest. + * + * A cold detached proxy takes ~2s locally, but this test runs inside a CI batch of twelve files + * on a shared runner where the same boot has blown a 15s budget. Raising the readiness wait to + * 45s fixed that and introduced a worse failure: the case ALSO spawns `node launcher update` + * (up to 30s) before the wait even starts, and `node launcher stop` (up to 30s) after it. With + * a 60s Bun timeout, a 45s wait leaves the readiness probe unable to finish inside the case at + * all — observed failing at 46-47s on macOS, which reads as a product defect and is not one. + * + * So the budget is derived from the timeout rather than guessed against it: the wait gets what + * remains after the spawns, and the Bun timeout is stated as the sum of its parts. The deadline + * exists to stop a HUNG proxy, not to assert a boot deadline the suite never intended to + * enforce — a slow-but-live proxy must still pass. + */ +const UPDATE_SPAWN_TIMEOUT_MS = 30_000; +const PROXY_READY_TIMEOUT_MS = 45_000; +/** Spawn + readiness + teardown spawn, plus headroom for fixture IO on a loaded runner. */ +const RECOVERY_CASE_TIMEOUT_MS = UPDATE_SPAWN_TIMEOUT_MS + PROXY_READY_TIMEOUT_MS + UPDATE_SPAWN_TIMEOUT_MS + 15_000; + async function waitForProxy(port: number): Promise { - // A cold detached proxy takes ~2s locally, but this test runs inside a CI - // batch of twelve files on a shared runner, where the same boot has been - // observed to blow a 15s budget and fail the whole shard. The test's own - // Bun timeout is 60s, so the readiness wait may use most of that: this - // deadline exists to stop a hung proxy, not to assert a boot deadline the - // suite never intended to enforce. - const deadline = Date.now() + 45_000; + const deadline = Date.now() + PROXY_READY_TIMEOUT_MS; while (Date.now() < deadline) { try { const response = await fetch(`http://127.0.0.1:${port}/healthz`, { @@ -200,7 +214,7 @@ esac env, stdout: "pipe", stderr: "pipe", - timeout: 30_000, + timeout: UPDATE_SPAWN_TIMEOUT_MS, }); const output = result.stdout.toString() + result.stderr.toString(); @@ -219,7 +233,7 @@ esac env, stdout: "ignore", stderr: "ignore", - timeout: 30_000, + timeout: UPDATE_SPAWN_TIMEOUT_MS, }) : null; if (stopped?.exitCode !== 0) { @@ -233,9 +247,28 @@ esac rmSync(root, { recursive: true, force: true }); } }, - 60_000, + RECOVERY_CASE_TIMEOUT_MS, ); + /** + * The budget arithmetic itself, pinned. + * + * The recovery case spawns `update`, waits for readiness, then spawns `stop`. A wait budget + * chosen independently of the Bun timeout is how this test became flaky: 45s of readiness + * inside a 60s case that also spends up to 60s on two spawns cannot finish, and it failed at + * 46-47s on macOS while the product was healthy. + * + * This asserts the relationship rather than the numbers, so raising any single budget in + * future cannot silently recreate the impossible one. + */ + test("the recovery case timeout can actually contain its own spawns and readiness wait", () => { + // Both spawns plus the readiness wait must fit, with room left for fixture IO. + const consumed = UPDATE_SPAWN_TIMEOUT_MS * 2 + PROXY_READY_TIMEOUT_MS; + expect(RECOVERY_CASE_TIMEOUT_MS).toBeGreaterThanOrEqual(consumed); + expect(RECOVERY_CASE_TIMEOUT_MS - consumed).toBeGreaterThanOrEqual(10_000); + }); + + test("both update paths surface an incomplete manifest-backed history restore after the stop", () => { // A codex-history-backup-*.json surviving `ocx stop` means exact metadata restoration // remains pending. It can be contention or an integrity refusal, so neither update path