starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-23 00:03:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cleanup): disambiguate hosts and scope removal lookup

Browse Source
This commit is contained in:
Brennan Benson
2026-08-14 18:40:50 -07:00
parent 6112242698
commit c51a1617a7
9 changed files with 328 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/runtime/orca-runtime.test.ts
+1
View File
@@ -2417,6 +2417,7 @@ describe('OrcaRuntimeService', () => {
await expect(
internals.resolveWorktreeRemovalTarget(TEST_WORKTREE_ID, 'runtime:env-b')
).rejects.toThrow('selector_not_found')
expect(internals.listResolvedWorktrees).not.toHaveBeenCalled()
})
it('claims the first window as authoritative and ignores later windows', () => {
src/main/runtime/orca-runtime.ts
+5 -16
View File
@@ -24582,25 +24582,13 @@ export class OrcaRuntimeService {
private async resolveWorktreeRemovalTarget(
worktreeSelector: string,
requiredHostId?: string
requiredHostId?: ExecutionHostId
): Promise<RuntimeWorktreeRemovalTarget> {
try {
const exactTarget = parseExactWorktreeIdSelector(worktreeSelector)
const worktree =
exactTarget && requiredHostId
? ((await this.listResolvedWorktrees()).find((candidate) => {
if (candidate.id !== exactTarget.id) {
return false
}
const repo = this.store
?.getRepos()
.find(
(entry) =>
entry.id === candidate.repoId &&
getRepoExecutionHostId(entry) === requiredHostId
)
return getWorktreeExecutionHostId(candidate, repo) === requiredHostId
}) ??
? ((await this.resolveExplicitWorktreeIdScoped(exactTarget.id, requiredHostId)) ??
(() => {
throw new Error('selector_not_found')
})())
@@ -29827,12 +29815,13 @@ export class OrcaRuntimeService {
}
private async resolveExplicitWorktreeIdScoped(
worktreeId: string
worktreeId: string,
requiredHostId?: ExecutionHostId
): Promise<ResolvedWorktree | null> {
if (!this.store) {
return null
}
return await resolveScopedWorktreeIdRow(this.repoWorktreeRowDeps(), worktreeId)
return await resolveScopedWorktreeIdRow(this.repoWorktreeRowDeps(), worktreeId, requiredHostId)
}
private async listRepoWorktreesForResolution(
src/main/runtime/repo-worktree-row-resolution.test.ts
+161
View File
@@ -0,0 +1,161 @@
import { describe, expect, it, vi } from 'vitest'
import type { ExecutionHostId } from '../../shared/execution-host'
import type { Repo } from '../../shared/repo-types'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types'
import type { Store } from '../persistence'
import {
resolveScopedWorktreeIdRow,
type RepoWorktreeRowDeps
} from './repo-worktree-row-resolution'
function repo(
id: string,
path: string,
options: Pick<Repo, 'connectionId' | 'executionHostId' | 'kind'> = {}
): Repo {
return {
id,
path,
displayName: id,
badgeColor: 'blue',
addedAt: 1,
...options
}
}
function gitWorktree(path: string): GitWorktreeInfo {
return {
path,
head: 'abc123',
branch: 'refs/heads/feature',
isBare: false,
isMainWorktree: false
}
}
function createDeps(repos: Repo[]): RepoWorktreeRowDeps & {
scanRepo: ReturnType<typeof vi.fn<RepoWorktreeRowDeps['scanRepo']>>
listFolderWorkspaces: ReturnType<typeof vi.fn<RepoWorktreeRowDeps['listFolderWorkspaces']>>
} {
const metaById: Record<string, WorktreeMeta> = {}
const store = {
getRepos: () => repos,
getAllWorktreeMeta: () => metaById,
getAllWorktreeLineage: () => ({}),
getProjects: () => [],
getSettings: () => ({}),
setWorktreeMeta: (worktreeId: string, updates: Partial<WorktreeMeta>) => {
const next = { ...metaById[worktreeId], ...updates } as WorktreeMeta
metaById[worktreeId] = next
return next
}
} as unknown as Store
const scanRepo = vi.fn<RepoWorktreeRowDeps['scanRepo']>(async (owner) => ({
ok: true,
worktrees: [gitWorktree(owner.id === 'unrelated' ? '/unrelated/worktree' : '/same/worktree')]
}))
const listFolderWorkspaces = vi.fn<RepoWorktreeRowDeps['listFolderWorkspaces']>(() => [])
return { store, scanRepo, listFolderWorkspaces }
}
describe('host-qualified scoped worktree resolution', () => {
it('scans only the selected local or SSH owner when repo ids and paths collide', async () => {
const owners = [
repo('shared', '/local/repo', { executionHostId: 'local' }),
repo('shared', '/remote/repo', {
connectionId: 'builder',
executionHostId: 'ssh:builder'
}),
repo('unrelated', '/unrelated/repo', { connectionId: 'slow-box' })
]
const deps = createDeps(owners)
const worktreeId = 'shared::/same/worktree'
await expect(
resolveScopedWorktreeIdRow(deps, worktreeId, 'ssh:builder')
).resolves.toMatchObject({ id: worktreeId, hostId: 'ssh:builder' })
expect(deps.scanRepo.mock.calls.map(([owner]) => owner)).toEqual([owners[1]])
deps.scanRepo.mockClear()
await expect(resolveScopedWorktreeIdRow(deps, worktreeId, 'local')).resolves.toMatchObject({
id: worktreeId,
hostId: 'local'
})
expect(deps.scanRepo.mock.calls.map(([owner]) => owner)).toEqual([owners[0]])
})
it.each([
['runtime:windows', String.raw`C:\Users\dev\orca worktree`],
['local', '/mnt/c/Users/dev/orca worktree']
] satisfies [ExecutionHostId, string][])(
'keeps %s path resolution scoped',
async (hostId, path) => {
const target = repo('shared', path, { executionHostId: hostId })
const deps = createDeps([
target,
repo('shared', '/other-host/repo', { executionHostId: 'ssh:unrelated' }),
repo('unrelated', '/unrelated/repo')
])
deps.scanRepo.mockImplementation(async () => ({ ok: true, worktrees: [gitWorktree(path)] }))
const worktreeId = `shared::${path}`
await expect(resolveScopedWorktreeIdRow(deps, worktreeId, hostId)).resolves.toMatchObject({
id: worktreeId,
hostId,
path
})
expect(deps.scanRepo.mock.calls.map(([owner]) => owner)).toEqual([target])
}
)
it('resolves only the selected folder-workspace owner without scanning Git providers', async () => {
const local = repo('folders', '/local/folders', {
kind: 'folder',
executionHostId: 'local'
})
const remote = repo('folders', '/remote/folders', {
kind: 'folder',
connectionId: 'builder',
executionHostId: 'ssh:builder'
})
const deps = createDeps([local, remote, repo('unrelated', '/unrelated/repo')])
const worktreeId = 'folders::/shared/folder'
deps.listFolderWorkspaces.mockImplementation((owner) =>
owner === remote
? ([
{
id: worktreeId,
repoId: owner.id,
path: '/shared/folder',
head: '',
branch: '',
isBare: false,
isMainWorktree: false,
displayName: 'folder',
comment: '',
hostId: 'ssh:builder'
} as unknown as Worktree
] as Worktree[])
: []
)
await expect(
resolveScopedWorktreeIdRow(deps, worktreeId, 'ssh:builder')
).resolves.toMatchObject({ id: worktreeId, hostId: 'ssh:builder' })
expect(deps.listFolderWorkspaces).toHaveBeenCalledExactlyOnceWith(remote)
expect(deps.scanRepo).not.toHaveBeenCalled()
})
it('does not scan when the requested host does not own the repo id', async () => {
const deps = createDeps([
repo('shared', '/local/repo', { executionHostId: 'local' }),
repo('unrelated', '/unrelated/repo', { executionHostId: 'runtime:other' })
])
await expect(
resolveScopedWorktreeIdRow(deps, 'shared::/same/worktree', 'ssh:builder')
).resolves.toBeNull()
expect(deps.scanRepo).not.toHaveBeenCalled()
})
})
src/main/runtime/repo-worktree-row-resolution.ts
+11 -4
View File
@@ -1,5 +1,5 @@
import { splitWorktreeId, splitWorktreeIdForFilesystem } from '../../shared/worktree/id'
import { getRepoExecutionHostId } from '../../shared/execution-host'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import { isFolderRepo } from '../../shared/repo-kind'
import { projectResolvedWorktreeLineage } from '../../shared/resolved-worktree-lineage'
import { withTimeout } from '../../shared/promise-timeout-fallback'
@@ -157,16 +157,23 @@ export async function resolveRepoWorktreeRows(
*/
export async function resolveScopedWorktreeIdRow(
deps: RepoWorktreeRowDeps,
worktreeId: string
worktreeId: string,
requiredHostId?: ExecutionHostId
): Promise<RepoWorktreeRow | null> {
const { store } = deps
const parsed = splitWorktreeIdForFilesystem(worktreeId)
if (!parsed?.repoId || !parsed.worktreePath) {
return null
}
const owners = store.getRepos().filter((repo) => repo.id === parsed.repoId)
const owners = store
.getRepos()
.filter(
(repo) =>
repo.id === parsed.repoId &&
(requiredHostId === undefined || getRepoExecutionHostId(repo) === requiredHostId)
)
// Why: one repo id can be registered on several execution hosts, and only the fleet scan decides
// between their rows. Hand those back to the unscoped path rather than guessing.
// between unqualified rows. A host qualifier narrows the same-id set without scanning other owners.
if (owners.length !== 1) {
return null
}
src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx
+49 -3
View File
@@ -5,6 +5,7 @@ import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { getWorkspaceCleanupCandidateIdentity } from './workspace-cleanup-host-identity'
import { CandidateRow } from './workspace-cleanup-candidate-row'
import { getWorkspaceCleanupCandidateAccessibleName } from './workspace-cleanup-host-label'
import { makeCandidate } from './workspace-cleanup-presentation-fixtures'
vi.mock('@/components/ui/tooltip', () => ({
@@ -128,7 +129,9 @@ describe('CandidateRow', () => {
)
})
const openButton = container?.querySelector(`[aria-label="Open ${candidate.displayName}"]`)
const openButton = container?.querySelector(
`[aria-label="Open ${getWorkspaceCleanupCandidateAccessibleName(candidate)}"]`
)
expect(openButton?.querySelector('.lucide-external-link')).not.toBeNull()
})
@@ -161,8 +164,9 @@ describe('CandidateRow', () => {
)
})
expect(container?.querySelector(`[aria-label="Select ${candidate.displayName}"]`)).toBeNull()
expect(container?.querySelector(`[aria-label="Remove ${candidate.displayName}"]`)).toBeNull()
const accessibleName = getWorkspaceCleanupCandidateAccessibleName(candidate)
expect(container?.querySelector(`[aria-label="Select ${accessibleName}"]`)).toBeNull()
expect(container?.querySelector(`[aria-label="Remove ${accessibleName}"]`)).toBeNull()
})
it.each([
@@ -201,4 +205,46 @@ describe('CandidateRow', () => {
expect(container?.textContent).toContain(label)
expect(container?.textContent).not.toContain('Ready')
})
it('visibly and accessibly distinguishes colliding local and SSH rows', () => {
const local = makeCandidate({ executionHostId: 'local' })
const remote = makeCandidate({ connectionId: 'builder', executionHostId: 'ssh:builder' })
act(() => {
root?.render(
<>
{[local, remote].map((candidate) => (
<CandidateRow
key={candidate.executionHostId}
identity={getWorkspaceCleanupCandidateIdentity(candidate)}
candidate={candidate}
expanded={false}
last={false}
lastActivityLabel="1d ago"
reviewInfo={{
hasReview: false,
label: null,
provider: null,
state: null,
title: null
}}
selected={false}
onIgnore={vi.fn()}
onRemove={vi.fn()}
onToggleExpanded={vi.fn()}
onToggleSelected={vi.fn()}
onView={vi.fn()}
/>
))}
</>
)
})
const localName = getWorkspaceCleanupCandidateAccessibleName(local)
const remoteName = getWorkspaceCleanupCandidateAccessibleName(remote)
expect(localName).not.toBe(remoteName)
expect(container?.querySelector(`[aria-label="Select ${localName}"]`)).not.toBeNull()
expect(container?.querySelector(`[aria-label="Select ${remoteName}"]`)).not.toBeNull()
expect(container?.querySelector(`[aria-label="Host: builder"]`)).not.toBeNull()
})
})
src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx
+18 -4
View File
@@ -12,6 +12,7 @@ import {
GitPullRequest,
HardDrive,
Loader2,
Monitor,
SquareTerminal,
Trash2
} from 'lucide-react'
@@ -41,6 +42,10 @@ import {
} from './workspace-cleanup-candidate-row-data'
import { StatusPill } from './workspace-cleanup-status-pill'
import { WorkspaceCleanupMetadataChip } from './workspace-cleanup-metadata-chip'
import {
getWorkspaceCleanupCandidateAccessibleName,
getWorkspaceCleanupCandidateHostLabel
} from './workspace-cleanup-host-label'
export type WorkspaceCleanupDeletionPhase = 'deleting' | 'queued'
@@ -107,6 +112,8 @@ export const CandidateRow = React.memo(function CandidateRow({
const gitLabel = getWorkspaceCleanupGitLabel(candidate)
const showGitMetadataChip = shouldShowGitMetadataChip(candidate)
const contextCount = getContextCount(candidate)
const candidateAccessibleName = getWorkspaceCleanupCandidateAccessibleName(candidate)
const hostLabel = getWorkspaceCleanupCandidateHostLabel(candidate)
const sizeValue =
sizeLabel ?? translate('components.workspace.cleanup.browse.notMeasured', 'Not measured')
const hasExpandableDetails =
@@ -134,7 +141,7 @@ export const CandidateRow = React.memo(function CandidateRow({
aria-label={translate(
'auto.components.workspace.cleanup.WorkspaceCleanupDialog.bbb1ab6a6f',
'Select {{value0}}',
{ value0: candidate.displayName }
{ value0: candidateAccessibleName }
)}
onClick={() => onToggleSelected(identity)}
className="mt-0.5 flex size-4 shrink-0 items-center justify-center rounded border border-border bg-background text-primary hover:bg-accent focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring"
@@ -152,6 +159,13 @@ export const CandidateRow = React.memo(function CandidateRow({
<span data-workspace-cleanup-row-name className="min-w-0 truncate text-sm font-medium">
{candidate.displayName}
</span>
<WorkspaceCleanupMetadataChip
icon={Monitor}
label={translate('components.workspace.cleanup.host.label', 'Host: {{value0}}', {
value0: hostLabel
})}
value={hostLabel}
/>
{deletionPhase ? (
<StatusPill tone="destructive">
{deletionPhase === 'queued'
@@ -303,7 +317,7 @@ export const CandidateRow = React.memo(function CandidateRow({
aria-label={translate(
'components.workspace.cleanup.browse.openWorkspaceNamed',
'Open {{value0}}',
{ value0: candidate.displayName }
{ value0: candidateAccessibleName }
)}
onClick={() => onView(candidate)}
>
@@ -323,7 +337,7 @@ export const CandidateRow = React.memo(function CandidateRow({
aria-label={translate(
'auto.components.workspace.cleanup.WorkspaceCleanupDialog.a9957007eb',
'Ignore {{value0}}',
{ value0: candidate.displayName }
{ value0: candidateAccessibleName }
)}
onClick={() => onIgnore(candidate)}
>
@@ -347,7 +361,7 @@ export const CandidateRow = React.memo(function CandidateRow({
aria-label={translate(
'auto.components.workspace.cleanup.WorkspaceCleanupDialog.3828408538',
'Remove {{value0}}',
{ value0: candidate.displayName }
{ value0: candidateAccessibleName }
)}
className="text-destructive hover:text-destructive"
onClick={() => onRemove(candidate)}
src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.test.tsx
+42
View File
@@ -4,6 +4,10 @@ import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { makeFacetCandidate } from './workspace-cleanup-facet.test.fixture'
import { WorkspaceCleanupConfirmRemove } from './workspace-cleanup-confirm-remove'
import {
getWorkspaceCleanupCandidateAccessibleName,
getWorkspaceCleanupCandidateHostLabel
} from './workspace-cleanup-host-label'
const listProbe = vi.hoisted(() => vi.fn())
@@ -31,6 +35,12 @@ vi.mock('@/components/ui/scroll-area', () => ({
ScrollArea: ({ children }: { children: ReactNode }) => <div>{children}</div>
}))
vi.mock('@/components/ui/tooltip', () => ({
Tooltip: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
let container: HTMLDivElement
let root: Root
@@ -73,4 +83,36 @@ describe('WorkspaceCleanupConfirmRemove', () => {
expect(listProbe).toHaveBeenCalledWith(candidates.length)
expect(container.querySelectorAll('span.truncate.text-sm.font-medium')).toHaveLength(5)
})
it('visibly and accessibly identifies each host for colliding confirmations', () => {
const local = makeFacetCandidate({ executionHostId: 'local' })
const remote = makeFacetCandidate({
connectionId: 'builder',
executionHostId: 'ssh:builder'
})
act(() => {
root.render(
<WorkspaceCleanupConfirmRemove
candidates={[local, remote]}
now={Date.now()}
reviewInfoByWorktreeId={new Map()}
progress={null}
onBack={vi.fn()}
onCancel={vi.fn()}
onConfirm={vi.fn()}
/>
)
})
expect(getWorkspaceCleanupCandidateAccessibleName(local)).not.toBe(
getWorkspaceCleanupCandidateAccessibleName(remote)
)
for (const candidate of [local, remote]) {
const name = getWorkspaceCleanupCandidateAccessibleName(candidate)
const hostLabel = getWorkspaceCleanupCandidateHostLabel(candidate)
expect(container.querySelector(`[role="group"][aria-label="${name}"]`)).not.toBeNull()
expect(container.querySelector(`[aria-label="Host: ${hostLabel}"]`)).not.toBeNull()
}
})
})
src/renderer/src/components/workspace-cleanup/workspace-cleanup-confirm-remove.tsx
+19 -2
View File
@@ -1,5 +1,5 @@
import React from 'react'
import { AlertTriangle, Loader2, Trash2, X } from 'lucide-react'
import { AlertTriangle, Loader2, Monitor, Trash2, X } from 'lucide-react'
import { Button } from '@/components/ui/button'
import { DialogDescription, DialogFooter, DialogHeader, DialogTitle } from '@/components/ui/dialog'
import { Progress } from '@/components/ui/progress'
@@ -24,6 +24,11 @@ import {
getWorkspaceCleanupCandidateIdentity,
getWorkspaceCleanupHostIdentity
} from './workspace-cleanup-host-identity'
import {
getWorkspaceCleanupCandidateAccessibleName,
getWorkspaceCleanupCandidateHostLabel
} from './workspace-cleanup-host-label'
import { WorkspaceCleanupMetadataChip } from './workspace-cleanup-metadata-chip'
const CONFIRM_REMOVE_ROW_ESTIMATE_PX = 76
@@ -201,10 +206,22 @@ function ConfirmRemoveRow({
const contextPillLabel = getContextPillLabel(candidate)
const showGitMetadataChip = shouldShowGitMetadataChip(candidate)
const factStatus = getCandidateFactStatus(candidate)
const hostLabel = getWorkspaceCleanupCandidateHostLabel(candidate)
return (
<div className={cn('border-b border-border/60 px-5 py-2.5', last && 'border-b-0')}>
<div
role="group"
aria-label={getWorkspaceCleanupCandidateAccessibleName(candidate)}
className={cn('border-b border-border/60 px-5 py-2.5', last && 'border-b-0')}
>
<div className="flex min-w-0 flex-wrap items-baseline gap-x-2 gap-y-0.5">
<span className="min-w-0 truncate text-sm font-medium">{candidate.displayName}</span>
<WorkspaceCleanupMetadataChip
icon={Monitor}
label={translate('components.workspace.cleanup.host.label', 'Host: {{value0}}', {
value0: hostLabel
})}
value={hostLabel}
/>
<span className="text-xs text-muted-foreground">
{translate(
'auto.components.workspace.cleanup.WorkspaceCleanupDialog.352f15d6fc',
src/renderer/src/components/workspace-cleanup/workspace-cleanup-host-label.ts
+22
View File
@@ -0,0 +1,22 @@
import { translate } from '@/i18n/i18n'
import { getExecutionHostLabel } from '../../../../shared/execution-host'
import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup'
import { resolveWorkspaceCleanupRemovalHostId } from './workspace-cleanup-host-identity'
export function getWorkspaceCleanupCandidateHostLabel(
candidate: WorkspaceCleanupCandidate
): string {
const hostId = resolveWorkspaceCleanupRemovalHostId(candidate)
return hostId
? getExecutionHostLabel(hostId)
: translate('components.workspace.cleanup.host.unknown', 'Unknown host')
}
export function getWorkspaceCleanupCandidateAccessibleName(
candidate: WorkspaceCleanupCandidate
): string {
return translate('components.workspace.cleanup.host.candidateName', '{{value0}} on {{value1}}', {
value0: candidate.displayName,
value1: getWorkspaceCleanupCandidateHostLabel(candidate)
})
}