diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3b436a92c..d5cfde68b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,6 +15,7 @@ ### Fixed - Fixed race conditions and potential hangs during agent startup by improving process lifecycle management +- Fixed RpcClient startup errors intermittently losing the child's stderr (e.g. the "Unknown provider" message) by waiting for the process to exit — and its stderr to fully drain — before reporting a failed start; also fixed an unhandled rejection when the agent process is killed before becoming ready - Improved reliability of file edits when snapshots share identical hash tags - Fixed terminal creation in ACP by properly wrapping shell commands and arguments, resolving execution failures (such as ENOENT errors) on Windows and POSIX platforms diff --git a/scripts/ci-test-ts.ts b/scripts/ci-test-ts.ts index a71eb2dce..4f3310c04 100755 --- a/scripts/ci-test-ts.ts +++ b/scripts/ci-test-ts.ts @@ -434,7 +434,9 @@ async function runTestCommand(testCommand: TestCommand): Promise { stdout: "inherit", stderr: "inherit", }); + const killTimer = setTimeout(() => proc.kill("SIGKILL"), chunkTimeoutMs()); const exitCode = await proc.exited; + clearTimeout(killTimer); if (exitCode !== 0) { throw new Error(`${testCommand.label} failed with exit code ${exitCode}: ${renderedCommand}`); } @@ -443,8 +445,25 @@ async function runTestCommand(testCommand: TestCommand): Promise { // Child env shared by every spawned test process: the parent env with all CI // credential / cloud-config variables scrubbed (see SCRUBBED_ENV_* above) and // GITHUB_ACTIONS cleared so suites resolve only against their own fixtures. +// +// GC knobs (both needed — they gate different JSC mechanisms): +// - `BUN_JSC_useConcurrentGC=0` stops the collector from marking concurrently +// with the mutator (868789972, an earlier GC crash under bun test). +// - `BUN_JSC_numberOfGCMarkers=1` removes the ParallelHelperPool marker +// threads. Bun 1.3.14 segfaults/aborts inside parallel marking +// (`DOMGCOutputConstraint::executeImplImpl` → `visitOutputConstraints` on a +// dead cell; also "Pure virtual function called!") on heap-heavy +// coding-agent chunks (~1.3GB RSS, native ghostty-vt cells). Repro: UI +// bucket chunk crashed ~25% of runs with `BUN_JSC_forceRAMSize=256MB`, +// 0/10 with markers=1, at zero measured wall-time cost. useConcurrentGC=0 +// alone did not prevent it — the crash predates this knob. function buildChildEnv(): Record { - const env: Record = { ...Bun.env, GITHUB_ACTIONS: "", BUN_JSC_useConcurrentGC: "0" }; + const env: Record = { + ...Bun.env, + GITHUB_ACTIONS: "", + BUN_JSC_useConcurrentGC: "0", + BUN_JSC_numberOfGCMarkers: "1", + }; for (const key of Object.keys(env)) { if (isScrubbedEnvVar(key)) { delete env[key]; @@ -453,6 +472,18 @@ function buildChildEnv(): Record { return env; } +// Per-chunk watchdog. A bun child that wedges (e.g. the panic handler +// deadlocking after a GC crash) would otherwise stall the whole run: the +// parallel path awaits the child's stdout/stderr pipes, which stay open as +// long as the wedged process — or any grandchild that inherited them — lives. +// After this many seconds the child is SIGKILLed and reported as a failure. +// Override with OMP_TEST_CHUNK_TIMEOUT (seconds). +function chunkTimeoutMs(): number { + const raw = Number(Bun.env.OMP_TEST_CHUNK_TIMEOUT?.trim()); + if (Number.isFinite(raw) && raw >= 1) return raw * 1000; + return 600_000; +} + // The standard `CI` signal is authoritative. In CI each bucket is its own // memory-capped runner job (a single fat invocation gets OOM-killed at 137), so // chunks run sequentially within a job and parallelism happens across jobs. @@ -632,7 +663,7 @@ export function formatFailureReport(failures: ChunkOutcome[], total: number, rep // at the end; `--full` streams every chunk's output inline as it completes. All // failures are collected and reported together instead of failing fast, so one // run surfaces every broken chunk and exits non-zero without a runner stack trace. -async function runTestCommandsInParallel(commands: TestCommand[], concurrency: number): Promise { +export async function runTestCommandsInParallel(commands: TestCommand[], concurrency: number): Promise { const env = buildChildEnv(); const queue = [...commands]; const failures: ChunkOutcome[] = []; @@ -642,6 +673,45 @@ async function runTestCommandsInParallel(commands: TestCommand[], concurrency: n `(OMP_TEST_CONCURRENCY=|all to change).`, ); + // Incremental, cancellable drain into a mutable sink, so a watchdog-killed + // chunk still reports whatever the child managed to print before it wedged. + function drainInto( + stream: ReadableStream, + sink: { text: string }, + ): { done: Promise; cancel: () => void } { + const decoder = new TextDecoder(); + const reader = stream.getReader(); + const done = (async () => { + try { + for (;;) { + const { done: ended, value } = await reader.read(); + if (ended) break; + sink.text += decoder.decode(value, { stream: true }); + } + } catch { + // cancelled or broken pipe — keep what was captured + } + sink.text += decoder.decode(); + })(); + return { done, cancel: () => void reader.cancel().catch(() => {}) }; + } + + // Wait for `promise` at most `ms`; resolves `true` when it settled in time. + // Never rejects. + async function settleWithin(promise: Promise, ms: number): Promise { + const { promise: expired, resolve } = Promise.withResolvers(); + const timer = setTimeout(() => resolve(false), ms); + const settled = await Promise.race([ + promise.then( + () => true, + () => true, + ), + expired, + ]); + clearTimeout(timer); + return settled; + } + async function worker(): Promise { for (;;) { const testCommand = queue.shift(); @@ -656,18 +726,35 @@ async function runTestCommandsInParallel(commands: TestCommand[], concurrency: n stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - new Response(proc.stdout as ReadableStream).text(), - new Response(proc.stderr as ReadableStream).text(), - proc.exited, - ]); + const stdout = { text: "" }; + const stderr = { text: "" }; + const stdoutDrain = drainInto(proc.stdout as ReadableStream, stdout); + const stderrDrain = drainInto(proc.stderr as ReadableStream, stderr); + const drains = Promise.all([stdoutDrain.done, stderrDrain.done]); + // Watchdog: a wedged child (e.g. bun's panic handler deadlocking + // after a GC crash) would otherwise hang this worker forever. + let timedOut = false; + const killTimer = setTimeout(() => { + timedOut = true; + proc.kill("SIGKILL"); + }, chunkTimeoutMs()); + const exitCode = await proc.exited; + clearTimeout(killTimer); + // Cap the post-exit drain: a leaked grandchild that inherited the + // pipes keeps them open indefinitely, and a pending read would keep + // the runner's event loop alive — cancel the readers instead. + if (!(await settleWithin(drains, 5000))) { + stdoutDrain.cancel(); + stderrDrain.cancel(); + await drains; + } completed += 1; const outcome: ChunkOutcome = { label: testCommand.label, command: renderedCommand, exitCode, seconds: (performance.now() - startedAt) / 1000, - output: `${stdout}${stderr}`, + output: `${stdout.text}${stderr.text}${timedOut ? `\n[watchdog] chunk exceeded ${Math.round(chunkTimeoutMs() / 1000)}s; killed with SIGKILL (OMP_TEST_CHUNK_TIMEOUT to change)\n` : ""}`, }; if (quiet) { process.stdout.write(`${formatProgressLine(outcome)}\n`); @@ -677,7 +764,7 @@ async function runTestCommandsInParallel(commands: TestCommand[], concurrency: n `\n==> [${completed}/${commands.length}] ${testCommand.label} (${status}, ${outcome.seconds.toFixed(1)}s)\n$ ${renderedCommand}\n${outcome.output}`, ); } - if (exitCode !== 0) { + if (exitCode !== 0 || timedOut) { failures.push(outcome); } }