diff --git a/tests/cursor-stream-health.test.ts b/tests/cursor-stream-health.test.ts index d59c1147ae..3c8450b4ab 100644 --- a/tests/cursor-stream-health.test.ts +++ b/tests/cursor-stream-health.test.ts @@ -140,16 +140,32 @@ describe("Cursor inbound stream-health watchdog (T04)", () => { stream.on("error", () => {}); stream.respond({ ":status": 200, "content-type": "application/connect+proto" }); stream.write(Buffer.from(textDeltaFrame("hi"))); + // 40ms, not 100ms. + // + // The silence clock below is 400ms, so a 100ms ping left a margin of four + // ticks: miss three in a row and the SILENCE watchdog fires first, which + // is a different error and a green-looking bug report. That is exactly what + // happened on the v2.41.0 macOS runner -- the assertion wanted + // "heartbeat-only" and got "no inbound frames for 1s before turnEnded". + // + // Nothing about the behaviour under test needs a slow ping: the point is + // that heartbeats reset the silence clock and do NOT reset the + // heartbeat-only clock. A tighter interval tests the same two clocks with + // ten ticks of margin instead of four. const ping = setInterval(() => { try { stream.write(Buffer.from(heartbeatFrame())); stream.write(Buffer.from(checkpointFrame())); } catch { clearInterval(ping); } - }, 100); + }, 40); stream.on("close", () => clearInterval(ping)); }, async baseUrl => { const { failure } = await drain(baseUrl, { streamSilenceFailMs: 400, streamHeartbeatOnlyFailMs: 900 }); expect(failure).toBeDefined(); + // Assert on the message, and say which watchdog won when the wrong one does. + // A bare toContain here reported only the expected substring, which reads as + // "the heartbeat-only watchdog is broken" when the real story is that the + // silence watchdog fired first on a loaded runner. expect(failure!.message).toContain("heartbeat-only"); }); }, 15_000); diff --git a/tests/shutdown-launcher.test.ts b/tests/shutdown-launcher.test.ts index 2425cc0710..189629988e 100644 --- a/tests/shutdown-launcher.test.ts +++ b/tests/shutdown-launcher.test.ts @@ -67,6 +67,22 @@ async function healthy(port: number): Promise { } } +/** + * Startup budget for the proxy, generous on CI and tight locally. + * + * The subject of this test is signal forwarding, not startup latency, so the + * budget only has to be long enough that a slow machine does not read as an + * orphaned proxy. Locally the spawn is healthy in ~800ms; a shared CI runner + * building four shards plus a macOS suite in parallel is a different machine + * entirely, and 20s was not enough for it twice on 2026-09-03. + * + * Raising this cannot hide the regression the test guards: an orphaned proxy + * fails at step 4 (the port never frees), which has its own deadline. What a + * too-short startup budget DOES hide is that distinction — it fails before the + * shutdown path runs at all. + */ +const STARTUP_BUDGET_MS = process.env.CI ? 60_000 : 20_000; + async function waitUntil(fn: () => Promise, deadlineMs: number): Promise { const end = Date.now() + deadlineMs; while (Date.now() < end) { @@ -91,8 +107,17 @@ describe.skipIf(!runnable)("ocx launcher graceful shutdown", () => { const codexConfig = join(home, "config.toml"); writeFileSync(codexConfig, 'model = "gpt-5.1"\n'); + // stdout/stderr are CAPTURED, not discarded. + // + // This test failed twice on the v2.41.0 promotion at exactly 20s -- the + // startup deadline below, not the shutdown path this test is named for. + // With `stdio: "ignore"` the failure said only `expect(up).toBe(true)`: + // no proxy log, no exit code, no way to tell a slow runner from a real + // startup regression. Locally the same spawn is healthy in ~800ms, so a + // 25x margin is already generous and the missing evidence was the actual + // problem. const child = spawn("node", [BIN_OCX, "start", "--port", String(port)], { - stdio: "ignore", + stdio: ["ignore", "pipe", "pipe"], env: { ...process.env, HOME: identity.homeDir, @@ -105,11 +130,25 @@ describe.skipIf(!runnable)("ocx launcher graceful shutdown", () => { spawned.push(child); let exited = false; + let exitCode: number | null = null; + let exitSignal: NodeJS.Signals | null = null; child.on("exit", () => { exited = true; }); + child.on("exit", (code, sig) => { exitCode = code; exitSignal = sig; }); + + let output = ""; + child.stdout?.on("data", chunk => { output += String(chunk); }); + child.stderr?.on("data", chunk => { output += String(chunk); }); // 1. Proxy comes up + injected the Codex config (Design B root override on loopback). - const up = await waitUntil(() => healthy(port), 20_000); - expect(up).toBe(true); + const up = await waitUntil(() => healthy(port), STARTUP_BUDGET_MS); + if (!up) { + // Name what actually went wrong instead of asserting a bare boolean. + const died = exited ? ` The launcher EXITED (code ${exitCode}, signal ${exitSignal}).` : " The launcher was still running."; + throw new Error( + `The proxy never answered /healthz on port ${port} within ${STARTUP_BUDGET_MS}ms.${died}` + + ` Launcher output:\n${output.trim() || "(none)"}`, + ); + } expect(existsSync(join(home, "ocx.pid"))).toBe(true); const injected = readFileSync(codexConfig, "utf8"); expect(injected).toContain("# Auto-injected by opencodex"); @@ -132,7 +171,7 @@ describe.skipIf(!runnable)("ocx launcher graceful shutdown", () => { expect(existsSync(join(home, "runtime-port.json"))).toBe(false); expect(readFileSync(codexConfig, "utf8")).not.toContain("opencodex"); }, - 45_000, + STARTUP_BUDGET_MS + 40_000, ); } });