From b35367b2fc0377131a9873797fe1a1dbba0f09a8 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 13 Jul 2026 14:32:21 -0700 Subject: [PATCH] fix(terminal): scope pre-handler state to admission --- .../pty-pre-handler-buffer.test.ts | 46 +++++++++++++++++++ .../terminal-pane/pty-pre-handler-buffer.ts | 17 +++++-- .../terminal-pane/pty-transport.test.ts | 21 +++++++++ .../components/terminal-pane/pty-transport.ts | 5 ++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts index 1df6b760a06..c27f82854b7 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts @@ -137,6 +137,52 @@ describe('pre-handler PTY buffer', () => { expect(handler).toHaveBeenCalledWith(-1) }) + it('keeps a replacement probe owned when an older same-id probe settles', async () => { + const target = 'pty-reused-probe-owner' + const oldPrefix = 'pty-reused-probe-old-' + const newPrefix = 'pty-reused-probe-new-' + const resolvers: ((alive: boolean | null) => void)[] = [] + const hasPty = vi.fn( + () => + new Promise((resolve) => { + resolvers.push(resolve) + }) + ) + const handler = vi.fn() + try { + bufferPreHandlerPtyExit(target, 1) + for (let index = 0; index < 64; index += 1) { + bufferPreHandlerPtyExit(`${oldPrefix}${index}`, index) + } + reconcilePreHandlerPtyExitAfterOverflow(target, hasPty, handler, () => true) + + clearPreHandlerPtyState(target) + bufferPreHandlerPtyExit(target, 2) + for (let index = 0; index < 64; index += 1) { + bufferPreHandlerPtyExit(`${newPrefix}${index}`, index) + } + reconcilePreHandlerPtyExitAfterOverflow(target, hasPty, handler, () => true) + expect(hasPty).toHaveBeenCalledTimes(2) + + resolvers[0]?.(true) + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + reconcilePreHandlerPtyExitAfterOverflow(target, hasPty, handler, () => true) + + expect(hasPty).toHaveBeenCalledTimes(2) + expect(handler).not.toHaveBeenCalled() + resolvers[1]?.(true) + await Promise.resolve() + } finally { + clearPreHandlerPtyState(target) + for (let index = 0; index < 64; index += 1) { + clearPreHandlerPtyState(`${oldPrefix}${index}`) + clearPreHandlerPtyState(`${newPrefix}${index}`) + } + } + }) + it('caps eviction tombstones at exactly 1024 ids without probing unrelated PTYs', async () => { const prefix = 'pty-exact-tombstone-cap-' const hasPty = vi.fn(async () => false) diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts index 998743c11d1..98a5a89eeff 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts @@ -26,7 +26,7 @@ const PRE_HANDLER_PTY_EXIT_MAX_PTYS = 64 // window avoids turning one overflow into liveness IPC for unrelated panes. const PRE_HANDLER_PTY_EVICTED_EXIT_MAX_PTYS = 1_024 const preHandlerPtyEvictedExitIds = new Set() -const preHandlerPtyEvictedExitProbes = new Set() +const preHandlerPtyEvictedExitProbes = new Map() // Why: legit pre-attach windows drain within milliseconds and hold little // data. Sustained accumulation means a pane lost its data handler (the // frozen-pane detach/attach race) — leave a breadcrumb for trace capture. @@ -137,12 +137,18 @@ export function reconcilePreHandlerPtyExitAfterOverflow( ) { return } - preHandlerPtyEvictedExitProbes.add(ptyId) + const probeToken = Symbol(ptyId) + preHandlerPtyEvictedExitProbes.set(ptyId, probeToken) // Why: the bounded exit buffer may evict a legitimate pre-registration exit. // Keep its tombstone through unknown liveness so reconnect can retry proof. void hasPty(ptyId) .then((alive) => { - if (!isCurrent() || !preHandlerPtyEvictedExitIds.has(ptyId) || alive === null) { + if ( + preHandlerPtyEvictedExitProbes.get(ptyId) !== probeToken || + !isCurrent() || + !preHandlerPtyEvictedExitIds.has(ptyId) || + alive === null + ) { return } preHandlerPtyEvictedExitIds.delete(ptyId) @@ -152,7 +158,10 @@ export function reconcilePreHandlerPtyExitAfterOverflow( }) .catch(() => {}) .finally(() => { - preHandlerPtyEvictedExitProbes.delete(ptyId) + // Why: a stale probe must not clear a newer same-ID generation's owner. + if (preHandlerPtyEvictedExitProbes.get(ptyId) === probeToken) { + preHandlerPtyEvictedExitProbes.delete(ptyId) + } }) } diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 351e214b276..7eb29b1ebfe 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -147,6 +147,27 @@ describe('createIpcPtyTransport', () => { expect(onDataCallback).not.toHaveBeenCalledWith('dead generation output') }) + it('does not drain an exit buffered before a same-id admission starts', async () => { + const { createIpcPtyTransport, ensurePtyDispatcher } = await import('./pty-transport') + const spawn = window.api.pty.spawn as unknown as ReturnType + spawn.mockResolvedValueOnce({ id: 'same-id' }) + ensurePtyDispatcher() + onExit?.({ id: 'same-id', code: 17 }) + const observedExit = vi.fn() + const transport = createIpcPtyTransport() + + await transport.connect({ + url: '', + sessionId: 'same-id', + callbacks: { onExit: observedExit } + }) + + expect(transport.isConnected()).toBe(true) + expect(transport.getPtyId()).toBe('same-id') + expect(observedExit).not.toHaveBeenCalled() + transport.disconnect() + }) + it('preserves replacement startup events while an older same-id admission rejects', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawn = window.api.pty.spawn as unknown as ReturnType diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index d562bb21bb7..5090199a892 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -708,6 +708,11 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra ? await acquirePtySessionConnectAdmission(options.sessionId) : undefined try { + if (options.sessionId) { + // Why: admission starts a new renderer generation; buffered events + // that predate this boundary belong to the previous same-ID owner. + clearPreHandlerPtyState(options.sessionId) + } storedCallbacks = options.callbacks if (destroyed) { return