From f9b5691e507f065813a5c63ebffdbc5b2145a7f8 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 24 Aug 2026 15:00:15 -0700 Subject: [PATCH] Restore previously-active tab when closing a browser tab When closing a browser tab, announce the MRU page selection before guest teardown to prevent fallback to registration order. Reorder closeBrowserTab() before destroyWorkspaceWebviews() consistently across all close-tab handlers. --- src/renderer/src/components/Terminal.tsx | 8 +++-- .../FloatingTerminalPanel.tsx | 4 +-- .../tab-group/useTabGroupTabCloseCommands.ts | 4 +-- src/renderer/src/store/slices/browser.test.ts | 15 ++++++++ src/renderer/src/store/slices/browser.ts | 35 ++++++++++++++++++- 5 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 05f62faa0a9..a05e006df5f 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -1777,8 +1777,9 @@ function Terminal(): React.JSX.Element | null { const hasUnifiedEntry = Object.values(state.unifiedTabsByWorktree).some((tabs) => tabs.some((tab) => tab.contentType === 'browser' && tab.entityId === tabId) ) - destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) closeBrowserTab(tabId) + // closeBrowserTab announces the MRU target before guest teardown can trigger bridge fallback. + destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) if (!hasUnifiedEntry && state.activeWorktreeId === owningWorktreeId) { const worktreeFile = state.openFiles.find((file) => file.worktreeId === owningWorktreeId) if (worktreeFile) { @@ -1796,8 +1797,9 @@ function Terminal(): React.JSX.Element | null { } return } - destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) closeBrowserTab(tabId) + // closeBrowserTab announces the MRU target before guest teardown can trigger bridge fallback. + destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) }, [closeBrowserTab, setActiveFile, setActiveTab, setActiveTabType, setActiveWorktree] ) @@ -1868,8 +1870,8 @@ function Terminal(): React.JSX.Element | null { } else if ( (state.browserTabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id) ) { - destroyWorkspaceWebviews(state.browserPagesByWorkspace, id) closeBrowserTab(id) + destroyWorkspaceWebviews(state.browserPagesByWorkspace, id) } else if (unifiedTab?.contentType === 'simulator') { // Why: simulator tabs live only in the unified-tab store, so the // entity-store checks above never match them. diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index f37a6968c36..ca6b25f6e07 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -896,8 +896,8 @@ export function FloatingTerminalPanel({ if (item.contentType === 'terminal') { closeTab(item.entityId, { reason: 'cleanup' }) } else if (item.contentType === 'browser') { - destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId) closeBrowserTab(item.entityId) + destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId) } else if (item.contentType === 'simulator') { closeUnifiedTab(item.id) } else { @@ -952,8 +952,8 @@ export function FloatingTerminalPanel({ onClose: () => { const latest = useAppStore.getState() if (item.contentType === 'browser') { - destroyWorkspaceWebviews(latest.browserPagesByWorkspace, item.entityId) closeBrowserTab(item.entityId) + destroyWorkspaceWebviews(latest.browserPagesByWorkspace, item.entityId) } else if (item.contentType === 'simulator') { closeUnifiedTab(item.id) } else { diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts index 9b6e85c2fcc..e2d90290043 100644 --- a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts @@ -99,8 +99,8 @@ export function useTabGroupTabCloseCommands({ reason: 'user' }) } - destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId) closeBrowserTab(item.entityId) + destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId) closeUnifiedTab(item.id) } else if (item.contentType === 'simulator') { closeUnifiedTab(item.id) @@ -159,8 +159,8 @@ export function useTabGroupTabCloseCommands({ reason: 'user' }) } - destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId) closeBrowserTab(item.entityId) + destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId) closeUnifiedTab(item.id) } else if (item.contentType === 'terminal') { closeTab(item.entityId) diff --git a/src/renderer/src/store/slices/browser.test.ts b/src/renderer/src/store/slices/browser.test.ts index 344def679d3..903e192d2f3 100644 --- a/src/renderer/src/store/slices/browser.test.ts +++ b/src/renderer/src/store/slices/browser.test.ts @@ -105,6 +105,21 @@ function makeAnnotation(pageId: string, id = 'annotation-1'): BrowserPageAnnotat } describe('createBrowserSlice annotations', () => { + it('announces the store-selected browser page before its guest is destroyed', () => { + const store = createTestStore() + const previous = store.getState().createBrowserTab('wt-1', 'https://previous.example.com') + const closing = store.getState().createBrowserTab('wt-1', 'https://closing.example.com') + store.getState().setActiveBrowserTab(previous.id) + store.getState().setActiveBrowserTab(closing.id) + mockApi.browser.notifyActiveTabChanged.mockClear() + + store.getState().closeBrowserTab(closing.id) + + expect(mockApi.browser.notifyActiveTabChanged).toHaveBeenCalledWith({ + browserPageId: previous.activePageId + }) + }) + it('keeps a requested canonical page identity distinct from its workspace', () => { const store = createTestStore() const tab = store.getState().createBrowserTab('wt-1', 'about:blank', { diff --git a/src/renderer/src/store/slices/browser.ts b/src/renderer/src/store/slices/browser.ts index 231ebfd916a..71b02e00831 100644 --- a/src/renderer/src/store/slices/browser.ts +++ b/src/renderer/src/store/slices/browser.ts @@ -805,6 +805,7 @@ export const createBrowserSlice: StateCreator = }, closeBrowserTab: (tabId) => { let remotePagesToClose: { worktreeId: string; handle: RemoteBrowserPageHandle }[] = [] + let activeBrowserWorktreeIdToNotify: string | null = null set((s) => { let owningWorktreeId: string | null = null let closedWorkspace: BrowserWorkspace | null = null @@ -865,6 +866,9 @@ export const createBrowserSlice: StateCreator = const isActiveTabInOwningWorktree = s.activeWorktreeId === owningWorktreeId && s.activeBrowserTabId === tabId + if (isActiveTabInOwningWorktree) { + activeBrowserWorktreeIdToNotify = owningWorktreeId + } const nextActiveTabTypeByWorktree = { ...s.activeTabTypeByWorktree } let nextActiveTabType = s.activeTabType if (remainingBrowserTabs.length === 0) { @@ -946,6 +950,34 @@ export const createBrowserSlice: StateCreator = get().closeUnifiedTab(workspaceItem.id) } } + + // Why: announce the MRU page before guest teardown so bridge fallback cannot choose registration order. + if (activeBrowserWorktreeIdToNotify) { + const state = get() + const activeWorkspaceId = state.activeBrowserTabIdByWorktree[activeBrowserWorktreeIdToNotify] + const activeWorkspace = activeWorkspaceId + ? findWorkspace(state.browserTabsByWorktree, activeWorkspaceId) + : null + const activePage = activeWorkspace?.activePageId + ? (state.browserPagesByWorkspace[activeWorkspace.id] ?? []).find( + (page) => page.id === activeWorkspace.activePageId + ) + : undefined + if ( + activeWorkspace?.activePageId && + isLocalBrowserPageOwner( + state, + activeBrowserWorktreeIdToNotify, + activePage?.browserRuntimeEnvironmentId + ) && + typeof window !== 'undefined' && + window.api?.browser + ) { + window.api.browser + .notifyActiveTabChanged({ browserPageId: activeWorkspace.activePageId }) + .catch(() => {}) + } + } }, shutdownWorktreeBrowsers: async (worktreeId) => { @@ -953,8 +985,9 @@ export const createBrowserSlice: StateCreator = // Why: snapshot before the loop — closeBrowserTab empties the array, so set() below couldn't recompute hadBrowserTabs. const hadBrowserTabs = workspaces.length > 0 for (const workspace of workspaces) { - destroyWorkspaceWebviews(get().browserPagesByWorkspace, workspace.id) + const browserPagesByWorkspace = get().browserPagesByWorkspace get().closeBrowserTab(workspace.id) + destroyWorkspaceWebviews(browserPagesByWorkspace, workspace.id) } set((s) => { const nextBrowserTabsByWorktree = { ...s.browserTabsByWorktree }