From ec9efe4638d110ec3aa791ee29ba8cc508ac694e Mon Sep 17 00:00:00 2001 From: jeganathan I Date: Sun, 5 Jul 2026 12:31:30 +0530 Subject: [PATCH] fix: distinguish duplicate project names in picker (#6272) * fix: distinguish duplicate project names in picker * review: disambiguate duplicate project picker details Co-authored-by: Orca --------- Co-authored-by: Jinwoo-H Co-authored-by: Orca --- .../new-workspace/ProjectCombobox.test.tsx | 30 +++ src/renderer/src/hooks/useComposerState.ts | 5 +- ...new-workspace-duplicate-project-details.ts | 147 ++++++++++ .../lib/new-workspace-project-options.test.ts | 251 ++++++++++++++++++ .../src/lib/new-workspace-project-options.ts | 28 +- 5 files changed, 457 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/lib/new-workspace-duplicate-project-details.ts diff --git a/src/renderer/src/components/new-workspace/ProjectCombobox.test.tsx b/src/renderer/src/components/new-workspace/ProjectCombobox.test.tsx index 63e88f9e0d1..cb0309d3e6e 100644 --- a/src/renderer/src/components/new-workspace/ProjectCombobox.test.tsx +++ b/src/renderer/src/components/new-workspace/ProjectCombobox.test.tsx @@ -138,4 +138,34 @@ describe('ProjectCombobox', () => { expect(onValueChange).toHaveBeenCalledWith('project-group:folder-group') }) + + it('renders directory details for duplicate project names', () => { + const duplicateProjects: NewWorkspaceProjectOption[] = [ + { + kind: 'project', + id: 'project:merchant-a', + projectId: 'project:merchant-a', + displayName: 'merchant', + badgeColor: '#111111', + detail: '/workspace/storefront/merchant' + }, + { + kind: 'project', + id: 'project:merchant-b', + projectId: 'project:merchant-b', + displayName: 'merchant', + badgeColor: '#222222', + detail: '/workspace/admin/merchant' + } + ] + + act(() => { + root.render( + + ) + }) + + expect(container.textContent).toContain('/workspace/storefront/merchant') + expect(container.textContent).toContain('/workspace/admin/merchant') + }) }) diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 33049241411..dc6e1aa1957 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -825,9 +825,10 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS projects, projectHostSetups, eligibleRepos, - projectGroups + projectGroups, + hosts: hostOptions }), - [eligibleRepos, projectGroups, projectHostSetups, projects] + [eligibleRepos, hostOptions, projectGroups, projectHostSetups, projects] ) const selectedRepoSettings = useMemo(() => { if (!settings) { diff --git a/src/renderer/src/lib/new-workspace-duplicate-project-details.ts b/src/renderer/src/lib/new-workspace-duplicate-project-details.ts new file mode 100644 index 00000000000..127b8b99812 --- /dev/null +++ b/src/renderer/src/lib/new-workspace-duplicate-project-details.ts @@ -0,0 +1,147 @@ +import { getExecutionHostLabel, type ExecutionHostId } from '../../../shared/execution-host' +import type { NewWorkspaceProjectOption } from './new-workspace-project-options' + +export type ProjectSetupDirectory = { + path: string + hostId: ExecutionHostId +} + +export type ProjectOptionDraft = Extract & { + detailSource: 'provider' | 'generic' +} + +type ProjectDirectoryDetailMode = 'path' | 'host-label' | 'host-id' + +function getRepeatedValues(values: readonly string[]): Set { + const seen = new Set() + const repeated = new Set() + for (const value of values) { + if (seen.has(value)) { + repeated.add(value) + continue + } + seen.add(value) + } + return repeated +} + +function getProjectDirectoryDetail( + directories: readonly ProjectSetupDirectory[], + hostLabelById: ReadonlyMap, + mode: ProjectDirectoryDetailMode +): string | null { + const detailByKey = new Map() + for (const directory of directories) { + const path = directory.path.trim() + if (!path) { + continue + } + const hostLabel = + hostLabelById.get(directory.hostId)?.trim() || getExecutionHostLabel(directory.hostId) + const detail = + mode === 'path' + ? path + : mode === 'host-id' + ? `${hostLabel} (${directory.hostId}) · ${path}` + : `${hostLabel} · ${path}` + const key = mode === 'path' ? path : `${directory.hostId}\0${path}` + detailByKey.set(key, detail) + } + + const distinctDetails = [...detailByKey.values()].sort() + if (distinctDetails.length === 0) { + return null + } + + const [firstDetail] = distinctDetails + return distinctDetails.length === 1 + ? firstDetail + : `${firstDetail} (+${distinctDetails.length - 1} more)` +} + +export function getDuplicateProjectDetailsById( + options: readonly ProjectOptionDraft[], + setupDirectoriesByProjectId: ReadonlyMap, + hostLabelById: ReadonlyMap +): Map { + const optionsByName = new Map() + for (const option of options) { + optionsByName.set(option.displayName, [ + ...(optionsByName.get(option.displayName) ?? []), + option + ]) + } + + const detailsById = new Map() + for (const sameNameOptions of optionsByName.values()) { + if (sameNameOptions.length < 2) { + continue + } + + const repeatedProviderDetails = getRepeatedValues( + sameNameOptions + .filter((option) => option.detailSource === 'provider') + .map((option) => option.detail) + ) + const ambiguousOptions = sameNameOptions.filter( + (option) => option.detailSource === 'generic' || repeatedProviderDetails.has(option.detail) + ) + if (ambiguousOptions.length === 0) { + continue + } + + const pathOnlyDetailsById = new Map() + for (const option of ambiguousOptions) { + const detail = getProjectDirectoryDetail( + setupDirectoriesByProjectId.get(option.projectId) ?? [], + hostLabelById, + 'path' + ) + if (detail) { + pathOnlyDetailsById.set(option.id, detail) + } + } + + const repeatedPathOnlyDetails = getRepeatedValues([...pathOnlyDetailsById.values()]) + const hostLabelDetailsById = new Map() + for (const option of ambiguousOptions) { + const pathOnlyDetail = pathOnlyDetailsById.get(option.id) + if (!pathOnlyDetail || !repeatedPathOnlyDetails.has(pathOnlyDetail)) { + continue + } + const detail = getProjectDirectoryDetail( + setupDirectoriesByProjectId.get(option.projectId) ?? [], + hostLabelById, + 'host-label' + ) + if (detail) { + hostLabelDetailsById.set(option.id, detail) + } + } + + const repeatedHostLabelDetails = getRepeatedValues([...hostLabelDetailsById.values()]) + for (const option of ambiguousOptions) { + const pathOnlyDetail = pathOnlyDetailsById.get(option.id) + if (!pathOnlyDetail) { + continue + } + let detail = pathOnlyDetail + if (repeatedPathOnlyDetails.has(pathOnlyDetail)) { + detail = hostLabelDetailsById.get(option.id) ?? pathOnlyDetail + if (repeatedHostLabelDetails.has(detail)) { + // Why: user-renamed hosts can share a label, so fall back to the + // stable host id only when the final visible detail still repeats. + detail = + getProjectDirectoryDetail( + setupDirectoriesByProjectId.get(option.projectId) ?? [], + hostLabelById, + 'host-id' + ) ?? detail + } + } + detailsById.set(option.id, detail) + } + } + + return detailsById +} diff --git a/src/renderer/src/lib/new-workspace-project-options.test.ts b/src/renderer/src/lib/new-workspace-project-options.test.ts index d96cc14ba4a..67c25ab2598 100644 --- a/src/renderer/src/lib/new-workspace-project-options.test.ts +++ b/src/renderer/src/lib/new-workspace-project-options.test.ts @@ -108,6 +108,257 @@ describe('buildNewWorkspaceProjectOptions', () => { expect(options.map((option) => option.id)).toEqual(['github:stablyai/orca']) }) + it('shows configured directories when project names are duplicated', () => { + const options = buildNewWorkspaceProjectOptions({ + projects: [ + project({ + id: 'project:merchant-a', + displayName: 'merchant', + providerIdentity: undefined + }), + project({ + id: 'project:merchant-b', + displayName: 'merchant', + providerIdentity: undefined + }) + ], + projectHostSetups: [ + setup({ + id: 'merchant-a-setup', + projectId: 'project:merchant-a', + repoId: 'merchant-a-repo', + path: '/workspace/storefront/merchant' + }), + setup({ + id: 'merchant-b-setup', + projectId: 'project:merchant-b', + repoId: 'merchant-b-repo', + path: '/workspace/admin/merchant' + }) + ], + eligibleRepos: [repo('merchant-a-repo'), repo('merchant-b-repo')] + }) + + expect(options).toEqual([ + expect.objectContaining({ + id: 'project:merchant-b', + detail: '/workspace/admin/merchant' + }), + expect.objectContaining({ + id: 'project:merchant-a', + detail: '/workspace/storefront/merchant' + }) + ]) + }) + + it('keeps provider details when duplicate project names are already distinguishable', () => { + const options = buildNewWorkspaceProjectOptions({ + projects: [ + project({ + id: 'github:acme/merchant', + displayName: 'merchant', + providerIdentity: { provider: 'github', owner: 'acme', repo: 'merchant' } + }), + project({ + id: 'github:contoso/merchant', + displayName: 'merchant', + providerIdentity: { provider: 'github', owner: 'contoso', repo: 'merchant' } + }) + ], + projectHostSetups: [ + setup({ + id: 'acme-setup', + projectId: 'github:acme/merchant', + repoId: 'acme-repo', + path: '/workspace/acme/merchant' + }), + setup({ + id: 'contoso-setup', + projectId: 'github:contoso/merchant', + repoId: 'contoso-repo', + path: '/workspace/contoso/merchant' + }) + ], + eligibleRepos: [repo('acme-repo'), repo('contoso-repo')] + }) + + expect(options.map((option) => option.detail).sort()).toEqual([ + 'acme/merchant', + 'contoso/merchant' + ]) + }) + + it('shows directory details for non-provider duplicates with different setup counts', () => { + const options = buildNewWorkspaceProjectOptions({ + projects: [ + project({ + id: 'project:merchant-single', + displayName: 'merchant', + providerIdentity: undefined + }), + project({ + id: 'project:merchant-multi', + displayName: 'merchant', + providerIdentity: undefined + }) + ], + projectHostSetups: [ + setup({ + id: 'merchant-single-setup', + projectId: 'project:merchant-single', + repoId: 'merchant-single-repo', + path: '/workspace/single/merchant' + }), + setup({ + id: 'merchant-multi-local-setup', + projectId: 'project:merchant-multi', + repoId: 'merchant-multi-local-repo', + path: '/workspace/multi/local/merchant' + }), + setup({ + id: 'merchant-multi-remote-setup', + projectId: 'project:merchant-multi', + hostId: 'ssh:builder', + repoId: 'merchant-multi-remote-repo', + path: '/workspace/multi/remote/merchant' + }) + ], + eligibleRepos: [ + repo('merchant-single-repo'), + repo('merchant-multi-local-repo'), + repo('merchant-multi-remote-repo') + ] + }) + + expect(options.map((option) => option.detail).sort()).toEqual([ + '/workspace/multi/local/merchant (+1 more)', + '/workspace/single/merchant' + ]) + }) + + it('keeps provider details while disambiguating non-provider duplicate names', () => { + const options = buildNewWorkspaceProjectOptions({ + projects: [ + project({ + id: 'github:acme/merchant', + displayName: 'merchant', + providerIdentity: { provider: 'github', owner: 'acme', repo: 'merchant' } + }), + project({ + id: 'project:merchant-local', + displayName: 'merchant', + providerIdentity: undefined + }) + ], + projectHostSetups: [ + setup({ + id: 'acme-setup', + projectId: 'github:acme/merchant', + repoId: 'acme-repo', + path: '/workspace/acme/merchant' + }), + setup({ + id: 'merchant-local-setup', + projectId: 'project:merchant-local', + repoId: 'merchant-local-repo', + path: '/workspace/local/merchant' + }) + ], + eligibleRepos: [repo('acme-repo'), repo('merchant-local-repo')] + }) + + expect(options.map((option) => option.detail).sort()).toEqual([ + '/workspace/local/merchant', + 'acme/merchant' + ]) + }) + + it('adds host labels when duplicate project directory details collide', () => { + const options = buildNewWorkspaceProjectOptions({ + projects: [ + project({ + id: 'project:merchant-local', + displayName: 'merchant', + providerIdentity: undefined + }), + project({ + id: 'project:merchant-remote', + displayName: 'merchant', + providerIdentity: undefined + }) + ], + projectHostSetups: [ + setup({ + id: 'merchant-local-setup', + projectId: 'project:merchant-local', + hostId: 'local', + repoId: 'merchant-local-repo', + path: '/workspace/merchant' + }), + setup({ + id: 'merchant-remote-setup', + projectId: 'project:merchant-remote', + hostId: 'ssh:builder', + repoId: 'merchant-remote-repo', + path: '/workspace/merchant' + }) + ], + eligibleRepos: [repo('merchant-local-repo'), repo('merchant-remote-repo')], + hosts: [ + { id: 'local', label: 'Local Mac' }, + { id: 'ssh:builder', label: 'Builder' } + ] + }) + + expect(options.map((option) => option.detail).sort()).toEqual([ + 'Builder · /workspace/merchant', + 'Local Mac · /workspace/merchant' + ]) + }) + + it('adds host ids when duplicate project host labels still collide', () => { + const options = buildNewWorkspaceProjectOptions({ + projects: [ + project({ + id: 'project:merchant-builder-a', + displayName: 'merchant', + providerIdentity: undefined + }), + project({ + id: 'project:merchant-builder-b', + displayName: 'merchant', + providerIdentity: undefined + }) + ], + projectHostSetups: [ + setup({ + id: 'merchant-builder-a-setup', + projectId: 'project:merchant-builder-a', + hostId: 'ssh:builder-a', + repoId: 'merchant-builder-a-repo', + path: '/workspace/merchant' + }), + setup({ + id: 'merchant-builder-b-setup', + projectId: 'project:merchant-builder-b', + hostId: 'ssh:builder-b', + repoId: 'merchant-builder-b-repo', + path: '/workspace/merchant' + }) + ], + eligibleRepos: [repo('merchant-builder-a-repo'), repo('merchant-builder-b-repo')], + hosts: [ + { id: 'ssh:builder-a', label: 'Builder' }, + { id: 'ssh:builder-b', label: 'Builder' } + ] + }) + + expect(options.map((option) => option.detail).sort()).toEqual([ + 'Builder (ssh:builder-a) · /workspace/merchant', + 'Builder (ssh:builder-b) · /workspace/merchant' + ]) + }) + it('filters project options by display name and detail', () => { const options: NewWorkspaceProjectOption[] = [ { diff --git a/src/renderer/src/lib/new-workspace-project-options.ts b/src/renderer/src/lib/new-workspace-project-options.ts index cabbb4f4cf6..bfa47e2a63b 100644 --- a/src/renderer/src/lib/new-workspace-project-options.ts +++ b/src/renderer/src/lib/new-workspace-project-options.ts @@ -1,6 +1,11 @@ import { projectHostSetupProjectionFromRepos } from '../../../shared/project-host-setup-projection' import type { Project, ProjectGroup, ProjectHostSetup, Repo } from '../../../shared/types' import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text' +import type { ExecutionHostRegistryEntry } from '../../../shared/execution-host-registry' +import { + getDuplicateProjectDetailsById, + type ProjectSetupDirectory +} from './new-workspace-duplicate-project-details' export const NEW_WORKSPACE_PROJECT_GROUP_OPTION_PREFIX = 'project-group:' export const NEW_WORKSPACE_FOLDER_SOURCE_OPTION_PREFIX = 'folder-source:' @@ -45,6 +50,7 @@ type BuildNewWorkspaceProjectOptionsInput = { projects: readonly Project[] projectHostSetups: readonly ProjectHostSetup[] eligibleRepos: readonly Repo[] + hosts?: readonly Pick[] } type BuildNewWorkspaceCreateTargetOptionsInput = BuildNewWorkspaceProjectOptionsInput & { @@ -85,7 +91,9 @@ export function buildNewWorkspaceProjectOptions( const { eligibleRepos } = input const { projects, projectHostSetups } = getProjectModel(input) const eligibleRepoIds = new Set(eligibleRepos.map((repo) => repo.id)) + const hostLabelById = new Map((input.hosts ?? []).map((host) => [host.id, host.label])) const readySetupCountsByProjectId = new Map() + const setupDirectoriesByProjectId = new Map() for (const setup of projectHostSetups) { if (setup.setupState !== 'ready' || !eligibleRepoIds.has(setup.repoId)) { @@ -95,9 +103,12 @@ export function buildNewWorkspaceProjectOptions( setup.projectId, (readySetupCountsByProjectId.get(setup.projectId) ?? 0) + 1 ) + const directories = setupDirectoriesByProjectId.get(setup.projectId) ?? [] + directories.push({ path: setup.path, hostId: setup.hostId }) + setupDirectoriesByProjectId.set(setup.projectId, directories) } - return projects + const options = projects .filter((project) => (readySetupCountsByProjectId.get(project.id) ?? 0) > 0) .map((project) => ({ kind: 'project' as const, @@ -105,8 +116,21 @@ export function buildNewWorkspaceProjectOptions( projectId: project.id, displayName: project.displayName, badgeColor: project.badgeColor, - detail: getProjectDetail(project, readySetupCountsByProjectId.get(project.id) ?? 0) + detail: getProjectDetail(project, readySetupCountsByProjectId.get(project.id) ?? 0), + detailSource: project.providerIdentity ? ('provider' as const) : ('generic' as const) })) + + const duplicateProjectDetailsById = getDuplicateProjectDetailsById( + options, + setupDirectoriesByProjectId, + hostLabelById + ) + + return options + .map(({ detailSource: _detailSource, ...option }) => { + const directoryDetail = duplicateProjectDetailsById.get(option.id) + return directoryDetail ? { ...option, detail: directoryDetail } : option + }) .sort((a, b) => a.displayName.localeCompare(b.displayName) || a.detail.localeCompare(b.detail)) }