Skip to content

Commit 2799994

Browse files
waleedlatif1claude
andauthored
fix(desktop): reopen a chat on the browser tab the user left it on (#7793)
* 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. * 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ * 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ * refactor(desktop): align the two tab-adoption paths and drop dead plumbing 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ * 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ * 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139YonWmiZUnPMTHoH4PtAJ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1a7c827 commit 2799994

10 files changed

Lines changed: 358 additions & 162 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,11 +408,21 @@ export function ResourceTabs({
408408

409409
const handleClose = useCallback(
410410
(id: string) => {
411-
const resource = resources.find((r) => r.id === id)
411+
const index = resources.findIndex((r) => r.id === id)
412+
const resource = resources[index]
412413
if (!resource) return
413414
const isMulti = selectedIds.has(resource.id) && selectedIds.size > 1
414415
const targets = isMulti ? resources.filter((r) => selectedIds.has(r.id)) : [resource]
415416
if (!confirmClosingRunningTerminals(targets, terminalTabs)) return
417+
// Closing the shown tab moves to its neighbour, right then left, so the
418+
// strip does not fall back to its last tab and jump. For a desktop tab
419+
// this is also the neighbour the desktop app itself picks.
420+
if (!isMulti && activeId === resource.id) {
421+
const sameKind = new Set(resources.filter((r) => r.type === resource.type).map((r) => r.id))
422+
const nextId =
423+
findNearestId(resources, index, sameKind) ?? findNearestId(resources, index, null)
424+
if (nextId) selectResource(nextId)
425+
}
416426
// A browser tab's page is closed natively and its resource dropped at
417427
// once; the tab list then confirms the removal. A shell's close answers
418428
// with the tab list, so its resource follows that list instead — a
@@ -451,7 +461,16 @@ export function ResourceTabs({
451461
}
452462
},
453463
// eslint-disable-next-line react-hooks/exhaustive-deps
454-
[chatId, desktopScopeId, onRemoveResource, resources, selectedIds, terminalTabs]
464+
[
465+
activeId,
466+
chatId,
467+
desktopScopeId,
468+
onRemoveResource,
469+
resources,
470+
selectResource,
471+
selectedIds,
472+
terminalTabs,
473+
]
455474
)
456475

457476
/**

apps/sim/app/workspace/[workspaceId]/home/home.tsx

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ export function Home({ chatId, userName, userId }: HomeProps) {
224224
const resourceSelectionOwnedByUserRef = useRef(false)
225225

226226
function handleResourceEvent(resourceId: string, options?: ResourceEventOptions) {
227-
const activeResourceId = activeResourceParamRef.current
227+
const activeResourceId = effectiveActiveResourceIdRef.current
228228
const presentation = resolveResourceEventPresentation({
229229
activeResourceId,
230230
activationRequested: shouldActivateResourceEvent(activeResourceId, resourceId, options),
@@ -317,7 +317,7 @@ export function Home({ chatId, userName, userId }: HomeProps) {
317317
const expandResource = () => {
318318
resourceCollapseOwnedByUserRef.current = false
319319
resourceSelectionOwnedByUserRef.current = true
320-
const activeResourceId = activeResourceParamRef.current
320+
const activeResourceId = effectiveActiveResourceIdRef.current
321321
if (activeResourceId) clearResourceActivity(activeResourceId)
322322
setResourceCollapsed(false)
323323
}
@@ -334,24 +334,18 @@ export function Home({ chatId, userName, userId }: HomeProps) {
334334
[setActiveResourceId, clearResourceActivity]
335335
)
336336

337-
const desktopTabResourceCallbacks = {
337+
const desktopTabResourceOptions = {
338+
scopeId: desktopScopeId,
339+
resources,
340+
activeResourceId,
341+
selectedResourceId: activeResourceParam,
338342
addResource,
339343
removeResource,
340344
selectResource: selectResourceFromUser,
341345
onResourceEvent: handleResourceEvent,
342346
}
343-
useBrowserTabResources({
344-
scopeId: desktopScopeId,
345-
resources,
346-
activeResourceId,
347-
...desktopTabResourceCallbacks,
348-
})
349-
useTerminalTabResources({
350-
scopeId: desktopScopeId,
351-
resources,
352-
activeResourceId,
353-
...desktopTabResourceCallbacks,
354-
})
347+
useBrowserTabResources(desktopTabResourceOptions)
348+
useTerminalTabResources(desktopTabResourceOptions)
355349

356350
const addResourceFromUser = useCallback(
357351
(resource: MothershipResource) => {

apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.test.tsx

Lines changed: 65 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createRoot, type Root } from 'react-dom/client'
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { MothershipResource } from '@/lib/copilot/resources/types'
88
import { useBrowserTabResources } from '@/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources'
9+
import type { DesktopTabResourceOptions } from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources'
910
import { useBrowserSessionStore } from '@/stores/browser-session/store'
1011

1112
const { sendBrowserPanelAction, openUrlInNewBrowserTab, openInPanelListeners } = vi.hoisted(() => ({
@@ -37,17 +38,7 @@ function pushTabs(scopeId: string, tabs: ReturnType<typeof tab>[], activeTabId:
3738
})
3839
}
3940

40-
interface HostProps {
41-
scopeId: string
42-
resources: MothershipResource[]
43-
activeResourceId: string | null
44-
addResource: (resource: MothershipResource) => void
45-
removeResource: (type: MothershipResource['type'], id: string) => void
46-
selectResource: (id: string) => void
47-
onResourceEvent: (id: string, options?: { activate?: boolean }) => void
48-
}
49-
50-
function Host(props: HostProps) {
41+
function Host(props: DesktopTabResourceOptions) {
5142
useBrowserTabResources(props)
5243
return null
5344
}
@@ -60,19 +51,25 @@ describe('useBrowserTabResources', () => {
6051
const selectResource = vi.fn()
6152
const onResourceEvent = vi.fn()
6253

63-
function render(overrides: Partial<HostProps> = {}) {
64-
const props: HostProps = {
54+
function render(overrides: Partial<DesktopTabResourceOptions> = {}) {
55+
const props: DesktopTabResourceOptions = {
6556
scopeId: SCOPE,
6657
resources: [],
6758
activeResourceId: null,
59+
selectedResourceId: null,
6860
addResource,
6961
removeResource,
7062
selectResource,
7163
onResourceEvent,
7264
...overrides,
7365
}
7466
act(() => root.render(<Host {...props} />))
75-
return (next: Partial<HostProps>) => act(() => root.render(<Host {...props} {...next} />))
67+
/** `alsoInThisCommit` lands a store push and the new props together. */
68+
return (next: Partial<DesktopTabResourceOptions>, alsoInThisCommit?: () => void) =>
69+
act(() => {
70+
alsoInThisCommit?.()
71+
root.render(<Host {...props} {...next} />)
72+
})
7673
}
7774

7875
beforeEach(() => {
@@ -153,11 +150,11 @@ describe('useBrowserTabResources', () => {
153150
{ type: 'browser', id: '1', title: 'Page 1' },
154151
{ type: 'browser', id: '2', title: 'Page 2' },
155152
]
156-
const rerender = render({ resources, activeResourceId: '1' })
153+
const rerender = render({ resources, activeResourceId: '1', selectedResourceId: '1' })
157154
pushTabs(SCOPE, [tab('1', true), tab('2')], '1')
158155
expect(sendBrowserPanelAction).not.toHaveBeenCalled()
159156

160-
rerender({ activeResourceId: '2' })
157+
rerender({ activeResourceId: '2', selectedResourceId: '2' })
161158
expect(sendBrowserPanelAction).toHaveBeenCalledExactlyOnceWith(
162159
'switch-tab',
163160
{ tabId: '2', claim: false },
@@ -169,21 +166,70 @@ describe('useBrowserTabResources', () => {
169166
expect(selectResource).not.toHaveBeenCalled()
170167
})
171168

169+
it('shows a page selected before the pages landed, once it arrives', () => {
170+
render({ selectedResourceId: '2', activeResourceId: '2' })
171+
expect(sendBrowserPanelAction).not.toHaveBeenCalled()
172+
173+
pushTabs(SCOPE, [tab('1', true), tab('2')], '1')
174+
expect(sendBrowserPanelAction).toHaveBeenCalledExactlyOnceWith(
175+
'switch-tab',
176+
{ tabId: '2', claim: false },
177+
SCOPE
178+
)
179+
})
180+
181+
it('does not claim the scope first report as a user switch', () => {
182+
const resources: MothershipResource[] = [
183+
{ type: 'browser', id: '1', title: 'Page 1' },
184+
{ type: 'browser', id: '2', title: 'Page 2' },
185+
]
186+
const rerender = render()
187+
pushTabs(SCOPE, [tab('1'), tab('2')], null)
188+
rerender({ resources, activeResourceId: '2', selectedResourceId: null })
189+
190+
// The desktop app reports the page it restored. The strip resolves to that
191+
// page on its own, so there is nothing here to claim for the user.
192+
pushTabs(SCOPE, [tab('1', true), tab('2')], '1')
193+
expect(selectResource).not.toHaveBeenCalled()
194+
expect(sendBrowserPanelAction).not.toHaveBeenCalled()
195+
})
196+
197+
it('claims a native switch away from a page it was already showing', () => {
198+
const resources: MothershipResource[] = [
199+
{ type: 'browser', id: '1', title: 'Page 1' },
200+
{ type: 'browser', id: '2', title: 'Page 2' },
201+
]
202+
const rerender = render()
203+
pushTabs(SCOPE, [tab('1', true), tab('2')], '1')
204+
rerender({ resources, activeResourceId: '1', selectedResourceId: null })
205+
expect(selectResource).not.toHaveBeenCalled()
206+
207+
// A keyboard shortcut in the page moves the desktop app to page 2. With no
208+
// explicit selection the strip resolves to that page in the same commit,
209+
// so the switch is only visible against the page the desktop app left.
210+
rerender({ resources, activeResourceId: '2' }, () => {
211+
useBrowserSessionStore
212+
.getState()
213+
.setTabsState({ scopeId: SCOPE, tabs: [tab('1'), tab('2', true)], activeTabId: '2' })
214+
})
215+
expect(selectResource).toHaveBeenCalledExactlyOnceWith('2')
216+
})
217+
172218
it('follows a native switch into the strip only while the user is on the browser', () => {
173219
const resources: MothershipResource[] = [
174220
{ type: 'browser', id: '1', title: 'Page 1' },
175221
{ type: 'browser', id: '2', title: 'Page 2' },
176222
{ type: 'file', id: 'f', title: 'notes.md' },
177223
]
178-
const rerender = render({ resources, activeResourceId: '1' })
224+
const rerender = render({ resources, activeResourceId: '1', selectedResourceId: '1' })
179225
pushTabs(SCOPE, [tab('1', true), tab('2')], '1')
180226

181227
pushTabs(SCOPE, [tab('1'), tab('2', true)], '2')
182228
expect(selectResource).toHaveBeenCalledExactlyOnceWith('2')
183229
expect(sendBrowserPanelAction).not.toHaveBeenCalled()
184230

185231
selectResource.mockClear()
186-
rerender({ activeResourceId: 'f' })
232+
rerender({ activeResourceId: 'f', selectedResourceId: 'f' })
187233
pushTabs(SCOPE, [tab('1', true), tab('2')], '1')
188234
expect(selectResource).not.toHaveBeenCalled()
189235
})
@@ -192,6 +238,7 @@ describe('useBrowserTabResources', () => {
192238
render({
193239
resources: [{ type: 'browser', id: '1', title: 'Page 1' }],
194240
activeResourceId: '1',
241+
selectedResourceId: '1',
195242
})
196243
pushTabs(SCOPE, [tab('1', true)], '1')
197244
act(() => {

apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ import { getErrorMessage } from '@sim/utils/errors'
55
import { onOpenInBrowserPanel } from '@/lib/browser-agent/open-in-panel'
66
import { browserTabTitle } from '@/lib/browser-agent/tab-label'
77
import { openUrlInNewBrowserTab, sendBrowserPanelAction } from '@/lib/browser-agent/transport'
8-
import type { MothershipResource } from '@/lib/copilot/resources/types'
98
import {
10-
type DesktopTabResourceCallbacks,
9+
type DesktopTabResourceOptions,
1110
useDesktopTabResources,
1211
} from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources'
1312
import { useBrowserSessionStore } from '@/stores/browser-session/store'
@@ -16,13 +15,6 @@ const logger = createLogger('BrowserTabResources')
1615

1716
const EMPTY_BROWSER_TABS: BrowserTabState[] = []
1817

19-
interface UseBrowserTabResourcesOptions extends DesktopTabResourceCallbacks {
20-
/** Desktop browser scope whose pages back this chat's browser tabs. */
21-
scopeId: string
22-
resources: readonly MothershipResource[]
23-
activeResourceId: string | null
24-
}
25-
2618
function switchBrowserTab(tabId: string, scopeId: string): void {
2719
sendBrowserPanelAction('switch-tab', { tabId, claim: false }, scopeId)
2820
}
@@ -31,15 +23,8 @@ function switchBrowserTab(tabId: string, scopeId: string): void {
3123
* Projects the desktop app's live browser pages into `browser` resource tabs,
3224
* one per page. See {@link useDesktopTabResources} for the shared model.
3325
*/
34-
export function useBrowserTabResources({
35-
scopeId,
36-
resources,
37-
activeResourceId,
38-
addResource,
39-
removeResource,
40-
selectResource,
41-
onResourceEvent,
42-
}: UseBrowserTabResourcesOptions): void {
26+
export function useBrowserTabResources(options: DesktopTabResourceOptions): void {
27+
const { scopeId, selectResource } = options
4328
const hasSession = useBrowserSessionStore((state) => state.sessions[scopeId] !== undefined)
4429
const browserTabs = useBrowserSessionStore(
4530
(state) => state.sessions[scopeId]?.tabs ?? EMPTY_BROWSER_TABS
@@ -64,19 +49,13 @@ export function useBrowserTabResources({
6449
selectResourceRef.current = selectResource
6550

6651
useDesktopTabResources({
52+
...options,
6753
type: 'browser',
68-
scopeId,
6954
tabs,
7055
hasSession,
7156
activeTabId,
7257
agentTabId,
7358
switchTab: switchBrowserTab,
74-
resources,
75-
activeResourceId,
76-
addResource,
77-
removeResource,
78-
selectResource,
79-
onResourceEvent,
8059
})
8160

8261
// Chat links clicked in the desktop app open in a new browser tab. The user

0 commit comments

Comments
 (0)