From f4b5f32862925ac06da8e2b2938104e5e988bd90 Mon Sep 17 00:00:00 2001 From: Aditya Borakati <41518783+ABorakati@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:20:19 +0100 Subject: [PATCH] test(e2e): boot the Playwright harness on Windows (#3804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(e2e): boot the Playwright harness on Windows The harness could not start on Windows. Metro was launched with spawn("npx", ...) and the isolated daemon resolved its runner with execSync("which tsx") — npx is a .cmd shim that Node's bare spawn cannot launch, and `which` does not exist there. Route both through process.execPath instead of the node_modules/.bin shims. Node refuses to spawn .cmd or .bat without shell: true, and shell: true is the wrong escape hatch: it concatenates argv unescaped (DEP0190), splits paths containing spaces, and sends kill signals to cmd.exe rather than the real child. Spawning Node directly has none of those properties and is identical on POSIX, so no platform branch is needed. The same reasoning fixes the published-server npm install, which passed "npm.cmd" to execFileSync and had been failing with EINVAL on Windows since Node's CVE-2024-27980 mitigation landed. Teardown now kills the process tree. A Windows signal reaches only the direct child, so Metro's and the supervisor's forked workers kept the listening port and the next run failed to bind. The daemon startup deadline goes from 20s to 90s. A cold tsx start was measured at 25s on Windows and exceeded 60s under load; the loop still fails immediately on a crashed child, so the ceiling only costs time that a genuinely slow start needs. The fake editor and gh binaries the harness puts on PATH gain .cmd wrappers, because Windows resolves PATH entries by PATHEXT and cannot execute an extension-less shebang file. The wrappers point at the same payload file so the recorded command name stays "cursor", "code", or "gh" on every platform. The asdf-backed local Elixir relay stays POSIX-only and now says so before spawning. * test(ci): follow shared e2e daemon launcher --------- Co-authored-by: ABorakati Co-authored-by: Mohamed Boudra --- docs/testing.md | 3 + .../browser/workspace-model-restart.spec.ts | 34 +++----- packages/app/e2e/support/global-setup.ts | 29 ++----- .../app/e2e/support/helpers/daemon-update.ts | 10 +-- .../app/e2e/support/helpers/e2e-worker.ts | 20 ++++- .../support/helpers/isolated-host-daemon.ts | 53 +++++------- .../e2e/support/helpers/local-elixir-relay.ts | 29 +++---- .../app/e2e/support/helpers/spawn-node.ts | 84 +++++++++++++++++++ scripts/daemon-launch-contract.test.mjs | 2 +- 9 files changed, 157 insertions(+), 107 deletions(-) create mode 100644 packages/app/e2e/support/helpers/spawn-node.ts diff --git a/docs/testing.md b/docs/testing.md index be290becc1..e34191dcae 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -189,6 +189,9 @@ Test suites in this repo are heavy. Running them in bulk freezes the machine, es - Helpers that create projects or workspaces own those records until cleanup. Their clients remove the daemon project on close, and an automatic fixture fails any test that still leaks a project record. Deleting only the temporary directory is not cleanup. Agent helpers pass the intended `workspaceId` through to agent creation; they never infer ownership from `cwd`. - Tests whose subject is daemon-global state, such as an empty history or daemon restart, start a dedicated host explicitly. Filenames and directories describe product behavior, never execution order or isolation mechanics. - Global setup accepts Metro as ready only when `/status` returns `packager-status:running`, then fetches the document's scripts so the cold bundle compilation finishes before Playwright's per-test timeout starts. A generic TCP listener is not sufficient readiness evidence. The browser suite uses direct local daemon connections and does not start a relay. +- The app Playwright harness boots on Windows as well as POSIX. Spawn Node entrypoints through `process.execPath`, not `npx` or `node_modules/.bin` shims: Node refuses to spawn `.cmd` or `.bat` without `shell: true`, and shell mode concatenates argv without escaping and sends kill signals to `cmd.exe` instead of the real child. +- Teardown kills the process tree, because a Windows signal reaches only the direct child and leaves forked workers holding the listening port. +- The `asdf`-backed local Elixir relay stays POSIX-only, so the `relay-deployment` Playwright project is unavailable on Windows. ## Pull-request test routing diff --git a/packages/app/e2e/browser/workspace-model-restart.spec.ts b/packages/app/e2e/browser/workspace-model-restart.spec.ts index 54cbb0c0e8..be96b1642c 100644 --- a/packages/app/e2e/browser/workspace-model-restart.spec.ts +++ b/packages/app/e2e/browser/workspace-model-restart.spec.ts @@ -1,6 +1,5 @@ import { randomUUID } from "node:crypto"; -import { execSync, spawn, type ChildProcess } from "node:child_process"; -import { once } from "node:events"; +import type { ChildProcess } from "node:child_process"; import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -23,6 +22,7 @@ import { submitNewWorkspaceEmpty, } from "../support/helpers/new-workspace"; import { selectSidebarStatusGrouping } from "../support/helpers/sidebar"; +import { killProcessTree, spawnTsx } from "../support/helpers/spawn-node"; import { waitForSidebarHydration } from "../support/helpers/workspace-ui"; import { getVisibleWorkspaceAgentTabIds } from "../support/helpers/workspace-tabs"; @@ -195,9 +195,11 @@ async function getAvailablePort(): Promise { async function waitForServer(port: number, child: ChildProcess): Promise { const startedAt = Date.now(); let lastConnectionError: unknown = null; - while (Date.now() - startedAt < 20_000) { - if (child.exitCode !== null) { - throw new Error(`Restart test daemon exited before listening (exit ${child.exitCode}).`); + while (Date.now() - startedAt < 90_000) { + if (child.exitCode !== null || child.signalCode !== null) { + throw new Error( + `Restart test daemon exited before listening (code ${String(child.exitCode)}, signal ${String(child.signalCode)}).`, + ); } try { await new Promise((resolve, reject) => { @@ -226,21 +228,6 @@ async function waitForServer(port: number, child: ChildProcess): Promise { ); } -async function stopProcess(child: ChildProcess): Promise { - if (child.exitCode !== null || child.signalCode !== null) return; - child.kill("SIGTERM"); - const timeout = setTimeout(() => { - if (child.exitCode === null && child.signalCode === null) { - child.kill("SIGKILL"); - } - }, 5000); - try { - await once(child, "exit"); - } finally { - clearTimeout(timeout); - } -} - async function startRestartDaemon(input: { paseoHome: string; origin: string; @@ -251,8 +238,7 @@ async function startRestartDaemon(input: { } const serverDir = path.resolve(__dirname, "../../../server"); - const tsxBin = execSync("which tsx").toString().trim(); - const child = spawn(tsxBin, ["scripts/supervisor-entrypoint.ts", "--dev"], { + const child = spawnTsx("scripts/supervisor-entrypoint.ts", ["--dev"], { cwd: serverDir, env: withDisabledE2ESpeechEnv({ ...process.env, @@ -276,7 +262,7 @@ async function startRestartDaemon(input: { try { await waitForServer(port, child); } catch (error) { - await stopProcess(child); + await killProcessTree(child); throw new Error( `${error instanceof Error ? error.message : String(error)}\nDaemon stderr:\n${stderr}`, { cause: error }, @@ -285,7 +271,7 @@ async function startRestartDaemon(input: { return { port, - close: () => stopProcess(child), + close: () => killProcessTree(child), }; } diff --git a/packages/app/e2e/support/global-setup.ts b/packages/app/e2e/support/global-setup.ts index 11f16ccc93..a7c087c635 100644 --- a/packages/app/e2e/support/global-setup.ts +++ b/packages/app/e2e/support/global-setup.ts @@ -1,5 +1,5 @@ import { spawn, type ChildProcess } from "node:child_process"; -import { once } from "node:events"; +import { killProcessTree } from "./helpers/spawn-node"; import { existsSync } from "node:fs"; import path from "node:path"; import net from "node:net"; @@ -148,28 +148,11 @@ export async function warmMetro(port: number): Promise { } } -async function stopProcess(child: ChildProcess | null): Promise { - if (!child || child.exitCode !== null || child.signalCode !== null) return; - child.kill("SIGTERM"); - let timeout: ReturnType | undefined; - try { - await Promise.race([ - once(child, "exit"), - new Promise((resolve) => { - timeout = setTimeout(() => { - if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL"); - resolve(); - }, 5_000); - }), - ]); - } finally { - if (timeout) clearTimeout(timeout); - } -} - function startMetro(port: number, buffer: ReturnType): ChildProcess { const appDir = path.resolve(__dirname, "../.."); - const child = spawn("npx", ["expo", "start", "--web", "--port", String(port)], { + const expoCli = require.resolve("expo/bin/cli"); + // Spawns Node directly to bypass Windows .cmd shim execution restrictions without shell: true. + const child = spawn(process.execPath, [expoCli, "start", "--web", "--port", String(port)], { cwd: appDir, env: { ...process.env, @@ -216,11 +199,11 @@ export default async function globalSetup() { console.log(`[e2e] Metro warmed on port ${metroPort}`); return async () => { - await stopProcess(metroProcess); + await killProcessTree(metroProcess); console.log("[e2e] Metro stopped"); }; } catch (error) { - await stopProcess(metroProcess); + await killProcessTree(metroProcess); throw error; } } diff --git a/packages/app/e2e/support/helpers/daemon-update.ts b/packages/app/e2e/support/helpers/daemon-update.ts index 7b52a9d205..6914ed5fe7 100644 --- a/packages/app/e2e/support/helpers/daemon-update.ts +++ b/packages/app/e2e/support/helpers/daemon-update.ts @@ -1,6 +1,6 @@ import { fork, type ChildProcess } from "node:child_process"; -import { once } from "node:events"; import path from "node:path"; +import { killProcessTree } from "./spawn-node"; export interface OutdatedDaemon { endpoint: string; @@ -55,14 +55,10 @@ export async function startOutdatedDaemon(options?: { endpoint: ready.endpoint, label: options?.desktopManaged === true ? "outdated Desktop host" : "outdated host", serverId: ready.serverId, - async close() { - if (child.exitCode !== null || child.signalCode !== null) return; - child.kill("SIGTERM"); - await once(child, "exit"); - }, + close: () => killProcessTree(child), }; } catch (error) { - child.kill("SIGTERM"); + await killProcessTree(child); throw error; } } diff --git a/packages/app/e2e/support/helpers/e2e-worker.ts b/packages/app/e2e/support/helpers/e2e-worker.ts index 13c496d660..2d61f50af5 100644 --- a/packages/app/e2e/support/helpers/e2e-worker.ts +++ b/packages/app/e2e/support/helpers/e2e-worker.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -19,11 +19,19 @@ async function createFakeEditorBin(): Promise { const binDir = await mkdtemp(path.join(tmpdir(), "paseo-e2e-editor-bin-")); let realGhPath = ""; try { - realGhPath = execSync("which gh").toString().trim(); + const locator = process.platform === "win32" ? "where.exe" : "which"; + const candidates = execFileSync(locator, ["gh"], { encoding: "utf8" }) + .split(/\r?\n/u) + .map((candidate) => candidate.trim()) + .filter(Boolean); + realGhPath = + candidates.find( + (candidate) => + process.platform !== "win32" || !/\.(?:cmd|bat)$/iu.test(path.extname(candidate)), + ) ?? ""; } catch { // The local GitHub fixture remains usable without a system gh binary. } - const fakeEditorSource = `#!/usr/bin/env node const fs = require("fs"); const path = require("path"); @@ -41,6 +49,9 @@ if (recordPath) { const editorPath = path.join(binDir, editorCommand); await writeFile(editorPath, fakeEditorSource); await chmod(editorPath, 0o755); + if (process.platform === "win32") { + await writeFile(`${editorPath}.cmd`, `@node "%~dp0${editorCommand}" %*\r\n`); + } } const fakeGhPath = path.join(binDir, "gh"); @@ -106,6 +117,9 @@ process.exit(result.status ?? 1); `; await writeFile(fakeGhPath, fakeGhSource); await chmod(fakeGhPath, 0o755); + if (process.platform === "win32") { + await writeFile(`${fakeGhPath}.cmd`, '@node "%~dp0gh" %*\r\n'); + } return binDir; } diff --git a/packages/app/e2e/support/helpers/isolated-host-daemon.ts b/packages/app/e2e/support/helpers/isolated-host-daemon.ts index 062b444d9f..da22a9ce16 100644 --- a/packages/app/e2e/support/helpers/isolated-host-daemon.ts +++ b/packages/app/e2e/support/helpers/isolated-host-daemon.ts @@ -1,16 +1,10 @@ -import { once } from "node:events"; -import { - spawn, - execFileSync, - execSync, - type ChildProcess, - type SpawnOptions, -} from "node:child_process"; +import { spawn, execFileSync, type ChildProcess, type SpawnOptions } from "node:child_process"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import net from "node:net"; import { tmpdir } from "node:os"; import path from "node:path"; import { withDisabledE2ESpeechEnv } from "./speech-env"; +import { killProcessTree, spawnTsx } from "./spawn-node"; export interface IsolatedHostDaemon { serverId: string; @@ -48,12 +42,14 @@ async function getAvailablePort(): Promise { } async function waitForServer(port: number, child: ChildProcess): Promise { - const deadline = Date.now() + 20_000; + const deadline = Date.now() + 90_000; let lastError: unknown = null; while (Date.now() < deadline) { - if (child.exitCode !== null) { - throw new Error(`Isolated host daemon exited before listening (exit ${child.exitCode})`); + if (child.exitCode !== null || child.signalCode !== null) { + throw new Error( + `Isolated host daemon exited before listening (code ${String(child.exitCode)}, signal ${String(child.signalCode)})`, + ); } try { await new Promise((resolve, reject) => { @@ -81,19 +77,6 @@ async function waitForServer(port: number, child: ChildProcess): Promise { ); } -async function stopProcess(child: ChildProcess): Promise { - if (child.exitCode !== null || child.signalCode !== null) return; - child.kill("SIGTERM"); - const timeout = setTimeout(() => { - if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL"); - }, 5_000); - try { - await once(child, "exit"); - } finally { - clearTimeout(timeout); - } -} - export async function startIsolatedHostDaemon( serverId: string, options: IsolatedHostDaemonOptions = {}, @@ -114,11 +97,17 @@ export async function startIsolatedHostDaemon( path.join(publishedPackageRoot, "package.json"), `${JSON.stringify({ private: true })}\n`, ); - const npmCommand = process.platform === "win32" ? "npm.cmd" : "npm"; try { + const npmCli = process.env.npm_execpath; + if (!npmCli || path.basename(npmCli).toLowerCase() !== "npm-cli.js") { + throw new Error( + "Published-version E2E requires npm_execpath from npm. Start it through `npm run test:e2e`.", + ); + } execFileSync( - npmCommand, + process.execPath, [ + npmCli, "install", "--no-audit", "--no-fund", @@ -128,6 +117,9 @@ export async function startIsolatedHostDaemon( { cwd: publishedPackageRoot, stdio: "ignore" }, ); } catch (error) { + if (!options.preserveHome) { + await rm(paseoHome, { recursive: true, force: true }); + } await rm(publishedPackageRoot, { recursive: true, force: true }); throw error; } @@ -155,7 +147,6 @@ export async function startIsolatedHostDaemon( const serverDir = publishedPackageRoot ? path.join(publishedPackageRoot, "node_modules", "@getpaseo", "server") : path.resolve(__dirname, "../../../../server"); - const tsxBin = execSync("which tsx").toString().trim(); const spawnDaemon = async (): Promise => { const spawnOptions: SpawnOptions = { cwd: serverDir, @@ -175,7 +166,7 @@ export async function startIsolatedHostDaemon( }; const child = publishedPackageRoot ? spawn(process.execPath, ["dist/scripts/supervisor-entrypoint.js"], spawnOptions) - : spawn(tsxBin, ["scripts/supervisor-entrypoint.ts", "--dev"], spawnOptions); + : spawnTsx("scripts/supervisor-entrypoint.ts", ["--dev"], spawnOptions); let stderr = ""; child.stderr?.on("data", (chunk: Buffer) => { @@ -187,7 +178,7 @@ export async function startIsolatedHostDaemon( await waitForServer(port, child); return child; } catch (error) { - await stopProcess(child); + await killProcessTree(child); throw new Error( `${error instanceof Error ? error.message : String(error)}\nDaemon stderr:\n${stderr}`, { cause: error }, @@ -216,13 +207,13 @@ export async function startIsolatedHostDaemon( getPid: () => child.pid, restart: async () => { if (closed) throw new Error(`Cannot restart closed isolated daemon ${serverId}`); - await stopProcess(child); + await killProcessTree(child); child = await spawnDaemon(); }, close: async () => { if (closed) return; closed = true; - await stopProcess(child); + await killProcessTree(child); if (!options.preserveHome) { await rm(paseoHome, { recursive: true, force: true }); } diff --git a/packages/app/e2e/support/helpers/local-elixir-relay.ts b/packages/app/e2e/support/helpers/local-elixir-relay.ts index 14d9a8b220..0d91b0fb0c 100644 --- a/packages/app/e2e/support/helpers/local-elixir-relay.ts +++ b/packages/app/e2e/support/helpers/local-elixir-relay.ts @@ -1,8 +1,8 @@ import { spawn, type ChildProcess } from "node:child_process"; -import { once } from "node:events"; import { existsSync } from "node:fs"; import net from "node:net"; import path from "node:path"; +import { killProcessTree } from "./spawn-node"; export interface LocalElixirRelay { endpoint: string; @@ -27,19 +27,6 @@ async function availablePort(): Promise { }); } -async function stopProcess(child: ChildProcess): Promise { - if (child.exitCode !== null || child.signalCode !== null) return; - child.kill("SIGTERM"); - const force = setTimeout(() => { - if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL"); - }, 5_000); - try { - await once(child, "exit"); - } finally { - clearTimeout(force); - } -} - async function waitUntilReady( port: number, child: ChildProcess, @@ -48,9 +35,9 @@ async function waitUntilReady( const deadline = Date.now() + 60_000; let lastError: unknown; while (Date.now() < deadline) { - if (child.exitCode !== null) { + if (child.exitCode !== null || child.signalCode !== null) { throw new Error( - `Elixir relay exited before becoming ready (exit ${child.exitCode}).\n${recentOutput()}`, + `Elixir relay exited before becoming ready (code ${String(child.exitCode)}, signal ${String(child.signalCode)}).\n${recentOutput()}`, ); } try { @@ -70,6 +57,12 @@ async function waitUntilReady( } export async function startLocalElixirRelay(): Promise { + if (process.platform === "win32") { + throw new Error( + "The local Elixir relay requires asdf, which is not available on Windows; the relay-deployment Playwright project is POSIX-only.", + ); + } + const relayRoot = process.env.PASEO_RELAY_CHECKOUT ?? path.resolve(__dirname, "../../../../../..", "paseo-relay"); if (!existsSync(path.join(relayRoot, "mix.exs"))) { @@ -107,14 +100,14 @@ export async function startLocalElixirRelay(): Promise { try { await waitUntilReady(port, child, () => output.join("\n")); } catch (error) { - await stopProcess(child); + await killProcessTree(child); throw error; } }; const stop = async () => { if (!child) return; - await stopProcess(child); + await killProcessTree(child); child = null; }; diff --git a/packages/app/e2e/support/helpers/spawn-node.ts b/packages/app/e2e/support/helpers/spawn-node.ts new file mode 100644 index 0000000000..4a72aa99ab --- /dev/null +++ b/packages/app/e2e/support/helpers/spawn-node.ts @@ -0,0 +1,84 @@ +import { execFile, spawn, type ChildProcess, type SpawnOptions } from "node:child_process"; +import { once } from "node:events"; + +/** + * Runs a repo-local TypeScript entrypoint through Node's own tsx loader instead + * of the `node_modules/.bin/tsx` shim. + * + * The shim is a platform trap, not a convenience: on Windows it is `tsx.cmd`, + * and Node refuses to spawn `.cmd`/`.bat` without `shell: true` (EINVAL, from + * the CVE-2024-27980 mitigation). Opting into `shell: true` to get around that + * trades one bug for three — argv is concatenated unescaped (DEP0190), a path + * containing spaces splits, and the kill signal lands on `cmd.exe` while the + * real child keeps the port. Spawning `process.execPath` has none of those + * properties and is byte-identical on POSIX, so there is no platform branch. + */ +export function spawnTsx(entrypoint: string, args: string[], options: SpawnOptions): ChildProcess { + if (entrypoint.startsWith("-")) { + throw new Error(`TypeScript entrypoint must be a path, received ${entrypoint}`); + } + return spawn(process.execPath, ["--import", "tsx", "--", entrypoint, ...args], options); +} + +/** + * Terminates a child and everything it forked, then waits for the exit. + * + * `child.kill()` on Windows maps to `TerminateProcess` against the direct child + * only. Metro and the daemon supervisor both fork workers, so the signal leaves + * grandchildren holding the listening port and the next run fails to bind. + * `taskkill /T` walks the tree; POSIX keeps the SIGTERM-then-SIGKILL ladder. + */ +export async function killProcessTree(child: ChildProcess | null): Promise { + if (!child || hasExited(child)) return; + const exited = once(child, "exit").then(() => undefined); + + if (process.platform === "win32") { + const pid = child.pid; + if (pid === undefined) { + throw new Error("Cannot terminate a Windows process tree without a PID"); + } + + // Register the exit listener before taskkill to avoid missing a fast exit. + // Bound taskkill itself so a stuck system utility cannot hang teardown. + const completed = Promise.withResolvers(); + execFile("taskkill", ["/pid", String(pid), "/T", "/F"], { timeout: 5_000 }, (error) => + completed.resolve(error), + ); + const taskkillError = await completed.promise; + if (taskkillError && !hasExited(child)) { + try { + child.kill("SIGKILL"); + } catch { + // The child can exit between the state check and the direct kill. + } + await waitForExitOrTimeout(exited, 5_000); + throw new Error(`Failed to terminate process tree for PID ${pid}`, { cause: taskkillError }); + } + if (!hasExited(child) && !(await waitForExitOrTimeout(exited, 5_000))) { + throw new Error(`Process tree for PID ${pid} did not exit after taskkill`); + } + return; + } + + child.kill("SIGTERM"); + if (await waitForExitOrTimeout(exited, 5_000)) return; + + child.kill("SIGKILL"); + if (!(await waitForExitOrTimeout(exited, 5_000))) { + throw new Error(`Process ${String(child.pid)} did not exit after SIGKILL`); + } +} + +function hasExited(child: ChildProcess): boolean { + return child.exitCode !== null || child.signalCode !== null; +} + +async function waitForExitOrTimeout(exited: Promise, timeoutMs: number): Promise { + const timedOut = Promise.withResolvers(); + const timeout = setTimeout(() => timedOut.resolve(false), timeoutMs); + try { + return await Promise.race([exited.then(() => true), timedOut.promise]); + } finally { + clearTimeout(timeout); + } +} diff --git a/scripts/daemon-launch-contract.test.mjs b/scripts/daemon-launch-contract.test.mjs index ff92cda12e..8843fccf06 100644 --- a/scripts/daemon-launch-contract.test.mjs +++ b/scripts/daemon-launch-contract.test.mjs @@ -62,7 +62,7 @@ test("every executable daemon entrypoint enters the supervisor", async () => { assert.match( appIsolatedHostDaemon, - /spawn\(tsxBin, \["scripts\/supervisor-entrypoint\.ts", "--dev"\]/, + /spawnTsx\("scripts\/supervisor-entrypoint\.ts", \["--dev"\]/, ); assertNoSpawnedWorkerEntrypoint("app e2e isolated host daemon", appIsolatedHostDaemon);