Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/coding-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
49 changes: 44 additions & 5 deletions packages/coding-agent/test/smithery-env-trust.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Let the warmup use its advertised 120-second budget

When the first Bun probe needs 45–120 seconds under the same shard contention this patch is intended to tolerate, this constant kills it at 45 seconds and resolveIn throws, so the 120-second beforeAll timeout can never absorb the observed >60-second cold start; the suite instead fails earlier than before. The warmup needs a separate longer spawn budget, while the shorter kill budget can remain for already-warmed per-test probes.

Useful? React with 👍 / 👎.


async function resolveIn(cwd: string, overrides: Record<string, string> = {}): Promise<Resolved> {
const env: Record<string, string> = {};
for (const [key, value] of Object.entries(process.env)) {
Expand All @@ -59,10 +65,32 @@ async function resolveIn(cwd: string, overrides: Record<string, string> = {}): 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 = [
Expand All @@ -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");
Expand Down
Loading