diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 7ad97e6b692c..214824191c75 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -18420,10 +18420,12 @@ describe('connectPanePty', () => { expect(window.api.ssh.needsPassphrasePrompt).not.toHaveBeenCalled() }) - it('fails closed when the same SSH worktree id is projected by two HUBs', async () => { + it('routes through the focused HUB when the same SSH worktree id is projected by two HUBs (#10491)', async () => { const { connectPanePty } = await import('./pty-connection') const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const { createIpcPtyTransport } = await import('./pty-transport') + const transport = createMockTransport() + transportFactoryQueue.push(transport) mockStoreState = { ...mockStoreState, tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: null }] }, @@ -18458,6 +18460,48 @@ describe('connectPanePty', () => { ) await flushAsyncTicks() + expect(createRemoteRuntimePtyTransport).toHaveBeenCalledWith('hub-a', expect.any(Object)) + expect(createIpcPtyTransport).not.toHaveBeenCalled() + }) + + it('fails closed when two HUB projections share an id and focus matches neither', async () => { + const { connectPanePty } = await import('./pty-connection') + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { createIpcPtyTransport } = await import('./pty-transport') + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: null }] }, + worktreesByRepo: { + repo1: [ + { + id: 'wt-1', + repoId: 'repo1', + path: '/srv/same-worktree', + hostId: 'ssh:same-private-target', + runtimeOwnerEnvironmentId: 'hub-a' + }, + { + id: 'wt-1', + repoId: 'repo1', + path: '/srv/same-worktree', + hostId: 'ssh:same-private-target', + runtimeOwnerEnvironmentId: 'hub-b' + } + ] + }, + settings: { + ...mockStoreState.settings, + activeRuntimeEnvironmentId: 'hub-c' + } + } as StoreState + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ restoredPtyIdByLeafId: { [LEAF_1]: null } }) as never + ) + await flushAsyncTicks() + expect(createRemoteRuntimePtyTransport).not.toHaveBeenCalled() expect(createIpcPtyTransport).not.toHaveBeenCalled() expect(window.api.ssh.connect).not.toHaveBeenCalled() diff --git a/src/renderer/src/lib/terminal-worktree-route.test.ts b/src/renderer/src/lib/terminal-worktree-route.test.ts index 24255b61c023..218356f187fb 100644 --- a/src/renderer/src/lib/terminal-worktree-route.test.ts +++ b/src/renderer/src/lib/terminal-worktree-route.test.ts @@ -72,6 +72,51 @@ describe('resolveTerminalWorktreeRoute', () => { it('does not treat a folder workspace as an unresolved worktree', () => { expect(resolveTerminalWorktreeRoute(localState(), folderWorkspaceKey('abc-123'))).not.toBeNull() }) + + it('routes a focused-host copy when the same worktree id is projected on two hosts (#10491)', () => { + const state = localState({ + settings: { activeRuntimeEnvironmentId: 'hub-a' }, + runtimeEnvironments: [{ id: 'hub-a' }, { id: 'hub-b' }], + worktreesByRepo: { + 'repo-1': [ + { id: 'repo-1::/w', repoId: 'repo-1', hostId: 'local' }, + { + id: 'repo-1::/w', + repoId: 'repo-1', + hostId: 'ssh:private-target', + runtimeOwnerEnvironmentId: 'hub-a' + } + ] + } + } as unknown as Partial) + expect(resolveTerminalWorktreeRoute(state, 'repo-1::/w')).toEqual({ + runtimeEnvironmentId: 'hub-a' + }) + }) + + it('fails closed when multi-host projections share an id and focus matches none (#10491)', () => { + const state = localState({ + settings: { activeRuntimeEnvironmentId: 'hub-c' }, + runtimeEnvironments: [{ id: 'hub-a' }, { id: 'hub-b' }, { id: 'hub-c' }], + worktreesByRepo: { + 'repo-1': [ + { + id: 'repo-1::/w', + repoId: 'repo-1', + hostId: 'ssh:private-a', + runtimeOwnerEnvironmentId: 'hub-a' + }, + { + id: 'repo-1::/w', + repoId: 'repo-1', + hostId: 'ssh:private-b', + runtimeOwnerEnvironmentId: 'hub-b' + } + ] + } + } as unknown as Partial) + expect(resolveTerminalWorktreeRoute(state, 'repo-1::/w')).toBeNull() + }) }) // STA-2639: teardown must follow the PTY's real host while spawn stays free to prefer a focused diff --git a/src/renderer/src/lib/worktree-operation-route-focus.ts b/src/renderer/src/lib/worktree-operation-route-focus.ts new file mode 100644 index 000000000000..f6b608b0cc0f --- /dev/null +++ b/src/renderer/src/lib/worktree-operation-route-focus.ts @@ -0,0 +1,72 @@ +import { + getSettingsFocusedExecutionHostId, + parseExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' + +export type WorktreeOperationRoute = { + executionHostId: ExecutionHostId | null + runtimeEnvironmentId: string | null +} + +export type WorktreeOperationRouteResolution = + | { kind: 'resolved'; route: WorktreeOperationRoute } + | { kind: 'ambiguous' } + | { kind: 'missing' } + +function routeMatchesFocusedHost( + route: WorktreeOperationRoute, + focusedHostId: ExecutionHostId +): boolean { + if (route.executionHostId === focusedHostId) { + return true + } + // Why: HUB-owned SSH rows keep executionHostId=ssh:* while transport ownership is the runtime. + const focused = parseExecutionHostId(focusedHostId) + return ( + focused?.kind === 'runtime' && + route.runtimeEnvironmentId != null && + route.runtimeEnvironmentId === focused.environmentId + ) +} + +// Why: same worktree/repo id can project on multiple hosts; only collapse when focus selects one (#10491). +export function preferFocusedRoute( + routes: Iterable, + settings: { activeRuntimeEnvironmentId?: string | null } | null | undefined +): WorktreeOperationRoute | null { + const candidates = [...routes] + if (candidates.length === 0) { + return null + } + if (candidates.length === 1) { + return candidates[0] + } + const focusedHostId = getSettingsFocusedExecutionHostId(settings) + const focusedMatches = candidates.filter((route) => routeMatchesFocusedHost(route, focusedHostId)) + return focusedMatches.length === 1 ? focusedMatches[0] : null +} + +export function resolveFromCandidateRoutes( + routes: Map, + settings: { activeRuntimeEnvironmentId?: string | null } | null | undefined +): WorktreeOperationRouteResolution { + if (routes.size === 0) { + return { kind: 'missing' } + } + const preferred = preferFocusedRoute(routes.values(), settings) + if (preferred) { + return { kind: 'resolved', route: preferred } + } + return routes.size > 1 ? { kind: 'ambiguous' } : { kind: 'missing' } +} + +export function addOperationRoute( + routes: Map, + route: WorktreeOperationRoute | null +): void { + if (!route) { + return + } + routes.set(JSON.stringify(route), route) +} diff --git a/src/renderer/src/lib/worktree-operation-route.test.ts b/src/renderer/src/lib/worktree-operation-route.test.ts index 8f2ef74d3b50..cc14ee661bf4 100644 --- a/src/renderer/src/lib/worktree-operation-route.test.ts +++ b/src/renderer/src/lib/worktree-operation-route.test.ts @@ -60,7 +60,7 @@ describe('resolveWorktreeOperationRouteResult', () => { }) }) - it('fails closed when the same SSH worktree is projected by two HUBs', () => { + it('prefers the focused HUB when the same SSH worktree is projected by two HUBs (#10491)', () => { expect( resolveWorktreeOperationRouteResult( { @@ -74,6 +74,29 @@ describe('resolveWorktreeOperationRouteResult', () => { }, WORKTREE_ID ) + ).toEqual({ + kind: 'resolved', + route: { + executionHostId: 'ssh:same-private-target', + runtimeEnvironmentId: 'hub-a' + } + }) + }) + + it('fails closed when two HUB projections share an id and focus matches neither', () => { + expect( + resolveWorktreeOperationRouteResult( + { + settings: { activeRuntimeEnvironmentId: 'hub-c' } as never, + worktreesByRepo: { + 'repo-1': [ + worktree('ssh:same-private-target', 'hub-a'), + worktree('ssh:same-private-target', 'hub-b') + ] + } + }, + WORKTREE_ID + ) ).toEqual({ kind: 'ambiguous' }) }) @@ -97,6 +120,39 @@ describe('resolveWorktreeOperationRouteResult', () => { }) }) + it('prefers the local projection when local focus selects one of two host copies (#10491)', () => { + expect( + resolveWorktreeOperationRouteResult( + { + worktreesByRepo: { + 'repo-1': [worktree('local'), worktree('ssh:ssh-1', 'hub-a')] + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'local', runtimeEnvironmentId: null } + }) + }) + + it('prefers the runtime projection when runtime focus selects one of two host copies (#10491)', () => { + expect( + resolveWorktreeOperationRouteResult( + { + settings: { activeRuntimeEnvironmentId: 'hub-a' } as never, + worktreesByRepo: { + 'repo-1': [worktree('local'), worktree('runtime:hub-a')] + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'runtime:hub-a', runtimeEnvironmentId: 'hub-a' } + }) + }) + it('deduplicates identical projections from the same HUB', () => { expect( resolveWorktreeOperationRouteResult( diff --git a/src/renderer/src/lib/worktree-operation-route.ts b/src/renderer/src/lib/worktree-operation-route.ts index 75608bc0c5aa..fa13fb3c2c2c 100644 --- a/src/renderer/src/lib/worktree-operation-route.ts +++ b/src/renderer/src/lib/worktree-operation-route.ts @@ -6,7 +6,8 @@ import { } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' import { getRepoIdFromWorktreeId } from '@/store/slices/worktree-helpers' -import { addRoute, resolveExactWorktreeRoute, routeForOwner } from './worktree-owner-route' +import { resolveExactWorktreeRoute, routeForOwner } from './worktree-owner-route' +import { addOperationRoute, resolveFromCandidateRoutes } from './worktree-operation-route-focus' import { findIndexedDetectedWorktrees, hasIndexedDetectedWorktree, @@ -197,39 +198,63 @@ function resolveFolderWorkspaceOperationRoute( } } -export function resolveExplicitWorktreeOperationRouteResult( +export function collectOwnerRoutes( state: WorktreeOperationRouteState, - worktreeId: string -): WorktreeOperationRouteResolution { - const exactRoutes = new Map() - const exactRepoIds = new Set() - const indexedWorktree = resolveIndexedWorktreeOwner(state.worktreesByRepo, worktreeId) - if (indexedWorktree.kind === 'ambiguous') { - return { kind: 'ambiguous' } - } - if (indexedWorktree.kind === 'resolved') { - exactRepoIds.add(indexedWorktree.owner.repoId) - const resolution = resolveExactWorktreeRoute(state, indexedWorktree.owner) - if (resolution.kind === 'ambiguous') { - return resolution + owners: Iterable, + worktreeId: string, + exactRoutes: Map, + exactRepoIds: Set +): void { + for (const worktree of owners) { + if (worktree.id !== worktreeId) { + continue } + exactRepoIds.add(worktree.repoId) + const resolution = resolveExactWorktreeRoute(state, worktree) if (resolution.kind === 'resolved') { - addRoute(exactRoutes, resolution.route) + addOperationRoute(exactRoutes, resolution.route) + } else if (resolution.kind === 'ambiguous') { + // Why: bare SSH route when multi-host repo recovery stays ambiguous; focus may still pick. + addOperationRoute(exactRoutes, routeForOwner(worktree)) } } - for (const worktree of findIndexedDetectedWorktrees(state.detectedWorktreesByRepo, worktreeId)) { - exactRepoIds.add(worktree.repoId) - const resolution = resolveExactWorktreeRoute(state, worktree) - if (resolution.kind === 'ambiguous') { - return resolution +} + +function collectRepoOperationRoutes( + repos: WorktreeOperationRouteState['repos'], + repoId: string +): Map { + const routes = new Map() + if (!repos) { + return routes + } + for (const repo of repos) { + if (repo.id !== repoId) { + continue } - if (resolution.kind === 'resolved') { - addRoute(exactRoutes, resolution.route) + if (!repo.executionHostId?.trim() && !repo.connectionId?.trim()) { + continue } + addOperationRoute(routes, routeForOwner({ hostId: getRepoExecutionHostId(repo) })) + } + return routes +} + +export function resolveExplicitWorktreeOperationRouteResult( + state: WorktreeOperationRouteState, + worktreeId: string +): WorktreeOperationRouteResolution { + const exactRoutes = new Map() + const exactRepoIds = new Set() + // Why: scan every projection so focus can select the unique active-host owner (#10491). + for (const worktrees of Object.values(state.worktreesByRepo ?? {})) { + collectOwnerRoutes(state, worktrees, worktreeId, exactRoutes, exactRepoIds) + } + for (const result of Object.values(state.detectedWorktreesByRepo ?? {})) { + collectOwnerRoutes(state, result.worktrees, worktreeId, exactRoutes, exactRepoIds) } if (exactRoutes.size > 0) { - const route = exactRoutes.values().next().value - return exactRoutes.size === 1 && route ? { kind: 'resolved', route } : { kind: 'ambiguous' } + return resolveFromCandidateRoutes(exactRoutes, state.settings) } if (exactRepoIds.size === 0) { exactRepoIds.add(getRepoIdFromWorktreeId(worktreeId)) @@ -237,21 +262,15 @@ export function resolveExplicitWorktreeOperationRouteResult( const repoRoutes = new Map() for (const repoId of exactRepoIds) { const resolution = resolveIndexedRepoOperationRoute(state.repos, repoId) - if (resolution.kind === 'ambiguous') { - return resolution - } if (resolution.kind === 'resolved') { - addRoute(repoRoutes, resolution.route) + addOperationRoute(repoRoutes, resolution.route) + } else if (resolution.kind === 'ambiguous') { + for (const route of collectRepoOperationRoutes(state.repos, repoId).values()) { + addOperationRoute(repoRoutes, route) + } } } - const route = repoRoutes.values().next().value - if (repoRoutes.size === 1 && route) { - return { kind: 'resolved', route } - } - if (repoRoutes.size > 1) { - return { kind: 'ambiguous' } - } - return { kind: 'missing' } + return resolveFromCandidateRoutes(repoRoutes, state.settings) } function resolveIndexedRepoOperationRoute( diff --git a/src/renderer/src/lib/worktree-runtime-owner.test.ts b/src/renderer/src/lib/worktree-runtime-owner.test.ts index 995d7c15715c..feb38dcde642 100644 --- a/src/renderer/src/lib/worktree-runtime-owner.test.ts +++ b/src/renderer/src/lib/worktree-runtime-owner.test.ts @@ -251,7 +251,7 @@ describe('getExplicitRuntimeEnvironmentIdForWorktree', () => { ) }) - it('fails closed for conflicting detected-only HUB ownership', () => { + it('prefers the focused HUB for conflicting detected-only ownership (#10491)', () => { const ambiguousState: WorktreeRuntimeOwnerState = { settings: { activeRuntimeEnvironmentId: 'hub-a' }, repos: [], @@ -280,6 +280,41 @@ describe('getExplicitRuntimeEnvironmentIdForWorktree', () => { } } + expect(getExplicitRuntimeEnvironmentIdForWorktree(ambiguousState, 'shared-worktree')).toBe( + 'hub-a' + ) + expect(getExecutionHostIdForWorktree(ambiguousState, 'shared-worktree')).toBe('ssh:private-a') + }) + + it('fails closed for conflicting detected-only ownership when focus matches neither', () => { + const ambiguousState: WorktreeRuntimeOwnerState = { + settings: { activeRuntimeEnvironmentId: 'hub-c' }, + repos: [], + worktreesByRepo: {}, + detectedWorktreesByRepo: { + 'repo-a': { + worktrees: [ + { + id: 'shared-worktree', + repoId: 'repo-a', + hostId: 'ssh:private-a', + runtimeOwnerEnvironmentId: 'hub-a' + } + ] + }, + 'repo-b': { + worktrees: [ + { + id: 'shared-worktree', + repoId: 'repo-b', + hostId: 'ssh:private-b', + runtimeOwnerEnvironmentId: 'hub-b' + } + ] + } + } + } + expect(getExplicitRuntimeEnvironmentIdForWorktree(ambiguousState, 'shared-worktree')).toBeNull() expect(getExecutionHostIdForWorktree(ambiguousState, 'shared-worktree')).toBe( 'runtime:unresolved-owner' diff --git a/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts b/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts index 20a325a5d7dd..46cae2acd23b 100644 --- a/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts +++ b/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts @@ -381,11 +381,15 @@ describe('file explorer deletion owner provenance', () => { expect(fsDeletePath).not.toHaveBeenCalled() }) - it('fails closed when an exact worktree ID belongs to multiple hosts', async () => { + it('fails closed when an exact worktree ID belongs to multiple hosts and focus matches neither', async () => { useAppStore.setState({ - repos: duplicateHostRepos(), + settings: { activeRuntimeEnvironmentId: 'other-hub' } as never, + repos: [], worktreesByRepo: { - [LOCAL_REPO_ID]: [makeWorktree('local'), makeWorktree(`ssh:${SSH_ID}`)] + [LOCAL_REPO_ID]: [ + makeWorktree(`ssh:${SSH_ID}`, 'hub-a'), + makeWorktree(`ssh:${SSH_ID}`, 'hub-b') + ] } }) const owner = getFileExplorerOperationOwner(LOCAL_WORKTREE_ID) @@ -402,9 +406,13 @@ describe('file explorer deletion owner provenance', () => { it('does not pop the batch confirm for an unresolved-owner multi-select', async () => { useAppStore.setState({ - repos: duplicateHostRepos(), + settings: { activeRuntimeEnvironmentId: 'other-hub' } as never, + repos: [], worktreesByRepo: { - [LOCAL_REPO_ID]: [makeWorktree('local'), makeWorktree(`ssh:${SSH_ID}`)] + [LOCAL_REPO_ID]: [ + makeWorktree(`ssh:${SSH_ID}`, 'hub-a'), + makeWorktree(`ssh:${SSH_ID}`, 'hub-b') + ] } }) const owner = getFileExplorerOperationOwner(LOCAL_WORKTREE_ID) diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 72d099a27318..3083fe5ed7a1 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -5911,10 +5911,11 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.remove).not.toHaveBeenCalled() }) - it('fails HUB-owned SSH removal closed when the exact id has two HUB owners', async () => { + it('fails HUB-owned SSH removal closed when the exact id has two HUB owners and focus matches neither', async () => { const store = createTestStore() const worktreeId = 'repo-ssh::/srv/same-wt' store.setState({ + settings: { activeRuntimeEnvironmentId: 'hub-c' } as never, worktreesByRepo: { 'repo-ssh': [ makeWorktree({ @@ -6106,25 +6107,45 @@ describe('worktree remote runtime mutations', () => { expect(store.getState().worktreesByRepo['repo-ssh']).toEqual([]) }) - it('fails closed before deleting an exact worktree id owned by multiple hosts', async () => { + it('fails closed before deleting an exact worktree id owned by multiple hosts when focus matches neither', async () => { const store = createTestStore() const worktreeId = 'repo-shared::/same/path' store.setState({ + settings: { activeRuntimeEnvironmentId: 'other-hub' } as never, repos: [ - { id: 'repo-shared', path: '/local', displayName: 'Local', badgeColor: '#000', addedAt: 0 }, { id: 'repo-shared', - path: '/remote', - displayName: 'SSH', + path: '/remote-a', + displayName: 'SSH A', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-a', + executionHostId: 'runtime:hub-a' + }, + { + id: 'repo-shared', + path: '/remote-b', + displayName: 'SSH B', badgeColor: '#111', addedAt: 1, - connectionId: 'ssh-1' + connectionId: 'ssh-b', + executionHostId: 'runtime:hub-b' } ], worktreesByRepo: { 'repo-shared': [ - makeWorktree({ id: worktreeId, repoId: 'repo-shared', hostId: 'local' }), - makeWorktree({ id: worktreeId, repoId: 'repo-shared', hostId: 'ssh:ssh-1' }) + makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'ssh:ssh-a', + runtimeOwnerEnvironmentId: 'hub-a' + }), + makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'ssh:ssh-b', + runtimeOwnerEnvironmentId: 'hub-b' + }) ] } } as Partial) @@ -6202,10 +6223,11 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.forceDeletePreservedBranch).not.toHaveBeenCalled() }) - it('fails preserved branch deletion closed for two HUB owners', async () => { + it('fails preserved branch deletion closed for two HUB owners when focus matches neither', async () => { const store = createTestStore() const worktreeId = 'repo-ssh::/srv/same-wt' store.setState({ + settings: { activeRuntimeEnvironmentId: 'hub-c' } as never, worktreesByRepo: { 'repo-ssh': [ makeWorktree({