fix(pty): retire the write stream's fd copy too, and keep the resize re-probe

Two gaps in the fd-retirement change.

`CustomWriteStream` is the third fd-addressed surface and takes its own plain-number
copy of the master at spawn, so `Terminal._fd = -1` never reaches it. Its `dispose()`
only cleared a pending immediate, and it was called from `destroy()` alone — the
EIO/EOF read-error path and the socket `'close'` path both reach `_close()` without
it. A residual `_writeQueue` and an in-flight `fs.write` therefore kept addressing
the number after libuv gave it up. Retire the copy and drop the queue in `dispose()`,
and call it from `_close()`.

Restore the relay's catch-block re-probe. It was never redundant with the patch:
libuv closes the fd synchronously inside `uv_close`, before the JS `'close'` that
runs `_close()`, so the reuse window opens ahead of any JS guard; and a relay host
installs node-pty from npm, where the patch is not applied at all. The re-probe is a
`process.kill(pid, 0)` on a path already inside an exception handler.

Both existing resize tests pinned liveness with a constant mock, which is why the
deletion passed CI invisibly; the new case is alive at the pre-probe and gone by the
time the ioctl throws.

Also adds the patch's first upstream reference: microsoft/node-pty#220 named this
mechanism in 2018 and was closed 2025-12-19 as completed after only improving the
error message; #827 is still open. Tracked as #18109.

config/patches/node-pty@1.1.0.patch

@@ -139,10 +139,10 @@ index 8c4fca9022a6d6f015bca87f61625cde2278f428..0a01730616488119aa21ef441cf3c441
  //# 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
+index e2f9bc9131077b53ebc32d207207ad82804ff185..6c63bfaaf75128d88f9a2efece13476348780cfd 100644
 --- a/lib/terminal.js
 +++ b/lib/terminal.js
