fix(mobile): close terminal session tabs authoritatively

This commit is contained in:
Brennan Benson
2026-07-28 16:07:05 -07:00
parent d3681f6306
commit b4c8c65dc8
3 changed files with 89 additions and 19 deletions
@@ -5216,6 +5216,7 @@ export default function SessionScreen() {
onRename: setRenameTarget,
onClear: (target) => void handleClearTerminal(target),
onClose: (target) => void handleCloseTerminal(target),
onCloseSessionTab: (tab) => void handleCloseSessionTab(tab),
bulkCloseActions
})}
onClose={() => setActionTarget(null)}
@@ -9,24 +9,38 @@ vi.mock('lucide-react-native', () => ({
SquareTerminal: vi.fn()
}))
type SheetArgs = Parameters<typeof getMobileTerminalActionSheetActions>[0]
function buildActions(overrides: Partial<SheetArgs> = {}) {
return getMobileTerminalActionSheetActions({
target: { handle: 'terminal-1' },
tabs: [],
isTabChatView: () => false,
nativeChatTranscriptIsLocalReadable: true,
onDismiss: vi.fn(),
onToggleChat: vi.fn(),
isPhoneMode: () => false,
onToggleDisplayMode: vi.fn(),
onRename: vi.fn(),
onClear: vi.fn(),
onClose: vi.fn(),
onCloseSessionTab: vi.fn(),
...overrides
})
}
const terminalTab = (id: string, handle: string | null) => ({
type: 'terminal',
id,
terminal: handle
})
describe('getMobileTerminalActionSheetActions', () => {
it('defers Rename until after the action sheet closes', () => {
const target = { handle: 'terminal-1' }
const onDismiss = vi.fn()
const onRename = vi.fn()
const actions = getMobileTerminalActionSheetActions({
target,
tabs: [],
isTabChatView: () => false,
nativeChatTranscriptIsLocalReadable: true,
onDismiss,
onToggleChat: vi.fn(),
isPhoneMode: () => false,
onToggleDisplayMode: vi.fn(),
onRename,
onClear: vi.fn(),
onClose: vi.fn()
})
const actions = buildActions({ target, onDismiss, onRename })
const rename = actions.find((action) => action.label === 'Rename')
expect(rename).toMatchObject({ closeBeforePress: true })
@@ -35,4 +49,51 @@ describe('getMobileTerminalActionSheetActions', () => {
expect(onRename).toHaveBeenCalledWith(target)
expect(onDismiss).not.toHaveBeenCalled()
})
// Why: handle-only close lets the pending-terminal effect resurrect the tab (#6927, #7345).
it('closes the handle through its session tab, not the handle-only path', () => {
const onClose = vi.fn()
const onCloseSessionTab = vi.fn()
const tab = terminalTab('tab-1::leaf-1', 'terminal-1')
const actions = buildActions({
target: { handle: 'terminal-1' },
tabs: [tab],
onClose,
onCloseSessionTab
})
actions.find((action) => action.label === 'Close')?.onPress()
expect(onCloseSessionTab).toHaveBeenCalledWith(tab)
expect(onClose).not.toHaveBeenCalled()
})
it('closes the split leaf that owns the handle, not its sibling', () => {
const onCloseSessionTab = vi.fn()
const first = terminalTab('tab-1::leaf-1', 'terminal-1')
const second = terminalTab('tab-1::leaf-2', 'terminal-2')
const actions = buildActions({
target: { handle: 'terminal-2' },
tabs: [first, second],
onCloseSessionTab
})
actions.find((action) => action.label === 'Close')?.onPress()
expect(onCloseSessionTab).toHaveBeenCalledWith(second)
})
it('falls back to the handle close when no session tab owns the handle', () => {
const onClose = vi.fn()
const onCloseSessionTab = vi.fn()
const target = { handle: 'terminal-9' }
const actions = buildActions({
target,
tabs: [terminalTab('tab-1::leaf-1', 'terminal-1'), terminalTab('tab-2::leaf-1', null)],
onClose,
onCloseSessionTab
})
actions.find((action) => action.label === 'Close')?.onPress()
expect(onClose).toHaveBeenCalledWith(target)
expect(onCloseSessionTab).not.toHaveBeenCalled()
})
})
@@ -7,9 +7,12 @@ type TerminalTab = MobileNativeChatTab & { id: string; terminal: string | null }
/** Builds the terminal long-press menu without adding another action block to the
* already dense session route. Native chat stays first as the view switch. */
export function getMobileTerminalActionSheetActions<Target extends { handle: string }>(args: {
export function getMobileTerminalActionSheetActions<
Target extends { handle: string },
Tab extends TerminalTab
>(args: {
target: Target | null
tabs: readonly TerminalTab[]
tabs: readonly Tab[]
isTabChatView: (tabId: string) => boolean
nativeChatTranscriptIsLocalReadable: boolean
onDismiss: () => void
@@ -18,7 +21,10 @@ export function getMobileTerminalActionSheetActions<Target extends { handle: str
onToggleDisplayMode: (handle: string) => void
onRename: (target: Target) => void
onClear: (target: Target) => void
/** Fallback for a live handle with no matching session tab. */
onClose: (target: Target) => void
/** Preferred path runs host teardown and records the local tombstone. */
onCloseSessionTab: (tab: Tab) => void
/** Appended after Close; receives the pressed tab's id so the session route's
* bulk-close builder can resolve the anchor itself. */
bulkCloseActions?: (anchorTabId: string | undefined, dismiss: () => void) => ActionSheetAction[]
@@ -28,6 +34,7 @@ export function getMobileTerminalActionSheetActions<Target extends { handle: str
return []
}
const phoneMode = args.isPhoneMode(target.handle)
const sessionTab = args.tabs.find((tab) => tab.terminal === target.handle)
return [
...getMobileNativeChatToggleActions({
terminalHandle: target.handle,
@@ -65,12 +72,13 @@ export function getMobileTerminalActionSheetActions<Target extends { handle: str
destructive: true,
onPress: () => {
args.onDismiss()
if (sessionTab) {
args.onCloseSessionTab(sessionTab)
return
}
args.onClose(target)
}
},
...(args.bulkCloseActions?.(
args.tabs.find((tab) => tab.terminal === target.handle)?.id,
args.onDismiss
) ?? [])
...(args.bulkCloseActions?.(sessionTab?.id, args.onDismiss) ?? [])
]
}