From 791577861b9e250010538fcddb77c12aaa6ea072 Mon Sep 17 00:00:00 2001 From: Yunqian Fan Date: Thu, 30 Jul 2026 09:57:50 +0800 Subject: [PATCH] fix(project-host-setup): carry identity across hosts (#9413) Allow setup when the selected project exists only on another host by carrying its validated provider identity with the request instead of reverse-parsing project IDs. Preserve host-qualified provider identity and reject mismatched payloads before linking. Make linking atomic for local and runtime imports, including clone setup: roll back only newly registered repos and invalidate the same caches as canonical removal. Cover local, runtime, host-qualified identity, mismatch, clone rollback, and renderer routing paths. Co-authored-by: fanyunqian.1 --- src/main/ipc/repos-remote.test.ts | 71 +++++++++ src/main/ipc/repos.ts | 54 ++++--- src/main/runtime/orca-runtime.test.ts | 139 ++++++++++++++++++ src/main/runtime/orca-runtime.ts | 80 +++++++--- .../methods/project-runtime-rpc-methods.ts | 9 ++ .../components/right-sidebar/FileExplorer.tsx | 5 +- ...plorer-inline-input-outside-click.test.tsx | 7 +- .../sidebar/worktree-keyboard-cycle.test.ts | 17 ++- src/renderer/src/store/slices/repos.test.ts | 5 +- src/renderer/src/store/slices/repos.ts | 9 +- .../project-host-setup-projection.test.ts | 17 +++ src/shared/project-host-setup-projection.ts | 6 +- src/shared/types.ts | 2 + 13 files changed, 359 insertions(+), 62 deletions(-) diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index 0022589443b..b6ec0118a61 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -1115,6 +1115,7 @@ describe('repos:addRemote', () => { }) mockStore.getRepos.mockReset().mockReturnValue([]) mockStore.addRepo.mockReset() + mockStore.removeProject.mockReset() mockStore.getSshTarget.mockReset() mockStore.updateRepo.mockReset() mockGitProvider.isGitRepoAsync.mockReset() @@ -2066,6 +2067,76 @@ describe('repos:add + repos:clone', () => { }) }) + it('sets up a folder when the selected project exists only on another host', async () => { + const added: Record[] = [] + mockStore.getRepos.mockImplementation(() => added) + mockStore.addRepo.mockImplementation((repo: Record) => added.push(repo)) + mockStore.updateRepo.mockImplementation((id, updates) => { + const repo = added.find((entry) => entry.id === id) + if (!repo) { + return null + } + Object.assign(repo, updates) + return { ...repo } + }) + mockStore.getProjects.mockImplementation(() => { + const repo = added.find((entry) => 'upstream' in entry) + return repo + ? [ + { + id: 'github:github.acme.test/acme/orca', + displayName: 'Orca', + providerIdentity: { + provider: 'github', + owner: 'acme', + repo: 'orca', + host: 'github.acme.test' + } + } + ] + : [] + }) + + const result = await handlers.get('projectHostSetups:setupExistingFolder')!(null, { + projectId: 'github:github.acme.test/acme/orca', + projectProviderIdentity: { + provider: 'github', + owner: 'acme', + repo: 'orca', + host: 'github.acme.test' + }, + hostId: 'local', + path: '/tmp/orca-local', + kind: 'git' + }) + + expect(added[0]?.upstream).toEqual({ + owner: 'acme', + repo: 'orca', + host: 'github.acme.test' + }) + expect(result).toHaveProperty('project.id', 'github:github.acme.test/acme/orca') + }) + + it('rolls back a new repo when the supplied identity does not match the project', async () => { + const added: Record[] = [] + mockStore.getRepos.mockImplementation(() => added) + mockStore.addRepo.mockImplementation((repo: Record) => added.push(repo)) + + await expect( + handlers.get('projectHostSetups:setupExistingFolder')!(null, { + projectId: 'github:acme/orca', + projectProviderIdentity: { provider: 'github', owner: 'other', repo: 'orca' }, + hostId: 'local', + path: '/tmp/mismatched-project', + kind: 'git' + }) + ).rejects.toThrow('Imported folder does not match the selected project identity.') + + expect(mockStore.removeProject).toHaveBeenCalledWith(added[0]?.id) + expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() + }) + it('prepares and invalidates roots when repos:update changes worktree base path', () => { const updated = { id: 'repo-update-root', diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 1a456e23410..01e11d0757e 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -89,7 +89,10 @@ import type { } from '../../shared/host-repo-catalog-contract' import { detectRepoIconAndUpstream } from '../repo-icon-autodetect' import { enrichMissingRepoGitRemoteIdentities } from '../repo-git-remote-identity-enrichment' -import { getProjectHostSetupForRepo } from '../../shared/project-host-setup-projection' +import { + getProjectHostSetupForRepo, + getProjectIdForProviderIdentity +} from '../../shared/project-host-setup-projection' import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID, @@ -235,20 +238,23 @@ function alignRepoWithRequestedProject( store: Store, repo: Repo, projectId: string, - setupMethod: ProjectHostSetupExistingFolderArgs['setupMethod'] = 'imported-existing-folder' + setupMethod: ProjectHostSetupExistingFolderArgs['setupMethod'] = 'imported-existing-folder', + requestedProviderIdentity?: ProjectHostSetupExistingFolderArgs['projectProviderIdentity'] ): ProjectHostSetupResult { let setup = getProjectHostSetupForRepo(store.getProjectHostSetups(), repo) if (setup.projectId !== projectId) { const project = store.getProjects().find((entry) => entry.id === projectId) - if (!project?.providerIdentity || project.providerIdentity.provider !== 'github') { + // Why: the selected project can exist only on the source host, so its structured identity travels with the request. + const identity = project?.providerIdentity ?? requestedProviderIdentity + if (!identity || getProjectIdForProviderIdentity(identity) !== projectId) { throw new Error('Imported folder does not match the selected project identity.') } // Why: stamp the selected project's provider identity when the folder lacks upstream, so projection can merge it. const updated = store.updateRepo(repo.id, { upstream: { - owner: project.providerIdentity.owner, - repo: project.providerIdentity.repo, - ...(project.providerIdentity.host ? { host: project.providerIdentity.host } : {}) + owner: identity.owner, + repo: identity.repo, + ...(identity.host ? { host: identity.host } : {}) } }) if (!updated) { @@ -801,6 +807,14 @@ const ProjectGroupMoveProjectArgs = z.object({ const ProjectHostSetupExistingFolderIpcArgs = z.object({ projectId: z.string().min(1), + projectProviderIdentity: z + .object({ + provider: z.literal('github'), + owner: z.string().min(1), + repo: z.string().min(1), + host: z.string().min(1).optional() + }) + .optional(), hostId: z.string().min(1), path: z.string().min(1), kind: z.enum(['git', 'folder']).optional(), @@ -1378,11 +1392,6 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v if (!parsedHost) { throw new Error(`Unsupported host: ${args.hostId}`) } - const existingProject = store.getProjects().find((project) => project.id === args.projectId) - if (!existingProject) { - throw new Error(`Project not found: ${args.projectId}`) - } - const result = parsedHost.kind === 'local' ? await addLocalRepoFromPath(store, args.path, args.kind) @@ -1400,15 +1409,26 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v if ('error' in result) { throw new Error(result.error) } + let aligned: ProjectHostSetupResult + try { + aligned = alignRepoWithRequestedProject( + store, + result.repo, + args.projectId, + args.setupMethod, + args.projectProviderIdentity + ) + } 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) + invalidateAuthorizedRootsCache() + } + throw err + } invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) emitRepoAdded('folder_picker', result.alreadyExisted) - const aligned = alignRepoWithRequestedProject( - store, - result.repo, - args.projectId, - args.setupMethod - ) if (result.alreadyExisted) { await prepareLocalWorktreeRootForRepo(store, aligned.repo) } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 209b76c1196..5daddfb70aa 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -6955,6 +6955,145 @@ describe('OrcaRuntimeService', () => { } }) + it('sets up a project whose identity exists only on the requesting host', async () => { + const tempRoot = await mkdtemp(join(tmpdir(), 'orca-runtime-cross-host-project-')) + const repos: Record[] = [] + getRepoUpstreamMock.mockResolvedValueOnce(null) + const runtimeStore = { + ...store, + getRepos: () => [...repos] as never, + addRepo: (repo: Record) => repos.push(repo), + getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, + updateRepo: (id: string, updates: Record) => { + const repo = repos.find((entry) => entry.id === id) + if (!repo) { + return null + } + Object.assign(repo, updates) + return { ...repo } as never + }, + getProjects: () => projectHostSetupProjectionFromRepos(repos as never).projects as never, + getProjectHostSetups: () => + projectHostSetupProjectionFromRepos(repos as never).setups as never + } + const runtime = new OrcaRuntimeService(runtimeStore as never) + + try { + execFileSync('git', ['init'], { cwd: tempRoot, stdio: 'ignore' }) + const result = await runtime.setupProjectExistingFolder({ + projectId: 'github:github.acme.test/acme/orca', + projectProviderIdentity: { + provider: 'github', + owner: 'acme', + repo: 'orca', + host: 'github.acme.test' + }, + hostId: 'runtime:env-1', + path: tempRoot, + kind: 'git' + }) + + expect(result.project).toMatchObject({ + id: 'github:github.acme.test/acme/orca', + providerIdentity: { + provider: 'github', + owner: 'acme', + repo: 'orca', + host: 'github.acme.test' + } + }) + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) + + it('rolls back a new runtime repo when project alignment fails', async () => { + const tempRoot = await mkdtemp(join(tmpdir(), 'orca-runtime-project-rollback-')) + const repos: Record[] = [] + getRepoUpstreamMock.mockResolvedValueOnce(null) + const runtimeStore = { + ...store, + getRepos: () => [...repos] as never, + addRepo: (repo: Record) => repos.push(repo), + getRepo: (id: string) => repos.find((repo) => repo.id === id) as never, + updateRepo: (id: string, updates: Record) => { + const repo = repos.find((entry) => entry.id === id) + if (!repo) { + return null + } + Object.assign(repo, updates) + return { ...repo } as never + }, + removeProject: (id: string) => { + const index = repos.findIndex((repo) => repo.id === id) + if (index !== -1) { + repos.splice(index, 1) + } + }, + getProjects: () => projectHostSetupProjectionFromRepos(repos as never).projects as never, + getProjectHostSetups: () => + projectHostSetupProjectionFromRepos(repos as never).setups as never + } + const runtime = new OrcaRuntimeService(runtimeStore as never) + + try { + execFileSync('git', ['init'], { cwd: tempRoot, stdio: 'ignore' }) + await expect( + runtime.setupProjectExistingFolder({ + projectId: 'git:git.example.test/acme/orca', + hostId: 'runtime:env-1', + path: tempRoot, + kind: 'git' + }) + ).rejects.toThrow('Imported folder does not match the selected project identity.') + + expect(repos).toHaveLength(0) + } finally { + await rm(tempRoot, { recursive: true, force: true }) + } + }) + + it('rolls back a newly cloned repo when project alignment fails', async () => { + const repos: Record[] = [] + const clonedRepo = { + id: 'cloned-repo', + path: '/tmp/cloned-repo', + displayName: 'cloned-repo', + badgeColor: '#737373', + addedAt: 1, + kind: 'git' + } + const runtimeStore = { + ...store, + getRepos: () => [...repos] as never, + removeProject: (id: string) => { + const index = repos.findIndex((repo) => repo.id === id) + if (index !== -1) { + repos.splice(index, 1) + } + }, + getProjects: () => projectHostSetupProjectionFromRepos(repos as never).projects as never, + getProjectHostSetups: () => + projectHostSetupProjectionFromRepos(repos as never).setups as never + } + const runtime = new OrcaRuntimeService(runtimeStore as never) + vi.spyOn(runtime, 'cloneRepo').mockImplementation(async () => { + repos.push(clonedRepo) + return clonedRepo as never + }) + + await expect( + runtime.setupProjectClone({ + projectId: 'git:git.example.test/acme/orca', + hostId: 'runtime:env-1', + url: 'https://git.example.test/acme/orca.git', + destination: '/tmp' + }) + ).rejects.toThrow('Imported folder does not match the selected project identity.') + + expect(repos).toHaveLength(0) + }) + it('keeps existing-folder imports split by runtime host on the same normalized path', async () => { const tempRoot = await mkdtemp(join(tmpdir(), 'orca-runtime-project-host-')) const repos: Record[] = [] diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 71b7407b2bb..4ad2536524c 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -346,6 +346,7 @@ import { splitWorktreeIdForFilesystem } from '../../shared/worktree-id' import { + getProjectIdForProviderIdentity, getProjectHostSetupForRepo, getProjectHostSetupWorktreeMeta } from '../../shared/project-host-setup-projection' @@ -16702,23 +16703,68 @@ export class OrcaRuntimeService { throw new Error('runtime_unavailable') } assertProjectHostSetupHostIsSupported(args.hostId) - let repo = await this.addRepo(args.path, args.kind === 'folder' ? 'folder' : 'git', args.hostId) + const knownRepoIds = new Set(this.listRepos().map((repo) => repo.id)) + const repo = await this.addRepo( + args.path, + args.kind === 'folder' ? 'folder' : 'git', + args.hostId + ) + return this.completeProjectHostSetup(args, repo, !knownRepoIds.has(repo.id)) + } + + async setupProjectClone(args: ProjectHostSetupCloneArgs): Promise { + // Why: guard before cloneRepo, which would otherwise clone to the local disk. + assertProjectHostSetupHostIsSupported(args.hostId) + const knownRepoIds = new Set(this.listRepos().map((repo) => repo.id)) + const repo = await this.cloneRepo(args.url, args.destination, args.hostId) + return this.completeProjectHostSetup( + { ...args, path: repo.path, kind: 'git', setupMethod: 'cloned' }, + repo, + !knownRepoIds.has(repo.id) + ) + } + + private completeProjectHostSetup( + args: ProjectHostSetupExistingFolderArgs, + initialRepo: Repo, + repoWasCreated: boolean + ): ProjectHostSetupResult { + try { + return this.linkRepoToProjectHostSetup(args, initialRepo) + } catch (err) { + if (repoWasCreated) { + // Why: a failed link must not leave a new repo registration or stale host caches behind. + this.store?.removeProject?.(initialRepo.id) + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(initialRepo.id) + invalidateAuthorizedRootsCache() + this.notifyReposChanged() + } + throw err + } + } + + private linkRepoToProjectHostSetup( + args: ProjectHostSetupExistingFolderArgs, + initialRepo: Repo + ): ProjectHostSetupResult { + if (!this.store) { + throw new Error('runtime_unavailable') + } + let repo = initialRepo let setup = getProjectHostSetupForRepo(this.listProjectHostSetups(), repo) if (setup.projectId !== args.projectId) { const existingProject = this.listProjects().find((project) => project.id === args.projectId) - if ( - !existingProject?.providerIdentity || - existingProject.providerIdentity.provider !== 'github' - ) { + // Why: the selected project can exist only on the source host, so its structured identity travels with the request. + const identity = existingProject?.providerIdentity ?? args.projectProviderIdentity + if (!identity || getProjectIdForProviderIdentity(identity) !== args.projectId) { throw new Error('Imported folder does not match the selected project identity.') } const updated = this.store.updateRepo(repo.id, { upstream: { - owner: existingProject.providerIdentity.owner, - repo: existingProject.providerIdentity.repo, - ...(existingProject.providerIdentity.host - ? { host: existingProject.providerIdentity.host } - : {}) + owner: identity.owner, + repo: identity.repo, + ...(identity.host ? { host: identity.host } : {}) } }) if (!updated) { @@ -16743,20 +16789,6 @@ export class OrcaRuntimeService { return { project, setup, repo } } - async setupProjectClone(args: ProjectHostSetupCloneArgs): Promise { - // Why: guard before cloneRepo, which would otherwise clone to the local disk. - assertProjectHostSetupHostIsSupported(args.hostId) - const repo = await this.cloneRepo(args.url, args.destination, args.hostId) - return await this.setupProjectExistingFolder({ - projectId: args.projectId, - hostId: args.hostId, - path: repo.path, - kind: 'git', - displayName: args.displayName, - setupMethod: 'cloned' - }) - } - updateProjectHostSetup(args: ProjectHostSetupUpdateArgs): ProjectHostSetupUpdateResult { if (!this.store?.updateProjectHostSetup) { throw new Error('runtime_unavailable') diff --git a/src/main/runtime/rpc/methods/project-runtime-rpc-methods.ts b/src/main/runtime/rpc/methods/project-runtime-rpc-methods.ts index 53eb1644819..60a821c3d31 100644 --- a/src/main/runtime/rpc/methods/project-runtime-rpc-methods.ts +++ b/src/main/runtime/rpc/methods/project-runtime-rpc-methods.ts @@ -3,8 +3,16 @@ import { normalizeExecutionHostId } from '../../../../shared/execution-host' import { defineMethod, type RpcMethod } from '../core' import { OptionalString, requiredString } from '../schemas' +const ProjectProviderIdentity = z.object({ + provider: z.literal('github'), + owner: requiredString('Missing project owner'), + repo: requiredString('Missing project repository'), + host: OptionalString +}) + const ProjectHostSetupExistingFolder = z.object({ projectId: requiredString('Missing project ID'), + projectProviderIdentity: ProjectProviderIdentity.optional(), hostId: requiredString('Missing host ID').transform((value, ctx) => { const hostId = normalizeExecutionHostId(value) if (!hostId) { @@ -21,6 +29,7 @@ const ProjectHostSetupExistingFolder = z.object({ const ProjectHostSetupClone = z.object({ projectId: requiredString('Missing project ID'), + projectProviderIdentity: ProjectProviderIdentity.optional(), hostId: requiredString('Missing host ID').transform((value, ctx) => { const hostId = normalizeExecutionHostId(value) if (!hostId) { diff --git a/src/renderer/src/components/right-sidebar/FileExplorer.tsx b/src/renderer/src/components/right-sidebar/FileExplorer.tsx index 045073f8186..81a41e20aea 100644 --- a/src/renderer/src/components/right-sidebar/FileExplorer.tsx +++ b/src/renderer/src/components/right-sidebar/FileExplorer.tsx @@ -47,10 +47,7 @@ import { buildAddProjectFromFolderModalData, canShowAddAsProjectAction } from './file-explorer-add-project-action' -import { - isRenameHotspotTarget, - resolveDirToggleTiming -} from './file-explorer-dir-toggle-timing' +import { isRenameHotspotTarget, resolveDirToggleTiming } from './file-explorer-dir-toggle-timing' import type { TreeNode } from './file-explorer-types' import { useFileExplorerSelection } from './useFileExplorerSelection' import { useFileExplorerVisibleRowProjection } from './useFileExplorerVisibleRowProjection' diff --git a/src/renderer/src/components/right-sidebar/file-explorer-inline-input-outside-click.test.tsx b/src/renderer/src/components/right-sidebar/file-explorer-inline-input-outside-click.test.tsx index c5543965db4..af7aa4b0726 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-inline-input-outside-click.test.tsx +++ b/src/renderer/src/components/right-sidebar/file-explorer-inline-input-outside-click.test.tsx @@ -37,12 +37,7 @@ function renderRenameRow(): { const onSubmit = vi.fn() const view = render(
- + {/* Rows are Radix context-menu triggers, so a genuine click on a neighbour used to be indistinguishable from Radix restoring focus after a close. */}