-@@ -172,6 +172,11 @@ var Terminal = /** @class */ (function () {
+@@ -172,6 +172,21 @@ var Terminal = /** @class */ (function () {
          this.end = function () { };
          this._writable = false;
          this._readable = false;
@@ -150,12 +150,22 @@ index e2f9bc9131077b53ebc32d207207ad82804ff185..a6af9300900eafa1c695c4207ba550f6
 +        // 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.
++        // Upstream named this mechanism in microsoft/node-pty#220 ("fd number got
++        // reattached to something else"), closed 2025-12-19 as completed after
++        // only improving the error message; #827 is still open. Windows guards in
++        // windowsPtyAgent.ts, Unix does not. Orca tracking: #18109.
 +        this._fd = -1;
++        // Orca: the write stream holds its own copy of that number, so retiring
++        // `_fd` alone leaves the queued and in-flight writes addressing it.
++        // Undefined on Windows and on `UnixTerminal.open()` handles.
++        if (this._writeStream) {
++            this._writeStream.dispose();
++        }
      };
      Terminal.prototype._parseEnv = function (env) {
          var keys = Object.keys(env || {});
 diff --git a/lib/unixTerminal.js b/lib/unixTerminal.js
-index 1ec12f796a822c78fba9ad7f6448c3987e325c23..09176dcccaa3a040acfbb8ee9534f020f034befd 100644
+index 1ec12f796a822c78fba9ad7f6448c3987e325c23..d838d795ecb9ea72e3bcc31113344947c006af7e 100644
 --- a/lib/unixTerminal.js
 +++ b/lib/unixTerminal.js
 @@ -28,8 +28,12 @@ var native = utils_1.loadNativeModule('pty');
@@ -197,6 +207,32 @@ index 1ec12f796a822c78fba9ad7f6448c3987e325c23..09176dcccaa3a040acfbb8ee9534f020
          pty.resize(this._fd, cols, rows);
          this._cols = cols;
          this._rows = rows;
+@@ -287,8 +301,15 @@ var CustomWriteStream = /** @class */ (function () {
+     CustomWriteStream.prototype.dispose = function () {
+         clearImmediate(this._writeImmediate);
+         this._writeImmediate = undefined;
++        // Orca: retire this stream's own copy of the master fd and drop what has
++        // not shipped, so nothing queued here reaches a reused descriptor.
++        this._fd = -1;
++        this._writeQueue.length = 0;
+     };
+     CustomWriteStream.prototype.write = function (data) {
++        if (this._fd < 0) {
++            return;
++        }
+         // Writes are put in a queue and processed asynchronously in order to handle
+         // backpressure from the kernel buffer.
+         var buffer = typeof data === 'string'
+@@ -304,7 +325,8 @@ var CustomWriteStream = /** @class */ (function () {
+     CustomWriteStream.prototype._processWriteQueue = function () {
+         var _this = this;
+         this._writeImmediate = undefined;
+-        if (this._writeQueue.length === 0) {
++        // Orca: an in-flight fs.write can re-enter here after dispose().
++        if (this._fd < 0 || this._writeQueue.length === 0) {
+             return;
+         }
+         var task = this._writeQueue[0];
 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: 929c703f3743efdd25929a6f7fdca716e3b54fb4a931bc32e1481633ea17defd
+  node-pty@1.1.0: e262847f57a1d4d3f2287a843822f7dcf3c9d8655892b07a69eba464e1317eaa
 
 importers:
 
@@ -156,7 +156,7 @@ importers:
         version: 3.3.1
       node-pty:
         specifier: ^1.1.0
-        version: 1.1.0(patch_hash=929c703f3743efdd25929a6f7fdca716e3b54fb4a931bc32e1481633ea17defd)
+        version: 1.1.0(patch_hash=e262847f57a1d4d3f2287a843822f7dcf3c9d8655892b07a69eba464e1317eaa)
       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=929c703f3743efdd25929a6f7fdca716e3b54fb4a931bc32e1481633ea17defd):
+  node-pty@1.1.0(patch_hash=e262847f57a1d4d3f2287a843822f7dcf3c9d8655892b07a69eba464e1317eaa):
     dependencies:
       node-addon-api: 7.1.1
 

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

@@ -3,10 +3,16 @@ 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.
+ * never invalidated `_fd`, and none of the three fd-addressed surfaces consulted
+ * anything: `resize()`, the `process` getter, and `CustomWriteStream`, which holds
+ * its own plain-number copy of the fd taken at spawn. Orca's patch retires all
+ * three in the same block that gives up the handle
+ * (config/patches/node-pty@1.1.0.patch).
+ *
+ * Scope: this narrows the window, it does not close it. libuv closes the fd
+ * synchronously inside `uv_close`, before the JS `'close'` that runs `_close()`,
+ * so callers still need their own liveness verdict for that tick — and a relay
+ * host installs node-pty from npm, where this patch is not applied at all.
  */
 
 const POSIX_SHELL = '/bin/sh'
@@ -25,14 +31,36 @@ 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 }> {
+type CustomWriteStream = { _fd: number; _writeQueue: unknown[]; write(data: string): void }
+
+/**
+ * `Terminal._close()` already shadows `terminal.write` with a no-op, so the stream
+ * itself is the surface that still reached the fd: a residual `_writeQueue` and an
+ * in-flight `fs.write` both re-enter it after the close.
+ */
+function writeStream(term: pty.IPty): CustomWriteStream {
+  return (term as unknown as { _writeStream: CustomWriteStream })._writeStream
+}
+
+/**
+ * Run `command` to completion and let node-pty finish giving up the master.
+ *
+ * `destroy: false` exercises only the EIO/EOF read-error path, which reaches
+ * `_close()` without ever calling `destroy()` — the path the exit of a shell
+ * actually takes, and the one the write stream was previously never told about.
+ */
+async function retiredPty(
+  command = 'exit 0',
+  { destroy = true }: { destroy?: boolean } = {}
+): 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?.()
+  if (destroy) {
+    ;(term as unknown as { destroy?: () => void }).destroy?.()
+  }
   await new Promise<void>((resolve) => setTimeout(resolve, 400))
   return { term, spawnFd }
 }
@@ -59,6 +87,17 @@ describeOnPosix('node-pty master fd retirement', () => {
     expect([term.cols, term.rows]).toEqual([80, 24])
   }, 15000)
 
+  it('retires the write stream fd on _close(), not only on destroy()', async () => {
+    const { term } = await retiredPty('exit 0', { destroy: false })
+
+    // The stream copied the fd number at spawn, so `Terminal._fd = -1` alone
+    // leaves it addressing a descriptor the kernel may already have reissued.
+    expect(writeStream(term)._fd).toBe(-1)
+
+    writeStream(term).write('x')
+    expect(writeStream(term)._writeQueue).toHaveLength(0)
+  }, 15000)
+
   it('names the spawn file rather than tcgetpgrp on a retired descriptor', async () => {
     const { term } = await retiredPty()
 
@@ -99,6 +138,28 @@ describeOnLinux('node-pty master fd reuse', () => {
     }
   }, 15000)
 
+  it('cannot write into a live pty handed the retired descriptor number', async () => {
+    const { term: retired, spawnFd } = await retiredPty('exit 0', { destroy: false })
+
+    const live = spawnPty('read -r line; printf "got:%s" "$line"')
+    let output = ''
+    live.onData((data) => {
+      output += data
+    })
+    try {
+      expect(masterFd(live)).toBe(spawnFd)
+
+      // Pre-patch this fs.write reached `live`'s master and the tty echoed it
+      // back: a retired pane's bytes landing in an unrelated terminal.
+      writeStream(retired).write('leak\r')
+      await new Promise<void>((resolve) => setTimeout(resolve, 300))
+
+      expect(output).not.toContain('leak')
+    } finally {
+      live.kill()
+    }
+  }, 15000)
+
   it('does not name a live pty foreground process off the retired descriptor', async () => {
     const { term: retired, spawnFd } = await retiredPty()
 

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

@@ -39,10 +39,11 @@ const STALE_PID = 424_242
 /**
  * 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.
+ * Orca's node-pty patch retires `_fd` when it gives up the master, so a patched
+ * handle answers a late resize with a no-op. That leaves this handler two cases
+ * it still has to contain: the tick between libuv closing the fd and node-pty's
+ * own handler observing it, and a relay host, which installs node-pty from npm
+ * and has no such guard.
  */
 function ebadfResize(): never {
   throw new Error('ioctl(2) failed, EBADF')
@@ -109,6 +110,26 @@ describe('PtyHandler.resize against a stale PTY handle', () => {
     )
   })
 
+  it('retires the entry when the pid goes absent between the pre-probe and the ioctl', () => {
+    // The race the catch-block re-probe exists for, and the one a constant
+    // liveness mock cannot express: alive when the pre-probe asks, gone by the
+    // time the ioctl fails. libuv closes the master synchronously inside
+    // `uv_close`, so this window opens before any JS guard can be set — and on
+    // a relay host, where node-pty comes from npm, there is no JS guard at all.
+    vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValueOnce(true).mockReturnValueOnce(false)
+    resize.mockImplementation(ebadfResize)
+    const stderr = vi.spyOn(process.stderr, 'write').mockReturnValue(true)
+
+    expect(() =>
+      dispatcher.callNotification('pty.resize', { id: PTY_1, cols: 120, rows: 40 })
+    ).not.toThrow()
+
+    expect(resize).toHaveBeenCalledTimes(1)
+    expect(handler.activePtyCount).toBe(0)
+    // Proven `exited` retires silently; only live-or-unverifiable is reported.
+    expect(stderr).not.toHaveBeenCalled()
+  })
+
   it('still resizes a live PTY, with the clamped geometry', () => {
     vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
 

src/relay/pty-handler.ts

@@ -2070,17 +2070,25 @@ export class PtyHandler {
     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.
+    // The patched node-pty retires `_fd` in the same block that gives up the
+    // master (config/patches/node-pty@1.1.0.patch), which makes a resize past
+    // that point a no-op rather than a TIOCSWINSZ aimed at a reused descriptor.
+    // That covers only part of the window and does not cover this process at
+    // all: libuv closes the fd synchronously inside `uv_close`, before the JS
+    // `'close'` that runs `_close()`, and a relay host installs node-pty from
+    // npm, where the patch is not applied. So the catch below stays.
     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}: ${err instanceof Error ? err.message : String(err)}\n`
+        `[pty-handler] resize failed for PTY ${id} whose process is still live or unverifiable: ${err instanceof Error ? err.message : String(err)}\n`
       )
     }
   }