mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 16:03:53 +00:00
fix sidebar project group owner identity
This commit is contained in:
Brennan Benson
@@ -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:<id>`, 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.
|
||||
@@ -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:<id>` | 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:<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, 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.
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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
|
||||
})
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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) &&
|
||||
|
||||
@@ -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'
|
||||
)
|
||||
})
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<AppState, 'repos'>,
|
||||
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))
|
||||
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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
|
||||
})
|
||||
|
||||
@@ -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<AppState, [], [], UISlice> = (set, get)
|
||||
pendingRevealWorktree: {
|
||||
worktreeId,
|
||||
behavior: options?.behavior ?? 'smooth',
|
||||
...(options?.ownerHostId ? { ownerHostId: options.ownerHostId } : {}),
|
||||
...(options?.highlight ? { highlight: true } : {}),
|
||||
...(options?.beginRename ? { beginRename: true } : {})
|
||||
}
|
||||
|
||||
@@ -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<FolderWorkspace, 'connectionId' | 'executionHostId' | 'projectGroupId'>
|
||||
): 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
|
||||
}
|
||||
@@ -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<readonly ProjectGroup[], ProjectGroupOwnerIndex>()
|
||||
|
||||
export function resolveFolderWorkspaceProjectGroupWithLegacySsh(
|
||||
index: ProjectGroupOwnerIndex,
|
||||
workspace: Pick<FolderWorkspace, 'connectionId' | 'executionHostId' | 'projectGroupId'>
|
||||
): 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) {
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user