Merge pull request #1982 from AsafMah/fix/js-eval-worker-init-flake

fix(eval): floor JS worker init timeout to stop terminate-mid-init CI flake

packages/coding-agent/CHANGELOG.md

@@ -1,6 +1,9 @@
 # Changelog
 
 ## [Unreleased]
+### Fixed
+
+- Fixed a flaky JS eval worker startup that intermittently failed unrelated CI runs. The worker-ready wait reused Bun's 5s default per-test timeout as its floor, so a slow cold-start under `--isolate` + high concurrency was aborted mid-init; terminating a still-initializing Bun worker is the documented SIGILL/SIGTRAP crash trigger, which took down the whole test file. Worker init now floors at a fixed 15s infrastructure budget (independent of, and still dominated by, a larger per-cell `timeout`), and the JS eval test suites set a 20s file-local timeout so cold starts complete instead of being torn down.
 
 ### Fixed
 

packages/coding-agent/src/eval/js/context-manager.ts

@@ -53,7 +53,13 @@ interface JsSession {
 const sessions = new Map<string, JsSession>();
 const startingSessions = new Map<string, Promise<JsSession>>();
 const resettingSessions = new Set<string>();
-const READY_TIMEOUT_MS_DEFAULT = 5_000;
+// Worker startup (module-graph import + WorkerCore construction) is infrastructure
+// cost, not user compute. Floor it independently of Bun's 5s default per-test timeout
+// so a slow cold-start under load isn't aborted mid-init — terminating a still-
+// initializing Bun worker triggers the same kind of terminate-race that motivates
+// avoiding `vm.runInContext` (see shared/indirect-eval.ts), here surfacing as a
+// SIGILL/SIGSEGV. Callers that pass a larger per-cell budget still dominate.
+const WORKER_INIT_TIMEOUT_MS = 15_000;
 
 export async function executeInVmContext(options: {
 	sessionKey: string;
@@ -191,9 +197,9 @@ async function acquireSession(sessionKey: string, snapshot: SessionSnapshot, tim
 			handleSessionMessage(session, msg);
 		});
 		try {
-			// Cold-start can exceed 5s on slow hosts. Let the caller's per-cell timeout dominate so
-			// users can grant more headroom when they raise `timeout` on a cell.
-			const readyTimeoutMs = Math.max(READY_TIMEOUT_MS_DEFAULT, timeoutMs ?? 0);
+			// Init headroom is the fixed infrastructure floor; the caller's per-cell timeout
+			// dominates when larger so users can grant more by raising `timeout` on a cell.
+			const readyTimeoutMs = Math.max(WORKER_INIT_TIMEOUT_MS, timeoutMs ?? 0);
 			await raceWithTimeout(readyPromise, readyTimeoutMs, "Timed out initializing JS eval worker");
 			worker.send({ type: "init", snapshot });
 			sessions.set(sessionKey, session);

packages/coding-agent/test/core/js-executor.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test";
+import { afterAll, afterEach, beforeAll, describe, expect, it, setDefaultTimeout, vi } from "bun:test";
 import * as path from "node:path";
 import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core";
 import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
@@ -8,6 +8,11 @@ import * as z from "zod/v4";
 import { disposeAllVmContexts } from "../../src/eval/js/context-manager";
 import { executeJs, type JsResult } from "../../src/eval/js/executor";
 
+// JS eval cold-starts a Bun worker; under --isolate + high CI concurrency that startup
+// can exceed Bun's 5s default per-test timeout, flaking the suite. Give the worker-backed
+// tests headroom above the worker-init floor (context-manager WORKER_INIT_TIMEOUT_MS).
+setDefaultTimeout(20_000);
+
 function createTool(
 	name: string,
 	execute: (toolCallId: string, args: unknown, signal?: AbortSignal) => Promise<AgentToolResult>,

packages/coding-agent/test/core/js-workflow-helpers.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, beforeAll, describe, expect, it } from "bun:test";
+import { afterAll, beforeAll, describe, expect, it, setDefaultTimeout } from "bun:test";
 import * as path from "node:path";
 import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
 import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
@@ -6,6 +6,11 @@ import { TempDir } from "@oh-my-pi/pi-utils";
 import { disposeAllVmContexts } from "../../src/eval/js/context-manager";
 import { executeJs, type JsResult } from "../../src/eval/js/executor";
 
+// JS eval cold-starts a Bun worker; under --isolate + high CI concurrency that startup
+// can exceed Bun's 5s default per-test timeout, flaking the suite. Give the worker-backed
+// tests headroom above the worker-init floor (context-manager WORKER_INIT_TIMEOUT_MS).
+setDefaultTimeout(20_000);
+
 function statusEvents(result: JsResult) {
 	return result.displayOutputs.filter(
 		(output): output is Extract<JsResult["displayOutputs"][number], { type: "status" }> => output.type === "status",