fix(terminal): do not unbind a live shell the new-terminal spawn adopted

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.
This commit is contained in:
Neil
2026-08-12 14:31:43 -07:00
parent 7589df8b34
commit ab6ce176f3
2 changed files with 53 additions and 0 deletions
@@ -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')
@@ -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)
})