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);