From 0c1aac25f00907a233e67fc8233947490b94cb85 Mon Sep 17 00:00:00 2001 From: gaebal-gajae Date: Thu, 6 Aug 2026 23:11:20 +0000 Subject: [PATCH] fix(test): isolate smithery-env-trust first-spawn cold-start under CI contention Root cause: the first Bun probe child absorbs cold compile of the probe + env module graph. Under coding-agent shard contention that cold start can exceed the 60s per-test budget (observed 60001ms on #3969 exact-head) while the four sibling cases complete in ~300ms once the module graph is warm. Fix (harness isolation, assertions unchanged): - beforeAll warmup spawn (120s suite budget) moves cold-start outside it() - resolveIn kills stalled children at 45s with a diagnostic instead of leaking pipes until the outer it() timeout Not a third blind per-test timeout bump; the 60s it() budgets stay. Lore-id: smithery-spawn-isolation-1 Confidence: high Scope-risk: narrow Reversibility: trivial Tested: not fully offline (local natives missing); exact-head CI verifies Not-tested: full parallel shard matrix offline --- packages/coding-agent/CHANGELOG.md | 1 + .../test/smithery-env-trust.test.ts | 49 +++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 63eb0d2a72..c34b2ca3f3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixed +- `smithery-env-trust.test.ts` warms the Bun probe child in `beforeAll` and kills stalled spawns at a 45s budget so the first case no longer absorbs cold-start compile cost into its per-test timeout under shard contention (observed 60001ms timeout after the 60s cap on #3969 exact-head CI). Assertions unchanged. - `smithery-env-trust.test.ts` raises the per-test child-process timeout from 30s to 60s so CI contention cannot fail at the previous 30s cap (Dev CI run 31128319216 timed out at 30004ms). - `smithery-env-trust.test.ts` now sets a 30s per-test timeout on all five child-process-spawning trust-boundary tests, preventing CI flake when the Bun child-process spawn + env-file-parse chain exceeds the default 5s budget under parallel shard contention (Dev CI run 31102063678). diff --git a/packages/coding-agent/test/smithery-env-trust.test.ts b/packages/coding-agent/test/smithery-env-trust.test.ts index 1166171f11..1626fffe2b 100644 --- a/packages/coding-agent/test/smithery-env-trust.test.ts +++ b/packages/coding-agent/test/smithery-env-trust.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -46,6 +46,12 @@ afterEach(() => { for (const dir of tempDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); }); +// Per-spawn budget for the probe child. After a suite warmup, healthy spawns +// finish in ~300ms; under extreme shard contention they may take a few seconds. +// Kill rather than wait for the outer it() timeout so a stalled child cannot +// pin the suite for the full 60s and leak pipes. +const PROBE_SPAWN_BUDGET_MS = 45_000; + async function resolveIn(cwd: string, overrides: Record = {}): Promise { const env: Record = {}; for (const [key, value] of Object.entries(process.env)) { @@ -59,10 +65,32 @@ async function resolveIn(cwd: string, overrides: Record = {}): P Object.assign(env, overrides); const proc = Bun.spawn([process.execPath, PROBE], { cwd, env, stdout: "pipe", stderr: "pipe" }); - const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); - const exitCode = await proc.exited; - if (exitCode !== 0) throw new Error(`probe failed (${exitCode}): ${stderr}`); - return JSON.parse(stdout.trim()) as Resolved; + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + try { + proc.kill(); + } catch { + // already exited + } + }, PROBE_SPAWN_BUDGET_MS); + try { + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + if (timedOut) { + throw new Error( + `probe timed out after ${PROBE_SPAWN_BUDGET_MS}ms and was killed` + + (stderr.trim() ? `: ${stderr.trim()}` : ""), + ); + } + if (exitCode !== 0) throw new Error(`probe failed (${exitCode}): ${stderr}`); + return JSON.parse(stdout.trim()) as Resolved; + } finally { + clearTimeout(timer); + } } const PLANTED = [ @@ -72,6 +100,17 @@ const PLANTED = [ ].join("\n"); describe("Smithery env trust boundary", () => { + // Cold-start the probe module graph outside per-test budgets. Under CI shard + // contention the first Bun child can spend tens of seconds compiling the + // probe + env stack; later spawns then complete in ~300ms. Without a warmup, + // the first it() absorbs cold-start into its 60s budget and flakes (observed + // 60001ms on #3969 exact-head after the 60s bump). beforeAll is the isolation + // fix; 120s matches other child-process suite budgets and is not a third + // blind per-test timeout bump. + beforeAll(async () => { + await resolveIn(projectDir()); + }, 120_000); + it("uses the built-in endpoints and no key by default", async () => { const resolved = await resolveIn(projectDir()); expect(resolved.url).toBe("https://smithery.ai");