diff --git a/apps/desktop/src/app/chat/preview-tile.tsx b/apps/desktop/src/app/chat/preview-tile.tsx index b9776adb05..0c79369d31 100644 --- a/apps/desktop/src/app/chat/preview-tile.tsx +++ b/apps/desktop/src/app/chat/preview-tile.tsx @@ -242,10 +242,15 @@ export function watchPreviewTiles(): void { } $rightRailActiveTabId.listen(reveal) - // Visible-list changes: a re-open of the already-active tab refreshes the - // target through the visible list too, while tab mutations belonging to a - // BACKGROUND session must not try to reveal panes that are not mounted. - $visiblePreviewTabs.listen(reveal) + // One listener for the visible list: re-home the selection FIRST (a session + // switch or an unpin can leave the active tab outside the visible set, and + // a stale reveal of a just-hidden tab is a no-op — its pane left the tree), + // THEN reveal the tab the re-home left active. Registered after the mirror, + // so the pane set is already synced when this runs. + $visiblePreviewTabs.listen(() => { + rehome() + reveal() + }) // A session switch (or an unpin) can leave the active tab outside the // visible set — re-home the selection to the first visible tab so the strip @@ -258,8 +263,6 @@ export function watchPreviewTiles(): void { } } - $visiblePreviewTabs.listen(rehome) - // And the reverse: clicking a preview TAB activates its pane in the TREE // only, so the store's selection must follow or `$previewTarget` (⌘L quote // labels, the titlebar's has-preview state) keeps reporting the previous diff --git a/apps/desktop/src/store/preview.test.ts b/apps/desktop/src/store/preview.test.ts index 229d99dc9b..be7a4f5de1 100644 --- a/apps/desktop/src/store/preview.test.ts +++ b/apps/desktop/src/store/preview.test.ts @@ -1,5 +1,8 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { group } from '@/components/pane-shell/tree/model' +import { $layoutTree, noteActiveTreeGroup } from '@/components/pane-shell/tree/store' + import { $rightRailActiveTabId, selectRightRailTab } from './layout' import { $browserPages, @@ -526,6 +529,79 @@ describe('preview session scoping', () => { expect($visiblePreviewTabs.get()).toHaveLength(1) }) + it('reusing an owned pinned row from another session keeps the owner id', () => { + $selectedStoredSessionId.set('sess-1') + openPreview(fileTarget('/work/a.html')) + setPreviewTabPinned($previewTabs.get()[0]!.id, true) + + $selectedStoredSessionId.set('sess-2') + openPreview({ ...fileTarget('/work/a.html'), label: 'refreshed' }) + + expect($previewTabs.get()).toHaveLength(1) + // The row keeps its OWNER's id — never an id/sessionId split (the id + // would say sess-2 while the owner says sess-1 until the next unpin). + expect($previewTabs.get()[0]).toMatchObject({ + id: 'file:sess-1:/work/a.html', + sessionId: 'sess-1', + pinned: true + }) + expect($previewTabs.get()[0]?.target.label).toBe('refreshed') + expect($rightRailActiveTabId.get()).toBe('file:sess-1:/work/a.html') + }) + + it('follows the focused session tile over the sidebar selection', () => { + $selectedStoredSessionId.set('sess-1') + openPreview(fileTarget('/work/a.html')) + $selectedStoredSessionId.set('sess-2') + openPreview(fileTarget('/work/b.html')) + + expect($visiblePreviewTabs.get().map(tab => tab.target.path)).toEqual(['/work/b.html']) + + // A sess-1 session TILE is active in the interacted zone while sess-2 is + // selected in the sidebar: the drawer belongs to the conversation being + // typed in, so the visible set follows the FOCUSED session. + const previousTree = $layoutTree.get() + + try { + $layoutTree.set(group(['session-tile:sess-1'], { active: 'session-tile:sess-1', id: 'grp-main' })) + noteActiveTreeGroup('grp-main') + + expect($visiblePreviewTabs.get().map(tab => tab.target.path)).toEqual(['/work/a.html']) + + // Leaving the tile (workspace/route active) falls back to the selection. + noteActiveTreeGroup(null) + expect($visiblePreviewTabs.get().map(tab => tab.target.path)).toEqual(['/work/b.html']) + } finally { + // A failed assertion must not leak the tree/group state into the next + // test — the focused derivation reads both. + noteActiveTreeGroup(null) + $layoutTree.set(previousTree) + } + }) + + it('re-owns the singleton Browser to the session that navigates it', () => { + $selectedStoredSessionId.set('sess-1') + openPreview(urlTarget('http://localhost:5174')) + + // A second session navigating the Browser takes the surface with it: + // keeping the original owner would leave the new URL invisible in the + // session that just opened it. + $selectedStoredSessionId.set('sess-2') + openPreview(urlTarget('http://localhost:9999')) + + expect($previewTabs.get()).toHaveLength(1) + // The Browser id is minted (never reused), so assert the stable parts: + // one surface, re-owned to the navigating session. + expect($previewTabs.get()[0]?.id).toMatch(/^url:browser-/) + expect($previewTabs.get()[0]).toMatchObject({ sessionId: 'sess-2' }) + expect($previewTabs.get()[0]?.target.url).toBe('http://localhost:9999') + expect($visiblePreviewTabs.get()).toHaveLength(1) + + // And sess-1 no longer sees the navigated surface — no context leak. + $selectedStoredSessionId.set('sess-1') + expect($visiblePreviewTabs.get()).toHaveLength(0) + }) + it('adoption dedupes against an already session-scoped row', () => { // Both rows for the same file coexist before the session appears. $previewTabs.set([ diff --git a/apps/desktop/src/store/preview.ts b/apps/desktop/src/store/preview.ts index 8c83db786f..10c7a4f91f 100644 --- a/apps/desktop/src/store/preview.ts +++ b/apps/desktop/src/store/preview.ts @@ -176,6 +176,7 @@ function parseTabList(parsed: unknown): PreviewTab[] { tab.target.kind === 'url' || tab.target.kind === 'file' ? previewTabId(tab.target, tab.sessionId) : tab.id + deduped.set(id, { ...tab, id }) } @@ -400,6 +401,7 @@ $focusedStoredSessionId.listen(sessionId => { if (tabs.some(tab => tab.sessionId == null && !tab.pinned)) { const activeId = $rightRailActiveTabId.get() let newActiveId = activeId + const updated = tabs.map(tab => { if (tab.sessionId != null || tab.pinned) { return tab @@ -750,24 +752,48 @@ export function openPreview(target: PreviewTarget) { const sessionId = $focusedStoredSessionId.get() ?? undefined const id = target.kind === 'url' ? browserTabId(current) : previewTabId(target, sessionId) const index = current.findIndex(tab => tab.id === id) + const existing = index === -1 && target.kind === 'file' ? current.find( tab => tab.pinned && tab.target.kind === 'file' && canonicalFileKey(tab.target) === canonicalFileKey(target) ) : current[index] + + // Reuse semantics depend on WHY the row was found: + // - Same-session re-open (any kind): keep the owner and pin state — only + // the target refreshes. + // - A PINNED file row reached from ANOTHER session: keep its owner (and + // therefore its owner-keyed id) — the pin is the explicit cross-session + // workspace contract, and rekeying to the opening session would leave + // `id` and `sessionId` disagreeing until the next unpin (which returns + // the tab to that owner anyway). + // - The singleton Browser or an artifact reached from another session: the + // surface NAVIGATES, so it re-owns to the opening session — otherwise the + // new URL would stay invisible in the session that just opened it. + // - Ownerless legacy rows: adopted — rekeyed and stamped — into the opening + // session (see the pinned-legacy test). + // The id is derived from the FINAL session id so the two can never split. + const tabSessionId = + existing?.sessionId == null || existing.pinned || existing.sessionId === sessionId + ? existing?.sessionId ?? sessionId + : sessionId + const tab: PreviewTab = { - id, + id: target.kind === 'url' ? id : previewTabId(target, tabSessionId), target: withRenderMode(target, existing?.target), - sessionId: existing?.sessionId ?? sessionId, + sessionId: tabSessionId, pinned: existing?.pinned } const replaceIndex = existing ? current.indexOf(existing) : -1 $previewTabs.set(replaceIndex === -1 ? [...current, tab] : current.map((item, i) => (i === replaceIndex ? tab : item))) - noteExplicitPreviewOpen(id) - selectRightRailTab(id) + noteExplicitPreviewOpen(tab.id) + // Select the row's FINAL id: when a pinned row is reused from another + // session, the id keeps the owner's session, so the pre-computed id would + // point at nothing. + selectRightRailTab(tab.id) } const blankPage = (): PreviewTarget => ({ kind: 'url', label: 'Browser', source: 'about:blank', url: 'about:blank' })