From c2564ba7a2487b5dcda8cba5080b0d8f8a6b62dd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 13 Jul 2026 17:53:02 -0700 Subject: [PATCH] fix(terminal): make pane recovery strictly best-effort in timer contexts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recovery fires from stall-watch timers, replay-guard releases, and onData — contexts where a throw becomes an unhandled error (CI verify caught this: pty-connection.test.ts mocks a partial store, other tests advance fake timers past the stall window, and the certification path hit a missing remountTerminalTabForRecovery). Guard the store action call and the ptyIdsByTabId reads so a partial surface yields a false return, never a throw, and pin it with a regression test. --- .../src/components/terminal-pane/pty-connection.ts | 4 ++-- .../terminal-pane/terminal-pane-recovery.test.ts | 13 +++++++++++++ .../terminal-pane/terminal-pane-recovery.ts | 10 +++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index d7b36d0f020..a7432fad614 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -3337,7 +3337,7 @@ export function connectPanePty( if (transport.isConnected() && transport.getPtyId() !== null) { return } - const storePtyId = useAppStore.getState().ptyIdsByTabId[deps.tabId]?.[0] ?? null + const storePtyId = useAppStore.getState().ptyIdsByTabId?.[deps.tabId]?.[0] ?? null void requestTerminalPaneRecovery({ tabId: deps.tabId, ptyId: transport.getPtyId() ?? storePtyId, @@ -3351,7 +3351,7 @@ export function connectPanePty( const unregisterUndeliverableWriteHandler = registerUndeliverableWriteHandler( pane.terminal, (reason) => { - const storePtyId = useAppStore.getState().ptyIdsByTabId[deps.tabId]?.[0] ?? null + const storePtyId = useAppStore.getState().ptyIdsByTabId?.[deps.tabId]?.[0] ?? null void requestTerminalPaneRecovery({ tabId: deps.tabId, ptyId: transport.getPtyId() ?? storePtyId, diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts index 8d1d511dd20..45eff4e4ffb 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts @@ -149,6 +149,19 @@ describe('requestTerminalPaneRecovery', () => { expect(mocks.remountTerminalTabForRecovery).not.toHaveBeenCalled() }) + it('never throws when the store surface is partial (timer/callback contexts)', async () => { + // Regression: recovery fires from stall-watch timers and write callbacks; + // an environment with a partial store (mocked suites, teardown races) must + // get a false return, not an unhandled TypeError. + mocks.remountTerminalTabForRecovery.mockImplementation(() => { + throw new TypeError('remountTerminalTabForRecovery is not a function') + }) + + await expect( + requestTerminalPaneRecovery({ tabId: 'tab-1', ptyId: 'pty-1', reason: 'write-stalled' }) + ).resolves.toBe(false) + }) + it('does not consume budget when the tab no longer exists', async () => { mocks.remountTerminalTabForRecovery.mockReturnValue(false) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts index d7b20d81e7c..65afa9fe006 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts @@ -78,7 +78,15 @@ export async function requestTerminalPaneRecovery(request: RecoveryRequest): Pro return false } } - const remounted = useAppStore.getState().remountTerminalTabForRecovery(request.tabId) + let remounted = false + try { + remounted = useAppStore.getState().remountTerminalTabForRecovery(request.tabId) + } catch { + // Why: recovery fires from timer and write-callback contexts (stall watch, + // replay guard, onData) — it is best-effort by contract and must never + // surface a throw there (partial store surfaces in tests, teardown races). + return false + } if (!remounted) { return false }