diff --git a/src/renderer/src/components/workspace-surface-projection.test.ts b/src/renderer/src/components/workspace-surface-projection.test.ts index 016184d001a..83b9f03484b 100644 --- a/src/renderer/src/components/workspace-surface-projection.test.ts +++ b/src/renderer/src/components/workspace-surface-projection.test.ts @@ -11,7 +11,7 @@ import { readFileSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { projectWorkspaceSurfaces } from './workspace-surface-projection' -import { getIndexedWorktreeMap } from '../store/worktree-repo-index' +import { getIndexedAllWorktrees, getIndexedWorktreeMap } from '../store/worktree-repo-index' import type { ExecutionHostId } from '../../../shared/execution-host' import type { FolderWorkspace } from '../../../shared/folder-workspace-types' import type { Worktree } from '../../../shared/worktree/types' @@ -163,6 +163,148 @@ describe('projectWorkspaceSurfaces', () => { }) }) +// The collapse only ever removes a duplicate id. Losing a surface the user owns +// unmounts live terminals, which is strictly worse than the duplicate mount this +// fixes, so every shape that reaches the workbench is pinned against dropping one. +describe('projectWorkspaceSurfaces never under-selects', () => { + const unqualifiedWorktree: Worktree = { ...localWorktree, hostId: undefined } + const secondSshWorktree: Worktree = { ...localWorktree, hostId: 'ssh:ci-box' } + const localOnlyFolder: FolderWorkspace = { + ...localFolder, + id: 'folder-local-only', + executionHostId: undefined + } + + it('emits every distinct id from a mixed local, SSH, runtime and folder catalog', () => { + const distinctWorktree: Worktree = { + ...sshWorktree, + id: 'repo-shared::/work/orca-ssh-only', + path: '/work/orca-ssh-only' + } + const distinctFolder: FolderWorkspace = { ...runtimeFolder, id: 'folder-runtime-only' } + const worktrees = [localWorktree, sshWorktree, secondSshWorktree, distinctWorktree] + const folderWorkspaces = [localFolder, runtimeFolder, localOnlyFolder, distinctFolder] + + const surfaceIds = project({ worktrees, folderWorkspaces }).map((surface) => surface.id) + + const expectedIds = new Set([ + ...worktrees.map((worktree) => worktree.id), + ...folderWorkspaces.map((workspace) => `folder:${workspace.id}`) + ]) + expect(new Set(surfaceIds)).toEqual(expectedIds) + expect(surfaceIds).toHaveLength(expectedIds.size) + }) + + it('mounts a local-only catalog whose rows never name a host', () => { + const otherUnqualified: Worktree = { + ...unqualifiedWorktree, + id: 'repo-shared::/work/orca-other', + path: '/work/orca-other' + } + + expect( + project({ + worktrees: [unqualifiedWorktree, otherUnqualified], + folderWorkspaces: [localOnlyFolder] + }) + ).toEqual([ + { id: SHARED_WORKTREE_ID, path: '/work/orca-feature' }, + { id: 'repo-shared::/work/orca-other', path: '/work/orca-other' }, + { id: 'folder:folder-local-only', path: '/work/orca-local' } + ]) + }) + + it('keeps the id when an unqualified row collides with a host-qualified one', () => { + // `composeWorktreeHostIdentity` gives an unqualified row its own bucket, so + // the pair survives the host-qualified index and must still collapse to one id. + expect(project({ worktrees: [unqualifiedWorktree, localWorktree] })).toEqual([ + { id: SHARED_WORKTREE_ID, path: '/work/orca-feature' } + ]) + expect(project({ worktrees: [localWorktree, unqualifiedWorktree] })).toEqual([ + { id: SHARED_WORKTREE_ID, path: '/work/orca-feature' } + ]) + }) + + it('collapses two SSH hosts publishing one worktree id, with no local row present', () => { + expect(project({ worktrees: [sshWorktree, secondSshWorktree] })).toEqual([ + { id: SHARED_WORKTREE_ID, path: '/work/orca-feature' } + ]) + }) + + it('keeps a git worktree and a folder workspace apart even on the same id text', () => { + // `folderWorkspaceKey` prefixes `folder:`; a worktree id is `repoId::path`. + const surfaces = project({ + worktrees: [{ ...localWorktree, id: 'folder-shared', path: '/work/collide' }], + folderWorkspaces: [localFolder] + }) + + expect(surfaces).toEqual([ + { id: 'folder-shared', path: '/work/collide' }, + { id: 'folder:folder-shared', path: '/work/orca-local' } + ]) + }) + + it('collapses folder rows across three hosts to one surface without losing the id', () => { + const sshFolder: FolderWorkspace = { + ...localFolder, + folderPath: '/ssh/orca', + executionHostId: undefined, + connectionId: 'build-box' + } + + const surfaces = project({ + folderWorkspaces: [localFolder, runtimeFolder, sshFolder], + activeWorkspaceId: 'folder:folder-shared', + activeWorkspaceResolvedHostId: 'ssh:build-box' + }) + + expect(surfaces).toEqual([{ id: 'folder:folder-shared', path: '/ssh/orca' }]) + }) + + it('does not let a folder row that names no host win the local tie-break', () => { + // `getCatalogOwnerHostId` defaults an unstamped row to `local`; honouring that + // would mount the unstamped row's path over the row that really is local. + const unstampedPeer: FolderWorkspace = { + ...localFolder, + folderPath: '/unknown/orca', + executionHostId: undefined, + connectionId: undefined + } + + expect( + project({ + folderWorkspaces: [localFolder, unstampedPeer], + activeWorkspaceId: 'folder:folder-shared', + activeWorkspaceResolvedHostId: 'local' + }) + ).toEqual([{ id: 'folder:folder-shared', path: '/work/orca-local' }]) + }) + + it('emits nothing extra and nothing missing for an empty catalog', () => { + expect(project({})).toEqual([]) + }) +}) + +// The feed swapped `useAllWorktrees` for `useWorktreeMap`. Both read the same +// WeakMap-cached snapshot of `worktreesByRepo`, so the zustand `Object.is` compare +// still re-renders on exactly the writes that replace the slice — no dropped update. +describe('worktree surface feed subscription identity', () => { + const worktreesByRepo = { 'repo-shared': [localWorktree, sshWorktree] } + + it('returns a stable map for an unchanged slice and a fresh one after a replace', () => { + expect(getIndexedWorktreeMap(worktreesByRepo)).toBe(getIndexedWorktreeMap(worktreesByRepo)) + expect(getIndexedWorktreeMap({ ...worktreesByRepo })).not.toBe( + getIndexedWorktreeMap(worktreesByRepo) + ) + }) + + it('exposes the same id set the host-qualified array does', () => { + expect(new Set(getIndexedWorktreeMap(worktreesByRepo).keys())).toEqual( + new Set(getIndexedAllWorktrees(worktreesByRepo).map((worktree) => worktree.id)) + ) + }) +}) + // Why source text: the per-id collapse lives in the store index, so the module // tests above stay green even if Terminal.tsx goes back to flattening the // host-qualified array itself. The feed is the half that has to be ratcheted. diff --git a/src/renderer/src/components/workspace-surface-projection.ts b/src/renderer/src/components/workspace-surface-projection.ts index f90d416d095..1a015a51744 100644 --- a/src/renderer/src/components/workspace-surface-projection.ts +++ b/src/renderer/src/components/workspace-surface-projection.ts @@ -1,4 +1,4 @@ -import type { ExecutionHostId } from '../../../shared/execution-host' +import { parseExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' import type { FolderWorkspace } from '../../../shared/folder-workspace-types' import type { Worktree } from '../../../shared/worktree/types' import { folderWorkspaceKey } from '../../../shared/workspace-scope' @@ -11,6 +11,23 @@ type FolderWorkspaceSurfaceRow = Pick< 'id' | 'folderPath' | 'connectionId' | 'executionHostId' > +/** + * The row's own host, or null when the row names none. + * + * `getCatalogOwnerHostId` defaults an unstamped row to `local`, which is the + * right answer for an owner lookup but the wrong one for a tie-break: it would + * let a row that never named a host win the `local` tie and mount another + * host's path. Only a row that names its own host may claim the collision. + */ +function getStampedFolderWorkspaceHostId( + workspace: FolderWorkspaceSurfaceRow +): ExecutionHostId | null { + const namesOwnHost = Boolean( + parseExecutionHostId(workspace.executionHostId) ?? workspace.connectionId?.trim() + ) + return namesOwnHost ? getCatalogOwnerHostId(workspace) : null +} + /** * The terminal workbench's mount set: exactly one surface per workspace id. * @@ -53,11 +70,12 @@ export function projectWorkspaceSurfaces({ // Why: a folder-workspace id is opaque, not path-derived, so colliding hosts // disagree on the path; only the active workspace's resolved host breaks the tie. // Deriving that host from the row alone is sufficient because every stored row is - // stamped with an explicit `executionHostId` by `folderWorkspaceWithFetchedOwner`. + // stamped with an explicit `executionHostId` by `folderWorkspaceWithFetchedOwner`; + // an unstamped row keeps first-wins rather than guessing. if ( activeWorkspaceResolvedHostId && id === activeWorkspaceId && - getCatalogOwnerHostId(workspace) === activeWorkspaceResolvedHostId + getStampedFolderWorkspaceHostId(workspace) === activeWorkspaceResolvedHostId ) { surfaces[existingIndex] = surface }