From b4c8c65dc869506005cd0a073d6f1d19d2ea15d3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:00:15 -0700 Subject: [PATCH] fix(mobile): close terminal session tabs authoritatively --- .../app/h/[hostId]/session/[worktreeId].tsx | 1 + ...bile-terminal-action-sheet-actions.test.ts | 87 ++++++++++++++++--- .../mobile-terminal-action-sheet-actions.ts | 20 +++-- 3 files changed, 89 insertions(+), 19 deletions(-) diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 7e4ac17933f..32c615d1b70 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -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)} diff --git a/mobile/src/session/mobile-terminal-action-sheet-actions.test.ts b/mobile/src/session/mobile-terminal-action-sheet-actions.test.ts index c6cbc1188ba..b7a59307340 100644 --- a/mobile/src/session/mobile-terminal-action-sheet-actions.test.ts +++ b/mobile/src/session/mobile-terminal-action-sheet-actions.test.ts @@ -9,24 +9,38 @@ vi.mock('lucide-react-native', () => ({ SquareTerminal: vi.fn() })) +type SheetArgs = Parameters[0] + +function buildActions(overrides: Partial = {}) { + 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() + }) }) diff --git a/mobile/src/session/mobile-terminal-action-sheet-actions.ts b/mobile/src/session/mobile-terminal-action-sheet-actions.ts index fd5ef5a5583..9684bc3e7a5 100644 --- a/mobile/src/session/mobile-terminal-action-sheet-actions.ts +++ b/mobile/src/session/mobile-terminal-action-sheet-actions.ts @@ -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(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 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 tab.terminal === target.handle) return [ ...getMobileNativeChatToggleActions({ terminalHandle: target.handle, @@ -65,12 +72,13 @@ export function getMobileTerminalActionSheetActions { 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) ?? []) ] }