Skip to content

Commit d5ba68f

Browse files
authored
Make the E2E (stdio MCP) job robust to two harness flakes (#1172)
The "E2E (stdio MCP)" CI job runs `stdio-mcp.test.ts` alone, so it always pays a cold `vite optimizeDeps` boot, the one variable step. Two unrelated harness issues let that job go red without a real product regression. 1. Blind boot timeout. The scenario's test timeout (180s) was set equal to the boot-URL wait inside `withLocalServer` (180s). When boot is slow both deadlines fire together and vitest's generic "Test timed out in 180000ms" wins, so the failure carries no clue about what boot got stuck on. Raise the scenario timeout to 240s so the boot wait trips first, and have that wait reject with the terminal tail (waitUntil otherwise throws a tail-less "timed out"). A slow or wedged boot now reports how far it got. 2. Teardown crash on a truncated recording. `toAsciicast` JSON.parsed every recording line; a PTY killed mid-write leaves a truncated final JSONL line, throwing "Unexpected end of JSON input" and failing an otherwise-passing test over a best-effort film artifact. Skip unparseable lines (a dropped frame, not a failure) and make the cast write best-effort, matching the recording fetch beside it. Verified: targeted run passes repeatedly (~3-7s); forcing the boot wait to trip yields the new descriptive error instead of a bare timeout.
1 parent 5250f64 commit d5ba68f

3 files changed

Lines changed: 60 additions & 18 deletions

File tree

e2e/local/local-server.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,29 @@ export const withLocalServer = (
5959
async (term) => {
6060
markRecordingStart(runDir, "terminal");
6161
markFocus(runDir, "terminal");
62-
const snapshot = await term.screen.waitUntil(
63-
(current) => TOKEN_URL.test(current.text),
64-
// A cold `executor web` runs vite's optimizeDeps before it prints
65-
// the URL; give CI runners headroom over the warm ~3s boot.
66-
{ timeoutMs: 180_000 },
67-
);
62+
// Capture the latest rendered screen on every poll so a boot that
63+
// never prints the URL is still diagnosable: `waitUntil` rejects
64+
// with a tail-less "timed out" on its deadline, so without this the
65+
// failure tells us nothing about how far boot actually got.
66+
let lastText = "";
67+
const snapshot = await term.screen
68+
.waitUntil(
69+
(current) => {
70+
lastText = current.text;
71+
return TOKEN_URL.test(current.text);
72+
},
73+
// A cold `executor web` runs vite's optimizeDeps before it prints
74+
// the URL; 180s is generous headroom over the warm ~3s boot. Kept
75+
// BELOW each scenario's test timeout (240s) so that on a slow or
76+
// wedged boot THIS error surfaces, with the terminal tail, rather
77+
// than racing vitest's generic timeout at the same deadline.
78+
{ timeoutMs: 180_000 },
79+
)
80+
.catch((cause: unknown) => {
81+
throw new Error(
82+
`executor web --foreground printed no ?_token URL within 180s (${String(cause)}):\n${lastText.slice(-600)}`,
83+
);
84+
});
6885
const url = TOKEN_URL.exec(snapshot.text)?.[0];
6986
if (!url) {
7087
throw new Error(

e2e/local/stdio-mcp.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,14 @@ const SECRET = "s3cr3t-from-the-vault";
3939

4040
scenario(
4141
"Local · a stdio MCP server's tools are detected on a fresh install, with env stored as a secret",
42-
{ timeout: 180_000 },
42+
// Must stay STRICTLY greater than the boot-URL wait in `withLocalServer`
43+
// (currently 180s). When this CI job runs `stdio-mcp.test.ts` alone it always
44+
// pays a cold `vite optimizeDeps` boot (no prior file to warm the cache), the
45+
// one variable step. If this test timeout equals the boot wait, both deadlines
46+
// fire together and vitest's generic "Test timed out" wins, swallowing the
47+
// harness's "printed no ?_token URL\n<terminal tail>" error that tells us what
48+
// boot actually got stuck on. Keep the gap so the boot diagnostic surfaces.
49+
{ timeout: 240_000 },
4350
Effect.gen(function* () {
4451
const cli = yield* Cli;
4552
const runDir = yield* RunDir;

e2e/src/surfaces/cli.ts

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,32 @@ export interface CliSurface {
2424
) => Effect.Effect<T>;
2525
}
2626

27+
interface AsciicastEvent {
28+
type: string;
29+
cols?: number;
30+
rows?: number;
31+
at_ms?: number;
32+
bytes?: number[];
33+
}
34+
2735
/** terminal-control's JSONL recording → asciicast v2 (what asciinema plays). */
2836
const toAsciicast = (recording: Uint8Array): string => {
2937
const events = new TextDecoder()
3038
.decode(recording)
3139
.split("\n")
3240
.filter(Boolean)
33-
.map(
34-
(line) =>
35-
JSON.parse(line) as {
36-
type: string;
37-
cols?: number;
38-
rows?: number;
39-
at_ms?: number;
40-
bytes?: number[];
41-
},
42-
);
41+
.flatMap((line): AsciicastEvent[] => {
42+
// The PTY can be killed mid-write on teardown (e.g. a vitest timeout
43+
// interrupts the fiber), leaving a truncated final JSONL line. A line
44+
// that doesn't parse is one dropped frame, never a failed test: skip it
45+
// instead of throwing.
46+
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: lenient parse of a best-effort recording artifact
47+
try {
48+
return [JSON.parse(line) as AsciicastEvent];
49+
} catch {
50+
return [];
51+
}
52+
});
4353
const header = events.find((event) => event.type === "header");
4454
const lines = [
4555
JSON.stringify({ version: 2, width: header?.cols ?? 80, height: header?.rows ?? 24 }),
@@ -89,7 +99,15 @@ export const makeCliSurface = (): CliSurface => ({
8999
Effect.promise(async () => {
90100
if (options?.record) {
91101
const recording = await session.recording().catch(() => undefined);
92-
if (recording) writeFileSync(options.record, toAsciicast(recording));
102+
// Writing the cast is a best-effort film artifact (symmetric with the
103+
// fetch above): a serialization or IO hiccup in teardown must never
104+
// fail an otherwise-passing test.
105+
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: best-effort artifact write
106+
try {
107+
if (recording) writeFileSync(options.record, toAsciicast(recording));
108+
} catch {
109+
// drop the cast; the test result stands
110+
}
93111
}
94112
await session.stop().catch(() => {});
95113
await tc[Symbol.asyncDispose]();

0 commit comments

Comments
 (0)