From ab6ce176f3fa6939ca2f5575d506170c073482c4 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:31:43 -0700 Subject: [PATCH] fix(terminal): do not unbind a live shell the new-terminal spawn adopted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the card is up the pane keeps its durable binding on purpose, so a failed replacement can put the card back. But main resolves a stable pane's owner from that same binding: if the shell answers again between the failed reattach and the click, the spawn adopts it and returns its id. Nothing was replaced, and clearing the binding then unbound a shell that is live and attached to this very pane — leaving it running with no durable record of what it owns. Detected by identity: an id equal to the one we were replacing means adoption, not replacement. Suppressing adoption outright would need a new spawn option plumbed renderer -> transport -> IPC; the user-visible oddity that remains is getting the old shell back rather than a new one, which is benign next to orphaning it. --- .../terminal-pane/pty-connection.test.ts | 46 +++++++++++++++++++ .../terminal-pane/pty-connection.ts | 7 +++ 2 files changed, 53 insertions(+) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 7d32949117f..1abe0f0715e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -9951,6 +9951,52 @@ describe('connectPanePty', () => { expect(spawnOptions[0]).not.toHaveProperty('launchConfig') }) + // The pane's durable binding still names the old shell while the card is up — deliberately, so a + // failed replacement can put the card back. Main resolves a stable pane's owner from that binding, + // so if the shell answers again between the failed reattach and this click, the spawn ADOPTS it + // and hands back its id. Nothing was replaced. Clearing the binding then would unbind a live + // shell that is attached to this very pane, leaving it with no durable record of what it owns. + it('keeps the binding when the new-terminal spawn adopts the old shell instead of replacing it', async () => { + const { connectPanePty } = await import('./pty-connection') + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState + const transport = createMockTransport('tab-pty') + transport.connect.mockImplementation(async (options: { sessionId?: string }) => { + if (options.sessionId) { + throw new Error('PTY "tab-pty" not found') + } + // Main adopted the pane's existing owner: same id back, no new shell. + return { id: 'tab-pty', isReattach: true } + }) + transportFactoryQueue.push(transport) + + const pane = createPane(1) + const manager = createManager(1) + const onPtyRecoveryState = vi.fn<(paneId: number, state: RecoveryStateProbe) => void>() + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'tab-pty' }, + onPtyRecoveryStateRef: { current: onPtyRecoveryState } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(30) + + const published = onPtyRecoveryState.mock.calls.find( + ([paneId, state]) => paneId === pane.id && state?.phase === 'disconnected' + )?.[1]?.unreachablePane + // Producer pin: without the card there is no action to invoke and the clauses below are vacuous. + expect(published).toBeDefined() + + published!.onStartNewTerminal() + await flushAsyncTicks(30) + + expect(deps.clearTabPtyId).not.toHaveBeenCalled() + expect(deps.clearExitedPanePtyLayoutBinding).not.toHaveBeenCalled() + }) + it('keeps ?25h in the live agent reattach reset when the snapshot leaves the cursor visible', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('tab-pty') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index cc70743a110..4528cb2e6ef 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5200,6 +5200,13 @@ export function connectPanePty( divertToUnreachableCard(sessionId) return } + if (freshPtyId === sessionId) { + // Nothing was replaced: the pane's durable binding still named this shell, and it + // answered again between the failed reattach and this click, so the spawn adopted + // it instead of creating one. Clearing below would unbind a shell that is live and + // now attached here — the pane would run with no durable record of what it owns. + return + } deps.clearExitedPanePtyLayoutBinding(pane.id, sessionId) deps.clearTabPtyId(deps.tabId, sessionId) })