fix(terminal): guard PTY handler unregistration against detach/attach remount races (frozen pane) (#7894)

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/components/terminal-pane/pty-dispatcher.ts

@@ -246,8 +246,12 @@ export function registerEagerPtyBuffer(
   const exitHandler = (code: number): void => {
     // Shell died before TerminalPane attached — clean up and notify the store
     // so the tab's ptyId is cleared and connectPanePty falls through to connect().
-    ptyDataHandlers.delete(ptyId)
-    ptyReplayHandlers.delete(ptyId)
+    // Identity-guarded like dispose(): never delete a handler a transport has
+    // since registered for this id.
+    if (ptyDataHandlers.get(ptyId) === dataHandler) {
+      ptyDataHandlers.delete(ptyId)
+      ptyReplayHandlers.delete(ptyId)
+    }
     ptyExitHandlers.delete(ptyId)
     eagerPtyHandles.delete(ptyId)
     onExit(ptyId, code)

src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts

@@ -15,6 +15,11 @@ const preHandlerPtyExit = new Map<string, number>()
 // and dropping the first setup-script bytes.
 const PRE_HANDLER_PTY_DATA_MAX_BYTES = 512 * 1024
 const PRE_HANDLER_PTY_DATA_MAX_PTYS = 64
+// 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.
+const PRE_HANDLER_PTY_DATA_WARN_BYTES = 64 * 1024
+const warnedLostHandlerPtyIds = new Set<string>()
 
 export function bufferPreHandlerPtyData(ptyId: string, data: string, meta?: PtyDataMeta): void {
   const chunk = clampUtf8Tail(data, PRE_HANDLER_PTY_DATA_MAX_BYTES)
@@ -42,6 +47,13 @@ export function bufferPreHandlerPtyData(ptyId: string, data: string, meta?: PtyD
     totalBytes -= chunks.shift()?.bytes ?? 0
   }
   preHandlerPtyData.set(ptyId, chunks)
+  if (totalBytes > PRE_HANDLER_PTY_DATA_WARN_BYTES && !warnedLostHandlerPtyIds.has(ptyId)) {
+    warnedLostHandlerPtyIds.add(ptyId)
+    console.warn(
+      `[pty] ${ptyId}: ${totalBytes} bytes buffered with no registered data handler; ` +
+        'the owning pane may have lost its handler to a detach/attach race'
+    )
+  }
 }
 
 export function drainPreHandlerPtyData(
@@ -49,6 +61,7 @@ export function drainPreHandlerPtyData(
   handler: (data: string, meta?: PtyDataMeta) => void
 ): void {
   const chunks = preHandlerPtyData.get(ptyId)
+  warnedLostHandlerPtyIds.delete(ptyId)
   if (!chunks) {
     return
   }
@@ -73,9 +86,11 @@ export function drainPreHandlerPtyExit(ptyId: string, handler: (code: number) =>
 
 export function clearPreHandlerPtyData(ptyId: string): void {
   preHandlerPtyData.delete(ptyId)
+  warnedLostHandlerPtyIds.delete(ptyId)
 }
 
 export function clearPreHandlerPtyState(ptyId: string): void {
   preHandlerPtyData.delete(ptyId)
   preHandlerPtyExit.delete(ptyId)
+  warnedLostHandlerPtyIds.delete(ptyId)
 }

src/renderer/src/components/terminal-pane/pty-transport.test.ts

@@ -137,6 +137,65 @@ describe('createIpcPtyTransport', () => {
     expect(onReplayData).toHaveBeenCalledWith('new replay')
   })
 
+  it('keeps the live handler when detach() runs after a newer transport attached to the same PTY', async () => {
+    // Why: pane->tab detach and split-group moves rehome the React subtree, so
+    // the NEW TerminalPane can attach to the same ptyId BEFORE the old pane's
+    // unmount detach() runs. An unconditional unregister deletes the live
+    // handler and the pane freezes with the PTY still alive (frozen-pane bug).
+    const { createIpcPtyTransport } = await import('./pty-transport')
+    const receivedByNewPane = vi.fn()
+    const replayedToNewPane = vi.fn()
+    const exitSeenByNewPane = vi.fn()
+    const receivedByOldPane = vi.fn()
+
+    const oldPane = createIpcPtyTransport({})
+    await oldPane.connect({ url: '', callbacks: { onData: receivedByOldPane } })
+
+    const newPane = createIpcPtyTransport({})
+    newPane.attach?.({
+      existingPtyId: 'pty-1',
+      callbacks: {
+        onData: receivedByNewPane,
+        onReplayData: replayedToNewPane,
+        onExit: exitSeenByNewPane
+      }
+    })
+    oldPane.detach?.()
+
+    onData?.({ id: 'pty-1', data: 'live output' })
+    onReplay?.({ id: 'pty-1', data: 'replay output' })
+
+    expect(receivedByNewPane).toHaveBeenCalledWith('live output')
+    expect(replayedToNewPane).toHaveBeenCalledWith('replay output')
+    expect(receivedByOldPane).not.toHaveBeenCalled()
+
+    onExit?.({ id: 'pty-1', code: 0 })
+    expect(exitSeenByNewPane).toHaveBeenCalledWith(0)
+  })
+
+  it('buffers data across a normal detach-then-attach gap and drains it to the next pane', async () => {
+    const { createIpcPtyTransport } = await import('./pty-transport')
+    const receivedByNewPane = vi.fn()
+
+    const oldPane = createIpcPtyTransport({})
+    await oldPane.connect({ url: '', callbacks: { onData: vi.fn() } })
+    oldPane.detach?.()
+
+    onData?.({ id: 'pty-1', data: 'buffered while detached' })
+    expect(receivedByNewPane).not.toHaveBeenCalled()
+
+    const newPane = createIpcPtyTransport({})
+    newPane.attach?.({
+      existingPtyId: 'pty-1',
+      callbacks: { onData: receivedByNewPane }
+    })
+
+    expect(receivedByNewPane).toHaveBeenCalledWith('buffered while detached')
+
+    onData?.({ id: 'pty-1', data: 'live after reattach' })
+    expect(receivedByNewPane).toHaveBeenCalledWith('live after reattach')
+  })
+
   it('exposes the connection identity captured at transport creation', async () => {
     const { createIpcPtyTransport } = await import('./pty-transport')
 

src/renderer/src/components/terminal-pane/pty-transport.ts

@@ -502,16 +502,41 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
   })
   let storedCallbacks: Parameters<PtyTransport['connect']>[0]['callbacks'] = {}
 
+  // Why: pane->tab detach / split-group moves rehome the React subtree, so a
+  // NEW TerminalPane can attach to the same ptyId before the OLD instance's
+  // detach() runs. Track the handlers THIS instance registered so unregister
+  // paths only delete map entries they still own — an unconditional delete
+  // destroys the live handler and the pane freezes (data diverts into the
+  // pre-handler buffer forever).
+  const ownedDataAndReplayHandlers = new Map<
+    string,
+    { data: (data: string, meta?: PtyDataMeta) => void; replay: (data: string) => void }
+  >()
+  const ownedExitHandlers = new Map<string, (code: number) => void>()
+
   function unregisterPtyHandlers(id: string): void {
-    ptyDataHandlers.delete(id)
-    ptyReplayHandlers.delete(id)
-    ptyExitHandlers.delete(id)
-    ptyTeardownHandlers.delete(id)
+    unregisterPtyDataAndStatusHandlers(id)
+    const ownedExit = ownedExitHandlers.get(id)
+    if (ownedExit && ptyExitHandlers.get(id) === ownedExit) {
+      ptyExitHandlers.delete(id)
+    }
+    ownedExitHandlers.delete(id)
+    if (ptyTeardownHandlers.get(id) === clearAccumulatedState) {
+      ptyTeardownHandlers.delete(id)
+    }
   }
 
   function unregisterPtyDataAndStatusHandlers(id: string): void {
-    ptyDataHandlers.delete(id)
-    ptyReplayHandlers.delete(id)
+    const owned = ownedDataAndReplayHandlers.get(id)
+    if (owned) {
+      if (ptyDataHandlers.get(id) === owned.data) {
+        ptyDataHandlers.delete(id)
+      }
+      if (ptyReplayHandlers.get(id) === owned.replay) {
+        ptyReplayHandlers.delete(id)
+      }
+    }
+    ownedDataAndReplayHandlers.delete(id)
   }
 
   // Why: shared by connect() and attach() to avoid duplicating title/bell/exit
@@ -520,7 +545,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
     // Why: relay pty.attach sends replay data via a dedicated pty:replay IPC
     // channel. Route it through onReplayData so the renderer engages the
     // replay guard and xterm auto-replies do not leak into the shell.
-    ptyReplayHandlers.set(id, (data) => {
+    const replayHandler = (data: string): void => {
       if (ptyId !== id) {
         return
       }
@@ -529,7 +554,8 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
       } else {
         storedCallbacks.onData?.(data)
       }
-    })
+    }
+    ptyReplayHandlers.set(id, replayHandler)
     const dataHandler = (data: string, meta?: PtyDataMeta): void => {
       if (ptyId !== id) {
         return
@@ -544,6 +570,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
       )
     }
     ptyDataHandlers.set(id, dataHandler)
+    ownedDataAndReplayHandlers.set(id, { data: dataHandler, replay: replayHandler })
     drainPreHandlerPtyData(id, dataHandler)
   }
 
@@ -599,6 +626,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
       onPtyExit?.(id)
     }
     ptyExitHandlers.set(id, exitHandler)
+    ownedExitHandlers.set(id, exitHandler)
     // Why: shutdownWorktreeTerminals bypasses the transport layer — it
     // kills PTYs directly via IPC without calling disconnect()/destroy().
     // This teardown callback lets unregisterPtyDataHandlers cancel