From c84a151d581319e2c3c97c3e1d194dc4b8c48aa9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 11 Sep 2026 17:10:05 -0700 Subject: [PATCH 1/6] 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/6] 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/6] 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/6] 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/6] 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(() => { From e6b5d28514e8a06ba5c3cf3cd28c29e09310f651 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 19:34:28 -0700 Subject: [PATCH 6/6] fix(desktop): resolve the shown tab instead of writing it back Reopening a chat could show the wrong page for as long as the chat history took to load. The strip wrote the desktop app's remembered tab into the selection from an effect, and that write had to wait for the history or the arrival order would decide what the chat opened on. With the history held back 2.5s, an instrumented run showed the wrong page for 2164ms before it corrected. The rule is now a pure function: an explicit selection wins, otherwise the last resource, except that a desktop-backed last resource defers to the tab the desktop app is showing. Nothing is written back, so the gate, the passive setter and the effect behind them are gone, and the same run now shows the remembered page immediately. A native switch is claimed as the user's against the tab the desktop app was showing rather than the one the strip shows, since with no explicit selection those are now the same tab. Closing the shown tab prefers a neighbour of its own kind, so the strip and the desktop app agree on what comes next. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ --- .../resource-tabs/resource-tabs.tsx | 10 +- .../app/workspace/[workspaceId]/home/home.tsx | 7 +- .../hooks/use-browser-tab-resources.test.tsx | 177 +++--------------- .../home/hooks/use-browser-tab-resources.ts | 4 +- .../[workspaceId]/home/hooks/use-chat.ts | 76 +++++--- .../home/hooks/use-desktop-tab-resources.ts | 141 +++++--------- .../hooks/use-terminal-tab-resources.test.tsx | 30 +-- .../home/hooks/use-terminal-tab-resources.ts | 8 +- .../home/resource-view-policy.test.ts | 63 +++++++ .../home/resource-view-policy.ts | 40 ++++ 10 files changed, 236 insertions(+), 320 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 19627b45254..9f1d18d94ad 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 @@ -414,11 +414,13 @@ export function ResourceTabs({ 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. + // Closing the shown tab moves to its neighbour, right then left, so the + // strip does not fall back to its last tab and jump. For a desktop tab + // this is also the neighbour the desktop app itself picks. if (!isMulti && activeId === resource.id) { - const nextId = findNearestId(resources, index, null) + const sameKind = new Set(resources.filter((r) => r.type === resource.type).map((r) => r.id)) + const nextId = + findNearestId(resources, index, sameKind) ?? findNearestId(resources, index, null) if (nextId) selectResource(nextId) } // A browser tab's page is closed natively and its resource dropped at diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 1721d8e61b4..05e78e0d2f1 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -224,7 +224,7 @@ export function Home({ chatId, userName, userId }: HomeProps) { const resourceSelectionOwnedByUserRef = useRef(false) function handleResourceEvent(resourceId: string, options?: ResourceEventOptions) { - const activeResourceId = activeResourceParamRef.current + const activeResourceId = effectiveActiveResourceIdRef.current const presentation = resolveResourceEventPresentation({ activeResourceId, activationRequested: shouldActivateResourceEvent(activeResourceId, resourceId, options), @@ -317,7 +317,7 @@ export function Home({ chatId, userName, userId }: HomeProps) { const expandResource = () => { resourceCollapseOwnedByUserRef.current = false resourceSelectionOwnedByUserRef.current = true - const activeResourceId = activeResourceParamRef.current + const activeResourceId = effectiveActiveResourceIdRef.current if (activeResourceId) clearResourceActivity(activeResourceId) setResourceCollapsed(false) } @@ -339,12 +339,9 @@ export function Home({ chatId, userName, userId }: HomeProps) { 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(desktopTabResourceOptions) 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 44ea000b42e..274b908ab54 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,7 +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 type { DesktopTabResourceOptions } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' const { sendBrowserPanelAction, openUrlInNewBrowserTab, openInPanelListeners } = vi.hoisted(() => ({ @@ -38,7 +38,7 @@ function pushTabs(scopeId: string, tabs: ReturnType[], activeTabId: }) } -function Host(props: DesktopTabStripOptions) { +function Host(props: DesktopTabResourceOptions) { useBrowserTabResources(props) return null } @@ -49,26 +49,27 @@ 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 = {}) { - const props: DesktopTabStripOptions = { + function render(overrides: Partial = {}) { + const props: DesktopTabResourceOptions = { scopeId: SCOPE, resources: [], activeResourceId: null, selectedResourceId: null, - hydrated: true, addResource, removeResource, selectResource, - restoreResource, onResourceEvent, ...overrides, } act(() => root.render()) - return (next: Partial) => - act(() => root.render()) + /** `alsoInThisCommit` lands a store push and the new props together. */ + return (next: Partial, alsoInThisCommit?: () => void) => + act(() => { + alsoInThisCommit?.() + root.render() + }) } beforeEach(() => { @@ -175,165 +176,24 @@ describe('useBrowserTabResources', () => { { 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', - }) - - 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('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 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() - }) - - 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', () => { + it('does not claim the scope first report as a user switch', () => { 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() + // The desktop app reports the page it restored. The strip resolves to that + // page on its own, so there is nothing here to claim for the user. pushTabs(SCOPE, [tab('1', true), tab('2')], '1') - expect(restoreResource).toHaveBeenCalledExactlyOnceWith('1') expect(selectResource).not.toHaveBeenCalled() 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' }, @@ -344,10 +204,15 @@ describe('useBrowserTabResources', () => { rerender({ resources, activeResourceId: '1', selectedResourceId: null }) expect(selectResource).not.toHaveBeenCalled() - // A keyboard shortcut in the page moved the desktop app off page 1. - pushTabs(SCOPE, [tab('1'), tab('2', true)], '2') + // A keyboard shortcut in the page moves the desktop app to page 2. With no + // explicit selection the strip resolves to that page in the same commit, + // so the switch is only visible against the page the desktop app left. + rerender({ resources, activeResourceId: '2' }, () => { + useBrowserSessionStore + .getState() + .setTabsState({ scopeId: SCOPE, tabs: [tab('1'), tab('2', true)], activeTabId: '2' }) + }) 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 98983b756dd..93fdb488d8a 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 @@ -6,7 +6,7 @@ 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 DesktopTabStripOptions, + type DesktopTabResourceOptions, useDesktopTabResources, } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' @@ -23,7 +23,7 @@ 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(options: DesktopTabStripOptions): void { +export function useBrowserTabResources(options: DesktopTabResourceOptions): void { const { scopeId, selectResource } = options const hasSession = useBrowserSessionStore((state) => state.sessions[scopeId] !== undefined) const browserTabs = useBrowserSessionStore( 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 5ba0de06c40..549a636aefa 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -117,6 +117,8 @@ import { dispatchStreamEvent, finalizeResidualToolCalls, } from '@/app/workspace/[workspaceId]/home/hooks/stream' +import { useNativeActiveTabIds } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' +import { resolveEffectiveResourceId } from '@/app/workspace/[workspaceId]/home/resource-view-policy' import { fetchMothershipChatHistory, type MothershipChatHistory, @@ -1306,6 +1308,12 @@ export interface UseChatOptions { * selection intentionally stays out of the URL. */ activeResourceState?: [string | null, Dispatch>] + /** + * Whether this surface projects the desktop app's browser and terminal tabs + * into its resources. Only then does the shown resource depend on which tab + * the desktop app displays. + */ + projectsDesktopTabs?: boolean /** Fired when the server's `traceparent` response header arrives, before any stream content. */ onRequestStarted?: (info: { requestId: string; userMessageId: string }) => void } @@ -1335,6 +1343,7 @@ export function getMothershipUseChatOptions( return { apiPath: MOTHERSHIP_CHAT_API_PATH, stopPath: '/api/mothership/chat/stop', + projectsDesktopTabs: true, ...options, } } @@ -1446,14 +1455,33 @@ export function useChat( const pendingResourceReordersRef = useRef(new Map()) const pendingResourceReorderFlushesRef = useRef(new Map>()) - // Derive the effective active resource ID for rendering without writing a - // passive fallback back into the user's URL selection. - const effectiveActiveResourceId = useMemo(() => { - if (resources.length === 0) return null - if (activeResourceId && resources.some((r) => r.id === activeResourceId)) - return activeResourceId - return resources[resources.length - 1].id - }, [resources, activeResourceId]) + // Sentinel used while no `chatId` is resolved; `adoptResolvedChatId` + // migrates this bucket onto the real chatId on first send. Rotated on + // home reset so a new pending chat starts with an empty bucket. + const pendingChatKeyRef = useRef(`${PENDING_CHAT_KEY_PREFIX}${generateShortId()}`) + const pendingDesktopScopeIdRef = useRef( + desktopChatScopeId(scopeKey, undefined, pendingChatKeyRef.current) + ) + const initialDesktopScopeId = desktopChatScopeId( + scopeKey, + initialChatId, + pendingChatKeyRef.current + ) + const desktopScopeIdRef = useRef(initialDesktopScopeId) + const [desktopScopeId, setDesktopScopeId] = useState(initialDesktopScopeId) + const nativeActiveTabIds = useNativeActiveTabIds( + options?.projectsDesktopTabs ? desktopScopeId : null + ) + + const nativeActiveTabIdsRef = useRef(nativeActiveTabIds) + nativeActiveTabIdsRef.current = nativeActiveTabIds + + // Derived for rendering rather than written back, so nothing the user did not + // choose ever lands in their selection. + const effectiveActiveResourceId = useMemo( + () => resolveEffectiveResourceId(resources, activeResourceId, nativeActiveTabIds), + [resources, activeResourceId, nativeActiveTabIds] + ) const activeResourceIdRef = useRef(effectiveActiveResourceId) activeResourceIdRef.current = effectiveActiveResourceId @@ -1502,10 +1530,6 @@ export function useChat( [queryClient] ) - // Sentinel used while no `chatId` is resolved; `adoptResolvedChatId` - // migrates this bucket onto the real chatId on first send. Rotated on - // home reset so a new pending chat starts with an empty bucket. - const pendingChatKeyRef = useRef(`${PENDING_CHAT_KEY_PREFIX}${generateShortId()}`) const [chatKey, setChatKey] = useState(initialChatId ?? pendingChatKeyRef.current) const chatKeyRef = useRef(chatKey) chatKeyRef.current = chatKey @@ -1567,16 +1591,6 @@ export function useChat( const detachedChatResolutionControllersRef = useRef>(new Set()) const streamReaderRef = useRef | null>(null) const chatIdRef = useRef(initialChatId) - const pendingDesktopScopeIdRef = useRef( - desktopChatScopeId(scopeKey, undefined, pendingChatKeyRef.current) - ) - const initialDesktopScopeId = desktopChatScopeId( - scopeKey, - initialChatId, - pendingChatKeyRef.current - ) - const desktopScopeIdRef = useRef(initialDesktopScopeId) - const [desktopScopeId, setDesktopScopeId] = useState(initialDesktopScopeId) /** Panel/chat selection — drives createNewChat + request chatId; may differ from chatIdRef while a stream is still finishing. */ const selectedChatIdRef = useRef(initialChatId) selectedChatIdRef.current = initialChatId @@ -2479,9 +2493,9 @@ export function useChat( // 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. + // tab the chat opens on. When the server holds nothing it writes no + // fallback at all: the selection stays empty so the shown resource can be + // resolved against the tab the desktop app remembers. const selectedResourceId = selectedResourceIdRef.current const hydratedActiveResourceId = selectedResourceId && mergedResources.some((resource) => resource.id === selectedResourceId) @@ -2490,11 +2504,13 @@ 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 + // The ref is set eagerly so a request sent in this commit still + // attaches a resource, through the same rule the render path uses. + activeResourceIdRef.current = resolveEffectiveResourceId( + mergedResources, + hydratedActiveResourceId, + nativeActiveTabIdsRef.current + ) 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 5d47b8355cd..a2a06067d1d 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 @@ -1,6 +1,29 @@ -import { useEffect, useRef } from 'react' +import { useEffect, useMemo, useRef } from 'react' import type { MothershipResource, MothershipResourceType } from '@/lib/copilot/resources/types' +import { terminalResourceId } from '@/lib/terminal/resource-id' import type { ResourceEventHandler } from '@/app/workspace/[workspaceId]/home/hooks/use-chat' +import type { NativeActiveTabIds } from '@/app/workspace/[workspaceId]/home/resource-view-policy' +import { useBrowserSessionStore } from '@/stores/browser-session/store' +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' + +/** + * The tab the desktop app currently shows for a chat, per kind, as resource + * ids. The strip prefers these over its own last-resource fallback. Pass null + * from a surface that projects no desktop tabs, so it never re-renders for a + * native switch it cannot show. + */ +export function useNativeActiveTabIds(scopeId: string | null): NativeActiveTabIds { + const browser = useBrowserSessionStore((state) => + scopeId === null ? null : (state.sessions[scopeId]?.activeTabId ?? null) + ) + const terminal = useCopilotTerminalStore((state) => + scopeId === null ? null : (state.sessions[scopeId]?.tabs.activeTerminalId ?? null) + ) + return useMemo( + () => ({ browser, terminal: terminal ? terminalResourceId(terminal) : null }), + [browser, terminal] + ) +} /** One live desktop tab, as the strip needs to know it. */ export interface DesktopTab { @@ -9,7 +32,7 @@ export interface DesktopTab { } /** What the strip shares with every kind of desktop-backed resource tab. */ -export interface DesktopTabStripOptions { +export interface DesktopTabResourceOptions { /** Desktop scope whose live tabs back this chat's resource tabs. */ scopeId: string resources: readonly MothershipResource[] @@ -17,29 +40,16 @@ export interface DesktopTabStripOptions { 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`, 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. - */ - 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 { +interface UseDesktopTabResourcesOptions extends DesktopTabResourceOptions { type: 'browser' | 'terminal' /** The desktop app's live tab list for the scope, in its order. */ tabs: readonly DesktopTab[] @@ -57,41 +67,13 @@ interface UseDesktopTabResourcesOptions extends DesktopTabStripOptions { switchTab: (tabId: string, scopeId: string) => void } -/** - * 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 nativeTabOffStrip( +/** Whether the resource the strip shows is a tab of this kind. */ +function stripShowsKind( resources: readonly MothershipResource[], activeResourceId: string | null, - activeTabId: string | null, type: MothershipResourceType -): string | null { - if (!activeTabId || activeTabId === activeResourceId) return null +): boolean { 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 } /** @@ -103,10 +85,9 @@ function nativeTabToAdopt( * 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. 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. + * strip while the user is on that kind of tab. Which resource the strip shows + * when nothing is selected is resolved by `resolveEffectiveResourceId`, not + * here. * * 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 @@ -123,11 +104,9 @@ export function useDesktopTabResources({ resources, activeResourceId, selectedResourceId, - hydrated, addResource, removeResource, selectResource, - restoreResource, onResourceEvent, }: UseDesktopTabResourcesOptions): void { /** @@ -149,23 +128,16 @@ 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 explicitSelectionRef = useRef(explicitSelection) - explicitSelectionRef.current = explicitSelection - 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. + * The last tab the desktop app reported showing. Unset until its first + * report, which carries the tab it remembers rather than a switch. Never + * unset again by an empty tab list, so reopening a tab still reads as a move. */ const previousActiveTabIdRef = useRef(null) const switchTabRef = useRef(switchTab) switchTabRef.current = switchTab const selectResourceRef = useRef(selectResource) selectResourceRef.current = selectResource - const restoreResourceRef = useRef(restoreResource) - restoreResourceRef.current = restoreResource const onResourceEventRef = useRef(onResourceEvent) onResourceEventRef.current = onResourceEvent @@ -215,43 +187,24 @@ export function useDesktopTabResources({ switchTabRef.current(selectedResourceId, scopeIdRef.current) }, [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 - // strip adopts that one rather than showing a page the user did not pick. - useEffect(() => { - 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, 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. + // A native switch while the user is on this kind of tab claims the selection + // the way a click on the tab would, so later agent activity only badges + // rather than taking the view. Measured against the tab the desktop app was + // showing, not the one the strip shows: with no explicit selection those are + // the same tab, and comparing them would never see a switch. Two switches + // are not the user's — the one this hook requested itself, and the scope's + // first report, which carries the tab the desktop app remembers. useEffect(() => { const previousActiveTabId = previousActiveTabIdRef.current - previousActiveTabIdRef.current = activeTabId + if (activeTabId !== null) previousActiveTabIdRef.current = activeTabId if (requestedTabIdRef.current === activeTabId) { requestedTabIdRef.current = null return } - const activeResourceId = activeResourceIdRef.current - if (previousActiveTabId !== null) { - const tabId = nativeTabOffStrip(resourcesRef.current, activeResourceId, activeTabId, type) - if (tabId) selectResourceRef.current(tabId) - 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) + if (!activeTabId || previousActiveTabId === null) return + if (activeTabId === previousActiveTabId) return + if (!stripShowsKind(resourcesRef.current, activeResourceIdRef.current, type)) return + selectResourceRef.current(activeTabId) }, [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 2b69e762e77..013a36d540d 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,7 +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 type { DesktopTabResourceOptions } 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' @@ -39,7 +39,7 @@ function pushTabs(scopeId: string, tabs: TerminalTabState[], activeTerminalId: s }) } -function Host(props: DesktopTabStripOptions) { +function Host(props: DesktopTabResourceOptions) { useTerminalTabResources(props) return null } @@ -50,25 +50,22 @@ 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 = {}) { - const props: DesktopTabStripOptions = { + function render(overrides: Partial = {}) { + const props: DesktopTabResourceOptions = { scopeId: SCOPE, resources: [], activeResourceId: null, selectedResourceId: null, - hydrated: true, addResource, removeResource, selectResource, - restoreResource, onResourceEvent, ...overrides, } act(() => root.render()) - return (next: Partial) => + return (next: Partial) => act(() => root.render()) } @@ -131,23 +128,6 @@ describe('useTerminalTabResources', () => { 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' }, 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 b9cb6826138..ae5432d192e 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 @@ -3,7 +3,7 @@ import type { TerminalTabState } from '@sim/terminal-protocol' import { terminalIdFromResourceId, terminalResourceId } from '@/lib/terminal/resource-id' import { switchTerminal } from '@/lib/terminal/transport' import { - type DesktopTabStripOptions, + type DesktopTabResourceOptions, useDesktopTabResources, } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' @@ -20,7 +20,7 @@ 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(options: DesktopTabStripOptions): void { +export function useTerminalTabResources(options: DesktopTabResourceOptions): void { const { scopeId } = options const hasSession = useCopilotTerminalStore((state) => state.sessions[scopeId] !== undefined) const terminalTabs = useCopilotTerminalStore( @@ -49,8 +49,8 @@ export function useTerminalTabResources(options: DesktopTabStripOptions): void { type: 'terminal', tabs, hasSession, - activeTabId: activeTerminalId && terminalResourceId(activeTerminalId), - agentTabId: agentTerminalId && terminalResourceId(agentTerminalId), + activeTabId: activeTerminalId ? terminalResourceId(activeTerminalId) : null, + agentTabId: agentTerminalId ? terminalResourceId(agentTerminalId) : null, switchTab: showTerminal, }) } diff --git a/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.test.ts b/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.test.ts index a16456d5add..80435314cf0 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.test.ts @@ -1,9 +1,72 @@ import { describe, expect, it } from 'vitest' +import type { MothershipResource } from '@/lib/copilot/resources/types' import { + resolveEffectiveResourceId, resolveResourceEventPresentation, resolveResourceSelectionUpdate, } from '@/app/workspace/[workspaceId]/home/resource-view-policy' +const PAGES: MothershipResource[] = [ + { type: 'browser', id: '1', title: 'Page 1' }, + { type: 'browser', id: '2', title: 'Page 2' }, + { type: 'browser', id: '3', title: 'Page 3' }, +] +const NOTES: MothershipResource = { type: 'file', id: 'notes', title: 'notes.md' } +const SHELLS: MothershipResource[] = [ + { type: 'terminal', id: 'terminal:1', title: 'one' }, + { type: 'terminal', id: 'terminal:2', title: 'two' }, +] +const NO_NATIVE = { browser: null, terminal: null } + +describe('resolveEffectiveResourceId', () => { + it('shows nothing when the strip is empty', () => { + expect(resolveEffectiveResourceId([], null, NO_NATIVE)).toBeNull() + expect(resolveEffectiveResourceId([], 'anything', NO_NATIVE)).toBeNull() + }) + + it('shows the selected resource while it is on screen', () => { + expect(resolveEffectiveResourceId(PAGES, '1', { browser: '3', terminal: null })).toBe('1') + }) + + it('falls back when the selection is no longer on screen', () => { + expect(resolveEffectiveResourceId(PAGES, 'deleted', NO_NATIVE)).toBe('3') + }) + + it('prefers the page the desktop app shows over the last one', () => { + expect(resolveEffectiveResourceId(PAGES, null, { browser: '2', terminal: null })).toBe('2') + }) + + it('prefers the shell the desktop app shows over the last one', () => { + expect( + resolveEffectiveResourceId(SHELLS, null, { browser: null, terminal: 'terminal:1' }) + ).toBe('terminal:1') + }) + + it('keeps the last resource when the desktop app reports nothing', () => { + expect(resolveEffectiveResourceId(PAGES, null, NO_NATIVE)).toBe('3') + expect(resolveEffectiveResourceId(PAGES, null)).toBe('3') + }) + + it('ignores a page the strip no longer holds, such as one just closed', () => { + expect(resolveEffectiveResourceId(PAGES, null, { browser: 'closed', terminal: null })).toBe('3') + }) + + it('ignores the desktop app when the last resource is not one of its tabs', () => { + expect( + resolveEffectiveResourceId([...PAGES, NOTES], null, { browser: '2', terminal: null }) + ).toBe('notes') + }) + + it('does not cross the two desktop kinds', () => { + expect(resolveEffectiveResourceId(PAGES, null, { browser: null, terminal: 'terminal:1' })).toBe( + '3' + ) + expect(resolveEffectiveResourceId(SHELLS, null, { browser: '1', terminal: null })).toBe( + 'terminal:2' + ) + }) +}) + const DEFAULT_INPUT = { activeResourceId: 'file-1', activationRequested: true, diff --git a/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.ts b/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.ts index d163b95259e..378f9ad8947 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/resource-view-policy.ts @@ -1,4 +1,44 @@ import type { SetStateAction } from 'react' +import type { MothershipResource } from '@/lib/copilot/resources/types' + +/** The tab each desktop-backed kind currently shows, as resource ids. */ +export interface NativeActiveTabIds { + browser: string | null + terminal: string | null +} + +/** + * Which resource the panel shows. + * + * An explicit selection wins whenever it is still on screen. Otherwise the + * strip falls back to its last resource — except for the desktop-backed kinds, + * where the desktop app is already showing the tab the user left the chat on. + * Preferring that tab is what makes reopening a chat land where the user left + * it, and deriving it here rather than writing it back means no arrival order + * of tabs, history or native state can leave a tab the user did not pick + * stored as their selection. + */ +export function resolveEffectiveResourceId( + resources: readonly MothershipResource[], + selectedResourceId: string | null, + nativeActiveTabIds?: NativeActiveTabIds +): string | null { + if (resources.length === 0) return null + if (selectedResourceId && resources.some((resource) => resource.id === selectedResourceId)) { + return selectedResourceId + } + const fallback = resources[resources.length - 1] + if (fallback.type === 'browser' || fallback.type === 'terminal') { + const nativeId = nativeActiveTabIds?.[fallback.type] ?? null + if ( + nativeId && + resources.some((resource) => resource.type === fallback.type && resource.id === nativeId) + ) { + return nativeId + } + } + return fallback.id +} export function resolveResourceSelectionUpdate( currentResourceId: string | null,