From 1f81ab9dd42af243ca39734b377c6795aedf71e7 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Tue, 3 Nov 2026 20:58:22 -0500 Subject: [PATCH 1/3] 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/3] 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/3] 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); + }), + ), ); });