Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
]
)

/**
Expand Down
23 changes: 10 additions & 13 deletions apps/sim/app/workspace/[workspaceId]/home/home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -334,24 +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,
...desktopTabResourceCallbacks,
})
useTerminalTabResources({
scopeId: desktopScopeId,
resources,
activeResourceId,
...desktopTabResourceCallbacks,
})
useBrowserTabResources(desktopTabResourceOptions)
useTerminalTabResources(desktopTabResourceOptions)

const addResourceFromUser = useCallback(
(resource: MothershipResource) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => ({
Expand Down Expand Up @@ -37,17 +38,7 @@ function pushTabs(scopeId: string, tabs: ReturnType<typeof tab>[], 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: DesktopTabStripOptions) {
useBrowserTabResources(props)
return null
}
Expand All @@ -58,21 +49,26 @@ 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<HostProps> = {}) {
const props: HostProps = {
function render(overrides: Partial<DesktopTabStripOptions> = {}) {
const props: DesktopTabStripOptions = {
scopeId: SCOPE,
resources: [],
activeResourceId: null,
selectedResourceId: null,
hydrated: true,
addResource,
removeResource,
selectResource,
restoreResource,
onResourceEvent,
...overrides,
}
act(() => root.render(<Host {...props} />))
return (next: Partial<HostProps>) => act(() => root.render(<Host {...props} {...next} />))
return (next: Partial<DesktopTabStripOptions>) =>
act(() => root.render(<Host {...props} {...next} />))
}

beforeEach(() => {
Expand Down Expand Up @@ -153,11 +149,27 @@ 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('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 },
Expand All @@ -169,21 +181,190 @@ describe('useBrowserTabResources', () => {
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', () => {
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('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' },
{ 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 moved the desktop app off page 1.
pushTabs(SCOPE, [tab('1'), tab('2', true)], '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', () => {
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')
expect(selectResource).toHaveBeenCalledExactlyOnceWith('2')
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()
})
Expand All @@ -192,6 +373,7 @@ describe('useBrowserTabResources', () => {
render({
resources: [{ type: 'browser', id: '1', title: 'Page 1' }],
activeResourceId: '1',
selectedResourceId: '1',
})
pushTabs(SCOPE, [tab('1', true)], '1')
act(() => {
Expand Down
Loading
Loading