diff --git a/src/renderer/src/components/cmd-j/palette-project-results.ts b/src/renderer/src/components/cmd-j/palette-project-results.ts index dc80c45b763e..ac0b595a6917 100644 --- a/src/renderer/src/components/cmd-j/palette-project-results.ts +++ b/src/renderer/src/components/cmd-j/palette-project-results.ts @@ -12,6 +12,7 @@ import type { Project, ProjectHostSetup } from '../../../../shared/project-types import type { Repo } from '../../../../shared/repo-types' import { translate } from '@/i18n/i18n' import { getProjectGroupHeaderKey } from '../../../../shared/sidebar-list/group-keys' +import { getDistinctProjectGroups } from '../../../../shared/project-groups' import { getProjectHeaderRevealTarget } from '../../../../shared/project-section-grouping' import type { ProjectGroupingModel } from '../../../../shared/project-section-grouping' @@ -81,7 +82,7 @@ function buildCmdJProjectSearchCandidates({ const repoMap = new Map(repos.map((repo) => [repo.id, repo])) const candidates: CmdJProjectSearchResult[] = [] - projectGroups.forEach((group, order) => { + getDistinctProjectGroups(projectGroups).forEach((group, order) => { candidates.push({ id: `project-group:${group.id}`, kind: 'project-group', diff --git a/src/renderer/src/components/cmd-j/palette-results.test.ts b/src/renderer/src/components/cmd-j/palette-results.test.ts index 4c87444e39ca..2686cacad3fe 100644 --- a/src/renderer/src/components/cmd-j/palette-results.test.ts +++ b/src/renderer/src/components/cmd-j/palette-results.test.ts @@ -483,6 +483,23 @@ describe('Cmd+J project and repo-group search', () => { }) }) + it('lists a same-id Project Group from two hosts once, by the name the sidebar shows', () => { + const search = (query: string) => + searchCmdJProjectResults({ + query, + projectGroups: [ + { ...projectGroup('group-1', 'Infrastructure'), executionHostId: 'local' }, + { ...projectGroup('group-1', 'Platform'), executionHostId: 'runtime:env-b' } + ], + repos: [], + projects: [], + projectHostSetups: [] + }).map((result) => [result.id, result.title]) + + expect(search('platform')).toEqual([['project-group:group-1', 'Platform']]) + expect(search('infra')).toEqual([]) + }) + it('finds a Project by project name and a repo-backed fallback by repo name', () => { const projectResults = searchCmdJProjectResults({ query: 'api', diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 28e04bfea435..9de684f1e72a 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -327,6 +327,7 @@ const WorktreeList = React.memo(function WorktreeList({ workspaceStatuses={workspaceStatuses} projectGrouping={projectGrouping} projectGroups={projectGroups} + visibleProjectGroups={visibleScope.visibleProjectGroupsForRows} onMoveWorktreeToStatus={statusMutations.moveWorktreeToStatus} onMoveWorktreesToStatus={statusMutations.moveWorktreesToStatus} onMoveWorktreesToStatusAtIndex={statusMutations.moveWorktreesToStatusAtIndex} diff --git a/src/renderer/src/components/sidebar/worktree-list-groups-same-id-host-groups.test.ts b/src/renderer/src/components/sidebar/worktree-list-groups-same-id-host-groups.test.ts new file mode 100644 index 000000000000..ef03e3a16880 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list-groups-same-id-host-groups.test.ts @@ -0,0 +1,125 @@ +/** + * #30045: moving a project to a managed server copies its project group to the server under the + * same id, and the desktop keeps its own copy. The sidebar must still draw that group once. + */ +import { describe, expect, it } from 'vitest' +import { buildRows } from './worktree-list/grouping/build-rows.test-fixture' +import { repo, worktree } from './worktree-list-groups-test-fixtures' +import { getRenderRowKey } from './worktree-list/listing/render-row' +import { getSidebarRowRevealAncestorKeys } from './worktree-list/navigation/reveal-ancestors' +import { getProjectGroupOccurrencePaths } from '../../../../shared/sidebar-list/project-group-sections' +import type { ProjectGroup } from '../../../../shared/project-group-types' +import type { Repo } from '../../../../shared/repo-types' +import type { Worktree } from '../../../../shared/worktree/types' +import type { ExecutionHostId } from '../../../../shared/execution-host' + +const REMOTE_HOST: ExecutionHostId = 'runtime:env-b' + +function group(executionHostId: ExecutionHostId, overrides: Partial = {}) { + return { + id: 'group-shared', + name: 'Platform', + parentPath: null, + executionHostId, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1, + ...overrides + } satisfies ProjectGroup +} + +const groupedRepo: Repo = { ...repo, projectGroupId: 'group-shared' } +const localWorktree: Worktree = { + ...worktree, + id: 'wt-local', + hostId: 'local' +} +const remoteWorktree: Worktree = { + ...worktree, + id: 'wt-remote', + hostId: REMOTE_HOST +} + +function rowsFor(projectGroups: readonly ProjectGroup[], collapsed = new Set()) { + const worktrees = [localWorktree, remoteWorktree] + return buildRows( + 'repo', + worktrees, + new Map([[groupedRepo.id, groupedRepo]]), + null, + collapsed, + undefined, + undefined, + undefined, + {}, + new Map(worktrees.map((entry) => [entry.id, entry])), + false, + undefined, + projectGroups + ) +} + +describe('a project group both hosts publish under one id', () => { + it('draws one header holding the project, not a second empty one', () => { + const rows = rowsFor([group('local'), group(REMOTE_HOST, { name: 'Renamed on server' })]) + + const groupHeaders = rows.filter((row) => row.type === 'header' && row.projectGroup) + expect(groupHeaders).toHaveLength(1) + // The same copy every id-keyed lookup (and Cmd+J) resolves to. + expect(groupHeaders[0]).toMatchObject({ + key: 'project-group:group-shared', + label: 'Renamed on server', + count: 1 + }) + expect(rows.filter((row) => row.type === 'item')).toHaveLength(2) + }) + + it('gives every rendered row a distinct key', () => { + const keys = rowsFor([group('local'), group(REMOTE_HOST)]).map(getRenderRowKey) + + expect(new Set(keys).size).toBe(keys.length) + }) + + it('keeps a same-id child group nested once under its parent', () => { + const parent = (hostId: ExecutionHostId) => group(hostId, { id: 'parent', name: 'Parent' }) + const child = (hostId: ExecutionHostId) => group(hostId, { parentGroupId: 'parent' }) + + const rows = rowsFor([parent('local'), child('local'), parent(REMOTE_HOST), child(REMOTE_HOST)]) + + expect( + rows.flatMap((row) => + row.type === 'header' && row.projectGroup ? [[row.key, row.projectGroupDepth]] : [] + ) + ).toEqual([ + ['project-group:parent', 0], + ['project-group:group-shared', 1] + ]) + }) + + it('reveals the drawn copy when the copies have different parents', () => { + const groups = [ + group('local'), + group(REMOTE_HOST, { id: 'parent', name: 'Parent' }), + group(REMOTE_HOST, { parentGroupId: 'parent' }) + ] + + const collapsedParentRows = rowsFor(groups, new Set(['project-group:parent'])) + expect( + collapsedParentRows.filter((row) => row.type === 'header').map((row) => row.key) + ).toEqual(['project-group:parent']) + expect( + getSidebarRowRevealAncestorKeys({ + rowKey: 'project-group:group-shared', + repoMap: new Map(), + projectGroups: groups + }) + ).toEqual(['project-group:parent']) + expect(getProjectGroupOccurrencePaths(groups).get('group-shared')?.parent?.key).toBe( + 'project-group:parent' + ) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts index 5b038c8902a6..e13f21c6386c 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.test.ts @@ -141,7 +141,7 @@ describe('worktree list folder reveal', () => { getFolderWorkspaceRevealGroupKeys( folderWorkspaceKey(folderWorkspace.id), [{ ...folderWorkspace, executionHostId: otherHostId }, folderWorkspace], - [root, child, otherRoot, otherChild], + [otherRoot, otherChild, root, child], { executionHostId, defaultHostId: 'local' } ) ).toEqual([ @@ -151,6 +151,31 @@ describe('worktree list folder reveal', () => { ]) } ) + + it('follows the parents the sidebar draws when same-id group copies disagree (#30045)', () => { + // The sidebar draws one header per group id, from the last copy, so reveal must open its parent. + const localChild = makeProjectGroup({ id: 'group-child', executionHostId: 'local' }) + const remoteRoot = makeProjectGroup({ id: 'remote-root', executionHostId: 'runtime:other' }) + const remoteChild = makeProjectGroup({ + id: localChild.id, + parentGroupId: remoteRoot.id, + executionHostId: 'runtime:other' + }) + const folderWorkspace = makeFolderWorkspace({ executionHostId: 'local' }) + + expect( + getFolderWorkspaceRevealGroupKeys( + folderWorkspaceKey(folderWorkspace.id), + [folderWorkspace], + [localChild, remoteRoot, remoteChild], + { executionHostId: 'local', defaultHostId: 'local' } + ) + ).toEqual([ + getProjectGroupHeaderKey(remoteRoot.id), + getProjectGroupHeaderKey(localChild.id), + 'host:local' + ]) + }) }) describe('reveal keys under non-repo grouping', () => { diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts index 22bd00bdcf6b..8dae63a1b4ef 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/folder-reveal.ts @@ -111,12 +111,17 @@ export function getFolderWorkspaceRevealGroupKeys( ) .map((group) => [group.id, group]) ) + // Why unfiltered: same-id host copies render under the last copy's parents, whichever host's row + // the folder is (getDistinctProjectGroups); header keys carry no host. + const renderedGroupsById = new Map(projectGroups.map((group) => [group.id, group])) const keys: string[] = [] const seen = new Set() - let groupId: string | null = folderWorkspace.projectGroupId + let groupId: string | null = groupsById.has(folderWorkspace.projectGroupId) + ? folderWorkspace.projectGroupId + : null while (groupId && !seen.has(groupId)) { seen.add(groupId) - const group = groupsById.get(groupId) + const group = renderedGroupsById.get(groupId) if (!group) { break } diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/reveal-ancestors.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/reveal-ancestors.ts index 0dd0cdc54286..2220d38d4d7a 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/reveal-ancestors.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/reveal-ancestors.ts @@ -72,7 +72,8 @@ export function getSidebarRowRevealAncestorKeys(args: { }): string[] { if (args.rowKey.startsWith('project-group:')) { const groupId = args.rowKey.slice('project-group:'.length) - const group = args.projectGroups.find((candidate) => candidate.id === groupId) + // Why the last copy: same-id host copies render as that one (getDistinctProjectGroups). + const group = args.projectGroups.findLast((candidate) => candidate.id === groupId) return getProjectGroupAncestorKeys(group?.parentGroupId, args.projectGroups) } const keys = new Set() diff --git a/src/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsx b/src/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsx index 5d005b1bf8f6..7a022e795c63 100644 --- a/src/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsx +++ b/src/renderer/src/components/sidebar/worktree-list/viewport/VirtualizedWorktreeViewport.tsx @@ -162,7 +162,7 @@ export const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktr prCache: props.prCache, workspaceStatuses, settings, - projectGroups, + projectGroups: props.visibleProjectGroups ?? projectGroups, projectGrouping: props.projectGrouping, flashRevealedRow: reveal.flashRevealedRow, markRevealScroll: scrollSuppression.markRevealScroll, diff --git a/src/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.ts b/src/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.ts index 34b3d4827c28..4629e4658df6 100644 --- a/src/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.ts +++ b/src/renderer/src/components/sidebar/worktree-list/viewport/viewport-props.ts @@ -85,6 +85,8 @@ export type VirtualizedWorktreeViewportProps = { workspaceStatuses: readonly WorkspaceStatusDefinition[] projectGrouping?: ProjectGroupingModel projectGroups?: readonly ProjectGroup[] + /** The host-filtered groups the rows were built from; reveal must walk the same parents. */ + visibleProjectGroups?: readonly ProjectGroup[] onMoveWorktreeToStatus: (worktreeId: string, status: WorkspaceStatus) => void onMoveWorktreesToStatus: (worktreeIds: readonly string[], status: WorkspaceStatus) => void onMoveWorktreesToStatusAtIndex: (args: WorktreeStatusDropAtIndexArgs) => void diff --git a/src/shared/project-groups.ts b/src/shared/project-groups.ts index 4cfa87080e21..13c91985bf8d 100644 --- a/src/shared/project-groups.ts +++ b/src/shared/project-groups.ts @@ -185,3 +185,14 @@ export function getNextProjectGroupOrder(repos: readonly Repo[], groupId: string } return max + 1 } + +/** + * One row per group id, in first-seen order. Hosts can publish one group under the same id (a + * project moved to a managed server keeps the desktop's copy, #30045); the last copy wins, as in + * every id-keyed group lookup, so the sidebar and search agree on its name. + */ +export function getDistinctProjectGroups>( + projectGroups: readonly T[] +): T[] { + return [...new Map(projectGroups.map((group) => [group.id, group])).values()] +} diff --git a/src/shared/sidebar-list/project-group-sections.ts b/src/shared/sidebar-list/project-group-sections.ts index ed80636b89cc..d28ba4738da5 100644 --- a/src/shared/sidebar-list/project-group-sections.ts +++ b/src/shared/sidebar-list/project-group-sections.ts @@ -5,7 +5,7 @@ import { } from './folder-workspace-lanes' import type { ProjectGroup } from '../project-group-types' import type { ProjectOrderBy } from '../ui-chrome-types' -import { getEffectiveProjectGroupManualRank } from '../project-groups' +import { getDistinctProjectGroups, getEffectiveProjectGroupManualRank } from '../project-groups' import { getProjectGroupHeaderKey } from './group-keys' import { appendOrderedGroups } from './group-sections' import { compareCodePoints, compareSidebarText } from './text-order' @@ -76,7 +76,8 @@ export function appendProjectGroupSections( ) } const childGroupsByParentId = new Map() - for (const group of projectGroups) { + // Why distinct: membership is id-keyed, so a same-id copy's header would sit empty (#30045). + for (const group of getDistinctProjectGroups(projectGroups)) { const parentId = group.parentGroupId && projectGroupsById.has(group.parentGroupId) ? group.parentGroupId : null const children = childGroupsByParentId.get(parentId) ?? [] @@ -152,7 +153,7 @@ export function appendProjectGroupSections( export function getProjectGroupOccurrencePaths(projectGroups: readonly ProjectGroup[]) { const groupsById = new Map(projectGroups.map((group) => [group.id, group])) const paths = new Map() - for (const group of projectGroups) { + for (const group of getDistinctProjectGroups(projectGroups)) { const pending: ProjectGroup[] = [] const seen = new Set() let current: ProjectGroup | undefined = group