From 327c152ccae84faa79ed939483c14c375a7ca539 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:19:45 -0700 Subject: [PATCH] fix(runtime): keyboard tab switches tell a paired server which tab this desktop shows (#33761) * fix(runtime): keyboard tab switches tell a paired server which tab this desktop shows A paired server keeps one tab selection per device and a server workspace reopens on it. Clicks, Cmd+1..9 and the palette updated it, but the next, previous, recent and terminal-only shortcuts changed the tab locally only, so the server kept an older pick and the desktop could reopen on the wrong tab. * fix(runtime): keyboard switches to agent chats and server browser tabs also reach the server Browser activations carry the visible local tab so later navigation cancels their deferred focus, matching terminals. * fix(runtime): keep keyboard agent-chat switches local in workspaces no server owns * fix(runtime): scope keyboard agent-chat activation to this client on old and new servers * fix(runtime): a keyboard switch to an agent chat reaches the server that hosts that chat --- .../ipc-tab-switch-owner-selection.test.ts | 154 ++++++++++++++++++ src/renderer/src/hooks/ipc-tab-switch.ts | 3 + ...structured-agent-session-tab-activation.ts | 22 ++- .../src/lib/cycled-tab-owner-activation.ts | 32 ++++ .../src/lib/terminal-tab-owner-activation.ts | 25 +++ 5 files changed, 231 insertions(+), 5 deletions(-) create mode 100644 src/renderer/src/hooks/ipc-tab-switch-owner-selection.test.ts create mode 100644 src/renderer/src/lib/cycled-tab-owner-activation.ts diff --git a/src/renderer/src/hooks/ipc-tab-switch-owner-selection.test.ts b/src/renderer/src/hooks/ipc-tab-switch-owner-selection.test.ts new file mode 100644 index 00000000000..0bb77cd391a --- /dev/null +++ b/src/renderer/src/hooks/ipc-tab-switch-owner-selection.test.ts @@ -0,0 +1,154 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + getState: vi.fn(), + navOrder: vi.fn(), + activateOnOwner: vi.fn(async (_args: Record) => true), + ownerEnvironmentId: vi.fn((): string | null => 'env-1'), + runtimeRpc: vi.fn(async () => ({})) +})) + +vi.mock('../store', () => ({ useAppStore: { getState: mocks.getState } })) +vi.mock('@/components/tab-bar/group-tab-order', () => ({ getActiveTabNavOrder: mocks.navOrder })) +vi.mock('@/runtime/web-runtime-session-tab-lifecycle', () => ({ + activateWebRuntimeSessionTab: mocks.activateOnOwner +})) +vi.mock('@/runtime/web-runtime-session-environment', () => ({ + isWebRuntimeSessionActive: (environmentId: string | null) => + environmentId === 'env-1' || environmentId === 'env-2' +})) +vi.mock('@/lib/worktree-runtime-owner', () => ({ + runtimeEnvironmentIdOrLocalUnlessTwin: () => mocks.ownerEnvironmentId() +})) +vi.mock('@/runtime/remote-browser-tab-ownership', () => ({ + browserWorkspaceHasRemoteOwner: () => true +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ callRuntimeRpc: mocks.runtimeRpc })) + +import { + handleSwitchRecentTab, + handleSwitchTabAcrossAllTypes, + handleSwitchTerminalTab +} from './ipc-tab-switch' + +const WT = 'repo::/srv/app' + +function agentTab(executionHostId: string) { + return { + id: 'agent-tab', + entityId: 'session-1', + worktreeId: WT, + contentType: 'agent-session', + executionHostId + } +} + +function serverWorkspaceStore(unifiedTabs: unknown[] = []) { + return { + activeWorktreeId: WT, + activeTabType: 'terminal', + activeTabId: 'web-terminal-b', + activeFileId: null, + activeBrowserTabId: null, + activeGroupIdByWorktree: { [WT]: 'g' }, + groupsByWorktree: { + [WT]: [ + { + id: 'g', + activeTabId: 'web-terminal-b', + tabOrder: ['web-terminal-a', 'web-terminal-b'], + recentTabIds: ['web-terminal-a', 'web-terminal-b'] + } + ] + }, + tabsByWorktree: { [WT]: [{ id: 'web-terminal-a' }, { id: 'web-terminal-b' }] }, + unifiedTabsByWorktree: { [WT]: unifiedTabs }, + terminalLayoutsByTabId: { 'web-terminal-a': { activeLeafId: 'leaf-a' } }, + setActiveTab: vi.fn(), + setActiveFile: vi.fn(), + setActiveBrowserTab: vi.fn(), + activateTab: vi.fn(), + setActiveTabType: vi.fn(), + getActiveTab: () => null + } +} + +// A server workspace reopens on the selection the server holds for this desktop, so every keyboard switch must update it. +describe('keyboard tab switches in a server workspace', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.ownerEnvironmentId.mockReturnValue('env-1') + mocks.navOrder.mockReturnValue([ + { type: 'terminal', id: 'web-terminal-a', tabId: 'web-terminal-a' }, + { type: 'terminal', id: 'web-terminal-b', tabId: 'web-terminal-b' } + ]) + }) + + it.each([ + ['previous tab', () => handleSwitchTabAcrossAllTypes(-1)], + ['previous terminal', () => handleSwitchTerminalTab(-1)], + ['recent tab', () => handleSwitchRecentTab()] + ])('tells the server about a %s switch', (_name, run) => { + mocks.getState.mockReturnValue(serverWorkspaceStore()) + expect(run()).toBe(true) + expect(mocks.activateOnOwner).toHaveBeenCalledWith( + expect.objectContaining({ + worktreeId: WT, + tabId: 'web-terminal-a', + environmentId: 'env-1', + leafId: 'leaf-a' + }) + ) + }) + + it('stays local for a workspace no server owns', () => { + mocks.ownerEnvironmentId.mockReturnValue(null) + mocks.getState.mockReturnValue(serverWorkspaceStore()) + expect(handleSwitchTabAcrossAllTypes(-1)).toBe(true) + expect(mocks.activateOnOwner).not.toHaveBeenCalled() + }) + + it('tells the server about a browser tab it hosts, cancelable by later navigation', () => { + mocks.getState.mockReturnValue(serverWorkspaceStore()) + mocks.navOrder.mockReturnValue([ + { type: 'browser', id: 'browser-ws', tabId: 'browser-tab' }, + { type: 'terminal', id: 'web-terminal-b', tabId: 'web-terminal-b' } + ]) + expect(handleSwitchTabAcrossAllTypes(-1)).toBe(true) + expect(mocks.activateOnOwner).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'browser-tab', environmentId: 'env-1' }) + ) + expect(mocks.activateOnOwner.mock.calls[0]?.[0]).toHaveProperty('expectedCurrentLocalTabId') + }) + + it.each([ + ['the workspace server', 'runtime:env-1', 'env-1'], + ['another server whose chat this workspace shows', 'runtime:env-2', 'env-2'] + ])('tells %s about an agent chat switch', (_name, executionHostId, environmentId) => { + mocks.getState.mockReturnValue(serverWorkspaceStore([agentTab(executionHostId)])) + mocks.navOrder.mockReturnValue([ + { type: 'agent-session', id: 'session-1', tabId: 'agent-tab' }, + { type: 'terminal', id: 'web-terminal-b', tabId: 'web-terminal-b' } + ]) + expect(handleSwitchTabAcrossAllTypes(-1)).toBe(true) + expect(mocks.runtimeRpc).toHaveBeenCalledWith( + { kind: 'environment', environmentId }, + 'session.tabs.activate', + expect.objectContaining({ + tabId: 'agent-session:session-1', + notifyClients: false, + navigation: 'caller' + }) + ) + }) + + it('keeps a switch to a chat this machine hosts local', () => { + mocks.getState.mockReturnValue(serverWorkspaceStore([agentTab('local')])) + mocks.navOrder.mockReturnValue([ + { type: 'agent-session', id: 'session-1', tabId: 'agent-tab' }, + { type: 'terminal', id: 'web-terminal-b', tabId: 'web-terminal-b' } + ]) + expect(handleSwitchTabAcrossAllTypes(-1)).toBe(true) + expect(mocks.runtimeRpc).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/hooks/ipc-tab-switch.ts b/src/renderer/src/hooks/ipc-tab-switch.ts index 62fab56aecc..98582e4c4e5 100644 --- a/src/renderer/src/hooks/ipc-tab-switch.ts +++ b/src/renderer/src/hooks/ipc-tab-switch.ts @@ -7,6 +7,7 @@ import { type TypeCyclableTab } from '@/components/terminal/tab-type-cycle' import { sanitizeRecentTabIds } from '../store/slices/tab-group-state' +import { activateCycledTabOnOwner } from '@/lib/cycled-tab-owner-activation' type AppStoreState = ReturnType @@ -76,6 +77,7 @@ export function activateCyclableTab(store: AppStoreState, next: TypeCyclableTab) } store.setActiveTabType('editor', worktreeId) } + activateCycledTabOnOwner(worktreeId, next) } /** Cycle the current type. Fresh installs use Cmd/Ctrl+Alt+[ / ]; migrated bindings may retain the inverse and remain rebindable. */ @@ -324,5 +326,6 @@ export function handleSwitchTerminalTab(direction: number): boolean { } store.setActiveTab(next.id) store.setActiveTabType('terminal', worktreeId) + activateCycledTabOnOwner(worktreeId, next) return true } diff --git a/src/renderer/src/lib/agent-session/structured-agent-session-tab-activation.ts b/src/renderer/src/lib/agent-session/structured-agent-session-tab-activation.ts index e3246c1f088..bc560cad8ca 100644 --- a/src/renderer/src/lib/agent-session/structured-agent-session-tab-activation.ts +++ b/src/renderer/src/lib/agent-session/structured-agent-session-tab-activation.ts @@ -49,16 +49,28 @@ export function activateStructuredAgentSessionTab(args: { state.activateTab(tab.id, { worktreeId: args.worktreeId }) state.setActiveTabType('agent-session', args.worktreeId) requestNativeChatComposerFocus(tab.id) - const environmentId = runtimeEnvironmentIdOrLocalUnlessTwin(state, args.worktreeId) + activateStructuredAgentSessionTabOnOwner(args.worktreeId, tab.entityId, args.target) + return true +} + +/** The chat's host keeps this client's tab selection; every local activation must reach it. */ +export function activateStructuredAgentSessionTabOnOwner( + worktreeId: string, + sessionId: string, + target?: RuntimeClientTarget, + options?: { callerOnly?: boolean } +): void { + const environmentId = runtimeEnvironmentIdOrLocalUnlessTwin(useAppStore.getState(), worktreeId) void callRuntimeRpc( - args.target ?? runtimeTargetForOwnerEnvironment(environmentId), + target ?? runtimeTargetForOwnerEnvironment(environmentId), 'session.tabs.activate', { - worktree: toRuntimeWorktreeSelector(args.worktreeId), - tabId: `agent-session:${tab.entityId}` + worktree: toRuntimeWorktreeSelector(worktreeId), + tabId: `agent-session:${sessionId}`, + // Why both: navigation scopes new hosts to this client; notifyClients:false protects old hosts. + ...(options?.callerOnly ? { notifyClients: false, navigation: 'caller' as const } : {}) } ) - return true } export function activateStructuredAgentSessionById(args: { diff --git a/src/renderer/src/lib/cycled-tab-owner-activation.ts b/src/renderer/src/lib/cycled-tab-owner-activation.ts new file mode 100644 index 00000000000..e0e9421dd91 --- /dev/null +++ b/src/renderer/src/lib/cycled-tab-owner-activation.ts @@ -0,0 +1,32 @@ +import { useAppStore } from '@/store' +import type { TypeCyclableTab } from '@/components/terminal/tab-type-cycle' +import { activateStructuredAgentSessionTabOnOwner } from '@/lib/agent-session/structured-agent-session-tab-activation' +import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session-environment' +import { structuredAgentSessionTargetForTab } from '@/runtime/structured-agent-session-owner' +import { + activateBrowserTabOnOwner, + activateTerminalTabOnOwner +} from './terminal-tab-owner-activation' + +/** Keyboard switches must reach the owner like clicks, or a server workspace reopens on its stale pick. */ +export function activateCycledTabOnOwner(worktreeId: string | null, next: TypeCyclableTab): void { + if (!worktreeId) { + return + } + const state = useAppStore.getState() + if (next.type === 'terminal') { + const leafId = state.terminalLayoutsByTabId?.[next.id]?.activeLeafId ?? null + activateTerminalTabOnOwner(worktreeId, next.id, leafId) + } else if (next.type === 'browser' && next.tabId) { + activateBrowserTabOnOwner(worktreeId, next.tabId, next.id) + } else if (next.type === 'agent-session') { + const tab = (state.unifiedTabsByWorktree?.[worktreeId] ?? []).find( + (candidate) => candidate.id === next.tabId + ) + // A chat names its own host; local chats stay local so paired phones don't follow every stop. + const target = tab ? structuredAgentSessionTargetForTab(state, tab) : null + if (target?.kind === 'environment' && isWebRuntimeSessionActive(target.environmentId)) { + activateStructuredAgentSessionTabOnOwner(worktreeId, next.id, target, { callerOnly: true }) + } + } +} diff --git a/src/renderer/src/lib/terminal-tab-owner-activation.ts b/src/renderer/src/lib/terminal-tab-owner-activation.ts index 970d10a12fc..916ef37b94e 100644 --- a/src/renderer/src/lib/terminal-tab-owner-activation.ts +++ b/src/renderer/src/lib/terminal-tab-owner-activation.ts @@ -3,6 +3,7 @@ import { runtimeEnvironmentIdOrLocalUnlessTwin } from '@/lib/worktree-runtime-ow import { activateWebRuntimeSessionTab } from '@/runtime/web-runtime-session-tab-lifecycle' import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session-environment' import { resolveWebSessionVisibleTabId } from '@/runtime/web-session-focus-intent' +import { browserWorkspaceHasRemoteOwner } from '@/runtime/remote-browser-tab-ownership' /** After local activation, notify the paired owner to store this client’s selection and wake parked panes. Focus late-arriving tabs unless navigation changed meanwhile. Local/direct-SSH workspaces need no owner round trip. */ export function activateTerminalTabOnOwner( @@ -23,3 +24,27 @@ export function activateTerminalTabOnOwner( expectedCurrentLocalTabId: resolveWebSessionVisibleTabId(state, worktreeId) }) } + +/** A browser tab the owner hosts; its selection must follow local activation like a terminal's. */ +export function activateBrowserTabOnOwner( + worktreeId: string, + unifiedTabId: string, + browserWorkspaceId: string +): void { + const state = useAppStore.getState() + const environmentId = runtimeEnvironmentIdOrLocalUnlessTwin(state, worktreeId) + if ( + !environmentId || + !isWebRuntimeSessionActive(environmentId) || + !browserWorkspaceHasRemoteOwner(state, browserWorkspaceId, environmentId) + ) { + return + } + void activateWebRuntimeSessionTab({ + worktreeId, + tabId: unifiedTabId, + environmentId, + // Later local navigation cancels this deferred focus, as for terminals. + expectedCurrentLocalTabId: resolveWebSessionVisibleTabId(state, worktreeId) + }) +}