diff --git a/src/renderer/src/hooks/__tests__/remote-workspace-target-sync-test-harness.ts b/src/renderer/src/hooks/__tests__/remote-workspace-target-sync-test-harness.ts index 3884704d2a1..adc0d3492a4 100644 --- a/src/renderer/src/hooks/__tests__/remote-workspace-target-sync-test-harness.ts +++ b/src/renderer/src/hooks/__tests__/remote-workspace-target-sync-test-harness.ts @@ -124,6 +124,7 @@ export function appState(overrides: Record = {}): AppState { hydrateTabsSession: vi.fn(), hydrateEditorSession: vi.fn(), hydrateBrowserSession: vi.fn(), + remoteWorkspaceHydratedTargetIds: new Set(), markRemoteWorkspaceHydrated: vi.fn(), clearRemoteWorkspaceHydrated: vi.fn(), setRemoteWorkspaceSyncStatus: vi.fn(), diff --git a/src/renderer/src/hooks/remote-workspace-session-merge-local-selection.test.ts b/src/renderer/src/hooks/remote-workspace-session-merge-local-selection.test.ts new file mode 100644 index 00000000000..8889d322bb3 --- /dev/null +++ b/src/renderer/src/hooks/remote-workspace-session-merge-local-selection.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from 'vitest' +import { mergeDirectSshRemoteWorkspaceSession } from './remote-workspace-session-merge' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { TerminalTab } from '../../../shared/terminal-tab-types' + +/** + * #19697 / #20938: every direct-SSH host republish (a title change while an agent works) carries + * the last uploaded selection. Selection is per view, so after the first hydrate it must not move. + */ +const WORKTREE = 'repo-1::/home/user/app' +const OTHER = 'repo-1::/home/user/other' + +function tab(id: string, title = id, worktreeId = WORKTREE): TerminalTab { + return { + id, + ptyId: null, + worktreeId, + title, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function session( + activeTabId: string | null, + tabs: TerminalTab[], + overrides: Partial = {} +): WorkspaceSessionState { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fields below are every one the merge reads for selection. + return { + activeRepoId: 'repo-1', + activeWorktreeId: WORKTREE, + activeWorkspaceKey: worktreeWorkspaceKey(WORKTREE), + activeTabId, + tabsByWorktree: { [WORKTREE]: tabs }, + terminalLayoutsByTabId: {}, + activeTabIdByWorktree: activeTabId ? { [WORKTREE]: activeTabId } : {}, + ...overrides + } as WorkspaceSessionState +} + +function merge( + current: WorkspaceSessionState, + remote: WorkspaceSessionState, + hydrated: boolean +): WorkspaceSessionState { + return mergeDirectSshRemoteWorkspaceSession( + current, + remote, + new Set([WORKTREE, OTHER]), + current.tabsByWorktree, + new Set(), + 'ssh:target-1', + undefined, + new Set(), + hydrated + ) +} + +const local = [tab('tab-1'), tab('tab-2'), tab('tab-3')] +const republished = [tab('tab-1', 'claude: working'), tab('tab-2'), tab('tab-3')] + +describe('direct-SSH merge keeps this view’s selection after the first hydrate', () => { + it('keeps the user’s tab when a host republish names another one', () => { + const merged = merge(session('tab-3', local), session('tab-1', republished), true) + + expect(merged.activeTabId).toBe('tab-3') + expect(merged.activeTabIdByWorktree?.[WORKTREE]).toBe('tab-3') + expect(merged.tabsByWorktree[WORKTREE]?.[0]?.title).toBe('claude: working') + }) + + it('keeps the user’s workspace when the host names a different one', () => { + const remote = session('other-1', republished, { + activeWorktreeId: OTHER, + activeWorkspaceKey: worktreeWorkspaceKey(OTHER), + tabsByWorktree: { [WORKTREE]: republished, [OTHER]: [tab('other-1', 'x', OTHER)] } + }) + + const merged = merge(session('tab-3', local), remote, true) + + expect(merged.activeWorktreeId).toBe(WORKTREE) + expect(merged.activeWorkspaceKey).toBe(worktreeWorkspaceKey(WORKTREE)) + expect(merged.activeTabId).toBe('tab-3') + }) + + it('falls back to the host pointer once the local tab is gone', () => { + const hostTabs = [tab('tab-1'), tab('tab-2')] + const current = session('tab-3', hostTabs) + + const merged = merge(current, session('tab-1', hostTabs), true) + + expect(merged.activeTabIdByWorktree?.[WORKTREE]).toBe('tab-1') + expect(merged.activeTabId).toBe('tab-1') + }) + + it('still adopts the host selection on the first hydrate', () => { + const merged = merge(session('tab-3', local), session('tab-1', republished), false) + + expect(merged.activeTabId).toBe('tab-1') + expect(merged.activeTabIdByWorktree?.[WORKTREE]).toBe('tab-1') + }) + + it('leaves a user on the home screen there when the host names no workspace', () => { + const current = session(null, local, { + activeWorktreeId: null, + activeWorkspaceKey: null, + activeTabIdByWorktree: { [WORKTREE]: 'tab-3' } + }) + const remote = session('tab-1', republished, { + activeRepoId: null, + activeWorktreeId: null, + activeWorkspaceKey: null + }) + + const merged = merge(current, remote, true) + + expect(merged.activeWorktreeId).toBeNull() + expect(merged.activeRepoId).toBeNull() + expect(merged.activeTabIdByWorktree?.[WORKTREE]).toBe('tab-3') + }) +}) diff --git a/src/renderer/src/hooks/remote-workspace-session-merge.ts b/src/renderer/src/hooks/remote-workspace-session-merge.ts index f8bbb0d5ab1..f02d7b98d1f 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge.ts @@ -37,7 +37,9 @@ export function mergeDirectSshRemoteWorkspaceSession( preserveLocalTerminalTabIds: ReadonlySet, replaceExecutionHostId?: ExecutionHostId, closedTabRecords?: ClosedTerminalTabTombstonesByTabId, - preserveLocalLayoutTabIds: ReadonlySet = new Set() + preserveLocalLayoutTabIds: ReadonlySet = new Set(), + /** True once the target hydrated: selection is per view, so a republish must not move it. */ + keepLocalSelection = false ): WorkspaceSessionState { // Live tabs across the worktrees this snapshot replaces. Close-suppression consults it so a tab // that is still live locally always beats its own tombstone. @@ -264,22 +266,64 @@ export function mergeDirectSshRemoteWorkspaceSession( // worktree came from local state left the pair disagreeing — and precisely in the case the // preservation exists for, since "the host named no worktree" is exactly when it can still name a // repo. + // Why: every host republish carries the last uploaded selection, so after the first hydrate a + // title change while an agent works would otherwise steal focus back (#19697, #20938). + // The home screen is not kept: hydration reads a null worktree as "restore the repo's default". + const keepsLocalView = activeOutsideTarget || (keepLocalSelection && localActiveWorkspaceSurvives) const keepsLocalWorkspace = - !activeOutsideTarget && remote.activeWorktreeId == null && preservedActiveWorktreeId != null - const activeWorktreeId = activeOutsideTarget + !keepsLocalView && remote.activeWorktreeId == null && preservedActiveWorktreeId != null + const activeWorktreeId = keepsLocalView ? current.activeWorktreeId : (remote.activeWorktreeId ?? preservedActiveWorktreeId) + const namesMergedTab = (worktreeId: string, tabId: string | null | undefined): tabId is string => + tabId != null && (tabsByWorktree[worktreeId] ?? []).some((tab) => tab.id === tabId) + const activeTabIdByWorktree = { + // Why the local entry survives: same rule one level down. A host that names no active tab for + // the target reopens the workspace on whatever sorts first, which is not where the user was. + ...omitTargetWorktrees(current.activeTabIdByWorktree), + ...Object.fromEntries( + [...replaceWorktreeIds].flatMap((worktreeId) => { + const localActiveTabId = current.activeTabIdByWorktree?.[worktreeId] + return localActiveTabId == null ? [] : [[worktreeId, localActiveTabId] as const] + }) + ), + // Why a suppressed id is left in place here: hydration validates both active-tab pointers + // against the tab rows it just built and nulls anything they no longer name, so nulling twice + // would only add a second rule that has to stay in step with that one. + ...remote.activeTabIdByWorktree, + // The host's pointer is only a fallback for a local one that no longer names a tab. + ...Object.fromEntries( + [...replaceWorktreeIds].flatMap((worktreeId) => { + const localActiveTabId = current.activeTabIdByWorktree?.[worktreeId] + return keepLocalSelection && namesMergedTab(worktreeId, localActiveTabId) + ? [[worktreeId, localActiveTabId] as const] + : [] + }) + ) + } + const localActiveTabStands = + activeOutsideTarget || + current.activeWorktreeId == null || + namesMergedTab(current.activeWorktreeId, current.activeTabId) + const fallbackActiveTabId = + current.activeWorktreeId == null + ? null + : (activeTabIdByWorktree[current.activeWorktreeId] ?? null) return { ...current, activeRepoId: - activeOutsideTarget || keepsLocalWorkspace ? current.activeRepoId : remote.activeRepoId, + keepsLocalView || keepsLocalWorkspace ? current.activeRepoId : remote.activeRepoId, activeWorktreeId, - activeWorkspaceKey: activeOutsideTarget + activeWorkspaceKey: keepsLocalView ? current.activeWorkspaceKey : activeWorktreeId ? worktreeWorkspaceKey(activeWorktreeId) : null, - activeTabId: activeOutsideTarget ? current.activeTabId : remote.activeTabId, + activeTabId: !keepsLocalView + ? remote.activeTabId + : localActiveTabStands + ? current.activeTabId + : fallbackActiveTabId, tabsByWorktree: { ...omitTargetWorktrees(current.tabsByWorktree), ...tabsByWorktree @@ -289,21 +333,7 @@ export function mergeDirectSshRemoteWorkspaceSession( ...(current.activeWorktreeIdsOnShutdown ?? []).filter((id) => !replaceWorktreeIds.has(id)), ...(remote.activeWorktreeIdsOnShutdown ?? []) ], - activeTabIdByWorktree: { - // Why the local entry survives: same rule one level down. A host that names no active tab for - // the target reopens the workspace on whatever sorts first, which is not where the user was. - ...omitTargetWorktrees(current.activeTabIdByWorktree), - ...Object.fromEntries( - [...replaceWorktreeIds].flatMap((worktreeId) => { - const localActiveTabId = current.activeTabIdByWorktree?.[worktreeId] - return localActiveTabId == null ? [] : [[worktreeId, localActiveTabId] as const] - }) - ), - // Why a suppressed id is left in place here: hydration validates both active-tab pointers - // against the tab rows it just built and nulls anything they no longer name, so nulling twice - // would only add a second rule that has to stay in step with that one. - ...remote.activeTabIdByWorktree - }, + activeTabIdByWorktree, remoteSessionIdsByTabId: { ...Object.fromEntries( Object.entries(current.remoteSessionIdsByTabId ?? {}).filter( diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts b/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts index 9dbe57cf0bc..8267859d175 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts @@ -192,7 +192,8 @@ export async function applyDirectSshRemoteWorkspaceSnapshot({ ) .map((tab) => tab.id) ) - ) + ), + state.remoteWorkspaceHydratedTargetIds.has(authority.targetId) ) if (!isArrivalCurrent(authority.targetId, arrival) || !isPreparationTokenCurrent(token)) { return 'stale'