fix(chat): harden reconnect lifecycle

This commit is contained in:
Brennan Benson
2026-07-17 17:13:38 -07:00
parent 795b600a1b
commit d6e5b01e3b
10 changed files with 232 additions and 28 deletions
@@ -147,6 +147,32 @@ describe('resolveNativeChatLeafRoute', () => {
).toEqual({ chatLeafId: 'stopped-agent', exitChat: false })
})
it('exits chat when the mounted agent has authoritatively returned to its shell', () => {
expect(
resolveNativeChatLeafRoute({
isChatViewMode: true,
chatLeafId: 'exited-agent',
activeLeafId: 'exited-agent',
chatLeafStillMounted: true,
activeLeafIsEligible: true,
chatLeafHasConfirmedAgentExit: true
})
).toEqual({ chatLeafId: null, exitChat: true })
})
it('moves chat to an eligible sibling after the owning agent exits', () => {
expect(
resolveNativeChatLeafRoute({
isChatViewMode: true,
chatLeafId: 'exited-agent',
activeLeafId: 'agent-sibling',
chatLeafStillMounted: true,
activeLeafIsEligible: true,
chatLeafHasConfirmedAgentExit: true
})
).toEqual({ chatLeafId: 'agent-sibling', exitChat: false })
})
it('attaches a tab-level chat request to the eligible active leaf', () => {
expect(
resolveNativeChatLeafRoute({
@@ -72,11 +72,12 @@ export function resolveNativeChatLeafRoute(args: {
activeLeafId: string | null
chatLeafStillMounted: boolean
activeLeafIsEligible: boolean
chatLeafHasConfirmedAgentExit?: boolean
}): NativeChatLeafRoute {
if (!args.isChatViewMode) {
return { chatLeafId: null, exitChat: false }
}
if (args.chatLeafId && args.chatLeafStillMounted) {
if (args.chatLeafId && args.chatLeafStillMounted && !args.chatLeafHasConfirmedAgentExit) {
// Why: agent/title evidence can disappear while local, SSH, or runtime
// transports reconnect. A mounted owning pane is not a terminal lifecycle
// event, so keep its chat surface until the pane itself is removed.
@@ -84,13 +85,16 @@ export function resolveNativeChatLeafRoute(args: {
}
// Manager hydration can briefly have no active pane; preserve the requested
// mode until a concrete leaf exists instead of toggling it off during mount.
if (!args.activeLeafId) {
if (!args.activeLeafId && !args.chatLeafHasConfirmedAgentExit) {
return { chatLeafId: args.chatLeafId, exitChat: false }
}
if (args.activeLeafIsEligible) {
if (
args.activeLeafIsEligible &&
(!args.chatLeafHasConfirmedAgentExit || args.activeLeafId !== args.chatLeafId)
) {
return { chatLeafId: args.activeLeafId, exitChat: false }
}
// Why: removing the chat-owning leaf must not move its composer onto a
// plain-shell sibling. Return the tab to terminal mode instead.
// Why: removing the owning leaf or confirming its agent exited must not leave
// the composer targeting a plain shell. Return the tab to terminal mode.
return { chatLeafId: null, exitChat: true }
}
@@ -103,7 +103,8 @@ import { shouldChatTakeOverMobileSurface } from '../native-chat/native-chat-send
import { canToggleNativeChat } from '../native-chat/native-chat-availability'
import {
nativeChatLaunchAgentForLeaf,
resolveNativeChatLeafRoute
resolveNativeChatLeafRoute,
type NativeChatLeafRoute
} from '../native-chat/native-chat-leaf-routing'
import { isNativeChatTranscriptLocalReadable } from '@/lib/native-chat-transcript-readability'
import { resolvePaneKeyForManager } from '@/lib/pane-manager/pane-key-resolution'
@@ -402,6 +403,7 @@ export default function TerminalPane({
} | null>(null)
const [quickCommandEditorOpen, setQuickCommandEditorOpen] = useState(false)
const [chatLeafId, setChatLeafId] = useState<string | null>(null)
const onAgentExitedRef = useRef<(leafId: string) => void>(() => {})
const [tabWideAgentHintLeafId, setTabWideAgentHintLeafId] = useState<string | null | undefined>(
undefined
)
@@ -784,6 +786,39 @@ export default function TerminalPane({
resolveTitleAgentForLeaf
]
)
const applyNativeChatLeafRoute = useCallback(
(route: NativeChatLeafRoute): void => {
if (route.chatLeafId !== chatLeafId) {
setChatLeafId(route.chatLeafId)
}
if (route.exitChat && unifiedTabId) {
// Why: event/effect replay must not flip terminal mode back to chat.
setTabViewMode(unifiedTabId, 'terminal')
}
},
[chatLeafId, setTabViewMode, unifiedTabId]
)
const handleConfirmedAgentExit = useCallback(
(leafId: string): void => {
if (leafId !== chatLeafId) {
return
}
const panes = managerRef.current?.getPanes() ?? []
const activeLeafId = managerRef.current?.getActivePane()?.leafId ?? null
applyNativeChatLeafRoute(
resolveNativeChatLeafRoute({
isChatViewMode,
chatLeafId,
activeLeafId,
chatLeafStillMounted: panes.some((pane) => pane.leafId === chatLeafId),
activeLeafIsEligible: isChatEligibleForLeaf(activeLeafId),
chatLeafHasConfirmedAgentExit: true
})
)
},
[applyNativeChatLeafRoute, chatLeafId, isChatEligibleForLeaf, isChatViewMode]
)
onAgentExitedRef.current = handleConfirmedAgentExit
const canToggleChatForLeaf = useCallback(
(leafId: string | null): boolean => {
// Scope the "always allow toggling back" rule to the leaf actually showing
@@ -1530,6 +1565,7 @@ export default function TerminalPane({
isActiveRef,
isVisibleRef,
onPtyExitRef,
onAgentExitedRef,
onPtyErrorRef,
clearTabPtyId,
consumeSuppressedPtyExit: useAppStore((store) => store.consumeSuppressedPtyExit),
@@ -1754,6 +1790,7 @@ export default function TerminalPane({
isActiveRef,
isVisibleRef,
onPtyExitRef,
onAgentExitedRef,
onPtyErrorRef,
clearTabPtyId,
consumeSuppressedPtyExit: useAppStore.getState().consumeSuppressedPtyExit,
@@ -1790,6 +1827,7 @@ export default function TerminalPane({
clearTerminalTabUnread,
clearTerminalPaneUnread,
showRestoredSessionBanner,
onAgentExitedRef,
onPtyExitRef,
setCacheTimerStartedAt,
setRuntimePaneTitle,
@@ -2989,21 +3027,14 @@ export default function TerminalPane({
chatLeafStillMounted,
activeLeafIsEligible: isChatEligibleForLeaf(activeLeafId)
})
if (route.chatLeafId !== chatLeafId) {
setChatLeafId(route.chatLeafId)
}
if (route.exitChat && unifiedTabId) {
// Why: effect replay must not flip terminal mode back to chat.
setTabViewMode(unifiedTabId, 'terminal')
}
applyNativeChatLeafRoute(route)
}, [
isChatViewMode,
chatLeafId,
activePane?.leafId,
chatLeafStillMounted,
isChatEligibleForLeaf,
unifiedTabId,
setTabViewMode
applyNativeChatLeafRoute,
isChatEligibleForLeaf
])
const chatPane =
isChatViewMode && chatLeafId
@@ -48,6 +48,7 @@ export type PtyConnectionDeps = {
isActiveRef: React.RefObject<boolean>
isVisibleRef: React.RefObject<boolean>
onPtyExitRef: React.RefObject<(ptyId: string) => void>
onAgentExitedRef: React.RefObject<(leafId: string) => void>
onPtyErrorRef?: React.RefObject<(paneId: number, message: string) => void>
clearTabPtyId: (tabId: string, ptyId: string) => void
consumeSuppressedPtyExit: (ptyId: string) => boolean
@@ -539,6 +539,7 @@ function createDeps(overrides: Record<string, unknown> = {}) {
isActiveRef: { current: true },
isVisibleRef: { current: true },
onPtyExitRef: { current: vi.fn() },
onAgentExitedRef: { current: vi.fn() },
onPtyErrorRef: { current: vi.fn() },
clearTabPtyId: vi.fn(),
consumeSuppressedPtyExit: vi.fn(() => false),
@@ -2532,6 +2533,30 @@ describe('connectPanePty', () => {
expect(manager.closePane).not.toHaveBeenCalled()
})
it('rebinds a provider replacement without granting fresh-spawn exit protection', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport('terminal-old')
transportFactoryQueue.push(transport)
const manager = createManager(1)
const deps = createDeps()
connectPanePty(createPane(1) as never, manager as never, deps as never)
const onPtyRebind = createdTransportOptions[0]?.onPtyRebind as
| ((ptyId: string) => void)
| undefined
const onPtyExit = createdTransportOptions[0]?.onPtyExit as ((ptyId: string) => void) | undefined
expect(onPtyRebind).toBeTypeOf('function')
expect(onPtyExit).toBeTypeOf('function')
onPtyRebind?.('terminal-reconnected')
onPtyExit?.('terminal-reconnected')
expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(1, 'terminal-reconnected')
expect(deps.updateTabPtyId).toHaveBeenCalledWith('tab-1', 'terminal-reconnected')
expect(deps.onPtyExitRef.current).toHaveBeenCalledWith('terminal-reconnected')
expect(manager.closePane).not.toHaveBeenCalled()
})
it('closes a split pane when an established PTY exits after output', async () => {
const { connectPanePty } = await import('./pty-connection')
const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null }
@@ -18179,6 +18204,7 @@ describe('connectPanePty', () => {
agentExitedHandler()
expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(makePaneKey('tab-1', LEAF_1), null)
expect(deps.onAgentExitedRef.current).toHaveBeenCalledWith(LEAF_1)
expect(mockStoreState.removeAgentStatus).not.toHaveBeenCalled()
})
@@ -2795,6 +2795,11 @@ export function connectPanePty(
// once the PTY exists, then let real hook events refine or complete it.
bindActivePanePty(ptyId, { seedInitialAgentStatus: true })
}
const onPtyRebind = (ptyId: string): void => {
// Why: provider handle rotation keeps the existing pane/session generation;
// treating it as a fresh spawn can strand a later real exit as a newborn.
bindActivePanePty(ptyId)
}
// ─── Attention signal: BEL ────────────────────────────────────────────
//
// BEL (0x07) is the attention signal. A BEL raises tab- and worktree-level
@@ -3083,6 +3088,9 @@ export function connectPanePty(
}
}
const onAgentExited = (): void => {
// Why: eligibility can disappear transiently during reconnect, but a
// confirmed shell-title transition is authoritative for native-chat exit.
deps.onAgentExitedRef.current(pane.leafId)
clearSuppressedTitleSideEffects()
clearCommandInferredPaneAgent()
requestKnownDroidReconfirmation()
@@ -3298,6 +3306,7 @@ export function connectPanePty(
...(paneStartup?.telemetry ? { telemetry: paneStartup.telemetry } : {}),
onPtyExit: onExit,
onPtySpawn,
onPtyRebind,
...(mainSideEffectAuthority
? {}
: {
@@ -157,6 +157,8 @@ export type IpcPtyTransportOptions = {
onPtyExit?: (ptyId: string) => void
onTitleChange?: (title: string, rawTitle: string) => void
onPtySpawn?: (ptyId: string) => void
/** Rebind an existing pane after its provider replaces the PTY identity. */
onPtyRebind?: (ptyId: string) => void
onBell?: () => void
onAgentBecameIdle?: (title: string) => void
onAgentBecameWorking?: () => void
@@ -306,11 +306,13 @@ describe('createRemoteRuntimePtyTransport', () => {
it('re-derives the host session handle after a transport close instead of resubscribing the stale one', async () => {
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
const onPtySpawn = vi.fn()
const onPtyRebind = vi.fn()
const transport = createRemoteRuntimePtyTransport('env-1', {
worktreeId: 'wt-1',
tabId: 'web-terminal-tab-1',
leafId: 'pane:1',
onPtySpawn
onPtySpawn,
onPtyRebind
})
transport.attach({
@@ -364,7 +366,8 @@ describe('createRemoteRuntimePtyTransport', () => {
expect(latestSubscribePayload()).toMatchObject({ terminal: 'terminal-2' })
)
expect(transport.getPtyId()).toContain('terminal-2')
expect(onPtySpawn).toHaveBeenCalledWith(expect.stringContaining('terminal-2'))
expect(onPtySpawn).not.toHaveBeenCalled()
expect(onPtyRebind).toHaveBeenCalledWith(expect.stringContaining('terminal-2'))
})
it('retires the mirror when the host no longer publishes the surface after a transport close', async () => {
@@ -463,6 +466,8 @@ describe('createRemoteRuntimePtyTransport', () => {
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
const onError = vi.fn()
const onPtyExit = vi.fn()
const onPtySpawn = vi.fn()
const onPtyRebind = vi.fn()
const onExit = vi.fn()
const onDisconnect = vi.fn()
const renderedScreen: string[] = []
@@ -470,7 +475,9 @@ describe('createRemoteRuntimePtyTransport', () => {
worktreeId: 'wt-1',
tabId: 'web-terminal-tab-1',
leafId: 'pane:1',
onPtyExit
onPtyExit,
onPtySpawn,
onPtyRebind
})
transport.attach({
@@ -543,6 +550,9 @@ describe('createRemoteRuntimePtyTransport', () => {
expect(onError).not.toHaveBeenCalled()
expect(onPtyExit).not.toHaveBeenCalled()
expect(onPtySpawn).not.toHaveBeenCalled()
expect(onPtyRebind).toHaveBeenCalledOnce()
expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-reconnected')
expect(onExit).not.toHaveBeenCalled()
expect(onDisconnect).not.toHaveBeenCalled()
expect(transport.getPtyId()).toBe('remote:env-1@@terminal-reconnected')
@@ -561,6 +571,88 @@ describe('createRemoteRuntimePtyTransport', () => {
expect(subscribedTerminals).toEqual(['terminal-stale', 'terminal-reconnected'])
})
it('coalesces concurrent stale errors for the handle that was replaced', async () => {
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
const onPtyExit = vi.fn()
const transport = createRemoteRuntimePtyTransport('env-1', {
worktreeId: 'wt-1',
tabId: 'web-terminal-tab-1',
leafId: 'pane:1',
onPtyExit
})
transport.attach({
existingPtyId: 'remote:env-1@@terminal-stale',
cols: 80,
rows: 24,
callbacks: {}
})
await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled())
let resolveHostList: (response: unknown) => void = () => {}
const hostListResponse = new Promise((resolve) => {
resolveHostList = resolve
})
let hostListCalls = 0
runtimeCall.mockImplementation((args: { method: string }) => {
if (args.method === 'terminal.send') {
return Promise.resolve({
ok: false,
error: { code: 'terminal_handle_stale', message: 'terminal_handle_stale' }
})
}
if (args.method === 'session.tabs.list') {
hostListCalls += 1
return hostListResponse
}
return Promise.resolve({ ok: true, result: {} })
})
const sendInputAccepted = transport.sendInputAccepted
if (!sendInputAccepted) {
throw new Error('Expected acknowledged remote terminal input')
}
const sends = Promise.all([sendInputAccepted('first'), sendInputAccepted('second')])
await vi.waitFor(() => expect(hostListCalls).toBe(1))
await expect(sends).resolves.toEqual([false, false])
resolveHostList({
ok: true,
result: {
worktree: 'wt-1',
publicationEpoch: 'epoch-1',
snapshotVersion: 2,
activeGroupId: null,
activeTabId: 'tab-1::pane:1',
activeTabType: 'terminal',
tabs: [
{
type: 'terminal',
id: 'tab-1::pane:1',
parentTabId: 'tab-1',
leafId: 'pane:1',
title: 'Claude Code',
isActive: true,
status: 'ready',
terminal: 'terminal-reconnected'
}
]
}
})
await vi.waitFor(() =>
expect(latestSubscribePayload()).toMatchObject({ terminal: 'terminal-reconnected' })
)
await Promise.resolve()
// Why: the second stale response belonged to terminal-stale. Replaying it
// against the replacement would add another polling loop and retire it.
expect(hostListCalls).toBe(1)
expect(onPtyExit).not.toHaveBeenCalled()
expect(transport.getPtyId()).toBe('remote:env-1@@terminal-reconnected')
expect(transport.isConnected()).toBe(true)
})
it('still retires the regular TUI surface after an explicit terminal exit', async () => {
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
const onPtyExit = vi.fn()
@@ -77,6 +77,7 @@ export function createRemoteRuntimePtyTransport(
activate,
onPtyExit,
onPtySpawn,
onPtyRebind,
onTitleChange,
onBell,
onAgentBecameIdle,
@@ -94,8 +95,8 @@ export function createRemoteRuntimePtyTransport(
let desiredViewport: { cols: number; rows: number } | null = null
let storedCallbacks: Parameters<PtyTransport['connect']>[0]['callbacks'] = {}
let resubscribing = false
let resubscribeRequested = false
let resubscribeRequiresReplacement = false
let resubscribeRequestedHandle: string | null = null
let resubscribeRequestedRequiresReplacement = false
let subscriptionGeneration = 0
let pendingViewportClaim = false
let pendingClaimInput = ''
@@ -504,7 +505,9 @@ export function createRemoteRuntimePtyTransport(
if (nextHandle !== previousHandle) {
handle = nextHandle
remotePtyId = toRemoteRuntimePtyId(nextHandle, currentRuntimeEnvironmentId)
onPtySpawn?.(remotePtyId)
// Why: the host replaced an existing mirror identity; rebinding must not
// apply fresh-spawn exit/agent seeding semantics to the mounted pane.
onPtyRebind?.(remotePtyId)
}
}
await subscribeToHandle()
@@ -515,8 +518,14 @@ export function createRemoteRuntimePtyTransport(
return
}
if (resubscribing) {
resubscribeRequested = true
resubscribeRequiresReplacement ||= requireReplacement
// Why: concurrent stale errors belong to the handle that produced them.
// Do not carry an old handle's replacement requirement onto its successor.
if (resubscribeRequestedHandle !== handle) {
resubscribeRequestedHandle = handle
resubscribeRequestedRequiresReplacement = requireReplacement
} else {
resubscribeRequestedRequiresReplacement ||= requireReplacement
}
return
}
resubscribing = true
@@ -530,10 +539,11 @@ export function createRemoteRuntimePtyTransport(
})
.finally(() => {
resubscribing = false
if (resubscribeRequested) {
resubscribeRequested = false
const pendingRequiresReplacement = resubscribeRequiresReplacement
resubscribeRequiresReplacement = false
const pendingHandle = resubscribeRequestedHandle
const pendingRequiresReplacement = resubscribeRequestedRequiresReplacement
resubscribeRequestedHandle = null
resubscribeRequestedRequiresReplacement = false
if (pendingHandle && pendingHandle === handle) {
scheduleResubscribeAfterTransportClose(pendingRequiresReplacement)
}
})
@@ -268,6 +268,7 @@ type UseTerminalPaneLifecycleDeps = {
isActiveRef: React.RefObject<boolean>
isVisibleRef: React.RefObject<boolean>
onPtyExitRef: React.RefObject<(ptyId: string) => void>
onAgentExitedRef: React.RefObject<(leafId: string) => void>
onPtyErrorRef?: React.RefObject<(paneId: number, message: string) => void>
clearTabPtyId: (tabId: string, ptyId: string) => void
consumeSuppressedPtyExit: (ptyId: string) => boolean
@@ -538,6 +539,7 @@ export function useTerminalPaneLifecycle({
isActiveRef,
isVisibleRef,
onPtyExitRef,
onAgentExitedRef,
onPtyErrorRef,
clearTabPtyId,
consumeSuppressedPtyExit,
@@ -757,6 +759,7 @@ export function useTerminalPaneLifecycle({
isActiveRef,
isVisibleRef,
onPtyExitRef,
onAgentExitedRef,
onPtyErrorRef,
clearTabPtyId,
consumeSuppressedPtyExit,