fix(sidebar): fail closed on ambiguous folder owners

This commit is contained in:
Brennan Benson
2026-08-10 13:00:54 -07:00
parent d83f9aae93
commit cd7cf2fd17
21 changed files with 695 additions and 732 deletions
+38 -21
View File
@@ -1,29 +1,46 @@
# Reduced #12532 owner-identity candidate
# #12532 reduced candidate final review
## Verdict
**NOT CLEAN.** The reviewed candidate `d83f9aae932c2d0eddd46e70cfd03e0cb78ac1fa` contained correctness, scope, and bounded-performance defects. Issue-scoped fixes are committed locally for another fresh review; PR #13599 was not pushed or retargeted.
## Gate
| Metric | Limit | Actual |
| --- | --- | --- |
| Total files | ≤45 | **28** |
| Production | ≤28 | **15** |
| Tests | — | **12** |
| Docs | — | **1** retention table |
| Metric | Limit | Actual |
| ----------- | ----- | ------ |
| Total files | ≤45 | **26** |
| Production | ≤28 | **13** |
| Tests | — | **11** |
| Docs | — | **2** |
## SHAs
| Role | SHA |
| --- | --- |
| Base (#13611) | `d0271cb46f2871c201f39c51e35356efd4658eac` |
| Archived broad 66-file tip | `7c64936ce7` / docs `39d2585a90` on `archive/issue-12532-broad-66file-7c64936-20260810-122250` |
| Reduced candidate | `253f8c3f9e48dfec505d7ec96e104f099cf365a1` on `brennanb2025/issue-12532-owner-identity-narrow` (unpushed) |
## Retention table
`issue-12532-reduced-retention-table.md`
| Role | SHA |
| --------------------- | ---------------------------------------------------------------------------------------------- |
| Base (#13611) | `d0271cb46f2871c201f39c51e35356efd4658eac` |
| Reviewed candidate | `d83f9aae932c2d0eddd46e70cfd03e0cb78ac1fa` |
| Candidate code commit | `253f8c3f9e48dfec505d7ec96e104f099cf365a1` |
| Archived broad tip | `7c64936ce7` / docs `39d2585a90` on `archive/issue-12532-broad-66file-7c64936-20260810-122250` |
## Findings fixed
- Removed owner-qualified folder identity propagation from `WorkspaceScope`, worktree/tab/session maps, active workspace IDs, and editor/file routing. Unambiguous folder activation keeps `folder:folder-workspace-1`; same-id ambiguity fails closed.
- Added optional owner selectors only at folder CRUD and path-status preload/IPC/persistence seams. Legacy unambiguous calls keep their prior payload shape, while ambiguous mutations require an exact owner.
- Made persistence folder lookup/update/delete and folder path-status selection fail closed on bare same-id ambiguity without purging a surviving sibling's shared bare session state.
- Scoped project-group removal, move, path auth, and sidebar row/collapse identity by owner without changing unrelated repo/worktree IDs or #13611 header geometry.
- Replaced quadratic filesystem-auth subtree materialization with a linear owner-indexed aggregation plus sorted path indexes; cycles fail closed and a 4,096-level adversarial chain remains bounded.
- Removed the sidebar's per-row full catalog scan by precomputing ambiguous group IDs.
- Corrected owner-qualified folder path-status cache eviction to use the same canonical identity codec as cache creation.
## Validation
- Focused + legacy identity/sticky suite: **1110 passed** (20 files)
- typecheck web/node: pass
- oxlint production files: 0 errors
- max-lines ratchet / reliability gates: pass
- #13611 virtual-rows / host-section-rows: **unchanged**
## Remaining QA
Independent review; multi-host UI smoke for same-id collapse/CRUD; full monorepo CI optional.
- Focused identity, CRUD, path-status, auth, persistence, activity/session, dashboard, and legacy suites: **866 passed** across **23 unique files**; the corrected owner-cache fixture was rerun separately (**5 passed**).
- Node `v24.19.0` node/CLI/web typechecks: pass.
- Changed-code quality: 0 findings; max-lines ratchet and reliability gates: pass.
- `git diff --check` and formatting: pass.
- Filesystem-auth adversarial 4,096-level chain and cycle fixtures: pass.
- #13611 `worktree-list-virtual-rows`, `host-section-rows`, and `WorktreeList` geometry: byte-identical to `d0271cb46f2871c201f39c51e35356efd4658eac`.
## Remaining work
Another independent reviewer must review the fixed local commit. Because this review found actionable defects, do not push or retarget PR #13599 yet; Electron and SSH multi-host QA remain post-review/post-push work.
+29 -31
View File
@@ -1,7 +1,7 @@
# #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`
**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
@@ -13,40 +13,38 @@
## 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:<id>`; 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:<id>` 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 |
| # | 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:<id>` | A bounded guard fails closed without changing workspace/tab identity |
**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.
**Production count: 13** (≤28). WorktreeList and all header geometry stay on the dependency base; owner selectors stop at sidebar, store, IPC, persistence, path-status, and auth seams.
## 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:<id>` when unambiguous |
| folder-workspace-connection, composer helpers/path-status | Not required for adversarial same-id group/folder CRUD |
| Dropped | Reason |
| --------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- |
| WorktreeCard / WorktreeContextMenu | Store fail-closed + optional owner covers delete; card 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:<id>` 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) |
| 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.
Same-id / fail-closed / auth-index / path-status / owner update-delete / normalize — no mass expectation rewrites of sticky geometry or bare-key consumers.
+28
View File
@@ -348,6 +348,34 @@ describe('filesystem-auth path containment', () => {
expect(elapsedMs).toBeLessThan(250)
})
it('bounds deep legacy folder hierarchies without materializing every subtree', () => {
const groupCount = 4_096
const projectGroups = Array.from({ length: groupCount }, (_, index) =>
makeProjectGroup({
id: `group-${index}`,
parentPath: index === 0 ? '/legacy/root' : null,
parentGroupId: index === 0 ? null : `group-${index - 1}`
})
)
const repos = [
{
...repo,
id: 'remote-leaf',
path: '/outside/repo',
projectGroupId: `group-${groupCount - 1}`,
connectionId: 'builder'
} as Repo
]
const store = makeStore(repos, { projectGroups, folderWorkspaces: [] })
const started = performance.now()
const allowed = isPathAllowed('/legacy/root', store)
const elapsedMs = performance.now() - started
expect(allowed).toBe(false)
expect(elapsedMs).toBeLessThan(250)
})
it.skipIf(process.platform === 'win32')(
'rejects missing descendants under a symlinked ancestor outside the repo',
async () => {
+196 -129
View File
@@ -5,16 +5,15 @@ import { realpath } from 'node:fs/promises'
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 { normalizeRuntimePathForComparison } from '../../shared/cross-platform-path'
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'
import { resolveFolderWorkspaceCatalogOwnerHostId } 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.'
@@ -67,13 +66,18 @@ function getLocalRepos(store: Store) {
type FolderAuthIndex = {
projectGroupIndex: ReturnType<typeof buildProjectGroupOwnerIndex>
ownerGroupIdsByOwner: Map<string, ProjectGroup[]>
reposByOwner: Map<string, Repo[]>
reposByOwnerAndGroupId: Map<string, Repo[]>
ownerSubtreeIds: Map<string, Map<string, Set<string>>>
unscopedSubtreeIds: Map<string, Set<string>>
repos: readonly Repo[]
projectGroups: readonly ProjectGroup[]
groupMembership: ReadonlyMap<string, GroupMembershipSummary>
legacyGroupMembership: ReadonlyMap<string, GroupMembershipSummary>
allRepoPaths: readonly string[]
localOwnerRepoPaths: readonly string[]
unconnectedRepoPathsByOwner: ReadonlyMap<string, readonly string[]>
}
type GroupMembershipSummary = {
repoCount: number
hasLocalOwner: boolean
hasUnconnectedRepo: boolean
unsafeCycle: boolean
}
function ownerGroupKey(ownerHostId: string, groupId: string): string {
@@ -85,118 +89,183 @@ function buildFolderAuthIndex(
repos: readonly Repo[]
): FolderAuthIndex {
const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups)
const ownerGroupIdsByOwner = new Map<string, ProjectGroup[]>()
const childrenByOwnerParent = new Map<string, string[]>()
const parentByGroup = new Map<string, string>()
const remainingChildren = new Map<string, number>()
const groupMembership = new Map<string, GroupMembershipSummary>()
const legacyParentsByGroup = new Map<string, Set<string>>()
const legacyChildrenByGroup = new Map<string, Set<string>>()
const legacyGroupMembership = new Map<string, GroupMembershipSummary>()
for (const group of projectGroups) {
const ownerHostId = getProjectGroupOwnerHostId(group)
const ownerGroups = ownerGroupIdsByOwner.get(ownerHostId) ?? []
ownerGroups.push(group)
ownerGroupIdsByOwner.set(ownerHostId, ownerGroups)
const key = ownerGroupKey(ownerHostId, group.id)
remainingChildren.set(key, 0)
groupMembership.set(key, emptyGroupMembershipSummary())
if (!legacyGroupMembership.has(group.id)) {
legacyGroupMembership.set(group.id, emptyGroupMembershipSummary())
}
if (group.parentGroupId) {
const parents = legacyParentsByGroup.get(group.id) ?? new Set<string>()
parents.add(group.parentGroupId)
legacyParentsByGroup.set(group.id, parents)
const children = legacyChildrenByGroup.get(group.parentGroupId) ?? new Set<string>()
children.add(group.id)
legacyChildrenByGroup.set(group.parentGroupId, children)
}
}
for (const group of projectGroups) {
if (!group.parentGroupId) {
continue
}
const ownerHostId = getProjectGroupOwnerHostId(group)
const key = ownerGroupKey(ownerHostId, group.id)
const parentKey = ownerGroupKey(ownerHostId, group.parentGroupId)
const children = childrenByOwnerParent.get(parentKey) ?? []
children.push(group.id)
childrenByOwnerParent.set(parentKey, children)
}
const ownerSubtreeIds = new Map<string, Map<string, Set<string>>>()
for (const [ownerHostId, groups] of ownerGroupIdsByOwner) {
const subtreeByRoot = new Map<string, Set<string>>()
for (const group of groups) {
if (subtreeByRoot.has(group.id)) {
continue
}
const subtree = new Set<string>()
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<string, Set<string>>()
for (const group of projectGroups) {
if (!unscopedSubtreeIds.has(group.id)) {
unscopedSubtreeIds.set(group.id, getProjectGroupSubtreeIds(projectGroups, group.id))
if (remainingChildren.has(parentKey)) {
parentByGroup.set(key, parentKey)
remainingChildren.set(parentKey, (remainingChildren.get(parentKey) ?? 0) + 1)
}
}
const reposByOwner = new Map<string, Repo[]>()
const reposByOwnerAndGroupId = new Map<string, Repo[]>()
const allRepoPaths: string[] = []
const localOwnerRepoPaths: string[] = []
const unconnectedRepoPathsByOwner = new Map<string, string[]>()
for (const repo of repos) {
const ownerHostId = getRepoExecutionHostId(repo)
const ownerRepos = reposByOwner.get(ownerHostId) ?? []
ownerRepos.push(repo)
reposByOwner.set(ownerHostId, ownerRepos)
const normalizedPath = normalizeRuntimePathForComparison(repo.path)
allRepoPaths.push(normalizedPath)
if (ownerHostId === LOCAL_EXECUTION_HOST_ID) {
localOwnerRepoPaths.push(normalizedPath)
}
if (!repo.connectionId) {
const ownerPaths = unconnectedRepoPathsByOwner.get(ownerHostId) ?? []
ownerPaths.push(normalizedPath)
unconnectedRepoPathsByOwner.set(ownerHostId, ownerPaths)
}
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<Repo>()
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)
const summary = groupMembership.get(key)
if (summary) {
summary.repoCount++
summary.hasLocalOwner ||= ownerHostId === LOCAL_EXECUTION_HOST_ID
summary.hasUnconnectedRepo ||= !repo.connectionId
}
const legacySummary = legacyGroupMembership.get(repo.projectGroupId)
if (legacySummary) {
legacySummary.repoCount++
legacySummary.hasLocalOwner ||= ownerHostId === LOCAL_EXECUTION_HOST_ID
legacySummary.hasUnconnectedRepo ||= !repo.connectionId
}
}
}
return candidates
}
function collectOwnerPathMembershipRepos(
index: FolderAuthIndex,
folderPath: string,
ownerHostId: string,
exclude: ReadonlySet<Repo>
): Repo[] {
const candidates: Repo[] = []
for (const repo of index.reposByOwner.get(ownerHostId) ?? []) {
if (exclude.has(repo)) {
const pending = [...remainingChildren]
.filter(([, childCount]) => childCount === 0)
.map(([key]) => key)
let processed = 0
while (pending.length > 0) {
const key = pending.pop()!
processed++
const parentKey = parentByGroup.get(key)
if (!parentKey) {
continue
}
if (isPathInsideOrEqual(folderPath, repo.path)) {
candidates.push(repo)
mergeGroupMembership(groupMembership.get(parentKey)!, groupMembership.get(key)!)
const nextChildCount = (remainingChildren.get(parentKey) ?? 1) - 1
remainingChildren.set(parentKey, nextChildCount)
if (nextChildCount === 0) {
pending.push(parentKey)
}
}
return candidates
if (processed !== groupMembership.size) {
for (const [key, childCount] of remainingChildren) {
if (childCount > 0) {
groupMembership.get(key)!.unsafeCycle = true
}
}
}
const legacyRemainingChildren = new Map(
[...legacyGroupMembership.keys()].map((groupId) => [
groupId,
legacyChildrenByGroup.get(groupId)?.size ?? 0
])
)
const legacyPending = [...legacyRemainingChildren]
.filter(([, childCount]) => childCount === 0)
.map(([groupId]) => groupId)
let legacyProcessed = 0
while (legacyPending.length > 0) {
const groupId = legacyPending.pop()!
legacyProcessed++
for (const parentId of legacyParentsByGroup.get(groupId) ?? []) {
const parentSummary = legacyGroupMembership.get(parentId)
if (!parentSummary) {
continue
}
mergeGroupMembership(parentSummary, legacyGroupMembership.get(groupId)!)
const nextChildCount = (legacyRemainingChildren.get(parentId) ?? 1) - 1
legacyRemainingChildren.set(parentId, nextChildCount)
if (nextChildCount === 0) {
legacyPending.push(parentId)
}
}
}
if (legacyProcessed !== legacyGroupMembership.size) {
for (const [groupId, childCount] of legacyRemainingChildren) {
if (childCount > 0) {
legacyGroupMembership.get(groupId)!.unsafeCycle = true
}
}
}
allRepoPaths.sort()
localOwnerRepoPaths.sort()
for (const paths of unconnectedRepoPathsByOwner.values()) {
paths.sort()
}
return {
projectGroupIndex,
groupMembership,
legacyGroupMembership,
allRepoPaths,
localOwnerRepoPaths,
unconnectedRepoPathsByOwner
}
}
function emptyGroupMembershipSummary(): GroupMembershipSummary {
return {
repoCount: 0,
hasLocalOwner: false,
hasUnconnectedRepo: false,
unsafeCycle: false
}
}
function mergeGroupMembership(target: GroupMembershipSummary, child: GroupMembershipSummary): void {
target.repoCount += child.repoCount
target.hasLocalOwner ||= child.hasLocalOwner
target.hasUnconnectedRepo ||= child.hasUnconnectedRepo
target.unsafeCycle ||= child.unsafeCycle
}
function hasIndexedPathInside(rootPath: string, sortedPaths: readonly string[]): boolean {
const root = normalizeRuntimePathForComparison(rootPath)
let low = 0
let high = sortedPaths.length
while (low < high) {
const middle = (low + high) >>> 1
if (sortedPaths[middle] < root) {
low = middle + 1
} else {
high = middle
}
}
const candidate = sortedPaths[low]
if (!candidate) {
return false
}
const prefix = root === '/' || /^[a-z]:\/$/i.test(root) ? root : `${root}/`
return candidate === root || candidate.startsWith(prefix)
}
function isRemoteOnlyFolderScopeWithIndex(
@@ -209,42 +278,34 @@ function isRemoteOnlyFolderScopeWithIndex(
if (ownerHostId !== LOCAL_EXECUTION_HOST_ID) {
return true
}
const summary =
index.groupMembership.get(ownerGroupKey(ownerHostId, projectGroupId)) ??
emptyGroupMembershipSummary()
if (summary.unsafeCycle) {
return true
}
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
}
const legacySummary =
index.legacyGroupMembership.get(projectGroupId) ?? emptyGroupMembershipSummary()
if (legacySummary.unsafeCycle) {
return true
}
if (sawLegacyCandidate && !sawLocalLegacyCandidate) {
const hasAnyCandidate =
legacySummary.repoCount > 0 || hasIndexedPathInside(folderPath, index.allRepoPaths)
const hasLocalCandidate =
legacySummary.hasLocalOwner || hasIndexedPathInside(folderPath, index.localOwnerRepoPaths)
if (hasAnyCandidate && !hasLocalCandidate) {
return true
}
}
const groupCandidates = collectOwnerGroupMembershipRepos(index, projectGroupId, ownerHostId)
if (groupCandidates.length === 0) {
if (summary.repoCount === 0) {
// Why: empty membership means no remote-linked evidence; authorize local folder scopes without path scans.
return false
}
if (groupCandidates.some((repo) => !repo.connectionId)) {
if (summary.hasUnconnectedRepo) {
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)) {
if (hasIndexedPathInside(folderPath, index.unconnectedRepoPathsByOwner.get(ownerHostId) ?? [])) {
return false
}
return true
@@ -276,10 +337,16 @@ function getLocalFolderScopeRoots(store: Store): string[] {
}
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.
if (!group && index.projectGroupIndex.byId.has(workspace.projectGroupId)) {
continue
}
// Why: narrow Store doubles may omit groups; folder roots stay additive from the workspace catalog.
const ownerHostId = group
? getProjectGroupOwnerHostId(group)
: getFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups)
: resolveFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups)
if (!ownerHostId) {
continue
}
if (
!isRemoteOnlyFolderScopeWithIndex(
index,
+26
View File
@@ -38,6 +38,8 @@ const {
updateProjectGroup: vi.fn(),
deleteProjectGroup: vi.fn(),
moveProjectToGroup: vi.fn(),
updateFolderWorkspace: vi.fn(),
removeFolderWorkspace: vi.fn(),
getSshTarget: vi.fn()
},
mockGitProvider: {
@@ -179,6 +181,8 @@ describe('projectGroups IPC validation', () => {
mockStore.updateProjectGroup.mockReset()
mockStore.deleteProjectGroup.mockReset()
mockStore.moveProjectToGroup.mockReset()
mockStore.updateFolderWorkspace.mockReset()
mockStore.removeFolderWorkspace.mockReset()
mockStore.addRepo.mockReset()
mockStore.getProjects.mockReset().mockReturnValue([])
mockStore.getProjectHostSetups.mockReset().mockReturnValue([])
@@ -412,6 +416,28 @@ describe('projectGroups IPC validation', () => {
expect(mockStore.moveProjectToGroup).not.toHaveBeenCalled()
})
it('forwards optional folder workspace mutation ownership to persistence', () => {
mockStore.updateFolderWorkspace.mockReturnValue({ id: 'same-folder' })
mockStore.removeFolderWorkspace.mockReturnValue(true)
handlers.get('folderWorkspaces:update')!(null, {
folderWorkspaceId: 'same-folder',
ownerHostId: 'ssh:builder',
updates: { name: 'Remote' }
})
handlers.get('folderWorkspaces:delete')!(null, {
folderWorkspaceId: 'same-folder',
ownerHostId: 'local'
})
expect(mockStore.updateFolderWorkspace).toHaveBeenCalledWith(
'same-folder',
{ name: 'Remote' },
'ssh:builder'
)
expect(mockStore.removeFolderWorkspace).toHaveBeenCalledWith('same-folder', 'local')
})
it('scans nested repositories over a connected SSH filesystem', async () => {
mockGitProvider.isGitRepoAsync.mockImplementation(async (path: string) => ({
isRepo: path === '/srv/platform/api',
+12 -5
View File
@@ -949,6 +949,7 @@ const FolderWorkspaceCreateArgs = z
const FolderWorkspaceUpdateArgs = z.object({
folderWorkspaceId: z.string().min(1),
ownerHostId: ProjectGroupOwnerHostId.optional(),
updates: z
.object({
name: z.string().optional(),
@@ -971,13 +972,15 @@ const FolderWorkspaceUpdateArgs = z.object({
})
const FolderWorkspaceSelectorArgs = z.object({
folderWorkspaceId: z.string().min(1)
folderWorkspaceId: z.string().min(1),
ownerHostId: ProjectGroupOwnerHostId.optional()
})
const FolderWorkspacePathStatusArgs = z.discriminatedUnion('scope', [
z.object({
scope: z.literal('folder-workspace'),
folderWorkspaceId: z.string().min(1)
folderWorkspaceId: z.string().min(1),
ownerHostId: ProjectGroupOwnerHostId.optional()
}),
z.object({
scope: z.literal('project-group'),
@@ -1562,7 +1565,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
typeof args.updates.folderPath === 'string' &&
args.updates.folderPath.trim().length > 0
) {
const workspace = store.getFolderWorkspace(args.folderWorkspaceId)
const workspace = store.getFolderWorkspace(args.folderWorkspaceId, args.ownerHostId)
if (!workspace) {
return null
}
@@ -1587,7 +1590,11 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
)
assertFolderWorkspacePathUsable(status)
}
const updated = store.updateFolderWorkspace(args.folderWorkspaceId, args.updates)
const updated = store.updateFolderWorkspace(
args.folderWorkspaceId,
args.updates,
args.ownerHostId
)
if (updated) {
notifyReposChanged(mainWindow)
}
@@ -1601,7 +1608,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
rawArgs,
'invalid_folder_workspace_delete_args'
)
const deleted = store.removeFolderWorkspace(args.folderWorkspaceId)
const deleted = store.removeFolderWorkspace(args.folderWorkspaceId, args.ownerHostId)
if (deleted) {
notifyReposChanged(mainWindow)
}
+73
View File
@@ -3736,6 +3736,79 @@ describe('Store', () => {
)
})
it('updates and removes same-id folder workspaces only for the selected owner', async () => {
const folderWorkspace = {
id: 'same-folder',
projectGroupId: 'same-id',
name: 'Folder',
folderPath: '/folder',
linkedTask: null,
comment: '',
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 1,
lastActivityAt: 1,
createdAt: 1,
updatedAt: 1
}
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
}
],
folderWorkspaces: [
{ ...folderWorkspace, name: 'Local', folderPath: '/local/folder', connectionId: null },
{
...folderWorkspace,
name: 'SSH',
folderPath: '/remote/folder',
connectionId: 'builder'
}
]
})
const store = await createStore()
expect(store.getFolderWorkspace('same-folder')).toBeUndefined()
expect(store.updateFolderWorkspace('same-folder', { name: 'Ambiguous' })).toBeNull()
expect(
store.updateFolderWorkspace('same-folder', { name: 'Remote' }, 'ssh:builder')?.name
).toBe('Remote')
expect(store.removeFolderWorkspace('same-folder')).toBe(false)
expect(store.removeFolderWorkspace('same-folder', 'local')).toBe(true)
expect(store.getFolderWorkspace('same-folder', 'ssh:builder')?.name).toBe('Remote')
})
it('adapts flat folder-scan groups into sparse nested folder scopes on load', async () => {
writeDataFile({
schemaVersion: 1,
+40 -23
View File
@@ -263,6 +263,7 @@ import { normalizeBrowserPageZoomLevel } from '../shared/browser-page-zoom'
import { persistedUIValuesEqual } from '../shared/persisted-ui-equality'
import { ActiveViewPreference } from './active-view-preference'
import {
resolveFolderWorkspaceCatalogOwnerHostId,
normalizeFolderWorkspaceName,
normalizeFolderWorkspaces
} from '../shared/folder-workspaces'
@@ -4488,10 +4489,20 @@ export class Store {
? { ...repo, projectGroupId: null }
: repo
})
const removedFolderWorkspaceKeys = new Set<string>()
for (const workspace of this.state.folderWorkspaces ?? []) {
const folderWorkspaces = this.state.folderWorkspaces ?? []
const removedFolderWorkspaces = folderWorkspaces.filter((workspace) => {
const group = resolveFolderWorkspaceProjectGroup(projectGroupIndex, workspace)
if (group && deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group))) {
return group && deletedGroupIdentities.has(getProjectGroupOwnerIdentity(group))
})
this.state.folderWorkspaces = folderWorkspaces.filter(
(workspace) => !removedFolderWorkspaces.includes(workspace)
)
const removedFolderWorkspaceKeys = new Set<string>()
for (const workspace of removedFolderWorkspaces) {
const hasSameIdSibling = this.state.folderWorkspaces.some(
(candidate) => candidate.id === workspace.id
)
if (!hasSameIdSibling) {
removedFolderWorkspaceKeys.add(folderWorkspaceKey(workspace.id))
this.state.workspaceSession = removeWorkspaceSessionOwner(
this.state.workspaceSession,
@@ -4500,10 +4511,6 @@ export class Store {
this.removeWorkspaceLineageForFolderParent(workspace.id)
}
}
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
@@ -4515,8 +4522,15 @@ export class Store {
)
}
getFolderWorkspace(id: string): FolderWorkspace | undefined {
return (this.state.folderWorkspaces ?? []).find((workspace) => workspace.id === id)
getFolderWorkspace(id: string, ownerHostId?: ExecutionHostId): FolderWorkspace | undefined {
const matches = (this.state.folderWorkspaces ?? []).filter(
(workspace) =>
workspace.id === id &&
(!ownerHostId ||
resolveFolderWorkspaceCatalogOwnerHostId(workspace, this.state.projectGroups ?? []) ===
ownerHostId)
)
return matches.length === 1 ? matches[0] : undefined
}
createFolderWorkspace(input: {
@@ -4598,9 +4612,10 @@ export class Store {
| 'firstAgentMessageRenameError'
| 'lastActivityAt'
>
>
>,
ownerHostId?: ExecutionHostId
): FolderWorkspace | null {
const workspace = this.getFolderWorkspace(id)
const workspace = this.getFolderWorkspace(id, ownerHostId)
if (!workspace) {
return null
}
@@ -4675,20 +4690,22 @@ export class Store {
return workspace
}
removeFolderWorkspace(id: string): boolean {
const before = this.state.folderWorkspaces?.length ?? 0
this.state.folderWorkspaces = (this.state.folderWorkspaces ?? []).filter(
(workspace) => workspace.id !== id
)
if ((this.state.folderWorkspaces?.length ?? 0) === before) {
removeFolderWorkspace(id: string, ownerHostId?: ExecutionHostId): boolean {
const workspace = this.getFolderWorkspace(id, ownerHostId)
if (!workspace) {
return false
}
this.state.workspaceSession = removeWorkspaceSessionOwner(
this.state.workspaceSession,
folderWorkspaceKey(id)
)!
this.removeWorkspaceLineageForFolderParent(id)
this.pruneMobileClientTabSelections((worktreeId) => worktreeId === folderWorkspaceKey(id))
this.state.folderWorkspaces = (this.state.folderWorkspaces ?? []).filter(
(candidate) => candidate !== workspace
)
if (!this.state.folderWorkspaces.some((candidate) => candidate.id === id)) {
this.state.workspaceSession = removeWorkspaceSessionOwner(
this.state.workspaceSession,
folderWorkspaceKey(id)
)!
this.removeWorkspaceLineageForFolderParent(id)
this.pruneMobileClientTabSelections((worktreeId) => worktreeId === folderWorkspaceKey(id))
}
this.scheduleSave()
return true
}
@@ -67,6 +67,71 @@ it('resolves same-id project-group paths by exact owner', () => {
).toThrow('folder_workspace_path_scope_not_found')
})
it('resolves same-id folder-workspace paths by exact owner and rejects a bare selector', () => {
const local = makeGroup({ id: 'same-id', parentPath: '/local' })
const ssh = makeGroup({ id: 'same-id', parentPath: '/remote', connectionId: 'builder' })
const store = {
getRepos: () => [],
getProjectGroups: () => [local, ssh],
getFolderWorkspaces: () => [
{
id: 'same-folder',
projectGroupId: 'same-id',
name: 'Local',
folderPath: '/local/folder',
connectionId: null,
linkedTask: null,
comment: '',
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 1,
lastActivityAt: 1,
createdAt: 1,
updatedAt: 1
},
{
id: 'same-folder',
projectGroupId: 'same-id',
name: 'SSH',
folderPath: '/remote/folder',
connectionId: 'builder',
linkedTask: null,
comment: '',
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 2,
lastActivityAt: 1,
createdAt: 1,
updatedAt: 1
}
]
}
expect(
resolveFolderWorkspaceStatusPath({
store,
request: {
scope: 'folder-workspace',
folderWorkspaceId: 'same-folder',
ownerHostId: 'ssh:builder'
}
})
).toEqual({
folderPath: '/remote/folder',
projectGroupId: 'same-id',
connectionId: 'builder',
ownerHostId: 'ssh:builder'
})
expect(() =>
resolveFolderWorkspaceStatusPath({
store,
request: { scope: 'folder-workspace', folderWorkspaceId: 'same-folder' }
})
).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-'))
@@ -14,6 +14,7 @@ import {
} from '../../shared/project-groups'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import type { FolderWorkspace, ProjectGroup, Repo } from '../../shared/types'
import { resolveFolderWorkspaceCatalogOwnerHostId } from '../../shared/folder-workspaces'
import type { IFilesystemProvider } from '../providers/types'
type FolderWorkspacePathStatusStore = {
@@ -194,14 +195,20 @@ export function resolveFolderWorkspaceStatusPath(args: {
}
}
const workspace = args.store
.getFolderWorkspaces?.()
.find((entry) => entry.id === request.folderWorkspaceId)
const matches = (args.store.getFolderWorkspaces?.() ?? []).filter(
(entry) =>
entry.id === request.folderWorkspaceId &&
(!request.ownerHostId ||
resolveFolderWorkspaceCatalogOwnerHostId(entry, projectGroups) === request.ownerHostId)
)
const workspace = matches.length === 1 ? matches[0] : null
if (!workspace) {
throw new Error('folder_workspace_path_scope_not_found')
}
const group = resolveFolderWorkspaceProjectGroup(projectGroupIndex, workspace)
// Why: fail closed only when the workspace row is missing; group may lag hydrate on Store doubles.
if (!group && projectGroupIndex.byId.has(workspace.projectGroupId)) {
throw new Error('folder_workspace_path_scope_not_found')
}
const ownerHostId = getFolderWorkspaceProjectGroupOwnerHostId(workspace, projectGroupIndex)
return {
folderPath: workspace.folderPath,
+2 -1
View File
@@ -1388,6 +1388,7 @@ export type PreloadApi = {
}) => Promise<FolderWorkspace>
update: (args: {
folderWorkspaceId: string
ownerHostId?: ExecutionHostId
updates: Partial<
Pick<
FolderWorkspace,
@@ -1408,7 +1409,7 @@ export type PreloadApi = {
>
>
}) => Promise<FolderWorkspace | null>
delete: (args: { folderWorkspaceId: string }) => Promise<boolean>
delete: (args: { folderWorkspaceId: string; ownerHostId?: ExecutionHostId }) => Promise<boolean>
}
sparsePresets: {
list: (args: { repoId: string }) => Promise<SparsePreset[]>
@@ -66,6 +66,7 @@ export function parseProjectGroupSidebarHeaderKey(
export type ProjectGroupSidebarIndex = {
byIdentity: ReadonlyMap<ProjectGroupSidebarIdentity, ProjectGroup>
byUnambiguousId: ReadonlyMap<string, ProjectGroup>
ambiguousIds: ReadonlySet<string>
}
export function buildProjectGroupSidebarIndex(
@@ -90,7 +91,7 @@ export function buildProjectGroupSidebarIndex(
byUnambiguousId.set(group.id, group)
}
}
return { byIdentity, byUnambiguousId }
return { byIdentity, byUnambiguousId, ambiguousIds }
}
export function findProjectGroupForSidebarOwner(
@@ -1555,10 +1555,7 @@ export function buildRows(
const repoEntries = sortRepoEntriesWithinGroup(groupByProjectGroupIdentity.get(identity) ?? [])
const childGroups = childGroupsByParentIdentity.get(identity) ?? []
// Why: bare project-group:<id> 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 keyOwner = projectGroupIndex.ambiguousIds.has(projectGroup.id) ? 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)
@@ -1685,10 +1682,9 @@ export function getGroupKeysForWorktree(
break
}
visited.add(identity)
const revealOwner =
projectGroups.filter((entry) => entry.id === group!.id).length > 1
? getProjectGroupOwnerHostId(group)
: undefined
const revealOwner = projectGroupIndex.ambiguousIds.has(group.id)
? getProjectGroupOwnerHostId(group)
: undefined
groupKeys.unshift(getProjectGroupHeaderKey(group.id, revealOwner))
group = findProjectGroupParentForSidebar(projectGroupIndex, group)
}
@@ -1,16 +1,12 @@
import type { FolderWorkspace, ProjectGroup, Repo } from '../../../../shared/types'
import {
getRepoExecutionHostId,
normalizeExecutionHostId,
toSshExecutionHostId,
type ExecutionHostId
} from '../../../../shared/execution-host'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host'
import {
buildProjectGroupOwnerIndex,
getProjectGroupIdentity,
getProjectGroupOwnerHostId,
getProjectGroupOwnerIdentity,
getProjectGroupOwnerSubtreeIdentities,
resolveFolderWorkspaceProjectGroup,
resolveProjectGroupOwner,
type ProjectGroupOwnerIndex
} from '../../../../shared/project-groups'
@@ -32,26 +28,6 @@ export type ProjectGroupRemovalTargets = {
folderWorkspaceIdentities: Set<string>
}
function getFolderWorkspacePreferredOwnerHostId(
workspace: Pick<FolderWorkspace, 'connectionId' | 'executionHostId'>
): ExecutionHostId | undefined {
return (
normalizeExecutionHostId(workspace.executionHostId) ??
(workspace.connectionId ? toSshExecutionHostId(workspace.connectionId) : undefined)
)
}
function resolveFolderWorkspaceProjectGroup(
index: ProjectGroupOwnerIndex,
workspace: Pick<FolderWorkspace, 'connectionId' | 'executionHostId' | 'projectGroupId'>
): ProjectGroup | null {
return resolveProjectGroupOwner(
index,
workspace.projectGroupId,
getFolderWorkspacePreferredOwnerHostId(workspace)
)
}
export function getProjectGroupRemovalFolderWorkspaceIdentity(
index: ProjectGroupOwnerIndex,
workspace: Pick<FolderWorkspace, 'id' | 'projectGroupId' | 'connectionId' | 'executionHostId'>
@@ -67,12 +67,12 @@ describe('same-id cross-host folder workspace identity', () => {
],
folderWorkspaces: [local, remote],
folderWorkspacePathStatuses: {
'local:folder-workspace:local:same-id': {
'local:folder-workspace:["local","same-id"]': {
status: { path: '/local/folder', exists: true },
checkedAt: 1,
requestSnapshot: 'local'
},
'local:folder-workspace:runtime:env-1:same-id': {
'local:folder-workspace:["runtime:env-1","same-id"]': {
status: { path: '/remote/folder', exists: true },
checkedAt: 1,
requestSnapshot: 'remote'
@@ -81,10 +81,11 @@ describe('same-id cross-host folder workspace identity', () => {
purgeWorktreeTerminalState
})
const deleteFolderWorkspace = vi.fn().mockResolvedValue(true)
vi.stubGlobal('window', {
api: {
folderWorkspaces: {
delete: vi.fn().mockResolvedValue(true)
delete: deleteFolderWorkspace
}
}
})
@@ -96,11 +97,13 @@ describe('same-id cross-host folder workspace identity', () => {
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(deleteFolderWorkspace).toHaveBeenCalledWith({
folderWorkspaceId: 'same-id',
ownerHostId: 'local'
})
expect(purgeWorktreeTerminalState).not.toHaveBeenCalled()
expect(Object.keys(store.getState().folderWorkspacePathStatuses)).toEqual([
'local:folder-workspace:runtime:env-1:same-id'
'local:folder-workspace:["runtime:env-1","same-id"]'
])
})
@@ -119,16 +122,13 @@ describe('same-id cross-host folder workspace identity', () => {
{ 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')
expect(localKey).toContain('["local","same-id"]')
expect(remoteKey).toContain('["runtime:env-1","same-id"]')
})
it('migrates legacy bare folder session tabs for the default owner on activation', () => {
it('keeps the legacy bare folder session identity on unambiguous 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',
@@ -161,18 +161,16 @@ describe('same-id cross-host folder workspace identity', () => {
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.activeWorkspaceKey).toBe(bareKey)
expect(state.activeWorktreeId).toBe(bareKey)
expect(state.tabsByWorktree[bareKey]?.[0]?.id).toBe('legacy-tab')
expect(state.activeTabIdByWorktree[bareKey]).toBe('legacy-tab')
expect(state.activeTabId).toBe('legacy-tab')
})
it('keeps owner-distinct same-id session tabs isolated across hosts', () => {
it('fails closed instead of owner-qualifying tabs for ambiguous folder ids', () => {
const store = createTestStore()
const localKey = folderWorkspaceKey('same-id', 'local')
const remoteKey = folderWorkspaceKey('same-id', 'runtime:env-1')
const bareKey = folderWorkspaceKey('same-id')
store.setState({
projectGroups: [
makeGroup({ id: 'same-group', executionHostId: 'local', parentPath: '/local' }),
@@ -193,24 +191,12 @@ describe('same-id cross-host folder workspace identity', () => {
})
],
tabsByWorktree: {
[localKey]: [
[bareKey]: [
{
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',
id: 'legacy-tab',
ptyId: 'pty-legacy',
worktreeId: bareKey,
title: 'Legacy',
customTitle: null,
color: null,
sortOrder: 0,
@@ -218,20 +204,38 @@ describe('same-id cross-host folder workspace identity', () => {
}
]
},
ptyIdsByTabId: {
'local-tab': ['pty-local'],
'remote-tab': ['pty-remote']
}
ptyIdsByTabId: { 'legacy-tab': ['pty-legacy'] }
})
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')
expect(store.getState().activeWorkspaceKey).toBeNull()
expect(store.getState().activeWorktreeId).toBeNull()
expect(store.getState().tabsByWorktree[bareKey]?.[0]?.id).toBe('legacy-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')
it('forwards the selected folder owner through local update IPC', async () => {
const local = makeFolder({ executionHostId: 'local' })
const remote = makeFolder({ executionHostId: 'runtime:env-1' })
const update = vi.fn().mockResolvedValue({ ...local, name: 'Updated' })
const store = createTestStore()
store.setState({
projectGroups: [
makeGroup({ executionHostId: 'local' }),
makeGroup({ executionHostId: 'runtime:env-1' })
],
folderWorkspaces: [local, remote]
})
vi.stubGlobal('window', { api: { folderWorkspaces: { update } } })
await expect(
store
.getState()
.updateFolderWorkspace('same-id', { name: 'Updated' }, { executionHostId: 'local' })
).resolves.toBe(true)
expect(update).toHaveBeenCalledWith({
folderWorkspaceId: 'same-id',
ownerHostId: 'local',
updates: { name: 'Updated' }
})
})
})
+68 -54
View File
@@ -46,6 +46,7 @@ import { applyManualRepoOrder, getManualRepoOrder } from '../../../../shared/man
import {
buildProjectGroupOwnerIndex,
getFolderWorkspaceProjectGroupOwnerHostId,
getProjectGroupIdentity,
getProjectGroupOwnerHostId,
getProjectGroupOwnerIdentity,
getProjectGroupSubtreeIds,
@@ -98,15 +99,11 @@ 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
getFolderWorkspaceOwnerIdentity,
resolveFolderWorkspaceCatalogOwnerHostId
} from '../../../../shared/folder-workspaces'
import { formatFolderWorkspaceCreateError } from '../../lib/folder-workspace-path-status'
import { getEnvironmentSshStateGeneration } from './runtime-environment-ssh'
@@ -1232,15 +1229,8 @@ function clearRestoredFolderWorkspaceSessionOwners(
next[key] = hostId
continue
}
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
})
const matches = state.folderWorkspaces.filter((entry) => entry.id === scope.folderWorkspaceId)
const workspace = matches.length === 1 ? matches[0] : undefined
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
@@ -1380,14 +1370,9 @@ async function reconcileFailedFolderWorkspaceUpdate(args: {
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
})
)
if (!remainingSameId) {
args.get().purgeWorktreeTerminalState([folderWorkspaceKey(args.folderWorkspaceId)])
}
}
} catch (err) {
console.warn('Failed to reconcile folder workspace after update failure:', err)
@@ -1433,14 +1418,14 @@ function settingsForRepoOwner(
function getFolderWorkspacePathStatusScopeKey(request: FolderWorkspacePathStatusRequest): string {
if (request.scope === 'project-group') {
return request.ownerHostId
? `project-group:${request.ownerHostId}:${request.projectGroupId}`
? `project-group:${getProjectGroupIdentity(request.projectGroupId, request.ownerHostId)}`
: `project-group:${request.projectGroupId}`
}
if (request.scope === 'path') {
return `path:${request.connectionId ?? ''}:${request.path}`
}
return request.ownerHostId
? `folder-workspace:${request.ownerHostId}:${request.folderWorkspaceId}`
? `folder-workspace:${getFolderWorkspaceIdentity(request.folderWorkspaceId, request.ownerHostId)}`
: `folder-workspace:${request.folderWorkspaceId}`
}
@@ -1576,21 +1561,22 @@ function getFolderWorkspaceStatusRequestSnapshot(
const catalog = getFolderPathStatusCatalog(state.projectGroups, state.repos)
const projectGroupIndex = catalog.projectGroupIndex
const folderMatches =
request.scope === 'folder-workspace'
? state.folderWorkspaces.filter(
(workspace) =>
workspace.id === request.folderWorkspaceId &&
(!request.ownerHostId ||
resolveFolderWorkspaceCatalogOwnerHostId(workspace, state.projectGroups) ===
request.ownerHostId)
)
: []
const scope =
request.scope === 'project-group'
? 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
)
})
: folderMatches.length === 1
? folderMatches[0]
: null
const projectGroup =
request.scope === 'project-group'
? scope && 'parentPath' in scope
@@ -2593,10 +2579,12 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
}
},
getFolderWorkspacePathStatusCacheKey: (request, options) =>
`${getRuntimeTargetCachePrefix(
getFolderWorkspacePathStatusCacheKey: (request, options) => {
const ownedRequest = getOwnerQualifiedFolderWorkspacePathStatusRequest(request, options)
return `${getRuntimeTargetCachePrefix(
getFolderWorkspacePathStatusRouteSettings(options, get().settings)
)}:${options?.ownerHostId ? `${options.ownerHostId}:` : ''}${getFolderWorkspacePathStatusScopeKey(request)}`,
)}:${getFolderWorkspacePathStatusScopeKey(ownedRequest)}`
},
getFreshFolderWorkspacePathStatus: (request, options) => {
const state = get()
@@ -2840,7 +2828,15 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
(state.activeWorktreeId === folderWorkspaceKey(folderWorkspaceId)
? (state.activeWorkspaceExecutionHostId ?? undefined)
: undefined)
if (!findFolderWorkspaceOwner(state, folderWorkspaceId, executionHostId)) {
const owner = findFolderWorkspaceOwner(state, folderWorkspaceId, executionHostId)
if (!owner) {
return false
}
const ownerHostId =
executionHostId ??
resolveFolderWorkspaceCatalogOwnerHostId(owner, state.projectGroups) ??
undefined
if (!ownerHostId) {
return false
}
const runtimeEnvironmentId = getRuntimeEnvironmentIdForFolderWorkspace(
@@ -2850,8 +2846,10 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
)
// Why: owner-scoped mutations must not follow whichever runtime happens to be focused.
const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId: runtimeEnvironmentId })
const ownerHostId = executionHostId ?? getRuntimeTargetHostId(target)
const updateIdentity = getFolderWorkspaceUpdateIdentity(ownerHostId, folderWorkspaceId)
const includeOwnerHostId =
executionHostId !== undefined ||
state.folderWorkspaces.filter((workspace) => workspace.id === folderWorkspaceId).length > 1
// Why: same gate as folderWorkspace.create — an older paired runtime would drop the Jira link silently.
if (
target.kind === 'environment' &&
@@ -2871,7 +2869,11 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
try {
const updated =
target.kind === 'local'
? await window.api.folderWorkspaces.update({ folderWorkspaceId, updates })
? await window.api.folderWorkspaces.update({
folderWorkspaceId,
...(includeOwnerHostId ? { ownerHostId } : {}),
updates
})
: (
await callRuntimeRpc<{ folderWorkspace: FolderWorkspace | null }>(
target,
@@ -2936,9 +2938,17 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
return false
}
const ownerHostId =
options?.ownerHostId ?? getFolderWorkspaceCatalogOwnerHostId(owner, state.projectGroups)
options?.ownerHostId ??
resolveFolderWorkspaceCatalogOwnerHostId(owner, state.projectGroups) ??
undefined
if (!ownerHostId) {
return false
}
// Why: catalog owner can come from the project group even when the folder row itself is still unstamped.
const ownerRuntime = parseExecutionHostId(ownerHostId)
const includeOwnerHostId =
options?.ownerHostId !== undefined ||
state.folderWorkspaces.filter((workspace) => workspace.id === folderWorkspaceId).length > 1
const runtimeEnvironmentId =
ownerRuntime?.kind === 'runtime'
? ownerRuntime.environmentId
@@ -2948,7 +2958,10 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
const target = getActiveRuntimeTarget({ activeRuntimeEnvironmentId: runtimeEnvironmentId })
const deleted =
target.kind === 'local'
? await window.api.folderWorkspaces.delete({ folderWorkspaceId })
? await window.api.folderWorkspaces.delete({
folderWorkspaceId,
...(includeOwnerHostId ? { ownerHostId } : {})
})
: (
await callRuntimeRpc<{ deleted: boolean }>(
target,
@@ -2968,21 +2981,22 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
),
folderWorkspacePathStatuses: Object.fromEntries(
Object.entries(s.folderWorkspacePathStatuses).filter(([cacheKey]) => {
const ownerScoped = `folder-workspace:${ownerHostId}:${folderWorkspaceId}`
const ownerScoped = getFolderWorkspacePathStatusScopeKey({
scope: 'folder-workspace',
folderWorkspaceId,
ownerHostId
})
const bareScoped = `folder-workspace:${folderWorkspaceId}`
return !cacheKey.includes(ownerScoped) && !cacheKey.endsWith(bareScoped)
return !cacheKey.endsWith(ownerScoped) && !cacheKey.endsWith(bareScoped)
})
)
}))
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 })
)
if (!remainingSameId) {
get().purgeWorktreeTerminalState([folderWorkspaceKey(folderWorkspaceId)])
}
return true
} catch (err) {
console.error('Failed to delete folder workspace:', err)
+23 -187
View File
@@ -97,16 +97,13 @@ 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,
@@ -878,110 +875,6 @@ function folderWorkspaceMatchesHost(
)
}
/** Promote legacy bare `folder:<id>` session maps to owner-qualified keys for one unambiguous owner. */
function migrateLegacyFolderWorkspaceSessionMaps(
s: AppState,
folderWorkspaceId: string,
ownerHostId: ExecutionHostId
): Partial<AppState> | 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<AppState, 'worktreesByRepo' | 'detectedWorktreesByRepo' | 'folderWorkspaces'>,
worktreeId: string,
@@ -989,18 +882,12 @@ function findKnownWorktreeById(
): Worktree | DetectedWorktreeListResult['worktrees'][number] | undefined {
const workspaceScope = parseWorkspaceKey(worktreeId)
if (workspaceScope?.type === 'folder') {
const preferredHostId = executionHostId ?? workspaceScope.ownerHostId
const matches = state.folderWorkspaces.filter(
(workspace) =>
workspace.id === workspaceScope.folderWorkspaceId &&
(!preferredHostId || folderWorkspaceMatchesHost(workspace, preferredHostId))
(!executionHostId || folderWorkspaceMatchesHost(workspace, executionHostId))
)
// 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
const folderWorkspace = matches.length === 1 ? matches[0] : null
if (!folderWorkspace) {
return undefined
}
@@ -3787,9 +3674,6 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
validIds.add(FLOATING_TERMINAL_WORKTREE_ID)
// Why: folder workspaces persist tabs under `folder:<id>` 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 ?? {})) {
@@ -4120,7 +4004,7 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
const activeScope = parseWorkspaceKey(get().activeWorkspaceKey ?? '')
const parentWorkspace =
activeScope?.type === 'folder'
? folderWorkspaceKey(activeScope.folderWorkspaceId, activeScope.ownerHostId)
? folderWorkspaceKey(activeScope.folderWorkspaceId)
: undefined
const createArgs = {
repoId,
@@ -6041,25 +5925,16 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
},
setActiveFolderWorkspace: (folderWorkspaceId, executionHostId) => {
const workspace = findKnownWorktreeById(
get(),
folderWorkspaceKey(folderWorkspaceId, executionHostId),
executionHostId
)
if (!workspace) {
// Why: tabs keep the legacy bare workspace id, so a same-id catalog cannot activate safely.
if (
get().folderWorkspaces.filter((workspace) => workspace.id === folderWorkspaceId).length !== 1
) {
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:<id>` session maps before activation so dual-read becomes a one-time migration.
const migratedSession = migrateLegacyFolderWorkspaceSessionMaps(
get(),
folderWorkspaceId,
ownerHostId
)
if (migratedSession) {
set(migratedSession)
const workspaceKey = folderWorkspaceKey(folderWorkspaceId)
const workspace = findKnownWorktreeById(get(), workspaceKey, executionHostId)
if (!workspace) {
return
}
if (shouldDeferActivationTerminalPrep()) {
markInputQuietSchedulerInput()
@@ -6070,71 +5945,32 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
const reconciledActiveTabId =
get().reconcileWorktreeTabModel(workspaceKey).activeRenderableTabId
set((s) => {
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 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 activeGroup = activeGroupId
? ((dualGroups?.value ?? []).find((group) => group.id === activeGroupId) ?? null)
? ((s.groupsByWorktree[workspaceKey] ?? []).find((group) => group.id === activeGroupId) ??
null)
: null
const activeUnifiedTabId = reconciledActiveTabId ?? activeGroup?.activeTabId ?? null
const activeUnifiedTab =
activeUnifiedTabId != null
? ((dualUnified?.value ?? []).find(
? ((s.unifiedTabsByWorktree[workspaceKey] ?? []).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 ||
file.worktreeId === folderWorkspaceKey(folderWorkspaceId))
)
? s.openFiles.some((file) => file.id === restoredFileId && file.worktreeId === workspaceKey)
: false
const browserTabs = dualBrowserTabs?.value ?? []
const browserTabs = s.browserTabsByWorktree[workspaceKey] ?? []
const browserTabStillOpen = restoredBrowserTabId
? browserTabs.some((tab) => tab.id === restoredBrowserTabId)
: false
const worktreeTabs = dualTabs?.value ?? []
const restoredTabId = dualActiveTab?.value ?? null
const worktreeTabs = s.tabsByWorktree[workspaceKey] ?? []
const restoredTabId = s.activeTabIdByWorktree[workspaceKey] ?? null
const tabStillExists = restoredTabId
? worktreeTabs.some((tab) => tab.id === restoredTabId)
: false
+18 -7
View File
@@ -20,23 +20,34 @@ export function getFolderWorkspaceCatalogOwnerHostId(
workspace: Pick<FolderWorkspace, 'connectionId' | 'executionHostId' | 'projectGroupId'>,
projectGroups: readonly ProjectGroup[] = []
): ExecutionHostId {
return (
resolveFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups) ?? LOCAL_EXECUTION_HOST_ID
)
}
export function resolveFolderWorkspaceCatalogOwnerHostId(
workspace: Pick<FolderWorkspace, 'connectionId' | 'executionHostId' | 'projectGroupId'>,
projectGroups: readonly ProjectGroup[] = []
): ExecutionHostId | null {
const executionHostId = normalizeExecutionHostId(workspace.executionHostId)
if (executionHostId) {
return executionHostId
}
if (projectGroups.length > 0) {
const index = buildProjectGroupOwnerIndex(projectGroups)
const group = resolveFolderWorkspaceProjectGroup(index, workspace)
if (group) {
return getFolderWorkspaceProjectGroupOwnerHostId(workspace, index)
}
return null
}
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)
)
return LOCAL_EXECUTION_HOST_ID
}
export function getFolderWorkspaceOwnerIdentity(
+1 -1
View File
@@ -326,7 +326,7 @@ export type ProjectGroup = {
export type WorkspaceScope =
| { type: 'worktree'; worktreeId: string }
| { type: 'folder'; folderWorkspaceId: string; ownerHostId?: ExecutionHostId }
| { type: 'folder'; folderWorkspaceId: string }
export type WorkspaceKey = `worktree:${string}` | `folder:${string}`
-87
View File
@@ -1,87 +0,0 @@
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<string, string> = { [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()
})
})
+4 -98
View File
@@ -1,18 +1,10 @@
import type { ExecutionHostId } from './execution-host'
import type { WorkspaceKey, WorkspaceScope } from './types'
export function worktreeWorkspaceKey(worktreeId: string): WorkspaceKey {
return `worktree:${worktreeId}`
}
/** 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)}`
}
export function folderWorkspaceKey(folderWorkspaceId: string): WorkspaceKey {
return `folder:${folderWorkspaceId}`
}
@@ -22,24 +14,8 @@ export function parseWorkspaceKey(value: string): WorkspaceScope | null {
return worktreeId.length > 0 ? { type: 'worktree', worktreeId } : null
}
if (value.startsWith('folder:')) {
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 }
const folderWorkspaceId = value.slice('folder:'.length)
return folderWorkspaceId.length > 0 ? { type: 'folder', folderWorkspaceId } : null
}
return null
}
@@ -56,80 +32,10 @@ export function getActiveSidebarWorkspaceId(
): string | null {
const scope = activeWorkspaceKey ? parseWorkspaceKey(activeWorkspaceKey) : null
if (scope?.type === 'folder') {
return folderWorkspaceKey(scope.folderWorkspaceId, scope.ownerHostId)
return folderWorkspaceKey(scope.folderWorkspaceId)
}
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:<id>` alias so pre-migration tabs/state keep resolving for that owner.
*/
export function dualReadFolderWorkspaceKeyedValue<T>(
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:<id>` 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<T>(
record: Record<string, T>,
resolveUnambiguousOwner: (folderWorkspaceId: string) => ExecutionHostId | null
): { record: Record<string, T>; migratedKeys: string[] } {
const migratedKeys: string[] = []
let next: Record<string, T> | 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 }
}