fix(ssh): a direct-SSH host republish no longer steals this view's tab selection (#27100)

* fix(ssh): a direct-SSH host republish no longer steals this view's tab selection

* fix(ssh): keep the home screen out of the kept selection

---------

Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
This commit is contained in:
OrcaWin
2026-10-09 19:40:02 -07:00
committed by GitHub
co-authored by m4air
parent 33d90bb066
commit 310c5d02e7
4 changed files with 178 additions and 22 deletions
@@ -124,6 +124,7 @@ export function appState(overrides: Record<string, unknown> = {}): AppState {
hydrateTabsSession: vi.fn(),
hydrateEditorSession: vi.fn(),
hydrateBrowserSession: vi.fn(),
remoteWorkspaceHydratedTargetIds: new Set<string>(),
markRemoteWorkspaceHydrated: vi.fn(),
clearRemoteWorkspaceHydrated: vi.fn(),
setRemoteWorkspaceSyncStatus: vi.fn(),
@@ -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> = {}
): 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')
})
})
@@ -37,7 +37,9 @@ export function mergeDirectSshRemoteWorkspaceSession(
preserveLocalTerminalTabIds: ReadonlySet<string>,
replaceExecutionHostId?: ExecutionHostId,
closedTabRecords?: ClosedTerminalTabTombstonesByTabId,
preserveLocalLayoutTabIds: ReadonlySet<string> = new Set()
preserveLocalLayoutTabIds: ReadonlySet<string> = 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(
@@ -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'