From 980e5d846b5ec8db177a82f1e2fd0123ce5ce0dd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:43:27 -0700 Subject: [PATCH] fix(pty): fence signals at a superseded PTY, and pin why kill is exempt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A signal means "interrupt my pane", so delivering one to a PTY the pane has already replaced is a misdirected interrupt. Fence it with the same lock-step proof used for write and resize. `pty:kill` stays deliberately unfenced and a test now pins that: a superseded PTY is orphaned, and reclaiming it is exactly what the orphan-cleanup callers ask for. Refusing there would break the operation that reclaims leaked shells — the opposite of the intent. The fence sits at the IPC boundary, above `tryGetProviderForPty`, so it covers local, daemon and SSH rather than the local path alone. Co-authored-by: Orca --- src/main/ipc/pty-superseded-operation-fence.test.ts | 12 +++++++++++- src/main/ipc/pty.ts | 6 ++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/main/ipc/pty-superseded-operation-fence.test.ts b/src/main/ipc/pty-superseded-operation-fence.test.ts index 5d8fc36a78f..c7c74bc0f72 100644 --- a/src/main/ipc/pty-superseded-operation-fence.test.ts +++ b/src/main/ipc/pty-superseded-operation-fence.test.ts @@ -80,7 +80,7 @@ describe('superseded PTY operation fence', () => { describe('the fence is wired into every mutating handler', () => { // The capability existed for months and was never called from these handlers. // Pin the call site, not the capability — that is the failure this program hit. - it.each(['pty:write', 'pty:writeAccepted', 'pty:resize'])( + it.each(['pty:write', 'pty:writeAccepted', 'pty:resize', 'pty:signal'])( '%s consults the fence', async (channel) => { const { readFileSync } = await import('node:fs') @@ -93,4 +93,14 @@ describe('the fence is wired into every mutating handler', () => { expect(source.slice(start, start + 700)).toContain('isSupersededPtyId') } ) + + // pty:kill is deliberately NOT fenced: a superseded PTY is orphaned, and + // reclaiming it is exactly what the orphan-cleanup callers ask for. + it('leaves pty:kill unfenced on purpose', async () => { + const { readFileSync } = await import('node:fs') + const source = readFileSync('src/main/ipc/pty.ts', 'utf-8') + const start = source.search(/ipcMain\.handle\(\s*'pty:kill'/) + expect(start).toBeGreaterThan(0) + expect(source.slice(start, start + 500)).not.toContain('isSupersededPtyId') + }) }) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index f2858c9f21b..ec58646d8b0 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -7257,6 +7257,12 @@ export function registerPtyHandlers( ipcMain.removeAllListeners('pty:signal') ipcMain.on('pty:signal', (_event, args: { id: string; signal: string }) => { + // Why fenced but pty:kill is not: a signal means "interrupt MY pane", so a + // superseded id is a misdirected interrupt. A kill on a superseded id is the + // opposite — that PTY is now orphaned and reclaiming it is the point. + if (isSupersededPtyId(args.id)) { + return + } tryGetProviderForPty(args.id) ?.sendSignal(args.id, args.signal) .catch(() => {})