fix(shell): guard Windows pty teardown against blocked process creation

packages/workspace-server/src/services/shell/shell.test.ts

@@ -99,6 +99,51 @@ describe("ShellService.destroy", () => {
     const { service } = createService();
     expect(() => service.destroy("nonexistent")).not.toThrow();
   });
+
+  it("drops the session and still emits exit when pty.destroy() throws", async () => {
+    const ptyProcess = createMockPtyProcess();
+    ptyProcess.destroy.mockImplementation(() => {
+      throw new Error("spawn UNKNOWN");
+    });
+    mockPty.spawn.mockReturnValue(ptyProcess);
+    const { service } = createService();
+    const exitHandler = vi.fn();
+    service.on(ShellEvent.Exit, exitHandler);
+
+    await service.create("session-1");
+
+    expect(() => service.destroy("session-1")).not.toThrow();
+    expect(service.hasSession("session-1")).toBe(false);
+    expect(service.getSessionCount()).toBe(0);
+    expect(exitHandler).toHaveBeenCalledWith({
+      sessionId: "session-1",
+      exitCode: 130,
+    });
+  });
+
+  it("destroyAll tears down every session when one pty.destroy() throws", async () => {
+    const failing = createMockPtyProcess();
+    failing.destroy.mockImplementation(() => {
+      throw new Error("spawn UNKNOWN");
+    });
+    const healthy = createMockPtyProcess();
+    mockPty.spawn
+      .mockReturnValueOnce(failing)
+      .mockReturnValueOnce(healthy)
+      .mockReturnValue(createMockPtyProcess());
+    const { service } = createService();
+
+    await service.create("session-1");
+    await service.create("session-2");
+    await service.create("session-3");
+
+    expect(() => service.destroyAll()).not.toThrow();
+
+    // A throw on the first shell must not abort the shutdown sweep, or the
+    // remaining shells survive after the app quits.
+    expect(healthy.destroy).toHaveBeenCalled();
+    expect(service.getSessionCount()).toBe(0);
+  });
 });
 
 describe("ShellService.createSession workspace env", () => {

packages/workspace-server/src/services/shell/shell.ts

@@ -197,14 +197,7 @@ export class ShellService extends TypedEventEmitter<ShellEvents> {
     disposables.push(
       ptyProcess.onExit(({ exitCode }) => {
         this.processTracking.unregister(ptyProcess.pid, "exited");
-        const session = this.sessions.get(sessionId);
-        if (session) {
-          for (const d of session.disposables) {
-            d.dispose();
-          }
-          session.pty.destroy();
-          this.sessions.delete(sessionId);
-        }
+        this.teardownSession(sessionId);
         this.emit(ShellEvent.Exit, { sessionId, exitCode });
         resolveExit({ exitCode });
       }),
@@ -277,14 +270,7 @@ export class ShellService extends TypedEventEmitter<ShellEvents> {
     disposables.push(
       ptyProcess.onExit(({ exitCode }) => {
         this.processTracking.unregister(ptyProcess.pid, "exited");
-        const session = this.sessions.get(sessionId);
-        if (session) {
-          for (const d of session.disposables) {
-            d.dispose();
-          }
-          session.pty.destroy();
-          this.sessions.delete(sessionId);
-        }
+        this.teardownSession(sessionId);
         this.emit(ShellEvent.Exit, { sessionId, exitCode });
         resolveExit({ exitCode });
       }),
@@ -341,20 +327,41 @@ export class ShellService extends TypedEventEmitter<ShellEvents> {
   destroy(sessionId: string): void {
     const session = this.sessions.get(sessionId);
     if (session) {
-      const pid = session.pty.pid;
-      this.processTracking.kill(pid);
-      for (const disposable of session.disposables) {
-        disposable.dispose();
-      }
-      session.pty.destroy();
-      this.sessions.delete(sessionId);
+      this.processTracking.kill(session.pty.pid);
+      this.teardownSession(sessionId);
       this.emit(ShellEvent.Exit, {
         sessionId,
         exitCode: DESTROYED_EXIT_CODE,
       });
     }
   }
 
+  /**
+   * Drop a session and release its pty. Teardown is best-effort: `pty.destroy()`
+   * reaches into platform-specific cleanup that can fail for environmental
+   * reasons, so the session is removed from the map first and the destroy is
+   * guarded. Otherwise one failing shell would leak its map entry and abort the
+   * `destroyAll()` sweep, leaving the remaining shells running after quit.
+   */
+  private teardownSession(sessionId: string): void {
+    const session = this.sessions.get(sessionId);
+    if (!session) {
+      return;
+    }
+    this.sessions.delete(sessionId);
+    for (const disposable of session.disposables) {
+      disposable.dispose();
+    }
+    try {
+      session.pty.destroy();
+    } catch (error) {
+      this.log.warn(
+        `Failed to destroy pty for shell session ${sessionId}`,
+        error,
+      );
+    }
+  }
+
   /**
    * Destroy all active shell sessions.
    * Used during application shutdown to ensure all child processes are cleaned up.

patches/node-pty.patch

@@ -51,3 +51,87 @@ index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..d2a5e7911fb823bef3c9d57d2bf35830
              },
              'msvs_settings': {
                  # Specify this setting here to override a setting from somewhere
+diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js
+index a358ffb..c3ff5b3 100644
+--- a/lib/windowsPtyAgent.js
++++ b/lib/windowsPtyAgent.js
+@@ -146,6 +146,10 @@ var WindowsPtyAgent = /** @class */ (function () {
+                             // Ignore if process cannot be found (kill ESRCH error)
+                         }
+                     });
++                }).catch(function () {
++                    // Reaping the console process list is best-effort. Nothing awaits
++                    // this promise, so a rejection would surface as an unhandled
++                    // rejection and crash-report from a routine terminal close.
+                 });
+                 this._ptyNative.kill(this._pty, this._useConptyDll);
+                 this._conoutSocketWorker.dispose();
+@@ -181,7 +185,21 @@ var WindowsPtyAgent = /** @class */ (function () {
+     WindowsPtyAgent.prototype._getConsoleProcessList = function () {
+         var _this = this;
+         return new Promise(function (resolve) {
+-            var agent = child_process_1.fork(path.join(__dirname, 'conpty_console_list_agent'), [_this._innerPid.toString()]);
++            var agent;
++            try {
++                agent = child_process_1.fork(path.join(__dirname, 'conpty_console_list_agent'), [_this._innerPid.toString()]);
++            }
++            catch (e) {
++                // fork() throws synchronously when CreateProcess fails outright --
++                // `spawn UNKNOWN` when antivirus/EDR blocks process creation. Fall back
++                // to the shell PID, matching the timeout path below.
++                resolve([_this._innerPid]);
++                return;
++            }
++            agent.on('error', function () {
++                clearTimeout(timeout);
++                resolve([_this._innerPid]);
++            });
+             agent.on('message', function (message) {
+                 clearTimeout(timeout);
+                 resolve(message.consoleProcessList);
+diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts
+index d705444..38681a1 100644
+--- a/src/windowsPtyAgent.ts
++++ b/src/windowsPtyAgent.ts
+@@ -7,7 +7,7 @@
+ import * as fs from 'fs';
+ import * as os from 'os';
+ import * as path from 'path';
+-import { fork } from 'child_process';
++import { ChildProcess, fork } from 'child_process';
+ import { Socket } from 'net';
+ import { ArgvOrCommandLine } from './types';
+ import { ConoutConnection } from './windowsConoutConnection';
+@@ -152,6 +152,10 @@ export class WindowsPtyAgent {
+               // Ignore if process cannot be found (kill ESRCH error)
+             }
+           });
++        }).catch(() => {
++          // Reaping the console process list is best-effort. Nothing awaits this
++          // promise, so a rejection would surface as an unhandled rejection and
++          // crash-report from a routine terminal close.
+         });
+         (this._ptyNative as IConptyNative).kill(this._pty, this._useConptyDll);
+         this._conoutSocketWorker.dispose();
+@@ -184,7 +188,20 @@ export class WindowsPtyAgent {
+
+   private _getConsoleProcessList(): Promise<number[]> {
+     return new Promise<number[]>(resolve => {
+-      const agent = fork(path.join(__dirname, 'conpty_console_list_agent'), [ this._innerPid.toString() ]);
++      let agent: ChildProcess;
++      try {
++        agent = fork(path.join(__dirname, 'conpty_console_list_agent'), [ this._innerPid.toString() ]);
++      } catch (e) {
++        // fork() throws synchronously when CreateProcess fails outright --
++        // `spawn UNKNOWN` when antivirus/EDR blocks process creation. Fall back
++        // to the shell PID, matching the timeout path below.
++        resolve([ this._innerPid ]);
++        return;
++      }
++      agent.on('error', () => {
++        clearTimeout(timeout);
++        resolve([ this._innerPid ]);
++      });
+       agent.on('message', message => {
+         clearTimeout(timeout);
+         resolve(message.consoleProcessList);