diff --git a/.changeset/windows-hide-child-process-console.md b/.changeset/windows-hide-child-process-console.md new file mode 100644 index 00000000000..77d53516a6e --- /dev/null +++ b/.changeset/windows-hide-child-process-console.md @@ -0,0 +1,7 @@ +--- +"@effect/platform-node-shared": patch +--- + +Pass Node's `windowsHide` flag for spawned Windows children by default (except detached processes), with an independent +`windowsHide` option for callers that need visible GUI windows. Process-group cleanup now invokes `taskkill` without a +`cmd.exe` wrapper and hides its window. diff --git a/packages/effect/src/unstable/process/ChildProcess.ts b/packages/effect/src/unstable/process/ChildProcess.ts index dca9a961cd9..1c019b540f9 100644 --- a/packages/effect/src/unstable/process/ChildProcess.ts +++ b/packages/effect/src/unstable/process/ChildProcess.ts @@ -432,6 +432,14 @@ export interface CommandOptions extends KillOptions { * Defaults to `true` on non-Windows platforms and `false` on Windows platforms. */ readonly detached?: boolean | undefined + /** + * If set to `true`, prevents the child process's console or GUI window from + * becoming visible on Windows. + * + * Defaults to `true` unless `detached` is set to `true`. This option has no + * effect on non-Windows platforms. + */ + readonly windowsHide?: boolean | undefined /** * Configuration options for the standard input stream for the child process. */ diff --git a/packages/platform-node-shared/src/NodeChildProcessSpawner.ts b/packages/platform-node-shared/src/NodeChildProcessSpawner.ts index 007b181b708..38299e217c5 100644 --- a/packages/platform-node-shared/src/NodeChildProcessSpawner.ts +++ b/packages/platform-node-shared/src/NodeChildProcessSpawner.ts @@ -40,6 +40,7 @@ import { } from "effect/unstable/process/ChildProcessSpawner" import * as NodeChildProcess from "node:child_process" import { PassThrough } from "node:stream" +import { buildSpawnOptions } from "./internal/nodeChildProcessSpawner.ts" import { handleErrnoException } from "./internal/utils.ts" import * as NodeSink from "./NodeSink.ts" import * as NodeStream from "./NodeStream.ts" @@ -65,6 +66,17 @@ const toPlatformError = ( type ExitCodeWithSignal = readonly [code: number | null, signal: NodeJS.Signals | null] type ExitSignal = Deferred.Deferred +const taskkill = ( + childProcess: NodeChildProcess.ChildProcess, + onExit: (error: NodeChildProcess.ExecException | null) => void = () => {} +) => + NodeChildProcess.execFile( + "taskkill", + ["/pid", String(childProcess.pid!), "/T", "/F"], + { windowsHide: true }, + onExit + ) + const make = Effect.gen(function*() { const fs = yield* FileSystem.FileSystem const path = yield* Path.Path @@ -354,7 +366,7 @@ const make = Effect.gen(function*() { ) => { if (globalThis.process.platform === "win32") { return Effect.callback((resume) => { - NodeChildProcess.exec(`taskkill /pid ${childProcess.pid} /T /F`, (error) => { + taskkill(childProcess, (error) => { if (error) { resume(Effect.fail(toPlatformError("kill", toError(error), command))) } else { @@ -376,9 +388,8 @@ const make = Effect.gen(function*() { signal: NodeJS.Signals ): void => { if (globalThis.process.platform === "win32") { - NodeChildProcess.exec(`taskkill /pid ${childProcess.pid} /T /F`, () => { - // ignore errors during best-effort cleanup - }) + // ignore errors during best-effort cleanup + taskkill(childProcess) return } try { @@ -471,13 +482,7 @@ const make = Effect.gen(function*() { const stdio = buildStdioArray(stdinConfig, stdoutConfig, stderrConfig, resolvedAdditionalFds) const [childProcess, exitSignal] = yield* Effect.acquireRelease( - spawn(cmd, { - cwd, - env, - stdio, - detached: cmd.options.detached ?? process.platform !== "win32", - shell: cmd.options.shell - }), + spawn(cmd, buildSpawnOptions(cmd.options, { cwd, env, stdio }, process.platform)), Effect.fnUntraced(function*([childProcess, exitSignal]) { const exited = yield* Deferred.isDone(exitSignal) const killWithTimeout = withTimeout(childProcess, cmd, cmd.options) diff --git a/packages/platform-node-shared/src/internal/nodeChildProcessSpawner.ts b/packages/platform-node-shared/src/internal/nodeChildProcessSpawner.ts new file mode 100644 index 00000000000..7cd1e9e95d6 --- /dev/null +++ b/packages/platform-node-shared/src/internal/nodeChildProcessSpawner.ts @@ -0,0 +1,16 @@ +import type * as ChildProcess from "effect/unstable/process/ChildProcess" +import type * as NodeChildProcess from "node:child_process" + +export const buildSpawnOptions = ( + options: ChildProcess.CommandOptions, + base: Pick, + platform: NodeJS.Platform +): NodeChildProcess.SpawnOptions => { + const detached = options.detached ?? platform !== "win32" + return { + ...base, + detached, + shell: options.shell, + windowsHide: options.windowsHide ?? !detached + } +} diff --git a/packages/platform-node-shared/test/NodeChildProcessSpawner.test.ts b/packages/platform-node-shared/test/NodeChildProcessSpawner.test.ts index 8188328c43a..7c646284d6c 100644 --- a/packages/platform-node-shared/test/NodeChildProcessSpawner.test.ts +++ b/packages/platform-node-shared/test/NodeChildProcessSpawner.test.ts @@ -1,7 +1,8 @@ +import { buildSpawnOptions } from "@effect/platform-node-shared/internal/nodeChildProcessSpawner" import * as NodeChildProcessSpawner from "@effect/platform-node-shared/NodeChildProcessSpawner" import * as NodeFileSystem from "@effect/platform-node-shared/NodeFileSystem" import * as NodePath from "@effect/platform-node-shared/NodePath" -import { assert, it } from "@effect/vitest" +import { assert, describe, it } from "@effect/vitest" import * as ChildProcessSpawnerTest from "effect-test/unstable/process/ChildProcessSpawnerTest" import * as Effect from "effect/Effect" import * as FileSystem from "effect/FileSystem" @@ -19,6 +20,52 @@ ChildProcessSpawnerTest.suite("NodeChildProcessSpawner", NodeServices, { processGroups: true }) +describe("buildSpawnOptions", () => { + const base = { stdio: "pipe" } as const + + it("defaults to hiding non-detached Windows children", () => { + assert.deepStrictEqual(buildSpawnOptions({}, base, "win32"), { + stdio: "pipe", + detached: false, + shell: undefined, + windowsHide: true + }) + assert.deepStrictEqual(buildSpawnOptions({ detached: true }, base, "win32"), { + stdio: "pipe", + detached: true, + shell: undefined, + windowsHide: false + }) + assert.deepStrictEqual(buildSpawnOptions({ detached: false }, base, "win32"), { + stdio: "pipe", + detached: false, + shell: undefined, + windowsHide: true + }) + }) + + it("allows windowsHide to be configured independently of detached", () => { + assert.deepStrictEqual( + buildSpawnOptions({ detached: false, windowsHide: false }, base, "win32"), + { + stdio: "pipe", + detached: false, + shell: undefined, + windowsHide: false + } + ) + assert.deepStrictEqual( + buildSpawnOptions({ detached: true, windowsHide: true }, base, "win32"), + { + stdio: "pipe", + detached: true, + shell: undefined, + windowsHide: true + } + ) + }) +}) + it.live("kills every process in a pipeline", () => Effect.gen(function*() { const fs = yield* FileSystem.FileSystem