diff --git a/issue-12532-reduced-candidate-report.md b/issue-12532-reduced-candidate-report.md deleted file mode 100644 index b1f1a1e3585..00000000000 --- a/issue-12532-reduced-candidate-report.md +++ /dev/null @@ -1,55 +0,0 @@ -# #12532 exact-candidate independent review - -## Verdict - -**NOT CLEAN.** Exact candidate `de06dbb4f2e2e27da7fa21592432439b985649db` retained four issue-scoped defect classes in persistence provenance, local filesystem authorization, same-ID folder deletion, and bulk owner/path-status work. The defects are fixed locally with deterministic coverage; PR #13599 was not pushed or retargeted, PR #13611 was not modified, and Electron QA remains post-acceptance. - -## Gate - -| Metric | Limit | Actual | -| ----------- | ----- | ------ | -| Total files | ≤45 | **55** | -| Production | ≤28 | **32** | -| Tests | — | **21** | -| Docs | — | **2** | - -Base/dependency #13611 remains `d0271cb46f2871c201f39c51e35356efd4658eac`. The exact candidate exceeds the retained-candidate hard gate by 10 total and 4 production files; the final local correction SHA is reported with worker completion. - -## Defects found and fixed in this review - -- Folder-scope SSH migration collapsed missing and explicit `connectionId: null`, allowing inferred SSH ownership to overwrite explicit local project-group and folder provenance. -- Indexed filesystem authorization inspected the first lexicographic prefix candidate only, so a sibling such as `scope-archive` could hide the remote descendant `scope/repo` and authorize a legacy remote-only root locally. -- Quick-delete and context-menu deletion compared the active folder by bare `folder:`, deactivating a surviving same-ID owner; the correction also handles legacy active sessions without owner provenance. -- Bulk folder ownership rebuilt the same project-group index for each row, while project-group path-status snapshots rescanned subtrees and repos for each scope. Both now cache by immutable catalog identity, and persistence group deletion uses sets instead of repeated sibling scans. - -The migration, authorization, deletion, owner-index, and path-status performance regressions failed deterministically before their narrow fixes. The owner-index reproduction read group IDs 786,432 times, and the path-status reproduction read group IDs 264,704 times before correction. - -## Prior findings re-audited - -- Folder context-menu deletion retains the selected local, SSH, or runtime owner. -- Composer requests, source-repo traversal, cache identity, and runtime routing retain the selected project-group owner. -- Same-ID folder activation and metadata lookup/save resolve the exact owner. -- Legacy SSH folders still resolve a single unstamped legacy group without broadening generic membership or local filesystem authorization. -- Runtime project moves query the owning runtime even when the client group catalog is absent or stale. -- Rename, delete, reorder, collapse, path-status, and host-filter paths remain owner-qualified and fail closed when ambiguous. -- Folder workspaces remain supported independently of git worktrees. - -## Remote compatibility decision - -No new stream opcode or required wire field was introduced. Existing optional `ownerHostId` selectors remain additive, and paired-runtime RPC schemas can discard selectors they do not understand while operating on the runtime's own unambiguous catalog. The cross-version terminal-wire suite passes; no capability negotiation change is required. - -## Validation - -- Node `v24.18.0` node/CLI/web typechecks: pass. -- Node `v26.5.0` node/CLI/web typechecks: pass. -- Final changed-test matrix: 1,000 tests across 21 files on Node 26; the Node 24 matrix passed 998 tests before the final two legacy-owner assertions, followed by 39/39 final deletion tests. -- Path-status invalidation suite: 23/23 on Node 24 and Node 26. -- Cross-version terminal-wire suite: 4/4 on Node 24. -- Full lint, native/type-aware quality, 73 reliability gates, max-lines ratchet, skill, and localization gates: pass; only pre-existing `WorktreeJumpPalette` hook warnings were reported. -- Changed-code quality and React Doctor changed gates: 0 new findings across 55 changed files. -- Frozen geometry-boundary file hashes match the exact candidate. -- `oxfmt --check` on edited files and `git diff --check`: pass. - -## Remaining work - -The coordinator should review the local correction commit and decide how to resolve the 55/32 hard-gate overage. Because meaningful defects were found, do not push or retarget PR #13599 from this worker result; Electron QA remains intentionally deferred until the corrected candidate is accepted for another clean review. diff --git a/issue-12532-reduced-retention-table.md b/issue-12532-reduced-retention-table.md deleted file mode 100644 index 309b39cc13a..00000000000 --- a/issue-12532-reduced-retention-table.md +++ /dev/null @@ -1,58 +0,0 @@ -# #12532 reduced candidate — retained production file table - -**Base:** `d0271cb46f` (#13611 dependency; branch never modified) -**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, or dashboard cards. - -## 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/folder-workspace-path-status.ts` | Path-status requests cannot name owner → cross-owner cache/RPC mix | Request type is the IPC/renderer contract boundary | -| 4 | `src/renderer/src/store/slices/repos.ts` | Group/folder CRUD and path status can select or purge the wrong owner | Single store owns routing and optimistic catalog state | -| 5 | `src/renderer/src/store/slices/project-group-removal-targets.ts` | Contained-project removal enumerates bare subtree across hosts | Delete-with-projects needs the selected owner subtree before IPC | -| 6 | `src/main/ipc/repos.ts` | Main IPC drops optional `ownerHostId` | Wire entry for group/folder selectors | -| 7 | `src/preload/api-types.ts` | Renderer types omit optional `ownerHostId` | Additive preload contract boundary | -| 8 | `src/main/persistence.ts` | Host-local group/folder CRUD mutates the first bare-id row | Durable owner-aware mutation boundary | -| 9 | `src/main/ipc/filesystem-auth.ts` | Same-id remote folders can authorize local paths | Security boundary uses precomputed owner indexes | -| 10 | `src/main/project-groups/folder-workspace-path-status.ts` | Path status resolves bare folder/group to the wrong connection | Main path probe must fail closed on ambiguity | -| 11 | `src/renderer/src/components/sidebar/worktree-list-groups.ts` | Same-id groups share render and collapse keys | Row model mints both identities | -| 12 | `src/renderer/src/components/sidebar/project-group-sidebar-identity.ts` | Sidebar callers otherwise resolve owners inconsistently | One precomputed sidebar lookup index | -| 13 | `src/renderer/src/store/slices/worktrees.ts` | Bare same-id folder activation selects one owner while tabs still share `folder:` | A bounded guard fails closed without changing workspace/tab identity | -| 14 | `src/renderer/src/components/sidebar/WorktreeList.tsx` | Rendered owner-qualified headers still send bare mutations and composer targets | Interaction handlers must carry the owner selected by the row | -| 15 | `src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts` | Folder rows discard owner-qualified keys, causing virtual DOM reuse | TanStack row keys are minted at this boundary | -| 16 | `src/renderer/src/components/sidebar/host-section-rows.ts` | Runtime folders route from connection/default host instead of their runtime stamp | Host-section assignment owns this projection | -| 17 | `src/renderer/src/components/sidebar/worktree-list-host-filtering.ts` | Runtime path status and filter rebuilds can route the wrong owner | Shared filter/path-status routing must use one host resolver | -| 18 | `src/renderer/src/components/sidebar/WorktreeContextMenu.tsx` | Move/create/remove actions expose foreign-owner groups and send bare selectors | The context menu independently owns these action call sites | -| 19 | `src/renderer/src/lib/new-workspace-project-options.ts` | Duplicate group IDs produce duplicate composer values and first-owner selection | Option identity and parsing live here | -| 20 | `src/renderer/src/hooks/useComposerState.ts` | Selection immediately loses the option owner | Composer state must retain owner through selection and creation | -| 21 | `src/renderer/src/components/NewWorkspaceComposerModal.tsx` | Sidebar initial-group owner is dropped at the modal boundary | Modal data is the launch contract | - -**Production count: 21** (≤28). Changes to dependency-origin files are narrow and remain only on this stacked candidate branch. - -## Explicitly dropped vs broad 38-file tip (and why) - -| Dropped | Reason | -| ------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | -| WorktreeCard | Card workspace identity can keep bare id when unambiguous | -| dashboard-snapshot-workspaces | Dashboard identity is unrelated to project-group catalog ownership | -| 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, rendered-order | Existing contracts remain bare internally; the rendered-view gate guarantees one owner before drag | -| ui.ts draft owner field | Legacy drafts fail closed only for ambiguity; transient sidebar launches carry owner through modal data | -| repos-runtime-routing-fixture | Test-only support (count under tests if reintroduced) | - -## Test budget (focused only) - -Same-id / fail-closed / auth-index / path-status / owner update-delete / normalize — 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 2966d42b579..a98cfe9f60d 100644 --- a/src/main/ipc/filesystem-auth.test.ts +++ b/src/main/ipc/filesystem-auth.test.ts @@ -141,6 +141,20 @@ describe('filesystem auth worktree roots', () => { }) describe('filesystem-auth path containment', () => { + it('does not authorize a runtime-owned repo path on the local filesystem', () => { + const runtimePath = resolve('/runtime-only/repo') + const store = makeStore([ + { + ...repo, + path: runtimePath, + connectionId: null, + executionHostId: 'runtime:env-1' + } + ]) + + expect(isPathAllowed(runtimePath, store)).toBe(false) + }) + it('authorizes missing nested descendants under an allowed repo', async () => { const tempRoot = await mkdtemp(join(tmpdir(), 'orca-auth-missing-')) try { diff --git a/src/main/ipc/filesystem-auth.ts b/src/main/ipc/filesystem-auth.ts index 31ab3049470..50b01a56c3b 100644 --- a/src/main/ipc/filesystem-auth.ts +++ b/src/main/ipc/filesystem-auth.ts @@ -61,7 +61,7 @@ export function invalidateAuthorizedRootsCache(): void { function getLocalRepos(store: Store) { // Why: SSH repo paths are remote-host paths; treating them as local roots could authorize unrelated local folders or probe SSH-only paths. - return store.getRepos().filter((repo) => !repo.connectionId) + return store.getRepos().filter((repo) => getRepoExecutionHostId(repo) === LOCAL_EXECUTION_HOST_ID) } type FolderAuthIndex = { diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 3150bfe8e57..1e3e8578730 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -111,7 +111,7 @@ import { getProjectGroupOwnerHostId, resolveProjectGroupOwner } from '../../shared/project-groups' -import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../../shared/folder-workspace-project-group-resolution' +import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../../shared/folder-workspaces' import { joinRemotePath } from '../ssh/ssh-remote-platform' import { assertFolderWorkspacePathUsable, diff --git a/src/main/persistence.ts b/src/main/persistence.ts index c291985030c..5dea1b4abf9 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -230,7 +230,6 @@ import { resolveProjectGroupMembership, resolveProjectGroupOwner } from '../shared/project-groups' -import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../shared/folder-workspace-project-group-resolution' import { createNestedProjectGroupResolver } from './project-groups/nested-repo-import' import { mergeLegacyCommitMessageAiIntoSourceControlAi, @@ -264,7 +263,8 @@ import { ActiveViewPreference } from './active-view-preference' import { resolveFolderWorkspaceCatalogOwnerHostId, normalizeFolderWorkspaceName, - normalizeFolderWorkspaces + normalizeFolderWorkspaces, + resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../shared/folder-workspaces' import { folderWorkspaceKey, diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts b/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts index f7dd0e3747b..545a95c4c31 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { getContextMenuWorktreeExecutionHost, + findContextMenuRepo, getFolderWorkspaceContextMenuDeleteOwner, getContextMenuProjectGroupTargets, isContextWorktreeDeletable, @@ -21,11 +22,24 @@ import { findFolderWorkspaceMetaOwner } from './use-worktree-meta-workspace' import type { FolderWorkspace, ProjectGroup, + Repo, Worktree, WorktreeLineage, WorkspaceStatusDefinition } from '../../../../shared/types' +describe('findContextMenuRepo', () => { + const repo = (executionHostId: 'local' | 'runtime:env-1'): Repo => + ({ id: 'same-repo', executionHostId }) as Repo + + it('resolves an inactive duplicate repo from the clicked row host', () => { + const local = repo('local') + const runtime = repo('runtime:env-1') + + expect(findContextMenuRepo([local, runtime], 'same-repo', 'runtime:env-1')).toBe(runtime) + }) +}) + describe('getContextMenuProjectGroupTargets', () => { const group = (executionHostId: 'local' | 'runtime:env-1'): ProjectGroup => ({ id: 'same-id', executionHostId }) as ProjectGroup diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index e2a6c5b174c..ca030e845ec 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -32,7 +32,12 @@ import { } from 'lucide-react' import { useAppStore } from '@/store' import type { AppState } from '@/store/types' -import { useAllWorktrees, useRepoById, useRepoMap, useWorktreeMap } from '@/store/selectors' +import { + selectRepoByIdForActiveWorkspace, + useAllWorktrees, + useRepoMap, + useWorktreeMap +} from '@/store/selectors' import { cn } from '@/lib/utils' import type { ProjectGroup, @@ -66,6 +71,7 @@ import { translate } from '@/i18n/i18n' import { parseWorkspaceKey, worktreeWorkspaceKey } from '../../../../shared/workspace-scope' import { getRepoExecutionHostId, + LOCAL_EXECUTION_HOST_ID, normalizeExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' @@ -96,6 +102,17 @@ const DELETE_POSITION_RESTORE_STABLE_FRAMES = 6 // data-[state=closed] exit animation short; hold the subtree for its duration. const PARENT_PICKER_EXIT_ANIMATION_MS = 200 +export function findContextMenuRepo( + repos: Repo[], + repoId: string, + executionHostId: ExecutionHostId +): Repo | null { + return selectRepoByIdForActiveWorkspace( + { repos, activeRepoId: repoId, activeWorkspaceExecutionHostId: executionHostId }, + repoId + ) +} + // Why: stable empty sentinels let closed menu wrappers subscribe to a referentially // stable value instead of the high-churn maps that delete teardown replaces. The // selector returns these when the menu is closed, so the wrapper stays inert to @@ -368,7 +385,10 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ const moveProjectToGroup = useAppStore((s) => s.moveProjectToGroup) const deleteFolderWorkspace = useAppStore((s) => s.deleteFolderWorkspace) const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) - const repo = useRepoById(worktree.repoId, normalizeExecutionHostId(worktree.hostId) ?? undefined) + const repo = useAppStore((s) => { + const executionHostId = normalizeExecutionHostId(worktree.hostId) ?? LOCAL_EXECUTION_HOST_ID + return findContextMenuRepo(s.repos, worktree.repoId, executionHostId) + }) const projectGroupTargets = useMemo( () => (repo ? getContextMenuProjectGroupTargets(repo, projectGroups) : null), [projectGroups, repo] diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index f6cead0c028..169147f3984 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -2213,13 +2213,17 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp if (!pendingRevealWorktree) { return } + const pendingRevealOwnerHostId = + pendingRevealWorktree.worktreeId === activeWorktreeId + ? (activeWorkspaceExecutionHostId ?? undefined) + : undefined if (agentSendTargetWorktreeId !== pendingRevealWorktree.worktreeId) { const folderGroupKeys = getFolderWorkspaceRevealGroupKeys( pendingRevealWorktree.worktreeId, folderWorkspaces, projectGroups, - pendingRevealWorktree.ownerHostId + pendingRevealOwnerHostId ) if (folderGroupKeys.length > 0) { for (const groupKey of folderGroupKeys) { @@ -2283,13 +2287,13 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp worktrees, folderWorkspaces, projectGroups, - pendingRevealWorktree.ownerHostId + pendingRevealOwnerHostId ) const targetIndex = findPreferredRenderRowIndexForWorktree( renderRows, pendingRevealWorktree.worktreeId, pinnedDisplayPolicy, - pendingRevealWorktree.ownerHostId + pendingRevealOwnerHostId ) const outcome = resolvePendingSidebarReveal({ targetIndex, targetWorktreeStillExists }) if (outcome === 'scroll-and-clear') { @@ -2372,6 +2376,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp } }, [ pendingRevealWorktree, + activeWorktreeId, + activeWorkspaceExecutionHostId, agentSendTargetWorktreeId, groupBy, worktrees, @@ -6804,7 +6810,6 @@ const WorktreeList = React.memo(function WorktreeList({ } revealWorktreeInSidebar(currentSidebarWorktreeId, { behavior: 'smooth', - ...(activeWorkspaceExecutionHostId ? { ownerHostId: activeWorkspaceExecutionHostId } : {}), highlight: true, beginRename: (detail as { beginRename?: boolean } | undefined)?.beginRename === true }) diff --git a/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.test.ts b/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.test.ts index 90b2de45a65..0f43496ee54 100644 --- a/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.test.ts +++ b/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.test.ts @@ -77,52 +77,6 @@ describe('getFolderSourceRepos', () => { ).map((item) => item.id) ).toEqual(['runtime-by-path', 'runtime-by-group']) }) - - it('ignores descendant ids contributed only by another owner', () => { - const localRoot = group({ - id: 'root', - name: 'Local root', - parentPath: '/local', - connectionId: null, - executionHostId: 'local' - }) - const localUnrelated = group({ - id: 'foreign-child', - name: 'Local unrelated', - parentPath: '/other', - connectionId: null, - executionHostId: 'local' - }) - const runtimeRoot = group({ - id: 'root', - name: 'Runtime root', - parentPath: '/runtime', - connectionId: null, - executionHostId: 'runtime:env-1' - }) - const runtimeChild = group({ - id: 'foreign-child', - name: 'Runtime child', - parentPath: '/runtime/child', - connectionId: null, - executionHostId: 'runtime:env-1', - parentGroupId: 'root' - }) - const localRepo = repo('local-unrelated', { - path: '/outside/root', - connectionId: null, - executionHostId: 'local', - projectGroupId: 'foreign-child' - }) - - expect( - getFolderSourceRepos( - [localRepo], - [localRoot, localUnrelated, runtimeRoot, runtimeChild], - localRoot - ) - ).toEqual([]) - }) }) describe('getFolderWorkspacePrimaryActionLabel', () => { diff --git a/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.ts b/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.ts index d9a718421af..c6b920bd9d8 100644 --- a/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.ts +++ b/src/renderer/src/components/sidebar/folder-workspace-composer-helpers.ts @@ -49,11 +49,8 @@ export function getFolderSourceRepos( return EMPTY_REPOS } const folderPath = projectGroup.parentPath + const groupIds = getProjectGroupSubtreeIds(projectGroups, projectGroup.id) const projectGroupHostId = getProjectGroupExecutionHostId(projectGroup) - const ownerProjectGroups = projectGroups.filter( - (group) => getProjectGroupExecutionHostId(group) === projectGroupHostId - ) - const groupIds = getProjectGroupSubtreeIds(ownerProjectGroups, projectGroup.id) return repos.filter( (repo) => isGitRepoKind(repo) && diff --git a/src/renderer/src/components/sidebar/project-group-header-dom.test.ts b/src/renderer/src/components/sidebar/project-group-header-dom.test.ts index ee329bbb642..7696e288dec 100644 --- a/src/renderer/src/components/sidebar/project-group-header-dom.test.ts +++ b/src/renderer/src/components/sidebar/project-group-header-dom.test.ts @@ -333,7 +333,9 @@ describe('duplicate project-group header identity (#12532)', () => { new Set(['same-folder']) ) - expect(rows.find((row) => row.type === 'folder-workspace')?.key).toBe( + const folderRow = rows.find((row) => row.type === 'folder-workspace') + expect(folderRow?.key).toBe('folder-workspace:runtime%3Aenv-1:same-folder') + expect(folderRow && getRenderRowKey(folderRow)).toBe( 'folder-workspace:runtime%3Aenv-1:same-folder' ) }) diff --git a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts index d6583530e7f..77e844a8681 100644 --- a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts @@ -63,16 +63,6 @@ const stickyHeaderIndexes = getStickyHeaderIndexes(rows) const virtualItems = rows.map((_, index) => virtualItem(index, index * 100)) describe('getRenderRowKey', () => { - it('preserves owner-qualified folder workspace keys across hosts', () => { - const folderRow = (key: string): RenderRow => - ({ type: 'folder-workspace', key }) as unknown as RenderRow - - expect([ - getRenderRowKey(folderRow('folder-workspace:local:same-id')), - getRenderRowKey(folderRow('folder-workspace:runtime%3Aenv-1:same-id')) - ]).toEqual(['folder-workspace:local:same-id', 'folder-workspace:runtime%3Aenv-1:same-id']) - }) - it('scopes repeated group headers to their host section', () => { expect(getRenderRowKey(groupRow('workspace-status:in-progress', 'local'))).toBe( 'hdr:local:workspace-status:in-progress' diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 6d168b3da2a..236fbd6f1a0 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -133,6 +133,7 @@ import { findActionableFolderProjectGroup, getNewWorkspaceProjectGroupOptionId, getNewWorkspaceProjectGroupHostId, + getNewWorkspaceProjectGroupsForOwner, getProjectGroupSelectorFromNewWorkspaceOptionId, type NewWorkspaceProjectOption } from '@/lib/new-workspace-project-options' @@ -809,9 +810,16 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS selectedProjectGroupId ]) const isProjectGroupTarget = selectedProjectGroup !== null + const selectedProjectGroupCatalog = useMemo( + () => + selectedProjectGroup + ? getNewWorkspaceProjectGroupsForOwner(projectGroups, selectedProjectGroup) + : projectGroups, + [projectGroups, selectedProjectGroup] + ) const folderSourceRepos = useMemo( - () => getFolderSourceRepos(repos, projectGroups, selectedProjectGroup), - [projectGroups, repos, selectedProjectGroup] + () => getFolderSourceRepos(repos, selectedProjectGroupCatalog, selectedProjectGroup), + [repos, selectedProjectGroup, selectedProjectGroupCatalog] ) const parsedFolderTargetHost = parseExecutionHostId(selectedProjectGroup?.executionHostId) const folderTargetRuntimeEnvironmentId = @@ -2838,7 +2846,11 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS ) return } - const nextSourceRepo = getFolderSourceRepos(repos, projectGroups, nextProjectGroup)[0] + const nextSourceRepo = getFolderSourceRepos( + repos, + getNewWorkspaceProjectGroupsForOwner(projectGroups, nextProjectGroup), + nextProjectGroup + )[0] setSelectedProjectGroupId(nextProjectGroup.id) setSelectedProjectGroupOwnerHostId(getNewWorkspaceProjectGroupHostId(nextProjectGroup)) setProjectError(null) diff --git a/src/renderer/src/lib/new-workspace-project-options.test.ts b/src/renderer/src/lib/new-workspace-project-options.test.ts index a3071c6eb88..8dce8429b67 100644 --- a/src/renderer/src/lib/new-workspace-project-options.test.ts +++ b/src/renderer/src/lib/new-workspace-project-options.test.ts @@ -4,6 +4,7 @@ import { buildNewWorkspaceFolderSourceOptions, buildNewWorkspaceProjectOptions, findActionableFolderProjectGroup, + getNewWorkspaceProjectGroupsForOwner, getRepoIdFromNewWorkspaceFolderSourceOptionId, isNewWorkspaceProjectOptionQueryTooLarge, searchNewWorkspaceProjectOptions, @@ -588,4 +589,22 @@ describe('findActionableFolderProjectGroup', () => { }) ).toBe(duplicates[1]) }) + + it('limits folder source traversal to the selected group owner', () => { + const localRoot = group({ id: 'root', executionHostId: 'local' }) + const localUnrelated = group({ id: 'foreign-child', executionHostId: 'local' }) + const runtimeRoot = group({ id: 'root', executionHostId: 'runtime:env-1' }) + const runtimeChild = group({ + id: 'foreign-child', + executionHostId: 'runtime:env-1', + parentGroupId: 'root' + }) + + expect( + getNewWorkspaceProjectGroupsForOwner( + [localRoot, localUnrelated, runtimeRoot, runtimeChild], + localRoot + ) + ).toEqual([localRoot, localUnrelated]) + }) }) diff --git a/src/renderer/src/lib/new-workspace-project-options.ts b/src/renderer/src/lib/new-workspace-project-options.ts index 398bbe837c3..27577438ef1 100644 --- a/src/renderer/src/lib/new-workspace-project-options.ts +++ b/src/renderer/src/lib/new-workspace-project-options.ts @@ -204,6 +204,16 @@ export function getNewWorkspaceProjectGroupHostId(group: ProjectGroup): Executio return connectionId ? toSshExecutionHostId(connectionId) : LOCAL_EXECUTION_HOST_ID } +export function getNewWorkspaceProjectGroupsForOwner( + projectGroups: readonly ProjectGroup[], + projectGroup: ProjectGroup +): ProjectGroup[] { + const ownerHostId = getNewWorkspaceProjectGroupHostId(projectGroup) + return projectGroups.filter( + (candidate) => getNewWorkspaceProjectGroupHostId(candidate) === ownerHostId + ) +} + /** * A folder workspace can only be backed by a group whose host is still actionable — a removed * or unreachable host must not be selectable, restorable from a draft, or kept once selected. diff --git a/src/renderer/src/lib/worktree-activation.ts b/src/renderer/src/lib/worktree-activation.ts index d483c3dd0b2..e5a18a5036b 100644 --- a/src/renderer/src/lib/worktree-activation.ts +++ b/src/renderer/src/lib/worktree-activation.ts @@ -274,12 +274,9 @@ export function activateAndRevealFolderWorkspace( const primaryTabId = ensureFolderWorkspaceInitialTerminal(folderWorkspace, opts?.startup) if (opts?.sidebarRevealBehavior) { - state.revealWorktreeInSidebar(workspaceKey, { - behavior: opts.sidebarRevealBehavior, - ownerHostId - }) + state.revealWorktreeInSidebar(workspaceKey, { behavior: opts.sidebarRevealBehavior }) } else { - state.revealWorktreeInSidebar(workspaceKey, { ownerHostId }) + state.revealWorktreeInSidebar(workspaceKey) } return { primaryTabId } diff --git a/src/renderer/src/store/selectors.test.ts b/src/renderer/src/store/selectors.test.ts index 68e2986a4f9..345a25c0eec 100644 --- a/src/renderer/src/store/selectors.test.ts +++ b/src/renderer/src/store/selectors.test.ts @@ -10,7 +10,6 @@ import { getWorktreeMapFromState, resetFloatingVisibleTabCountSelectorCacheForTest, selectRepoByIdForActiveWorkspace, - selectRepoByIdForExecutionHost, selectFloatingVisibleTabCount, selectFloatingWorkspaceHasUnread } from './selectors' @@ -320,29 +319,6 @@ describe('store selectors', () => { expect(activeRepo ? isGitRepoKind(activeRepo) : false).toBe(true) }) - it('resolves an inactive duplicate repo from the clicked row host', () => { - const local = makeRepo({ - id: 'same-repo', - path: '/local/repo', - displayName: 'local', - executionHostId: 'local' - }) - const runtime = makeRepo({ - id: 'same-repo', - path: '/runtime/repo', - displayName: 'runtime', - executionHostId: toRuntimeExecutionHostId('env-1') - }) - - expect( - selectRepoByIdForExecutionHost( - { repos: [local, runtime] }, - 'same-repo', - toRuntimeExecutionHostId('env-1') - ) - ).toBe(runtime) - }) - it('fails a paired-hub repo fallback closed when rival HUB rows share the repo ID', () => { const repoForHub = (environmentId: string) => makeRepo({ diff --git a/src/renderer/src/store/selectors.ts b/src/renderer/src/store/selectors.ts index e41b2133859..b955666760f 100644 --- a/src/renderer/src/store/selectors.ts +++ b/src/renderer/src/store/selectors.ts @@ -187,39 +187,28 @@ export function selectRepoByIdForActiveWorkspace( } const repo = getCachedRepoMap(state.repos).get(repoId) ?? null if (repoId === state.activeRepoId && state.activeWorkspaceExecutionHostId) { - return selectRepoByIdForExecutionHost(state, repoId, state.activeWorkspaceExecutionHostId) + const repoCandidates = state.repos.filter((candidate) => candidate.id === repoId) + const hostMatch = repoCandidates.find( + (candidate) => getRepoExecutionHostId(candidate) === state.activeWorkspaceExecutionHostId + ) + if (hostMatch) { + return hostMatch + } + // Why: withRepoHostOwnership keeps a paired-hub worktree on its own SSH host while the repo + // stays hub-owned, so that one mismatch still names the right repo; every other stays closed. + if (parseExecutionHostId(state.activeWorkspaceExecutionHostId)?.kind !== 'ssh') { + return null + } + const pairedHubRepos = repoCandidates.filter( + (candidate) => parseExecutionHostId(getRepoExecutionHostId(candidate))?.kind === 'runtime' + ) + return pairedHubRepos.length === 1 ? pairedHubRepos[0] : null } return repo } -export function selectRepoByIdForExecutionHost( - state: Pick, - repoId: string, - executionHostId: ExecutionHostId -): Repo | null { - const repoCandidates = state.repos.filter((candidate) => candidate.id === repoId) - const hostMatch = repoCandidates.find( - (candidate) => getRepoExecutionHostId(candidate) === executionHostId - ) - if (hostMatch) { - return hostMatch - } - // Why: paired-hub worktrees can name a private SSH host while their repo remains hub-owned. - if (parseExecutionHostId(executionHostId)?.kind !== 'ssh') { - return null - } - const pairedHubRepos = repoCandidates.filter( - (candidate) => parseExecutionHostId(getRepoExecutionHostId(candidate))?.kind === 'runtime' - ) - return pairedHubRepos.length === 1 ? pairedHubRepos[0] : null -} - -export const useRepoById = (repoId: string | null, executionHostId?: ExecutionHostId) => - useAppStore((s) => - repoId && executionHostId - ? selectRepoByIdForExecutionHost(s, repoId, executionHostId) - : selectRepoByIdForActiveWorkspace(s, repoId) - ) +export const useRepoById = (repoId: string | null) => + useAppStore((s) => selectRepoByIdForActiveWorkspace(s, repoId)) export const useProjectHostSetupProjection = () => useAppStore((s) => getProjectHostSetupProjectionFromState(s)) 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 b3835d57152..e6a60daf929 100644 --- a/src/renderer/src/store/slices/project-group-removal-targets.ts +++ b/src/renderer/src/store/slices/project-group-removal-targets.ts @@ -9,7 +9,7 @@ import { resolveProjectGroupOwner, type ProjectGroupOwnerIndex } from '../../../../shared/project-groups' -import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../../../../shared/folder-workspace-project-group-resolution' +import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../../../../shared/folder-workspaces' import { getRepoHostIdentity } from './repo-host-identity' export type ProjectGroupRemovalProjectTarget = { diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 22d116f7e36..81dacbf01fa 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -103,9 +103,9 @@ import { getFolderWorkspaceIdentity, getFolderWorkspaceOwnerIdentity, resolveFolderWorkspaceCatalogOwnerHostId, - resolveFolderWorkspaceCatalogOwnerHostIdFromIndex + resolveFolderWorkspaceCatalogOwnerHostIdFromIndex, + resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../../../../shared/folder-workspaces' -import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from '../../../../shared/folder-workspace-project-group-resolution' import { formatFolderWorkspaceCreateError } from '../../lib/folder-workspace-path-status' import { getEnvironmentSshStateGeneration } from './runtime-environment-ssh' import { getRuntimeEnvironmentConnectionGeneration } from './runtime-status' diff --git a/src/renderer/src/store/slices/ui.test.ts b/src/renderer/src/store/slices/ui.test.ts index 7ffae75ef9d..b8bccc235a4 100644 --- a/src/renderer/src/store/slices/ui.test.ts +++ b/src/renderer/src/store/slices/ui.test.ts @@ -1298,7 +1298,6 @@ describe('createUISlice hydratePersistedUI', () => { store.getState().revealWorktreeInSidebar('repo1::/feature', { behavior: 'smooth', - ownerHostId: 'runtime:env-1', highlight: true, beginRename: true }) @@ -1306,7 +1305,6 @@ describe('createUISlice hydratePersistedUI', () => { expect(store.getState().pendingRevealWorktree).toEqual({ worktreeId: 'repo1::/feature', behavior: 'smooth', - ownerHostId: 'runtime:env-1', highlight: true, beginRename: true }) diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 07a82c7b0de..e1b9c89e6ab 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -137,7 +137,6 @@ import { getRepoHostIdentity } from './repo-host-identity' export type PendingSidebarWorktreeReveal = { worktreeId: string behavior: 'auto' | 'smooth' - ownerHostId?: ExecutionHostId highlight?: boolean beginRename?: boolean } @@ -963,7 +962,6 @@ export type UISlice = { worktreeId: string, options?: { behavior?: PendingSidebarWorktreeReveal['behavior'] - ownerHostId?: ExecutionHostId highlight?: boolean beginRename?: boolean } @@ -2374,7 +2372,6 @@ export const createUISlice: StateCreator = (set, get) pendingRevealWorktree: { worktreeId, behavior: options?.behavior ?? 'smooth', - ...(options?.ownerHostId ? { ownerHostId: options.ownerHostId } : {}), ...(options?.highlight ? { highlight: true } : {}), ...(options?.beginRename ? { beginRename: true } : {}) } diff --git a/src/shared/folder-workspace-project-group-resolution.ts b/src/shared/folder-workspace-project-group-resolution.ts deleted file mode 100644 index 1ca2e6754b6..00000000000 --- a/src/shared/folder-workspace-project-group-resolution.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { normalizeExecutionHostId } from './execution-host' -import { - resolveFolderWorkspaceProjectGroup, - resolveProjectGroupOwner, - type ProjectGroupOwnerIndex -} from './project-groups' -import type { FolderWorkspace, ProjectGroup } from './types' - -export function resolveFolderWorkspaceProjectGroupWithLegacySsh( - index: ProjectGroupOwnerIndex, - workspace: Pick -): ProjectGroup | null { - const strict = resolveFolderWorkspaceProjectGroup(index, workspace) - if (strict || !workspace.connectionId) { - return strict - } - const group = resolveProjectGroupOwner(index, workspace.projectGroupId) - return group && - group.connectionId === undefined && - !normalizeExecutionHostId(group.executionHostId) - ? group - : null -} diff --git a/src/shared/folder-workspaces.ts b/src/shared/folder-workspaces.ts index ba6e83c5d04..6b4c19e180a 100644 --- a/src/shared/folder-workspaces.ts +++ b/src/shared/folder-workspaces.ts @@ -8,9 +8,9 @@ import { buildProjectGroupOwnerIndex, getFolderWorkspaceProjectGroupOwnerHostId, resolveFolderWorkspaceProjectGroup, + resolveProjectGroupOwner, type ProjectGroupOwnerIndex } from './project-groups' -import { resolveFolderWorkspaceProjectGroupWithLegacySsh } from './folder-workspace-project-group-resolution' import type { FolderWorkspace, ProjectGroup } from './types' import { isTuiAgent } from './tui-agent-config' import { normalizeStoredTaskSourceContext } from './task-source-context' @@ -19,6 +19,22 @@ import { isWorkspaceLinkedItemSourceContextMatch } from './workspace-linked-item const projectGroupOwnerIndexCache = new WeakMap() +export function resolveFolderWorkspaceProjectGroupWithLegacySsh( + index: ProjectGroupOwnerIndex, + workspace: Pick +): ProjectGroup | null { + const strict = resolveFolderWorkspaceProjectGroup(index, workspace) + if (strict || !workspace.connectionId) { + return strict + } + const group = resolveProjectGroupOwner(index, workspace.projectGroupId) + return group && + group.connectionId === undefined && + !normalizeExecutionHostId(group.executionHostId) + ? group + : null +} + function getProjectGroupOwnerIndex(projectGroups: readonly ProjectGroup[]): ProjectGroupOwnerIndex { const cached = projectGroupOwnerIndexCache.get(projectGroups) if (cached) { diff --git a/src/shared/project-groups.test.ts b/src/shared/project-groups.test.ts index 7a9a5ac1740..291ff4f0491 100644 --- a/src/shared/project-groups.test.ts +++ b/src/shared/project-groups.test.ts @@ -188,6 +188,18 @@ describe('project-groups', () => { ).toHaveLength(1) }) + it('does not reinterpret explicit local provenance as a unique SSH owner', () => { + const ssh = projectGroup({ id: 'same-id', connectionId: 'builder' }) + const index = buildProjectGroupOwnerIndex([ssh]) + + expect( + resolveFolderWorkspaceProjectGroup(index, { + projectGroupId: 'same-id', + connectionId: null + }) + ).toBeNull() + }) + it('rejects folder workspaces stamped for missing foreign owners', () => { const local = projectGroup({ id: 'same-id', diff --git a/src/shared/project-groups.ts b/src/shared/project-groups.ts index 720c529695f..bbbd4129043 100644 --- a/src/shared/project-groups.ts +++ b/src/shared/project-groups.ts @@ -97,11 +97,7 @@ export function resolveFolderWorkspaceProjectGroup( const ownerHostId = workspace.connectionId ? toSshExecutionHostId(workspace.connectionId) : LOCAL_EXECUTION_HOST_ID - const group = resolveProjectGroupMembership(index, workspace.projectGroupId, ownerHostId) - // Why: null predates owner stamps; a named SSH owner is authoritative. - return workspace.connectionId === null - ? (group ?? resolveProjectGroupOwner(index, workspace.projectGroupId)) - : group + return resolveProjectGroupMembership(index, workspace.projectGroupId, ownerHostId) } return resolveProjectGroupOwner(index, workspace.projectGroupId) }