Preserve execution host identity throughout folder sidebar rows (#26634)

Folder rows used the bare folder ID for active state, DOM IDs, virtual keys
and activation, so same-ID folders on two hosts collided. Use the existing
host-qualified worktree identity, matching git worktree rows.

Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
Neil
2026-10-08 22:34:43 -07:00
committed by GitHub
co-authored by m4air
parent 6a15cd8972
commit 4e232f96c7
11 changed files with 228 additions and 29 deletions
@@ -68,7 +68,7 @@ function setFolderWorkspaceFixtureState(
experimentalNewWorktreeCardStyle?: boolean
nestedGroup?: boolean
} = {}
): void {
) {
const parentGroup: ProjectGroup | null = options.nestedGroup
? {
id: 'folder-parent-group-1',
@@ -152,6 +152,7 @@ function setFolderWorkspaceFixtureState(
worktreeLineageById: {},
worktreesByRepo: {}
}
return { folderWorkspace, group }
}
// Why: describe title is shared across the split files so test full names stay stable.
@@ -160,12 +161,39 @@ describe('WorktreeList lineage child card renderer', () => {
await loadWorktreeList()
}, 60_000)
it.each(['runtime:host-a', 'runtime:host-b'] as const)(
'marks only the selected folder host %s as active',
async (selectedHost) => {
const { folderWorkspace, group } = setFolderWorkspaceFixtureState()
const hosts = ['runtime:host-a', 'runtime:host-b'] as const
mockStore.state.folderWorkspaces = hosts.map((executionHostId) => ({
...folderWorkspace,
executionHostId,
projectGroupId: executionHostId,
name: executionHostId
}))
mockStore.state.projectGroups = hosts.map((executionHostId) => ({
...group,
id: executionHostId,
executionHostId
}))
mockStore.state.activeWorkspaceExecutionHostId = selectedHost
const markup = await renderWorktreeListMarkup()
const options = Array.from(markup.matchAll(/<div[^>]*role="option"[^>]*>/g), ([tag]) => tag)
expect(options).toHaveLength(2)
const active = options.filter((tag) => tag.includes('aria-current="page"'))
expect(active).toHaveLength(1)
expect(active[0]).toContain(`data-worktree-host-identity="${selectedHost}|folder:`)
expect(markup.match(/data-worktree-card-active="true"/g)).toHaveLength(1)
}
)
it('points aria-activedescendant at the active folder workspace row', async () => {
setFolderWorkspaceFixtureState()
const markup = await renderWorktreeListMarkup()
expect(markup).toContain(
'aria-activedescendant="worktree-list-option-folder%3Afolder-workspace-1"'
'aria-activedescendant="worktree-list-option-local%7Cfolder%3Afolder-workspace-1"'
)
})
@@ -1,4 +1,5 @@
import type { HostSectionRow } from '../../host-section-rows'
import { getFolderWorkspaceHostIdentity } from '../../../../../../shared/folder-workspace-worktree'
type WorktreeItemRow = Extract<HostSectionRow, { type: 'item' }>
export type RenderRow =
@@ -25,7 +26,7 @@ export function getRenderRowKey(row: RenderRow): string {
return `pending:${row.creationId}`
}
if (row.type === 'folder-workspace') {
return `folder-workspace:${row.folderWorkspace.id}`
return `folder-workspace:${getFolderWorkspaceHostIdentity(row.folderWorkspace)}`
}
return `wt:${row.rowKey}`
}
@@ -10,6 +10,7 @@ import type { WorkspaceStatusDefinition, Worktree } from '../../../../../../shar
import type { WorktreeLineage } from '../../../../../../shared/worktree/lineage-types'
import type { ExecutionHostId } from '../../../../../../shared/execution-host'
import { folderWorkspaceKey } from '../../../../../../shared/workspace-scope'
import { getFolderWorkspaceHostIdentity } from '../../../../../../shared/folder-workspace-worktree'
import { getHostDisplayLabelOverrides } from '../../../../../../shared/host-setting-overrides'
import { buildRows } from '../grouping/build-rows'
import type { ProjectGroupingModel } from '../grouping/project-grouping'
@@ -55,6 +56,7 @@ function collectRenderedSidebarRowKeys(sectionRows: ReturnType<typeof addHostSec
} else if (row.type === 'item') {
keys.add(row.rowKey)
} else if (row.type === 'folder-workspace') {
keys.add(getFolderWorkspaceHostIdentity(row.folderWorkspace))
keys.add(folderWorkspaceKey(row.folderWorkspace.id))
} else if (row.type === 'pending-creation') {
keys.add(`pending:${row.creationId}`)
@@ -1,4 +1,4 @@
import { folderWorkspaceKey } from '../../../../../../shared/workspace-scope'
import { getFolderWorkspaceHostIdentity } from '../../../../../../shared/folder-workspace-worktree'
import type { RenderRow } from '../listing/render-row'
import {
getWorktreeExecutionHostId,
@@ -33,7 +33,7 @@ export function getRenderRowOptionId(
return getWorktreeOptionId(row.rowKey)
}
if (row.type === 'folder-workspace') {
return getWorktreeOptionId(folderWorkspaceKey(row.folderWorkspace.id))
return getWorktreeOptionId(getFolderWorkspaceHostIdentity(row.folderWorkspace))
}
return undefined
}
@@ -1,8 +1,18 @@
import type { FolderWorkspace } from '../../../../../../shared/folder-workspace-types'
import type { ProjectGroup } from '../../../../../../shared/project-group-types'
import type { WorkspaceStatusDefinition, Worktree } from '../../../../../../shared/worktree/types'
import { folderWorkspaceToWorktree } from '../../../../../../shared/folder-workspace-worktree'
import { parseWorkspaceKey } from '../../../../../../shared/workspace-scope'
import {
folderWorkspaceToWorktree,
getFolderWorkspaceHostIdentity
} from '../../../../../../shared/folder-workspace-worktree'
import {
normalizeWorkspaceSessionKeyToWorkspaceId,
parseWorkspaceKey
} from '../../../../../../shared/workspace-scope'
import {
composeWorktreeHostIdentity,
getExecutionHostIdFromWorktreeHostIdentity
} from '../../../../../../shared/worktree/host-qualified-identity'
import { getProjectGroupHeaderKey } from '../grouping/group-keys'
import type { ExecutionHostId } from '../../../../../../shared/execution-host'
import { getFolderWorkspaceLaneKey } from '../grouping/folder-workspace-lanes'
@@ -12,13 +22,22 @@ import { getHostSectionCollapseKey } from '../../host-section-collapse'
function findFolderWorkspaceByKey(
worktreeId: string,
folderWorkspaces: readonly FolderWorkspace[]
folderWorkspaces: readonly FolderWorkspace[],
executionHostId?: ExecutionHostId | null
): FolderWorkspace | null {
const scope = parseWorkspaceKey(worktreeId)
if (scope?.type !== 'folder') {
return null
}
return folderWorkspaces.find((workspace) => workspace.id === scope.folderWorkspaceId) ?? null
return (
folderWorkspaces.find(
(workspace) =>
workspace.id === scope.folderWorkspaceId &&
(!executionHostId ||
getFolderWorkspaceHostIdentity(workspace) ===
composeWorktreeHostIdentity(executionHostId, worktreeId))
) ?? null
)
}
export function getKnownSidebarWorktreeById(
@@ -28,15 +47,17 @@ export function getKnownSidebarWorktreeById(
worktrees?: readonly Worktree[],
executionHostId?: ExecutionHostId | null
): Worktree | null {
const worktree = executionHostId
const workspaceId = normalizeWorkspaceSessionKeyToWorkspaceId(worktreeId)
const ownerHostId = getExecutionHostIdFromWorktreeHostIdentity(worktreeId) ?? executionHostId
const worktree = ownerHostId
? (worktrees?.find(
(candidate) => candidate.id === worktreeId && candidate.hostId === executionHostId
(candidate) => candidate.id === workspaceId && candidate.hostId === ownerHostId
) ?? null)
: worktreeMap.get(worktreeId)
: worktreeMap.get(workspaceId)
if (worktree) {
return worktree
}
const folderWorkspace = findFolderWorkspaceByKey(worktreeId, folderWorkspaces)
const folderWorkspace = findFolderWorkspaceByKey(workspaceId, folderWorkspaces, ownerHostId)
return folderWorkspace ? folderWorkspaceToWorktree(folderWorkspace) : null
}
@@ -55,7 +76,7 @@ export function sidebarWorkspaceStillExists(
) {
return true
}
return findFolderWorkspaceByKey(worktreeId, folderWorkspaces) !== null
return findFolderWorkspaceByKey(worktreeId, folderWorkspaces, executionHostId) !== null
}
export function getFolderWorkspaceRevealGroupKeys(
@@ -0,0 +1,89 @@
import { expect, it } from 'vitest'
import { makeFolderWorkspace } from '@/store/slices/worktrees-slice-test-fixtures'
import type { ProjectGroup } from '../../../../../../shared/project-group-types'
import { folderWorkspaceToWorktree } from '../../../../../../shared/folder-workspace-worktree'
import { getWorktreeHostIdentity } from '../../../../../../shared/worktree/host-qualified-identity'
import { getRenderRowKey, type RenderRow } from '../listing/render-row'
import { getWorktreeOptionId } from '../rows/option-dom'
import { getActiveDescendantOptionId, getRenderRowOptionId } from './active-descendant-option'
import { getKnownSidebarWorktreeById, sidebarWorkspaceStillExists } from './folder-reveal'
import {
getRenderRowSidebarKey,
renderRowContainsWorktree,
rowKeyMatchesRenderRow
} from './render-row-lookup'
const group: ProjectGroup = {
id: 'group',
name: 'Group',
parentPath: '/folder',
parentGroupId: null,
createdFrom: 'folder-scan',
tabOrder: 0,
isCollapsed: false,
color: null,
createdAt: 0,
updatedAt: 0
}
const rows: Extract<RenderRow, { type: 'folder-workspace' }>[] = (
['local', 'runtime:host'] as const
).map((executionHostId) => ({
type: 'folder-workspace',
key: executionHostId,
folderWorkspace: makeFolderWorkspace({ id: 'same', executionHostId }),
projectGroup: group,
depth: 0,
groupDepth: 0
}))
const workspaceId = 'folder:same'
it.each(['local', 'runtime:host'] as const)(
'resolves folder reveal requests to the requested host %s',
(host) => {
const workspaces = rows.map((row) => row.folderWorkspace)
const key = `${host}|${workspaceId}`
expect(getKnownSidebarWorktreeById(key, new Map(), workspaces)?.hostId).toBe(host)
expect(getKnownSidebarWorktreeById(workspaceId, new Map(), workspaces, [], host)?.hostId).toBe(
host
)
expect(sidebarWorkspaceStillExists(workspaceId, [], workspaces, host)).toBe(true)
expect(sidebarWorkspaceStillExists(workspaceId, [], workspaces, 'runtime:missing')).toBe(false)
}
)
it('gives same-ID folders distinct DOM, virtual and sidebar row identities', () => {
expect(new Set(rows.map(getRenderRowKey)).size).toBe(2)
expect(new Set(rows.map((row) => getRenderRowOptionId(row))).size).toBe(2)
for (const row of rows) {
const key = getWorktreeHostIdentity(folderWorkspaceToWorktree(row.folderWorkspace))
expect(getRenderRowSidebarKey(row)).toBe(key)
expect(rowKeyMatchesRenderRow(row, key)).toBe(true)
expect(
rowKeyMatchesRenderRow(
rows.find((other) => other !== row)!,
key
)
).toBe(false)
}
})
it.each(['local', 'runtime:host'] as const)(
'announces the selected folder on %s instead of the first same-ID row',
(host) => {
const selected = rows.find((row) => row.folderWorkspace.executionHostId === host)!
const key = getWorktreeHostIdentity(folderWorkspaceToWorktree(selected.folderWorkspace))
expect(
getActiveDescendantOptionId({
activeWorktreeId: workspaceId,
activeWorkspaceExecutionHostId: host,
pinnedDisplayPolicy: 'single-location',
renderRows: rows,
virtualItems: [{ index: 0 }, { index: 1 }]
})
).toBe(getWorktreeOptionId(key))
for (const row of rows) {
expect(renderRowContainsWorktree(row, workspaceId, host)).toBe(row === selected)
expect(renderRowContainsWorktree(row, workspaceId)).toBe(true)
}
}
)
@@ -1,9 +1,15 @@
import { folderWorkspaceKey } from '../../../../../../shared/workspace-scope'
import { folderWorkspaceToWorktree } from '../../../../../../shared/folder-workspace-worktree'
import {
folderWorkspaceToWorktree,
getFolderWorkspaceHostIdentity
} from '../../../../../../shared/folder-workspace-worktree'
import { getWorktreeExecutionHostId } from '../../../../../../shared/execution-host'
import type { ExecutionHostId } from '../../../../../../shared/execution-host'
import type { Worktree } from '../../../../../../shared/worktree/types'
import { getWorktreeHostIdentity } from '../../../../../../shared/worktree/host-qualified-identity'
import {
composeWorktreeHostIdentity,
getWorktreeHostIdentity
} from '../../../../../../shared/worktree/host-qualified-identity'
import type { RenderRow } from '../listing/render-row'
import type { PinnedWorktreeDisplayPolicy } from '../grouping/row-types'
import { isPinnedWorktreeRow, type WorktreeItemRow } from '../listing/renderable-rows'
@@ -16,7 +22,7 @@ export function getRenderRowSidebarKey(row: RenderRow): string | null {
return row.rowKey
}
if (row.type === 'folder-workspace') {
return folderWorkspaceKey(row.folderWorkspace.id)
return getFolderWorkspaceHostIdentity(row.folderWorkspace)
}
if (row.type === 'pending-creation') {
return `pending:${row.creationId}`
@@ -34,6 +40,9 @@ export function rowKeyMatchesRenderRow(row: RenderRow, rowKey: string): boolean
if (row.type === 'lineage-group') {
return row.rows.some((item) => item.rowKey === rowKey)
}
if (row.type === 'folder-workspace' && rowKey === folderWorkspaceKey(row.folderWorkspace.id)) {
return true
}
return getRenderRowSidebarKey(row) === rowKey
}
@@ -59,7 +68,12 @@ export function renderRowContainsWorktree(
return false
}
if (row.type === 'folder-workspace') {
return folderWorkspaceKey(row.folderWorkspace.id) === worktreeId
return (
folderWorkspaceKey(row.folderWorkspace.id) === worktreeId &&
(executionHostId === undefined ||
getFolderWorkspaceHostIdentity(row.folderWorkspace) ===
composeWorktreeHostIdentity(executionHostId, worktreeId))
)
}
if (row.type === 'lineage-group') {
return row.rows.some((item) => itemMatchesWorktree(item, worktreeId, executionHostId))
@@ -7,7 +7,10 @@ import type {
WorktreeLineage
} from '../../../../../../shared/worktree/lineage-types'
import type { Worktree } from '../../../../../../shared/worktree/types'
import { getWorktreeHostIdentity } from '../../../../../../shared/worktree/host-qualified-identity'
import {
composeWorktreeHostIdentity,
getWorktreeHostIdentity
} from '../../../../../../shared/worktree/host-qualified-identity'
import type { FolderWorkspacePathStatus } from '../../../../../../shared/folder-workspace-path-status'
import { isConfirmedStaleFolderPathStatus } from '../../../../../../shared/folder-workspace-path-status'
import { folderWorkspaceToWorktree } from '../../../../../../shared/folder-workspace-worktree'
@@ -25,6 +28,7 @@ export type FolderWorkspaceRowContext = {
newCardStyle: boolean
settings: AppState['settings']
activeWorktreeId: string | null
activeWorkspaceExecutionHostId: AppState['activeWorkspaceExecutionHostId']
currentWorktreeId: string | null
selectedWorktreeIds: ReadonlySet<string>
repoMap: Map<string, Repo>
@@ -60,6 +64,11 @@ export function renderFolderWorkspaceVirtualRow(args: {
const { ctx, row, vItem } = args
const folderWorktree = folderWorkspaceToWorktree(row.folderWorkspace)
const folderWorktreeIdentity = getWorktreeHostIdentity(folderWorktree)
const isActiveWorktree =
ctx.activeWorktreeId === folderWorktree.id &&
(!ctx.activeWorkspaceExecutionHostId ||
folderWorktreeIdentity ===
composeWorktreeHostIdentity(ctx.activeWorkspaceExecutionHostId, folderWorktree.id))
const pathStatus = ctx.getCachedFolderWorkspacePathStatus({
scope: 'folder-workspace',
folderWorkspaceId: row.folderWorkspace.id
@@ -88,13 +97,13 @@ export function renderFolderWorkspaceVirtualRow(args: {
return (
<div
key={vItem.key}
id={getWorktreeOptionId(folderWorktree.id)}
id={getWorktreeOptionId(folderWorktreeIdentity)}
role="option"
aria-selected={ctx.selectedWorktreeIds.has(folderWorktreeIdentity)}
aria-current={ctx.activeWorktreeId === folderWorktree.id ? 'page' : undefined}
aria-current={isActiveWorktree ? 'page' : undefined}
data-worktree-id={folderWorktree.id}
data-worktree-host-identity={folderWorktreeIdentity}
data-worktree-row-key={folderWorktree.id}
data-worktree-row-key={folderWorktreeIdentity}
data-worktree-virtual-row
data-worktree-virtual-row-key={String(vItem.key)}
data-worktree-virtual-row-start={vItem.start}
@@ -103,7 +112,7 @@ export function renderFolderWorkspaceVirtualRow(args: {
className="absolute left-0 right-0 top-0"
style={{ transform: getVirtualRowTransform(vItem.start) }}
onClickCapture={ctx.onRowClickCapture}
onPointerDown={(event) => ctx.onRowPointerDown(event, folderWorktree, folderWorktree.id)}
onPointerDown={(event) => ctx.onRowPointerDown(event, folderWorktree, folderWorktreeIdentity)}
>
<div
className="relative"
@@ -112,13 +121,13 @@ export function renderFolderWorkspaceVirtualRow(args: {
<WorktreeCard
worktree={folderWorktree}
repo={undefined}
isActive={ctx.activeWorktreeId === folderWorktree.id}
isActive={isActiveWorktree}
isCurrentWorktree={ctx.currentWorktreeId === folderWorktree.id}
contentIndent={cardContentIndent}
flushSurface
nativeDragEnabled={false}
onImmediateActivate={activationDisabled ? undefined : ctx.onImmediateActivate}
activationRowKey={folderWorktree.id}
activationRowKey={folderWorktreeIdentity}
onSelectionGesture={(event) => ctx.onSelectionGesture(event, folderWorktree)}
onContextMenuSelect={ctx.onContextMenuSelect}
statusPrDisplay={folderPrDisplay}
@@ -140,6 +140,7 @@ export function buildWorktreeVirtualRowContext(args: BuildArgs): WorktreeVirtual
newCardStyle: args.newCardStyle,
settings: args.settings,
activeWorktreeId: props.activeWorktreeId,
activeWorkspaceExecutionHostId: props.activeWorkspaceExecutionHostId,
currentWorktreeId: props.currentWorktreeId,
selectedWorktreeIds: props.selectedWorktreeIds,
repoMap: props.repoMap,
@@ -4,6 +4,7 @@ import type { WorkspaceAttachment } from './worktree/types'
import { getWorkspaceAttachments } from './workspace-attachments'
import {
folderWorkspaceRepoId,
getFolderWorkspaceHostIdentity,
folderWorkspaceToWorktree,
projectGroupIdFromRepoId
} from './folder-workspace-worktree'
@@ -30,6 +31,22 @@ function makeFolderWorkspace(overrides: Partial<FolderWorkspace> = {}): FolderWo
}
describe('folderWorkspaceToWorktree', () => {
it.each([
[{}, 'local|folder:folder-workspace-1'],
[{ connectionId: 'ssh host' }, 'ssh:ssh%20host|folder:folder-workspace-1'],
[
{ executionHostId: 'runtime:host' as const, connectionId: 'ignored' },
'runtime:host|folder:folder-workspace-1'
],
[
{ executionHostId: 'local' as const, connectionId: 'ignored' },
'local|folder:folder-workspace-1'
]
])('keeps folder row identity on its explicit or legacy host %j', (fields, identity) => {
const workspace = makeFolderWorkspace(fields)
expect(getFolderWorkspaceHostIdentity(workspace)).toBe(identity)
expect(folderWorkspaceToWorktree(workspace).hostId).toBe(identity.split('|')[0])
})
it('preserves multiple attachments and projects a bare Linear task without enabling review checks', () => {
const linkedItems: WorkspaceAttachment[] = [
{ provider: 'github', type: 'pr', number: 7 },
+21 -4
View File
@@ -1,7 +1,8 @@
import type { FolderWorkspace } from './folder-workspace-types'
import type { Worktree } from './worktree/types'
import { folderWorkspaceKey } from './workspace-scope'
import { parseExecutionHostId, toSshExecutionHostId } from './execution-host'
import { parseExecutionHostId, toSshExecutionHostId, type ExecutionHostId } from './execution-host'
import { composeWorktreeHostIdentity } from './worktree/host-qualified-identity'
import { normalizeWorkspaceCreatorProvenance } from './workspace-creator-provenance'
import { getWorkspaceAttachments } from './workspace-attachments'
import { normalizeWorkspaceAttachment } from './workspace-attachment-normalization'
@@ -28,6 +29,24 @@ export function projectGroupIdFromRepoId(repoId: string | null | undefined): str
return projectGroupId === '' ? null : projectGroupId
}
function getFolderWorkspaceWorktreeHostId(
workspace: Pick<FolderWorkspace, 'executionHostId' | 'connectionId'>
): ExecutionHostId {
return (
workspace.executionHostId ??
(workspace.connectionId ? toSshExecutionHostId(workspace.connectionId) : 'local')
)
}
export function getFolderWorkspaceHostIdentity(
workspace: Pick<FolderWorkspace, 'id' | 'executionHostId' | 'connectionId'>
): string {
return composeWorktreeHostIdentity(
getFolderWorkspaceWorktreeHostId(workspace),
folderWorkspaceKey(workspace.id)
)
}
export function folderWorkspaceToWorktree(folderWorkspace: FolderWorkspace): Worktree {
const linkedTask = folderWorkspace.linkedTask
const selectedTask = linkedTask
@@ -36,9 +55,7 @@ export function folderWorkspaceToWorktree(folderWorkspace: FolderWorkspace): Wor
linkedItems: folderWorkspace.linkedItems
}).find((item) => item.type === 'issue')
const creatorProvenance = normalizeWorkspaceCreatorProvenance(folderWorkspace.creatorProvenance)
const hostId =
folderWorkspace.executionHostId ??
(folderWorkspace.connectionId ? toSshExecutionHostId(folderWorkspace.connectionId) : 'local')
const hostId = getFolderWorkspaceWorktreeHostId(folderWorkspace)
const parsedHost = parseExecutionHostId(hostId)
return {
id: folderWorkspaceKey(folderWorkspace.id),