diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 98a867d4591af62d08d847cb60910f161766eeb3..63e89541e1cd0257dbe36299c9e4ebf6c248a61a 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -1,5 +1,4 @@ -// @effect-diagnostics nodeBuiltinImport:off - the Windows reveal smoke test drives a real PowerShell through Node process and filesystem APIs. -import * as NodeChildProcess from "node:child_process"; +// @effect-diagnostics nodeBuiltinImport:off - the Windows launch smoke tests use filesystem notifications to observe real PowerShell launches. import * as NodeFS from "node:fs"; import * as NodeOS from "node:os"; import * as NodePath from "node:path"; @@ -124,6 +123,50 @@ it.effect("launches the default browser through the platform command", () => { ); }); +it.effect.each(["win32", "linux"] as const)( + "launches the browser through PowerShell on %s", + (platform) => { + let spawned: ChildProcess.StandardCommand | undefined; + return Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + + yield* launcher.launchBrowser("https://example.com/some path"); + + assert.ok(spawned); + assert.equal( + spawned.command, + platform === "win32" + ? "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" + : "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe", + ); + assert.deepEqual(spawned.args.slice(0, -1), [ + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", + "Bypass", + "-EncodedCommand", + ]); + const encodedCommand = spawned.args[spawned.args.length - 1] ?? ""; + const decodedCommand = Buffer.from(encodedCommand, "base64").toString("utf16le"); + assert.equal( + decodedCommand, + "$ProgressPreference = 'SilentlyContinue'; Start 'https://example.com/some path'", + ); + assert.equal(spawned.options.detached, platform !== "win32"); + }).pipe( + Effect.provide( + layerTest({ + platform, + env: { SYSTEMROOT: "C:\\Windows", WSL_DISTRO_NAME: "Ubuntu" }, + onSpawn: (command) => { + spawned = command; + }, + }), + ), + ); + }, +); + it.effect("launches an installed editor with platform-safe arguments", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; @@ -163,6 +206,7 @@ it.effect("launches an installed editor with platform-safe arguments", () => ); assert.deepEqual(spawned.args, []); assert.equal(spawned.options.shell, true); + assert.equal(spawned.options.detached, true); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); @@ -291,6 +335,7 @@ it.effect.skipIf(windowsHost)("reveals a file in Finder with open -R on macOS", assert.ok(spawned); assert.equal(spawned.command, "open"); assert.deepEqual(spawned.args, ["-R", "/workspace/media/linux-mini-v2.mp4"]); + assert.equal(spawned.options.detached, true); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); @@ -349,65 +394,95 @@ it.effect("reveals a file in File Explorer through PowerShell on Windows", () => "$ProgressPreference = 'SilentlyContinue'; Start-Process 'explorer.exe' -ArgumentList ('/select,\"' + 'C:\\workspace with spaces\\media\\author''s clip.mp4' + '\"')", ); assert.equal(spawned.options.shell, false); + assert.equal(spawned.options.detached, false); }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); -// Real-chain smoke check for the Explorer selection contract: runs the exact -// PowerShell source the reveal launch encodes, against a stub that records -// the raw argument tail it receives, and asserts a spaced path arrives as the -// single `/select,""` switch. Mock argv assertions cannot prove this — -// only Windows' own PowerShell -> CreateProcess quoting chain can, so the -// test runs only where that chain exists. -// oxlint-disable-next-line t3code/no-global-process-runtime -- the skip decision needs the real host platform, outside any Effect runtime. -it.skipIf(process.platform !== "win32")( - "delivers the raw /select switch for spaced paths through real PowerShell", - { timeout: 60_000 }, - async () => { - const tempDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-reveal-smoke-")); - try { - const recorderPath = NodePath.join(tempDir, "recorder.cmd"); - const outputPath = NodePath.join(tempDir, "argv.txt"); - NodeFS.writeFileSync(recorderPath, `@echo off\r\n>"${outputPath}" echo(%*\r\n`); +// Exercise real spawning, ignored stdio, unref, and scope cleanup. Only the +// destination application is replaced, so tests never open Explorer or a browser. +for (const kind of ["browser", "reveal"] as const) { + it.live.skipIf(!windowsHost)( + `completes a ${kind} launch through real PowerShell after the launcher scope closes`, + () => + Effect.gen(function* () { + const tempDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-reveal-smoke-")); + const outputPath = NodePath.join(tempDir, "argv.txt"); + const watcher = NodeFS.watch(tempDir, { signal: AbortSignal.timeout(20_000) }); + try { + const recorderPath = NodePath.join(tempDir, "recorder.cmd"); + const stagingPath = NodePath.join(tempDir, "argv.tmp"); + NodeFS.writeFileSync( + recorderPath, + `@echo off\r\n>"${stagingPath}" echo(%*\r\nmove /y "${stagingPath}" "${outputPath}" >nul\r\n`, + ); + // The rename publishes a complete result; no sleeps or file polling. + const recorded = new Promise((resolve, reject) => { + watcher.on("change", () => { + if (NodeFS.existsSync(outputPath)) { + resolve(NodeFS.readFileSync(outputPath, "utf8").trim()); + } + }); + watcher.on("error", reject); + watcher.on("close", () => + reject(new Error("PowerShell did not run the recording stub")), + ); + }); - const target = "C:/workspace with spaces/media/author's clip.mp4"; - const explorerTarget = target.replaceAll("/", "\\"); - const source = ExternalLauncher.buildFileExplorerRevealPowerShellSource( - recorderPath, - explorerTarget, - ); - const powerShellPath = `${process.env.SYSTEMROOT ?? "C:\\Windows"}\\System32\\WindowsPowerShell\\v1.0\\powershell.exe`; - NodeChildProcess.execFileSync( - powerShellPath, - [ - "-NoProfile", - "-NonInteractive", - "-ExecutionPolicy", - "Bypass", - "-EncodedCommand", - Buffer.from(source, "utf16le").toString("base64"), - ], - { timeout: 30_000 }, - ); - - // Start-Process returns before the recorder runs; wait for its output. - // The waits run outside the Effect runtime on purpose: the test - // exercises the real Windows process chain in real time. - // @effect-diagnostics-next-line globalTimers:off - const sleep = (millis: number) => new Promise((resolve) => setTimeout(resolve, millis)); - // @effect-diagnostics-next-line globalDate:off - const deadline = Date.now() + 20_000; - // @effect-diagnostics-next-line globalDate:off - while (!NodeFS.existsSync(outputPath) && Date.now() < deadline) { - await sleep(100); - } - await sleep(200); - const recorded = NodeFS.readFileSync(outputPath, "utf8").trim(); - assert.equal(recorded, `/select,"${explorerTarget}"`); - } finally { - NodeFS.rmSync(tempDir, { recursive: true, force: true }); - } - }, -); + const target = "C:/workspace with spaces/media/author's clip.mp4"; + yield* Effect.all( + [ + Effect.gen(function* () { + const realSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const recordingSpawner = ChildProcessSpawner.make((command) => { + assert.ok(ChildProcess.isStandardCommand(command)); + const source = Buffer.from(command.args.at(-1) ?? "", "base64") + .toString("utf16le") + .replace("'explorer.exe'", `'${recorderPath.replaceAll("'", "''")}'`); + return realSpawner.spawn( + ChildProcess.make( + command.command, + [ + ...command.args.slice(0, -1), + Buffer.from(source, "utf16le").toString("base64"), + ], + command.options, + ), + ); + }); + const launcher = yield* ExternalLauncher.make.pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, recordingSpawner), + ); + if (kind === "browser") { + yield* launcher.launchBrowser(recorderPath); + } else { + yield* launcher.launchEditor({ + editor: "file-manager", + cwd: target, + reveal: true, + }); + } + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), + Effect.promise(() => + recorded.then((value) => { + assert.equal( + value, + kind === "browser" ? "" : `/select,"${target.replaceAll("/", "\\")}"`, + ); + }), + ), + ], + { concurrency: "unbounded" }, + ); + } finally { + watcher.close(); + // The recording stub's cmd.exe can still hold the directory for a + // moment after it publishes its output, which Windows reports as EPERM. + NodeFS.rmSync(tempDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 }); + } + }), + { timeout: 30_000 }, + ); +} it.effect("does not advertise reveal on Windows when PowerShell is missing", () => Effect.gen(function* () { @@ -488,6 +563,7 @@ it.effect.skipIf(windowsHost)( // The reveal routes through interop PowerShell so Explorer receives its // raw `/select,""` switch even for spaced paths. assert.equal(spawned.command, "powershell.exe"); + assert.equal(spawned.options.detached, true); const encodedCommand = spawned.args[spawned.args.length - 1] ?? ""; const decodedCommand = Buffer.from(encodedCommand, "base64").toString("utf16le"); assert.equal( diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index 8ab130b01e38ed8292ce70b939ff1cb6bdf4cedd..3f2dab8f4499423a13c6ffcb742c88bcd500835f 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -62,6 +62,8 @@ interface EditorLaunch { readonly target: string; readonly command: string; readonly args: ReadonlyArray; + /** Defaults to true. See WINDOWS_POWERSHELL_DETACHED. */ + readonly detached?: boolean; } interface ProcessLaunch { @@ -93,6 +95,12 @@ const POWERSHELL_ARGUMENTS_PREFIX = [ "-EncodedCommand", ] as const; +// Windows PowerShell started with DETACHED_PROCESS exits 0 without running its +// command, so native Windows launches through it stay attached. The child is +// still unref'd with ignored stdio, so nothing waits on it. WSL interop +// PowerShell starts from a Linux process and keeps the detached launch. +const WINDOWS_POWERSHELL_DETACHED = false; + const DETACHED_IGNORE_STDIO_OPTIONS = { detached: true, stdin: "ignore", @@ -219,7 +227,11 @@ function shouldUseWindowsHostFromWsl( ); } -function resolveWindowsBrowserLaunch(target: string, command: string): ProcessLaunch { +function resolveWindowsBrowserLaunch( + target: string, + command: string, + detached: boolean, +): ProcessLaunch { const encodedCommand = encodeUtf16LeBase64( `$ProgressPreference = 'SilentlyContinue'; Start ${escapePowerShellStringLiteral(target)}`, ); @@ -227,7 +239,7 @@ function resolveWindowsBrowserLaunch(target: string, command: string): ProcessLa command, args: [...POWERSHELL_ARGUMENTS_PREFIX, encodedCommand], options: { - detached: true, + detached, shell: false, stdin: "ignore", stdout: "ignore", @@ -407,11 +419,15 @@ function buildBrowserLaunch( } if (platform === "win32") { - return resolveWindowsBrowserLaunch(target, resolvePowerShellPath(env)); + return resolveWindowsBrowserLaunch( + target, + resolvePowerShellPath(env), + WINDOWS_POWERSHELL_DETACHED, + ); } if (shouldUseWindowsHostFromWsl(platform, env)) { - return resolveWindowsBrowserLaunch(target, resolveWslPowerShellPath()); + return resolveWindowsBrowserLaunch(target, resolveWslPowerShellPath(), true); } return { @@ -588,14 +604,8 @@ const resolveEditorLaunch = Effect.fn("resolveEditorLaunch")(function* ( * when the path has spaces and Explorer misparses it, silently opening a * fallback folder. A single `-ArgumentList` string in Windows PowerShell 5.1 * reaches the child's command line verbatim, preserving the raw switch. - * - * Exported so the Windows smoke test can drive the identical source through a - * real PowerShell against a recording stub instead of Explorer. */ -export function buildFileExplorerRevealPowerShellSource( - explorerCommand: string, - target: string, -): string { +function buildFileExplorerRevealPowerShellSource(explorerCommand: string, target: string): string { return `$ProgressPreference = 'SilentlyContinue'; Start-Process ${escapePowerShellStringLiteral(explorerCommand)} -ArgumentList ('/select,"' + ${escapePowerShellStringLiteral(target)} + '"')`; } @@ -636,11 +646,14 @@ const resolveFileManagerRevealLaunch = Effect.fn("resolveFileManagerRevealLaunch } if (platform === "win32") { - return fileExplorerRevealLaunch( - target, - normalizeWindowsFileManagerPath(target), - resolvePowerShellPath(env), - ); + return { + ...fileExplorerRevealLaunch( + target, + normalizeWindowsFileManagerPath(target), + resolvePowerShellPath(env), + ), + detached: WINDOWS_POWERSHELL_DETACHED, + }; } if ( @@ -744,7 +757,7 @@ const launchEditorProcess = Effect.fn("externalLauncher.launchEditorProcess")(fu command: spawnCommand.command, args: spawnCommand.args, options: { - detached: true, + detached: launch.detached ?? true, shell: spawnCommand.shell, stdin: "ignore", stdout: "ignore",