From 1f81ab9dd42af243ca39734b377c6795aedf71e7 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 3 Nov 2026 20:58:22 -0500 Subject: [PATCH 1/5] fix(acp): preserve Windows shim stdio pipes --- .../Adapters/AcpAdapterV2.test.ts | 48 +++++++++++++++++++ .../src/server/AcpSessionRuntime.ts | 3 +- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 7234c561b9163..7379d98cb18ce 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -52,6 +52,7 @@ import * as Stream from "effect/Stream"; import * as Scope from "effect/Scope"; import * as TestClock from "effect/testing/TestClock"; import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; +import * as ChildProcess from "effect/process/ChildProcess"; import * as EffectAcpErrors from "effect-acp/errors"; import type * as EffectAcpProtocol from "effect-acp/protocol"; import type * as EffectAcpSchema from "effect-acp/compat"; @@ -515,6 +516,53 @@ function makeMockRuntime(input: { }); } +it.live("does not detach an owned Windows ACP process and retains PID tree cleanup", () => + Effect.gen(function* () { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const path = yield* Path.Path; + const terminated: number[] = []; + let spawnedPid = 0; + const runtime = yield* AcpSessionRuntime.make({ + spawn: { + command: process.execPath, + args: [ + yield* path.fromFileUrl(new URL("../../../scripts/acp-mock-agent.ts", import.meta.url)), + ], + }, + cwd: process.cwd(), + clientInfo: { name: "test", version: "1" }, + ownDetachedProcessGroup: true, + processGroupPlatform: "win32", + windowsProcessTreeTerminator: (pid) => + Effect.sync(() => { + terminated.push(pid); + }), + }).pipe( + Effect.provideService(HostProcess.Platform, "win32"), + Effect.provideService( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => { + if (!ChildProcess.isStandardCommand(command)) + return Effect.die("Expected standard command"); + assert.isFalse(command.options.detached); + return spawner.spawn(command).pipe( + Effect.tap((child) => + Effect.sync(() => { + spawnedPid = Number(child.pid); + }), + ), + ); + }), + ), + ); + if (runtime.terminateProcessGroup === undefined) + return yield* Effect.die("Missing tree cleanup"); + yield* runtime.terminateProcessGroup; + assert.deepEqual(terminated, [spawnedPid]); + assert.isAbove(spawnedPid, 0); + }).pipe(Effect.provide(layerTest), Effect.scoped), +); + function rawProtocolMethod(event: EffectAcpProtocol.AcpProtocolLogEvent): string | undefined { if (event.stage !== "raw" || typeof event.payload !== "string") return undefined; for (const line of event.payload.split("\n")) { diff --git a/packages/provider-acp/src/server/AcpSessionRuntime.ts b/packages/provider-acp/src/server/AcpSessionRuntime.ts index 0ef446ef3453a..8f32444d095ed 100644 --- a/packages/provider-acp/src/server/AcpSessionRuntime.ts +++ b/packages/provider-acp/src/server/AcpSessionRuntime.ts @@ -1244,6 +1244,7 @@ export const make = Effect.fn("AcpSessionRuntime.make")(function* ( > { const random = yield* SecureRandom.SecureRandom; const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const hostPlatform = yield* HostProcess.Platform; const runtimeScope = yield* Scope.fork(yield* Scope.Scope); const eventQueue = yield* Queue.unbounded(); const modeStateRef = yield* Ref.make(undefined); @@ -1455,7 +1456,7 @@ export const make = Effect.fn("AcpSessionRuntime.make")(function* ( extendEnv: options.spawn.extendEnv ?? true, ...(options.ownDetachedProcessGroup === undefined ? {} - : { detached: options.ownDetachedProcessGroup }), + : { detached: options.ownDetachedProcessGroup && hostPlatform !== "win32" }), shell: containedSpawnCommand.shell, }), ) From d3b320b309ab07abd929a62e674f3510d39e990e Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Wed, 4 Nov 2026 20:59:47 -0500 Subject: [PATCH 2/5] test(acp): exercise Windows cleanup without intercepting taskkill --- .../Adapters/AcpAdapterV2.test.ts | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index c92c2b5a72335..1130129311f20 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -516,8 +516,9 @@ function makeMockRuntime(input: { }); } -it.live("does not detach an owned Windows ACP process and retains PID tree cleanup", () => - Effect.gen(function* () { +it.live("does not detach an owned Windows ACP process and retains PID tree cleanup", () => { + let finalizerPid = 0; + return Effect.gen(function* () { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; const path = yield* Path.Path; const terminated: number[] = []; @@ -544,6 +545,15 @@ it.live("does not detach an owned Windows ACP process and retains PID tree clean ChildProcessSpawner.make((command) => { if (!ChildProcess.isStandardCommand(command)) return Effect.die("Expected standard command"); + if (command.command === "taskkill") { + finalizerPid = Number(command.args[1]); + assert.equal(finalizerPid, spawnedPid); + // Exercise the Windows finalizer while terminating the mock child on any host. + return spawner.spawn( + ChildProcess.make(process.execPath, ["-e", `process.kill(${finalizerPid})`]), + ); + } + assert.equal(command.command, process.execPath); assert.isFalse(command.options.detached); return spawner.spawn(command).pipe( Effect.tap((child) => @@ -560,8 +570,12 @@ it.live("does not detach an owned Windows ACP process and retains PID tree clean yield* runtime.terminateProcessGroup; assert.deepEqual(terminated, [spawnedPid]); assert.isAbove(spawnedPid, 0); - }).pipe(Effect.provide(layerTest), Effect.scoped), -); + }).pipe( + Effect.provide(layerTest), + Effect.scoped, + Effect.tap(() => Effect.sync(() => assert.isAbove(finalizerPid, 0))), + ); +}); const isProviderAdapterProtocolError = Schema.is(ProviderAdapter.ProviderAdapterProtocolError); From b5e625d78554b49c38cb660e79f5209e4ca33e05 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Wed, 4 Nov 2026 21:11:12 -0500 Subject: [PATCH 3/5] test(acp): compare cleanup PID after scope disposal --- .../orchestration-v2/Adapters/AcpAdapterV2.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 1130129311f20..2e8630309ff04 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -518,11 +518,11 @@ function makeMockRuntime(input: { it.live("does not detach an owned Windows ACP process and retains PID tree cleanup", () => { let finalizerPid = 0; + let spawnedPid = 0; return Effect.gen(function* () { const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; const path = yield* Path.Path; const terminated: number[] = []; - let spawnedPid = 0; const runtime = yield* AcpSessionRuntime.make({ spawn: { command: process.execPath, @@ -547,10 +547,9 @@ it.live("does not detach an owned Windows ACP process and retains PID tree clean return Effect.die("Expected standard command"); if (command.command === "taskkill") { finalizerPid = Number(command.args[1]); - assert.equal(finalizerPid, spawnedPid); // Exercise the Windows finalizer while terminating the mock child on any host. return spawner.spawn( - ChildProcess.make(process.execPath, ["-e", `process.kill(${finalizerPid})`]), + ChildProcess.make(process.execPath, ["-e", `process.kill(${spawnedPid})`]), ); } assert.equal(command.command, process.execPath); @@ -573,7 +572,12 @@ it.live("does not detach an owned Windows ACP process and retains PID tree clean }).pipe( Effect.provide(layerTest), Effect.scoped, - Effect.tap(() => Effect.sync(() => assert.isAbove(finalizerPid, 0))), + Effect.tap(() => + Effect.sync(() => { + assert.isAbove(spawnedPid, 0); + assert.equal(finalizerPid, spawnedPid); + }), + ), ); }); From 23680ecd6843917958ab909e3bd4c1cb48029076 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 03:24:31 +0000 Subject: [PATCH 4/5] test(acp): tidy Windows shim regression and drop shadowed platform read AI-assisted; verified with the tests and tools listed above. --- .../src/orchestration-v2/Adapters/AcpAdapterV2.test.ts | 9 ++------- packages/provider-acp/src/server/AcpSessionRuntime.ts | 1 - 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 1aafb582bb0a2..9237fbdae69ae 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -568,17 +568,12 @@ it.live("does not detach an owned Windows ACP process and retains PID tree clean if (runtime.terminateProcessGroup === undefined) return yield* Effect.die("Missing tree cleanup"); yield* runtime.terminateProcessGroup; - assert.deepEqual(terminated, [spawnedPid]); assert.isAbove(spawnedPid, 0); + assert.deepEqual(terminated, [spawnedPid]); }).pipe( Effect.provide(layerTest), Effect.scoped, - Effect.tap(() => - Effect.sync(() => { - assert.isAbove(spawnedPid, 0); - assert.equal(finalizerPid, spawnedPid); - }), - ), + Effect.tap(() => Effect.sync(() => assert.equal(finalizerPid, spawnedPid))), ); }); diff --git a/packages/provider-acp/src/server/AcpSessionRuntime.ts b/packages/provider-acp/src/server/AcpSessionRuntime.ts index 1d1fa80518c19..0df9eb54ba8de 100644 --- a/packages/provider-acp/src/server/AcpSessionRuntime.ts +++ b/packages/provider-acp/src/server/AcpSessionRuntime.ts @@ -1619,7 +1619,6 @@ export const make = Effect.fn("AcpSessionRuntime.make")(function* ( Effect.uninterruptible(terminateOwnedProcessGroupImpl), ); if (options.ownDetachedProcessGroup === true) { - const hostPlatform = yield* HostProcess.Platform; const forceTerminateOwnedProcessGroup = hostPlatform === "win32" ? terminateWindowsProcessTreeWithTaskkill(Number(child.pid)).pipe( From d19a57d11277c57aa75f510189c54523ce17e6ef Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 13 Nov 2026 21:18:42 +0000 Subject: [PATCH 5/5] test(acp): assert detached spawn option per host platform Replace the end-to-end taskkill regression test with a spawner that records the detached option on Windows and POSIX hosts, and note why Windows skips it. Co-Authored-By: Claude Opus 5.5 --- .../Adapters/AcpAdapterV2.test.ts | 88 ++++++------------- .../src/server/AcpSessionRuntime.ts | 2 + 2 files changed, 31 insertions(+), 59 deletions(-) diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index b58d579c0e7ca..28b1897e58d81 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -572,65 +572,35 @@ function makeMockRuntime(input: { }); } -it.live("does not detach an owned Windows ACP process and retains PID tree cleanup", () => { - let finalizerPid = 0; - let spawnedPid = 0; - return Effect.gen(function* () { - const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const path = yield* Path.Path; - const terminated: number[] = []; - const runtime = yield* AcpSessionRuntime.make({ - spawn: { - command: process.execPath, - args: [ - yield* path.fromFileUrl(new URL("../../../scripts/acp-mock-agent.ts", import.meta.url)), - ], - }, - cwd: process.cwd(), - clientInfo: { name: "test", version: "1" }, - ownDetachedProcessGroup: true, - processGroupPlatform: "win32", - windowsProcessTreeTerminator: (pid) => - Effect.sync(() => { - terminated.push(pid); - }), - }).pipe( - Effect.provideService(HostProcess.Platform, "win32"), - Effect.provideService( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make((command) => { - if (!ChildProcess.isStandardCommand(command)) - return Effect.die("Expected standard command"); - if (command.command === "taskkill") { - finalizerPid = Number(command.args[1]); - // Exercise the Windows finalizer while terminating the mock child on any host. - return spawner.spawn( - ChildProcess.make(process.execPath, ["-e", `process.kill(${spawnedPid})`]), - ); - } - assert.equal(command.command, process.execPath); - assert.isFalse(command.options.detached); - return spawner.spawn(command).pipe( - Effect.tap((child) => - Effect.sync(() => { - spawnedPid = Number(child.pid); - }), - ), - ); - }), - ), - ); - if (runtime.terminateProcessGroup === undefined) - return yield* Effect.die("Missing tree cleanup"); - yield* runtime.terminateProcessGroup; - assert.isAbove(spawnedPid, 0); - assert.deepEqual(terminated, [spawnedPid]); - }).pipe( - Effect.provide(layerTest), - Effect.scoped, - Effect.tap(() => Effect.sync(() => assert.equal(finalizerPid, spawnedPid))), - ); -}); +it.effect("detaches owned ACP processes on POSIX hosts but not on Windows", () => + Effect.gen(function* () { + for (const [platform, expected] of [ + ["win32", false], + ["linux", true], + ] as const) { + let detached: boolean | undefined; + yield* AcpSessionRuntime.make({ + spawn: { command: "acp-agent", args: [] }, + cwd: process.cwd(), + clientInfo: { name: "test", version: "1" }, + ownDetachedProcessGroup: true, + processGroupPlatform: platform, + }).pipe( + Effect.provideService(HostProcess.Platform, platform), + Effect.provideService( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => { + if (ChildProcess.isStandardCommand(command)) detached = command.options.detached; + return Effect.die("spawn stopped by test"); + }), + ), + Effect.scoped, + Effect.exit, + ); + assert.equal(detached, expected, platform); + } + }).pipe(Effect.provide(layerTest)), +); const isProviderAdapterProtocolError = Schema.is(ProviderAdapter.ProviderAdapterProtocolError); diff --git a/packages/provider-acp/src/server/AcpSessionRuntime.ts b/packages/provider-acp/src/server/AcpSessionRuntime.ts index ae7bf31253091..c603dd937fcaf 100644 --- a/packages/provider-acp/src/server/AcpSessionRuntime.ts +++ b/packages/provider-acp/src/server/AcpSessionRuntime.ts @@ -1465,6 +1465,8 @@ export const make = Effect.fn("AcpSessionRuntime.make")(function* ( ...(options.spawn.cwd ? { cwd: options.spawn.cwd } : {}), ...(spawnEnvironment ? { env: spawnEnvironment } : {}), extendEnv: options.spawn.extendEnv ?? true, + // Windows `detached` gives the child its own console, which breaks stdio pipes through + // `.cmd` shims; taskkill /T cleans up the tree there without it. ...(options.ownDetachedProcessGroup === undefined ? {} : { detached: options.ownDetachedProcessGroup && hostPlatform !== "win32" }),