From b6d549d002fbb5248881588128b62cf4720e162d Mon Sep 17 00:00:00 2001 From: Gille <4317663+helix4u@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:15:26 -0600 Subject: [PATCH] fix(desktop): restore missing Bot Chat panes before claiming focus --- .../src/components/pane-shell/tree/store.ts | 11 +- .../src/store/session-pane-focus.test.ts | 105 ++++++++++++++++++ apps/desktop/src/store/session-states.test.ts | 7 +- apps/desktop/src/store/session-states.ts | 13 ++- 4 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 apps/desktop/src/store/session-pane-focus.test.ts diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index a451e570e3..07e9ed09f7 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -1040,6 +1040,13 @@ export function revealTreePane(paneId: string) { // Reveal beats a Close: un-dismiss and let adoption put the pane back. if ($dismissedPanes.get().has(paneId)) { setDismissed(paneId, false) + } + + // A layout replacement can omit a still-registered pane without dismissing + // it. Reconcile that saved contribution before claiming to reveal it. + const currentTree = $layoutTree.get() + + if (currentTree && !findGroupOfPane(currentTree, paneId)) { adoptContributedPanes() } @@ -1064,8 +1071,8 @@ export function revealTreePane(paneId: string) { if (hiddenNow.has(paneId)) { setTreePaneHidden(paneId, false) - - return + // Reactive unhide preserves a visible sibling. Explicit reveal must also + // front this pane and restore its group below. } const tree = $layoutTree.get() diff --git a/apps/desktop/src/store/session-pane-focus.test.ts b/apps/desktop/src/store/session-pane-focus.test.ts new file mode 100644 index 0000000000..92b190a375 --- /dev/null +++ b/apps/desktop/src/store/session-pane-focus.test.ts @@ -0,0 +1,105 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +async function setup() { + const tree = await import('@/components/pane-shell/tree/store') + const model = await import('@/components/pane-shell/tree/model') + const { registry } = await import('@/contrib/registry') + const session = await import('@/store/session') + const states = await import('@/store/session-states') + const { paneMirror } = await import('@/app/chat/pane-mirror') + const { openSession } = await import('@/app/open-session') + + registry.register({ + area: 'panes', + data: { placement: 'main', uncloseable: true }, + id: 'workspace', + render: () => null, + title: 'Chat' + }) + tree.declareDefaultTree(model.group(['workspace'], { active: 'workspace', id: 'main' })) + tree.watchContributedPanes() + paneMirror({ + source: states.$sessionTiles, + key: tile => tile.storedSessionId, + prefix: 'session-tile', + dir: () => 'center', + minWidth: '20rem', + title: id => id, + render: () => null, + close: states.closeSessionTile + })() + session.$selectedStoredSessionId.set('previous-chat') + + const scope = { + ownerRoute: { connectionId: 'remote-a', mode: 'remote' as const, profile: 'writer' }, + workspaceMode: 'bots' as const, + workspaceOwnerKey: 'remote-a::writer', + workspaceTabTitle: 'Bot Chat' + } + + states.openSessionTile('canonical-chat', 'center', 'workspace', undefined, scope) + + return { model, openSession, scope, session, states, tree } +} + +describe('focusing a saved Bot Chat requires a visible pane', () => { + let ctx: Awaited> + const paneId = 'session-tile:canonical-chat' + + beforeEach(async () => { + window.localStorage.clear() + vi.resetModules() + ctx = await setup() + }) + + it('re-adopts a saved tab after a profile overlay replaces the layout', async () => { + const { applyDesktopOverlay } = await import('@/store/profile-share') + const { model, scope, states, tree } = ctx + const saved = states.$sessionTiles.get() + applyDesktopOverlay('imported-profile', { + version: 1, + layoutTree: model.group(['workspace'], { active: 'workspace', id: 'imported-main' }) + }) + expect(model.findGroupOfPane(tree.$layoutTree.get()!, paneId)).toBeNull() + + expect(states.focusWorkspaceOwnerSessionTile(scope.workspaceOwnerKey, undefined, ['canonical-chat'])).toBe( + 'canonical-chat' + ) + expect(tree.isPaneVisible(paneId)).toBe(true) + expect(tree.$activeTreeGroup.get()).toBe('imported-main') + expect(states.$sessionTiles.get()).toEqual(saved) + expect(states.sessionTileOwnerRoute('canonical-chat')).toEqual(scope.ownerRoute) + }) + + it('fronts and un-minimizes a hidden chat instead of leaving its sibling active', () => { + const { model, scope, states, tree } = ctx + tree.$layoutTree.set(model.group(['workspace', paneId], { active: 'workspace', id: 'main', minimized: true })) + tree.setTreePaneHidden(paneId, true) + + expect(states.focusWorkspaceOwnerSessionTile(scope.workspaceOwnerKey, undefined, ['canonical-chat'])).toBe( + 'canonical-chat' + ) + expect(tree.isPaneVisible(paneId)).toBe(true) + expect(model.findGroupOfPane(tree.$layoutTree.get()!, paneId)?.active).toBe(paneId) + }) + + it('reports a miss through both helpers if the layout cannot place the saved tab', () => { + const { scope, session, states, tree } = ctx + tree.$layoutTree.set(null) + + expect(states.focusOpenSession('canonical-chat', scope)).toBeNull() + expect(states.focusWorkspaceOwnerSessionTile(scope.workspaceOwnerKey, undefined, ['canonical-chat'])).toBeNull() + expect(session.$selectedStoredSessionId.get()).toBe('previous-chat') + expect(states.$sessionTiles.get().map(tile => tile.storedSessionId)).toEqual(['canonical-chat']) + }) + + it('keeps an existing tab in place and does not navigate or duplicate it', () => { + const { openSession, scope, states, tree } = ctx + const navigate = vi.fn() + openSession('canonical-chat', navigate, 'in-place', scope) + + expect(tree.isPaneVisible(paneId)).toBe(true) + expect(navigate).not.toHaveBeenCalled() + expect(states.$sessionTiles.get().map(tile => tile.storedSessionId)).toEqual(['canonical-chat']) + }) +}) diff --git a/apps/desktop/src/store/session-states.test.ts b/apps/desktop/src/store/session-states.test.ts index a5f887cbb0..1926948f27 100644 --- a/apps/desktop/src/store/session-states.test.ts +++ b/apps/desktop/src/store/session-states.test.ts @@ -320,6 +320,7 @@ describe('SessionTile workspace scope', () => { $selectedStoredSessionId.set('bot-chat') openSessionTile('bot-chat', 'center', undefined, undefined, scope) + $layoutTree.set(group(['workspace', tilePane('bot-chat')], { active: 'workspace', id: 'main' })) expect($sessionTiles.get()).toEqual([ expect.objectContaining({ @@ -337,6 +338,7 @@ describe('SessionTile workspace scope', () => { // new tip must front that tile, not open the same chat twice. setSessions([{ _lineage_ids: ['seg-1', 'seg-2', 'seg-3'], _lineage_root_id: 'seg-1', id: 'seg-3' } as never]) openSessionTile('seg-2') + $layoutTree.set(group(['workspace', tilePane('seg-2')], { active: 'workspace', id: 'main' })) expect(focusOpenSession('seg-3')).toBe('tile') expect($sessionTiles.get().map(t => t.storedSessionId)).toEqual(['seg-2']) @@ -487,6 +489,7 @@ describe('focusWorkspaceOwnerSessionTile', () => { openSessionTile('thread', 'center', 'workspace', undefined, botA) rememberActivePane(workspaceScopeKey('bots', 'bot:a'), tilePane('closed-bot-chat')) $sessionTiles.set($sessionTiles.get().filter(t => t.storedSessionId !== 'closed-bot-chat')) + $layoutTree.set(group(['workspace', tilePane('thread')], { active: 'workspace', id: 'main' })) expect(focusWorkspaceOwnerSessionTile('bot:a')).toBe('thread') }) @@ -533,6 +536,7 @@ describe('focusWorkspaceOwnerSessionTile', () => { it('a throwing probe keeps the tile — reconciliation must not break the click', () => { openSessionTile('bot-chat', 'center', 'workspace', undefined, botA) + $layoutTree.set(group(['workspace', tilePane('bot-chat')], { active: 'workspace', id: 'main' })) expect( focusWorkspaceOwnerSessionTile('bot:a', () => { @@ -542,8 +546,9 @@ describe('focusWorkspaceOwnerSessionTile', () => { expect($sessionTiles.get().map(t => t.storedSessionId)).toEqual(['bot-chat']) }) - it('no probe keeps the old behavior byte for byte', () => { + it('fronts a visible tile without a probe', () => { openSessionTile('bot-chat', 'center', 'workspace', undefined, botA) + $layoutTree.set(group(['workspace', tilePane('bot-chat')], { active: 'workspace', id: 'main' })) expect(focusWorkspaceOwnerSessionTile('bot:a')).toBe('bot-chat') expect($sessionTiles.get().map(t => t.storedSessionId)).toEqual(['bot-chat']) diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index fd9cb8cf60..afd217b79b 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -25,6 +25,7 @@ import { $activeTreeGroup, $layoutTree, focusedSessionTabAnchor, + isPaneVisible, moveTreePane, noteActiveTreeGroup, revealTreePane @@ -1551,10 +1552,12 @@ export function focusOpenSession( const tree = $layoutTree.get() const group = tree ? findGroupOfPane(tree, paneId) : null - if (group) { - noteActiveTreeGroup(group.id) + if (!group || !isPaneVisible(paneId)) { + return null } + noteActiveTreeGroup(group.id) + return 'tile' } @@ -1634,9 +1637,9 @@ export function focusWorkspaceOwnerSessionTile( const paneId = resolveRememberedActivePane(workspaceScopeKey('bots', workspaceOwnerKey), paneIds) ?? paneIds[0] const storedSessionId = paneId.slice(TILE_PANE_PREFIX.length) - focusOpenSession(storedSessionId, { workspaceMode: 'bots', workspaceOwnerKey }) - - return storedSessionId + return focusOpenSession(storedSessionId, { workspaceMode: 'bots', workspaceOwnerKey }) === 'tile' + ? storedSessionId + : null } /** Does a sidebar click still need to navigate after `focusOpenSession`? A miss