Avoid double-closing Windows ConPTY handles (#3359)

src/main/providers/local-pty-provider.test.ts

@@ -472,6 +472,25 @@ describe('LocalPtyProvider', () => {
       expect(onExit).toHaveBeenCalledWith(id, -1)
     })
 
+    it('does not destroy after an intentional Windows shutdown kill', async () => {
+      Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+      const killSpy = vi.fn()
+      const destroySpy = vi.fn(() => {
+        killSpy()
+      })
+      spawnMock.mockReturnValue({
+        ...mockProc,
+        kill: killSpy,
+        destroy: destroySpy
+      })
+
+      const { id } = await provider.spawn({ cols: 80, rows: 24 })
+      await provider.shutdown(id, { immediate: true })
+
+      expect(killSpy).toHaveBeenCalledTimes(1)
+      expect(destroySpy).not.toHaveBeenCalled()
+    })
+
     it('is a no-op for unknown PTY ids', async () => {
       await provider.shutdown('nonexistent', { immediate: true })
       expect(mockProc.kill).not.toHaveBeenCalled()
@@ -611,5 +630,23 @@ describe('LocalPtyProvider', () => {
       const list = await provider.listProcesses()
       expect(list).toHaveLength(0)
     })
+
+    it('does not destroy after intentional Windows orphan kills', async () => {
+      Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+      const destroySpy = vi.fn()
+      const killSpy = vi.fn()
+      spawnMock.mockReturnValue({
+        ...mockProc,
+        kill: killSpy,
+        destroy: destroySpy
+      })
+
+      await provider.spawn({ cols: 80, rows: 24 })
+
+      provider.killAll()
+
+      expect(killSpy).toHaveBeenCalledTimes(1)
+      expect(destroySpy).not.toHaveBeenCalled()
+    })
   })
 })

src/main/providers/local-pty-provider.ts

@@ -114,7 +114,7 @@ function clearPtyState(id: string): void {
   ptyLoadGeneration.delete(id)
 }
 
-function destroyPtyProcess(proc: pty.IPty): void {
+function destroyPtyProcess(proc: pty.IPty, options: { alreadyKilled?: boolean } = {}): void {
   // Why: node-pty's UnixTerminal.destroy() closes the master socket, which
   // releases the ptmx fd to the OS — without this call the fd leaks until GC
   // (see docs/fix-pty-fd-leak.md). destroy() also registers a close listener
@@ -122,9 +122,11 @@ function destroyPtyProcess(proc: pty.IPty): void {
   // the time that listener runs the child may have exited and its pid been
   // recycled to an unrelated user process — SIGHUP would land on a Chrome tab,
   // editor, etc. Neutralize proc.kill on this instance before calling
-  // destroy() to defuse the hazard. Windows exempt: WindowsTerminal.destroy
-  // IS a kill() call via _deferNoArgs, so neutralizing it would leak the
-  // ConPTY agent.
+  // destroy() to defuse the hazard. On Windows, destroy() is itself kill();
+  // skip it only after we have already killed the ConPTY.
+  if (process.platform === 'win32' && options.alreadyKilled) {
+    return
+  }
   if (process.platform !== 'win32') {
     ;(proc as unknown as { kill: (sig?: string) => void }).kill = () => {}
   }
@@ -142,7 +144,7 @@ function safeKillAndClean(id: string, proc: pty.IPty): void {
   } catch {
     /* Process may already be dead */
   }
-  destroyPtyProcess(proc)
+  destroyPtyProcess(proc, { alreadyKilled: true })
   clearPtyState(id)
 }
 
@@ -557,7 +559,7 @@ export class LocalPtyProvider implements IPtyProvider {
     } catch {
       /* Process may already be dead */
     }
-    destroyPtyProcess(proc)
+    destroyPtyProcess(proc, { alreadyKilled: true })
     ptyProcesses.delete(id)
     ptyShellName.delete(id)
     ptyLoadGeneration.delete(id)

src/main/rate-limits/hidden-pty-cleanup.test.ts

@@ -69,7 +69,7 @@ describe('cleanupHiddenRateLimitPty', () => {
     expect(killMock).toHaveBeenCalledWith()
   })
 
-  it('does not neutralize kill on Windows because destroy closes ConPTY through kill', () => {
+  it('does not destroy after an intentional Windows kill because destroy kills again', () => {
     setPlatform('win32')
     const term = {
       kill: vi.fn(),
@@ -80,7 +80,22 @@ describe('cleanupHiddenRateLimitPty', () => {
 
     cleanupHiddenRateLimitPty(term, [], { kill: true })
 
-    expect(term.kill).toHaveBeenCalledTimes(2)
+    expect(term.kill).toHaveBeenCalledTimes(1)
+    expect(term.destroy).not.toHaveBeenCalled()
+  })
+
+  it('destroys a Windows PTY after natural exit so ConPTY cleanup still runs', () => {
+    setPlatform('win32')
+    const term = {
+      kill: vi.fn(),
+      destroy: vi.fn(() => {
+        term.kill()
+      })
+    }
+
+    cleanupHiddenRateLimitPty(term, [], { kill: false })
+
+    expect(term.kill).toHaveBeenCalledTimes(1)
     expect(term.destroy).toHaveBeenCalledTimes(1)
   })
 })

src/main/rate-limits/hidden-pty-cleanup.ts

@@ -22,6 +22,12 @@ export function cleanupHiddenRateLimitPty(
     } catch {
       /* already exited */
     }
+
+    // Why: node-pty WindowsTerminal.destroy() calls kill() again, which can
+    // close the same ConPTY handle twice after an intentional termination.
+    if (process.platform === 'win32') {
+      return
+    }
   }
 
   // Why: node-pty destroy releases the master PTY fd; on POSIX, neutralize