fix(pty): retire the node-pty master fd instead of probing around it

node-pty hands the master to libuv, which closes it on EIO/EOF, but `_close()`
never invalidated `_fd` and neither `resize()` nor the `process` getter consulted
anything. A late ioctl therefore addressed whatever the kernel had since reissued
that number to. On Linux the very next pty gets it: resizing an exited handle
silently resized an unrelated live pane, with no error for #17832's
probe-retry-classify to observe.

Retire `_fd` in the same block that gives up the master and guard both fd-addressed
syscalls on it, so a stale handle is unreachable by construction. The relay's resize
keeps the isProcessAlive-proven retirement — still the only evidence of `exited` —
and drops the re-probe-and-classify arm the ioctl no longer needs.

Windows is unaffected: WindowsTerminal.resize goes through the conpty agent and
reads no fd.

config/patches/node-pty@1.1.0.patch

@@ -138,8 +138,24 @@ index 8c4fca9022a6d6f015bca87f61625cde2278f428..0a01730616488119aa21ef441cf3c441
  process.exit(0);
  //# sourceMappingURL=conpty_console_list_agent.js.map
 \ No newline at end of file
+diff --git a/lib/terminal.js b/lib/terminal.js
+index e2f9bc9131077b53ebc32d207207ad82804ff185..a6af9300900eafa1c695c4207ba550f64937e542 100644
+--- a/lib/terminal.js
++++ b/lib/terminal.js
+@@ -172,6 +172,11 @@ var Terminal = /** @class */ (function () {
+         this.end = function () { };
+         this._writable = false;
+         this._readable = false;
++        // Orca: libuv closes the master fd on EIO/EOF, and the kernel may hand
++        // that number straight to the next open(2). Retire it in the same block
++        // that gives up the handle so no later ioctl can address a reused fd.
++        // Inert on Windows, where `_fd` is written once and never read back.
++        this._fd = -1;
+     };
+     Terminal.prototype._parseEnv = function (env) {
+         var keys = Object.keys(env || {});
 diff --git a/lib/unixTerminal.js b/lib/unixTerminal.js
-index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088bf1865877 100644
+index 1ec12f796a822c78fba9ad7f6448c3987e325c23..09176dcccaa3a040acfbb8ee9534f020f034befd 100644
 --- a/lib/unixTerminal.js
 +++ b/lib/unixTerminal.js
 @@ -28,8 +28,12 @@ var native = utils_1.loadNativeModule('pty');
@@ -157,6 +173,30 @@ index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088b
  var DEFAULT_FILE = 'sh';
  var DEFAULT_NAME = 'xterm';
  var DESTROY_SOCKET_TIMEOUT_MS = 200;
+@@ -234,6 +238,11 @@ var UnixTerminal = /** @class */ (function (_super) {
+          * Gets the name of the process.
+          */
+         get: function () {
++            // Orca: tcgetpgrp on a retired fd would name whatever process now
++            // owns that descriptor, so a closed master reports the spawn file.
++            if (this._fd < 0) {
++                return this._file;
++            }
+             if (process.platform === 'darwin') {
+                 var title = pty.process(this._fd);
+                 return (title !== 'kernel_task') ? title : this._file;
+@@ -250,6 +259,11 @@ var UnixTerminal = /** @class */ (function (_super) {
+         if (cols <= 0 || rows <= 0 || isNaN(cols) || isNaN(rows) || cols === Infinity || rows === Infinity) {
+             throw new Error('resizing must be done using positive cols and rows');
+         }
++        // Orca: a retired master is unreachable rather than EBADF-or-worse; cols
++        // and rows stay at the last size actually applied instead of a claim.
++        if (this._fd < 0) {
++            return;
++        }
+         pty.resize(this._fd, cols, rows);
+         this._cols = cols;
+         this._rows = rows;
 diff --git a/src/conpty_console_list_agent.ts b/src/conpty_console_list_agent.ts
 index 181ccabbbe9c4948a9725fb1db907a68e9de01fc..67f31facf85562b67adbfbd04ce28ddd8eeb4a79 100644
 --- a/src/conpty_console_list_agent.ts

pnpm-lock.yaml

@@ -115,7 +115,7 @@ patchedDependencies:
   '@xterm/addon-webgl@0.20.0-beta.299': 94687e89a0115e6e6aa102837f986debdc029c091527ee5eb4a4e17ceaf9473e
   '@xterm/xterm@6.1.0-beta.303': 98756bcedc402bcdb7c6ab7b015d2e59cd18e97b03a2c06a27e95bb3ba429d9d
   lint-staged@16.4.0: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
-  node-pty@1.1.0: 40b6b6b814c89a8a29c995495ea86cff2cf6766f42124b5702aedf4ce0565c0e
+  node-pty@1.1.0: 929c703f3743efdd25929a6f7fdca716e3b54fb4a931bc32e1481633ea17defd
 
 importers:
 
@@ -156,7 +156,7 @@ importers:
         version: 3.3.1
       node-pty:
         specifier: ^1.1.0
-        version: 1.1.0(patch_hash=40b6b6b814c89a8a29c995495ea86cff2cf6766f42124b5702aedf4ce0565c0e)
+        version: 1.1.0(patch_hash=929c703f3743efdd25929a6f7fdca716e3b54fb4a931bc32e1481633ea17defd)
       posthog-node:
         specifier: ^5.33.3
         version: 5.33.3
@@ -12194,7 +12194,7 @@ snapshots:
 
   node-int64@0.4.0: {}
 
-  node-pty@1.1.0(patch_hash=40b6b6b814c89a8a29c995495ea86cff2cf6766f42124b5702aedf4ce0565c0e):
+  node-pty@1.1.0(patch_hash=929c703f3743efdd25929a6f7fdca716e3b54fb4a931bc32e1481633ea17defd):
     dependencies:
       node-addon-api: 7.1.1
 

src/main/pty/node-pty-master-fd-retirement.test.ts

@@ -0,0 +1,120 @@
+import * as pty from 'node-pty'
+import { describe, expect, it } from 'vitest'
+
+/**
+ * node-pty hands the master fd to libuv, which closes it on EIO/EOF, but upstream
+ * never invalidated `_fd`, and neither `resize()` nor the `process` getter consulted
+ * anything. Orca's patch retires `_fd` in the same block that gives up the handle
+ * (config/patches/node-pty@1.1.0.patch), which is what makes a stale handle
+ * unreachable instead of merely unlucky.
+ */
+
+const POSIX_SHELL = '/bin/sh'
+
+function spawnPty(command: string, cols = 80, rows = 24): pty.IPty {
+  return pty.spawn(POSIX_SHELL, ['-c', command], {
+    name: 'xterm-256color',
+    cols,
+    rows,
+    cwd: process.cwd(),
+    env: { ...process.env }
+  })
+}
+
+function masterFd(term: pty.IPty): number {
+  return (term as unknown as { fd: number }).fd
+}
+
+/** Run `command` to completion and let node-pty finish giving up the master. */
+async function retiredPty(command = 'exit 0'): Promise<{ term: pty.IPty; spawnFd: number }> {
+  const term = spawnPty(command)
+  const spawnFd = masterFd(term)
+  await new Promise<void>((resolve) => {
+    term.onExit(() => resolve())
+  })
+  ;(term as unknown as { destroy?: () => void }).destroy?.()
+  await new Promise<void>((resolve) => setTimeout(resolve, 400))
+  return { term, spawnFd }
+}
+
+// Windows never reaches this code: WindowsTerminal.resize goes through the conpty
+// agent and reads no fd, so the sentinel is written and never consulted there.
+const describeOnPosix = process.platform === 'win32' ? describe.skip : describe
+
+describeOnPosix('node-pty master fd retirement', () => {
+  it('invalidates the descriptor once it gives up the handle', async () => {
+    const { term, spawnFd } = await retiredPty()
+
+    expect(spawnFd).toBeGreaterThanOrEqual(0)
+    expect(masterFd(term)).toBe(-1)
+  }, 15000)
+
+  it('answers a resize past retirement without issuing the ioctl', async () => {
+    const { term } = await retiredPty()
+
+    // Pre-patch this threw `ioctl(2) failed, EBADF` out of whatever called it.
+    expect(() => term.resize(200, 50)).not.toThrow()
+    // Geometry stays at the last size actually applied rather than claiming one
+    // that no descriptor ever received.
+    expect([term.cols, term.rows]).toEqual([80, 24])
+  }, 15000)
+
+  it('names the spawn file rather than tcgetpgrp on a retired descriptor', async () => {
+    const { term } = await retiredPty()
+
+    expect(term.process).toBe(POSIX_SHELL)
+  }, 15000)
+})
+
+// Linux frees the master synchronously enough that the very next pty is handed the
+// same descriptor number every time, which makes the reuse hazard directly
+// observable rather than a race to reproduce.
+const describeOnLinux = process.platform === 'linux' ? describe : describe.skip
+
+describeOnLinux('node-pty master fd reuse', () => {
+  it('cannot resize a live pty handed the retired descriptor number', async () => {
+    const { term: retired, spawnFd } = await retiredPty()
+
+    const live = spawnPty('sleep 1; stty size')
+    let output = ''
+    live.onData((data) => {
+      output += data
+    })
+    try {
+      // The premise of this test: the kernel really did reissue the number. If it
+      // stops holding, the assertion below would pass for the wrong reason.
+      expect(masterFd(live)).toBe(spawnFd)
+
+      // Pre-patch this reached TIOCSWINSZ on `live`'s master and silently resized
+      // a terminal it has no relationship to — no error, nothing for a liveness
+      // probe of the retired pid to observe.
+      retired.resize(200, 50)
+
+      await new Promise<void>((resolve) => {
+        live.onExit(() => resolve())
+      })
+      expect(output.trim()).toBe('24 80')
+    } finally {
+      live.kill()
+    }
+  }, 15000)
+
+  it('does not name a live pty foreground process off the retired descriptor', async () => {
+    const { term: retired, spawnFd } = await retiredPty()
+
+    // `exec` replaces the shell, so the foreground pgrp's cmdline is distinct
+    // from the file this pty was spawned with.
+    const live = spawnPty('exec sleep 5')
+    try {
+      expect(masterFd(live)).toBe(spawnFd)
+      // Let the shell finish exec'ing, or its own cmdline is still the fallback.
+      await new Promise<void>((resolve) => setTimeout(resolve, 300))
+
+      // Pre-patch this read tcgetpgrp off `live`'s master and reported `sleep`,
+      // attributing an unrelated pane's process to a pty that had already exited.
+      expect(retired.process).toBe(POSIX_SHELL)
+    } finally {
+      live.kill()
+    }
+  }, 15000)
+})

src/relay/pty-handler-resize-stale-pty.test.ts

@@ -36,7 +36,14 @@ import type { MockDispatcher } from './pty-handler-test-harness'
 const PTY_1 = testPtyId(1)
 const STALE_PID = 424_242
 
-/** node-pty's native `pty.resize` error when the master fd is already closed. */
+/**
+ * node-pty's native `pty.resize` error when the ioctl reaches a closed master.
+ *
+ * Orca's node-pty patch retires `_fd` when it gives up the master, so a real
+ * handle answers a late resize with a no-op instead. What is left for this
+ * handler to contain is the tick between libuv closing the fd on a read error
+ * and node-pty's own handler observing it.
+ */
 function ebadfResize(): never {
   throw new Error('ioctl(2) failed, EBADF')
 }
@@ -55,7 +62,7 @@ describe('PtyHandler.resize against a stale PTY handle', () => {
     }))
     resize = vi.fn()
     // A shell that exited without node-pty producing `onExit`: the record is
-    // still in the pool and undisposed, but the master fd behind it is closed.
+    // still in the pool and undisposed, but the master behind it is gone.
     mockPtySpawn.mockReturnValue({ ...mockPtyInstance, pid: STALE_PID, resize })
     await dispatcher.callRequest('pty.spawn', {})
     expect(handler.activePtyCount).toBe(1)
@@ -80,7 +87,7 @@ describe('PtyHandler.resize against a stale PTY handle', () => {
     expect(handler.activePtyCount).toBe(0)
   })
 
-  it('contains an ioctl failure whose process is still live, and keeps the record', () => {
+  it('contains an ioctl failure without re-classifying liveness, and keeps the record', () => {
     vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
     resize.mockImplementation(ebadfResize)
     const stderr = vi.spyOn(process.stderr, 'write').mockReturnValue(true)
@@ -94,7 +101,8 @@ describe('PtyHandler.resize against a stale PTY handle', () => {
       dispatcher.callNotification('pty.resize', { id: PTY_1, cols: 90, rows: 30 })
     ).not.toThrow()
 
-    // Loss of an fd is not evidence the shell exited, so the claim is retained.
+    // Loss of an fd observed the handle, not the host that owns the pid, so it is
+    // `unverifiable` and the claim is retained. Only the probe above retires.
     expect(handler.activePtyCount).toBe(1)
     expect(stderr.mock.calls.map(([line]) => String(line)).join('')).toContain(
       'ioctl(2) failed, EBADF'

src/relay/pty-handler.ts

@@ -2060,30 +2060,27 @@ export class PtyHandler {
     if (!managed || managed.disposed) {
       return
     }
-    // Why probe first (same probe attach() and listProcesses() run): a shell
-    // that exited without node-pty's `onExit` leaves this entry holding a closed
-    // master fd, and `UnixTerminal.resize` has no fd guard — the ioctl throws
-    // `ioctl(2) failed, EBADF` straight out of this notification handler into the
-    // dispatcher's generic parse-error catch, where it is logged and nothing
-    // else. Nothing retired the entry, so it kept being advertised as live and
-    // kept holding `activePtyCount` above zero, which is what stops a relay with
+    // Why probe (same probe attach() and listProcesses() run): a shell that
+    // exited without node-pty's `onExit` leaves an undisposed entry behind, and
+    // while it stays the relay keeps advertising a dead shell and keeps holding
+    // `activePtyCount` above zero, which is what stops a relay with
     // `relayGracePeriodSeconds: 0` from ever reaching its idle-no-ptys exit
-    // (#12423).
+    // (#12423). This is retirement, not ioctl safety: only ESRCH from the host
+    // that owns the pid is evidence of `exited`.
     if (this.reapPtyProvenExited(managed)) {
       return
     }
+    // Safety is the handle's own, not this probe's: node-pty retires `_fd` in
+    // the same block that gives up the master (config/patches/node-pty@1.1.0.patch),
+    // so a resize past that point is a no-op instead of a TIOCSWINSZ aimed at a
+    // closed — or kernel-reused — descriptor. What survives is the tick between
+    // libuv closing the fd on a read error and node-pty's handler seeing it, and
+    // an EBADF there is `unverifiable`: it observed the handle, not the host.
     try {
       managed.pty.resize(cols, rows)
     } catch (err) {
-      // A failed ioctl observed the handle, not the host's process table, so on
-      // its own it is `unverifiable`. Re-probe: a now-absent pid retires the
-      // entry, anything else keeps it and is contained here rather than
-      // escaping as a parse error on every later resize.
-      if (this.reapPtyProvenExited(managed)) {
-        return
-      }
       process.stderr.write(
-        `[pty-handler] resize failed for PTY ${id} whose process is still live or unverifiable: ${err instanceof Error ? err.message : String(err)}\n`
+        `[pty-handler] resize failed for PTY ${id}: ${err instanceof Error ? err.message : String(err)}\n`
       )
     }
   }