Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/ipc/repos-local-add-and-project-setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,8 @@ describe('repos:add + repos:clone', () => {
})
).rejects.toThrow('Imported folder does not match the selected project identity.')

expect(mockStore.removeProject).toHaveBeenCalledWith(added[0]?.id)
expect(mockStore.removeProjectForHost).toHaveBeenCalledWith(added[0]?.id, 'local')
expect(mockStore.removeProject).not.toHaveBeenCalled()
expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled()
})

Expand Down
2 changes: 2 additions & 0 deletions src/main/ipc/repos-remote-test-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type ReposIpcMocks = {
| 'getRepos'
| 'addRepo'
| 'removeProject'
| 'removeProjectForHost'
| 'getRepo'
| 'getProjects'
| 'getProjectHostSetups'
Expand Down Expand Up @@ -53,6 +54,7 @@ export function createReposIpcMocks(): ReposIpcMocks {
getRepos: vi.fn().mockReturnValue([]),
addRepo: vi.fn(),
removeProject: vi.fn(),
removeProjectForHost: vi.fn(),
getRepo: vi.fn(),
updateRepo: vi.fn(),
getProjects: vi.fn().mockReturnValue([]),
Expand Down
4 changes: 2 additions & 2 deletions src/main/ipc/repos/project-host-setup-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
} from '../../../shared/project-types'
import { getProjectIdForProviderIdentity } from '../../../shared/project-host-setup-projection'
import { getProjectHostSetupForRepo } from '../../../shared/project-host-setup-lookup'
import { parseExecutionHostId } from '../../../shared/execution-host'
import { getRepoExecutionHostId, parseExecutionHostId } from '../../../shared/execution-host'
import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation'
import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cache'
import { emitRepoAdded } from './repo-added-telemetry'
Expand Down Expand Up @@ -177,7 +177,7 @@ export function registerProjectHostSetupHandlers(mainWindow: BrowserWindow, stor
} catch (err) {
// Why: an import that cannot be linked must not leave a new repo registration or authorization root behind.
if (!result.alreadyExisted) {
store.removeProject(result.repo.id)
store.removeProjectForHost(result.repo.id, getRepoExecutionHostId(result.repo))
invalidateAuthorizedRootsCache()
}
throw err
Expand Down
24 changes: 24 additions & 0 deletions src/main/persistence-duplicate-repo-id-host-scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,30 @@ describe('deleting one host copy of a repo id shared by two hosts', () => {
expect(store.getRepos().map((repo) => repo.path)).toEqual(['/laptop/dup', '/remote/dup'])
})

it.each(['git', 'folder'] as const)(
'refuses an id-only removeProject that would take the other host %s row and its metadata',
async (kind) => {
// A stale renderer catalog once sent id-only removal when it could not see the SSH twin (#13071).
const store = await createStoreFromState({
repos: duplicateIdRepos().map((repo) => ({ ...repo, kind }))
})
store.setWorktreeMetaForHost('dup::/remote/dup', 'ssh:ssh-1', { displayName: 'SSH wt' })

expect(() => store.removeProject('dup')).toThrow('repo_id_ambiguous')

expect(store.getRepos().map((repo) => repo.path)).toEqual(['/laptop/dup', '/remote/dup'])
expect(Object.values(store.getAllWorktreeMeta()).map((meta) => meta.displayName)).toContain(
'SSH wt'
)

store.removeProjectForHost('dup', 'local')
expect(store.getRepos().map((repo) => repo.path)).toEqual(['/remote/dup'])
expect(Object.values(store.getAllWorktreeMeta()).map((meta) => meta.displayName)).toContain(
'SSH wt'
)
}
)

it('removeProjectForHost drops only the addressed host row', async () => {
const store = await createStoreWithDuplicateRepoId()

Expand Down
11 changes: 9 additions & 2 deletions src/main/persistence/loading-store/repo-lifecycle-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,16 @@ export class RepoLifecycleOperations {
}

removeProject(id: string): void {
const repoRemoved = this[repoLifecycleOperationsContext].runtime.state.repos.some(
(repo) => repo.id === id
const matchingHostIds = new Set(
this[repoLifecycleOperationsContext].runtime.state.repos
.filter((repo) => repo.id === id)
.map((repo) => getRepoExecutionHostId(repo))
)
// Why: an id-only delete would take every host's registration and metadata; callers must name the host (#13071).
if (matchingHostIds.size > 1) {
throw new Error(`repo_id_ambiguous: ${id} exists on more than one host; remove it by host.`)
}
const repoRemoved = matchingHostIds.size > 0
this[repoLifecycleOperationsContext].runtime.state.repos = this[
repoLifecycleOperationsContext
].runtime.state.repos.filter((r) => r.id !== id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ describe('OrcaRuntimeService', () => {
Object.assign(repo, updates)
return { ...repo } as never
},
removeProject: (id: string) => {
removeProjectForHost: (id: string) => {
const index = repos.findIndex((repo) => repo.id === id)
if (index !== -1) {
repos.splice(index, 1)
Expand Down Expand Up @@ -309,7 +309,7 @@ describe('OrcaRuntimeService', () => {
const runtimeStore = {
...store,
getRepos: () => [...repos] as never,
removeProject: (id: string) => {
removeProjectForHost: (id: string) => {
const index = repos.findIndex((repo) => repo.id === id)
if (index !== -1) {
repos.splice(index, 1)
Expand Down
6 changes: 5 additions & 1 deletion src/main/runtime/runtime-project-host-setup-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
} from '../../shared/project-types'
import type { Repo } from '../../shared/repo-types'
import {
getRepoExecutionHostId,
getSshTargetIdForExecutionHost,
parseExecutionHostId,
type ExecutionHostId
Expand Down Expand Up @@ -162,7 +163,10 @@ export class RuntimeProjectHostSetupController {
return this.linkRepo(args, initialRepo)
} catch (error) {
if (repoWasCreated) {
this.deps.getStore()?.removeProject?.(initialRepo.id)
// Why: host-scoped so a rollback never takes a same-id row on another host.
this.deps
.getStore()
?.removeProjectForHost?.(initialRepo.id, getRepoExecutionHostId(initialRepo))
this.deps.invalidateResolvedWorktrees()
this.deps.invalidateWorktreeScan(initialRepo.id)
invalidateAuthorizedRootsCache()
Expand Down
13 changes: 6 additions & 7 deletions src/renderer/src/components/sidebar/RemoveFolderDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { translate } from '@/i18n/i18n'
import {
getRepoExecutionHostId,
isRuntimeOwnedSshTargetId,
type ExecutionHostId
normalizeExecutionHostId
} from '../../../../shared/execution-host'

// Why: interpolated into the sentence so locales control where the name sits;
Expand All @@ -29,7 +29,8 @@ const RemoveFolderDialog = React.memo(function RemoveFolderDialog() {
const isOpen = activeModal === 'confirm-remove-folder'
const repoId = typeof modalData.repoId === 'string' ? modalData.repoId : ''
const displayName = typeof modalData.displayName === 'string' ? modalData.displayName : ''
const hostId = typeof modalData.hostId === 'string' ? (modalData.hostId as ExecutionHostId) : null
const hostId =
typeof modalData.hostId === 'string' ? normalizeExecutionHostId(modalData.hostId) : null

// Why: for an SSH project the files live on the remote host's disk, not the
// user's — "still on your disk" would be misleading. Name the host (using the
Expand Down Expand Up @@ -75,11 +76,9 @@ const RemoveFolderDialog = React.memo(function RemoveFolderDialog() {
const [descriptionBeforeName, descriptionAfterName] = description.split(NAME_TOKEN)

const handleConfirm = useCallback(() => {
if (repoId) {
void removeProject(repoId, {
...(hostId ? { hostId } : {}),
errorFeedback: 'toast'
})
// Why: without the opener's host a bare id could remove another host's row (#13071).
if (repoId && hostId) {
void removeProject(repoId, { hostId, errorFeedback: 'toast' })
}
closeModal()
}, [closeModal, hostId, removeProject, repoId])
Expand Down
5 changes: 2 additions & 3 deletions src/renderer/src/components/sidebar/delete-worktree-flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getWorkspaceDeleteLineage } from './workspace-delete-lineage'
import { resolveSshWorkspaceForget } from './ssh-workspace-forget-resolution'
import { isPairedWebClientWindow } from '@/lib/desktop-window-chrome'
import { parseWorkspaceKey } from '../../../../shared/workspace-scope'
import { getRepoExecutionHostId } from '../../../../shared/execution-host'
import { getWorktreeExecutionHostId } from '../../../../shared/execution-host'
import {
resolveWorktreeBatchDeleteTargets,
toWorktreeDeleteIdentities,
Expand Down Expand Up @@ -54,12 +54,11 @@ export function runWorktreeDelete(worktreeId: string, options: WorktreeDeleteOpt
hostId: target.hostId,
settings: state.settings
})
const hostId = repo ? getRepoExecutionHostId(repo) : target.hostId
// Why: git refuses to delete the primary checkout; users can still remove the owning project from Orca (disk contents kept).
state.openModal('confirm-remove-folder', {
repoId: target.repoId,
displayName: repo?.displayName ?? target.displayName,
...(hostId ? { hostId } : {})
hostId: getWorktreeExecutionHostId(target, repo ?? undefined)
})
return
}
Expand Down
47 changes: 28 additions & 19 deletions src/renderer/src/store/project-groups/project-group-mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { ProjectGroup } from '../../../../shared/project-group-types'
import type { Repo } from '../../../../shared/repo-types'
import { selectProjectGroupRemovalTargets } from '../slices/project-group-removal-targets'
import {
catalogOwnsHost,
getProjectGroupHostId,
projectGroupMatchesOwnerHost,
resolveProjectGroupOwnerHostId,
Expand Down Expand Up @@ -180,26 +179,36 @@ export function createProjectGroupMutationActions(

const removedProjectIds: string[] = []
const failedProjectRemovals: ProjectRemovalFailure[] = []
// Why: the group's catalog can hold rows from several hosts (a local catalog also owns SSH rows),
// so each project is removed on its own host rather than on the group's.
const findOwnedProjects = (projectId: string): Repo[] =>
get().repos.filter(
(repo) =>
repo.id === projectId &&
(!ownerHostId || catalogOwnsHost(ownerHostId, getRepoExecutionHostId(repo)))
)
for (const projectId of targets.projectIds) {
const ownedProjects = findOwnedProjects(projectId)
const projectHostId =
ownedProjects.length === 1 ? getRepoExecutionHostId(ownedProjects[0]) : undefined
try {
if (ownedProjects.length > 0) {
await get().removeProject(projectId, { hostId: projectHostId })
// Why: members were captured before the cascade cleared membership; a same-id row on another
// host outside the group must survive, so each member row is removed on its own host.
const membersById = new Map<string, Repo[]>()
for (const member of targets.projects) {
const row = findRepoForHost(get().repos, member.id, { hostId: member.hostId })
if (row) {
membersById.set(member.id, [...(membersById.get(member.id) ?? []), row])
}
}
for (const projectId of new Set(targets.projectIds)) {
const members = membersById.get(projectId) ?? []
// Why: an unstamped group already routes to the focused host, so only that host's member is removed.
const focusedMember = ownerHostId
? null
: findRepoForHost(members, projectId, { settings: get().settings })
const removable = ownerHostId ? members : focusedMember ? [focusedMember] : []
const removableHostIds = removable.map((row) => getRepoExecutionHostId(row))
for (const hostId of removableHostIds) {
try {
await get().removeProject(projectId, { hostId })
} catch (err) {
console.error('Failed to remove contained project:', err)
}
} catch (err) {
console.error('Failed to remove contained project:', err)
}
const stillExists = findOwnedProjects(projectId).length > 0
// Why: an ambiguous unstamped member with no focused-host row was left in place, so it counts as remaining.
const stillExists =
(members.length > 0 && removableHostIds.length === 0) ||
removableHostIds.some((hostId) =>
get().repos.some((repo) => repoMatchesHostIdentity(repo, projectId, hostId))
)
if (stillExists) {
failedProjectRemovals.push({
projectId,
Expand Down
28 changes: 10 additions & 18 deletions src/renderer/src/store/repos/repo-removal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { AppState } from '../types'
import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id'
import { getWorktreeIdFromVisitKey, getWorktreeVisitKey } from '@/lib/worktree-visit-recency'
import { omitSparsePresetsForRepos } from '../slices/sparse-presets'
import { findRepoForHost, repoMatchesHostIdentity } from '../slices/repo-host-identity'
import { repoMatchesHostIdentity } from '../slices/repo-host-identity'
import {
callRuntimeRpc,
getActiveRuntimeTarget,
Expand Down Expand Up @@ -57,11 +57,10 @@ export function createRepoRemovalActions(
return {
removeProject: async (projectId, options) => {
try {
// Why: pass an explicit hostId so a duplicate id across hosts resolves to the intended row, not the focused-host fallback.
const ownerRepo = findRepoForHost(get().repos, projectId, {
settings: get().settings,
hostId: options?.hostId
})
// Why: exact host match only; the unique-candidate/focused fallbacks could pick another host's row (#13071).
const ownerRepo = get().repos.find((repo) =>
repoMatchesHostIdentity(repo, projectId, options.hostId)
)
if (!ownerRepo) {
return
}
Expand All @@ -79,14 +78,8 @@ export function createRepoRemovalActions(
)
}
}
// Why: derive the target from the owner's settings (via options.hostId) so an SSH host removal never routes repo.rm to the focused runtime.
const target = getActiveRuntimeTarget(
settingsForRepoOwner(get(), projectId, options?.hostId)
)
// Why: repos:remove is id-only and would delete every host's row; scope local removal to the owning host so cross-host duplicates keep other rows.
const idExistsOnOtherHost = get().repos.some(
(repo) => repo.id === projectId && getRepoExecutionHostId(repo) !== ownerHostId
)
// Why: derive the target from the owner row's host so an SSH host removal never routes repo.rm to the focused runtime.
const target = getActiveRuntimeTarget(settingsForRepoOwner(get(), projectId, ownerHostId))
// Why before the host call: its announcement can start a listing refresh that drops these tabs.
const chatDraftKeys = captureWorkspaceChatDraftKeys(
get(),
Expand All @@ -96,10 +89,9 @@ export function createRepoRemovalActions(
}))
)
try {
// Why: always host-scoped; this catalog may be stale and miss a same-id row on another host (#13071).
await (target.kind === 'local'
? idExistsOnOtherHost
? window.api.repos.removeForHost({ repoId: projectId, hostId: ownerHostId })
: window.api.repos.remove({ repoId: projectId })
? window.api.repos.removeForHost({ repoId: projectId, hostId: ownerHostId })
: callRuntimeRpc(target, 'repo.rm', { repo: projectId }, { timeoutMs: 15_000 }))
} catch (err) {
// Why: the owner already dropped this project, so purge the local ghost row instead of aborting (#11994).
Expand Down Expand Up @@ -285,7 +277,7 @@ export function createRepoRemovalActions(
} catch (err) {
console.error('Failed to remove repo:', err)
// Why: bulk and background callers aggregate their own failures, so only opted-in single-project entry points toast (#11994).
if (options?.errorFeedback === 'toast') {
if (options.errorFeedback === 'toast') {
toast.error(
translate('auto.store.slices.repos.removeProjectFailed', 'Failed to remove project'),
{
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/src/store/repos/repo-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,11 @@ export type RepoSlice = {
groupId: string | null,
order?: number
) => Promise<boolean>
// options.hostId disambiguates which host's row to remove when the id exists on multiple hosts; else the focused host is assumed.
// options.hostId is required: a bare id can resolve to another host's row (#13071). Removes nothing when that host has no row.
// options.errorFeedback defaults to 'silent' so bulk/background callers keep their own aggregate reporting.
removeProject: (
projectId: string,
options?: { hostId?: ExecutionHostId; errorFeedback?: 'toast' | 'silent' }
options: { hostId: ExecutionHostId; errorFeedback?: 'toast' | 'silent' }
) => Promise<void>
updateProject: (projectId: string, updates: ProjectUpdate) => Promise<boolean>
// options.hostId targets a specific host's row + RPC target when the id exists on multiple hosts; else the focused host is assumed.
Expand Down
15 changes: 12 additions & 3 deletions src/renderer/src/store/slices/project-group-removal-targets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@ import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host'
import { catalogOwnsHost, getProjectGroupHostId } from './project-group-owner-routing'

export type ProjectGroupRemovalProject = { id: string; hostId: ExecutionHostId }

export type ProjectGroupRemovalTargets = {
groupExists: boolean
deletedGroupIds: Set<string>
projectIds: string[]
// Why: ids repeat across hosts and membership is per row, so removal must target the exact member rows.
projects: ProjectGroupRemovalProject[]
}

export function selectProjectGroupRemovalTargets(
Expand All @@ -27,25 +31,30 @@ export function selectProjectGroupRemovalTargets(
return {
groupExists: false,
deletedGroupIds: new Set(),
projectIds: []
projectIds: [],
projects: []
}
}

const deletedGroupIds = getProjectGroupSubtreeIds(ownerGroups, groupId)
const projectIds: string[] = []
const projects: ProjectGroupRemovalProject[] = []
for (const repo of repos) {
const repoHostId = getRepoExecutionHostId(repo)
if (
repo.projectGroupId &&
deletedGroupIds.has(repo.projectGroupId) &&
ownsRowHost(getRepoExecutionHostId(repo))
ownsRowHost(repoHostId)
) {
projectIds.push(repo.id)
projects.push({ id: repo.id, hostId: repoHostId })
}
}

return {
groupExists: true,
deletedGroupIds,
projectIds
projectIds,
projects
}
}
Loading
Loading