From c84a151d581319e2c3c97c3e1d194dc4b8c48aa9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 11 Sep 2026 17:10:05 -0700 Subject: [PATCH 1/5] fix(desktop): reopen a chat on the browser tab the user left it on The resource strip pushed its own last-tab fallback onto the desktop app whenever a chat opened without an explicit selection, overriding the tab the desktop remembers the user was on. The shared desktop-tab hook now switches the native tab only for an explicit selection, adopts the desktop's active tab when the strip is on its fallback, and defers a selected tab that has not landed yet until it does. Chat hydration no longer writes a browser or terminal tab into the URL as a fallback. --- .../app/workspace/[workspaceId]/home/home.tsx | 3 + .../hooks/use-browser-tab-resources.test.tsx | 85 ++++++++++++++++++- .../home/hooks/use-browser-tab-resources.ts | 5 ++ .../[workspaceId]/home/hooks/use-chat.ts | 18 ++-- .../home/hooks/use-desktop-tab-resources.ts | 66 ++++++++++++-- .../hooks/use-terminal-tab-resources.test.tsx | 39 ++++++++- .../home/hooks/use-terminal-tab-resources.ts | 5 ++ 7 files changed, 195 insertions(+), 26 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index c169af084e8..51f0c9ad548 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -338,18 +338,21 @@ export function Home({ chatId, userName, userId }: HomeProps) { addResource, removeResource, selectResource: selectResourceFromUser, + restoreResource: setActiveResourceId, onResourceEvent: handleResourceEvent, } useBrowserTabResources({ scopeId: desktopScopeId, resources, activeResourceId, + selectedResourceId: activeResourceParam, ...desktopTabResourceCallbacks, }) useTerminalTabResources({ scopeId: desktopScopeId, resources, activeResourceId, + selectedResourceId: activeResourceParam, ...desktopTabResourceCallbacks, }) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx index 2563e158eac..d18d39c6811 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx @@ -41,9 +41,11 @@ interface HostProps { scopeId: string resources: MothershipResource[] activeResourceId: string | null + selectedResourceId: string | null addResource: (resource: MothershipResource) => void removeResource: (type: MothershipResource['type'], id: string) => void selectResource: (id: string) => void + restoreResource: (id: string) => void onResourceEvent: (id: string, options?: { activate?: boolean }) => void } @@ -58,6 +60,7 @@ describe('useBrowserTabResources', () => { const addResource = vi.fn() const removeResource = vi.fn() const selectResource = vi.fn() + const restoreResource = vi.fn() const onResourceEvent = vi.fn() function render(overrides: Partial = {}) { @@ -65,9 +68,11 @@ describe('useBrowserTabResources', () => { scopeId: SCOPE, resources: [], activeResourceId: null, + selectedResourceId: null, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, ...overrides, } @@ -153,11 +158,82 @@ describe('useBrowserTabResources', () => { { type: 'browser', id: '1', title: 'Page 1' }, { type: 'browser', id: '2', title: 'Page 2' }, ] - const rerender = render({ resources, activeResourceId: '1' }) + const rerender = render({ resources, activeResourceId: '1', selectedResourceId: '1' }) pushTabs(SCOPE, [tab('1', true), tab('2')], '1') expect(sendBrowserPanelAction).not.toHaveBeenCalled() - rerender({ activeResourceId: '2' }) + rerender({ activeResourceId: '2', selectedResourceId: '2' }) + expect(sendBrowserPanelAction).toHaveBeenCalledExactlyOnceWith( + 'switch-tab', + { tabId: '2', claim: false }, + SCOPE + ) + + // The requested switch landing is not a native change to follow. + pushTabs(SCOPE, [tab('1'), tab('2', true)], '2') + expect(selectResource).not.toHaveBeenCalled() + }) + + it('adopts the native active page on reopen instead of pushing the fallback tab', () => { + const resources: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + { type: 'browser', id: '3', title: 'Page 3' }, + ] + // The user left this chat on page 2. On reopen the strip starts empty, the + // pages land, and it falls back to its last tab until it learns better. + const rerender = render() + pushTabs(SCOPE, [tab('1'), tab('2', true), tab('3')], '2') + rerender({ resources, activeResourceId: '3', selectedResourceId: null }) + + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + expect(restoreResource).toHaveBeenCalledExactlyOnceWith('2') + expect(selectResource).not.toHaveBeenCalled() + + // The adopted tab is now both the selection and the native page: settled. + restoreResource.mockClear() + rerender({ activeResourceId: '2', selectedResourceId: '2' }) + expect(restoreResource).not.toHaveBeenCalled() + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + }) + + it('adopts the native active page when the selection is stale', () => { + const rerender = render({ selectedResourceId: 'deleted-file' }) + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + rerender({ + resources: [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + ], + activeResourceId: '2', + selectedResourceId: 'deleted-file', + }) + + expect(restoreResource).toHaveBeenCalledExactlyOnceWith('1') + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + }) + + it('leaves a fallback that is not a browser tab alone', () => { + const rerender = render() + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + rerender({ + resources: [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'file', id: 'f', title: 'notes.md' }, + ], + activeResourceId: 'f', + selectedResourceId: null, + }) + + expect(restoreResource).not.toHaveBeenCalled() + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + }) + + it('switches to a selected page once it lands, as after a reload with the tab in the URL', () => { + render({ selectedResourceId: '2' }) + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') expect(sendBrowserPanelAction).toHaveBeenCalledExactlyOnceWith( 'switch-tab', { tabId: '2', claim: false }, @@ -175,7 +251,7 @@ describe('useBrowserTabResources', () => { { type: 'browser', id: '2', title: 'Page 2' }, { type: 'file', id: 'f', title: 'notes.md' }, ] - const rerender = render({ resources, activeResourceId: '1' }) + const rerender = render({ resources, activeResourceId: '1', selectedResourceId: '1' }) pushTabs(SCOPE, [tab('1', true), tab('2')], '1') pushTabs(SCOPE, [tab('1'), tab('2', true)], '2') @@ -183,7 +259,7 @@ describe('useBrowserTabResources', () => { expect(sendBrowserPanelAction).not.toHaveBeenCalled() selectResource.mockClear() - rerender({ activeResourceId: 'f' }) + rerender({ activeResourceId: 'f', selectedResourceId: 'f' }) pushTabs(SCOPE, [tab('1', true), tab('2')], '1') expect(selectResource).not.toHaveBeenCalled() }) @@ -192,6 +268,7 @@ describe('useBrowserTabResources', () => { render({ resources: [{ type: 'browser', id: '1', title: 'Page 1' }], activeResourceId: '1', + selectedResourceId: '1', }) pushTabs(SCOPE, [tab('1', true)], '1') act(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts index 51885bd161f..c3155eeaa0f 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts @@ -21,6 +21,7 @@ interface UseBrowserTabResourcesOptions extends DesktopTabResourceCallbacks { scopeId: string resources: readonly MothershipResource[] activeResourceId: string | null + selectedResourceId: string | null } function switchBrowserTab(tabId: string, scopeId: string): void { @@ -35,9 +36,11 @@ export function useBrowserTabResources({ scopeId, resources, activeResourceId, + selectedResourceId, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, }: UseBrowserTabResourcesOptions): void { const hasSession = useBrowserSessionStore((state) => state.sessions[scopeId] !== undefined) @@ -73,9 +76,11 @@ export function useBrowserTabResources({ switchTab: switchBrowserTab, resources, activeResourceId, + selectedResourceId, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, }) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index 0ac26689ac1..57e951dc159 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -2476,22 +2476,22 @@ export function useChat( ) if (mergedResources.length > 0) { - // An explicit selection wins. Otherwise fall back to the last resource - // the server holds, not the last on screen: local-only browser tabs can - // land before the history does, and which side arrives first must not - // decide which tab the chat opens on. + // An explicit selection wins. Otherwise pin the last resource the server + // holds, not the last on screen: local-only browser tabs can land before + // the history does, and which side arrives first must not decide which + // tab the chat opens on. When the server holds nothing, hydration writes + // no fallback: the desktop app remembers which of its tabs the user was + // on, and the desktop tab hooks adopt that tab instead of the last one. const selectedResourceId = selectedResourceIdRef.current const hydratedActiveResourceId = selectedResourceId && mergedResources.some((resource) => resource.id === selectedResourceId) ? selectedResourceId - : ( - restorableResources[restorableResources.length - 1] ?? - mergedResources[mergedResources.length - 1] - ).id + : (restorableResources[restorableResources.length - 1]?.id ?? null) // Replacing the array with an identical one still re-renders the tab // strip and panel — skip the no-op so open panels don't flash. if (!resourcesUnchanged) { - activeResourceIdRef.current = hydratedActiveResourceId + activeResourceIdRef.current = + hydratedActiveResourceId ?? mergedResources[mergedResources.length - 1].id setResources(mergedResources) setActiveResourceId(hydratedActiveResourceId) } diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts index 865ad133f15..553d8076928 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts @@ -14,6 +14,12 @@ export interface DesktopTabResourceCallbacks { removeResource: (resourceType: MothershipResourceType, resourceId: string) => void /** Explicit user selection, which claims the strip's selection for the user. */ selectResource: (resourceId: string) => void + /** + * Adopts the desktop app's remembered tab as the shown resource without + * claiming the selection for the user, so agent activity can still take the + * view the way it does on any chat open. + */ + restoreResource: (resourceId: string) => void /** Agent activity on a tab, subject to the panel's user-ownership policy. */ onResourceEvent: ResourceEventHandler } @@ -37,7 +43,10 @@ interface UseDesktopTabResourcesOptions extends DesktopTabResourceCallbacks { /** Shows a tab natively without claiming it for the user. */ switchTab: (tabId: string, scopeId: string) => void resources: readonly MothershipResource[] + /** The resource the strip shows: the explicit selection or its fallback. */ activeResourceId: string | null + /** The explicit selection alone, without the strip's fallback. */ + selectedResourceId: string | null } /** @@ -49,7 +58,10 @@ interface UseDesktopTabResourcesOptions extends DesktopTabResourceCallbacks { * a resource tab closes its native tab at the strip, which then comes back * through the same list. Visible selection is routed the same way — choosing * a resource tab switches the native tab, and a native switch follows into the - * strip while the user is on that kind of tab. + * strip while the user is on that kind of tab. Without an explicit selection + * the desktop app's own active tab wins: it remembers the tab the user left a + * chat on, so reopening the chat lands there instead of on the strip's + * last-tab fallback. * * The agent never moves the visible tab itself. Its tab is announced as * resource activity, so the existing view policy decides whether to show it or @@ -65,9 +77,11 @@ export function useDesktopTabResources({ switchTab, resources, activeResourceId, + selectedResourceId, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, }: UseDesktopTabResourcesOptions): void { /** @@ -81,6 +95,8 @@ export function useDesktopTabResources({ const knownScopeRef = useRef(scopeId) /** The native switch this hook asked for and has not seen land yet. */ const requestedTabIdRef = useRef(null) + /** A selected tab that is not live yet, such as a reload with the tab in the URL. */ + const pendingSelectedTabIdRef = useRef(null) const scopeIdRef = useRef(scopeId) scopeIdRef.current = scopeId const tabsRef = useRef(tabs) @@ -95,6 +111,8 @@ export function useDesktopTabResources({ switchTabRef.current = switchTab const selectResourceRef = useRef(selectResource) selectResourceRef.current = selectResource + const restoreResourceRef = useRef(restoreResource) + restoreResourceRef.current = restoreResource const onResourceEventRef = useRef(onResourceEvent) onResourceEventRef.current = onResourceEvent @@ -105,6 +123,7 @@ export function useDesktopTabResources({ knownScopeRef.current = scopeId known.clear() requestedTabIdRef.current = null + pendingSelectedTabIdRef.current = null } const resourceTabIds = new Set( resources.filter((resource) => resource.type === type).map((resource) => resource.id) @@ -118,6 +137,15 @@ export function useDesktopTabResources({ if (!known.has(tab.id)) addResource({ type, id: tab.id, title: tab.title }) } + const pendingSelectedTabId = pendingSelectedTabIdRef.current + if (pendingSelectedTabId && tabs.some((tab) => tab.id === pendingSelectedTabId)) { + pendingSelectedTabIdRef.current = null + if (pendingSelectedTabId !== activeTabIdRef.current) { + requestedTabIdRef.current = pendingSelectedTabId + switchTabRef.current(pendingSelectedTabId, scopeId) + } + } + if (!hasSession) return const liveTabIds = new Set(tabs.map((tab) => tab.id)) for (const tabId of known) { @@ -127,15 +155,35 @@ export function useDesktopTabResources({ } }, [addResource, hasSession, removeResource, resources, scopeId, tabs, type]) - // Selecting a resource tab shows its native tab. Keyed on the selection - // alone: a native push must not re-assert a selection it just moved away - // from, or the two sides would trade switches forever. + // Selecting a resource tab shows its native tab. Keyed on the explicit + // selection alone: a native push must not re-assert a selection it just + // moved away from, or the two sides would trade switches forever, and the + // strip's fallback is not a choice to impose on the desktop app. A selected + // tab that has not landed yet is switched to by the projection above once it + // does, so a reload with the tab in the URL still shows that page. + useEffect(() => { + pendingSelectedTabIdRef.current = null + if (!selectedResourceId || selectedResourceId === activeTabIdRef.current) return + if (!tabsRef.current.some((tab) => tab.id === selectedResourceId)) { + pendingSelectedTabIdRef.current = selectedResourceId + return + } + requestedTabIdRef.current = selectedResourceId + switchTabRef.current(selectedResourceId, scopeIdRef.current) + }, [selectedResourceId]) + + // With no effective selection the strip falls back to a tab of its own + // choosing. The desktop app still shows the tab the user was last on, so the + // strip adopts that one rather than showing a page the user did not pick. useEffect(() => { - if (!activeResourceId || activeResourceId === activeTabIdRef.current) return - if (!tabsRef.current.some((tab) => tab.id === activeResourceId)) return - requestedTabIdRef.current = activeResourceId - switchTabRef.current(activeResourceId, scopeIdRef.current) - }, [activeResourceId]) + if (selectedResourceId && selectedResourceId === activeResourceId) return + const activeTabId = activeTabIdRef.current + if (!activeTabId || activeTabId === activeResourceId) return + const activeResource = resourcesRef.current.find((resource) => resource.id === activeResourceId) + if (activeResource?.type !== type) return + if (!tabsRef.current.some((tab) => tab.id === activeTabId)) return + restoreResourceRef.current(activeTabId) + }, [activeResourceId, selectedResourceId, type]) // A native switch while the user is on this kind of tab follows into the // strip. The switch this hook requested itself is not a native change of mind. diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx index ef79b3ed7c2..2554f75cd48 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx @@ -42,9 +42,11 @@ interface HostProps { scopeId: string resources: MothershipResource[] activeResourceId: string | null + selectedResourceId: string | null addResource: (resource: MothershipResource) => void removeResource: (type: MothershipResource['type'], id: string) => void selectResource: (id: string) => void + restoreResource: (id: string) => void onResourceEvent: (id: string, options?: { activate?: boolean }) => void } @@ -59,6 +61,7 @@ describe('useTerminalTabResources', () => { const addResource = vi.fn() const removeResource = vi.fn() const selectResource = vi.fn() + const restoreResource = vi.fn() const onResourceEvent = vi.fn() function render(overrides: Partial = {}) { @@ -66,9 +69,11 @@ describe('useTerminalTabResources', () => { scopeId: SCOPE, resources: [], activeResourceId: null, + selectedResourceId: null, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, ...overrides, } @@ -120,31 +125,56 @@ describe('useTerminalTabResources', () => { { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, ] - const rerender = render({ resources, activeResourceId: 'terminal:1' }) + const rerender = render({ + resources, + activeResourceId: 'terminal:1', + selectedResourceId: 'terminal:1', + }) pushTabs(SCOPE, [shell('1', true), shell('2')], '1') expect(switchTerminal).not.toHaveBeenCalled() - rerender({ activeResourceId: 'terminal:2' }) + rerender({ activeResourceId: 'terminal:2', selectedResourceId: 'terminal:2' }) expect(switchTerminal).toHaveBeenCalledExactlyOnceWith('2', SCOPE, { claim: false }) pushTabs(SCOPE, [shell('1'), shell('2', true)], '2') expect(selectResource).not.toHaveBeenCalled() }) + it('adopts the native active shell on reopen instead of pushing the fallback tab', () => { + const rerender = render() + pushTabs(SCOPE, [shell('1', true), shell('2')], '1') + rerender({ + resources: [ + { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, + { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, + ], + activeResourceId: 'terminal:2', + selectedResourceId: null, + }) + + expect(switchTerminal).not.toHaveBeenCalled() + expect(restoreResource).toHaveBeenCalledExactlyOnceWith('terminal:1') + expect(selectResource).not.toHaveBeenCalled() + }) + it('follows a native switch into the strip only while the user is on a terminal', () => { const resources: MothershipResource[] = [ { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, { type: 'file', id: 'f', title: 'notes.md' }, ] - const rerender = render({ resources, activeResourceId: 'terminal:1' }) + const rerender = render({ + resources, + activeResourceId: 'terminal:1', + selectedResourceId: 'terminal:1', + }) pushTabs(SCOPE, [shell('1', true), shell('2')], '1') pushTabs(SCOPE, [shell('1'), shell('2', true)], '2') expect(selectResource).toHaveBeenCalledExactlyOnceWith('terminal:2') selectResource.mockClear() - rerender({ activeResourceId: 'f' }) + rerender({ activeResourceId: 'f', selectedResourceId: 'f' }) pushTabs(SCOPE, [shell('1', true), shell('2')], '1') expect(selectResource).not.toHaveBeenCalled() }) @@ -156,6 +186,7 @@ describe('useTerminalTabResources', () => { { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, ], activeResourceId: 'terminal:1', + selectedResourceId: 'terminal:1', }) pushTabs(SCOPE, [shell('1', true), shell('2')], '1') act(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts index e794b0debd7..f1d66dd7b34 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts @@ -16,6 +16,7 @@ interface UseTerminalTabResourcesOptions extends DesktopTabResourceCallbacks { scopeId: string resources: readonly MothershipResource[] activeResourceId: string | null + selectedResourceId: string | null } function showTerminal(resourceId: string, scopeId: string): void { @@ -32,9 +33,11 @@ export function useTerminalTabResources({ scopeId, resources, activeResourceId, + selectedResourceId, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, }: UseTerminalTabResourcesOptions): void { const hasSession = useCopilotTerminalStore((state) => state.sessions[scopeId] !== undefined) @@ -69,9 +72,11 @@ export function useTerminalTabResources({ switchTab: showTerminal, resources, activeResourceId, + selectedResourceId, addResource, removeResource, selectResource, + restoreResource, onResourceEvent, }) } From 81e0abfdbbabed0221e730b42afea4e078228737 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 02:50:29 -0700 Subject: [PATCH 2/5] fix(desktop): adopt the remembered tab without claiming the user's selection Review round on the reopen fix. A late first report of the desktop app's active tab carries the tab it remembers, not a switch the user made, so it is adopted rather than claimed and agent activity can still take the view on chat open. A move away from a tab the desktop was already showing stays the user's own. Adoption now waits for the chat history to be applied, so the arrival order of the tab list and the history no longer decides which resource a chat opens on, and it skips a tab the strip has already dropped, so closing the shown tab cannot write the closed id back. Closing the shown tab selects its neighbour the way the desktop app picks the next native tab, instead of flashing through the strip's last tab. The two wrapper hooks now share one options type with the strip, and the adopt rule lives in a single helper. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ --- .../resource-tabs/resource-tabs.tsx | 21 ++- .../app/workspace/[workspaceId]/home/home.tsx | 24 ++-- .../hooks/use-browser-tab-resources.test.tsx | 106 +++++++++++++--- .../home/hooks/use-browser-tab-resources.ts | 34 +---- .../home/hooks/use-desktop-tab-resources.ts | 120 ++++++++++++------ .../hooks/use-terminal-tab-resources.test.tsx | 13 +- .../home/hooks/use-terminal-tab-resources.ts | 34 +---- 7 files changed, 202 insertions(+), 150 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx index 66d91c0e0be..19627b45254 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx @@ -408,11 +408,19 @@ export function ResourceTabs({ const handleClose = useCallback( (id: string) => { - const resource = resources.find((r) => r.id === id) + const index = resources.findIndex((r) => r.id === id) + const resource = resources[index] if (!resource) return const isMulti = selectedIds.has(resource.id) && selectedIds.size > 1 const targets = isMulti ? resources.filter((r) => selectedIds.has(r.id)) : [resource] if (!confirmClosingRunningTerminals(targets, terminalTabs)) return + // Closing the shown tab moves to its neighbour, right then left, the way + // the desktop app picks the next native tab, so the strip does not fall + // back to its last tab and jump once the close lands. + if (!isMulti && activeId === resource.id) { + const nextId = findNearestId(resources, index, null) + if (nextId) selectResource(nextId) + } // A browser tab's page is closed natively and its resource dropped at // once; the tab list then confirms the removal. A shell's close answers // with the tab list, so its resource follows that list instead — a @@ -451,7 +459,16 @@ export function ResourceTabs({ } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [chatId, desktopScopeId, onRemoveResource, resources, selectedIds, terminalTabs] + [ + activeId, + chatId, + desktopScopeId, + onRemoveResource, + resources, + selectResource, + selectedIds, + terminalTabs, + ] ) /** diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 51f0c9ad548..1721d8e61b4 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -334,27 +334,21 @@ export function Home({ chatId, userName, userId }: HomeProps) { [setActiveResourceId, clearResourceActivity] ) - const desktopTabResourceCallbacks = { + const desktopTabResourceOptions = { + scopeId: desktopScopeId, + resources, + activeResourceId, + selectedResourceId: activeResourceParam, + // A chat without an id has nothing stored to wait for. + hydrated: resolvedChatId === undefined || !isChatHistoryPending, addResource, removeResource, selectResource: selectResourceFromUser, restoreResource: setActiveResourceId, onResourceEvent: handleResourceEvent, } - useBrowserTabResources({ - scopeId: desktopScopeId, - resources, - activeResourceId, - selectedResourceId: activeResourceParam, - ...desktopTabResourceCallbacks, - }) - useTerminalTabResources({ - scopeId: desktopScopeId, - resources, - activeResourceId, - selectedResourceId: activeResourceParam, - ...desktopTabResourceCallbacks, - }) + useBrowserTabResources(desktopTabResourceOptions) + useTerminalTabResources(desktopTabResourceOptions) const addResourceFromUser = useCallback( (resource: MothershipResource) => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx index d18d39c6811..c2274762eca 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx @@ -37,17 +37,7 @@ function pushTabs(scopeId: string, tabs: ReturnType[], activeTabId: }) } -interface HostProps { - scopeId: string - resources: MothershipResource[] - activeResourceId: string | null - selectedResourceId: string | null - addResource: (resource: MothershipResource) => void - removeResource: (type: MothershipResource['type'], id: string) => void - selectResource: (id: string) => void - restoreResource: (id: string) => void - onResourceEvent: (id: string, options?: { activate?: boolean }) => void -} +type HostProps = Parameters[0] function Host(props: HostProps) { useBrowserTabResources(props) @@ -69,6 +59,7 @@ describe('useBrowserTabResources', () => { resources: [], activeResourceId: null, selectedResourceId: null, + hydrated: true, addResource, removeResource, selectResource, @@ -206,7 +197,6 @@ describe('useBrowserTabResources', () => { { type: 'browser', id: '2', title: 'Page 2' }, ], activeResourceId: '2', - selectedResourceId: 'deleted-file', }) expect(restoreResource).toHaveBeenCalledExactlyOnceWith('1') @@ -229,20 +219,94 @@ describe('useBrowserTabResources', () => { expect(sendBrowserPanelAction).not.toHaveBeenCalled() }) - it('switches to a selected page once it lands, as after a reload with the tab in the URL', () => { - render({ selectedResourceId: '2' }) + it('adopts the native active page only once the chat history has been applied', () => { + const resources: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + ] + const rerender = render({ hydrated: false }) + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + rerender({ resources, activeResourceId: '2', selectedResourceId: null, hydrated: false }) + expect(restoreResource).not.toHaveBeenCalled() + + rerender({ resources, activeResourceId: '2', selectedResourceId: null, hydrated: true }) + expect(restoreResource).toHaveBeenCalledExactlyOnceWith('1') + }) + + it('leaves a stored resource the history pinned alone once hydrated', () => { + const rerender = render({ hydrated: false }) + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + rerender({ + resources: [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + { type: 'file', id: 'f', title: 'notes.md' }, + ], + activeResourceId: 'f', + selectedResourceId: 'f', + hydrated: true, + }) + expect(restoreResource).not.toHaveBeenCalled() + }) + + it('does not adopt a page the user just closed in the strip', () => { + const rerender = render() + pushTabs(SCOPE, [tab('1'), tab('2', true), tab('3')], '2') + rerender({ + resources: [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + { type: 'browser', id: '3', title: 'Page 3' }, + ], + activeResourceId: '2', + selectedResourceId: '2', + }) + expect(restoreResource).not.toHaveBeenCalled() + + // The strip dropped page 2 before the native close landed. + rerender({ + resources: [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '3', title: 'Page 3' }, + ], + activeResourceId: '3', + selectedResourceId: null, + }) + expect(restoreResource).not.toHaveBeenCalled() + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + }) + + it('adopts the first reported active page without claiming it', () => { + const resources: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + ] + const rerender = render() + // The pages land before the desktop app reports which one it shows. + pushTabs(SCOPE, [tab('1'), tab('2')], null) + rerender({ resources, activeResourceId: '2', selectedResourceId: null }) + expect(restoreResource).not.toHaveBeenCalled() + + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + expect(restoreResource).toHaveBeenCalledExactlyOnceWith('1') + expect(selectResource).not.toHaveBeenCalled() expect(sendBrowserPanelAction).not.toHaveBeenCalled() + }) + it('claims a native switch away from a page it was already showing', () => { + const resources: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + ] + const rerender = render() pushTabs(SCOPE, [tab('1', true), tab('2')], '1') - expect(sendBrowserPanelAction).toHaveBeenCalledExactlyOnceWith( - 'switch-tab', - { tabId: '2', claim: false }, - SCOPE - ) + rerender({ resources, activeResourceId: '1', selectedResourceId: null }) + expect(selectResource).not.toHaveBeenCalled() - // The requested switch landing is not a native change to follow. + // A keyboard shortcut in the page moved the desktop app off page 1. pushTabs(SCOPE, [tab('1'), tab('2', true)], '2') - expect(selectResource).not.toHaveBeenCalled() + expect(selectResource).toHaveBeenCalledExactlyOnceWith('2') + expect(restoreResource).not.toHaveBeenCalled() }) it('follows a native switch into the strip only while the user is on the browser', () => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts index c3155eeaa0f..98983b756dd 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts @@ -5,9 +5,8 @@ import { getErrorMessage } from '@sim/utils/errors' import { onOpenInBrowserPanel } from '@/lib/browser-agent/open-in-panel' import { browserTabTitle } from '@/lib/browser-agent/tab-label' import { openUrlInNewBrowserTab, sendBrowserPanelAction } from '@/lib/browser-agent/transport' -import type { MothershipResource } from '@/lib/copilot/resources/types' import { - type DesktopTabResourceCallbacks, + type DesktopTabStripOptions, useDesktopTabResources, } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' @@ -16,14 +15,6 @@ const logger = createLogger('BrowserTabResources') const EMPTY_BROWSER_TABS: BrowserTabState[] = [] -interface UseBrowserTabResourcesOptions extends DesktopTabResourceCallbacks { - /** Desktop browser scope whose pages back this chat's browser tabs. */ - scopeId: string - resources: readonly MothershipResource[] - activeResourceId: string | null - selectedResourceId: string | null -} - function switchBrowserTab(tabId: string, scopeId: string): void { sendBrowserPanelAction('switch-tab', { tabId, claim: false }, scopeId) } @@ -32,17 +23,8 @@ function switchBrowserTab(tabId: string, scopeId: string): void { * Projects the desktop app's live browser pages into `browser` resource tabs, * one per page. See {@link useDesktopTabResources} for the shared model. */ -export function useBrowserTabResources({ - scopeId, - resources, - activeResourceId, - selectedResourceId, - addResource, - removeResource, - selectResource, - restoreResource, - onResourceEvent, -}: UseBrowserTabResourcesOptions): void { +export function useBrowserTabResources(options: DesktopTabStripOptions): void { + const { scopeId, selectResource } = options const hasSession = useBrowserSessionStore((state) => state.sessions[scopeId] !== undefined) const browserTabs = useBrowserSessionStore( (state) => state.sessions[scopeId]?.tabs ?? EMPTY_BROWSER_TABS @@ -67,21 +49,13 @@ export function useBrowserTabResources({ selectResourceRef.current = selectResource useDesktopTabResources({ + ...options, type: 'browser', - scopeId, tabs, hasSession, activeTabId, agentTabId, switchTab: switchBrowserTab, - resources, - activeResourceId, - selectedResourceId, - addResource, - removeResource, - selectResource, - restoreResource, - onResourceEvent, }) // Chat links clicked in the desktop app open in a new browser tab. The user diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts index 553d8076928..23ef2ab8a0b 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts @@ -24,10 +24,30 @@ export interface DesktopTabResourceCallbacks { onResourceEvent: ResourceEventHandler } -interface UseDesktopTabResourcesOptions extends DesktopTabResourceCallbacks { - type: 'browser' | 'terminal' +/** What the strip shares with every kind of desktop-backed resource tab. */ +export interface DesktopTabStripOptions extends DesktopTabResourceCallbacks { /** Desktop scope whose live tabs back this chat's resource tabs. */ scopeId: string + resources: readonly MothershipResource[] + /** The resource the strip shows: the explicit selection or its fallback. */ + activeResourceId: string | null + /** The explicit selection alone, without the strip's fallback. */ + selectedResourceId: string | null + /** + * Whether the chat's stored resources have been applied to the strip. + * + * Adopting a tab writes it to `activeResourceId`, which is the one place the + * rest of the surface reads as the shown resource, so adopting on top of a + * provisional fallback would let the arrival order of the tab list and the + * chat history decide what the chat opens on. Waiting makes the outcome the + * same either way: the history pins a stored resource, or it pins nothing + * and the desktop app's remembered tab stands. + */ + hydrated: boolean +} + +interface UseDesktopTabResourcesOptions extends DesktopTabStripOptions { + type: 'browser' | 'terminal' /** The desktop app's live tab list for the scope, in its order. */ tabs: readonly DesktopTab[] /** @@ -42,11 +62,24 @@ interface UseDesktopTabResourcesOptions extends DesktopTabResourceCallbacks { agentTabId: string | null /** Shows a tab natively without claiming it for the user. */ switchTab: (tabId: string, scopeId: string) => void - resources: readonly MothershipResource[] - /** The resource the strip shows: the explicit selection or its fallback. */ - activeResourceId: string | null - /** The explicit selection alone, without the strip's fallback. */ - selectedResourceId: string | null +} + +/** + * The desktop app's active tab to adopt in place of the strip's fallback: one + * the strip does not show yet, of the same kind as the fallback, and still in + * the strip — a tab just closed there stays the desktop app's active tab until + * the close lands. + */ +function nativeTabToAdopt( + resources: readonly MothershipResource[], + activeResourceId: string | null, + activeTabId: string | null, + type: MothershipResourceType +): string | null { + if (!activeTabId || activeTabId === activeResourceId) return null + if (resources.find((resource) => resource.id === activeResourceId)?.type !== type) return null + const live = resources.some((resource) => resource.type === type && resource.id === activeTabId) + return live ? activeTabId : null } /** @@ -78,6 +111,7 @@ export function useDesktopTabResources({ resources, activeResourceId, selectedResourceId, + hydrated, addResource, removeResource, selectResource, @@ -95,8 +129,6 @@ export function useDesktopTabResources({ const knownScopeRef = useRef(scopeId) /** The native switch this hook asked for and has not seen land yet. */ const requestedTabIdRef = useRef(null) - /** A selected tab that is not live yet, such as a reload with the tab in the URL. */ - const pendingSelectedTabIdRef = useRef(null) const scopeIdRef = useRef(scopeId) scopeIdRef.current = scopeId const tabsRef = useRef(tabs) @@ -107,6 +139,15 @@ export function useDesktopTabResources({ resourcesRef.current = resources const activeResourceIdRef = useRef(activeResourceId) activeResourceIdRef.current = activeResourceId + /** Whether the strip shows an explicit selection rather than its fallback. */ + const explicitSelection = selectedResourceId !== null && selectedResourceId === activeResourceId + const hydratedRef = useRef(hydrated) + hydratedRef.current = hydrated + /** + * The tab the desktop app showed last, to tell a change of the shown tab + * from the scope's first report. Starts unset, like the scope itself. + */ + const previousActiveTabIdRef = useRef(null) const switchTabRef = useRef(switchTab) switchTabRef.current = switchTab const selectResourceRef = useRef(selectResource) @@ -123,7 +164,7 @@ export function useDesktopTabResources({ knownScopeRef.current = scopeId known.clear() requestedTabIdRef.current = null - pendingSelectedTabIdRef.current = null + previousActiveTabIdRef.current = null } const resourceTabIds = new Set( resources.filter((resource) => resource.type === type).map((resource) => resource.id) @@ -137,15 +178,6 @@ export function useDesktopTabResources({ if (!known.has(tab.id)) addResource({ type, id: tab.id, title: tab.title }) } - const pendingSelectedTabId = pendingSelectedTabIdRef.current - if (pendingSelectedTabId && tabs.some((tab) => tab.id === pendingSelectedTabId)) { - pendingSelectedTabIdRef.current = null - if (pendingSelectedTabId !== activeTabIdRef.current) { - requestedTabIdRef.current = pendingSelectedTabId - switchTabRef.current(pendingSelectedTabId, scopeId) - } - } - if (!hasSession) return const liveTabIds = new Set(tabs.map((tab) => tab.id)) for (const tabId of known) { @@ -158,16 +190,10 @@ export function useDesktopTabResources({ // Selecting a resource tab shows its native tab. Keyed on the explicit // selection alone: a native push must not re-assert a selection it just // moved away from, or the two sides would trade switches forever, and the - // strip's fallback is not a choice to impose on the desktop app. A selected - // tab that has not landed yet is switched to by the projection above once it - // does, so a reload with the tab in the URL still shows that page. + // strip's fallback is not a choice to impose on the desktop app. useEffect(() => { - pendingSelectedTabIdRef.current = null if (!selectedResourceId || selectedResourceId === activeTabIdRef.current) return - if (!tabsRef.current.some((tab) => tab.id === selectedResourceId)) { - pendingSelectedTabIdRef.current = selectedResourceId - return - } + if (!tabsRef.current.some((tab) => tab.id === selectedResourceId)) return requestedTabIdRef.current = selectedResourceId switchTabRef.current(selectedResourceId, scopeIdRef.current) }, [selectedResourceId]) @@ -176,29 +202,41 @@ export function useDesktopTabResources({ // choosing. The desktop app still shows the tab the user was last on, so the // strip adopts that one rather than showing a page the user did not pick. useEffect(() => { - if (selectedResourceId && selectedResourceId === activeResourceId) return - const activeTabId = activeTabIdRef.current - if (!activeTabId || activeTabId === activeResourceId) return - const activeResource = resourcesRef.current.find((resource) => resource.id === activeResourceId) - if (activeResource?.type !== type) return - if (!tabsRef.current.some((tab) => tab.id === activeTabId)) return - restoreResourceRef.current(activeTabId) - }, [activeResourceId, selectedResourceId, type]) + if (!hydrated || explicitSelection) return + const tabId = nativeTabToAdopt( + resourcesRef.current, + activeResourceId, + activeTabIdRef.current, + type + ) + if (tabId) restoreResourceRef.current(tabId) + }, [activeResourceId, explicitSelection, hydrated, type]) // A native switch while the user is on this kind of tab follows into the - // strip. The switch this hook requested itself is not a native change of mind. + // strip. The switch this hook requested itself is not a native change of + // mind, and neither is the scope's first report: that one carries the tab + // the desktop app remembers, so it is adopted rather than claimed. A move + // away from a tab it was already showing is the user's own. useEffect(() => { + const previousActiveTabId = previousActiveTabIdRef.current + previousActiveTabIdRef.current = activeTabId if (requestedTabIdRef.current === activeTabId) { requestedTabIdRef.current = null return } - const activeResource = resourcesRef.current.find( - (resource) => resource.id === activeResourceIdRef.current - ) - if (!activeTabId || activeResource?.type !== type || activeResource.id === activeTabId) { + const activeResourceId = activeResourceIdRef.current + if (previousActiveTabId !== null) { + const activeResource = resourcesRef.current.find( + (resource) => resource.id === activeResourceId + ) + if (activeTabId && activeResource?.type === type && activeResource.id !== activeTabId) { + selectResourceRef.current(activeTabId) + } return } - selectResourceRef.current(activeTabId) + if (!hydratedRef.current) return + const tabId = nativeTabToAdopt(resourcesRef.current, activeResourceId, activeTabId, type) + if (tabId) restoreResourceRef.current(tabId) }, [activeTabId, type]) // The agent's tab surfaces like any other agent activity. diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx index 2554f75cd48..cc339080054 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx @@ -38,17 +38,7 @@ function pushTabs(scopeId: string, tabs: TerminalTabState[], activeTerminalId: s }) } -interface HostProps { - scopeId: string - resources: MothershipResource[] - activeResourceId: string | null - selectedResourceId: string | null - addResource: (resource: MothershipResource) => void - removeResource: (type: MothershipResource['type'], id: string) => void - selectResource: (id: string) => void - restoreResource: (id: string) => void - onResourceEvent: (id: string, options?: { activate?: boolean }) => void -} +type HostProps = Parameters[0] function Host(props: HostProps) { useTerminalTabResources(props) @@ -70,6 +60,7 @@ describe('useTerminalTabResources', () => { resources: [], activeResourceId: null, selectedResourceId: null, + hydrated: true, addResource, removeResource, selectResource, diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts index f1d66dd7b34..b9cb6826138 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts @@ -1,24 +1,15 @@ import { useMemo } from 'react' import type { TerminalTabState } from '@sim/terminal-protocol' -import type { MothershipResource } from '@/lib/copilot/resources/types' import { terminalIdFromResourceId, terminalResourceId } from '@/lib/terminal/resource-id' import { switchTerminal } from '@/lib/terminal/transport' import { - type DesktopTabResourceCallbacks, + type DesktopTabStripOptions, useDesktopTabResources, } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' const EMPTY_TERMINAL_TABS: TerminalTabState[] = [] -interface UseTerminalTabResourcesOptions extends DesktopTabResourceCallbacks { - /** Desktop terminal scope whose shells back this chat's terminal tabs. */ - scopeId: string - resources: readonly MothershipResource[] - activeResourceId: string | null - selectedResourceId: string | null -} - function showTerminal(resourceId: string, scopeId: string): void { void switchTerminal(terminalIdFromResourceId(resourceId), scopeId, { claim: false }).catch( () => {} @@ -29,17 +20,8 @@ function showTerminal(resourceId: string, scopeId: string): void { * Projects the desktop app's live shells into `terminal` resource tabs, one * per shell. See {@link useDesktopTabResources} for the shared model. */ -export function useTerminalTabResources({ - scopeId, - resources, - activeResourceId, - selectedResourceId, - addResource, - removeResource, - selectResource, - restoreResource, - onResourceEvent, -}: UseTerminalTabResourcesOptions): void { +export function useTerminalTabResources(options: DesktopTabStripOptions): void { + const { scopeId } = options const hasSession = useCopilotTerminalStore((state) => state.sessions[scopeId] !== undefined) const terminalTabs = useCopilotTerminalStore( (state) => state.sessions[scopeId]?.tabs.tabs ?? EMPTY_TERMINAL_TABS @@ -63,20 +45,12 @@ export function useTerminalTabResources({ ) useDesktopTabResources({ + ...options, type: 'terminal', - scopeId, tabs, hasSession, activeTabId: activeTerminalId && terminalResourceId(activeTerminalId), agentTabId: agentTerminalId && terminalResourceId(agentTerminalId), switchTab: showTerminal, - resources, - activeResourceId, - selectedResourceId, - addResource, - removeResource, - selectResource, - restoreResource, - onResourceEvent, }) } From 3435157cffc9637f50620ddb65c48252d8103abb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 03:03:03 -0700 Subject: [PATCH 3/5] fix(desktop): show a selected tab that arrives after the tab list The effect that shows an explicitly selected tab was keyed on the selection alone, so a selection made before the desktop app published its tab list was dropped rather than applied when the tab arrived. It is now keyed on that tab being live as well, which covers the late arrival without a retry ref to arm and disarm. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ --- .../hooks/use-browser-tab-resources.test.tsx | 16 ++++++++++++++++ .../home/hooks/use-desktop-tab-resources.ts | 18 ++++++++++++------ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx index c2274762eca..660c76c19ad 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx @@ -165,6 +165,22 @@ describe('useBrowserTabResources', () => { expect(selectResource).not.toHaveBeenCalled() }) + it('shows a page selected before the pages landed, once it arrives', () => { + render({ selectedResourceId: '2', activeResourceId: '2' }) + expect(sendBrowserPanelAction).not.toHaveBeenCalled() + + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + expect(sendBrowserPanelAction).toHaveBeenCalledExactlyOnceWith( + 'switch-tab', + { tabId: '2', claim: false }, + SCOPE + ) + + // The requested switch landing is not a native change to follow. + pushTabs(SCOPE, [tab('1'), tab('2', true)], '2') + expect(selectResource).not.toHaveBeenCalled() + }) + it('adopts the native active page on reopen instead of pushing the fallback tab', () => { const resources: MothershipResource[] = [ { type: 'browser', id: '1', title: 'Page 1' }, diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts index 23ef2ab8a0b..bc895c6f937 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts @@ -187,16 +187,22 @@ export function useDesktopTabResources({ } }, [addResource, hasSession, removeResource, resources, scopeId, tabs, type]) + /** Whether the selected resource is one of this kind's live tabs. */ + const selectedTabIsLive = + selectedResourceId !== null && tabs.some((tab) => tab.id === selectedResourceId) + // Selecting a resource tab shows its native tab. Keyed on the explicit - // selection alone: a native push must not re-assert a selection it just - // moved away from, or the two sides would trade switches forever, and the - // strip's fallback is not a choice to impose on the desktop app. + // selection alone — the strip's fallback is not a choice to impose on the + // desktop app, and a native push must not re-assert a selection it just + // moved away from, or the two sides would trade switches forever — and on + // that tab being live, so a selection made before the desktop app published + // its tab list is shown once the tab arrives rather than dropped. useEffect(() => { - if (!selectedResourceId || selectedResourceId === activeTabIdRef.current) return - if (!tabsRef.current.some((tab) => tab.id === selectedResourceId)) return + if (!selectedResourceId || !selectedTabIsLive) return + if (selectedResourceId === activeTabIdRef.current) return requestedTabIdRef.current = selectedResourceId switchTabRef.current(selectedResourceId, scopeIdRef.current) - }, [selectedResourceId]) + }, [selectedResourceId, selectedTabIsLive]) // With no effective selection the strip falls back to a tab of its own // choosing. The desktop app still shows the tab the user was last on, so the From 7943024cddd8d226b56c5c13a4b36f04f08215bb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 09:48:44 -0700 Subject: [PATCH 4/5] refactor(desktop): align the two tab-adoption paths and drop dead plumbing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quality pass on the reopen fix. The late-arrival adoption now carries the same guards as the hydrated one, so a first report of the desktop app's active tab can no longer override a selection the user made before the tab list arrived. Both guards are pinned by tests that fail when either is removed. The predicate the adopt and claim paths share moved into one helper, so the single difference between them — adoption needs the tab to still be in the strip, following the user does not — is stated once. Removes a ref nothing read and an options interface with no second consumer. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ --- .../hooks/use-browser-tab-resources.test.tsx | 49 +++++++--- .../[workspaceId]/home/hooks/use-chat.ts | 3 + .../home/hooks/use-desktop-tab-resources.ts | 91 ++++++++++--------- 3 files changed, 90 insertions(+), 53 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx index 660c76c19ad..1c35ad089eb 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx @@ -249,19 +249,21 @@ describe('useBrowserTabResources', () => { expect(restoreResource).toHaveBeenCalledExactlyOnceWith('1') }) - it('leaves a stored resource the history pinned alone once hydrated', () => { - const rerender = render({ hydrated: false }) - pushTabs(SCOPE, [tab('1', true), tab('2')], '1') - rerender({ - resources: [ - { type: 'browser', id: '1', title: 'Page 1' }, - { type: 'browser', id: '2', title: 'Page 2' }, - { type: 'file', id: 'f', title: 'notes.md' }, - ], - activeResourceId: 'f', - selectedResourceId: 'f', - hydrated: true, + it('leaves an explicitly selected page alone when the history is applied', () => { + const resources: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + ] + // The user is on page 2 by choice while the desktop app shows page 1. + const rerender = render({ + resources, + activeResourceId: '2', + selectedResourceId: '2', + hydrated: false, }) + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + + rerender({ resources, activeResourceId: '2', selectedResourceId: '2', hydrated: true }) expect(restoreResource).not.toHaveBeenCalled() }) @@ -309,6 +311,29 @@ describe('useBrowserTabResources', () => { expect(sendBrowserPanelAction).not.toHaveBeenCalled() }) + it('does not let a first report override a selection made before the pages landed', () => { + const resources: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + ] + // The pages land first, with the desktop app not yet reporting which it shows. + const rerender = render({ selectedResourceId: '2', activeResourceId: '2' }) + pushTabs(SCOPE, [tab('1'), tab('2')], null) + rerender({ resources, selectedResourceId: '2', activeResourceId: '2' }) + // The selection is honoured by switching the native page to it. + expect(sendBrowserPanelAction).toHaveBeenCalledWith( + 'switch-tab', + { tabId: '2', claim: false }, + SCOPE + ) + + // The desktop app then reports the page it was already on. The strip must + // not move onto it, or the selection the user made would be lost. + pushTabs(SCOPE, [tab('1', true), tab('2')], '1') + expect(restoreResource).not.toHaveBeenCalled() + expect(selectResource).not.toHaveBeenCalled() + }) + it('claims a native switch away from a page it was already showing', () => { const resources: MothershipResource[] = [ { type: 'browser', id: '1', title: 'Page 1' }, diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index 57e951dc159..5ba0de06c40 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -2490,6 +2490,9 @@ export function useChat( // Replacing the array with an identical one still re-renders the tab // strip and panel — skip the no-op so open panels don't flash. if (!resourcesUnchanged) { + // The ref keeps an eager fallback so a request sent in this commit + // still attaches a resource; the selection itself stays empty so the + // desktop app's remembered tab can win. activeResourceIdRef.current = hydratedActiveResourceId ?? mergedResources[mergedResources.length - 1].id setResources(mergedResources) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts index bc895c6f937..5d47b8355cd 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts @@ -8,24 +8,8 @@ export interface DesktopTab { title: string } -export interface DesktopTabResourceCallbacks { - /** Adds a tab without activating it; activation goes through {@link onResourceEvent}. */ - addResource: (resource: MothershipResource) => void - removeResource: (resourceType: MothershipResourceType, resourceId: string) => void - /** Explicit user selection, which claims the strip's selection for the user. */ - selectResource: (resourceId: string) => void - /** - * Adopts the desktop app's remembered tab as the shown resource without - * claiming the selection for the user, so agent activity can still take the - * view the way it does on any chat open. - */ - restoreResource: (resourceId: string) => void - /** Agent activity on a tab, subject to the panel's user-ownership policy. */ - onResourceEvent: ResourceEventHandler -} - /** What the strip shares with every kind of desktop-backed resource tab. */ -export interface DesktopTabStripOptions extends DesktopTabResourceCallbacks { +export interface DesktopTabStripOptions { /** Desktop scope whose live tabs back this chat's resource tabs. */ scopeId: string resources: readonly MothershipResource[] @@ -35,15 +19,24 @@ export interface DesktopTabStripOptions extends DesktopTabResourceCallbacks { selectedResourceId: string | null /** * Whether the chat's stored resources have been applied to the strip. - * - * Adopting a tab writes it to `activeResourceId`, which is the one place the - * rest of the surface reads as the shown resource, so adopting on top of a + * Adopting a tab writes it to `activeResourceId`, so adopting on top of a * provisional fallback would let the arrival order of the tab list and the - * chat history decide what the chat opens on. Waiting makes the outcome the - * same either way: the history pins a stored resource, or it pins nothing - * and the desktop app's remembered tab stands. + * chat history decide what the chat opens on. */ hydrated: boolean + /** Adds a tab without activating it; activation goes through {@link onResourceEvent}. */ + addResource: (resource: MothershipResource) => void + removeResource: (resourceType: MothershipResourceType, resourceId: string) => void + /** Explicit user selection, which claims the strip's selection for the user. */ + selectResource: (resourceId: string) => void + /** + * Adopts the desktop app's remembered tab as the shown resource without + * claiming the selection for the user, so agent activity can still take the + * view the way it does on any chat open. + */ + restoreResource: (resourceId: string) => void + /** Agent activity on a tab, subject to the panel's user-ownership policy. */ + onResourceEvent: ResourceEventHandler } interface UseDesktopTabResourcesOptions extends DesktopTabStripOptions { @@ -65,21 +58,40 @@ interface UseDesktopTabResourcesOptions extends DesktopTabStripOptions { } /** - * The desktop app's active tab to adopt in place of the strip's fallback: one - * the strip does not show yet, of the same kind as the fallback, and still in - * the strip — a tab just closed there stays the desktop app's active tab until - * the close lands. + * The desktop app's active tab when it is not the tab the strip shows, and the + * strip is on one of this kind. Null when the two already agree or the strip + * is showing something else entirely. */ -function nativeTabToAdopt( +function nativeTabOffStrip( resources: readonly MothershipResource[], activeResourceId: string | null, activeTabId: string | null, type: MothershipResourceType ): string | null { if (!activeTabId || activeTabId === activeResourceId) return null - if (resources.find((resource) => resource.id === activeResourceId)?.type !== type) return null - const live = resources.some((resource) => resource.type === type && resource.id === activeTabId) - return live ? activeTabId : null + return resources.find((resource) => resource.id === activeResourceId)?.type === type + ? activeTabId + : null +} + +/** + * The same tab, narrowed to one the strip still holds as a resource: a tab + * just closed there stays the desktop app's active tab until the close lands, + * and adopting it would show a tab that is gone. Following a switch the user + * made needs no such check — a brand-new tab is followed before the strip has + * projected it. + */ +function nativeTabToAdopt( + resources: readonly MothershipResource[], + activeResourceId: string | null, + activeTabId: string | null, + type: MothershipResourceType +): string | null { + const tabId = nativeTabOffStrip(resources, activeResourceId, activeTabId, type) + if (!tabId) return null + return resources.some((resource) => resource.type === type && resource.id === tabId) + ? tabId + : null } /** @@ -131,8 +143,6 @@ export function useDesktopTabResources({ const requestedTabIdRef = useRef(null) const scopeIdRef = useRef(scopeId) scopeIdRef.current = scopeId - const tabsRef = useRef(tabs) - tabsRef.current = tabs const activeTabIdRef = useRef(activeTabId) activeTabIdRef.current = activeTabId const resourcesRef = useRef(resources) @@ -141,6 +151,8 @@ export function useDesktopTabResources({ activeResourceIdRef.current = activeResourceId /** Whether the strip shows an explicit selection rather than its fallback. */ const explicitSelection = selectedResourceId !== null && selectedResourceId === activeResourceId + const explicitSelectionRef = useRef(explicitSelection) + explicitSelectionRef.current = explicitSelection const hydratedRef = useRef(hydrated) hydratedRef.current = hydrated /** @@ -187,7 +199,6 @@ export function useDesktopTabResources({ } }, [addResource, hasSession, removeResource, resources, scopeId, tabs, type]) - /** Whether the selected resource is one of this kind's live tabs. */ const selectedTabIsLive = selectedResourceId !== null && tabs.some((tab) => tab.id === selectedResourceId) @@ -232,15 +243,13 @@ export function useDesktopTabResources({ } const activeResourceId = activeResourceIdRef.current if (previousActiveTabId !== null) { - const activeResource = resourcesRef.current.find( - (resource) => resource.id === activeResourceId - ) - if (activeTabId && activeResource?.type === type && activeResource.id !== activeTabId) { - selectResourceRef.current(activeTabId) - } + const tabId = nativeTabOffStrip(resourcesRef.current, activeResourceId, activeTabId, type) + if (tabId) selectResourceRef.current(tabId) return } - if (!hydratedRef.current) return + // Same guards as the adopt effect above: a first report must not override + // a selection the user made before the tab list arrived. + if (!hydratedRef.current || explicitSelectionRef.current) return const tabId = nativeTabToAdopt(resourcesRef.current, activeResourceId, activeTabId, type) if (tabId) restoreResourceRef.current(tabId) }, [activeTabId, type]) From f3e4e12e9cc42f2f2efb8bace6a57d318a0abd9f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 09:53:21 -0700 Subject: [PATCH 5/5] test(desktop): give the tab-resource hosts the shared options interface The test hosts took their props through a type alias derived from the hook signature. The repo asks for an interface, and the hook already exports one that is exactly this shape, so the hosts use it directly instead of restating it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ --- .../home/hooks/use-browser-tab-resources.test.tsx | 12 ++++++------ .../home/hooks/use-terminal-tab-resources.test.tsx | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx index 1c35ad089eb..44ea000b42e 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx @@ -6,6 +6,7 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { MothershipResource } from '@/lib/copilot/resources/types' import { useBrowserTabResources } from '@/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources' +import type { DesktopTabStripOptions } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' const { sendBrowserPanelAction, openUrlInNewBrowserTab, openInPanelListeners } = vi.hoisted(() => ({ @@ -37,9 +38,7 @@ function pushTabs(scopeId: string, tabs: ReturnType[], activeTabId: }) } -type HostProps = Parameters[0] - -function Host(props: HostProps) { +function Host(props: DesktopTabStripOptions) { useBrowserTabResources(props) return null } @@ -53,8 +52,8 @@ describe('useBrowserTabResources', () => { const restoreResource = vi.fn() const onResourceEvent = vi.fn() - function render(overrides: Partial = {}) { - const props: HostProps = { + function render(overrides: Partial = {}) { + const props: DesktopTabStripOptions = { scopeId: SCOPE, resources: [], activeResourceId: null, @@ -68,7 +67,8 @@ describe('useBrowserTabResources', () => { ...overrides, } act(() => root.render()) - return (next: Partial) => act(() => root.render()) + return (next: Partial) => + act(() => root.render()) } beforeEach(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx index cc339080054..2b69e762e77 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx @@ -6,6 +6,7 @@ import type { TerminalTabState } from '@sim/terminal-protocol' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { MothershipResource } from '@/lib/copilot/resources/types' +import type { DesktopTabStripOptions } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useTerminalTabResources } from '@/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources' import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' @@ -38,9 +39,7 @@ function pushTabs(scopeId: string, tabs: TerminalTabState[], activeTerminalId: s }) } -type HostProps = Parameters[0] - -function Host(props: HostProps) { +function Host(props: DesktopTabStripOptions) { useTerminalTabResources(props) return null } @@ -54,8 +53,8 @@ describe('useTerminalTabResources', () => { const restoreResource = vi.fn() const onResourceEvent = vi.fn() - function render(overrides: Partial = {}) { - const props: HostProps = { + function render(overrides: Partial = {}) { + const props: DesktopTabStripOptions = { scopeId: SCOPE, resources: [], activeResourceId: null, @@ -69,7 +68,8 @@ describe('useTerminalTabResources', () => { ...overrides, } act(() => root.render()) - return (next: Partial) => act(() => root.render()) + return (next: Partial) => + act(() => root.render()) } beforeEach(() => {