From 3a9b11e3a3f827b88e8525e2ab7b86b26284d639 Mon Sep 17 00:00:00 2001
From: badcuban <wilfredoleon94@gmail.com>
Date: Fri, 12 Jun 2026 16:51:23 -0400
Subject: [PATCH] fix: closing a terminal on Windows never killed its processes

node-pty's WindowsTerminal.kill(signal) throws 'Signals not supported
on windows.' whenever a signal is passed. The terminal manager's kill
escalation always passes SIGTERM (then SIGKILL), so on win32 both
attempts fail silently and the shell plus everything it spawned (dev
servers, watchers) keeps running detached until the app exits, while
the session and its history are deleted.

On win32, call kill() with no signal instead and skip the escalation:
node-pty's signal-less Windows kill terminates every process attached
to the pty's console. POSIX behavior is unchanged.

The two existing escalation tests ran against the host platform and
would exercise the win32 path on Windows machines, so they now pin
platform: 'linux'; a new test pins win32 and asserts a single
signal-less kill.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../src/terminal/Layers/Manager.test.ts       | 25 ++++++++++++++++++-
 apps/server/src/terminal/Layers/Manager.ts    | 25 ++++++++++++++++++-
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/apps/server/src/terminal/Layers/Manager.test.ts b/apps/server/src/terminal/Layers/Manager.test.ts
index 2ebf8481957..2b36148d6f8 100644
--- a/apps/server/src/terminal/Layers/Manager.test.ts
+++ b/apps/server/src/terminal/Layers/Manager.test.ts
@@ -983,7 +983,10 @@ it.layer(
 
   it.effect("escalates terminal shutdown to SIGKILL when process does not exit in time", () =>
     Effect.gen(function* () {
-      const { manager, ptyAdapter } = yield* createManager(5, { processKillGraceMs: 10 });
+      const { manager, ptyAdapter } = yield* createManager(5, {
+        platform: "linux",
+        processKillGraceMs: 10,
+      });
       yield* manager.open(openInput());
       const process = ptyAdapter.processes[0];
       expect(process).toBeDefined();
@@ -999,6 +1002,25 @@ it.layer(
     }).pipe(Effect.provide(TestClock.layer())),
   );
 
+  it.effect("kills the terminal without a signal on Windows", () =>
+    Effect.gen(function* () {
+      const { manager, ptyAdapter } = yield* createManager(5, {
+        platform: "win32",
+        subprocessInspector: () =>
+          Effect.succeed({ hasRunningSubprocess: false, childCommand: null }),
+      });
+      yield* manager.open(openInput());
+      const process = ptyAdapter.processes[0];
+      expect(process).toBeDefined();
+      if (!process) return;
+
+      yield* manager.close({ threadId: "thread-1" });
+      yield* waitFor(Effect.sync(() => process.killed));
+
+      expect(process.killSignals).toEqual([undefined]);
+    }),
+  );
+
   it.effect("publishes closed events when terminals are explicitly closed", () =>
     Effect.gen(function* () {
       const { manager, getEvents } = yield* createManager();
@@ -1501,6 +1523,7 @@ it.layer(
     Effect.gen(function* () {
       const scope = yield* Scope.make("sequential");
       const { manager, ptyAdapter } = yield* createManager(5, {
+        platform: "linux",
         processKillGraceMs: 10,
       }).pipe(Effect.provideService(Scope.Scope, scope));
       yield* manager.open(openInput());
diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts
index cd490de1e3f..7271105ac16 100644
--- a/apps/server/src/terminal/Layers/Manager.ts
+++ b/apps/server/src/terminal/Layers/Manager.ts
@@ -75,7 +75,7 @@ class TerminalProcessSignalError extends Schema.TaggedErrorClass<TerminalProcess
   {
     message: Schema.String,
     cause: Schema.optional(Schema.Defect()),
-    signal: Schema.Literals(["SIGTERM", "SIGKILL"]),
+    signal: Schema.optional(Schema.Literals(["SIGTERM", "SIGKILL"])),
   },
 ) {}
 
@@ -1075,6 +1075,29 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith
       threadId: string,
       terminalId: string,
     ) {
+      if (platform === "win32") {
+        // node-pty throws on Windows whenever a signal is passed; a signal-less
+        // kill terminates every process attached to the pty's console
+        // (shell and child processes such as dev servers), so no escalation step.
+        yield* Effect.try({
+          try: () => process.kill(),
+          catch: (cause) =>
+            new TerminalProcessSignalError({
+              message: "Failed to kill terminal console process tree.",
+              cause,
+            }),
+        }).pipe(
+          Effect.catch((error) =>
+            Effect.logWarning("failed to kill terminal process", {
+              threadId,
+              terminalId,
+              error: error.message,
+            }),
+          ),
+        );
+        return;
+      }
+
       const terminated = yield* Effect.try({
         try: () => process.kill("SIGTERM"),
         catch: (cause) =>