diff --git a/src/renderer/src/components/sidebar/project-group-header-dom.test.ts b/src/renderer/src/components/sidebar/project-group-header-dom.test.ts index bb3b6e06c85..2e7523e5ecb 100644 --- a/src/renderer/src/components/sidebar/project-group-header-dom.test.ts +++ b/src/renderer/src/components/sidebar/project-group-header-dom.test.ts @@ -30,7 +30,10 @@ describe('Project Group header drag DOM source', () => { const source = readHeaderDragSource() expect(source).toContain('const updateProjectGroup = useAppStore((s) => s.updateProjectGroup)') - expect(source).toContain('void updateProjectGroup(groupId, { tabOrder })') + // Why: the commit must carry the group's owner host so a non-focused host still persists the order. + expect(source).toContain( + 'void updateProjectGroup(groupId, { tabOrder }, { hostId: ownerHostId ?? undefined })' + ) }) it('keeps grab cursor on the title surface and dual handle attrs on row + surface', () => { diff --git a/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts b/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts index 80f608f8665..0127660c712 100644 --- a/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts +++ b/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts @@ -1,6 +1,7 @@ import { useCallback, useEffect, useMemo } from 'react' import type React from 'react' import { useAppStore } from '@/store' +import { getProjectGroupHostId } from '@/store/slices/project-group-owner-routing' import type { ProjectGroup } from '../../../../../../shared/project-group-types' import type { ProjectOrderBy } from '../../../../../../shared/ui-chrome-types' import type { Repo } from '../../../../../../shared/repo-types' @@ -83,6 +84,16 @@ export function useWorktreeSidebarHeaderDrag(args: { () => new Map(projectGroups.map((group) => [group.id, group])), [projectGroups] ) + // Why: an id shared by two hosts has no single owner, so leave it unrouted rather than guess. + const projectGroupOwnerHostIdByGroupId = useMemo(() => { + const byGroupId = new Map() + for (const group of projectGroups) { + const hostId = getProjectGroupHostId(group) + const existing = byGroupId.get(group.id) + byGroupId.set(group.id, existing === undefined || existing === hostId ? hostId : null) + } + return byGroupId + }, [projectGroups]) // Why: reorder keeps scrollTop stable; flag direct scroll input so anchor-restore won't chase the moved row (jumpy drop). const suppressScrollCorrectionForHeaderCommit = useCallback(() => { @@ -157,9 +168,11 @@ export function useWorktreeSidebarHeaderDrag(args: { return } suppressScrollCorrectionForHeaderCommit() - void updateProjectGroup(groupId, { tabOrder }) + // Why: manual order persists on the group's own host; the focused host may not hold this row. + const ownerHostId = projectGroupOwnerHostIdByGroupId.get(groupId) + void updateProjectGroup(groupId, { tabOrder }, { hostId: ownerHostId ?? undefined }) }, - [suppressScrollCorrectionForHeaderCommit, updateProjectGroup] + [projectGroupOwnerHostIdByGroupId, suppressScrollCorrectionForHeaderCommit, updateProjectGroup] ) // Drag applies only in manual order; still construct the controller inert for stable hook order. const repoDrag = useRepoHeaderDrag({ diff --git a/src/renderer/src/components/sidebar/worktree-list/rows/SectionHeader.tsx b/src/renderer/src/components/sidebar/worktree-list/rows/SectionHeader.tsx index 727438f997a..b4ed149712a 100644 --- a/src/renderer/src/components/sidebar/worktree-list/rows/SectionHeader.tsx +++ b/src/renderer/src/components/sidebar/worktree-list/rows/SectionHeader.tsx @@ -8,6 +8,8 @@ import { RepoForkIndicator } from '@/components/repo/repo-fork-indicator' import type { FolderWorkspacePathStatus } from '../../../../../../shared/folder-workspace-path-status' import { isConfirmedStaleFolderPathStatus } from '../../../../../../shared/folder-workspace-path-status' import type { ProjectGroup } from '../../../../../../shared/project-group-types' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' +import { getProjectGroupHostId } from '@/store/slices/project-group-owner-routing' import type { WorkspaceStatus, WorkspaceStatusDefinition @@ -56,8 +58,8 @@ export type SectionHeaderRowContext = { }) => FolderWorkspacePathStatus | null toggleGroupWithScrollAnchor: (groupKey: string) => void projectActions: RepoHeaderProjectActions - onRenameProjectGroup: (groupId: string, currentName: string) => void - onDeleteProjectGroup: (groupId: string, groupName: string) => void + onRenameProjectGroup: (groupId: string, currentName: string, hostId?: ExecutionHostId) => void + onDeleteProjectGroup: (groupId: string, groupName: string, hostId?: ExecutionHostId) => void onCreateFolderWorkspace: (projectGroup: ProjectGroup) => void onWorkspaceStatusDragOver: (event: React.DragEvent, status: WorkspaceStatus) => void onWorkspaceStatusDragLeave: (event: React.DragEvent) => void @@ -92,6 +94,11 @@ export function renderWorktreeSectionHeaderRow(args: { isProjectGroupHeader && !row.repo && typeof row.projectGroup?.id === 'string' ? row.projectGroup.id : undefined + // Why: rename/delete must route to the host that owns this row, not to whichever host has focus. + const projectGroupHostIdForHeader = + row.projectGroup && 'createdFrom' in row.projectGroup + ? getProjectGroupHostId(row.projectGroup) + : undefined const repoHeaderIndex = projectIdForHeader !== undefined ? headerDrag.repoHeaderIndexByRepoId.get(projectIdForHeader) @@ -353,6 +360,7 @@ export function renderWorktreeSectionHeaderRow(args: { {isProjectGroupHeader && !row.repo && projectGroupIdForHeader ? ( void - onDelete: (groupId: string, groupName: string) => void + onRename: (groupId: string, currentName: string, hostId?: ExecutionHostId) => void + onDelete: (groupId: string, groupName: string, hostId?: ExecutionHostId) => void }): React.JSX.Element { return ( @@ -64,10 +68,10 @@ export function ProjectGroupHeaderMenu({ onClick={stopRepoHeaderMenuEvent} onKeyDown={stopRepoHeaderMenuEvent} > - onRename(groupId, label)}> + onRename(groupId, label, hostId)}> {translate('auto.components.sidebar.WorktreeList.4d7b73658c', 'Rename group')} - onDelete(groupId, label)}> + onDelete(groupId, label, hostId)}> {translate('auto.components.sidebar.WorktreeList.902115cdbe', 'Delete group')} diff --git a/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs-owner-host.test.tsx b/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs-owner-host.test.tsx new file mode 100644 index 00000000000..399ee0021e2 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs-owner-host.test.tsx @@ -0,0 +1,135 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useProjectGroupDialogs, type ProjectGroupDialogs } from './use-project-group-dialogs' +import type { ProjectGroup } from '../../../../../../shared/project-group-types' + +const mocks = vi.hoisted(() => ({ + moveProjectToGroup: vi.fn(), + createProjectGroup: vi.fn(), + updateProjectGroup: vi.fn(), + deleteProjectGroupWithContainedProjects: vi.fn(), + toastError: vi.fn() +})) + +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ + moveProjectToGroup: mocks.moveProjectToGroup, + createProjectGroup: mocks.createProjectGroup, + updateProjectGroup: mocks.updateProjectGroup, + deleteProjectGroupWithContainedProjects: mocks.deleteProjectGroupWithContainedProjects + }) +})) + +vi.mock('sonner', () => ({ + toast: { error: mocks.toastError } +})) + +const remoteGroup: ProjectGroup = { + id: 'group-1', + name: 'Projects', + parentPath: '/srv/projects', + parentGroupId: null, + createdFrom: 'folder-scan', + executionHostId: 'runtime:env-1', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 +} + +let latest: ProjectGroupDialogs | null = null +const roots: Root[] = [] + +function HookProbe(): null { + latest = useProjectGroupDialogs({ + repos: [], + repoMap: new Map(), + projectGroups: [remoteGroup] + }) + return null +} + +async function renderHookProbe(): Promise { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + roots.push(root) + await act(async () => { + root.render() + }) +} + +beforeEach(() => { + vi.clearAllMocks() + mocks.updateProjectGroup.mockResolvedValue(true) + mocks.deleteProjectGroupWithContainedProjects.mockResolvedValue({ + status: 'deleted-group', + groupId: remoteGroup.id, + requestedProjectIds: [], + removedProjectIds: [], + failedProjectRemovals: [] + }) +}) + +afterEach(async () => { + await act(async () => { + for (const root of roots.splice(0)) { + root.unmount() + } + }) + latest = null +}) + +describe('project group dialogs carry the owner host', () => { + it('renames through the host that owns the group row', async () => { + await renderHookProbe() + await act(async () => { + latest!.handleRenameProjectGroup(remoteGroup.id, remoteGroup.name, 'runtime:env-1') + }) + await act(async () => { + await latest!.handleSubmitProjectGroupName('Renamed') + }) + + expect(mocks.updateProjectGroup).toHaveBeenCalledWith( + remoteGroup.id, + { name: 'Renamed' }, + { hostId: 'runtime:env-1' } + ) + expect(mocks.toastError).not.toHaveBeenCalled() + }) + + it('surfaces a toast when the owner host does not apply the rename', async () => { + mocks.updateProjectGroup.mockResolvedValue(false) + await renderHookProbe() + await act(async () => { + latest!.handleRenameProjectGroup(remoteGroup.id, remoteGroup.name, 'runtime:env-1') + }) + await act(async () => { + await latest!.handleSubmitProjectGroupName('Renamed') + }) + + expect(mocks.toastError).toHaveBeenCalledWith('Failed to rename group', { + description: "The group's host did not apply the new name." + }) + }) + + it('deletes through the host that owns the group row', async () => { + await renderHookProbe() + await act(async () => { + latest!.handleDeleteProjectGroup(remoteGroup.id, remoteGroup.name, 'runtime:env-1') + }) + await act(async () => { + await latest!.handleConfirmDeleteProjectGroup() + }) + + expect(mocks.deleteProjectGroupWithContainedProjects).toHaveBeenCalledWith(remoteGroup.id, { + removeContainedProjects: false, + hostId: 'runtime:env-1' + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs.ts b/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs.ts index b66eb737af5..f6c8f127bc3 100644 --- a/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs.ts +++ b/src/renderer/src/components/sidebar/worktree-list/rows/use-project-group-dialogs.ts @@ -5,15 +5,18 @@ import { translate } from '@/i18n/i18n' import { selectProjectGroupRemovalTargets } from '@/store/slices/project-group-removal-targets' import type { ProjectGroup } from '../../../../../../shared/project-group-types' import type { Repo } from '../../../../../../shared/repo-types' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' export type ProjectGroupNameDialogState = | { type: 'create-from-repo'; repo: Repo } - | { type: 'rename'; groupId: string; currentName: string } + // hostId is the group row's owner host, so the mutation is not routed to whichever host has focus. + | { type: 'rename'; groupId: string; currentName: string; hostId?: ExecutionHostId } export type ProjectGroupDeleteDialogState = { groupId: string groupName: string removeContainedProjects: boolean + hostId?: ExecutionHostId } export type ProjectGroupDialogs = ReturnType @@ -95,9 +98,12 @@ export function useProjectGroupDialogs(args: { [moveProjectToGroup] ) - const handleRenameProjectGroup = useCallback((groupId: string, currentName: string) => { - setNameDialog({ type: 'rename', groupId, currentName }) - }, []) + const handleRenameProjectGroup = useCallback( + (groupId: string, currentName: string, hostId?: ExecutionHostId) => { + setNameDialog({ type: 'rename', groupId, currentName, hostId }) + }, + [] + ) const handleSubmitProjectGroupName = useCallback( async (name: string) => { @@ -111,7 +117,25 @@ export function useProjectGroupDialogs(args: { } return } - await updateProjectGroup(nameDialog.groupId, { name }) + const renamed = await updateProjectGroup( + nameDialog.groupId, + { name }, + { hostId: nameDialog.hostId } + ) + if (!renamed) { + toast.error( + translate( + 'auto.components.sidebar.WorktreeList.groupRenameFailed', + 'Failed to rename group' + ), + { + description: translate( + 'auto.components.sidebar.WorktreeList.groupRenameFailedDesc', + "The group's host did not apply the new name." + ) + } + ) + } }, [createProjectGroup, moveProjectToGroup, nameDialog, updateProjectGroup] ) @@ -120,7 +144,12 @@ export function useProjectGroupDialogs(args: { if (!deleteDialog) { return null } - return selectProjectGroupRemovalTargets(projectGroups, repos, deleteDialog.groupId) + return selectProjectGroupRemovalTargets( + projectGroups, + repos, + deleteDialog.groupId, + deleteDialog.hostId + ) }, [deleteDialog, projectGroups, repos]) const deleteProjectCount = deleteTargets?.projectIds.length ?? 0 const deleteProjectNames = useMemo( @@ -133,9 +162,12 @@ export function useProjectGroupDialogs(args: { const removeContainedProjects = deleteProjectCount > 0 && deleteDialog?.removeContainedProjects === true - const handleDeleteProjectGroup = useCallback((groupId: string, groupName: string) => { - setDeleteDialog({ groupId, groupName, removeContainedProjects: false }) - }, []) + const handleDeleteProjectGroup = useCallback( + (groupId: string, groupName: string, hostId?: ExecutionHostId) => { + setDeleteDialog({ groupId, groupName, removeContainedProjects: false, hostId }) + }, + [] + ) const handleConfirmDeleteProjectGroup = useCallback(async () => { if (!deleteDialog) { @@ -144,7 +176,8 @@ export function useProjectGroupDialogs(args: { try { reportProjectGroupDeleteFailures( await deleteProjectGroupWithContainedProjects(deleteDialog.groupId, { - removeContainedProjects + removeContainedProjects, + hostId: deleteDialog.hostId }) ) } finally { 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 570077a1c26..e341f77695b 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 @@ -47,8 +47,8 @@ export type VirtualizedWorktreeViewportProps = { handleCreateGroupFromRepo: (repo: Repo) => void handleMoveProjectToGroup: (repo: Repo, groupId: string) => void handleRemoveProjectFromGroup: (repo: Repo) => void - handleRenameProjectGroup: (groupId: string, currentName: string) => void - handleDeleteProjectGroup: (groupId: string, groupName: string) => void + handleRenameProjectGroup: (groupId: string, currentName: string, hostId?: ExecutionHostId) => void + handleDeleteProjectGroup: (groupId: string, groupName: string, hostId?: ExecutionHostId) => void handleCreateFolderWorkspace: (projectGroup: ProjectGroup) => void activeModal: string pendingRevealWorktree: PendingSidebarWorktreeReveal | null diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 8e8cf2ab5e2..9ce1cced932 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5333,6 +5333,8 @@ "f94466bc39": "{{value0}} of {{value1}} contained project{{value2}} remained after deleting the group.", "groupDeleteFailed": "Failed to delete group", "groupDeleteFailedDesc": "Something went wrong while deleting the group. No projects were removed.", + "groupRenameFailed": "Failed to rename group", + "groupRenameFailedDesc": "The group's host did not apply the new name.", "failedNestWorkspace": "Failed to nest workspace", "sidebarRowMissing": "Target no longer exists", "failedUnnestWorkspace": "Failed to unnest workspace" diff --git a/src/renderer/src/store/slices/project-group-owner-routing.ts b/src/renderer/src/store/slices/project-group-owner-routing.ts new file mode 100644 index 00000000000..78da4385a53 --- /dev/null +++ b/src/renderer/src/store/slices/project-group-owner-routing.ts @@ -0,0 +1,94 @@ +import type { GlobalSettings } from '../../../../shared/global-settings-types' +import type { ProjectGroup } from '../../../../shared/project-group-types' +import { + LOCAL_EXECUTION_HOST_ID, + normalizeExecutionHostId, + parseExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../../../shared/execution-host' +import { findIndexedProjectGroupOwner } from '@/lib/worktree-runtime-owner-index' + +type ProjectGroupHostParts = Pick +type ProjectGroupOwnerRecord = Pick +type RoutingSettings = Pick | null | undefined + +type ProjectGroupOwnerRoutingState = { + projectGroups: readonly ProjectGroupOwnerRecord[] + settings: RoutingSettings +} + +// Why: persisted rows predate host stamping and may carry padded/unparseable ids; normalize so +// routing and catalog identity agree on the same owner host. +export function getProjectGroupHostId(group: ProjectGroupHostParts): ExecutionHostId { + const executionHostId = normalizeExecutionHostId(group.executionHostId) + if (executionHostId) { + return executionHostId + } + const connectionId = group.connectionId?.trim() + return connectionId ? toSshExecutionHostId(connectionId) : LOCAL_EXECUTION_HOST_ID +} + +export function catalogOwnsHost(catalogHostId: string, rowHostId: string): boolean { + if (catalogHostId !== LOCAL_EXECUTION_HOST_ID) { + return catalogHostId === rowHostId + } + return parseExecutionHostId(rowHostId)?.kind !== 'runtime' +} + +export function projectGroupMatchesOwnerHost( + group: ProjectGroupOwnerRecord, + groupId: string, + ownerHostId: ExecutionHostId | null +): boolean { + if (group.id !== groupId) { + return false + } + return ownerHostId ? catalogOwnsHost(ownerHostId, getProjectGroupHostId(group)) : true +} + +export function resolveProjectGroupOwnerHostId( + state: ProjectGroupOwnerRoutingState, + groupId: string, + hostId?: ExecutionHostId +): ExecutionHostId | null { + const owner = findIndexedProjectGroupOwner(state.projectGroups, groupId, hostId) + if (!owner) { + return null + } + if (hostId) { + return hostId + } + // Why: an unstamped row carries no owner, so keep the focused-host behavior instead of assuming local. + if (!owner.executionHostId && !owner.connectionId) { + return null + } + return getProjectGroupHostId(owner) +} + +// Why: the sidebar lists groups from every host, so mutations must route to the row's owner +// instead of whichever host currently has focus. Mirrors settingsForRepoOwner. +export function settingsForProjectGroupOwner( + state: ProjectGroupOwnerRoutingState, + groupId: string, + hostId?: ExecutionHostId +): RoutingSettings { + const ownerHostId = resolveProjectGroupOwnerHostId(state, groupId, hostId) + if (!ownerHostId) { + return state.settings + } + const parsed = parseExecutionHostId(ownerHostId) + if (parsed?.kind === 'runtime') { + return state.settings + ? { ...state.settings, activeRuntimeEnvironmentId: parsed.environmentId } + : { activeRuntimeEnvironmentId: parsed.environmentId } + } + // Why: direct-SSH groups live in the local main process catalog, so they route through window.api. + if ( + (parsed?.kind === 'local' || parsed?.kind === 'ssh') && + state.settings?.activeRuntimeEnvironmentId + ) { + return { ...state.settings, activeRuntimeEnvironmentId: null } + } + return state.settings +} diff --git a/src/renderer/src/store/slices/project-group-removal-targets.ts b/src/renderer/src/store/slices/project-group-removal-targets.ts index ee0c11c3c4b..667d74ceb1f 100644 --- a/src/renderer/src/store/slices/project-group-removal-targets.ts +++ b/src/renderer/src/store/slices/project-group-removal-targets.ts @@ -1,6 +1,8 @@ import type { ProjectGroup } from '../../../../shared/project-group-types' import type { Repo } from '../../../../shared/repo-types' import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' +import { catalogOwnsHost, getProjectGroupHostId } from './project-group-owner-routing' export type ProjectGroupRemovalTargets = { groupExists: boolean @@ -11,9 +13,16 @@ export type ProjectGroupRemovalTargets = { export function selectProjectGroupRemovalTargets( projectGroups: readonly ProjectGroup[], repos: readonly Repo[], - groupId: string + groupId: string, + // Why: ids repeat across hosts; without an owner host the caller would target another host's rows. + hostId?: ExecutionHostId | null ): ProjectGroupRemovalTargets { - const groupExists = projectGroups.some((group) => group.id === groupId) + const ownsRowHost = (rowHostId: string): boolean => + hostId ? catalogOwnsHost(hostId, rowHostId) : true + const ownerGroups = hostId + ? projectGroups.filter((group) => ownsRowHost(getProjectGroupHostId(group))) + : projectGroups + const groupExists = ownerGroups.some((group) => group.id === groupId) if (!groupExists) { return { groupExists: false, @@ -22,10 +31,14 @@ export function selectProjectGroupRemovalTargets( } } - const deletedGroupIds = getProjectGroupSubtreeIds(projectGroups, groupId) + const deletedGroupIds = getProjectGroupSubtreeIds(ownerGroups, groupId) const projectIds: string[] = [] for (const repo of repos) { - if (repo.projectGroupId && deletedGroupIds.has(repo.projectGroupId)) { + if ( + repo.projectGroupId && + deletedGroupIds.has(repo.projectGroupId) && + ownsRowHost(getRepoExecutionHostId(repo)) + ) { projectIds.push(repo.id) } } diff --git a/src/renderer/src/store/slices/repos-project-groups-owner-routing.test.ts b/src/renderer/src/store/slices/repos-project-groups-owner-routing.test.ts new file mode 100644 index 00000000000..e9f7d978c87 --- /dev/null +++ b/src/renderer/src/store/slices/repos-project-groups-owner-routing.test.ts @@ -0,0 +1,303 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { FolderWorkspace } from '../../../../shared/folder-workspace-types' +import type { ProjectGroup } from '../../../../shared/project-group-types' +import type { Repo } from '../../../../shared/repo-types' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '../../runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' +import { createTestStore } from './store-test-helpers' + +const folderScanGroup: ProjectGroup = { + id: 'group-1', + name: 'Projects', + parentPath: '/srv/projects', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 +} + +const baseRepo: Repo = { + id: 'repo', + path: '/repo', + displayName: 'Repo', + badgeColor: '#111', + addedAt: 1 +} + +const baseFolderWorkspace: FolderWorkspace = { + id: 'folder-workspace-1', + projectGroupId: folderScanGroup.id, + name: 'Notes', + folderPath: '/srv/projects/notes', + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 0, + createdAt: 1, + updatedAt: 1 +} + +const projectGroupsUpdate = vi.fn() +const projectGroupsDelete = vi.fn() +const runtimeEnvironmentCall = vi.fn() +const runtimeEnvironmentTransportCall = vi.fn() + +beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + vi.clearAllMocks() + projectGroupsUpdate.mockResolvedValue(null) + projectGroupsDelete.mockResolvedValue(false) + runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args) + }) + vi.stubGlobal('window', { + api: { + repos: { remove: vi.fn() }, + projectGroups: { update: projectGroupsUpdate, delete: projectGroupsDelete }, + runtimeEnvironments: { call: runtimeEnvironmentTransportCall } + } + }) +}) + +function runtimeRpcResponse(result: unknown) { + return { id: 'rpc-1', ok: true, result, _meta: { runtimeId: 'runtime-remote' } } +} + +describe('project group mutations route to the owning host', () => { + it('renames a runtime-owned folder-scan group while the local host is focused', async () => { + const runtimeGroup: ProjectGroup = { ...folderScanGroup, executionHostId: 'runtime:env-1' } + runtimeEnvironmentCall.mockResolvedValue( + runtimeRpcResponse({ group: { ...runtimeGroup, name: 'Renamed' } }) + ) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [runtimeGroup] + }) + + await expect( + store.getState().updateProjectGroup(runtimeGroup.id, { name: 'Renamed' }) + ).resolves.toBe(true) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'projectGroup.update', + params: { groupId: runtimeGroup.id, updates: { name: 'Renamed' } }, + timeoutMs: 15_000 + }) + expect(projectGroupsUpdate).not.toHaveBeenCalled() + expect(store.getState().projectGroups[0]).toMatchObject({ + name: 'Renamed', + executionHostId: 'runtime:env-1' + }) + }) + + it('deletes a local folder-scan group while a runtime host is focused', async () => { + projectGroupsDelete.mockResolvedValue(true) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + projectGroups: [{ ...folderScanGroup, executionHostId: 'local' }] + }) + + await expect(store.getState().deleteProjectGroup(folderScanGroup.id)).resolves.toBe(true) + + expect(projectGroupsDelete).toHaveBeenCalledWith({ groupId: folderScanGroup.id }) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + expect(store.getState().projectGroups).toEqual([]) + }) + + it('routes a direct-SSH group through the local catalog, not the focused runtime', async () => { + projectGroupsDelete.mockResolvedValue(true) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + projectGroups: [{ ...folderScanGroup, connectionId: 'conn-1' }] + }) + + await expect(store.getState().deleteProjectGroup(folderScanGroup.id)).resolves.toBe(true) + + expect(projectGroupsDelete).toHaveBeenCalledWith({ groupId: folderScanGroup.id }) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + + it('prefers an explicit hostId over both the focused host and a colliding row', async () => { + runtimeEnvironmentCall.mockResolvedValue(runtimeRpcResponse({ deleted: true })) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [ + { ...folderScanGroup, executionHostId: 'local' }, + { ...folderScanGroup, executionHostId: 'runtime:env-1' } + ] + }) + + await expect( + store.getState().deleteProjectGroup(folderScanGroup.id, { hostId: 'runtime:env-1' }) + ).resolves.toBe(true) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'projectGroup.delete', + params: { groupId: folderScanGroup.id }, + timeoutMs: 15_000 + }) + expect(projectGroupsDelete).not.toHaveBeenCalled() + expect(store.getState().projectGroups).toMatchObject([{ executionHostId: 'local' }]) + }) + + it('keeps the focused host when a colliding id has no unambiguous owner', async () => { + projectGroupsDelete.mockResolvedValue(true) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [ + { ...folderScanGroup, executionHostId: 'local' }, + { ...folderScanGroup, executionHostId: 'runtime:env-1' } + ] + }) + + await expect(store.getState().deleteProjectGroup(folderScanGroup.id)).resolves.toBe(true) + + expect(projectGroupsDelete).toHaveBeenCalledWith({ groupId: folderScanGroup.id }) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) +}) + +describe('project group state cascades stay scoped to the owner host', () => { + it('leaves another host rows intact when deleting a colliding local group', async () => { + projectGroupsDelete.mockResolvedValue(true) + const runtimeChild: ProjectGroup = { + ...folderScanGroup, + id: 'child', + parentGroupId: folderScanGroup.id, + executionHostId: 'runtime:env-1' + } + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [ + { ...folderScanGroup, executionHostId: 'local' }, + { ...folderScanGroup, name: 'Remote projects', executionHostId: 'runtime:env-1' }, + runtimeChild + ], + folderWorkspaces: [ + { ...baseFolderWorkspace, executionHostId: 'local' }, + { + ...baseFolderWorkspace, + id: 'folder-workspace-remote', + executionHostId: 'runtime:env-1' + } + ], + repos: [ + { ...baseRepo, id: 'local-repo', projectGroupId: folderScanGroup.id }, + { + ...baseRepo, + id: 'remote-repo', + projectGroupId: folderScanGroup.id, + executionHostId: 'runtime:env-1' + } + ] + }) + + await expect( + store.getState().deleteProjectGroup(folderScanGroup.id, { hostId: 'local' }) + ).resolves.toBe(true) + + expect(store.getState().projectGroups).toMatchObject([ + { id: folderScanGroup.id, executionHostId: 'runtime:env-1' }, + { id: 'child', executionHostId: 'runtime:env-1' } + ]) + expect(store.getState().folderWorkspaces.map((workspace) => workspace.id)).toEqual([ + 'folder-workspace-remote' + ]) + expect(store.getState().repos).toMatchObject([ + { id: 'local-repo', projectGroupId: null }, + { id: 'remote-repo', projectGroupId: folderScanGroup.id } + ]) + }) + + it('renames only the owner host row when the id exists on two hosts', async () => { + projectGroupsUpdate.mockResolvedValue({ + ...folderScanGroup, + executionHostId: 'local', + name: 'Renamed' + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [ + { ...folderScanGroup, executionHostId: 'local' }, + { ...folderScanGroup, name: 'Remote projects', executionHostId: 'runtime:env-1' } + ] + }) + + await expect( + store.getState().updateProjectGroup( + folderScanGroup.id, + { name: 'Renamed' }, + { + hostId: 'local' + } + ) + ).resolves.toBe(true) + + expect(store.getState().projectGroups).toMatchObject([ + { name: 'Renamed', executionHostId: 'local' }, + { name: 'Remote projects', executionHostId: 'runtime:env-1' } + ]) + }) + + it('removes contained projects only from the owner host', async () => { + projectGroupsDelete.mockResolvedValue(true) + const reposRemove = vi.fn().mockResolvedValue(undefined) + const reposRemoveForHost = vi.fn().mockResolvedValue(undefined) + vi.stubGlobal('window', { + api: { + repos: { remove: reposRemove, removeForHost: reposRemoveForHost }, + projectGroups: { update: projectGroupsUpdate, delete: projectGroupsDelete }, + runtimeEnvironments: { call: runtimeEnvironmentTransportCall } + } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [ + { ...folderScanGroup, executionHostId: 'local' }, + { ...folderScanGroup, executionHostId: 'runtime:env-1' } + ], + repos: [ + { ...baseRepo, id: 'local-repo', projectGroupId: folderScanGroup.id }, + { + ...baseRepo, + id: 'remote-repo', + projectGroupId: folderScanGroup.id, + executionHostId: 'runtime:env-1' + } + ] + }) + + const result = await store + .getState() + .deleteProjectGroupWithContainedProjects(folderScanGroup.id, { + removeContainedProjects: true, + hostId: 'local' + }) + + expect(result).toMatchObject({ + status: 'deleted-group', + requestedProjectIds: ['local-repo'], + removedProjectIds: ['local-repo'] + }) + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['remote-repo']) + }) +}) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 2a802bc57b2..f0fd6fcbae8 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -52,6 +52,13 @@ import { isPathInsideOrEqual } from '../../../../shared/cross-platform-path' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' import { structuralValuesEqual } from '../../../../shared/structural-value-equality' import { selectProjectGroupRemovalTargets } from './project-group-removal-targets' +import { + catalogOwnsHost, + getProjectGroupHostId, + projectGroupMatchesOwnerHost, + resolveProjectGroupOwnerHostId, + settingsForProjectGroupOwner +} from './project-group-owner-routing' import { reconcileCatalogRows, reconcileFetchedRepos } from './repo-identity-reconcile' import { retainValidFilterRepoIds } from './repo-filter-selection' import { readRuntimeWorktreeVisibilitySnapshot } from './worktree-visibility-owner-settings' @@ -218,6 +225,8 @@ export type FolderWorkspacePathStatusCacheEntry = { export type DeleteProjectGroupWithContainedProjectsOptions = { removeContainedProjects: boolean + // hostId disambiguates which host's group row to delete when the id exists on multiple hosts. + hostId?: ExecutionHostId } type AllHostCatalogFetchOptions = { @@ -1120,24 +1129,10 @@ function mergeProjectCompatibilityForHostRepoChange({ }) } -function getProjectGroupHostId(group: Pick) { - if (group.executionHostId) { - return group.executionHostId - } - return group.connectionId ? toSshExecutionHostId(group.connectionId) : LOCAL_EXECUTION_HOST_ID -} - function getProjectGroupHostIdentity(group: ProjectGroup): string { return JSON.stringify([getProjectGroupHostId(group), group.id]) } -function catalogOwnsHost(catalogHostId: string, rowHostId: string): boolean { - if (catalogHostId !== LOCAL_EXECUTION_HOST_ID) { - return catalogHostId === rowHostId - } - return parseExecutionHostId(rowHostId)?.kind !== 'runtime' -} - function mergeFetchedProjectGroupsForHost( previous: readonly ProjectGroup[], fetched: ProjectGroup[], @@ -1186,6 +1181,37 @@ function getFolderWorkspaceHostIdentity( return JSON.stringify([getFolderWorkspaceHostId(workspace, projectGroups), workspace.id]) } +// Why: a group id can exist on several hosts; only the deleted owner's rows may be cascaded away. +function applyProjectGroupDeleteCascade( + s: Pick, + groupId: string, + ownerHostId: ExecutionHostId | null +): Pick { + const ownsRowHost = (rowHostId: string): boolean => + ownerHostId ? catalogOwnsHost(ownerHostId, rowHostId) : true + const ownerGroups = s.projectGroups.filter((group) => ownsRowHost(getProjectGroupHostId(group))) + const deletedGroupIds = getProjectGroupSubtreeIds(ownerGroups, groupId) + const isDeletedGroup = (group: ProjectGroup): boolean => + deletedGroupIds.has(group.id) && ownsRowHost(getProjectGroupHostId(group)) + return { + projectGroups: s.projectGroups.filter((group) => !isDeletedGroup(group)), + folderWorkspaces: s.folderWorkspaces.filter( + (workspace) => + !deletedGroupIds.has(workspace.projectGroupId) || + // Why: resolve the workspace's host against the pre-delete group list, which still holds its owner row. + !ownsRowHost(getFolderWorkspaceHostId(workspace, s.projectGroups)) + ), + repos: s.repos.map((repo) => + repo.projectGroupId && + deletedGroupIds.has(repo.projectGroupId) && + ownsRowHost(getRepoExecutionHostId(repo)) + ? { ...repo, projectGroupId: null } + : repo + ), + folderWorkspacePathStatuses: {} + } +} + function getFolderWorkspaceUpdateIdentity( hostId: ExecutionHostId, folderWorkspaceId: string @@ -1883,11 +1909,13 @@ export type RepoSlice = { options?: { executionHostId?: ExecutionHostId } ) => Promise deleteFolderWorkspace: (folderWorkspaceId: string) => Promise + // options.hostId targets a specific host's row + RPC target when the id exists on multiple hosts; else the group's own host owns the call. updateProjectGroup: ( groupId: string, - updates: Partial> + updates: Partial>, + options?: { hostId?: ExecutionHostId } ) => Promise - deleteProjectGroup: (groupId: string) => Promise + deleteProjectGroup: (groupId: string, options?: { hostId?: ExecutionHostId }) => Promise deleteProjectGroupWithContainedProjects: ( groupId: string, options: DeleteProjectGroupWithContainedProjectsOptions @@ -2990,10 +3018,13 @@ export const createRepoSlice: StateCreator = (set, } }, - updateProjectGroup: async (groupId, updates) => { + updateProjectGroup: async (groupId, updates, options) => { try { - // Why: project groups are focused-host-scoped by design; all CRUD routes by the focused host and the list is replaced, not merged. - const target = getActiveRuntimeTarget(get().settings) + // Why: the sidebar lists groups from every host, so the mutation follows the group's owner, not the focused host. + const ownerHostId = resolveProjectGroupOwnerHostId(get(), groupId, options?.hostId) + const target = getActiveRuntimeTarget( + settingsForProjectGroupOwner(get(), groupId, options?.hostId) + ) const updated = target.kind === 'local' ? await window.api.projectGroups.update({ groupId, updates }) @@ -3010,7 +3041,9 @@ export const createRepoSlice: StateCreator = (set, } const ownedGroup = projectGroupWithFetchedOwner(updated, target) set((s) => ({ - projectGroups: s.projectGroups.map((group) => (group.id === groupId ? ownedGroup : group)), + projectGroups: s.projectGroups.map((group) => + projectGroupMatchesOwnerHost(group, groupId, ownerHostId) ? ownedGroup : group + ), folderWorkspacePathStatuses: {} })) return true @@ -3020,10 +3053,13 @@ export const createRepoSlice: StateCreator = (set, } }, - deleteProjectGroup: async (groupId) => { + deleteProjectGroup: async (groupId, options) => { try { - // Why: project groups are focused-host-scoped by design (see updateProjectGroup). - const target = getActiveRuntimeTarget(get().settings) + // Why: deletion targets the group's owner host (see updateProjectGroup); focus may be elsewhere. + const ownerHostId = resolveProjectGroupOwnerHostId(get(), groupId, options?.hostId) + const target = getActiveRuntimeTarget( + settingsForProjectGroupOwner(get(), groupId, options?.hostId) + ) const deleted = target.kind === 'local' ? await window.api.projectGroups.delete({ groupId }) @@ -3038,21 +3074,7 @@ export const createRepoSlice: StateCreator = (set, if (!deleted) { return false } - set((s) => { - const deletedGroupIds = getProjectGroupSubtreeIds(s.projectGroups, groupId) - return { - projectGroups: s.projectGroups.filter((group) => !deletedGroupIds.has(group.id)), - folderWorkspaces: s.folderWorkspaces.filter( - (workspace) => !deletedGroupIds.has(workspace.projectGroupId) - ), - repos: s.repos.map((repo) => - repo.projectGroupId && deletedGroupIds.has(repo.projectGroupId) - ? { ...repo, projectGroupId: null } - : repo - ), - folderWorkspacePathStatuses: {} - } - }) + set((s) => applyProjectGroupDeleteCascade(s, groupId, ownerHostId)) return true } catch (err) { console.error('Failed to delete project group:', err) @@ -3061,7 +3083,13 @@ export const createRepoSlice: StateCreator = (set, }, deleteProjectGroupWithContainedProjects: async (groupId, options) => { - const targets = selectProjectGroupRemovalTargets(get().projectGroups, get().repos, groupId) + const ownerHostId = resolveProjectGroupOwnerHostId(get(), groupId, options.hostId) + const targets = selectProjectGroupRemovalTargets( + get().projectGroups, + get().repos, + groupId, + ownerHostId + ) const requestedProjectIds = options.removeContainedProjects ? targets.projectIds : [] if (!targets.groupExists) { return { @@ -3073,7 +3101,9 @@ export const createRepoSlice: StateCreator = (set, } } - const deleted = await get().deleteProjectGroup(groupId) + const deleted = await get().deleteProjectGroup(groupId, { + hostId: ownerHostId ?? undefined + }) if (!deleted) { return { status: 'group-delete-failed', @@ -3096,16 +3126,26 @@ export const createRepoSlice: StateCreator = (set, const removedProjectIds: string[] = [] const failedProjectRemovals: ProjectRemovalFailure[] = [] + // Why: the group's catalog can hold rows from several hosts (a local catalog also owns SSH rows), + // so each project is removed on its own host rather than on the group's. + const findOwnedProjects = (projectId: string): Repo[] => + get().repos.filter( + (repo) => + repo.id === projectId && + (!ownerHostId || catalogOwnsHost(ownerHostId, getRepoExecutionHostId(repo))) + ) for (const projectId of targets.projectIds) { - const existedBeforeRemoval = get().repos.some((repo) => repo.id === projectId) + const ownedProjects = findOwnedProjects(projectId) + const projectHostId = + ownedProjects.length === 1 ? getRepoExecutionHostId(ownedProjects[0]) : undefined try { - if (existedBeforeRemoval) { - await get().removeProject(projectId) + if (ownedProjects.length > 0) { + await get().removeProject(projectId, { hostId: projectHostId }) } } catch (err) { console.error('Failed to remove contained project:', err) } - const stillExists = get().repos.some((repo) => repo.id === projectId) + const stillExists = findOwnedProjects(projectId).length > 0 if (stillExists) { failedProjectRemovals.push({ projectId,