From 253f8c3f9e48dfec505d7ec96e104f099cf365a1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:30:33 -0700 Subject: [PATCH] fix(sidebar): narrow owner-qualify same-id group identity Disambiguate multi-host project groups and folder workspaces only at catalog normalize, store CRUD, sidebar row keys, activation dual-read, IPC, persistence, and filesystem-auth. Keep bare IDs when unambiguous (including folder:folder-workspace-1); fail closed on collision. Leave #13611 sticky geometry untouched and drop broad WorktreeCard/dashboard session-routing propagation from the prior 66-file tip. --- issue-12532-reduced-retention-table.md | 52 ++ src/main/ipc/filesystem-auth.test.ts | 84 +++ src/main/ipc/filesystem-auth.ts | 240 +++++++-- src/main/ipc/repos-remote.test.ts | 62 +++ src/main/ipc/repos.ts | 77 ++- src/main/persistence.test.ts | 343 ++++++++++++ src/main/persistence.ts | 186 +++++-- .../folder-workspace-path-status.test.ts | 30 +- .../folder-workspace-path-status.ts | 48 +- src/preload/api-types.ts | 4 +- .../project-group-sidebar-identity.test.ts | 47 ++ .../sidebar/project-group-sidebar-identity.ts | 151 ++++++ .../sidebar/worktree-list-groups.ts | 241 +++++++-- .../project-group-removal-targets.test.ts | 90 +++- .../slices/project-group-removal-targets.ts | 124 ++++- .../repos-project-group-owner-update.test.ts | 124 +++++ ...pos-project-group-path-status-perf.test.ts | 58 ++ .../repos-project-groups-delete.test.ts | 144 ++++- ...-same-id-folder-workspace-identity.test.ts | 237 ++++++++ src/renderer/src/store/slices/repos.ts | 508 +++++++++++++++--- src/renderer/src/store/slices/worktrees.ts | 205 ++++++- src/shared/folder-workspace-path-status.ts | 6 +- src/shared/folder-workspaces.ts | 111 +++- src/shared/project-groups.test.ts | 118 +++- src/shared/project-groups.ts | 171 +++++- src/shared/types.ts | 2 +- src/shared/workspace-scope.test.ts | 87 +++ src/shared/workspace-scope.ts | 102 +++- 28 files changed, 3336 insertions(+), 316 deletions(-) create mode 100644 issue-12532-reduced-retention-table.md create mode 100644 src/renderer/src/components/sidebar/project-group-sidebar-identity.test.ts create mode 100644 src/renderer/src/components/sidebar/project-group-sidebar-identity.ts create mode 100644 src/renderer/src/store/slices/repos-project-group-owner-update.test.ts create mode 100644 src/renderer/src/store/slices/repos-project-group-path-status-perf.test.ts create mode 100644 src/renderer/src/store/slices/repos-same-id-folder-workspace-identity.test.ts create mode 100644 src/shared/workspace-scope.test.ts diff --git a/issue-12532-reduced-retention-table.md b/issue-12532-reduced-retention-table.md new file mode 100644 index 00000000000..415131d0aa4 --- /dev/null +++ b/issue-12532-reduced-retention-table.md @@ -0,0 +1,52 @@ +# #12532 reduced candidate — retained production file table + +**Base:** `d0271cb46f` (#13611 geometry locked) +**Archived broad tip:** `7c64936ce7` / docs `39d2585a90` on `archive/issue-12532-broad-66file-7c64936-20260810-122250` +**Hard gate:** ≤45 total / ≤28 production + +## Retention policy + +- Bare IDs remain the default when the catalog has a **single** owner for that bare id (legacy `folder:folder-workspace-1` unchanged). +- Owner-qualified refs are used only when **same bare id appears on multiple owners**, or when the caller already supplies `ownerHostId`. +- Ambiguous bare mutate/delete/auth **fails closed**. +- No global rewrite of worktree IDs, tabs, palette, chat, dashboard cards, WorktreeCard/ContextMenu, or #13611 geometry. + +## Retained production files + +| # | File | Failing behavior without it | Why a narrower seam cannot absorb it | +| --- | --- | --- | --- | +| 1 | `src/shared/project-groups.ts` | Same bare `group.id` on local+SSH collapses into one row on normalize; subtree delete/update walks bare parent edges across hosts | Canonical owner identity + fail-closed `resolveProjectGroupOwner` must live in shared code used by store, IPC, auth | +| 2 | `src/shared/folder-workspaces.ts` | Same bare folder id dropped on normalize; row keys collide in multi-host catalogs | Catalog identity for folders is parallel to groups; must stay co-located with normalize | +| 3 | `src/shared/workspace-scope.ts` | No dual-read for `folder:folder-workspace-1` vs optional owner-qualified form; parse cannot recover owner segment | Session key codec is shared main↔renderer; dual-read helpers belong at the codec, not duplicated | +| 4 | `src/shared/types.ts` | `WorkspaceScope` cannot carry optional `ownerHostId` from parse | One-line additive field; no alternative without a parallel type | +| 5 | `src/shared/folder-workspace-path-status.ts` | Path-status requests cannot name owner → cross-owner cache/RPC mix | Request type is the IPC/renderer contract boundary | +| 6 | `src/renderer/src/store/slices/repos.ts` | `updateProjectGroup`/`deleteProjectGroup` map by bare id and replace/delete both hosts; folder delete purges wrong session; path-status routes by focus | Single store owns CRUD orchestration; IPC alone cannot fix optimistic local state | +| 7 | `src/renderer/src/store/slices/project-group-removal-targets.ts` | Contained-project removal enumerates bare subtree across hosts | Delete-with-projects path must use owner-scoped subtree before IPC | +| 8 | `src/main/ipc/repos.ts` | Main IPC drops optional `ownerHostId` → host store cannot disambiguate | Wire entry for project-group/folder selectors | +| 9 | `src/preload/api-types.ts` | Renderer types omit optional `ownerHostId` | Type contract for preload; additive only | +| 10 | `src/main/persistence.ts` | Host-local group update/delete by bare id mutates wrong row; session maps need dual-read/migrate for bare folder keys | Persistence is the durable store for groups/folders/sessions on this host | +| 11 | `src/main/ipc/filesystem-auth.ts` | Authorize uses bare group/folder membership → same-id remote folder can authorize local path (or fail open) | Security boundary must index by owner once per rebuild (linear) | +| 12 | `src/main/project-groups/folder-workspace-path-status.ts` | Path status resolves bare folder/group to wrong connection | Main path probe must fail-closed on ambiguous owner | +| 13 | `src/renderer/src/components/sidebar/worktree-list-groups.ts` | Same-id groups share collapse key `project-group:`; folder rows share list keys | Row model is where collapse/render keys are minted; virtualizer host stamp alone does not fix collapse Set | +| 14 | `src/renderer/src/components/sidebar/project-group-sidebar-identity.ts` | Call sites reimplement owner resolve inconsistently | Thin shared index for sidebar owner lookup (kept small; avoids bloating WorktreeList) | +| 15 | `src/renderer/src/store/slices/worktrees.ts` | `setActiveFolderWorkspace` always used bare `folder:` so same-id hosts share session maps; no dual-read migrate of `folder:folder-workspace-1` | Activation/session map ownership lives in worktrees slice; cannot dual-read tabs without this seam | + +**Production count: 15** (≤28). WorktreeList left on base: collapse keys come from `worktree-list-groups`; store fail-closes bare ambiguous mutate; optional `ownerHostId` on APIs covers explicit callers/tests. + +## Explicitly dropped vs broad 38-file tip (and why) + +| Dropped | Reason | +| --- | --- | +| WorktreeCard / WorktreeContextMenu | Store fail-closed + optional owner covers delete; card can keep bare id when unambiguous | +| dashboard-snapshot-workspaces | Dual-read in session codec/store activation; dashboard not required for issue #12532 same-id group headers | +| worktrees.ts bulk dual-read | Prefer bare session keys when unambiguous; avoid global session routing rewrite | +| folder-workspace-session-owner, workspace-session-host-persistence, worktree-runtime-owner | Cascade from always-qualified keys; not needed if default stays bare | +| folder-workspace-worktree | Keep bare synthetic `folder:` when unambiguous | +| folder-workspace-connection, composer helpers/path-status | Not required for adversarial same-id group/folder CRUD | +| project-group-header-drag/drop/contract, move-targets, header-section-boundaries, folder-reveal, host-filtering, rendered-order, virtual-rows | Geometry/drag cascade; #13611 host stamp already unique for virtualizer; drag can follow store owner later if proven | +| ui.ts draft owner field | Optional convenience; not required for fail-closed CRUD | +| repos-runtime-routing-fixture | Test-only support (count under tests if reintroduced) | + +## Test budget (focused only) + +Same-id / fail-closed / dual-read / auth-index / path-status / owner update-delete / normalize — prefer **≤20** test files, no mass expectation rewrites of sticky geometry or bare-key consumers. diff --git a/src/main/ipc/filesystem-auth.test.ts b/src/main/ipc/filesystem-auth.test.ts index c6fa78b126d..6d8731bd3de 100644 --- a/src/main/ipc/filesystem-auth.test.ts +++ b/src/main/ipc/filesystem-auth.test.ts @@ -264,6 +264,90 @@ describe('filesystem-auth path containment', () => { } }) + it('authorizes only the local same-id folder owner', async () => { + const tempRoot = await mkdtemp(join(tmpdir(), 'orca-auth-same-id-folders-')) + try { + const localPath = join(tempRoot, 'local') + const sshPath = join(tempRoot, 'ssh') + await mkdir(localPath) + await mkdir(sshPath) + const localGroup = makeProjectGroup({ id: 'same-id', parentPath: localPath }) + const sshGroup = makeProjectGroup({ + id: 'same-id', + parentPath: sshPath, + connectionId: 'ssh-1' + }) + const store = makeStore([], { + projectGroups: [localGroup, sshGroup], + folderWorkspaces: [ + makeFolderWorkspace({ + id: 'local-folder', + projectGroupId: 'same-id', + folderPath: localPath, + connectionId: null + }), + makeFolderWorkspace({ + id: 'ssh-folder', + projectGroupId: 'same-id', + folderPath: sshPath, + connectionId: 'ssh-1' + }) + ] + }) + + await expect(resolveAuthorizedPath(localPath, store)).resolves.toBe(await realpath(localPath)) + await expect(resolveAuthorizedPath(sshPath, store)).rejects.toThrow('Access denied') + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) + + it('keeps large multi-owner folder authorization linear in catalog size', () => { + const groupCount = 2_048 + const repoCount = 2_048 + let pathReads = 0 + const projectGroups = Array.from({ length: groupCount }, (_, index) => + makeProjectGroup({ + id: index < 16 ? 'same-id' : `group-${index}`, + parentPath: index % 16 === 0 ? `/local/group-${index}` : null, + executionHostId: index % 16 === 0 ? 'local' : `runtime:env-${index % 16}` + }) + ) + const folderWorkspaces = Array.from({ length: groupCount }, (_, index) => + makeFolderWorkspace({ + id: index < 16 ? 'same-folder' : `folder-${index}`, + projectGroupId: index < 16 ? 'same-id' : `group-${index}`, + folderPath: `/folder-${index}`, + executionHostId: index % 16 === 0 ? 'local' : `runtime:env-${index % 16}`, + connectionId: index % 16 === 0 ? null : undefined + }) + ) + const repos = Array.from({ length: repoCount }, (_, index) => { + const ownerIndex = index % 16 + return { + ...repo, + id: `repo-${index}`, + get path() { + pathReads += 1 + return `/repo-${index}` + }, + projectGroupId: ownerIndex === 0 ? 'same-id' : `group-${index}`, + executionHostId: ownerIndex === 0 ? 'local' : `runtime:env-${ownerIndex}`, + connectionId: ownerIndex === 0 ? null : undefined + } as Repo + }) + const store = makeStore(repos, { projectGroups, folderWorkspaces }) + + const started = performance.now() + const allowed = isPathAllowed('/local/group-0', store) + const elapsedMs = performance.now() - started + + expect(allowed).toBe(true) + // Why: indexed auth must not re-scan every path for every group/folder on each authorize. + expect(pathReads).toBeLessThan(repoCount * 4) + expect(elapsedMs).toBeLessThan(250) + }) + it.skipIf(process.platform === 'win32')( 'rejects missing descendants under a symlinked ancestor outside the repo', async () => { diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index b172c2c1894..d8cc22afbf0 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -6,8 +6,15 @@ import type { Store } from '../persistence' import { isRepoRoot, listRepoWorktrees } from '../repo-worktrees' import { computeWorkspaceRoot, getWorktreePathSettings } from './worktree-logic' import { isPathInsideOrEqual } from '../../shared/cross-platform-path' -import { getProjectGroupSubtreeIds } from '../../shared/project-groups' -import type { FolderWorkspace, ProjectGroup, Repo } from '../../shared/types' +import { + buildProjectGroupOwnerIndex, + getProjectGroupOwnerHostId, + getProjectGroupSubtreeIds, + resolveFolderWorkspaceProjectGroup +} from '../../shared/project-groups' +import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' +import type { ProjectGroup, Repo } from '../../shared/types' +import { getFolderWorkspaceCatalogOwnerHostId } from '../../shared/folder-workspaces' export const PATH_ACCESS_DENIED_MESSAGE = 'Access denied: path resolves outside allowed directories. If this blocks a legitimate workflow, please file a GitHub issue.' @@ -58,43 +65,189 @@ function getLocalRepos(store: Store) { return store.getRepos().filter((repo) => !repo.connectionId) } -function getFolderScopeCandidateRepos( - folderPath: string, - projectGroupId: string, - projectGroups: readonly ProjectGroup[], +type FolderAuthIndex = { + projectGroupIndex: ReturnType + ownerGroupIdsByOwner: Map + reposByOwner: Map + reposByOwnerAndGroupId: Map + ownerSubtreeIds: Map>> + unscopedSubtreeIds: Map> repos: readonly Repo[] -): Repo[] { - const groupIds = getProjectGroupSubtreeIds(projectGroups, projectGroupId) - return repos.filter( - (repo) => - (typeof repo.projectGroupId === 'string' && groupIds.has(repo.projectGroupId)) || - isPathInsideOrEqual(folderPath, repo.path) - ) + projectGroups: readonly ProjectGroup[] } -function isRemoteOnlyFolderScope( - folderPath: string, - projectGroupId: string, - connectionId: string | null | undefined, +function ownerGroupKey(ownerHostId: string, groupId: string): string { + return `${ownerHostId}\0${groupId}` +} + +function buildFolderAuthIndex( projectGroups: readonly ProjectGroup[], repos: readonly Repo[] +): FolderAuthIndex { + const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups) + const ownerGroupIdsByOwner = new Map() + const childrenByOwnerParent = new Map() + for (const group of projectGroups) { + const ownerHostId = getProjectGroupOwnerHostId(group) + const ownerGroups = ownerGroupIdsByOwner.get(ownerHostId) ?? [] + ownerGroups.push(group) + ownerGroupIdsByOwner.set(ownerHostId, ownerGroups) + if (!group.parentGroupId) { + continue + } + const parentKey = ownerGroupKey(ownerHostId, group.parentGroupId) + const children = childrenByOwnerParent.get(parentKey) ?? [] + children.push(group.id) + childrenByOwnerParent.set(parentKey, children) + } + + const ownerSubtreeIds = new Map>>() + for (const [ownerHostId, groups] of ownerGroupIdsByOwner) { + const subtreeByRoot = new Map>() + for (const group of groups) { + if (subtreeByRoot.has(group.id)) { + continue + } + const subtree = new Set() + const pending = [group.id] + while (pending.length > 0) { + const groupId = pending.pop()! + if (subtree.has(groupId)) { + continue + } + subtree.add(groupId) + for (const childId of childrenByOwnerParent.get(ownerGroupKey(ownerHostId, groupId)) ?? + []) { + pending.push(childId) + } + } + subtreeByRoot.set(group.id, subtree) + } + ownerSubtreeIds.set(ownerHostId, subtreeByRoot) + } + + const unscopedSubtreeIds = new Map>() + for (const group of projectGroups) { + if (!unscopedSubtreeIds.has(group.id)) { + unscopedSubtreeIds.set(group.id, getProjectGroupSubtreeIds(projectGroups, group.id)) + } + } + + const reposByOwner = new Map() + const reposByOwnerAndGroupId = new Map() + for (const repo of repos) { + const ownerHostId = getRepoExecutionHostId(repo) + const ownerRepos = reposByOwner.get(ownerHostId) ?? [] + ownerRepos.push(repo) + reposByOwner.set(ownerHostId, ownerRepos) + if (typeof repo.projectGroupId === 'string') { + const key = ownerGroupKey(ownerHostId, repo.projectGroupId) + const groupRepos = reposByOwnerAndGroupId.get(key) ?? [] + groupRepos.push(repo) + reposByOwnerAndGroupId.set(key, groupRepos) + } + } + + return { + projectGroupIndex, + ownerGroupIdsByOwner, + reposByOwner, + reposByOwnerAndGroupId, + ownerSubtreeIds, + unscopedSubtreeIds, + repos, + projectGroups + } +} + +function collectOwnerGroupMembershipRepos( + index: FolderAuthIndex, + projectGroupId: string, + ownerHostId: string +): Repo[] { + const groupIds = + index.ownerSubtreeIds.get(ownerHostId)?.get(projectGroupId) ?? new Set([projectGroupId]) + const seen = new Set() + const candidates: Repo[] = [] + for (const groupId of groupIds) { + for (const repo of index.reposByOwnerAndGroupId.get(ownerGroupKey(ownerHostId, groupId)) ?? + []) { + if (!seen.has(repo)) { + seen.add(repo) + candidates.push(repo) + } + } + } + return candidates +} + +function collectOwnerPathMembershipRepos( + index: FolderAuthIndex, + folderPath: string, + ownerHostId: string, + exclude: ReadonlySet +): Repo[] { + const candidates: Repo[] = [] + for (const repo of index.reposByOwner.get(ownerHostId) ?? []) { + if (exclude.has(repo)) { + continue + } + if (isPathInsideOrEqual(folderPath, repo.path)) { + candidates.push(repo) + } + } + return candidates +} + +function isRemoteOnlyFolderScopeWithIndex( + index: FolderAuthIndex, + folderPath: string, + projectGroupId: string, + ownerHostId: string, + inferLegacyOwner: boolean ): boolean { - if (connectionId) { + if (ownerHostId !== LOCAL_EXECUTION_HOST_ID) { return true } - const candidates = getFolderScopeCandidateRepos(folderPath, projectGroupId, projectGroups, repos) - return candidates.length > 0 && candidates.every((repo) => Boolean(repo.connectionId)) -} - -function getFolderWorkspaceConnectionId( - workspace: FolderWorkspace, - projectGroups: readonly ProjectGroup[] -): string | null { - return ( - workspace.connectionId ?? - projectGroups.find((group) => group.id === workspace.projectGroupId)?.connectionId ?? - null + if (inferLegacyOwner) { + const groupIds = index.unscopedSubtreeIds.get(projectGroupId) ?? new Set([projectGroupId]) + let sawLegacyCandidate = false + let sawLocalLegacyCandidate = false + for (const repo of index.repos) { + const inGroup = typeof repo.projectGroupId === 'string' && groupIds.has(repo.projectGroupId) + // Why: only pay path reads when group membership alone cannot classify the scope. + if (!inGroup && !isPathInsideOrEqual(folderPath, repo.path)) { + continue + } + sawLegacyCandidate = true + if (getRepoExecutionHostId(repo) === LOCAL_EXECUTION_HOST_ID) { + sawLocalLegacyCandidate = true + break + } + } + if (sawLegacyCandidate && !sawLocalLegacyCandidate) { + return true + } + } + const groupCandidates = collectOwnerGroupMembershipRepos(index, projectGroupId, ownerHostId) + if (groupCandidates.length === 0) { + // Why: empty membership means no remote-linked evidence; authorize local folder scopes without path scans. + return false + } + if (groupCandidates.some((repo) => !repo.connectionId)) { + return false + } + // Why: path containment only matters when every group-linked repo is remote-linked. + const pathCandidates = collectOwnerPathMembershipRepos( + index, + folderPath, + ownerHostId, + new Set(groupCandidates) ) + if (pathCandidates.some((repo) => !repo.connectionId)) { + return false + } + return true } function getLocalFolderScopeRoots(store: Store): string[] { @@ -102,23 +255,40 @@ function getLocalFolderScopeRoots(store: Store): string[] { const repos = scopeStore.getRepos() // Why: many filesystem tests use narrow Store doubles; folder scopes are additive. const projectGroups = scopeStore.getProjectGroups?.() ?? [] + // Why: one owner-indexed pass avoids O((groups+folders)*(groups+repos)) remote-only checks. + const index = buildFolderAuthIndex(projectGroups, repos) const roots: string[] = [] for (const group of projectGroups) { if ( group.parentPath && - !isRemoteOnlyFolderScope(group.parentPath, group.id, group.connectionId, projectGroups, repos) + !isRemoteOnlyFolderScopeWithIndex( + index, + group.parentPath, + group.id, + getProjectGroupOwnerHostId(group), + group.connectionId === undefined && + group.executionHostId === undefined && + index.projectGroupIndex.byId.get(group.id)?.length === 1 + ) ) { roots.push(resolve(group.parentPath)) } } for (const workspace of scopeStore.getFolderWorkspaces?.() ?? []) { + const group = resolveFolderWorkspaceProjectGroup(index.projectGroupIndex, workspace) + // Why: narrow Store doubles may omit projectGroups; folder roots stay additive from the workspace catalog. + const ownerHostId = group + ? getProjectGroupOwnerHostId(group) + : getFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups) if ( - !isRemoteOnlyFolderScope( + !isRemoteOnlyFolderScopeWithIndex( + index, workspace.folderPath, workspace.projectGroupId, - getFolderWorkspaceConnectionId(workspace, projectGroups), - projectGroups, - repos + ownerHostId, + workspace.connectionId === undefined && + workspace.executionHostId === undefined && + index.projectGroupIndex.byId.get(workspace.projectGroupId)?.length === 1 ) ) { roots.push(resolve(workspace.folderPath)) diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index b6ec0118a61..4fe5a780e55 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -350,6 +350,68 @@ describe('projectGroups IPC validation', () => { expect(mockStore.updateProjectGroup).not.toHaveBeenCalled() }) + it('forwards optional project group mutation ownership to persistence', () => { + mockStore.updateProjectGroup.mockReturnValue({ id: 'group-1' }) + mockStore.deleteProjectGroup.mockReturnValue(true) + mockStore.moveProjectToGroup.mockReturnValue({ id: 'repo-1' }) + + handlers.get('projectGroups:update')!(null, { + groupId: 'group-1', + ownerHostId: 'ssh:builder', + updates: { name: 'Remote' } + }) + handlers.get('projectGroups:delete')!(null, { + groupId: 'group-1', + ownerHostId: 'local' + }) + handlers.get('projectGroups:moveProject')!(null, { + projectId: 'repo-1', + groupId: 'group-1', + order: 2, + ownerHostId: 'runtime:env-1' + }) + + expect(mockStore.updateProjectGroup).toHaveBeenCalledWith( + 'group-1', + { name: 'Remote' }, + 'ssh:builder' + ) + expect(mockStore.deleteProjectGroup).toHaveBeenCalledWith('group-1', 'local') + expect(mockStore.moveProjectToGroup).toHaveBeenCalledWith( + 'repo-1', + 'group-1', + 2, + 'runtime:env-1' + ) + }) + + it('rejects invalid project group mutation owners before persistence', () => { + expect(() => + handlers.get('projectGroups:update')!(null, { + groupId: 'group-1', + ownerHostId: 'ssh:', + updates: { name: 'Remote' } + }) + ).toThrow('invalid_project_group_update_args') + expect(() => + handlers.get('projectGroups:delete')!(null, { + groupId: 'group-1', + ownerHostId: 'builder' + }) + ).toThrow('invalid_project_group_delete_args') + expect(() => + handlers.get('projectGroups:moveProject')!(null, { + projectId: 'repo-1', + groupId: 'group-1', + ownerHostId: 'runtime:' + }) + ).toThrow('invalid_project_group_move_repo_args') + + expect(mockStore.updateProjectGroup).not.toHaveBeenCalled() + expect(mockStore.deleteProjectGroup).not.toHaveBeenCalled() + expect(mockStore.moveProjectToGroup).not.toHaveBeenCalled() + }) + it('scans nested repositories over a connected SSH filesystem', async () => { mockGitProvider.isGitRepoAsync.mockImplementation(async (path: string) => ({ isRepo: path === '/srv/platform/api', diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index bb1d8e6ff69..7e7812e4719 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -102,8 +102,16 @@ import { LOCAL_EXECUTION_HOST_ID, normalizeExecutionHostId, parseExecutionHostId, + toSshExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' +import { + buildProjectGroupOwnerIndex, + getFolderWorkspaceProjectGroupOwnerHostId, + getProjectGroupOwnerHostId, + resolveFolderWorkspaceProjectGroup, + resolveProjectGroupOwner +} from '../../shared/project-groups' import { joinRemotePath } from '../ssh/ssh-remote-platform' import { assertFolderWorkspacePathUsable, @@ -798,8 +806,21 @@ const ProjectGroupCreateArgs = z.object({ createdFrom: z.enum(['manual', 'folder-scan', 'migration']).optional() }) +const ProjectGroupOwnerHostId = z + .string() + .min(1) + .transform((value, ctx) => { + const hostId = normalizeExecutionHostId(value) + if (!hostId) { + ctx.addIssue({ code: 'custom', message: 'Invalid owner host ID' }) + return z.NEVER + } + return hostId + }) + const ProjectGroupUpdateArgs = z.object({ groupId: z.string().min(1), + ownerHostId: ProjectGroupOwnerHostId.optional(), updates: z.object({ name: z.string().optional(), isCollapsed: z.boolean().optional(), @@ -809,13 +830,15 @@ const ProjectGroupUpdateArgs = z.object({ }) const ProjectGroupSelectorArgs = z.object({ - groupId: z.string().min(1) + groupId: z.string().min(1), + ownerHostId: ProjectGroupOwnerHostId.optional() }) const ProjectGroupMoveProjectArgs = z.object({ projectId: z.string().min(1), groupId: z.string().nullable(), - order: z.number().finite().optional() + order: z.number().finite().optional(), + ownerHostId: ProjectGroupOwnerHostId.optional() }) const ProjectHostSetupExistingFolderIpcArgs = z.object({ @@ -958,7 +981,8 @@ const FolderWorkspacePathStatusArgs = z.discriminatedUnion('scope', [ }), z.object({ scope: z.literal('project-group'), - projectGroupId: z.string().min(1) + projectGroupId: z.string().min(1), + ownerHostId: ProjectGroupOwnerHostId.optional() }), z.object({ scope: z.literal('path'), @@ -1488,7 +1512,15 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v 'invalid_folder_workspace_create_args' ) const projectGroups = store.getProjectGroups() - const group = projectGroups.find((entry) => entry.id === args.projectGroupId) + const group = resolveProjectGroupOwner( + buildProjectGroupOwnerIndex(projectGroups), + args.projectGroupId, + args.connectionId !== undefined + ? args.connectionId + ? toSshExecutionHostId(args.connectionId) + : LOCAL_EXECUTION_HOST_ID + : undefined + ) const folderPath = typeof args.folderPath === 'string' && args.folderPath.trim().length > 0 ? args.folderPath @@ -1501,8 +1533,13 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v folderPath, projectGroupId: group.id, connectionId: args.connectionId ?? group.connectionId ?? null, - projectGroups, - repos: store.getRepos() + projectGroups: projectGroups.filter( + (candidate) => + getProjectGroupOwnerHostId(candidate) === getProjectGroupOwnerHostId(group) + ), + repos: store + .getRepos() + .filter((repo) => getRepoExecutionHostId(repo) === getProjectGroupOwnerHostId(group)) }, { getSshFilesystemProvider } ) @@ -1530,16 +1567,21 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v return null } const projectGroups = store.getProjectGroups() + const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups) + const group = resolveFolderWorkspaceProjectGroup(projectGroupIndex, workspace) + if (!group) { + return null + } + const ownerHostId = getFolderWorkspaceProjectGroupOwnerHostId(workspace, projectGroupIndex) const status = await getFolderWorkspacePathStatusForPath( { folderPath: args.updates.folderPath, projectGroupId: workspace.projectGroupId, - connectionId: - workspace.connectionId ?? - projectGroups.find((entry) => entry.id === workspace.projectGroupId)?.connectionId ?? - null, - projectGroups, - repos: store.getRepos() + connectionId: workspace.connectionId ?? group.connectionId ?? null, + projectGroups: projectGroups.filter( + (candidate) => getProjectGroupOwnerHostId(candidate) === ownerHostId + ), + repos: store.getRepos().filter((repo) => getRepoExecutionHostId(repo) === ownerHostId) }, { getSshFilesystemProvider } ) @@ -1589,7 +1631,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v rawArgs, 'invalid_project_group_update_args' ) - const updated = store.updateProjectGroup(args.groupId, args.updates) + const updated = store.updateProjectGroup(args.groupId, args.updates, args.ownerHostId) if (updated) { notifyReposChanged(mainWindow) } @@ -1602,7 +1644,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v rawArgs, 'invalid_project_group_delete_args' ) - const deleted = store.deleteProjectGroup(args.groupId) + const deleted = store.deleteProjectGroup(args.groupId, args.ownerHostId) if (deleted) { notifyReposChanged(mainWindow) } @@ -1615,7 +1657,12 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v rawArgs, 'invalid_project_group_move_repo_args' ) - const moved = store.moveProjectToGroup(args.projectId, args.groupId, args.order) + const moved = store.moveProjectToGroup( + args.projectId, + args.groupId, + args.order, + args.ownerHostId + ) if (moved) { notifyReposChanged(mainWindow) } diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 0e507090c1f..1efa5e1a690 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -3524,6 +3524,218 @@ describe('Store', () => { expect(store.getRepo('sibling')?.projectGroupId).toBe(sibling.id) }) + it('moves duplicate repo ids only within the exact project group owner', async () => { + const makeOwnedGroup = (name: string, connectionId: string | null): ProjectGroup => ({ + id: 'same-group', + name, + parentPath: null, + connectionId, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }) + writeDataFile({ + schemaVersion: 1, + repos: [ + makeRepo({ id: 'same-repo', path: '/local', projectGroupOrder: 900 }), + makeRepo({ + id: 'same-repo', + path: '/ssh', + connectionId: 'builder', + projectGroupOrder: 1 + }), + makeRepo({ + id: 'ssh-sibling', + path: '/ssh/sibling', + connectionId: 'builder', + projectGroupId: 'same-group', + projectGroupOrder: 5 + }) + ], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + projectGroups: [ + makeOwnedGroup('Local', null), + makeOwnedGroup('SSH', 'builder'), + { ...makeOwnedGroup('Local only', null), id: 'local-only' } + ] + }) + const store = await createStore() + + expect(store.moveProjectToGroup('same-repo', null)).toBeNull() + expect(store.moveProjectToGroup('same-repo', null, undefined, 'ssh:missing')).toBeNull() + expect(store.moveProjectToGroup('same-repo', 'local-only', undefined, 'ssh:builder')).toBeNull() + expect( + store.moveProjectToGroup('same-repo', 'same-group', undefined, 'ssh:builder') + ).toMatchObject({ + path: '/ssh', + projectGroupId: 'same-group', + projectGroupOrder: 6 + }) + + const repos = store.getRepos() + const localRepo = repos.find((entry) => entry.path === '/local') + expect(localRepo).toMatchObject({ projectGroupOrder: 900 }) + expect(localRepo?.projectGroupId).toBeUndefined() + expect(repos.find((entry) => entry.path === '/ssh')).toMatchObject({ + projectGroupId: 'same-group', + projectGroupOrder: 6 + }) + }) + + it('updates and deletes same-id project groups only within the selected owner', async () => { + const projectGroups = [ + { + id: 'root', + name: 'Local root', + parentPath: '/local', + connectionId: null, + parentGroupId: null, + createdFrom: 'folder-scan' as const, + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + { + id: 'child', + name: 'Local child', + parentPath: '/local/child', + connectionId: null, + parentGroupId: 'root', + createdFrom: 'folder-scan' as const, + tabOrder: 1, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + { + id: 'root', + name: 'SSH root', + parentPath: '/remote', + connectionId: 'builder', + parentGroupId: null, + createdFrom: 'folder-scan' as const, + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + { + id: 'child', + name: 'SSH child', + parentPath: '/remote/child', + connectionId: 'builder', + parentGroupId: 'root', + createdFrom: 'folder-scan' as const, + tabOrder: 1, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + ] + writeDataFile({ + schemaVersion: 1, + repos: [ + makeRepo({ id: 'local-repo', path: '/local/repo', projectGroupId: 'child' }), + makeRepo({ + id: 'ssh-repo', + path: '/remote/repo', + projectGroupId: 'child', + connectionId: 'builder' + }) + ], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + projectGroups + }) + const store = await createStore() + const localWorkspace = store.createFolderWorkspace({ + projectGroupId: 'child', + connectionId: null + }) + const sshWorkspace = store.createFolderWorkspace({ + projectGroupId: 'child', + connectionId: 'builder' + }) + + expect(store.updateProjectGroup('root', { name: 'Ambiguous' })).toBeNull() + expect(store.updateProjectGroup('root', { name: 'Missing' }, 'ssh:missing')).toBeNull() + expect(store.updateProjectGroup('root', { name: 'Remote' }, 'ssh:builder')?.name).toBe('Remote') + expect(store.deleteProjectGroup('root', 'local')).toBe(true) + + expect(store.getProjectGroups().map((group) => [group.name, group.connectionId])).toEqual([ + ['Remote', 'builder'], + ['SSH child', 'builder'] + ]) + expect(store.getRepo('local-repo')?.projectGroupId).toBeNull() + expect(store.getRepo('ssh-repo')?.projectGroupId).toBe('child') + expect(store.getFolderWorkspace(localWorkspace.id)).toBeUndefined() + expect(store.getFolderWorkspace(sshWorkspace.id)).toBeDefined() + }) + + it('resolves same-id folder workspace creation from explicit connection ownership', async () => { + writeDataFile({ + schemaVersion: 1, + repos: [], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + projectGroups: [ + { + id: 'same-id', + name: 'Local', + parentPath: '/local', + connectionId: null, + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + { + id: 'same-id', + name: 'SSH', + parentPath: '/remote', + connectionId: 'builder', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 1, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + ] + }) + const store = await createStore() + + expect( + store.createFolderWorkspace({ projectGroupId: 'same-id', connectionId: null }).folderPath + ).toBe('/local') + expect( + store.createFolderWorkspace({ projectGroupId: 'same-id', connectionId: 'builder' }).folderPath + ).toBe('/remote') + expect(() => store.createFolderWorkspace({ projectGroupId: 'same-id' })).toThrow( + 'Folder-backed project group not found.' + ) + }) + it('adapts flat folder-scan groups into sparse nested folder scopes on load', async () => { writeDataFile({ schemaVersion: 1, @@ -5304,6 +5516,137 @@ describe('Store', () => { expect(store.getFolderWorkspaces()[0]).toMatchObject({ id: 'fw-1', connectionId: 'ssh-1' }) }) + it('does not backfill folder scope provenance across same-id owners', async () => { + writeDataFile({ + schemaVersion: 1, + repos: [ + makeRepo({ + id: 'remote-repo', + path: '/remote/repo', + projectGroupId: 'same-id', + connectionId: 'ssh-1' + }) + ], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + projectGroups: [ + { + id: 'same-id', + name: 'Local', + parentPath: '/local', + connectionId: null, + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + { + id: 'same-id', + name: 'SSH', + parentPath: '/remote', + connectionId: 'ssh-1', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 1, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + ], + folderWorkspaces: [ + { + id: 'local-folder', + projectGroupId: 'same-id', + name: 'Local folder', + folderPath: '/local', + connectionId: null + }, + { + id: 'ssh-folder', + projectGroupId: 'same-id', + name: 'SSH folder', + folderPath: '/remote', + connectionId: 'ssh-1' + } + ] + }) + + const store = await createStore() + + expect(store.getProjectGroups().map((group) => [group.name, group.connectionId])).toEqual([ + ['Local', null], + ['SSH', 'ssh-1'] + ]) + expect( + store.getFolderWorkspaces().map((workspace) => [workspace.name, workspace.connectionId]) + ).toEqual([ + ['Local folder', null], + ['SSH folder', 'ssh-1'] + ]) + }) + + it('normalizes same-id runtime folder workspaces against their explicit owner', async () => { + writeDataFile({ + schemaVersion: 1, + repos: [], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + projectGroups: [ + { + id: 'same-id', + name: 'Runtime', + parentPath: '/runtime', + executionHostId: 'runtime:hub-1', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + { + id: 'same-id', + name: 'Local', + parentPath: '/local', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 1, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + ], + folderWorkspaces: [ + { + id: 'runtime-folder', + projectGroupId: 'same-id', + name: 'Runtime folder', + executionHostId: 'runtime:hub-1' + } + ] + }) + + const store = await createStore() + + expect(store.getFolderWorkspaces()).toEqual([ + expect.objectContaining({ + id: 'runtime-folder', + folderPath: '/runtime', + executionHostId: 'runtime:hub-1' + }) + ]) + }) + it('removes folder workspace metadata and its scoped session state only', async () => { const store = await createStore() const group = store.createProjectGroup({ diff --git a/src/main/persistence.ts b/src/main/persistence.ts index c2e36d3ce9d..df56de55ffa 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -219,12 +219,18 @@ import { isLegacyRepoForExternalWorktreeVisibility } from '../shared/worktree-ow import { sanitizeRepoIcon } from '../shared/repo-icon' import { normalizeRepoBadgeColor } from '../shared/repo-badge-color' import { + buildProjectGroupOwnerIndex, clearMissingProjectGroupMemberships, createProjectGroup, getNextProjectGroupOrder, - getProjectGroupSubtreeIds, + getProjectGroupOwnerHostId, + getProjectGroupOwnerIdentity, + getProjectGroupOwnerSubtreeIdentities, normalizeProjectGroupName, - normalizeProjectGroups + normalizeProjectGroups, + resolveProjectGroupMembership, + resolveProjectGroupOwner, + resolveFolderWorkspaceProjectGroup } from '../shared/project-groups' import { createNestedProjectGroupResolver } from './project-groups/nested-repo-import' import { @@ -2684,13 +2690,31 @@ function removeWorkspaceSessionOwners( function inferFolderScopeConnectionIdForMigration(args: { folderPath: string - projectGroupId: string + projectGroup: ProjectGroup projectGroups: readonly ProjectGroup[] + projectGroupIndex: ReturnType repos: readonly Repo[] }): string | null { - const groupIds = getProjectGroupSubtreeIds(args.projectGroups, args.projectGroupId) + if (args.projectGroupIndex.byId.get(args.projectGroup.id)?.length !== 1) { + return null + } + const groupIdentities = getProjectGroupOwnerSubtreeIdentities( + args.projectGroups, + args.projectGroup + ) const groupRepos = args.repos.filter( - (repo) => typeof repo.projectGroupId === 'string' && groupIds.has(repo.projectGroupId) + (repo) => + typeof repo.projectGroupId === 'string' && + (() => { + const exactGroup = resolveProjectGroupMembership( + args.projectGroupIndex, + repo.projectGroupId, + getRepoExecutionHostId(repo) + ) + const group = + exactGroup ?? resolveProjectGroupOwner(args.projectGroupIndex, repo.projectGroupId) + return group !== null && groupIdentities.has(getProjectGroupOwnerIdentity(group)) + })() ) const candidateRepos = groupRepos.length > 0 @@ -2720,15 +2744,17 @@ function backfillFolderScopeConnectionIds(state: PersistedState): { } { const groups = state.projectGroups ?? [] const repos = state.repos ?? [] + const originalProjectGroupIndex = buildProjectGroupOwnerIndex(groups) let changed = false const projectGroups = groups.map((group) => { - if (group.connectionId || !group.parentPath) { + if (getProjectGroupOwnerHostId(group) !== LOCAL_EXECUTION_HOST_ID || !group.parentPath) { return group } const connectionId = inferFolderScopeConnectionIdForMigration({ folderPath: group.parentPath, - projectGroupId: group.id, + projectGroup: group, projectGroups: groups, + projectGroupIndex: originalProjectGroupIndex, repos }) if (!connectionId) { @@ -2737,20 +2763,24 @@ function backfillFolderScopeConnectionIds(state: PersistedState): { changed = true return { ...group, connectionId } }) - const groupsById = new Map(projectGroups.map((group) => [group.id, group])) + const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups) const folderWorkspaces = (state.folderWorkspaces ?? []).map((workspace) => { - if (workspace.connectionId) { + if (workspace.connectionId || normalizeExecutionHostId(workspace.executionHostId)) { return workspace } - const groupConnectionId = groupsById.get(workspace.projectGroupId)?.connectionId ?? null + const group = resolveProjectGroupOwner(projectGroupIndex, workspace.projectGroupId) + const groupConnectionId = group?.connectionId ?? null const connectionId = groupConnectionId ?? - inferFolderScopeConnectionIdForMigration({ - folderPath: workspace.folderPath, - projectGroupId: workspace.projectGroupId, - projectGroups, - repos - }) + (group + ? inferFolderScopeConnectionIdForMigration({ + folderPath: workspace.folderPath, + projectGroup: group, + projectGroups, + projectGroupIndex, + repos + }) + : null) if (!connectionId) { return workspace } @@ -3769,6 +3799,16 @@ export class Store { this.loadNeedsSave = true } + // Why: legacy folder groups derive SSH ownership from their assigned repos; stamp that owner before exact-owner membership validation. + const folderScopeConnectionMigration = backfillFolderScopeConnectionIds({ + ...result, + workspaceSession: migratedScrollback.session + }) + if (folderScopeConnectionMigration.changed) { + this.loadNeedsSave = true + } + result = folderScopeConnectionMigration.state + const repos = clearMissingProjectGroupMemberships(result.repos, result.projectGroups ?? []) const projectHostSetupCompatibility = mergeProjectHostSetupCompatibilityState(result, repos) if (!projectHostSetupCompatibilityStateEqual(result, projectHostSetupCompatibility)) { @@ -3785,21 +3825,12 @@ export class Store { } result = { ...result, + repos, + ...projectHostSetupCompatibility, automations: automationContextMigration.state.automations, automationRuns: automationContextMigration.state.automationRuns } - const folderScopeConnectionMigration = backfillFolderScopeConnectionIds({ - ...result, - repos, - ...projectHostSetupCompatibility, - workspaceSession: migratedScrollback.session - }) - if (folderScopeConnectionMigration.changed) { - this.loadNeedsSave = true - } - result = folderScopeConnectionMigration.state - if (normalizeWorktreeLinkedItemMetadata(result)) { this.loadNeedsSave = true } @@ -4399,9 +4430,15 @@ export class Store { updateProjectGroup( groupId: string, - updates: Partial> + updates: Partial>, + ownerHostId?: ExecutionHostId ): ProjectGroup | null { - const group = (this.state.projectGroups ?? []).find((entry) => entry.id === groupId) + const projectGroups = this.state.projectGroups ?? [] + const group = resolveProjectGroupOwner( + buildProjectGroupOwnerIndex(projectGroups), + groupId, + ownerHostId + ) if (!group) { return null } @@ -4422,24 +4459,39 @@ export class Store { return group } - deleteProjectGroup(groupId: string): boolean { + deleteProjectGroup(groupId: string, ownerHostId?: ExecutionHostId): boolean { + const projectGroups = this.state.projectGroups ?? [] + const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups) + const rootGroup = resolveProjectGroupOwner(projectGroupIndex, groupId, ownerHostId) + if (!rootGroup) { + return false + } const before = this.state.projectGroups?.length ?? 0 - const deletedGroupIds = getProjectGroupSubtreeIds(this.state.projectGroups ?? [], groupId) - this.state.projectGroups = (this.state.projectGroups ?? []).filter( - (group) => !deletedGroupIds.has(group.id) + const deletedGroupIdentities = getProjectGroupOwnerSubtreeIdentities(projectGroups, rootGroup) + this.state.projectGroups = projectGroups.filter( + (group) => !deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group)) ) if ((this.state.projectGroups?.length ?? 0) === before) { return false } // Why: groups are sidebar organization only, so deleting one ungroups its repos rather than deleting them. - this.state.repos = this.state.repos.map((repo) => - repo.projectGroupId && deletedGroupIds.has(repo.projectGroupId) + this.state.repos = this.state.repos.map((repo) => { + if (!repo.projectGroupId) { + return repo + } + const group = resolveProjectGroupMembership( + projectGroupIndex, + repo.projectGroupId, + getRepoExecutionHostId(repo) + ) + return group && deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group)) ? { ...repo, projectGroupId: null } : repo - ) + }) const removedFolderWorkspaceKeys = new Set() for (const workspace of this.state.folderWorkspaces ?? []) { - if (deletedGroupIds.has(workspace.projectGroupId)) { + const group = resolveFolderWorkspaceProjectGroup(projectGroupIndex, workspace) + if (group && deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group))) { removedFolderWorkspaceKeys.add(folderWorkspaceKey(workspace.id)) this.state.workspaceSession = removeWorkspaceSessionOwner( this.state.workspaceSession, @@ -4448,9 +4500,10 @@ export class Store { this.removeWorkspaceLineageForFolderParent(workspace.id) } } - this.state.folderWorkspaces = (this.state.folderWorkspaces ?? []).filter( - (workspace) => !deletedGroupIds.has(workspace.projectGroupId) - ) + this.state.folderWorkspaces = (this.state.folderWorkspaces ?? []).filter((workspace) => { + const group = resolveFolderWorkspaceProjectGroup(projectGroupIndex, workspace) + return !group || !deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group)) + }) this.pruneMobileClientTabSelections((worktreeId) => removedFolderWorkspaceKeys.has(worktreeId)) this.scheduleSave() return true @@ -4476,8 +4529,15 @@ export class Store { createdWithAgent?: FolderWorkspace['createdWithAgent'] pendingFirstAgentMessageRename?: boolean }): FolderWorkspace { - const group = (this.state.projectGroups ?? []).find( - (entry) => entry.id === input.projectGroupId + const projectGroupIndex = buildProjectGroupOwnerIndex(this.state.projectGroups ?? []) + const group = resolveProjectGroupOwner( + projectGroupIndex, + input.projectGroupId, + input.connectionId !== undefined + ? input.connectionId + ? toSshExecutionHostId(input.connectionId) + : LOCAL_EXECUTION_HOST_ID + : undefined ) const folderPath = typeof input.folderPath === 'string' && input.folderPath.trim().length > 0 @@ -4633,21 +4693,43 @@ export class Store { return true } - moveProjectToGroup(repoId: string, groupId: string | null, order?: number): Repo | null { - const repo = this.state.repos.find((entry) => entry.id === repoId) - if (!repo) { + moveProjectToGroup( + repoId: string, + groupId: string | null, + order?: number, + ownerHostId?: ExecutionHostId + ): Repo | null { + const hasExplicitOwner = ownerHostId !== undefined + if (hasExplicitOwner && !parseExecutionHostId(ownerHostId)) { return null } - const normalizedGroupId = - groupId && (this.state.projectGroups ?? []).some((group) => group.id === groupId) - ? groupId - : null - const siblingRepos = this.state.repos.filter((entry) => entry.id !== repoId) - repo.projectGroupId = normalizedGroupId + const matchingRepos = this.state.repos.filter( + (entry) => + entry.id === repoId && (!hasExplicitOwner || getRepoExecutionHostId(entry) === ownerHostId) + ) + if (matchingRepos.length !== 1) { + return null + } + const repo = matchingRepos[0] + const repoOwnerHostId = getRepoExecutionHostId(repo) + if ( + groupId !== null && + !resolveProjectGroupOwner( + buildProjectGroupOwnerIndex(this.state.projectGroups ?? []), + groupId, + repoOwnerHostId + ) + ) { + return null + } + const siblingRepos = this.state.repos.filter( + (entry) => entry !== repo && getRepoExecutionHostId(entry) === repoOwnerHostId + ) + repo.projectGroupId = groupId repo.projectGroupOrder = typeof order === 'number' && Number.isFinite(order) ? order - : getNextProjectGroupOrder(siblingRepos, normalizedGroupId) + : getNextProjectGroupOrder(siblingRepos, groupId) this.scheduleSave() return this.hydrateRepo(repo) } diff --git a/src/main/project-groups/folder-workspace-path-status.test.ts b/src/main/project-groups/folder-workspace-path-status.test.ts index 495f4690434..8e88f26aba6 100644 --- a/src/main/project-groups/folder-workspace-path-status.test.ts +++ b/src/main/project-groups/folder-workspace-path-status.test.ts @@ -6,7 +6,8 @@ import { describe, expect, it, vi } from 'vitest' import { getFolderWorkspacePathStatus, getFolderWorkspacePathStatusForPath, - inferFolderWorkspacePathConnection + inferFolderWorkspacePathConnection, + resolveFolderWorkspaceStatusPath } from './folder-workspace-path-status' import type { IFilesystemProvider } from '../providers/types' import type { ProjectGroup, Repo } from '../../shared/types' @@ -39,6 +40,33 @@ function makeRepo(overrides: Partial = {}): Repo { } } +it('resolves same-id project-group paths by exact owner', () => { + const local = makeGroup({ id: 'same-id', parentPath: '/local' }) + const ssh = makeGroup({ id: 'same-id', parentPath: '/remote', connectionId: 'builder' }) + const store = { + getRepos: () => [], + getProjectGroups: () => [local, ssh] + } + + expect( + resolveFolderWorkspaceStatusPath({ + store, + request: { scope: 'project-group', projectGroupId: 'same-id', ownerHostId: 'ssh:builder' } + }) + ).toEqual({ + folderPath: '/remote', + projectGroupId: 'same-id', + connectionId: 'builder', + ownerHostId: 'ssh:builder' + }) + expect(() => + resolveFolderWorkspaceStatusPath({ + store, + request: { scope: 'project-group', projectGroupId: 'same-id' } + }) + ).toThrow('folder_workspace_path_scope_not_found') +}) + describe('folder workspace path status', () => { it('reports existing local directories and local files', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-folder-status-')) diff --git a/src/main/project-groups/folder-workspace-path-status.ts b/src/main/project-groups/folder-workspace-path-status.ts index d8df44da72c..0350cabff88 100644 --- a/src/main/project-groups/folder-workspace-path-status.ts +++ b/src/main/project-groups/folder-workspace-path-status.ts @@ -4,7 +4,15 @@ import type { FolderWorkspacePathStatus, FolderWorkspacePathStatusRequest } from '../../shared/folder-workspace-path-status' -import { getProjectGroupSubtreeIds } from '../../shared/project-groups' +import { + buildProjectGroupOwnerIndex, + getFolderWorkspaceProjectGroupOwnerHostId, + getProjectGroupOwnerHostId, + getProjectGroupSubtreeIds, + resolveProjectGroupOwner, + resolveFolderWorkspaceProjectGroup +} from '../../shared/project-groups' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import type { FolderWorkspace, ProjectGroup, Repo } from '../../shared/types' import type { IFilesystemProvider } from '../providers/types' @@ -152,19 +160,29 @@ export async function getFolderWorkspacePathStatusForPath( export function resolveFolderWorkspaceStatusPath(args: { store: FolderWorkspacePathStatusStore request: FolderWorkspacePathStatusRequest -}): { folderPath: string; projectGroupId: string | null; connectionId?: string | null } { +}): { + folderPath: string + projectGroupId: string | null + connectionId?: string | null + ownerHostId?: ExecutionHostId +} { const { request } = args + const projectGroups = args.store.getProjectGroups?.() ?? [] + const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups) if (request.scope === 'project-group') { - const group = args.store - .getProjectGroups?.() - .find((entry) => entry.id === request.projectGroupId) + const group = resolveProjectGroupOwner( + projectGroupIndex, + request.projectGroupId, + request.ownerHostId + ) if (!group?.parentPath) { throw new Error('folder_workspace_path_scope_not_found') } return { folderPath: group.parentPath, projectGroupId: group.id, - connectionId: group.connectionId ?? null + connectionId: group.connectionId ?? null, + ownerHostId: getProjectGroupOwnerHostId(group) } } @@ -182,13 +200,14 @@ export function resolveFolderWorkspaceStatusPath(args: { if (!workspace) { throw new Error('folder_workspace_path_scope_not_found') } - const group = args.store - .getProjectGroups?.() - .find((entry) => entry.id === workspace.projectGroupId) + const group = resolveFolderWorkspaceProjectGroup(projectGroupIndex, workspace) + // Why: fail closed only when the workspace row is missing; group may lag hydrate on Store doubles. + const ownerHostId = getFolderWorkspaceProjectGroupOwnerHostId(workspace, projectGroupIndex) return { folderPath: workspace.folderPath, projectGroupId: workspace.projectGroupId, - connectionId: workspace.connectionId ?? group?.connectionId ?? null + connectionId: workspace.connectionId ?? group?.connectionId ?? null, + ownerHostId } } @@ -198,13 +217,18 @@ export async function getFolderWorkspacePathStatus( deps: FolderWorkspacePathStatusDeps ): Promise { const scope = resolveFolderWorkspaceStatusPath({ store, request }) + const projectGroups = store.getProjectGroups?.() ?? [] return getFolderWorkspacePathStatusForPath( { folderPath: scope.folderPath, projectGroupId: scope.projectGroupId, connectionId: scope.connectionId, - projectGroups: store.getProjectGroups?.() ?? [], - repos: store.getRepos() + projectGroups: scope.ownerHostId + ? projectGroups.filter((group) => getProjectGroupOwnerHostId(group) === scope.ownerHostId) + : projectGroups, + repos: scope.ownerHostId + ? store.getRepos().filter((repo) => getRepoExecutionHostId(repo) === scope.ownerHostId) + : store.getRepos() }, deps ) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 1cae8cd2359..77c00313891 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1345,13 +1345,15 @@ export type PreloadApi = { }) => Promise update: (args: { groupId: string + ownerHostId?: ExecutionHostId updates: Partial> }) => Promise - delete: (args: { groupId: string }) => Promise + delete: (args: { groupId: string; ownerHostId?: ExecutionHostId }) => Promise moveProject: (args: { projectId: string groupId: string | null order?: number + ownerHostId?: ExecutionHostId }) => Promise scanNested: (args: { path: string diff --git a/src/renderer/src/components/sidebar/project-group-sidebar-identity.test.ts b/src/renderer/src/components/sidebar/project-group-sidebar-identity.test.ts new file mode 100644 index 00000000000..86e09d24a7c --- /dev/null +++ b/src/renderer/src/components/sidebar/project-group-sidebar-identity.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import type { ProjectGroup } from '../../../../shared/types' +import { + buildProjectGroupSidebarIndex, + findProjectGroupForSidebarOwner, + getProjectGroupSidebarIdentity, + hasSingleProjectGroupMutationOwner +} from './project-group-sidebar-identity' + +function group(id: string, executionHostId: string): ProjectGroup { + return { + id, + name: `${executionHostId}:${id}`, + parentPath: null, + executionHostId, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } +} + +describe('project-group sidebar identity', () => { + it('resolves duplicate ids only with their catalog owner', () => { + const local = group('same-id', 'local') + const runtime = group('same-id', 'runtime:env-1') + const index = buildProjectGroupSidebarIndex([local, runtime]) + + expect(findProjectGroupForSidebarOwner(index, 'same-id')).toBeUndefined() + expect(findProjectGroupForSidebarOwner(index, 'same-id', 'local')).toBe(local) + expect(findProjectGroupForSidebarOwner(index, 'same-id', 'runtime:env-1')).toBe(runtime) + expect(getProjectGroupSidebarIdentity(local)).not.toBe(getProjectGroupSidebarIdentity(runtime)) + }) + + it('disables focused-host mutations for mixed-owner and mismatched lists', () => { + const localA = group('a', 'local') + const localB = group('b', 'local') + const runtime = group('c', 'runtime:env-1') + + expect(hasSingleProjectGroupMutationOwner([localA, localB], 'local')).toBe(true) + expect(hasSingleProjectGroupMutationOwner([localA, localB], 'runtime:env-1')).toBe(false) + expect(hasSingleProjectGroupMutationOwner([localA, runtime], 'local')).toBe(false) + }) +}) diff --git a/src/renderer/src/components/sidebar/project-group-sidebar-identity.ts b/src/renderer/src/components/sidebar/project-group-sidebar-identity.ts new file mode 100644 index 00000000000..5afba9bdb32 --- /dev/null +++ b/src/renderer/src/components/sidebar/project-group-sidebar-identity.ts @@ -0,0 +1,151 @@ +import type { FolderWorkspace, ProjectGroup, Repo } from '../../../../shared/types' +import { + getRepoExecutionHostId, + normalizeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../../../shared/execution-host' +import { + getProjectGroupOwnerHostId as getSharedProjectGroupOwnerHostId, + getProjectGroupOwnerIdentity +} from '../../../../shared/project-groups' + +export type ProjectGroupSidebarIdentity = string + +export function getProjectGroupOwnerHostId( + group: Pick +): ExecutionHostId { + return getSharedProjectGroupOwnerHostId(group) +} + +export function getProjectGroupSidebarIdentity( + group: Pick +): ProjectGroupSidebarIdentity { + return getProjectGroupOwnerIdentity(group) +} + +export function hasSingleProjectGroupMutationOwner( + groups: readonly ProjectGroup[], + routedHostId: ExecutionHostId +): boolean { + if (groups.length === 0) { + return false + } + return groups.every((group) => getProjectGroupOwnerHostId(group) === routedHostId) +} + +export function parseProjectGroupSidebarHeaderKey( + key: string +): { groupId: string; ownerHostId?: ExecutionHostId } | null { + const prefix = 'project-group:' + if (!key.startsWith(prefix)) { + return null + } + const value = key.slice(prefix.length) + const separator = value.indexOf(':') + if (separator === -1) { + return { groupId: value } + } + let ownerHostId: ExecutionHostId | null + let groupId: string + try { + ownerHostId = normalizeExecutionHostId(decodeURIComponent(value.slice(0, separator))) + groupId = decodeURIComponent(value.slice(separator + 1)) + } catch { + return null + } + if (!ownerHostId) { + return null + } + return { + ownerHostId, + groupId + } +} + +export type ProjectGroupSidebarIndex = { + byIdentity: ReadonlyMap + byUnambiguousId: ReadonlyMap +} + +export function buildProjectGroupSidebarIndex( + projectGroups: readonly ProjectGroup[] +): ProjectGroupSidebarIndex { + const byIdentity = new Map() + const byUnambiguousId = new Map() + const ambiguousIds = new Set() + for (const group of projectGroups) { + byIdentity.set(getProjectGroupSidebarIdentity(group), group) + if (ambiguousIds.has(group.id)) { + continue + } + const existing = byUnambiguousId.get(group.id) + if ( + existing && + getProjectGroupSidebarIdentity(existing) !== getProjectGroupSidebarIdentity(group) + ) { + byUnambiguousId.delete(group.id) + ambiguousIds.add(group.id) + } else { + byUnambiguousId.set(group.id, group) + } + } + return { byIdentity, byUnambiguousId } +} + +export function findProjectGroupForSidebarOwner( + index: ProjectGroupSidebarIndex, + groupId: string | null | undefined, + ownerHostId?: ExecutionHostId | null +): ProjectGroup | undefined { + if (!groupId) { + return undefined + } + if (ownerHostId) { + const exact = index.byIdentity.get(JSON.stringify([ownerHostId, groupId])) + if (exact) { + return exact + } + } + return index.byUnambiguousId.get(groupId) +} + +export function findProjectGroupForRepo( + index: ProjectGroupSidebarIndex, + repo: Pick | undefined, + defaultHostId: ExecutionHostId +): ProjectGroup | undefined { + if (!repo?.projectGroupId) { + return undefined + } + const ownerHostId = + repo.connectionId || repo.executionHostId ? getRepoExecutionHostId(repo) : defaultHostId + return findProjectGroupForSidebarOwner(index, repo.projectGroupId, ownerHostId) +} + +export function findProjectGroupForFolderWorkspace( + index: ProjectGroupSidebarIndex, + workspace: Pick +): ProjectGroup | undefined { + const explicitHostId = normalizeExecutionHostId(workspace.executionHostId) + // Why: connectionId null is an explicit local stamp; leave undefined only when owner is unknown. + const ownerHostId = + explicitHostId ?? + (workspace.connectionId + ? toSshExecutionHostId(workspace.connectionId) + : workspace.connectionId === null + ? ('local' as ExecutionHostId) + : undefined) + return findProjectGroupForSidebarOwner(index, workspace.projectGroupId, ownerHostId) +} + +export function findProjectGroupParentForSidebar( + index: ProjectGroupSidebarIndex, + group: ProjectGroup +): ProjectGroup | undefined { + return findProjectGroupForSidebarOwner( + index, + group.parentGroupId, + getProjectGroupOwnerHostId(group) + ) +} diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.ts b/src/renderer/src/components/sidebar/worktree-list-groups.ts index 777e9fcfbe6..def461aabd5 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.ts @@ -29,6 +29,7 @@ import { getEffectiveProjectGroupManualRank, UNGROUPED_PROJECT_GROUP_KEY } from '../../../../shared/project-groups' +import { getFolderWorkspaceRowKey } from '../../../../shared/folder-workspaces' import { cloneDefaultWorkspaceStatuses } from '../../../../shared/workspace-statuses' import type { AppState } from '../../store/types' import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from '../../store/slices/github-cache-key' @@ -51,6 +52,18 @@ import { getCyclicProjectedWorktreeLineageIds, getLineageRenderInfo } from './worktree-lineage-projection' +import { + getFolderWorkspaceExecutionHostIdForRows, + getProjectGroupExecutionHostIdForRows +} from './worktree-list-host-filtering' +import { + buildProjectGroupSidebarIndex, + findProjectGroupForFolderWorkspace, + findProjectGroupForRepo, + findProjectGroupParentForSidebar, + getProjectGroupOwnerHostId, + getProjectGroupSidebarIdentity +} from './project-group-sidebar-identity' export { getLineageRenderInfo } from './worktree-lineage-projection' @@ -74,11 +87,15 @@ export type GroupHeaderRow = { icon?: React.ComponentType<{ className?: string }> repo?: Repo projectGroup?: ProjectGroup | { id: null; name: 'Ungrouped'; tabOrder: number } + projectGroupOwnerHostId?: ExecutionHostId + collapseKey?: string projectGroupDepth?: number hostId?: ExecutionHostId hostWorktreeCounts?: ReadonlyMap hostWorktreeIds?: ReadonlyMap + hostFolderWorkspaceIds?: ReadonlyMap worktreeIds?: readonly string[] + folderWorkspaceIds?: readonly string[] } export type WorktreeRow = { @@ -178,6 +195,15 @@ type WorktreeGroupEntry = { repoIds: Set } +type ProjectGroupHostMetrics = { + count: number + hostCounts: Map + hostWorktreeIds: Map + hostFolderWorkspaceIds: Map + worktreeIds: string[] + folderWorkspaceIds: string[] +} + type ProjectGroupingIndex = { projectById: Map setupByRepoId: Map @@ -365,8 +391,16 @@ export const PROJECT_GROUP_META = { icon: FolderTree } as const -export function getProjectGroupHeaderKey(groupId: string | null): string { - return groupId ? `project-group:${groupId}` : UNGROUPED_PROJECT_GROUP_KEY +export function getProjectGroupHeaderKey( + groupId: string | null, + ownerHostId?: ExecutionHostId +): string { + if (!groupId) { + return UNGROUPED_PROJECT_GROUP_KEY + } + return ownerHostId + ? `project-group:${encodeURIComponent(ownerHostId)}:${encodeURIComponent(groupId)}` + : `project-group:${groupId}` } export const PINNED_GROUP_KEY = 'pinned' @@ -1260,7 +1294,10 @@ export function buildRows( tone: PROJECT_GROUP_META.tone, icon: PROJECT_GROUP_META.icon, repo, - projectGroupDepth + projectGroupDepth, + hostWorktreeCounts: getHostWorktreeCounts(group.items, repoMap, defaultHostId), + hostWorktreeIds: getHostWorktreeIds(group.items, repoMap, defaultHostId), + worktreeIds: group.items.map((worktree) => worktree.id) } : groupBy === 'workspace-status' ? (() => { @@ -1369,13 +1406,15 @@ export function buildRows( return result } - const groupByProjectGroupId = new Map() + const projectGroupIndex = buildProjectGroupSidebarIndex(projectGroups) + const groupByProjectGroupIdentity = new Map() for (const entry of orderedGroups) { const repo = entry[1].repo - const projectGroupId = repo?.projectGroupId ?? null - const list = groupByProjectGroupId.get(projectGroupId) ?? [] + const projectGroup = findProjectGroupForRepo(projectGroupIndex, repo, defaultHostId) + const projectGroupIdentity = projectGroup ? getProjectGroupSidebarIdentity(projectGroup) : null + const list = groupByProjectGroupIdentity.get(projectGroupIdentity) ?? [] list.push(entry) - groupByProjectGroupId.set(projectGroupId, list) + groupByProjectGroupIdentity.set(projectGroupIdentity, list) } const sortRepoEntriesWithinGroup = (entries: OrderedGroupEntry[]): OrderedGroupEntry[] => { @@ -1394,67 +1433,162 @@ export function buildRows( }) } - const projectGroupsById = new Map(projectGroups.map((group) => [group.id, group])) - const folderWorkspacesByProjectGroupId = new Map() + const folderWorkspacesByProjectGroupIdentity = new Map() for (const workspace of folderWorkspaces) { - const group = projectGroupsById.get(workspace.projectGroupId) + const group = findProjectGroupForFolderWorkspace(projectGroupIndex, workspace) if (!group?.parentPath) { continue } - const list = folderWorkspacesByProjectGroupId.get(workspace.projectGroupId) ?? [] + const identity = getProjectGroupSidebarIdentity(group) + const list = folderWorkspacesByProjectGroupIdentity.get(identity) ?? [] list.push(workspace) - folderWorkspacesByProjectGroupId.set(workspace.projectGroupId, list) + folderWorkspacesByProjectGroupIdentity.set(identity, list) } - for (const list of folderWorkspacesByProjectGroupId.values()) { + for (const list of folderWorkspacesByProjectGroupIdentity.values()) { list.sort((left, right) => { const leftOrder = left.manualOrder ?? left.sortOrder const rightOrder = right.manualOrder ?? right.sortOrder return rightOrder - leftOrder || left.name.localeCompare(right.name) }) } - const childGroupsByParentId = new Map() + const childGroupsByParentIdentity = new Map() for (const group of projectGroups) { - const parentId = - group.parentGroupId && projectGroupsById.has(group.parentGroupId) ? group.parentGroupId : null - const children = childGroupsByParentId.get(parentId) ?? [] + const parent = findProjectGroupParentForSidebar(projectGroupIndex, group) + const parentIdentity = parent ? getProjectGroupSidebarIdentity(parent) : null + const children = childGroupsByParentIdentity.get(parentIdentity) ?? [] children.push(group) - childGroupsByParentId.set(parentId, children) + childGroupsByParentIdentity.set(parentIdentity, children) } - for (const groups of childGroupsByParentId.values()) { + for (const groups of childGroupsByParentIdentity.values()) { groups.sort( (left, right) => left.tabOrder - right.tabOrder || left.name.localeCompare(right.name) ) } - const getProjectGroupSubtreeCount = (groupId: string): number => { - const directCount = groupByProjectGroupId.get(groupId)?.length ?? 0 - const folderWorkspaceCount = folderWorkspacesByProjectGroupId.get(groupId)?.length ?? 0 - const children = childGroupsByParentId.get(groupId) ?? [] - return children.reduce( - (count, child) => count + getProjectGroupSubtreeCount(child.id), - directCount + folderWorkspaceCount - ) + const projectGroupHostMetricsByIdentity = new Map() + const getProjectGroupHostMetrics = (identity: string): ProjectGroupHostMetrics => { + const cached = projectGroupHostMetricsByIdentity.get(identity) + if (cached) { + return cached + } + const metrics: ProjectGroupHostMetrics = { + count: 0, + hostCounts: new Map(), + hostWorktreeIds: new Map(), + hostFolderWorkspaceIds: new Map(), + worktreeIds: [], + folderWorkspaceIds: [] + } + const incrementHostCount = (hostId: ExecutionHostId, amount: number): void => { + metrics.hostCounts.set(hostId, (metrics.hostCounts.get(hostId) ?? 0) + amount) + if (!metrics.hostWorktreeIds.has(hostId)) { + metrics.hostWorktreeIds.set(hostId, []) + } + if (!metrics.hostFolderWorkspaceIds.has(hostId)) { + metrics.hostFolderWorkspaceIds.set(hostId, []) + } + } + for (const [, entry] of groupByProjectGroupIdentity.get(identity) ?? []) { + metrics.count += 1 + const entryHostIds = new Set() + for (const worktree of entry.items) { + const hostId = getWorktreeExecutionHostId( + worktree, + repoMap.get(worktree.repoId), + defaultHostId + ) + entryHostIds.add(hostId) + const hostWorktreeIds = metrics.hostWorktreeIds.get(hostId) ?? [] + hostWorktreeIds.push(worktree.id) + metrics.hostWorktreeIds.set(hostId, hostWorktreeIds) + metrics.worktreeIds.push(worktree.id) + } + for (const repoId of entry.repoIds) { + const entryRepo = repoMap.get(repoId) + entryHostIds.add( + entryRepo?.connectionId || entryRepo?.executionHostId + ? getRepoExecutionHostId(entryRepo) + : defaultHostId + ) + } + for (const hostId of entryHostIds) { + incrementHostCount(hostId, 1) + } + } + const projectGroup = projectGroupIndex.byIdentity.get(identity) + for (const folderWorkspace of folderWorkspacesByProjectGroupIdentity.get(identity) ?? []) { + metrics.count += 1 + const hostId = getFolderWorkspaceExecutionHostIdForRows({ + folderWorkspace, + projectGroup, + defaultHostId + }) + incrementHostCount(hostId, 1) + metrics.hostFolderWorkspaceIds.get(hostId)!.push(folderWorkspace.id) + metrics.folderWorkspaceIds.push(folderWorkspace.id) + } + for (const child of childGroupsByParentIdentity.get(identity) ?? []) { + const childMetrics = getProjectGroupHostMetrics(getProjectGroupSidebarIdentity(child)) + metrics.count += childMetrics.count + metrics.worktreeIds.push(...childMetrics.worktreeIds) + metrics.folderWorkspaceIds.push(...childMetrics.folderWorkspaceIds) + for (const [hostId, count] of childMetrics.hostCounts) { + incrementHostCount(hostId, count) + metrics.hostWorktreeIds + .get(hostId)! + .push(...(childMetrics.hostWorktreeIds.get(hostId) ?? [])) + metrics.hostFolderWorkspaceIds + .get(hostId)! + .push(...(childMetrics.hostFolderWorkspaceIds.get(hostId) ?? [])) + } + } + if (metrics.hostCounts.size === 0 && projectGroup) { + incrementHostCount(getProjectGroupExecutionHostIdForRows(projectGroup, defaultHostId), 0) + } + projectGroupHostMetricsByIdentity.set(identity, metrics) + return metrics } const appendProjectGroup = (projectGroup: ProjectGroup, depth: number): void => { - const repoEntries = sortRepoEntriesWithinGroup(groupByProjectGroupId.get(projectGroup.id) ?? []) - const childGroups = childGroupsByParentId.get(projectGroup.id) ?? [] - const key = getProjectGroupHeaderKey(projectGroup.id) + const identity = getProjectGroupSidebarIdentity(projectGroup) + const ownerHostId = getProjectGroupOwnerHostId(projectGroup) + const repoEntries = sortRepoEntriesWithinGroup(groupByProjectGroupIdentity.get(identity) ?? []) + const childGroups = childGroupsByParentIdentity.get(identity) ?? [] + // Why: bare project-group: stays when the bare id is unique; only same-id multi-owner rows need owner stamps. + const keyOwner = + projectGroups.filter((group) => group.id === projectGroup.id).length > 1 + ? ownerHostId + : undefined + const key = getProjectGroupHeaderKey(projectGroup.id, keyOwner) + // Why: same-id groups on distinct owners collapse independently in persisted sidebar state. + const collapseKey = getProjectGroupHeaderKey(projectGroup.id, keyOwner) + const hostMetrics = getProjectGroupHostMetrics(identity) result.push({ type: 'header', key, label: projectGroup.name, - count: getProjectGroupSubtreeCount(projectGroup.id), + count: hostMetrics.count, tone: PROJECT_GROUP_META.tone, icon: PROJECT_GROUP_META.icon, projectGroup, - projectGroupDepth: depth + projectGroupOwnerHostId: ownerHostId, + collapseKey, + projectGroupDepth: depth, + hostWorktreeCounts: hostMetrics.hostCounts.size > 0 ? hostMetrics.hostCounts : undefined, + hostWorktreeIds: + hostMetrics.hostWorktreeIds.size > 0 ? hostMetrics.hostWorktreeIds : undefined, + hostFolderWorkspaceIds: + hostMetrics.hostFolderWorkspaceIds.size > 0 + ? hostMetrics.hostFolderWorkspaceIds + : undefined, + worktreeIds: hostMetrics.worktreeIds, + folderWorkspaceIds: hostMetrics.folderWorkspaceIds }) - if (!collapsedGroups.has(key)) { - for (const folderWorkspace of folderWorkspacesByProjectGroupId.get(projectGroup.id) ?? []) { + if (!collapsedGroups.has(collapseKey)) { + for (const folderWorkspace of folderWorkspacesByProjectGroupIdentity.get(identity) ?? []) { result.push({ type: 'folder-workspace', - key: `folder-workspace:${folderWorkspace.id}`, + key: getFolderWorkspaceRowKey(folderWorkspace, [projectGroup]), folderWorkspace, projectGroup, depth: 0, @@ -1466,16 +1600,16 @@ export function buildRows( appendProjectGroup(childGroup, depth + 1) } } - groupByProjectGroupId.delete(projectGroup.id) + groupByProjectGroupIdentity.delete(identity) } - for (const projectGroup of childGroupsByParentId.get(null) ?? []) { + for (const projectGroup of childGroupsByParentIdentity.get(null) ?? []) { appendProjectGroup(projectGroup, 0) } - const remainingRepoEntries = [...(groupByProjectGroupId.get(null) ?? [])] - for (const [projectGroupId, entries] of groupByProjectGroupId) { - if (projectGroupId === null || projectGroupsById.has(projectGroupId)) { + const remainingRepoEntries = [...(groupByProjectGroupIdentity.get(null) ?? [])] + for (const [projectGroupIdentity, entries] of groupByProjectGroupIdentity) { + if (projectGroupIdentity === null || projectGroupIndex.byIdentity.has(projectGroupIdentity)) { continue } // Why: startup can have repos from hosts whose project-group metadata was @@ -1541,21 +1675,22 @@ export function getGroupKeysForWorktree( return [groupKey] } const repo = repoMap.get(worktree.repoId) - const groupIds: string[] = [] - const groupsById = new Map(projectGroups.map((group) => [group.id, group])) + const groupKeys: string[] = [] + const projectGroupIndex = buildProjectGroupSidebarIndex(projectGroups) const visited = new Set() - let currentGroupId = repo?.projectGroupId ?? null - while (currentGroupId && !visited.has(currentGroupId)) { - const group = groupsById.get(currentGroupId) - if (!group) { - // Why: repos can arrive before their remote Project Group metadata; reveal - // keys must match the top-level fallback rows buildRows actually renders. + let group = findProjectGroupForRepo(projectGroupIndex, repo, getRepoExecutionHostId(repo ?? {})) + while (group) { + const identity = getProjectGroupSidebarIdentity(group) + if (visited.has(identity)) { break } - visited.add(currentGroupId) - groupIds.unshift(currentGroupId) - const parentId = group.parentGroupId ?? null - currentGroupId = parentId && groupsById.has(parentId) ? parentId : null + visited.add(identity) + const revealOwner = + projectGroups.filter((entry) => entry.id === group!.id).length > 1 + ? getProjectGroupOwnerHostId(group) + : undefined + groupKeys.unshift(getProjectGroupHeaderKey(group.id, revealOwner)) + group = findProjectGroupParentForSidebar(projectGroupIndex, group) } - return [...groupIds.map((id) => getProjectGroupHeaderKey(id)), groupKey] + return [...groupKeys, groupKey] } diff --git a/src/renderer/src/store/slices/project-group-removal-targets.test.ts b/src/renderer/src/store/slices/project-group-removal-targets.test.ts index 0cf5a53ace9..1bff1649342 100644 --- a/src/renderer/src/store/slices/project-group-removal-targets.test.ts +++ b/src/renderer/src/store/slices/project-group-removal-targets.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import type { ProjectGroup, Repo } from '../../../../shared/types' +import type { FolderWorkspace, ProjectGroup, Repo } from '../../../../shared/types' import { selectProjectGroupRemovalTargets } from './project-group-removal-targets' const rootGroup: ProjectGroup = { @@ -41,6 +41,33 @@ function makeRepo(id: string, projectGroupId: string | null): Repo { } } +function withOwner(group: ProjectGroup, executionHostId: string): ProjectGroup { + return { ...group, executionHostId } +} + +function makeFolderWorkspace( + id: string, + projectGroupId: string, + executionHostId: 'local' | `runtime:${string}` +): FolderWorkspace { + return { + id, + projectGroupId, + executionHostId, + name: id, + folderPath: `/${id}`, + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0, + createdAt: 1, + updatedAt: 1 + } +} + describe('selectProjectGroupRemovalTargets', () => { it('selects direct and nested child projects in repo order', () => { const result = selectProjectGroupRemovalTargets( @@ -78,4 +105,65 @@ describe('selectProjectGroupRemovalTargets', () => { expect([...result.deletedGroupIds]).toEqual([]) expect(result.projectIds).toEqual([]) }) + + it('requires an owner when group ids are ambiguous', () => { + const result = selectProjectGroupRemovalTargets( + [withOwner(rootGroup, 'local'), withOwner(rootGroup, 'runtime:env-1')], + [], + rootGroup.id + ) + + expect(result.groupExists).toBe(false) + expect(result.ownerHostId).toBeNull() + }) + + it('does not fall back to a foreign owner when the requested owner is stale', () => { + const result = selectProjectGroupRemovalTargets( + [withOwner(rootGroup, 'local')], + [], + rootGroup.id, + 'runtime:env-1' + ) + + expect(result.groupExists).toBe(false) + expect(result.ownerHostId).toBeNull() + }) + + it('scopes descendants, repos, and folders to the selected owner', () => { + const localRoot = withOwner(rootGroup, 'local') + const localChild = withOwner(childGroup, 'local') + const runtimeRoot = withOwner(rootGroup, 'runtime:env-1') + const runtimeChild = withOwner(childGroup, 'runtime:env-1') + const localRepo: Repo = { + ...makeRepo('same-repo', rootGroup.id), + executionHostId: 'local' + } + const runtimeRepo: Repo = { + ...makeRepo('same-repo', childGroup.id), + executionHostId: 'runtime:env-1' + } + const localFolder = makeFolderWorkspace('same-folder', childGroup.id, 'local') + const runtimeFolder = makeFolderWorkspace('same-folder', childGroup.id, 'runtime:env-1') + + const result = selectProjectGroupRemovalTargets( + [localRoot, localChild, runtimeRoot, runtimeChild], + [localRepo, runtimeRepo], + rootGroup.id, + 'runtime:env-1', + [localFolder, runtimeFolder] + ) + + expect(result.ownerHostId).toBe('runtime:env-1') + expect(result.deletedGroupIdentities).toEqual( + new Set(['["runtime:env-1","root"]', '["runtime:env-1","child"]']) + ) + expect(result.projectTargets).toEqual([ + { + projectId: 'same-repo', + ownerHostId: 'runtime:env-1', + identity: 'runtime:env-1\0same-repo' + } + ]) + expect(result.folderWorkspaceIdentities).toEqual(new Set(['["runtime:env-1","same-folder"]'])) + }) }) 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 33bd8ba121a..1a8b5e0d212 100644 --- a/src/renderer/src/store/slices/project-group-removal-targets.ts +++ b/src/renderer/src/store/slices/project-group-removal-targets.ts @@ -1,37 +1,131 @@ -import type { ProjectGroup, Repo } from '../../../../shared/types' -import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups' +import type { FolderWorkspace, ProjectGroup, Repo } from '../../../../shared/types' +import { + getRepoExecutionHostId, + normalizeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../../../shared/execution-host' +import { + buildProjectGroupOwnerIndex, + getProjectGroupIdentity, + getProjectGroupOwnerHostId, + getProjectGroupOwnerIdentity, + getProjectGroupOwnerSubtreeIdentities, + resolveProjectGroupOwner, + type ProjectGroupOwnerIndex +} from '../../../../shared/project-groups' +import { getRepoHostIdentity } from './repo-host-identity' + +export type ProjectGroupRemovalProjectTarget = { + projectId: string + ownerHostId: ExecutionHostId + identity: string +} export type ProjectGroupRemovalTargets = { groupExists: boolean + ownerHostId: ExecutionHostId | null deletedGroupIds: Set + deletedGroupIdentities: Set projectIds: string[] + projectTargets: ProjectGroupRemovalProjectTarget[] + folderWorkspaceIdentities: Set +} + +function getFolderWorkspacePreferredOwnerHostId( + workspace: Pick +): ExecutionHostId | undefined { + return ( + normalizeExecutionHostId(workspace.executionHostId) ?? + (workspace.connectionId ? toSshExecutionHostId(workspace.connectionId) : undefined) + ) +} + +function resolveFolderWorkspaceProjectGroup( + index: ProjectGroupOwnerIndex, + workspace: Pick +): ProjectGroup | null { + return resolveProjectGroupOwner( + index, + workspace.projectGroupId, + getFolderWorkspacePreferredOwnerHostId(workspace) + ) +} + +export function getProjectGroupRemovalFolderWorkspaceIdentity( + index: ProjectGroupOwnerIndex, + workspace: Pick +): string | null { + const group = resolveFolderWorkspaceProjectGroup(index, workspace) + return group ? JSON.stringify([getProjectGroupOwnerHostId(group), workspace.id]) : null +} + +function emptyRemovalTargets(): ProjectGroupRemovalTargets { + return { + groupExists: false, + ownerHostId: null, + deletedGroupIds: new Set(), + deletedGroupIdentities: new Set(), + projectIds: [], + projectTargets: [], + folderWorkspaceIdentities: new Set() + } } export function selectProjectGroupRemovalTargets( projectGroups: readonly ProjectGroup[], repos: readonly Repo[], - groupId: string + groupId: string, + ownerHostId?: ExecutionHostId, + folderWorkspaces: readonly FolderWorkspace[] = [] ): ProjectGroupRemovalTargets { - const groupExists = projectGroups.some((group) => group.id === groupId) - if (!groupExists) { - return { - groupExists: false, - deletedGroupIds: new Set(), - projectIds: [] - } + const index = buildProjectGroupOwnerIndex(projectGroups) + const rootGroup = resolveProjectGroupOwner(index, groupId, ownerHostId) + if (!rootGroup) { + return emptyRemovalTargets() } - const deletedGroupIds = getProjectGroupSubtreeIds(projectGroups, groupId) - const projectIds: string[] = [] + const selectedOwnerHostId = getProjectGroupOwnerHostId(rootGroup) + const deletedGroupIdentities = getProjectGroupOwnerSubtreeIdentities(projectGroups, rootGroup) + const deletedGroupIds = new Set( + projectGroups.flatMap((group) => + deletedGroupIdentities.has( + getProjectGroupIdentity(group.id, getProjectGroupOwnerHostId(group)) + ) + ? [group.id] + : [] + ) + ) + const projectTargets: ProjectGroupRemovalProjectTarget[] = [] + const seenProjectIdentities = new Set() for (const repo of repos) { - if (repo.projectGroupId && deletedGroupIds.has(repo.projectGroupId)) { - projectIds.push(repo.id) + if (!repo.projectGroupId) { + continue } + const repoOwnerHostId = getRepoExecutionHostId(repo) + const groupIdentity = getProjectGroupIdentity(repo.projectGroupId, repoOwnerHostId) + const identity = getRepoHostIdentity(repo) + if (deletedGroupIdentities.has(groupIdentity) && !seenProjectIdentities.has(identity)) { + seenProjectIdentities.add(identity) + projectTargets.push({ projectId: repo.id, ownerHostId: repoOwnerHostId, identity }) + } + } + const folderWorkspaceIdentities = new Set() + for (const workspace of folderWorkspaces) { + const group = resolveFolderWorkspaceProjectGroup(index, workspace) + if (!group || !deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group))) { + continue + } + folderWorkspaceIdentities.add(JSON.stringify([getProjectGroupOwnerHostId(group), workspace.id])) } return { groupExists: true, + ownerHostId: selectedOwnerHostId, deletedGroupIds, - projectIds + deletedGroupIdentities, + projectIds: projectTargets.map((target) => target.projectId), + projectTargets, + folderWorkspaceIdentities } } diff --git a/src/renderer/src/store/slices/repos-project-group-owner-update.test.ts b/src/renderer/src/store/slices/repos-project-group-owner-update.test.ts new file mode 100644 index 00000000000..2843aa7f5da --- /dev/null +++ b/src/renderer/src/store/slices/repos-project-group-owner-update.test.ts @@ -0,0 +1,124 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ProjectGroup } from '../../../../shared/types' +import { createTestStore } from './store-test-helpers' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '../../runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' + +function group(executionHostId: 'local' | `runtime:${string}`): ProjectGroup { + return { + id: 'same-id', + name: executionHostId, + parentPath: null, + executionHostId, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } +} + +describe('project-group owner-qualified updates', () => { + beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('updates only the routed owner when ids overlap', async () => { + const local = group('local') + const runtime = group('runtime:env-1') + const runtimeCall = vi.fn((request: RuntimeEnvironmentCallRequest) => { + const compatibility = createCompatibleRuntimeStatusResponseIfNeeded(request) + if (compatibility) { + return compatibility + } + return { + id: 'rpc-project-group-update', + ok: true, + result: { group: { ...runtime, tabOrder: 4 } }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { call: runtimeCall } + } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [local, runtime] + }) + + await store + .getState() + .updateProjectGroup('same-id', { tabOrder: 4 }, { ownerHostId: 'runtime:env-1' }) + + expect(store.getState().projectGroups).toEqual([local, { ...runtime, tabOrder: 4 }]) + expect(runtimeCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'projectGroup.update', + params: { groupId: 'same-id', updates: { tabOrder: 4 } }, + timeoutMs: 15_000 + }) + }) + + it('routes local updates by owner while a runtime is focused', async () => { + const local = group('local') + const runtime = group('runtime:env-1') + const update = vi.fn().mockResolvedValue({ ...local, name: 'renamed' }) + vi.stubGlobal('window', { + api: { + projectGroups: { update }, + runtimeEnvironments: { call: vi.fn() } + } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + projectGroups: [local, runtime] + }) + + await store + .getState() + .updateProjectGroup('same-id', { name: 'renamed' }, { ownerHostId: 'local' }) + + expect(update).toHaveBeenCalledWith({ + groupId: 'same-id', + updates: { name: 'renamed' }, + ownerHostId: 'local' + }) + expect(store.getState().projectGroups).toEqual([{ ...local, name: 'renamed' }, runtime]) + }) + + it('does not update when an omitted owner is ambiguous', async () => { + const local = group('local') + const runtime = group('runtime:env-1') + const runtimeCall = vi.fn() + const update = vi.fn() + vi.stubGlobal('window', { + api: { + projectGroups: { update }, + runtimeEnvironments: { call: runtimeCall } + } + }) + const store = createTestStore() + store.setState({ projectGroups: [local, runtime] }) + + await expect(store.getState().updateProjectGroup('same-id', { tabOrder: 4 })).resolves.toBe( + false + ) + + expect(update).not.toHaveBeenCalled() + expect(runtimeCall).not.toHaveBeenCalled() + expect(store.getState().projectGroups).toEqual([local, runtime]) + }) +}) diff --git a/src/renderer/src/store/slices/repos-project-group-path-status-perf.test.ts b/src/renderer/src/store/slices/repos-project-group-path-status-perf.test.ts new file mode 100644 index 00000000000..bfc22e2ed70 --- /dev/null +++ b/src/renderer/src/store/slices/repos-project-group-path-status-perf.test.ts @@ -0,0 +1,58 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ProjectGroup, Repo } from '../../../../shared/types' +import { createTestStore } from './store-test-helpers' + +function group(index: number): ProjectGroup { + const ownerIndex = index % 16 + return { + id: index < 16 ? 'same-id' : `group-${index}`, + name: `Group ${index}`, + parentPath: ownerIndex === 0 && index === 0 ? '/workspace' : null, + executionHostId: ownerIndex === 0 ? 'local' : `runtime:env-${ownerIndex}`, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: index, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } +} + +describe('project-group path-status catalog cache', () => { + afterEach(() => vi.unstubAllGlobals()) + + it('reuses the owner-scoped snapshot across 12,208 groups and 16 owners', async () => { + let pathReads = 0 + const projectGroups = Array.from({ length: 12_208 }, (_, index) => group(index)) + const repos = Array.from({ length: 12_208 }, (_, index) => { + const ownerIndex = index % 16 + return { + id: `repo-${index}`, + get path() { + pathReads += 1 + return `/workspace/repo-${index}` + }, + displayName: `Repo ${index}`, + badgeColor: '#000', + addedAt: 1, + projectGroupId: ownerIndex === 0 ? 'same-id' : null, + executionHostId: ownerIndex === 0 ? 'local' : `runtime:env-${ownerIndex}` + } as Repo + }) + const getPathStatus = vi.fn().mockResolvedValue({ path: '/workspace', exists: true }) + vi.stubGlobal('window', { api: { folderWorkspaces: { getPathStatus } } }) + const store = createTestStore() + store.setState({ projectGroups, repos }) + const request = { scope: 'project-group' as const, projectGroupId: 'same-id' } + const options = { force: true, runtimeEnvironmentId: null, ownerHostId: 'local' as const } + + await store.getState().fetchFolderWorkspacePathStatus(request, options) + const readsAfterFirstSnapshot = pathReads + await store.getState().fetchFolderWorkspacePathStatus(request, options) + + expect(readsAfterFirstSnapshot).toBeGreaterThan(0) + expect(pathReads).toBe(readsAfterFirstSnapshot) + expect(getPathStatus).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/store/slices/repos-project-groups-delete.test.ts b/src/renderer/src/store/slices/repos-project-groups-delete.test.ts index b1692509c00..1773c766cce 100644 --- a/src/renderer/src/store/slices/repos-project-groups-delete.test.ts +++ b/src/renderer/src/store/slices/repos-project-groups-delete.test.ts @@ -6,6 +6,7 @@ import { type RuntimeEnvironmentCallRequest } from '../../runtime/runtime-compatibility-test-fixture' import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' +import { getRepoExecutionHostId } from '../../../../shared/execution-host' const remoteRepo: Repo = { id: 'remote-repo', @@ -110,17 +111,22 @@ describe('project group deletion store routing', () => { result: { deleted: false }, _meta: { runtimeId: 'runtime-remote' } }) - const groupedRepo = { ...remoteRepo, projectGroupId: projectGroup.id } + const runtimeGroup = { ...projectGroup, executionHostId: 'runtime:env-1' } + const groupedRepo: Repo = { + ...remoteRepo, + projectGroupId: projectGroup.id, + executionHostId: 'runtime:env-1' + } const store = createTestStore() store.setState({ settings: { activeRuntimeEnvironmentId: 'env-1' } as never, - projectGroups: [projectGroup], + projectGroups: [runtimeGroup], repos: [groupedRepo] }) await expect(store.getState().deleteProjectGroup(projectGroup.id)).resolves.toBe(false) - expect(store.getState().projectGroups).toEqual([projectGroup]) + expect(store.getState().projectGroups).toEqual([runtimeGroup]) expect(store.getState().repos).toEqual([groupedRepo]) expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ selector: 'env-1', @@ -131,6 +137,85 @@ describe('project group deletion store routing', () => { expect(projectGroupsDelete).not.toHaveBeenCalled() }) + it('deletes and cleans up only the selected owner when ids overlap', async () => { + const localGroup = { ...projectGroup, executionHostId: 'local' } + const runtimeGroup = { ...projectGroup, executionHostId: 'runtime:env-1' } + const localFolder: FolderWorkspace = { + id: 'same-folder', + projectGroupId: projectGroup.id, + executionHostId: 'local', + name: 'Local folder', + folderPath: '/local', + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0, + createdAt: 1, + updatedAt: 1 + } + const runtimeFolder = { + ...localFolder, + name: 'Runtime folder', + folderPath: '/runtime', + executionHostId: 'runtime:env-1' as const + } + const localRepo: Repo = { + ...remoteRepo, + id: 'same-repo', + projectGroupId: projectGroup.id, + executionHostId: 'local' + } + const runtimeRepo: Repo = { + ...localRepo, + path: '/runtime-repo', + executionHostId: 'runtime:env-1' + } + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-delete-group', + ok: true, + result: { deleted: true }, + _meta: { runtimeId: 'runtime-remote' } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + projectGroups: [localGroup, runtimeGroup], + folderWorkspaces: [localFolder, runtimeFolder], + repos: [localRepo, runtimeRepo] + }) + + await expect( + store.getState().deleteProjectGroup(projectGroup.id, { ownerHostId: 'runtime:env-1' }) + ).resolves.toBe(true) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'projectGroup.delete', + params: { groupId: projectGroup.id }, + timeoutMs: 15_000 + }) + expect(store.getState().projectGroups).toEqual([localGroup]) + expect(store.getState().folderWorkspaces).toEqual([localFolder]) + expect(store.getState().repos).toEqual([localRepo, { ...runtimeRepo, projectGroupId: null }]) + expect(projectGroupsDelete).not.toHaveBeenCalled() + }) + + it('does not delete when an omitted owner is ambiguous', async () => { + const localGroup = { ...projectGroup, executionHostId: 'local' } + const runtimeGroup = { ...projectGroup, executionHostId: 'runtime:env-1' } + const store = createTestStore() + store.setState({ projectGroups: [localGroup, runtimeGroup] }) + + await expect(store.getState().deleteProjectGroup(projectGroup.id)).resolves.toBe(false) + + expect(projectGroupsDelete).not.toHaveBeenCalled() + expect(runtimeEnvironmentTransportCall).not.toHaveBeenCalled() + expect(store.getState().projectGroups).toEqual([localGroup, runtimeGroup]) + }) + it('deletes only the group when contained project removal is not requested', async () => { projectGroupsDelete.mockResolvedValue(true) const groupedRepo = { ...remoteRepo, id: 'direct', projectGroupId: projectGroup.id } @@ -191,6 +276,59 @@ describe('project group deletion store routing', () => { expect(store.getState().repos).toEqual([siblingRepo]) }) + it('removes contained projects through the selected repo owner', async () => { + const localGroup = { ...projectGroup, executionHostId: 'local' } + const runtimeGroup = { ...projectGroup, executionHostId: 'runtime:env-1' } + const localRepo: Repo = { + ...remoteRepo, + id: 'same-repo', + projectGroupId: projectGroup.id, + executionHostId: 'local' + } + const runtimeRepo: Repo = { + ...localRepo, + path: '/runtime-repo', + executionHostId: 'runtime:env-1' + } + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-delete-group', + ok: true, + result: { deleted: true }, + _meta: { runtimeId: 'runtime-remote' } + }) + const store = createTestStore() + const removeProject = vi.fn( + async (projectId: string, options?: { hostId?: string }): Promise => { + store.setState((state) => ({ + repos: state.repos.filter( + (repo) => repo.id !== projectId || getRepoExecutionHostId(repo) !== options?.hostId + ) + })) + } + ) + store.setState({ + projectGroups: [localGroup, runtimeGroup], + repos: [localRepo, runtimeRepo], + removeProject + }) + + await expect( + store.getState().deleteProjectGroupWithContainedProjects(projectGroup.id, { + removeContainedProjects: true, + ownerHostId: 'runtime:env-1' + }) + ).resolves.toEqual({ + status: 'deleted-group', + groupId: projectGroup.id, + requestedProjectIds: ['same-repo'], + removedProjectIds: ['same-repo'], + failedProjectRemovals: [] + }) + + expect(removeProject).toHaveBeenCalledWith('same-repo', { hostId: 'runtime:env-1' }) + expect(store.getState().repos).toEqual([localRepo]) + }) + it('does not remove contained projects when group deletion fails', async () => { projectGroupsDelete.mockResolvedValue(false) const groupedRepo = { ...remoteRepo, id: 'direct', projectGroupId: projectGroup.id } diff --git a/src/renderer/src/store/slices/repos-same-id-folder-workspace-identity.test.ts b/src/renderer/src/store/slices/repos-same-id-folder-workspace-identity.test.ts new file mode 100644 index 00000000000..2d467f62aba --- /dev/null +++ b/src/renderer/src/store/slices/repos-same-id-folder-workspace-identity.test.ts @@ -0,0 +1,237 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { FolderWorkspace, ProjectGroup } from '../../../../shared/types' +import { folderWorkspaceKey } from '../../../../shared/workspace-scope' +import { createTestStore } from './store-test-helpers' + +function makeGroup(overrides: Partial = {}): ProjectGroup { + return { + id: overrides.id ?? 'same-group', + name: overrides.name ?? 'Same Group', + parentPath: overrides.parentPath ?? '/workspace', + connectionId: overrides.connectionId ?? null, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1, + ...(overrides.executionHostId ? { executionHostId: overrides.executionHostId } : {}) + } +} + +function makeFolder(overrides: Partial = {}): FolderWorkspace { + return { + id: overrides.id ?? 'same-id', + projectGroupId: overrides.projectGroupId ?? 'same-group', + name: overrides.name ?? 'Workspace', + folderPath: overrides.folderPath ?? '/workspace/folder', + connectionId: overrides.connectionId ?? null, + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 1, + createdAt: 1, + updatedAt: 1, + ...(overrides.executionHostId ? { executionHostId: overrides.executionHostId } : {}) + } +} + +describe('same-id cross-host folder workspace identity', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('deletes only the owner-qualified folder row and its session keys', async () => { + const local = makeFolder({ + name: 'Local', + folderPath: '/local/folder', + connectionId: null, + executionHostId: 'local' + }) + const remote = makeFolder({ + name: 'Remote', + folderPath: '/remote/folder', + connectionId: null, + executionHostId: 'runtime:env-1' + }) + const store = createTestStore() + const purgeWorktreeTerminalState = vi.fn() + store.setState({ + projectGroups: [ + makeGroup({ executionHostId: 'local', parentPath: '/local' }), + makeGroup({ executionHostId: 'runtime:env-1', parentPath: '/remote' }) + ], + folderWorkspaces: [local, remote], + folderWorkspacePathStatuses: { + 'local:folder-workspace:local:same-id': { + status: { path: '/local/folder', exists: true }, + checkedAt: 1, + requestSnapshot: 'local' + }, + 'local:folder-workspace:runtime:env-1:same-id': { + status: { path: '/remote/folder', exists: true }, + checkedAt: 1, + requestSnapshot: 'remote' + } + }, + purgeWorktreeTerminalState + }) + + vi.stubGlobal('window', { + api: { + folderWorkspaces: { + delete: vi.fn().mockResolvedValue(true) + } + } + }) + + await expect( + store.getState().deleteFolderWorkspace('same-id', { ownerHostId: 'local' }) + ).resolves.toBe(true) + + const remaining = store.getState().folderWorkspaces + expect(remaining).toHaveLength(1) + expect(remaining[0]?.executionHostId).toBe('runtime:env-1') + expect(purgeWorktreeTerminalState).toHaveBeenCalledWith([ + folderWorkspaceKey('same-id', 'local') + ]) + expect(Object.keys(store.getState().folderWorkspacePathStatuses)).toEqual([ + 'local:folder-workspace:runtime:env-1:same-id' + ]) + }) + + it('keeps path-status cache keys independent per owner for same folder id', () => { + const store = createTestStore() + const localKey = store + .getState() + .getFolderWorkspacePathStatusCacheKey( + { scope: 'folder-workspace', folderWorkspaceId: 'same-id' }, + { ownerHostId: 'local' } + ) + const remoteKey = store + .getState() + .getFolderWorkspacePathStatusCacheKey( + { scope: 'folder-workspace', folderWorkspaceId: 'same-id' }, + { ownerHostId: 'runtime:env-1' } + ) + expect(localKey).not.toBe(remoteKey) + expect(localKey).toContain('local:') + expect(localKey).toContain('folder-workspace:same-id') + expect(remoteKey).toContain('runtime:env-1:') + expect(remoteKey).toContain('folder-workspace:same-id') + }) + + it('migrates legacy bare folder session tabs for the default owner on activation', () => { + const store = createTestStore() + const bareKey = folderWorkspaceKey('folder-workspace-1') + const qualifiedKey = folderWorkspaceKey('folder-workspace-1', 'local') + const folder = makeFolder({ + id: 'folder-workspace-1', + projectGroupId: 'group-a', + connectionId: null, + executionHostId: 'local', + folderPath: '/local/folder' + }) + store.setState({ + projectGroups: [makeGroup({ id: 'group-a', executionHostId: 'local', parentPath: '/local' })], + folderWorkspaces: [folder], + // Why: reconnectable pty keeps the tab out of orphan cleanup during activation reconcile. + tabsByWorktree: { + [bareKey]: [ + { + id: 'legacy-tab', + ptyId: 'pty-legacy', + worktreeId: bareKey, + title: 'Legacy', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + ptyIdsByTabId: { 'legacy-tab': ['pty-legacy'] }, + activeTabIdByWorktree: { [bareKey]: 'legacy-tab' } + }) + + store.getState().setActiveFolderWorkspace('folder-workspace-1', 'local') + + const state = store.getState() + expect(state.activeWorkspaceKey).toBe(qualifiedKey) + expect(state.activeWorktreeId).toBe(qualifiedKey) + expect(state.tabsByWorktree[qualifiedKey]?.[0]?.id).toBe('legacy-tab') + expect(state.tabsByWorktree[bareKey]).toBeUndefined() + expect(state.activeTabIdByWorktree[qualifiedKey]).toBe('legacy-tab') + expect(state.activeTabId).toBe('legacy-tab') + }) + + it('keeps owner-distinct same-id session tabs isolated across hosts', () => { + const store = createTestStore() + const localKey = folderWorkspaceKey('same-id', 'local') + const remoteKey = folderWorkspaceKey('same-id', 'runtime:env-1') + store.setState({ + projectGroups: [ + makeGroup({ id: 'same-group', executionHostId: 'local', parentPath: '/local' }), + makeGroup({ id: 'same-group', executionHostId: 'runtime:env-1', parentPath: '/remote' }) + ], + folderWorkspaces: [ + makeFolder({ + id: 'same-id', + name: 'Local', + executionHostId: 'local', + folderPath: '/local/folder' + }), + makeFolder({ + id: 'same-id', + name: 'Remote', + executionHostId: 'runtime:env-1', + folderPath: '/remote/folder' + }) + ], + tabsByWorktree: { + [localKey]: [ + { + id: 'local-tab', + ptyId: 'pty-local', + worktreeId: localKey, + title: 'Local', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ], + [remoteKey]: [ + { + id: 'remote-tab', + ptyId: 'pty-remote', + worktreeId: remoteKey, + title: 'Remote', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + ptyIdsByTabId: { + 'local-tab': ['pty-local'], + 'remote-tab': ['pty-remote'] + } + }) + + store.getState().setActiveFolderWorkspace('same-id', 'local') + expect(store.getState().activeWorkspaceKey).toBe(localKey) + expect(store.getState().tabsByWorktree[localKey]?.[0]?.id).toBe('local-tab') + expect(store.getState().tabsByWorktree[remoteKey]?.[0]?.id).toBe('remote-tab') + + store.getState().setActiveFolderWorkspace('same-id', 'runtime:env-1') + expect(store.getState().activeWorkspaceKey).toBe(remoteKey) + expect(store.getState().tabsByWorktree[localKey]?.[0]?.id).toBe('local-tab') + expect(store.getState().tabsByWorktree[remoteKey]?.[0]?.id).toBe('remote-tab') + }) +}) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index f8fa9ebc61e..1b038ff1729 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -43,10 +43,21 @@ import { isGitRepoKind } from '../../../../shared/repo-kind' import { sanitizeRepoIcon } from '../../../../shared/repo-icon' import { normalizeRepoBadgeColor } from '../../../../shared/repo-badge-color' import { applyManualRepoOrder, getManualRepoOrder } from '../../../../shared/manual-repo-order' -import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups' +import { + buildProjectGroupOwnerIndex, + getFolderWorkspaceProjectGroupOwnerHostId, + getProjectGroupOwnerHostId, + getProjectGroupOwnerIdentity, + getProjectGroupSubtreeIds, + resolveFolderWorkspaceProjectGroup, + resolveProjectGroupOwner +} from '../../../../shared/project-groups' import { isPathInsideOrEqual } from '../../../../shared/cross-platform-path' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id' -import { selectProjectGroupRemovalTargets } from './project-group-removal-targets' +import { + getProjectGroupRemovalFolderWorkspaceIdentity, + selectProjectGroupRemovalTargets +} from './project-group-removal-targets' import { reconcileFetchedRepos } from './repo-identity-reconcile' import { mergeSshRepoReadoptions, @@ -87,7 +98,16 @@ import { } from '../../../../shared/execution-host' import { isRemovedRuntimeHostId } from './stale-runtime-host-rows' import { cleanupEphemeralVmRuntimesForDeleted } from '@/lib/ephemeral-vm-runtime-cleanup' -import { folderWorkspaceKey, parseWorkspaceKey } from '../../../../shared/workspace-scope' +import { + folderWorkspaceKey, + folderWorkspaceSessionKeys, + parseWorkspaceKey +} from '../../../../shared/workspace-scope' +import { + getFolderWorkspaceCatalogOwnerHostId, + getFolderWorkspaceIdentity, + getFolderWorkspaceOwnerIdentity +} from '../../../../shared/folder-workspaces' import { formatFolderWorkspaceCreateError } from '../../lib/folder-workspace-path-status' import { getEnvironmentSshStateGeneration } from './runtime-environment-ssh' import { getRuntimeEnvironmentConnectionGeneration } from './runtime-status' @@ -202,6 +222,7 @@ export type FolderWorkspacePathStatusCacheEntry = { export type DeleteProjectGroupWithContainedProjectsOptions = { removeContainedProjects: boolean + ownerHostId?: ExecutionHostId } type AllHostCatalogFetchOptions = { @@ -319,6 +340,15 @@ function getProjectSetupRuntimeTarget( : { kind: 'local' } } +function getProjectGroupRuntimeTarget( + ownerHostId: ExecutionHostId +): ReturnType { + const parsedHost = parseExecutionHostId(ownerHostId) + return parsedHost?.kind === 'runtime' + ? { kind: 'environment', environmentId: parsedHost.environmentId } + : { kind: 'local' } +} + function getProjectUpdateRuntimeTarget( state: AppState, projectId: string @@ -953,15 +983,8 @@ 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]) + return getProjectGroupOwnerIdentity(group) } function catalogOwnsHost(catalogHostId: string, rowHostId: string): boolean { @@ -978,7 +1001,7 @@ function mergeFetchedProjectGroupsForHost( ): ProjectGroup[] { const fetchedIdentities = new Set(fetched.map(getProjectGroupHostIdentity)) const preserved = previous.filter((group) => { - const existingHostId = getProjectGroupHostId(group) + const existingHostId = getProjectGroupOwnerHostId(group) return ( !catalogOwnsHost(hostId, existingHostId) || fetchedIdentities.has(getProjectGroupHostIdentity(group)) @@ -1001,7 +1024,7 @@ function getFolderWorkspaceHostId( const matchingHosts = new Set( projectGroups .filter((group) => group.id === workspace.projectGroupId) - .map(getProjectGroupHostId) + .map(getProjectGroupOwnerHostId) ) return matchingHosts.size === 1 ? ([...matchingHosts][0] as ExecutionHostId) @@ -1209,7 +1232,15 @@ function clearRestoredFolderWorkspaceSessionOwners( next[key] = hostId continue } - const workspace = state.folderWorkspaces.find((entry) => entry.id === scope.folderWorkspaceId) + const workspace = state.folderWorkspaces.find((entry) => { + if (entry.id !== scope.folderWorkspaceId) { + return false + } + if (!scope.ownerHostId) { + return true + } + return getFolderWorkspaceCatalogOwnerHostId(entry, state.projectGroups) === scope.ownerHostId + }) if (workspace && !state.projectGroups.some((group) => group.id === workspace.projectGroupId)) { // Why: ownership resolves via the project group; if that catalog is still missing, keep the restored host owner so a session write doesn't move runtime tabs local. next[key] = hostId @@ -1346,7 +1377,17 @@ async function reconcileFailedFolderWorkspaceUpdate(args: { : {}) })) if (!refreshed) { - args.get().purgeWorktreeTerminalState([folderWorkspaceKey(args.folderWorkspaceId)]) + const remainingSameId = args + .get() + .folderWorkspaces.some((workspace) => workspace.id === args.folderWorkspaceId) + args.get().purgeWorktreeTerminalState( + remainingSameId + ? [folderWorkspaceKey(args.folderWorkspaceId, args.ownerHostId)] + : folderWorkspaceSessionKeys({ + folderWorkspaceId: args.folderWorkspaceId, + ownerHostId: args.ownerHostId + }) + ) } } catch (err) { console.warn('Failed to reconcile folder workspace after update failure:', err) @@ -1391,12 +1432,16 @@ function settingsForRepoOwner( function getFolderWorkspacePathStatusScopeKey(request: FolderWorkspacePathStatusRequest): string { if (request.scope === 'project-group') { - return `project-group:${request.projectGroupId}` + return request.ownerHostId + ? `project-group:${request.ownerHostId}:${request.projectGroupId}` + : `project-group:${request.projectGroupId}` } if (request.scope === 'path') { return `path:${request.connectionId ?? ''}:${request.path}` } - return `folder-workspace:${request.folderWorkspaceId}` + return request.ownerHostId + ? `folder-workspace:${request.ownerHostId}:${request.folderWorkspaceId}` + : `folder-workspace:${request.folderWorkspaceId}` } function getRuntimeTargetCachePrefix( @@ -1406,7 +1451,10 @@ function getRuntimeTargetCachePrefix( return target.kind === 'local' ? 'local' : `environment:${target.environmentId}` } -type FolderWorkspacePathStatusRouteOptions = { runtimeEnvironmentId?: string | null } +type FolderWorkspacePathStatusRouteOptions = { + runtimeEnvironmentId?: string | null + ownerHostId?: ExecutionHostId +} type AddRepoPathRouteOptions = { runtimeEnvironmentId?: string | null } type RuntimeCatalogFetchOptions = { runtimeEnvironmentId?: string | null } @@ -1526,17 +1574,30 @@ function getFolderWorkspaceStatusRequestSnapshot( ) } + const catalog = getFolderPathStatusCatalog(state.projectGroups, state.repos) + const projectGroupIndex = catalog.projectGroupIndex const scope = request.scope === 'project-group' - ? state.projectGroups.find((group) => group.id === request.projectGroupId) - : state.folderWorkspaces.find((workspace) => workspace.id === request.folderWorkspaceId) + ? resolveProjectGroupOwner(projectGroupIndex, request.projectGroupId, request.ownerHostId) + : state.folderWorkspaces.find((workspace) => { + if (workspace.id !== request.folderWorkspaceId) { + return false + } + if (!request.ownerHostId) { + return true + } + return ( + getFolderWorkspaceCatalogOwnerHostId(workspace, state.projectGroups) === + request.ownerHostId + ) + }) const projectGroup = request.scope === 'project-group' ? scope && 'parentPath' in scope ? scope : null : scope && 'projectGroupId' in scope - ? state.projectGroups.find((group) => group.id === scope.projectGroupId) + ? resolveFolderWorkspaceProjectGroup(projectGroupIndex, scope) : null const folderPath = request.scope === 'project-group' @@ -1563,11 +1624,29 @@ function getFolderWorkspaceStatusRequestSnapshot( if (!folderPath || !projectGroupId) { return null } - const groupIds = getProjectGroupSubtreeIds(state.projectGroups, projectGroupId) + const cachedBase = + request.scope === 'project-group' && projectGroup + ? getProjectGroupPathStatusSnapshotBase(catalog, projectGroup) + : scope && 'folderPath' in scope && projectGroup + ? getFolderWorkspacePathStatusSnapshotBase(catalog, scope, projectGroup) + : null + if (cachedBase) { + return getFolderPathStatusSnapshotWithSshState(cachedBase, state.sshConnectionStates) + } + const ownerHostId = projectGroup + ? request.scope === 'folder-workspace' && scope && 'projectGroupId' in scope + ? getFolderWorkspaceProjectGroupOwnerHostId(scope, projectGroupIndex) + : getProjectGroupOwnerHostId(projectGroup) + : null + const ownerGroups = ownerHostId + ? state.projectGroups.filter((group) => getProjectGroupOwnerHostId(group) === ownerHostId) + : state.projectGroups + const groupIds = getProjectGroupSubtreeIds(ownerGroups, projectGroupId) const candidateRepos = state.repos.filter( (repo) => - (typeof repo.projectGroupId === 'string' && groupIds.has(repo.projectGroupId)) || - isPathInsideOrEqual(folderPath, repo.path) + (!ownerHostId || getRepoExecutionHostId(repo) === ownerHostId) && + ((typeof repo.projectGroupId === 'string' && groupIds.has(repo.projectGroupId)) || + isPathInsideOrEqual(folderPath, repo.path)) ) const relevantConnectionIds = new Set() if (scopeConnectionId) { @@ -1600,6 +1679,164 @@ function getFolderWorkspaceStatusRequestSnapshot( ].join('\0') } +type FolderPathStatusSnapshotBase = { + folderPath: string + projectGroupId: string + scopeConnectionId: string + relevantConnectionIds: readonly string[] + repoFingerprint: string +} + +type FolderPathStatusCatalog = { + projectGroupIndex: ReturnType + groupsByOwner: ReadonlyMap + reposByOwner: ReadonlyMap + groupSnapshots: Map + workspaceSnapshots: WeakMap +} + +const folderPathStatusCatalogs = new WeakMap< + readonly ProjectGroup[], + WeakMap +>() + +function getFolderPathStatusCatalog( + projectGroups: readonly ProjectGroup[], + repos: readonly Repo[] +): FolderPathStatusCatalog { + let byRepos = folderPathStatusCatalogs.get(projectGroups) + if (!byRepos) { + byRepos = new WeakMap() + folderPathStatusCatalogs.set(projectGroups, byRepos) + } + const existing = byRepos.get(repos) + if (existing) { + return existing + } + const groupsByOwner = new Map() + for (const group of projectGroups) { + const ownerHostId = getProjectGroupOwnerHostId(group) + const ownerGroups = groupsByOwner.get(ownerHostId) ?? [] + ownerGroups.push(group) + groupsByOwner.set(ownerHostId, ownerGroups) + } + const reposByOwner = new Map() + for (const repo of repos) { + const ownerHostId = getRepoExecutionHostId(repo) + const ownerRepos = reposByOwner.get(ownerHostId) ?? [] + ownerRepos.push(repo) + reposByOwner.set(ownerHostId, ownerRepos) + } + const catalog: FolderPathStatusCatalog = { + projectGroupIndex: buildProjectGroupOwnerIndex(projectGroups), + groupsByOwner, + reposByOwner, + groupSnapshots: new Map(), + workspaceSnapshots: new WeakMap() + } + byRepos.set(repos, catalog) + return catalog +} + +function buildFolderPathStatusSnapshotBase( + catalog: FolderPathStatusCatalog, + projectGroup: ProjectGroup, + folderPath: string, + scopeConnectionId: string | null | undefined +): FolderPathStatusSnapshotBase { + const ownerHostId = getProjectGroupOwnerHostId(projectGroup) + const groupIds = getProjectGroupSubtreeIds( + catalog.groupsByOwner.get(ownerHostId) ?? [], + projectGroup.id + ) + const candidateRepos = (catalog.reposByOwner.get(ownerHostId) ?? []).filter( + (repo) => + (typeof repo.projectGroupId === 'string' && groupIds.has(repo.projectGroupId)) || + isPathInsideOrEqual(folderPath, repo.path) + ) + const relevantConnectionIds = new Set() + if (scopeConnectionId) { + relevantConnectionIds.add(scopeConnectionId) + } + for (const repo of candidateRepos) { + if (repo.connectionId) { + relevantConnectionIds.add(repo.connectionId) + } + } + return { + folderPath, + projectGroupId: projectGroup.id, + scopeConnectionId: scopeConnectionId ?? '', + relevantConnectionIds: [...relevantConnectionIds].sort(), + repoFingerprint: candidateRepos + .map( + (repo) => `${repo.id}:${repo.path}:${repo.projectGroupId ?? ''}:${repo.connectionId ?? ''}` + ) + .sort() + .join('|') + } +} + +function getProjectGroupPathStatusSnapshotBase( + catalog: FolderPathStatusCatalog, + projectGroup: ProjectGroup +): FolderPathStatusSnapshotBase | null { + if (!projectGroup.parentPath) { + return null + } + const identity = getProjectGroupOwnerIdentity(projectGroup) + const existing = catalog.groupSnapshots.get(identity) + if (existing) { + return existing + } + const snapshot = buildFolderPathStatusSnapshotBase( + catalog, + projectGroup, + projectGroup.parentPath, + projectGroup.connectionId + ) + catalog.groupSnapshots.set(identity, snapshot) + return snapshot +} + +function getFolderWorkspacePathStatusSnapshotBase( + catalog: FolderPathStatusCatalog, + workspace: FolderWorkspace, + projectGroup: ProjectGroup +): FolderPathStatusSnapshotBase { + const existing = catalog.workspaceSnapshots.get(workspace) + if (existing) { + return existing + } + const snapshot = buildFolderPathStatusSnapshotBase( + catalog, + projectGroup, + workspace.folderPath, + workspace.connectionId ?? projectGroup.connectionId + ) + catalog.workspaceSnapshots.set(workspace, snapshot) + return snapshot +} + +function getFolderPathStatusSnapshotWithSshState( + base: FolderPathStatusSnapshotBase, + sshConnectionStates: AppState['sshConnectionStates'] +): string { + const sshFingerprint = base.relevantConnectionIds + .map( + (connectionId) => + `${connectionId}:${sshConnectionStates.get(connectionId)?.status ?? 'missing'}` + ) + .join('|') + return [ + base.folderPath, + base.projectGroupId, + base.scopeConnectionId, + sshFingerprint, + base.repoFingerprint + ].join('\0') +} + function getFreshFolderWorkspacePathStatusFromCache(args: { entry: FolderWorkspacePathStatusCacheEntry | undefined requestSnapshot: string | null @@ -1611,6 +1848,19 @@ function getFreshFolderWorkspacePathStatusFromCache(args: { return Date.now() - entry.checkedAt < FOLDER_WORKSPACE_PATH_STATUS_TTL_MS ? entry.status : null } +function getOwnerQualifiedFolderWorkspacePathStatusRequest( + request: FolderWorkspacePathStatusRequest, + options: FolderWorkspacePathStatusRouteOptions | undefined +): FolderWorkspacePathStatusRequest { + if ( + (request.scope === 'project-group' || request.scope === 'folder-workspace') && + options?.ownerHostId + ) { + return { ...request, ownerHostId: options.ownerHostId } + } + return request +} + function getFolderWorkspacePathStatusRequestSnapshotForRead( state: AppState, request: FolderWorkspacePathStatusRequest @@ -1673,7 +1923,10 @@ export type RepoSlice = { runtimeEnvironmentId?: string | null mode: 'group' | 'separate' }) => Promise - createProjectGroup: (name: string) => Promise + createProjectGroup: ( + name: string, + options?: { ownerHostId?: ExecutionHostId } + ) => Promise createFolderWorkspace: ( args: { projectGroupId: string @@ -1704,12 +1957,19 @@ export type RepoSlice = { updates: FolderWorkspaceUpdates, options?: { executionHostId?: ExecutionHostId } ) => Promise - deleteFolderWorkspace: (folderWorkspaceId: string) => Promise + deleteFolderWorkspace: ( + folderWorkspaceId: string, + options?: { ownerHostId?: ExecutionHostId } + ) => Promise updateProjectGroup: ( groupId: string, - updates: Partial> + updates: Partial>, + options?: { ownerHostId?: ExecutionHostId } + ) => Promise + deleteProjectGroup: ( + groupId: string, + options?: { ownerHostId?: ExecutionHostId } ) => Promise - deleteProjectGroup: (groupId: string) => Promise deleteProjectGroupWithContainedProjects: ( groupId: string, options: DeleteProjectGroupWithContainedProjectsOptions @@ -1717,7 +1977,8 @@ export type RepoSlice = { moveProjectToGroup: ( projectId: string, groupId: string | null, - order?: number + order?: number, + options?: { ownerHostId?: ExecutionHostId } ) => Promise // options.hostId disambiguates which host's row to remove when the id exists on multiple hosts; else the focused host is assumed. // options.errorFeedback defaults to 'silent' so bulk/background callers keep their own aggregate reporting. @@ -2335,19 +2596,21 @@ export const createRepoSlice: StateCreator = (set, getFolderWorkspacePathStatusCacheKey: (request, options) => `${getRuntimeTargetCachePrefix( getFolderWorkspacePathStatusRouteSettings(options, get().settings) - )}:${getFolderWorkspacePathStatusScopeKey(request)}`, + )}:${options?.ownerHostId ? `${options.ownerHostId}:` : ''}${getFolderWorkspacePathStatusScopeKey(request)}`, getFreshFolderWorkspacePathStatus: (request, options) => { const state = get() const cacheKey = get().getFolderWorkspacePathStatusCacheKey(request, options) const cached = state.folderWorkspacePathStatuses[cacheKey] - const requestSnapshot = getFolderWorkspacePathStatusRequestSnapshotForRead(state, request) + const ownedRequest = getOwnerQualifiedFolderWorkspacePathStatusRequest(request, options) + const requestSnapshot = getFolderWorkspacePathStatusRequestSnapshotForRead(state, ownedRequest) return getFreshFolderWorkspacePathStatusFromCache({ entry: cached, requestSnapshot }) }, fetchFolderWorkspacePathStatus: async (request, options) => { const cacheKey = get().getFolderWorkspacePathStatusCacheKey(request, options) - const requestSnapshot = getFolderWorkspaceStatusRequestSnapshot(get(), request) + const ownedRequest = getOwnerQualifiedFolderWorkspacePathStatusRequest(request, options) + const requestSnapshot = getFolderWorkspaceStatusRequestSnapshot(get(), ownedRequest) const cached = get().folderWorkspacePathStatuses[cacheKey] const freshCachedStatus = getFreshFolderWorkspacePathStatusFromCache({ entry: cached, @@ -2362,7 +2625,7 @@ export const createRepoSlice: StateCreator = (set, ) const status = target.kind === 'local' - ? await window.api.folderWorkspaces.getPathStatus(request) + ? await window.api.folderWorkspaces.getPathStatus(ownedRequest) : ( await callRuntimeRpc<{ status: FolderWorkspacePathStatus }>( target, @@ -2374,7 +2637,7 @@ export const createRepoSlice: StateCreator = (set, set((state) => ({ folderWorkspacePathStatuses: requestSnapshot !== null && - getFolderWorkspaceStatusRequestSnapshot(state, request) === requestSnapshot + getFolderWorkspaceStatusRequestSnapshot(state, ownedRequest) === requestSnapshot ? { ...state.folderWorkspacePathStatuses, [cacheKey]: { status, checkedAt: Date.now(), requestSnapshot } @@ -2487,14 +2750,25 @@ export const createRepoSlice: StateCreator = (set, } }, - createProjectGroup: async (name) => { + createProjectGroup: async (name, options) => { try { - const target = getActiveRuntimeTarget(get().settings) + const hasExplicitOwner = options?.ownerHostId !== undefined + const parsedOwner = hasExplicitOwner ? parseExecutionHostId(options.ownerHostId!) : null + if (hasExplicitOwner && !parsedOwner) { + return null + } + const target = parsedOwner + ? parsedOwner.kind === 'runtime' + ? { kind: 'environment' as const, environmentId: parsedOwner.environmentId } + : { kind: 'local' as const } + : getActiveRuntimeTarget(get().settings) + const connectionId = parsedOwner?.kind === 'ssh' ? parsedOwner.targetId : undefined const group = target.kind === 'local' ? await window.api.projectGroups.create({ name, - createdFrom: 'manual' + createdFrom: 'manual', + ...(connectionId ? { connectionId } : {}) }) : ( await callRuntimeRpc<{ group: ProjectGroup }>( @@ -2504,7 +2778,9 @@ export const createRepoSlice: StateCreator = (set, { timeoutMs: 15_000 } ) ).group - const ownedGroup = projectGroupWithFetchedOwner(group, target) + const ownedGroup = hasExplicitOwner + ? { ...group, executionHostId: options.ownerHostId } + : projectGroupWithFetchedOwner(group, target) set((s) => ({ projectGroups: [...s.projectGroups, ownedGroup], folderWorkspacePathStatuses: {} @@ -2653,12 +2929,20 @@ export const createRepoSlice: StateCreator = (set, } }, - deleteFolderWorkspace: async (folderWorkspaceId) => { + deleteFolderWorkspace: async (folderWorkspaceId, options) => { const state = get() - if (!findFolderWorkspaceOwner(state, folderWorkspaceId)) { + const owner = findFolderWorkspaceOwner(state, folderWorkspaceId, options?.ownerHostId) + if (!owner) { return false } - const runtimeEnvironmentId = getRuntimeEnvironmentIdForFolderWorkspace(state, folderWorkspaceId) + const ownerHostId = + options?.ownerHostId ?? getFolderWorkspaceCatalogOwnerHostId(owner, state.projectGroups) + // Why: catalog owner can come from the project group even when the folder row itself is still unstamped. + const ownerRuntime = parseExecutionHostId(ownerHostId) + const runtimeEnvironmentId = + ownerRuntime?.kind === 'runtime' + ? ownerRuntime.environmentId + : getRuntimeEnvironmentIdForFolderWorkspace(state, folderWorkspaceId, options?.ownerHostId) try { // Why: deletion targets the folder's owner; focus may be on a different host. const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId: runtimeEnvironmentId }) @@ -2676,14 +2960,29 @@ export const createRepoSlice: StateCreator = (set, if (!deleted) { return false } - const workspaceKey = folderWorkspaceKey(folderWorkspaceId) + const ownerIdentity = getFolderWorkspaceIdentity(folderWorkspaceId, ownerHostId) set((s) => ({ folderWorkspaces: s.folderWorkspaces.filter( - (workspace) => workspace.id !== folderWorkspaceId + (workspace) => + getFolderWorkspaceOwnerIdentity(workspace, s.projectGroups) !== ownerIdentity ), - folderWorkspacePathStatuses: {} + folderWorkspacePathStatuses: Object.fromEntries( + Object.entries(s.folderWorkspacePathStatuses).filter(([cacheKey]) => { + const ownerScoped = `folder-workspace:${ownerHostId}:${folderWorkspaceId}` + const bareScoped = `folder-workspace:${folderWorkspaceId}` + return !cacheKey.includes(ownerScoped) && !cacheKey.endsWith(bareScoped) + }) + ) })) - get().purgeWorktreeTerminalState([workspaceKey]) + const remainingSameId = get().folderWorkspaces.some( + (workspace) => workspace.id === folderWorkspaceId + ) + // Why: same-id cross-host rows share the legacy bare key; only drop it when no sibling remains. + get().purgeWorktreeTerminalState( + remainingSameId + ? [folderWorkspaceKey(folderWorkspaceId, ownerHostId)] + : folderWorkspaceSessionKeys({ folderWorkspaceId, ownerHostId }) + ) return true } catch (err) { console.error('Failed to delete folder workspace:', err) @@ -2691,13 +2990,21 @@ 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) + const group = resolveProjectGroupOwner( + buildProjectGroupOwnerIndex(get().projectGroups), + groupId, + options?.ownerHostId + ) + if (!group) { + return false + } + const ownerHostId = getProjectGroupOwnerHostId(group) + const target = getProjectGroupRuntimeTarget(ownerHostId) const updated = target.kind === 'local' - ? await window.api.projectGroups.update({ groupId, updates }) + ? await window.api.projectGroups.update({ groupId, updates, ownerHostId }) : ( await callRuntimeRpc<{ group: ProjectGroup | null }>( target, @@ -2710,8 +3017,11 @@ export const createRepoSlice: StateCreator = (set, return false } const ownedGroup = projectGroupWithFetchedOwner(updated, target) + const ownedIdentity = getProjectGroupHostIdentity(ownedGroup) set((s) => ({ - projectGroups: s.projectGroups.map((group) => (group.id === groupId ? ownedGroup : group)), + projectGroups: s.projectGroups.map((group) => + getProjectGroupHostIdentity(group) === ownedIdentity ? ownedGroup : group + ), folderWorkspacePathStatuses: {} })) return true @@ -2721,13 +3031,25 @@ 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) + const targets = selectProjectGroupRemovalTargets( + get().projectGroups, + get().repos, + groupId, + options?.ownerHostId, + get().folderWorkspaces + ) + if (!targets.ownerHostId) { + return false + } + const target = getProjectGroupRuntimeTarget(targets.ownerHostId) const deleted = target.kind === 'local' - ? await window.api.projectGroups.delete({ groupId }) + ? await window.api.projectGroups.delete({ + groupId, + ownerHostId: targets.ownerHostId + }) : ( await callRuntimeRpc<{ deleted: boolean }>( target, @@ -2740,14 +3062,22 @@ export const createRepoSlice: StateCreator = (set, return false } set((s) => { - const deletedGroupIds = getProjectGroupSubtreeIds(s.projectGroups, groupId) + const projectTargetIdentities = new Set( + targets.projectTargets.map((projectTarget) => projectTarget.identity) + ) + const projectGroupIndex = buildProjectGroupOwnerIndex(s.projectGroups) return { - projectGroups: s.projectGroups.filter((group) => !deletedGroupIds.has(group.id)), + projectGroups: s.projectGroups.filter( + (group) => !targets.deletedGroupIdentities.has(getProjectGroupHostIdentity(group)) + ), folderWorkspaces: s.folderWorkspaces.filter( - (workspace) => !deletedGroupIds.has(workspace.projectGroupId) + (workspace) => + !targets.folderWorkspaceIdentities.has( + getProjectGroupRemovalFolderWorkspaceIdentity(projectGroupIndex, workspace) ?? '' + ) ), repos: s.repos.map((repo) => - repo.projectGroupId && deletedGroupIds.has(repo.projectGroupId) + repo.projectGroupId && projectTargetIdentities.has(getRepoHostIdentity(repo)) ? { ...repo, projectGroupId: null } : repo ), @@ -2762,7 +3092,13 @@ export const createRepoSlice: StateCreator = (set, }, deleteProjectGroupWithContainedProjects: async (groupId, options) => { - const targets = selectProjectGroupRemovalTargets(get().projectGroups, get().repos, groupId) + const targets = selectProjectGroupRemovalTargets( + get().projectGroups, + get().repos, + groupId, + options.ownerHostId, + get().folderWorkspaces + ) const requestedProjectIds = options.removeContainedProjects ? targets.projectIds : [] if (!targets.groupExists) { return { @@ -2774,7 +3110,9 @@ export const createRepoSlice: StateCreator = (set, } } - const deleted = await get().deleteProjectGroup(groupId) + const deleted = await get().deleteProjectGroup(groupId, { + ownerHostId: targets.ownerHostId ?? undefined + }) if (!deleted) { return { status: 'group-delete-failed', @@ -2797,16 +3135,19 @@ export const createRepoSlice: StateCreator = (set, const removedProjectIds: string[] = [] const failedProjectRemovals: ProjectRemovalFailure[] = [] - for (const projectId of targets.projectIds) { - const existedBeforeRemoval = get().repos.some((repo) => repo.id === projectId) + for (const projectTarget of targets.projectTargets) { + const { projectId, ownerHostId, identity } = projectTarget + const existedBeforeRemoval = get().repos.some( + (repo) => getRepoHostIdentity(repo) === identity + ) try { if (existedBeforeRemoval) { - await get().removeProject(projectId) + await get().removeProject(projectId, { hostId: ownerHostId }) } } catch (err) { console.error('Failed to remove contained project:', err) } - const stillExists = get().repos.some((repo) => repo.id === projectId) + const stillExists = get().repos.some((repo) => getRepoHostIdentity(repo) === identity) if (stillExists) { failedProjectRemovals.push({ projectId, @@ -2826,18 +3167,42 @@ export const createRepoSlice: StateCreator = (set, } }, - moveProjectToGroup: async (projectId, groupId, order) => { + moveProjectToGroup: async (projectId, groupId, order, options) => { try { - if (!findRepoForHost(get().repos, projectId, { settings: get().settings })) { + const hasExplicitOwner = options?.ownerHostId !== undefined + if (hasExplicitOwner && !parseExecutionHostId(options.ownerHostId!)) { return false } - const target = getActiveRuntimeTarget(settingsForRepoOwner(get(), projectId)) + const matchingRepos = get().repos.filter( + (repo) => + repo.id === projectId && + (!hasExplicitOwner || getRepoExecutionHostId(repo) === options.ownerHostId) + ) + if (matchingRepos.length !== 1) { + return false + } + const ownerHostId = getRepoExecutionHostId(matchingRepos[0]) + if ( + groupId !== null && + !resolveProjectGroupOwner( + buildProjectGroupOwnerIndex(get().projectGroups), + groupId, + ownerHostId + ) + ) { + return false + } + const target = getProjectGroupRuntimeTarget(ownerHostId) + // Why: omit ownerHostId when the bare group id is unique so legacy preload callers keep their call shape. + const groupIdIsAmbiguous = + groupId !== null && get().projectGroups.filter((group) => group.id === groupId).length > 1 const moved = target.kind === 'local' ? await window.api.projectGroups.moveProject({ projectId, groupId, - order + order, + ...(groupIdIsAmbiguous || hasExplicitOwner ? { ownerHostId } : {}) }) : ( await callRuntimeRpc<{ repo: Repo | null }>( @@ -2850,18 +3215,17 @@ export const createRepoSlice: StateCreator = (set, if (!moved) { return false } - const ownedMoved = repoWithFetchedOwner(moved, target) - const movedHostId = getRepoExecutionHostId(ownedMoved) + const ownedMoved = { ...repoWithFetchedOwner(moved, target), executionHostId: ownerHostId } set((s) => { const nextRepos = s.repos.map((repo) => - repoMatchesHostIdentity(repo, projectId, movedHostId) ? ownedMoved : repo + repoMatchesHostIdentity(repo, projectId, ownerHostId) ? ownedMoved : repo ) return { repos: nextRepos, ...mergeProjectCompatibilityForHostRepoChange({ previous: { projects: s.projects, projectHostSetups: s.projectHostSetups }, nextRepos, - hostId: movedHostId + hostId: ownerHostId }), folderWorkspacePathStatuses: {} } diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 51613ebdb8a..c427d49943a 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -97,13 +97,16 @@ import { captureWorktreeOperationGenerationGuard } from '@/lib/worktree-operatio import { getEnvironmentSshStateGeneration } from './runtime-environment-ssh' import { getRuntimeEnvironmentConnectionGeneration } from './runtime-status' import { + dualReadFolderWorkspaceKeyedValue, folderWorkspaceKey, getActiveSidebarWorkspaceId, isWorkspaceKey, + migrateFolderWorkspaceKeyedRecord, parseWorkspaceKey, worktreeWorkspaceKey } from '../../../../shared/workspace-scope' import { folderWorkspaceToWorktree } from '../../../../shared/folder-workspace-worktree' +import { getFolderWorkspaceCatalogOwnerHostId } from '../../../../shared/folder-workspaces' import { CLIENT_WORKTREE_CREATE_MAX_ATTEMPTS, getClientWorktreeCreateCandidate, @@ -875,6 +878,110 @@ function folderWorkspaceMatchesHost( ) } +/** Promote legacy bare `folder:` session maps to owner-qualified keys for one unambiguous owner. */ +function migrateLegacyFolderWorkspaceSessionMaps( + s: AppState, + folderWorkspaceId: string, + ownerHostId: ExecutionHostId +): Partial | null { + const resolveOwner = (id: string): ExecutionHostId | null => + id === folderWorkspaceId ? ownerHostId : null + const tabs = migrateFolderWorkspaceKeyedRecord(s.tabsByWorktree, resolveOwner) + const activeFileIdByWorktree = migrateFolderWorkspaceKeyedRecord( + s.activeFileIdByWorktree, + resolveOwner + ) + const activeBrowserTabIdByWorktree = migrateFolderWorkspaceKeyedRecord( + s.activeBrowserTabIdByWorktree, + resolveOwner + ) + const activeTabTypeByWorktree = migrateFolderWorkspaceKeyedRecord( + s.activeTabTypeByWorktree, + resolveOwner + ) + const activeTabIdByWorktree = migrateFolderWorkspaceKeyedRecord( + s.activeTabIdByWorktree, + resolveOwner + ) + const browserTabsByWorktree = migrateFolderWorkspaceKeyedRecord( + s.browserTabsByWorktree, + resolveOwner + ) + const groupsByWorktree = migrateFolderWorkspaceKeyedRecord(s.groupsByWorktree, resolveOwner) + const unifiedTabsByWorktree = migrateFolderWorkspaceKeyedRecord( + s.unifiedTabsByWorktree, + resolveOwner + ) + const activeGroupIdByWorktree = migrateFolderWorkspaceKeyedRecord( + s.activeGroupIdByWorktree, + resolveOwner + ) + // Why: openFiles rows also stamp worktreeId; rewrite legacy bare folder stamps for this owner. + let openFilesChanged = false + const nextOpenFiles = s.openFiles.map((file) => { + const scope = parseWorkspaceKey(file.worktreeId) + if ( + scope?.type === 'folder' && + !scope.ownerHostId && + scope.folderWorkspaceId === folderWorkspaceId + ) { + openFilesChanged = true + return { ...file, worktreeId: folderWorkspaceKey(folderWorkspaceId, ownerHostId) } + } + return file + }) + const bareKey = folderWorkspaceKey(folderWorkspaceId) + const qualifiedKey = folderWorkspaceKey(folderWorkspaceId, ownerHostId) + const activeMigrated = + s.activeWorkspaceKey === bareKey || s.activeWorktreeId === bareKey + ? { activeWorkspaceKey: qualifiedKey, activeWorktreeId: qualifiedKey } + : null + const changed = + tabs.migratedKeys.length > 0 || + activeFileIdByWorktree.migratedKeys.length > 0 || + activeBrowserTabIdByWorktree.migratedKeys.length > 0 || + activeTabTypeByWorktree.migratedKeys.length > 0 || + activeTabIdByWorktree.migratedKeys.length > 0 || + browserTabsByWorktree.migratedKeys.length > 0 || + groupsByWorktree.migratedKeys.length > 0 || + unifiedTabsByWorktree.migratedKeys.length > 0 || + activeGroupIdByWorktree.migratedKeys.length > 0 || + openFilesChanged || + activeMigrated != null + if (!changed) { + return null + } + return { + ...(tabs.migratedKeys.length > 0 ? { tabsByWorktree: tabs.record } : {}), + ...(activeFileIdByWorktree.migratedKeys.length > 0 + ? { activeFileIdByWorktree: activeFileIdByWorktree.record } + : {}), + ...(activeBrowserTabIdByWorktree.migratedKeys.length > 0 + ? { activeBrowserTabIdByWorktree: activeBrowserTabIdByWorktree.record } + : {}), + ...(activeTabTypeByWorktree.migratedKeys.length > 0 + ? { activeTabTypeByWorktree: activeTabTypeByWorktree.record } + : {}), + ...(activeTabIdByWorktree.migratedKeys.length > 0 + ? { activeTabIdByWorktree: activeTabIdByWorktree.record } + : {}), + ...(browserTabsByWorktree.migratedKeys.length > 0 + ? { browserTabsByWorktree: browserTabsByWorktree.record } + : {}), + ...(groupsByWorktree.migratedKeys.length > 0 + ? { groupsByWorktree: groupsByWorktree.record } + : {}), + ...(unifiedTabsByWorktree.migratedKeys.length > 0 + ? { unifiedTabsByWorktree: unifiedTabsByWorktree.record } + : {}), + ...(activeGroupIdByWorktree.migratedKeys.length > 0 + ? { activeGroupIdByWorktree: activeGroupIdByWorktree.record } + : {}), + ...(openFilesChanged ? { openFiles: nextOpenFiles } : {}), + ...(activeMigrated != null ? activeMigrated : {}) + } +} + function findKnownWorktreeById( state: Pick, worktreeId: string, @@ -882,11 +989,18 @@ function findKnownWorktreeById( ): Worktree | DetectedWorktreeListResult['worktrees'][number] | undefined { const workspaceScope = parseWorkspaceKey(worktreeId) if (workspaceScope?.type === 'folder') { - const folderWorkspace = state.folderWorkspaces.find( + const preferredHostId = executionHostId ?? workspaceScope.ownerHostId + const matches = state.folderWorkspaces.filter( (workspace) => workspace.id === workspaceScope.folderWorkspaceId && - (!executionHostId || folderWorkspaceMatchesHost(workspace, executionHostId)) + (!preferredHostId || folderWorkspaceMatchesHost(workspace, preferredHostId)) ) + // Why: bare folder keys dual-read only when the owner is unambiguous; multi-owner same-id must use owner-qualified keys. + const folderWorkspace = preferredHostId + ? (matches[0] ?? null) + : matches.length === 1 + ? matches[0] + : null if (!folderWorkspace) { return undefined } @@ -3673,6 +3787,9 @@ export const createWorktreeSlice: StateCreator validIds.add(FLOATING_TERMINAL_WORKTREE_ID) // Why: folder workspaces persist tabs under `folder:` keys that authoritative repo scans never return. for (const workspace of get().folderWorkspaces ?? []) { + const ownerHostId = getFolderWorkspaceCatalogOwnerHostId(workspace, get().projectGroups) + validIds.add(folderWorkspaceKey(workspace.id, ownerHostId)) + // Why: pre-owner session rows still use bare folder ids. validIds.add(folderWorkspaceKey(workspace.id)) } for (const key of Object.keys(get().restoredRuntimeHostIdByWorkspaceSessionKey ?? {})) { @@ -4003,7 +4120,7 @@ export const createWorktreeSlice: StateCreator const activeScope = parseWorkspaceKey(get().activeWorkspaceKey ?? '') const parentWorkspace = activeScope?.type === 'folder' - ? folderWorkspaceKey(activeScope.folderWorkspaceId) + ? folderWorkspaceKey(activeScope.folderWorkspaceId, activeScope.ownerHostId) : undefined const createArgs = { repoId, @@ -5924,11 +6041,26 @@ export const createWorktreeSlice: StateCreator }, setActiveFolderWorkspace: (folderWorkspaceId, executionHostId) => { - const workspaceKey = folderWorkspaceKey(folderWorkspaceId) - const workspace = findKnownWorktreeById(get(), workspaceKey, executionHostId) + const workspace = findKnownWorktreeById( + get(), + folderWorkspaceKey(folderWorkspaceId, executionHostId), + executionHostId + ) if (!workspace) { return } + // Why: synthetic folder worktrees stamp hostId from catalog ownership; prefer the caller host when provided. + const ownerHostId = executionHostId ?? workspace.hostId ?? LOCAL_EXECUTION_HOST_ID + const workspaceKey = folderWorkspaceKey(folderWorkspaceId, ownerHostId) + // Why: promote legacy bare `folder:` session maps before activation so dual-read becomes a one-time migration. + const migratedSession = migrateLegacyFolderWorkspaceSessionMaps( + get(), + folderWorkspaceId, + ownerHostId + ) + if (migratedSession) { + set(migratedSession) + } if (shouldDeferActivationTerminalPrep()) { markInputQuietSchedulerInput() } @@ -5938,32 +6070,71 @@ export const createWorktreeSlice: StateCreator const reconciledActiveTabId = get().reconcileWorktreeTabModel(workspaceKey).activeRenderableTabId set((s) => { - const restoredFileId = s.activeFileIdByWorktree[workspaceKey] ?? null - const restoredBrowserTabId = s.activeBrowserTabIdByWorktree[workspaceKey] ?? null - const restoredTabType = s.activeTabTypeByWorktree[workspaceKey] ?? 'terminal' - const activeGroupId = - s.activeGroupIdByWorktree[workspaceKey] ?? s.groupsByWorktree[workspaceKey]?.[0]?.id ?? null + const dualTabs = dualReadFolderWorkspaceKeyedValue((key) => s.tabsByWorktree[key], { + folderWorkspaceId, + ownerHostId + }) + const dualActiveFile = dualReadFolderWorkspaceKeyedValue( + (key) => s.activeFileIdByWorktree[key], + { folderWorkspaceId, ownerHostId } + ) + const dualActiveBrowser = dualReadFolderWorkspaceKeyedValue( + (key) => s.activeBrowserTabIdByWorktree[key], + { folderWorkspaceId, ownerHostId } + ) + const dualActiveTabType = dualReadFolderWorkspaceKeyedValue( + (key) => s.activeTabTypeByWorktree[key], + { folderWorkspaceId, ownerHostId } + ) + const dualActiveTab = dualReadFolderWorkspaceKeyedValue( + (key) => s.activeTabIdByWorktree[key], + { folderWorkspaceId, ownerHostId } + ) + const dualBrowserTabs = dualReadFolderWorkspaceKeyedValue( + (key) => s.browserTabsByWorktree[key], + { folderWorkspaceId, ownerHostId } + ) + const dualGroups = dualReadFolderWorkspaceKeyedValue((key) => s.groupsByWorktree[key], { + folderWorkspaceId, + ownerHostId + }) + const dualUnified = dualReadFolderWorkspaceKeyedValue((key) => s.unifiedTabsByWorktree[key], { + folderWorkspaceId, + ownerHostId + }) + const dualActiveGroup = dualReadFolderWorkspaceKeyedValue( + (key) => s.activeGroupIdByWorktree[key], + { folderWorkspaceId, ownerHostId } + ) + const restoredFileId = dualActiveFile?.value ?? null + const restoredBrowserTabId = dualActiveBrowser?.value ?? null + const restoredTabType = dualActiveTabType?.value ?? 'terminal' + const activeGroupId = dualActiveGroup?.value ?? dualGroups?.value?.[0]?.id ?? null const activeGroup = activeGroupId - ? ((s.groupsByWorktree[workspaceKey] ?? []).find((group) => group.id === activeGroupId) ?? - null) + ? ((dualGroups?.value ?? []).find((group) => group.id === activeGroupId) ?? null) : null const activeUnifiedTabId = reconciledActiveTabId ?? activeGroup?.activeTabId ?? null const activeUnifiedTab = activeUnifiedTabId != null - ? ((s.unifiedTabsByWorktree[workspaceKey] ?? []).find( + ? ((dualUnified?.value ?? []).find( (tab) => tab.id === activeUnifiedTabId && (!activeGroup || tab.groupId === activeGroup.id) ) ?? null) : null const fileStillOpen = restoredFileId - ? s.openFiles.some((file) => file.id === restoredFileId && file.worktreeId === workspaceKey) + ? s.openFiles.some( + (file) => + file.id === restoredFileId && + (file.worktreeId === workspaceKey || + file.worktreeId === folderWorkspaceKey(folderWorkspaceId)) + ) : false - const browserTabs = s.browserTabsByWorktree[workspaceKey] ?? [] + const browserTabs = dualBrowserTabs?.value ?? [] const browserTabStillOpen = restoredBrowserTabId ? browserTabs.some((tab) => tab.id === restoredBrowserTabId) : false - const worktreeTabs = s.tabsByWorktree[workspaceKey] ?? [] - const restoredTabId = s.activeTabIdByWorktree[workspaceKey] ?? null + const worktreeTabs = dualTabs?.value ?? [] + const restoredTabId = dualActiveTab?.value ?? null const tabStillExists = restoredTabId ? worktreeTabs.some((tab) => tab.id === restoredTabId) : false diff --git a/src/shared/folder-workspace-path-status.ts b/src/shared/folder-workspace-path-status.ts index 7b9ebefc4c0..8ccb53e4b14 100644 --- a/src/shared/folder-workspace-path-status.ts +++ b/src/shared/folder-workspace-path-status.ts @@ -1,3 +1,5 @@ +import type { ExecutionHostId } from './execution-host' + export type FolderWorkspacePathStatusReason = | 'missing' | 'not-directory' @@ -7,8 +9,8 @@ export type FolderWorkspacePathStatusReason = export const FOLDER_WORKSPACE_PATH_STATUS_TTL_MS = 10_000 export type FolderWorkspacePathStatusRequest = - | { scope: 'folder-workspace'; folderWorkspaceId: string } - | { scope: 'project-group'; projectGroupId: string } + | { scope: 'folder-workspace'; folderWorkspaceId: string; ownerHostId?: ExecutionHostId } + | { scope: 'project-group'; projectGroupId: string; ownerHostId?: ExecutionHostId } | { scope: 'path'; path: string; connectionId?: string | null } export type FolderWorkspacePathStatus = { diff --git a/src/shared/folder-workspaces.ts b/src/shared/folder-workspaces.ts index 675117dd3eb..b1bb74bc50a 100644 --- a/src/shared/folder-workspaces.ts +++ b/src/shared/folder-workspaces.ts @@ -1,9 +1,69 @@ +import { + LOCAL_EXECUTION_HOST_ID, + normalizeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from './execution-host' +import { + buildProjectGroupOwnerIndex, + getFolderWorkspaceProjectGroupOwnerHostId, + resolveFolderWorkspaceProjectGroup, + resolveProjectGroupOwner +} from './project-groups' import type { FolderWorkspace, ProjectGroup } from './types' import { isTuiAgent } from './tui-agent-config' import { normalizeStoredTaskSourceContext } from './task-source-context' import { normalizeWorkspaceLinkedItem } from './workspace-linked-item' import { isWorkspaceLinkedItemSourceContextMatch } from './workspace-linked-item-source-context' +export function getFolderWorkspaceCatalogOwnerHostId( + workspace: Pick, + projectGroups: readonly ProjectGroup[] = [] +): ExecutionHostId { + const executionHostId = normalizeExecutionHostId(workspace.executionHostId) + if (executionHostId) { + return executionHostId + } + if (workspace.connectionId) { + return toSshExecutionHostId(workspace.connectionId) + } + if (workspace.connectionId === null) { + return LOCAL_EXECUTION_HOST_ID + } + if (projectGroups.length === 0) { + return LOCAL_EXECUTION_HOST_ID + } + return getFolderWorkspaceProjectGroupOwnerHostId( + workspace, + buildProjectGroupOwnerIndex(projectGroups) + ) +} + +export function getFolderWorkspaceOwnerIdentity( + workspace: Pick, + projectGroups: readonly ProjectGroup[] = [] +): string { + return getFolderWorkspaceIdentity( + workspace.id, + getFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups) + ) +} + +export function getFolderWorkspaceIdentity( + folderWorkspaceId: string, + ownerHostId: ExecutionHostId +): string { + return JSON.stringify([ownerHostId, folderWorkspaceId]) +} + +export function getFolderWorkspaceRowKey( + workspace: Pick, + projectGroups: readonly ProjectGroup[] = [] +): string { + const ownerHostId = getFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups) + return `folder-workspace:${encodeURIComponent(ownerHostId)}:${encodeURIComponent(workspace.id)}` +} + export function normalizeFolderWorkspaceName( name: string | null | undefined, fallback = 'Untitled workspace' @@ -19,12 +79,7 @@ export function normalizeFolderWorkspaces( if (!Array.isArray(value)) { return [] } - const folderGroups = new Map() - for (const group of projectGroups) { - if (group.parentPath) { - folderGroups.set(group.id, group) - } - } + const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups) const workspaces: FolderWorkspace[] = [] const seen = new Set() @@ -36,13 +91,20 @@ export function normalizeFolderWorkspaces( if ( typeof raw.id !== 'string' || raw.id.trim().length === 0 || - seen.has(raw.id) || - typeof raw.projectGroupId !== 'string' || - !folderGroups.has(raw.projectGroupId) + typeof raw.projectGroupId !== 'string' ) { continue } - const group = folderGroups.get(raw.projectGroupId) + const executionHostId = normalizeExecutionHostId(raw.executionHostId) + const group = + resolveFolderWorkspaceProjectGroup(projectGroupIndex, { + projectGroupId: raw.projectGroupId, + connectionId: raw.connectionId, + executionHostId + }) ?? resolveProjectGroupOwner(projectGroupIndex, raw.projectGroupId) + if (!group?.parentPath) { + continue + } const folderPath = typeof raw.folderPath === 'string' && raw.folderPath.trim().length > 0 ? raw.folderPath @@ -50,21 +112,36 @@ export function normalizeFolderWorkspaces( if (!folderPath) { continue } + const connectionId = + typeof raw.connectionId === 'string' + ? raw.connectionId + : raw.connectionId === null + ? null + : (group?.connectionId ?? null) + // Why: multi-host catalogs keep same-id folder rows; identity is owner+id, not bare id. + const identity = getFolderWorkspaceOwnerIdentity( + { + id: raw.id, + projectGroupId: raw.projectGroupId, + connectionId, + ...(executionHostId ? { executionHostId } : {}) + }, + projectGroups + ) + if (seen.has(identity)) { + continue + } const now = Date.now() const linkedTask = normalizeWorkspaceLinkedItem(raw.linkedTask) const linkedTaskSourceContext = normalizeStoredTaskSourceContext(raw.linkedTaskSourceContext) - seen.add(raw.id) + seen.add(identity) workspaces.push({ id: raw.id, projectGroupId: raw.projectGroupId, name: normalizeFolderWorkspaceName(raw.name), folderPath, - connectionId: - typeof raw.connectionId === 'string' - ? raw.connectionId - : raw.connectionId === null - ? null - : (group?.connectionId ?? null), + connectionId, + ...(executionHostId ? { executionHostId } : {}), linkedTask, linkedTaskSourceContext: isWorkspaceLinkedItemSourceContextMatch( linkedTask, diff --git a/src/shared/project-groups.test.ts b/src/shared/project-groups.test.ts index 4c11e8d598b..9537caf3c5a 100644 --- a/src/shared/project-groups.test.ts +++ b/src/shared/project-groups.test.ts @@ -1,14 +1,19 @@ import { describe, expect, it } from 'vitest' import { + buildProjectGroupOwnerIndex, clearMissingProjectGroupMemberships, createProjectGroup, getEffectiveProjectGroupManualRank, getNextProjectGroupOrder, + getProjectGroupOwnerIdentity, + getProjectGroupOwnerSubtreeIdentities, getProjectGroupSubtreeIds, normalizeProjectGroupName, - normalizeProjectGroups + normalizeProjectGroups, + resolveFolderWorkspaceProjectGroup, + resolveProjectGroupOwner } from './project-groups' -import type { Repo } from './types' +import type { ProjectGroup, Repo } from './types' function repo(overrides: Partial): Repo { return { @@ -22,6 +27,22 @@ function repo(overrides: Partial): Repo { } } +function projectGroup(overrides: Partial): ProjectGroup { + return { + id: overrides.id ?? 'group-1', + name: overrides.name ?? 'Group', + parentPath: null, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1, + ...overrides + } +} + describe('project-groups', () => { it('creates a durable project group with normalized defaults', () => { const group = createProjectGroup({ @@ -84,6 +105,80 @@ describe('project-groups', () => { expect(groups.find((group) => group.id === 'invalid')?.executionHostId).toBeUndefined() }) + it('preserves same-id groups and parentage independently across owners', () => { + const groups = normalizeProjectGroups([ + projectGroup({ id: 'root', name: 'Local root' }), + projectGroup({ id: 'child', name: 'Local child', parentGroupId: 'root' }), + projectGroup({ id: 'root', name: 'SSH root', connectionId: 'builder' }), + projectGroup({ + id: 'child', + name: 'SSH child', + connectionId: 'builder', + parentGroupId: 'root' + }), + projectGroup({ id: 'root', name: 'Duplicate SSH root', connectionId: 'builder' }) + ]) + + expect(groups.map((group) => [group.name, group.parentGroupId])).toEqual([ + ['Local child', 'root'], + ['Local root', null], + ['SSH child', 'root'], + ['SSH root', null] + ]) + }) + + it('resolves mutations exactly by owner and rejects ambiguous legacy selectors', () => { + const local = projectGroup({ id: 'same-id', name: 'Local' }) + const ssh = projectGroup({ id: 'same-id', name: 'SSH', connectionId: 'builder' }) + const index = buildProjectGroupOwnerIndex([local, ssh]) + + expect(resolveProjectGroupOwner(index, 'same-id', 'local')).toBe(local) + expect(resolveProjectGroupOwner(index, 'same-id', 'ssh:builder')).toBe(ssh) + expect(resolveProjectGroupOwner(index, 'same-id')).toBeNull() + expect(resolveProjectGroupOwner(index, 'same-id', 'ssh:missing')).toBeNull() + }) + + it('resolves folder membership by owner and rejects unstamped same-id ambiguity', () => { + const local = projectGroup({ id: 'same-id', name: 'Local' }) + const runtime = projectGroup({ + id: 'same-id', + name: 'Runtime', + executionHostId: 'runtime:env-1' + }) + const index = buildProjectGroupOwnerIndex([local, runtime]) + + expect( + resolveFolderWorkspaceProjectGroup(index, { + projectGroupId: 'same-id', + connectionId: null + }) + ).toBe(local) + expect( + resolveFolderWorkspaceProjectGroup(index, { + projectGroupId: 'same-id', + executionHostId: 'runtime:env-1' + }) + ).toBe(runtime) + expect(resolveFolderWorkspaceProjectGroup(index, { projectGroupId: 'same-id' })).toBeNull() + }) + + it('collects descendants only from the selected owner hierarchy', () => { + const localRoot = projectGroup({ id: 'root', name: 'Local root' }) + const localChild = projectGroup({ id: 'child', parentGroupId: 'root' }) + const sshRoot = projectGroup({ id: 'root', name: 'SSH root', connectionId: 'builder' }) + const sshChild = projectGroup({ + id: 'child', + parentGroupId: 'root', + connectionId: 'builder' + }) + + expect( + getProjectGroupOwnerSubtreeIdentities([localRoot, localChild, sshRoot, sshChild], localRoot) + ).toEqual( + new Set([getProjectGroupOwnerIdentity(localRoot), getProjectGroupOwnerIdentity(localChild)]) + ) + }) + it('clears repo memberships whose group no longer exists', () => { const groups = [createProjectGroup({ name: 'Known', createdFrom: 'manual', tabOrder: 0 })] const repos = clearMissingProjectGroupMemberships( @@ -98,6 +193,25 @@ describe('project-groups', () => { expect(repos.find((entry) => entry.id === 'missing')?.projectGroupId).toBeNull() }) + it('does not retain repo membership from another owner with the same group id', () => { + const localGroup = projectGroup({ id: 'same-id', name: 'Local' }) + const sshGroup = projectGroup({ id: 'same-id', name: 'SSH', connectionId: 'builder' }) + const repos = clearMissingProjectGroupMemberships( + [ + repo({ id: 'local', projectGroupId: 'same-id' }), + repo({ id: 'ssh', projectGroupId: 'same-id', connectionId: 'builder' }), + repo({ id: 'wrong-local', projectGroupId: 'ssh-only' }) + ], + [localGroup, sshGroup, projectGroup({ id: 'ssh-only', connectionId: 'builder' })] + ) + + expect(repos.map((entry) => [entry.id, entry.projectGroupId])).toEqual([ + ['local', 'same-id'], + ['ssh', 'same-id'], + ['wrong-local', null] + ]) + }) + it('falls back to global repo order when projectGroupOrder is unset', () => { const repoOrder = new Map([ ['a', 0], diff --git a/src/shared/project-groups.ts b/src/shared/project-groups.ts index 6d9b02d8354..b779e95c87c 100644 --- a/src/shared/project-groups.ts +++ b/src/shared/project-groups.ts @@ -1,8 +1,143 @@ -import { normalizeExecutionHostId } from './execution-host' -import type { Repo, ProjectGroup, ProjectGroupCreatedFrom } from './types' +import { + getRepoExecutionHostId, + LOCAL_EXECUTION_HOST_ID, + normalizeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from './execution-host' +import type { FolderWorkspace, Repo, ProjectGroup, ProjectGroupCreatedFrom } from './types' export const UNGROUPED_PROJECT_GROUP_KEY = 'project-group:ungrouped' +export type ProjectGroupOwnerIndex = { + byIdentity: ReadonlyMap + byId: ReadonlyMap +} + +export function getProjectGroupOwnerHostId( + group: Pick +): ExecutionHostId { + const executionHostId = normalizeExecutionHostId(group.executionHostId) + if (executionHostId) { + return executionHostId + } + return group.connectionId ? toSshExecutionHostId(group.connectionId) : LOCAL_EXECUTION_HOST_ID +} + +export function getProjectGroupOwnerIdentity( + group: Pick +): string { + return getProjectGroupIdentity(group.id, getProjectGroupOwnerHostId(group)) +} + +export function getProjectGroupIdentity(groupId: string, ownerHostId: ExecutionHostId): string { + return JSON.stringify([ownerHostId, groupId]) +} + +export function buildProjectGroupOwnerIndex( + projectGroups: readonly ProjectGroup[] +): ProjectGroupOwnerIndex { + const byIdentity = new Map() + const byId = new Map() + for (const group of projectGroups) { + byIdentity.set(getProjectGroupOwnerIdentity(group), group) + const matches = byId.get(group.id) ?? [] + matches.push(group) + byId.set(group.id, matches) + } + return { byIdentity, byId } +} + +export function resolveProjectGroupOwner( + index: ProjectGroupOwnerIndex, + groupId: string, + ownerHostId?: ExecutionHostId +): ProjectGroup | null { + if (ownerHostId) { + return index.byIdentity.get(getProjectGroupIdentity(groupId, ownerHostId)) ?? null + } + const matches = index.byId.get(groupId) ?? [] + return matches.length === 1 ? (matches[0] ?? null) : null +} + +export function resolveProjectGroupMembership( + index: ProjectGroupOwnerIndex, + groupId: string, + ownerHostId: ExecutionHostId +): ProjectGroup | null { + return index.byIdentity.get(getProjectGroupIdentity(groupId, ownerHostId)) ?? null +} + +export function getFolderWorkspaceProjectGroupOwnerHostId( + workspace: Pick, + index: ProjectGroupOwnerIndex +): ExecutionHostId { + const executionHostId = normalizeExecutionHostId(workspace.executionHostId) + if (executionHostId) { + return executionHostId + } + const group = resolveFolderWorkspaceProjectGroup(index, workspace) + if (group) { + return getProjectGroupOwnerHostId(group) + } + if (workspace.connectionId) { + return toSshExecutionHostId(workspace.connectionId) + } + return LOCAL_EXECUTION_HOST_ID +} + +export function resolveFolderWorkspaceProjectGroup( + index: ProjectGroupOwnerIndex, + workspace: Pick +): ProjectGroup | null { + const executionHostId = normalizeExecutionHostId(workspace.executionHostId) + if (executionHostId) { + return resolveProjectGroupMembership(index, workspace.projectGroupId, executionHostId) + } + if (workspace.connectionId !== undefined) { + const ownerHostId = workspace.connectionId + ? toSshExecutionHostId(workspace.connectionId) + : LOCAL_EXECUTION_HOST_ID + const group = resolveProjectGroupMembership(index, workspace.projectGroupId, ownerHostId) + return group ?? resolveProjectGroupOwner(index, workspace.projectGroupId) + } + return resolveProjectGroupOwner(index, workspace.projectGroupId) +} + +export function getProjectGroupOwnerSubtreeIdentities( + projectGroups: readonly ProjectGroup[], + rootGroup: ProjectGroup +): Set { + const childrenByParentIdentity = new Map() + for (const group of projectGroups) { + if (!group.parentGroupId) { + continue + } + const parentIdentity = getProjectGroupIdentity( + group.parentGroupId, + getProjectGroupOwnerHostId(group) + ) + const children = childrenByParentIdentity.get(parentIdentity) ?? [] + children.push(group) + childrenByParentIdentity.set(parentIdentity, children) + } + + const subtreeIdentities = new Set() + const pending = [rootGroup] + while (pending.length > 0) { + const group = pending.pop()! + const identity = getProjectGroupOwnerIdentity(group) + if (subtreeIdentities.has(identity)) { + continue + } + subtreeIdentities.add(identity) + for (const child of childrenByParentIdentity.get(identity) ?? []) { + pending.push(child) + } + } + return subtreeIdentities +} + function createProjectGroupId(): string { const randomUUID = globalThis.crypto?.randomUUID if (randomUUID) { @@ -52,13 +187,12 @@ export function normalizeProjectGroups(value: unknown): ProjectGroup[] { continue } const raw = candidate as Partial - if (typeof raw.id !== 'string' || seen.has(raw.id)) { + if (typeof raw.id !== 'string') { continue } - seen.add(raw.id) const now = Date.now() const executionHostId = normalizeExecutionHostId(raw.executionHostId) - groups.push({ + const group: ProjectGroup = { id: raw.id, name: normalizeProjectGroupName(typeof raw.name === 'string' ? raw.name : ''), parentPath: typeof raw.parentPath === 'string' ? raw.parentPath : null, @@ -83,16 +217,27 @@ export function normalizeProjectGroups(value: unknown): ProjectGroup[] { typeof raw.createdAt === 'number' && Number.isFinite(raw.createdAt) ? raw.createdAt : now, updatedAt: typeof raw.updatedAt === 'number' && Number.isFinite(raw.updatedAt) ? raw.updatedAt : now, - // Why: runtime-owned groups otherwise look local after persistence reload. ...(executionHostId ? { executionHostId } : {}) - }) + } + const identity = getProjectGroupOwnerIdentity(group) + if (seen.has(identity)) { + continue + } + seen.add(identity) + groups.push(group) } groups.sort( (left, right) => left.tabOrder - right.tabOrder || left.name.localeCompare(right.name) ) - const groupIds = new Set(groups.map((group) => group.id)) + const groupIdentities = new Set(groups.map(getProjectGroupOwnerIdentity)) for (const group of groups) { - if (group.parentGroupId === group.id || !groupIds.has(group.parentGroupId ?? '')) { + const parentIdentity = group.parentGroupId + ? getProjectGroupIdentity(group.parentGroupId, getProjectGroupOwnerHostId(group)) + : null + if ( + group.parentGroupId === group.id || + (parentIdentity !== null && !groupIdentities.has(parentIdentity)) + ) { group.parentGroupId = null } } @@ -100,9 +245,10 @@ export function normalizeProjectGroups(value: unknown): ProjectGroup[] { } export function clearMissingProjectGroupMemberships(repos: Repo[], groups: ProjectGroup[]): Repo[] { - const groupIds = new Set(groups.map((group) => group.id)) + const index = buildProjectGroupOwnerIndex(groups) return repos.map((repo) => - repo.projectGroupId && !groupIds.has(repo.projectGroupId) + repo.projectGroupId && + !resolveProjectGroupMembership(index, repo.projectGroupId, getRepoExecutionHostId(repo)) ? { ...repo, projectGroupId: null } : repo ) @@ -130,8 +276,7 @@ export function getProjectGroupSubtreeIds( continue } subtreeIds.add(groupId) - // Why: imported project-group trees can be very wide; `push(...children)` - // can exceed V8's argument limit while collecting descendants. + // Why: avoid V8 argument limits in wide imported project-group trees. for (const childGroupId of childGroupsByParentId.get(groupId) ?? []) { pending.push(childGroupId) } diff --git a/src/shared/types.ts b/src/shared/types.ts index cd874d94037..7fb4099d1b2 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -326,7 +326,7 @@ export type ProjectGroup = { export type WorkspaceScope = | { type: 'worktree'; worktreeId: string } - | { type: 'folder'; folderWorkspaceId: string } + | { type: 'folder'; folderWorkspaceId: string; ownerHostId?: ExecutionHostId } export type WorkspaceKey = `worktree:${string}` | `folder:${string}` diff --git a/src/shared/workspace-scope.test.ts b/src/shared/workspace-scope.test.ts new file mode 100644 index 00000000000..6dda189661a --- /dev/null +++ b/src/shared/workspace-scope.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from 'vitest' +import { + dualReadFolderWorkspaceKeyedValue, + folderWorkspaceKey, + folderWorkspaceSessionKeys, + migrateFolderWorkspaceKeyedRecord, + parseWorkspaceKey +} from './workspace-scope' + +describe('owner-qualified folder workspace keys', () => { + it('round-trips owner-qualified keys for same-id multi-host rows', () => { + const localKey = folderWorkspaceKey('same-id', 'local') + const sshKey = folderWorkspaceKey('same-id', 'ssh:builder-1') + + expect(localKey).not.toBe(sshKey) + expect(parseWorkspaceKey(localKey)).toEqual({ + type: 'folder', + folderWorkspaceId: 'same-id', + ownerHostId: 'local' + }) + expect(parseWorkspaceKey(sshKey)).toEqual({ + type: 'folder', + folderWorkspaceId: 'same-id', + ownerHostId: 'ssh:builder-1' + }) + }) + + it('keeps legacy bare folder keys parseable', () => { + expect(parseWorkspaceKey(folderWorkspaceKey('legacy-folder'))).toEqual({ + type: 'folder', + folderWorkspaceId: 'legacy-folder' + }) + }) + + it('lists both owner-qualified and legacy session keys for cleanup', () => { + expect( + folderWorkspaceSessionKeys({ folderWorkspaceId: 'same-id', ownerHostId: 'local' }) + ).toEqual([folderWorkspaceKey('same-id', 'local'), folderWorkspaceKey('same-id')]) + }) + + it('dual-reads owner-qualified first, then legacy bare alias', () => { + const bare = folderWorkspaceKey('folder-workspace-1') + const qualified = folderWorkspaceKey('folder-workspace-1', 'local') + const record: Record = { [bare]: 'legacy-tabs' } + + expect( + dualReadFolderWorkspaceKeyedValue((key) => record[key], { + folderWorkspaceId: 'folder-workspace-1', + ownerHostId: 'local' + }) + ).toEqual({ value: 'legacy-tabs', key: bare, viaLegacyAlias: true }) + + record[qualified] = 'canonical-tabs' + expect( + dualReadFolderWorkspaceKeyedValue((key) => record[key], { + folderWorkspaceId: 'folder-workspace-1', + ownerHostId: 'local' + }) + ).toEqual({ value: 'canonical-tabs', key: qualified, viaLegacyAlias: false }) + }) + + it('migrates unambiguous legacy bare folder keys for the default owner', () => { + const bare = folderWorkspaceKey('folder-workspace-1') + const qualified = folderWorkspaceKey('folder-workspace-1', 'local') + const { record, migratedKeys } = migrateFolderWorkspaceKeyedRecord( + { [bare]: [{ id: 'tab-1' }], [folderWorkspaceKey('ambiguous')]: [{ id: 'tab-2' }] }, + (folderWorkspaceId) => (folderWorkspaceId === 'folder-workspace-1' ? 'local' : null) + ) + + expect(migratedKeys).toEqual([bare]) + expect(record[qualified]).toEqual([{ id: 'tab-1' }]) + expect(record[bare]).toBeUndefined() + // Multi-owner / unresolved bare keys stay put so callers fail closed. + expect(record[folderWorkspaceKey('ambiguous')]).toEqual([{ id: 'tab-2' }]) + }) + + it('does not overwrite an existing owner-qualified row when migrating bare aliases', () => { + const bare = folderWorkspaceKey('folder-workspace-1') + const qualified = folderWorkspaceKey('folder-workspace-1', 'local') + const { record } = migrateFolderWorkspaceKeyedRecord( + { [bare]: 'legacy', [qualified]: 'canonical' }, + () => 'local' + ) + expect(record[qualified]).toBe('canonical') + expect(record[bare]).toBeUndefined() + }) +}) diff --git a/src/shared/workspace-scope.ts b/src/shared/workspace-scope.ts index 4993c0599d7..60ac6f607e0 100644 --- a/src/shared/workspace-scope.ts +++ b/src/shared/workspace-scope.ts @@ -1,10 +1,18 @@ +import type { ExecutionHostId } from './execution-host' import type { WorkspaceKey, WorkspaceScope } from './types' export function worktreeWorkspaceKey(worktreeId: string): WorkspaceKey { return `worktree:${worktreeId}` } -export function folderWorkspaceKey(folderWorkspaceId: string): WorkspaceKey { +/** Owner-qualified folder session keys keep same-id multi-host rows independent. */ +export function folderWorkspaceKey( + folderWorkspaceId: string, + ownerHostId?: ExecutionHostId +): WorkspaceKey { + if (ownerHostId) { + return `folder:${encodeURIComponent(ownerHostId)}:${encodeURIComponent(folderWorkspaceId)}` + } return `folder:${folderWorkspaceId}` } @@ -14,8 +22,24 @@ export function parseWorkspaceKey(value: string): WorkspaceScope | null { return worktreeId.length > 0 ? { type: 'worktree', worktreeId } : null } if (value.startsWith('folder:')) { - const folderWorkspaceId = value.slice('folder:'.length) - return folderWorkspaceId.length > 0 ? { type: 'folder', folderWorkspaceId } : null + const rest = value.slice('folder:'.length) + if (rest.length === 0) { + return null + } + // Why: owner-qualified keys encode both segments so host ids with colons stay unambiguous. + const separator = rest.indexOf(':') + if (separator > 0) { + try { + const ownerHostId = decodeURIComponent(rest.slice(0, separator)) as ExecutionHostId + const folderWorkspaceId = decodeURIComponent(rest.slice(separator + 1)) + if (ownerHostId.length > 0 && folderWorkspaceId.length > 0) { + return { type: 'folder', folderWorkspaceId, ownerHostId } + } + } catch { + // Fall through to legacy id-only form when decoding fails. + } + } + return { type: 'folder', folderWorkspaceId: rest } } return null } @@ -32,10 +56,80 @@ export function getActiveSidebarWorkspaceId( ): string | null { const scope = activeWorkspaceKey ? parseWorkspaceKey(activeWorkspaceKey) : null if (scope?.type === 'folder') { - return folderWorkspaceKey(scope.folderWorkspaceId) + return folderWorkspaceKey(scope.folderWorkspaceId, scope.ownerHostId) } if (scope?.type === 'worktree') { return scope.worktreeId } return activeWorktreeId } + +export function folderWorkspaceSessionKeys(args: { + folderWorkspaceId: string + ownerHostId: ExecutionHostId +}): string[] { + return [ + folderWorkspaceKey(args.folderWorkspaceId, args.ownerHostId), + // Why: older sessions still key by bare folder id; purge both so one owner cleanup cannot leave the legacy alias behind for the deleted row only when it was sole occupant is handled by callers. + folderWorkspaceKey(args.folderWorkspaceId) + ] +} + +/** + * Dual-read contract for owner-qualified folder session keys: + * prefer the canonical owner-qualified key, then fall back to the legacy bare + * `folder:` alias so pre-migration tabs/state keep resolving for that owner. + */ +export function dualReadFolderWorkspaceKeyedValue( + get: (key: string) => T | undefined, + args: { folderWorkspaceId: string; ownerHostId: ExecutionHostId } +): { value: T; key: string; viaLegacyAlias: boolean } | null { + const qualified = folderWorkspaceKey(args.folderWorkspaceId, args.ownerHostId) + const fromQualified = get(qualified) + if (fromQualified !== undefined) { + return { value: fromQualified, key: qualified, viaLegacyAlias: false } + } + const bare = folderWorkspaceKey(args.folderWorkspaceId) + const fromBare = get(bare) + if (fromBare !== undefined) { + return { value: fromBare, key: bare, viaLegacyAlias: true } + } + return null +} + +/** + * Migrate legacy bare `folder:` map keys to owner-qualified form when the + * catalog can resolve an unambiguous owner. Multi-owner same-id bare keys stay + * untouched so callers can fail closed instead of guessing. + */ +export function migrateFolderWorkspaceKeyedRecord( + record: Record, + resolveUnambiguousOwner: (folderWorkspaceId: string) => ExecutionHostId | null +): { record: Record; migratedKeys: string[] } { + const migratedKeys: string[] = [] + let next: Record | null = null + for (const [key, value] of Object.entries(record)) { + const scope = parseWorkspaceKey(key) + if (scope?.type !== 'folder' || scope.ownerHostId) { + continue + } + const ownerHostId = resolveUnambiguousOwner(scope.folderWorkspaceId) + if (!ownerHostId) { + continue + } + const qualified = folderWorkspaceKey(scope.folderWorkspaceId, ownerHostId) + if (qualified === key) { + continue + } + if (!next) { + next = { ...record } + } + // Why: if both forms already exist, keep the canonical owner-qualified row. + if (!(qualified in next)) { + next[qualified] = value + } + delete next[key] + migratedKeys.push(key) + } + return { record: next ?? record, migratedKeys } +}