fix(platform-node): clean process groups after successful leader exit

.changeset/cleanup-successful-process-groups.md

@@ -0,0 +1,5 @@
+---
+"@effect/platform-node-shared": patch
+---
+
+Clean up referenced POSIX process groups on scope closure after their leader exits successfully or is terminated by a signal.

packages/effect/test/unstable/process/ChildProcessSpawnerTest.ts

@@ -473,11 +473,12 @@ export const suite = (
               const ready = path.join(directory, "ready")
               const handle = yield* ChildProcess.make("sh", [
                 "-c",
-                "trap '' TERM; : > \"$1\"; while :; do sleep 1; done",
+                "trap '' TERM; : > \"$1\"; exec cat",
                 "force-kill",
                 ready
               ], {
                 killSignal: "SIGKILL",
+                stdin: "pipe",
                 stdout: "ignore",
                 stderr: "ignore"
               })
@@ -491,19 +492,13 @@ export const suite = (
                 TestClock.withLive
               )
 
-              const completed = yield* handle.kill({
+              yield* handle.kill({
                 killSignal: "SIGTERM",
                 forceKillAfter: "50 millis"
-              }).pipe(
-                Effect.as(true),
-                Effect.timeoutOrElse({
-                  duration: "1 second",
-                  orElse: () => Effect.succeed(false)
-                }),
-                TestClock.withLive
-              )
+              }).pipe(TestClock.withLive)
 
-              assert.isTrue(completed)
+              assert.isFalse(yield* handle.isRunning)
+              assert.isTrue(Exit.isFailure(yield* Effect.exit(handle.exitCode)))
             }))
 
           it.effect("should force kill a process when its scope closes", () =>
@@ -929,8 +924,18 @@ export const suite = (
             // Start the process that spawns children and grandchildren
             const handle = yield* ChildProcess.make("./spawn-children.sh", { cwd })
 
-            // Give it time to spawn all processes
-            yield* TestClock.withLive(Effect.sleep("100 millis"))
+            const readyCount = yield* handle.stdout.pipe(
+              Stream.decodeText,
+              Stream.splitLines,
+              Stream.filter((line) =>
+                line.includes(" started with PID ")
+              ),
+              Stream.take(7),
+              Stream.runCount,
+              Effect.timeout("2 seconds"),
+              TestClock.withLive
+            )
+            assert.strictEqual(readyCount, 7, "all seven processes must report readiness before stdout closes")
 
             // Verify the main process is running
             const isRunningBeforeKill = yield* handle.isRunning
@@ -942,9 +947,7 @@ export const suite = (
               "ps aux | grep spawn-children.sh | grep -v grep | wc -l"
             ])
             const beforeKill = yield* decodeByteStream(beforeKillHandle.stdout).pipe(
-              Effect.map((s) =>
-                Number.parseInt(s.trim())
-              ),
+              Effect.map((s) => Number.parseInt(s.trim())),
               Effect.orElseSucceed(() => 0)
             )
             assert.isAtLeast(beforeKill, 7)

packages/platform/node-shared/src/NodeChildProcessSpawner.ts

@@ -21,6 +21,9 @@
  * wait after `SIGKILL`. Zombie descendants can consume either full bound. On
  * Windows, `taskkill` terminates the tree and only the leader's exit is awaited.
  *
+ * POSIX cleanup targets a numeric process-group ID. If the group disappears
+ * and its ID is reused before cleanup, an unrelated group may be signalled.
+ *
  * @since 4.0.0
  */
 import type * as Arr from "effect/Array"
@@ -545,6 +548,8 @@ const make = Effect.gen(function*() {
               const [code] = yield* Deferred.await(exitSignal)
               if (code !== 0 && Predicate.isNotNull(code)) {
                 yield* Effect.ignore(killProcessGroup(cmd, childProcess, cmd.options.killSignal ?? "SIGTERM"))
+              } else if (isReferenced && process.platform !== "win32" && cmd.options.detached !== false) {
+                yield* Effect.ignore(terminateProcessGroup(cmd, childProcess, exitSignal, cmd.options))
               }
               return
             }

packages/platform/node-shared/test/NodeChildProcessSpawner.test.ts

@@ -80,12 +80,28 @@ it.live("kills every process in a pipeline", () =>
     const childHeartbeat = `${directory}/child-heartbeat`
     const handle = yield* ChildProcess.make(
       "sh",
-      ["-c", "while :; do printf x >> \"$1\"; sleep 0.01; done", "pipeline-root", rootHeartbeat]
+      [
+        "-c",
+        "printf x >> \"$1\"; printf 'ROOT_READY\\n'; while :; do printf x >> \"$1\"; sleep 0.01; done",
+        "pipeline-root",
+        rootHeartbeat
+      ]
     ).pipe(ChildProcess.pipeTo(ChildProcess.make(
       "sh",
-      ["-c", "while :; do printf x >> \"$1\"; sleep 0.01; done", "pipeline-child", childHeartbeat]
+      [
+        "-c",
+        "printf x >> \"$1\"; printf 'CHILD_READY\\n'; read -r ready; printf '%s\\n' \"$ready\"; while :; do printf x >> \"$1\"; sleep 0.01; done",
+        "pipeline-child",
+        childHeartbeat
+      ]
     )))
-    yield* Effect.sleep("100 millis")
+    const readyLines = yield* handle.stdout.pipe(
+      Stream.decodeText,
+      Stream.splitLines,
+      Stream.take(2),
+      Stream.runCollect
+    )
+    assert.deepStrictEqual(readyLines, ["CHILD_READY", "ROOT_READY"])
     yield* handle.kill({ killSignal: "SIGKILL" })
     const rootSizeAfterKill = (yield* fs.stat(rootHeartbeat)).size
     const childSizeAfterKill = (yield* fs.stat(childHeartbeat)).size
@@ -114,7 +130,7 @@ const startProcessGroup = (mode: "exit-on-signal" | "ignore-signal", options?: C
     const fs = yield* FileSystem.FileSystem
     const directory = yield* fs.makeTempDirectoryScoped()
     const marker = `${directory}/marker`
-    const scope = yield* Scope.make()
+    const scope = yield* Scope.fork(yield* Effect.scope)
     const handle = yield* Scope.provide(scope)(ChildProcess.make(
       process.execPath,
       [processGroupFixture, "leader", mode, marker],
@@ -157,6 +173,39 @@ const timed = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
   })
 
 describe.skipIf(process.platform === "win32")("process group cleanup", () => {
+  it.live("scope release cleans descendants after the leader exits successfully", () =>
+    Effect.gen(function*() {
+      const fs = yield* FileSystem.FileSystem
+      const { descendantPid, handle, marker, scope } = yield* startProcessGroup("exit-on-signal", { stdin: "pipe" })
+      yield* Effect.addFinalizer(() => killDescendant(descendantPid))
+
+      yield* Stream.run(Stream.make(new TextEncoder().encode("exit\n")), handle.stdin)
+      assert.strictEqual(yield* handle.exitCode, 0)
+      assert.doesNotThrow(() => process.kill(descendantPid, 0), "descendant must still be alive after the leader exits")
+      assert.isFalse(yield* fs.exists(marker))
+
+      yield* Scope.close(scope, Exit.void)
+
+      assert.strictEqual(yield* fs.readFileString(marker), "exited")
+    }).pipe(Effect.scoped, Effect.provide(NodeServices)))
+
+  it.live("scope release cleans descendants after the leader is killed externally", () =>
+    Effect.gen(function*() {
+      const fs = yield* FileSystem.FileSystem
+      const { descendantPid, handle, marker, scope } = yield* startProcessGroup("exit-on-signal")
+      yield* Effect.addFinalizer(() => killDescendant(descendantPid))
+
+      process.kill(handle.pid, "SIGKILL")
+      assert.isTrue(Exit.isFailure(yield* Effect.exit(handle.exitCode)))
+      assert.doesNotThrow(() => process.kill(descendantPid, 0), "descendant must survive the leader's SIGKILL")
+      assert.isFalse(yield* fs.exists(marker))
+
+      yield* Scope.close(scope, Exit.void)
+
+      assert.isTrue(yield* fs.exists(marker), "scope release must wait for the descendant's exit marker")
+      assert.strictEqual(yield* fs.readFileString(marker), "exited")
+    }).pipe(Effect.scoped, Effect.provide(NodeServices)))
+
   it.live("scope release waits for descendants that outlive the leader", () =>
     Effect.gen(function*() {
       const fs = yield* FileSystem.FileSystem

packages/platform/node-shared/test/fixtures/process-group.ts

@@ -10,6 +10,7 @@ if (role === "leader") {
   spawn(process.execPath, [process.argv[1], "descendant", mode, marker], {
     stdio: ["ignore", "inherit", "inherit"]
   })
+  process.stdin.once("data", () => process.exit(0))
   setInterval(() => {}, 1_000)
 } else {
   if (mode === "exit-on-signal") {