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..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 @@ -408,11 +408,21 @@ 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, 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 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 // 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 +461,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 c169af084e8..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) } @@ -334,24 +334,18 @@ export function Home({ chatId, userName, userId }: HomeProps) { [setActiveResourceId, clearResourceActivity] ) - const desktopTabResourceCallbacks = { + const desktopTabResourceOptions = { + scopeId: desktopScopeId, + resources, + activeResourceId, + selectedResourceId: activeResourceParam, addResource, removeResource, selectResource: selectResourceFromUser, onResourceEvent: handleResourceEvent, } - useBrowserTabResources({ - scopeId: desktopScopeId, - resources, - activeResourceId, - ...desktopTabResourceCallbacks, - }) - useTerminalTabResources({ - scopeId: desktopScopeId, - resources, - activeResourceId, - ...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 2563e158eac..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,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 { DesktopTabResourceOptions } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' const { sendBrowserPanelAction, openUrlInNewBrowserTab, openInPanelListeners } = vi.hoisted(() => ({ @@ -37,17 +38,7 @@ function pushTabs(scopeId: string, tabs: ReturnType[], activeTabId: }) } -interface HostProps { - scopeId: string - resources: MothershipResource[] - activeResourceId: string | null - addResource: (resource: MothershipResource) => void - removeResource: (type: MothershipResource['type'], id: string) => void - selectResource: (id: string) => void - onResourceEvent: (id: string, options?: { activate?: boolean }) => void -} - -function Host(props: HostProps) { +function Host(props: DesktopTabResourceOptions) { useBrowserTabResources(props) return null } @@ -60,11 +51,12 @@ describe('useBrowserTabResources', () => { const selectResource = vi.fn() const onResourceEvent = vi.fn() - function render(overrides: Partial = {}) { - const props: HostProps = { + function render(overrides: Partial = {}) { + const props: DesktopTabResourceOptions = { scopeId: SCOPE, resources: [], activeResourceId: null, + selectedResourceId: null, addResource, removeResource, selectResource, @@ -72,7 +64,12 @@ describe('useBrowserTabResources', () => { ...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(() => { @@ -153,11 +150,11 @@ 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 }, @@ -169,13 +166,62 @@ 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 + ) + }) + + 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() + pushTabs(SCOPE, [tab('1'), tab('2')], null) + rerender({ resources, activeResourceId: '2', selectedResourceId: null }) + + // 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(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') + rerender({ resources, activeResourceId: '1', selectedResourceId: null }) + expect(selectResource).not.toHaveBeenCalled() + + // 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') + }) + it('follows a native switch into the strip only while the user is on the browser', () => { const resources: MothershipResource[] = [ { type: 'browser', id: '1', title: 'Page 1' }, { 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 +229,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 +238,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..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 @@ -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 DesktopTabResourceOptions, useDesktopTabResources, } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' @@ -16,13 +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 -} - function switchBrowserTab(tabId: string, scopeId: string): void { sendBrowserPanelAction('switch-tab', { tabId, claim: false }, scopeId) } @@ -31,15 +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, - addResource, - removeResource, - selectResource, - onResourceEvent, -}: UseBrowserTabResourcesOptions): void { +export function useBrowserTabResources(options: DesktopTabResourceOptions): void { + const { scopeId, selectResource } = options const hasSession = useBrowserSessionStore((state) => state.sessions[scopeId] !== undefined) const browserTabs = useBrowserSessionStore( (state) => state.sessions[scopeId]?.tabs ?? EMPTY_BROWSER_TABS @@ -64,19 +49,13 @@ export function useBrowserTabResources({ selectResourceRef.current = selectResource useDesktopTabResources({ + ...options, type: 'browser', - scopeId, tabs, hasSession, activeTabId, agentTabId, switchTab: switchBrowserTab, - resources, - activeResourceId, - addResource, - removeResource, - selectResource, - 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-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index 0ac26689ac1..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 @@ -2476,22 +2490,27 @@ 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 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) ? 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 + // 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 865ad133f15..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 { @@ -8,7 +31,15 @@ export interface DesktopTab { title: string } -export interface DesktopTabResourceCallbacks { +/** What the strip shares with every kind of desktop-backed resource tab. */ +export interface DesktopTabResourceOptions { + /** 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 /** Adds a tab without activating it; activation goes through {@link onResourceEvent}. */ addResource: (resource: MothershipResource) => void removeResource: (resourceType: MothershipResourceType, resourceId: string) => void @@ -18,10 +49,8 @@ export interface DesktopTabResourceCallbacks { onResourceEvent: ResourceEventHandler } -interface UseDesktopTabResourcesOptions extends DesktopTabResourceCallbacks { +interface UseDesktopTabResourcesOptions extends DesktopTabResourceOptions { type: 'browser' | 'terminal' - /** Desktop scope whose live tabs back this chat's resource tabs. */ - scopeId: string /** The desktop app's live tab list for the scope, in its order. */ tabs: readonly DesktopTab[] /** @@ -36,8 +65,15 @@ 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[] - activeResourceId: string | null +} + +/** Whether the resource the strip shows is a tab of this kind. */ +function stripShowsKind( + resources: readonly MothershipResource[], + activeResourceId: string | null, + type: MothershipResourceType +): boolean { + return resources.find((resource) => resource.id === activeResourceId)?.type === type } /** @@ -49,7 +85,9 @@ 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. 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 @@ -65,6 +103,7 @@ export function useDesktopTabResources({ switchTab, resources, activeResourceId, + selectedResourceId, addResource, removeResource, selectResource, @@ -83,14 +122,18 @@ 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) resourcesRef.current = resources const activeResourceIdRef = useRef(activeResourceId) activeResourceIdRef.current = activeResourceId + /** + * 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) @@ -105,6 +148,7 @@ export function useDesktopTabResources({ knownScopeRef.current = scopeId known.clear() requestedTabIdRef.current = null + previousActiveTabIdRef.current = null } const resourceTabIds = new Set( resources.filter((resource) => resource.type === type).map((resource) => resource.id) @@ -127,29 +171,39 @@ 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. + const selectedTabIsLive = + selectedResourceId !== null && tabs.some((tab) => tab.id === selectedResourceId) + + // Selecting a resource tab shows its native tab. Keyed on the explicit + // 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 (!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 || !selectedTabIsLive) return + if (selectedResourceId === activeTabIdRef.current) return + requestedTabIdRef.current = selectedResourceId + switchTabRef.current(selectedResourceId, scopeIdRef.current) + }, [selectedResourceId, selectedTabIsLive]) - // 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. + // 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 + if (activeTabId !== null) 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) { - return - } + if (!activeTabId || previousActiveTabId === null) return + if (activeTabId === previousActiveTabId) return + if (!stripShowsKind(resourcesRef.current, activeResourceIdRef.current, type)) return selectResourceRef.current(activeTabId) }, [activeTabId, type]) 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..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,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 { 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' @@ -38,17 +39,7 @@ function pushTabs(scopeId: string, tabs: TerminalTabState[], activeTerminalId: s }) } -interface HostProps { - scopeId: string - resources: MothershipResource[] - activeResourceId: string | null - addResource: (resource: MothershipResource) => void - removeResource: (type: MothershipResource['type'], id: string) => void - selectResource: (id: string) => void - onResourceEvent: (id: string, options?: { activate?: boolean }) => void -} - -function Host(props: HostProps) { +function Host(props: DesktopTabResourceOptions) { useTerminalTabResources(props) return null } @@ -61,11 +52,12 @@ describe('useTerminalTabResources', () => { const selectResource = vi.fn() const onResourceEvent = vi.fn() - function render(overrides: Partial = {}) { - const props: HostProps = { + function render(overrides: Partial = {}) { + const props: DesktopTabResourceOptions = { scopeId: SCOPE, resources: [], activeResourceId: null, + selectedResourceId: null, addResource, removeResource, selectResource, @@ -73,7 +65,8 @@ describe('useTerminalTabResources', () => { ...overrides, } act(() => root.render()) - return (next: Partial) => act(() => root.render()) + return (next: Partial) => + act(() => root.render()) } beforeEach(() => { @@ -120,11 +113,15 @@ 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') @@ -137,14 +134,18 @@ describe('useTerminalTabResources', () => { { 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 +157,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..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 @@ -1,23 +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 DesktopTabResourceOptions, 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 -} - function showTerminal(resourceId: string, scopeId: string): void { void switchTerminal(terminalIdFromResourceId(resourceId), scopeId, { claim: false }).catch( () => {} @@ -28,15 +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, - addResource, - removeResource, - selectResource, - onResourceEvent, -}: UseTerminalTabResourcesOptions): void { +export function useTerminalTabResources(options: DesktopTabResourceOptions): void { + const { scopeId } = options const hasSession = useCopilotTerminalStore((state) => state.sessions[scopeId] !== undefined) const terminalTabs = useCopilotTerminalStore( (state) => state.sessions[scopeId]?.tabs.tabs ?? EMPTY_TERMINAL_TABS @@ -60,18 +45,12 @@ export function useTerminalTabResources({ ) useDesktopTabResources({ + ...options, type: 'terminal', - scopeId, tabs, hasSession, - activeTabId: activeTerminalId && terminalResourceId(activeTerminalId), - agentTabId: agentTerminalId && terminalResourceId(agentTerminalId), + activeTabId: activeTerminalId ? terminalResourceId(activeTerminalId) : null, + agentTabId: agentTerminalId ? terminalResourceId(agentTerminalId) : null, switchTab: showTerminal, - resources, - activeResourceId, - addResource, - removeResource, - selectResource, - onResourceEvent, }) } 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,