From fa923c67e377d1790c47a620a4e252ccc311ebbe Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 16 Oct 2026 15:53:12 -0700 Subject: [PATCH] fix(projects): a manual folder-to-Git change proves the folder is the Git root first (#29282) * fix(projects): refuse a folder-to-Git type change that would hide folder workspaces (STA-10638) A folder project's extra workspaces exist only as metadata rows that the folder listing shows. Changing the project to Git switches it to `git worktree list`, so those workspaces drop out of the next refresh and their saved tabs are purged as if they had been deleted. Only the automatic upgrade checked for this; `orca project setup-update --kind git`, repo updates and clone-into-folder did not. The check now lives in the one repo write every type change goes through, so any caller gets the same refusal and nothing is written. The automatic upgrade and both clone paths use the same check and keep the project as a folder project. * test(repos): give the repo handler store mock the metadata read the clone path now uses * fix(projects): a manual folder-to-Git change proves the folder is the Git root first (STA-10638) Only the automatic upgrade checked that a folder project's path is the top of a Git repository and recorded Git's spelling of that root. A manual change (orca project setup-update --kind git, repo updates, clone into a folder project) flipped the type without either, so a project inside another repo became a broken Git project, and a project whose path Git spells differently lost its main workspace's id and tabs on the next refresh. All of them now run the same proof and write the proven root with the type change. SSH projects keep the unverified change because their Git runs on a host this process can't probe. * Revalidate folder-to-Git conversion targets after Git proof * Explain when a project changes during its Git check * Fix clone conversion for replaced folder project * test(runtime): create real Git root for clone upgrade fixture * test: normalize runtime clone root stamp expectation * test: check clone registration identity without unknown access * chore: restore main's pnpm-lock.yaml that the main merge reverted --- src/main/folder-repo-git-conversion.ts | 167 ++++++++++++++++++ src/main/ipc/folder-repo-git-upgrade.ts | 41 ++--- .../repos-local-add-and-project-setup.test.ts | 19 +- .../ipc/repos-local-clone-lifecycle.test.ts | 70 +++++++- .../ipc/repos/project-host-setup-handlers.ts | 12 +- src/main/ipc/repos/repo-clone-lifecycle.ts | 27 ++- src/main/ipc/repos/repo-update-handler.ts | 34 +++- src/main/persistence-update-repo.test.ts | 57 +++++- ...ository-project-operations-part-02.spec.ts | 27 ++- .../methods/project-runtime-rpc-methods.ts | 4 +- .../runtime-project-host-setup-controller.ts | 25 ++- ...ime-project-host-setup-publication.test.ts | 8 +- .../runtime-repository-clone-controller.ts | 26 ++- .../runtime-repository-settings-controller.ts | 10 +- 14 files changed, 436 insertions(+), 91 deletions(-) create mode 100644 src/main/folder-repo-git-conversion.ts diff --git a/src/main/folder-repo-git-conversion.ts b/src/main/folder-repo-git-conversion.ts new file mode 100644 index 00000000000..b23a0ad4bd8 --- /dev/null +++ b/src/main/folder-repo-git-conversion.ts @@ -0,0 +1,167 @@ +import { realpathSync } from 'node:fs' +import type { Repo } from '../shared/repo-types' +import type { + ProjectHostSetupUpdateArgs, + ProjectHostSetupUpdateResult +} from '../shared/project-types' +import { + getRepoExecutionHostId, + LOCAL_EXECUTION_HOST_ID, + parseExecutionHostId +} from '../shared/execution-host' +import type { Store } from './persistence' +import { isFolderRepo } from '../shared/repo-kind' +import { normalizeRuntimePathForComparison } from '../shared/cross-platform-path' +import { getGitRepoRootAsync, isGitRepoAsync } from './git/repo' +import { findExtraFolderWorkspaceIds } from './folder-workspace-meta-presence' + +export type FolderToGitConversion = { + updates: Pick + /** Git spells the root exactly as the project does, so its external worktrees can be told apart. */ + rootSpellingMatches: boolean +} + +function resolveRealPath(pathValue: string): string { + try { + return realpathSync(pathValue) + } catch { + return pathValue + } +} + +/** + * Git's verdict on a folder project's path, or null when it must stay a folder project. + * + * Refuse unless the folder *is* the work-tree root: git accepts any path inside a work tree, so a + * project inside another repo would otherwise flip. The stamped root lets the listing map Git's + * spelling back to the project's path, so the main workspace keeps its id (and its tabs). + */ +export async function resolveFolderToGitConversion( + repoPath: string +): Promise { + if (!(await isGitRepoAsync(repoPath))) { + return null + } + const gitRoot = await getGitRepoRootAsync(repoPath) + if (resolveRealPath(gitRoot) !== resolveRealPath(repoPath)) { + return null + } + return { + updates: { folderUpgradeGitRootPath: gitRoot }, + rootSpellingMatches: + normalizeRuntimePathForComparison(gitRoot) === normalizeRuntimePathForComparison(repoPath) + } +} + +/** + * The extra fields a manual change of `repo` to `nextKind` must write with it. Throws when a + * folder project's path is not a Git root, so callers write nothing. An SSH project's Git runs on + * a host this process can't probe, so it keeps the unverified change it had before. + */ +export async function proveRepoKindChange( + repo: Repo | undefined, + nextKind: Repo['kind'] | undefined, + allowRuntimeHost = false +): Promise> { + if (!repo || nextKind !== 'git' || !isFolderRepo(repo) || repo.connectionId) { + return {} + } + const hostId = getRepoExecutionHostId(repo) + if ( + hostId !== LOCAL_EXECUTION_HOST_ID && + !(allowRuntimeHost && parseExecutionHostId(hostId)?.kind === 'runtime') + ) { + return {} + } + const conversion = await resolveFolderToGitConversion(repo.path) + if (!conversion) { + throw new Error( + `${repo.path} isn't the top folder of a Git repository, so this project's type was not changed.` + ) + } + return conversion.updates +} + +/** Recheck the selected row after Git yields, then apply the synchronous write to that row. */ +export async function commitProvenRepoKindChange( + store: Pick, + repo: Repo, + commit: (proven: Pick) => T, + allowRuntimeHost = false +): Promise { + const proven = await proveRepoKindChange(repo, 'git', allowRuntimeHost) + const current = store + .getRepos() + .find( + (entry) => + entry.id === repo.id && getRepoExecutionHostId(entry) === getRepoExecutionHostId(repo) + ) + if (!current || current.path !== repo.path || !isFolderRepo(current)) { + throw new Error( + 'The project changed while Orca was checking it, so its type was not changed. Try again.' + ) + } + return commit(proven) +} + +type ProjectHostSetupStore = Pick & { + getProjectHostSetups?: Store['getProjectHostSetups'] +} + +/** + * A project setup update whose type change is proven first. The proven root is stamped right after + * the setup write, with no await between, so nothing observes the project in between; a refused + * setup write (folder workspaces a Git project can't show) throws before the stamp. + */ +export async function updateProjectHostSetupWithProvenKind( + store: ProjectHostSetupStore, + updateSetup: Store['updateProjectHostSetup'], + args: ProjectHostSetupUpdateArgs, + allowRuntimeHost = false +): Promise { + if (args.updates.kind !== 'git') { + return updateSetup(args) + } + const setup = store.getProjectHostSetups?.().find((entry) => entry.id === args.setupId) + const repo = setup?.repoId + ? store + .getRepos() + .find( + (entry) => entry.id === setup.repoId && getRepoExecutionHostId(entry) === setup.hostId + ) + : undefined + if (!repo || !isFolderRepo(repo)) { + return updateSetup(args) + } + return commitProvenRepoKindChange( + store, + repo, + (proven) => { + const result = updateSetup(args) + if (!result?.repo || !proven.folderUpgradeGitRootPath) { + return result + } + const stamped = store.updateRepo(result.repo.id, proven, getRepoExecutionHostId(result.repo)) + return stamped ? { ...result, repo: stamped } : result + }, + allowRuntimeHost + ) +} + +/** + * The fields that turn a folder project a clone just landed in into a Git project, or null to keep + * it a folder project (other folder workspaces, or not a Git root). Re-checks after the Git probe: + * a workspace created meanwhile would otherwise make the write refuse a finished clone. + */ +export async function resolveClonedFolderConversion( + store: Pick, + repo: Repo +): Promise | null> { + const hasOtherWorkspaces = (): boolean => + findExtraFolderWorkspaceIds(repo, store.getAllWorktreeMeta()).length > 0 + if (!isFolderRepo(repo) || hasOtherWorkspaces()) { + return null + } + const conversion = await resolveFolderToGitConversion(repo.path) + return conversion && !hasOtherWorkspaces() ? conversion.updates : null +} diff --git a/src/main/ipc/folder-repo-git-upgrade.ts b/src/main/ipc/folder-repo-git-upgrade.ts index 9fdd02a136d..1b4a7284d30 100644 --- a/src/main/ipc/folder-repo-git-upgrade.ts +++ b/src/main/ipc/folder-repo-git-upgrade.ts @@ -1,5 +1,4 @@ import { stat } from 'node:fs/promises' -import { realpathSync } from 'node:fs' import { join } from 'node:path' import type { BrowserWindow } from 'electron' import type { Repo } from '../../shared/repo-types' @@ -9,7 +8,7 @@ import { isFolderRepo } from '../../shared/repo-kind' import { findExtraFolderWorkspaceIds } from '../folder-workspace-meta-presence' import { normalizeRuntimePathForComparison } from '../../shared/cross-platform-path' import { isWslUncPath } from '../../shared/wsl-paths' -import { getGitRepoRootAsync, isGitRepoAsync } from '../git/repo' +import { resolveFolderToGitConversion } from '../folder-repo-git-conversion' import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation' import { invalidateAuthorizedRootsCache } from './registered-worktree-roots-cache' import { notifyReposChanged } from './repos/repos-changed-notification' @@ -73,39 +72,23 @@ async function readGitMarkerSignature(repoPath: string): Promise } } -function resolveRealPath(pathValue: string): string { - try { - return realpathSync(pathValue) - } catch { - return pathValue - } -} - /** - * Git's verdict on the folder, or null when it must stay a folder project. - * - * Two comparisons, deliberately at different strictness: - * - Refuse unless the folder *is* the work-tree root. `.git` existing proves nothing — - * git accepts any path inside a work tree, so a stray marker in a subdirectory of - * another repo would otherwise flip a project whose path is not a repo root. - * - Only hide external worktrees when the stored spelling also matches. Add Project - * stores the root as `rev-parse --show-toplevel` spells it while a folder project keeps - * the path the user picked; when a symlinked parent makes those differ, the root reads - * as an *external* worktree, and hiding those would hide the project's only workspace. + * Only hide external worktrees when the stored spelling also matches. Add Project stores the root + * as `rev-parse --show-toplevel` spells it while a folder project keeps the path the user picked; + * when a symlinked parent makes those differ, the root reads as an *external* worktree, and hiding + * those would hide the project's only workspace. This is the watch's import policy, not part of + * proving the conversion, so a manual type change keeps the user's own visibility setting. */ async function resolveUpgrade( repoPath: string -): Promise<{ folderUpgradeGitRootPath: string; externalWorktreeVisibility?: 'hide' } | null> { - if (!(await isGitRepoAsync(repoPath))) { +): Promise<{ folderUpgradeGitRootPath?: string; externalWorktreeVisibility?: 'hide' } | null> { + const conversion = await resolveFolderToGitConversion(repoPath) + if (!conversion) { return null } - const gitRoot = await getGitRepoRootAsync(repoPath) - if (resolveRealPath(gitRoot) !== resolveRealPath(repoPath)) { - return null - } - return normalizeRuntimePathForComparison(gitRoot) === normalizeRuntimePathForComparison(repoPath) - ? { folderUpgradeGitRootPath: gitRoot, externalWorktreeVisibility: 'hide' } - : { folderUpgradeGitRootPath: gitRoot } + return conversion.rootSpellingMatches + ? { ...conversion.updates, externalWorktreeVisibility: 'hide' } + : conversion.updates } type UpgradeResult = 'upgraded' | 'blocked' | 'rejected' diff --git a/src/main/ipc/repos-local-add-and-project-setup.test.ts b/src/main/ipc/repos-local-add-and-project-setup.test.ts index eca40c3c161..88fe0d9fb18 100644 --- a/src/main/ipc/repos-local-add-and-project-setup.test.ts +++ b/src/main/ipc/repos-local-add-and-project-setup.test.ts @@ -354,7 +354,7 @@ describe('repos:add + repos:clone', () => { expect(mockStore.addRepo).not.toHaveBeenCalled() }) - it('prepares and invalidates roots when repos:update changes worktree base path', () => { + it('prepares and invalidates roots when repos:update changes worktree base path', async () => { const updated = { id: 'repo-update-root', path: '/tmp/repo-update-root', @@ -365,7 +365,7 @@ describe('repos:add + repos:clone', () => { } mockStore.updateRepo.mockReturnValue(updated) - const result = handlers.get('repos:update')!(null, { + const result = await handlers.get('repos:update')!(null, { repoId: updated.id, updates: { worktreeBasePath: ' ../worktrees ' } }) @@ -374,11 +374,12 @@ describe('repos:add + repos:clone', () => { expect(mockStore.updateRepo).toHaveBeenCalledWith(updated.id, { worktreeBasePath: '../worktrees' }) + expect(mockStore.getRepos).not.toHaveBeenCalled() expect(prepareLocalWorktreeRootForRepoMock).toHaveBeenCalledWith(mockStore, updated) expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() }) - it('persists agent worktree visibility through local repos:update', () => { + it('persists agent worktree visibility through local repos:update', async () => { const updated = { id: 'repo-agent-visibility', path: '/tmp/repo-agent-visibility', @@ -389,7 +390,7 @@ describe('repos:add + repos:clone', () => { } mockStore.updateRepo.mockReturnValue(updated) - const result = handlers.get('repos:update')!(null, { + const result = await handlers.get('repos:update')!(null, { repoId: updated.id, updates: { agentWorktreeVisibility: 'show' } }) @@ -400,7 +401,7 @@ describe('repos:add + repos:clone', () => { }) }) - it('validates source definitions and preferences through local repos:update', () => { + it('validates source definitions and preferences through local repos:update', async () => { const updated = { id: 'repo-source-visibility', path: '/tmp/repo-source-visibility', @@ -410,7 +411,7 @@ describe('repos:add + repos:clone', () => { } mockStore.updateRepo.mockReturnValue(updated) - handlers.get('repos:update')!(null, { + await handlers.get('repos:update')!(null, { repoId: updated.id, updates: { customWorktreeVisibilitySources: [ @@ -433,7 +434,7 @@ describe('repos:add + repos:clone', () => { }) }) - it('prepares and invalidates roots when project host setup update changes worktree base path', () => { + it('prepares and invalidates roots when project host setup update changes worktree base path', async () => { const repo = { id: 'repo-setup-update-root', path: '/tmp/repo-setup-update-root', @@ -449,12 +450,12 @@ describe('repos:add + repos:clone', () => { } mockStore.updateProjectHostSetup.mockReturnValue(result) - expect( + await expect( handlers.get('projectHostSetups:update')!(null, { setupId: 'setup-1', updates: { worktreeBasePath: '../worktrees' } }) - ).toBe(result) + ).resolves.toBe(result) expect(prepareLocalWorktreeRootForRepoMock).toHaveBeenCalledWith(mockStore, repo) expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() diff --git a/src/main/ipc/repos-local-clone-lifecycle.test.ts b/src/main/ipc/repos-local-clone-lifecycle.test.ts index d3d94f1b82d..b60bff50181 100644 --- a/src/main/ipc/repos-local-clone-lifecycle.test.ts +++ b/src/main/ipc/repos-local-clone-lifecycle.test.ts @@ -30,6 +30,7 @@ import { registerRepoHandlers } from './repos' import { clearGitCapabilityStateForTests } from '../git/git-capability-state' import { resetSshProviderAuthorities } from '../ssh/ssh-provider-authority' import { DEFAULT_REPO_BADGE_COLOR } from '../../shared/constants' +import { isGitRepoAsync } from '../git/repo' import { listSubmodulePaths } from '../git/status' import { createMockCloneProcess, @@ -155,7 +156,11 @@ describe('repos:add + repos:clone', () => { expect(configReads).toHaveLength(2) }) - it('preserves existing badgeColor when repos:clone upgrades folder->git after dedupe', async () => { + it.each([ + { replacement: false, extraWorkspace: false }, + { replacement: true, extraWorkspace: false }, + { replacement: true, extraWorkspace: true } + ])('uses the current folder after clone proof: %j', async ({ replacement, extraWorkspace }) => { const destination = await createTempRoot() const clonePath = join(destination, 'orca') const existing = { @@ -166,8 +171,21 @@ describe('repos:add + repos:clone', () => { addedAt: 1, kind: 'folder' } - const upgraded = { ...existing, kind: 'git' as const } + const current = replacement + ? { ...existing, id: 'replacement-folder', badgeColor: '#2563eb' } + : existing + const upgraded = { ...current, kind: 'git' as const } mockStore.getRepos.mockReturnValue([existing]) + mockStore.getAllWorktreeMeta.mockReturnValue({}) + vi.mocked(isGitRepoAsync).mockImplementationOnce(async () => { + mockStore.getRepos.mockReturnValue([current]) + if (extraWorkspace) { + mockStore.getAllWorktreeMeta.mockReturnValue({ + [`${current.id}::${clonePath}::workspace:extra`]: {} + }) + } + return true + }) mockStore.updateRepo.mockReturnValue(upgraded) const result = await handlers.get('repos:clone')!(null, { @@ -175,17 +193,51 @@ describe('repos:add + repos:clone', () => { destination }) - expect(mockStore.updateRepo).toHaveBeenCalledWith(existing.id, { - kind: 'git', - projectHostSetupMethod: 'cloned' - }) - expect(result).toEqual(upgraded) - expect(result).toHaveProperty('badgeColor', '#8b5cf6') - expect(prepareLocalWorktreeRootForRepoMock).toHaveBeenCalledWith(mockStore, upgraded) + if (extraWorkspace) { + expect(mockStore.updateRepo).not.toHaveBeenCalled() + expect(result).toEqual(current) + } else { + expect(mockStore.updateRepo).toHaveBeenCalledWith(current.id, { + kind: 'git', + folderUpgradeGitRootPath: clonePath, + projectHostSetupMethod: 'cloned' + }) + expect(result).toEqual(upgraded) + } + expect(result).toHaveProperty('badgeColor', current.badgeColor) + expect(prepareLocalWorktreeRootForRepoMock).toHaveBeenCalledWith(mockStore, result) expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() expect(mockStore.addRepo).not.toHaveBeenCalled() }) + it('registers the completed clone with a fresh id when its saved folder is removed during proof', async () => { + const destination = await createTempRoot() + const clonePath = join(destination, 'orca') + const existing = { + id: 'removed-folder', + path: clonePath, + displayName: 'orca', + badgeColor: '#8b5cf6', + addedAt: 1, + kind: 'folder' as const + } + mockStore.getRepos.mockReturnValue([existing]) + vi.mocked(isGitRepoAsync).mockImplementationOnce(async () => { + mockStore.getRepos.mockReturnValue([]) + return true + }) + + const result = await handlers.get('repos:clone')!(null, { + url: 'https://example.com/orca.git', + destination + }) + + expect(result).not.toHaveProperty('id', existing.id) + expect(result).toMatchObject({ path: clonePath, kind: 'git' }) + expect(mockStore.addRepo).toHaveBeenCalledWith(result) + expect(mockStore.updateRepo).not.toHaveBeenCalled() + }) + it('rejects a dot-segment URL before creating the destination or spawning git', async () => { const tempRoot = await createTempRoot() const destination = join(tempRoot, 'destination') diff --git a/src/main/ipc/repos/project-host-setup-handlers.ts b/src/main/ipc/repos/project-host-setup-handlers.ts index 07d6119dfc8..efc995cfd8f 100644 --- a/src/main/ipc/repos/project-host-setup-handlers.ts +++ b/src/main/ipc/repos/project-host-setup-handlers.ts @@ -16,6 +16,7 @@ import { getProjectIdForProviderIdentity } from '../../../shared/project-host-se import { getProjectHostSetupForRepo } from '../../../shared/project-host-setup-lookup' import { getRepoExecutionHostId, parseExecutionHostId } from '../../../shared/execution-host' import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation' +import { updateProjectHostSetupWithProvenKind } from '../../folder-repo-git-conversion' import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cache' import { emitRepoAdded } from './repo-added-telemetry' import { notifyReposChanged } from './repos-changed-notification' @@ -107,14 +108,21 @@ export function registerProjectHostSetupHandlers(mainWindow: BrowserWindow, stor ipcMain.handle( 'projectHostSetups:update', - (_event, rawArgs: ProjectHostSetupUpdateArgs): ProjectHostSetupUpdateResult => { + async (_event, rawArgs: ProjectHostSetupUpdateArgs): Promise => { const args = parseProjectGroupIpcArgs( ProjectHostSetupUpdateIpcArgs, rawArgs, 'project_host_setup_update_invalid_args' ) assertWslMoveHostSetupIdWritable(store, args.setupId) - const result = store.updateProjectHostSetup(args) + const result = await updateProjectHostSetupWithProvenKind( + store, + (setupArgs) => { + assertWslMoveHostSetupIdWritable(store, setupArgs.setupId) + return store.updateProjectHostSetup(setupArgs) + }, + args + ) if (!result) { throw new Error(`Project host setup not found: ${args.setupId}`) } diff --git a/src/main/ipc/repos/repo-clone-lifecycle.ts b/src/main/ipc/repos/repo-clone-lifecycle.ts index 880b8f8f13a..b70f5a0fc0f 100644 --- a/src/main/ipc/repos/repo-clone-lifecycle.ts +++ b/src/main/ipc/repos/repo-clone-lifecycle.ts @@ -25,6 +25,7 @@ import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cac import { emitRepoAdded } from './repo-added-telemetry' import { notifyReposChanged } from './repos-changed-notification' import { runWithClonePathLock } from './clone-path-lock' +import { resolveClonedFolderConversion } from '../../folder-repo-git-conversion' import { findExtraFolderWorkspaceIds } from '../../folder-workspace-meta-presence' type ActiveCloneMetadata = { @@ -253,13 +254,19 @@ export function registerRepoCloneHandlers(mainWindow: BrowserWindow, store: Stor // Why: check after clone (path didn't exist before); reuse+upgrade a folder repo clone landed into instead of duplicating. const existing = findSaved() if (existing) { - // A folder project with other workspaces stays a folder project: a Git project can't show them. + const conversion = await resolveClonedFolderConversion(store, existing) + const current = findSaved() if ( - isFolderRepo(existing) && - findExtraFolderWorkspaceIds(existing, store.getAllWorktreeMeta()).length === 0 + current && + conversion && + findExtraFolderWorkspaceIds(current, store.getAllWorktreeMeta()).length === 0 && + current.path === existing.path && + isFolderRepo(current) && + getRepoExecutionHostId(current) === getRepoExecutionHostId(existing) ) { - const updated = store.updateRepo(existing.id, { + const updated = store.updateRepo(current.id, { kind: 'git', + ...conversion, projectHostSetupMethod: 'cloned' }) if (updated) { @@ -273,11 +280,13 @@ export function registerRepoCloneHandlers(mainWindow: BrowserWindow, store: Stor } // Why: git re-created this project's folder, so its worktree root is gone with it and // the authorized-roots cache still holds the answers from before the folder came back. - await prepareLocalWorktreeRootForRepo(store, existing) - invalidateAuthorizedRootsCache() - notifyReposChanged(mainWindow) - emitRepoAdded('clone_url', true, true) - return existing + if (current) { + await prepareLocalWorktreeRootForRepo(store, current) + invalidateAuthorizedRootsCache() + notifyReposChanged(mainWindow) + emitRepoAdded('clone_url', true, true) + return current + } } const detected = await detectRepoIconAndUpstream({ diff --git a/src/main/ipc/repos/repo-update-handler.ts b/src/main/ipc/repos/repo-update-handler.ts index 9ef2537aecb..53b5fe0ff60 100644 --- a/src/main/ipc/repos/repo-update-handler.ts +++ b/src/main/ipc/repos/repo-update-handler.ts @@ -2,8 +2,9 @@ import type { BrowserWindow } from 'electron' import { ipcMain } from 'electron' import type { Store } from '../../persistence' import type { Repo } from '../../../shared/repo-types' +import { isFolderRepo } from '../../../shared/repo-kind' import type { ExecutionHostId } from '../../../shared/execution-host' -import { normalizeExecutionHostId } from '../../../shared/execution-host' +import { getRepoExecutionHostId, normalizeExecutionHostId } from '../../../shared/execution-host' import { normalizeRepoBadgeColor } from '../../../shared/repo-badge-color' import { sanitizeRepoIcon } from '../../../shared/repo-icon' import { normalizeGhAccountBinding } from '../../../shared/github/account-binding' @@ -14,6 +15,7 @@ import { normalizeWorktreeVisibilitySourcePreferences } from '../../../shared/worktree/visibility-sources' import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation' +import { commitProvenRepoKindChange } from '../../folder-repo-git-conversion' import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cache' import { notifyReposChanged } from './repos-changed-notification' import { @@ -24,7 +26,7 @@ import { export function registerRepoUpdateHandler(mainWindow: BrowserWindow, store: Store): void { ipcMain.handle( 'repos:update', - ( + async ( _event, args: { repoId: string @@ -61,7 +63,7 @@ export function registerRepoUpdateHandler(mainWindow: BrowserWindow, store: Stor ghAccount?: GhAccountBinding | null } } - ) => { + ): Promise => { if (isWslMoveSourceHost(args.hostId)) { assertWslMoveProjectWritable(store, { repoId: args.repoId }) } @@ -225,9 +227,29 @@ export function registerRepoUpdateHandler(mainWindow: BrowserWindow, store: Stor if (args.hostId && !hostId) { return null } - const updated = hostId - ? store.updateRepo(args.repoId, updates, hostId) - : store.updateRepo(args.repoId, updates) + const write = (proven: Pick): Repo | null => { + if (isWslMoveSourceHost(args.hostId)) { + assertWslMoveProjectWritable(store, { repoId: args.repoId }) + } + return hostId + ? store.updateRepo(args.repoId, { ...updates, ...proven }, hostId) + : store.updateRepo(args.repoId, { ...updates, ...proven }) + } + let updated: Repo | null + if (updates.kind === 'git') { + const target = store + .getRepos() + .find( + (repo) => + repo.id === args.repoId && (!hostId || getRepoExecutionHostId(repo) === hostId) + ) + updated = + target && isFolderRepo(target) + ? await commitProvenRepoKindChange(store, target, write) + : write({}) + } else { + updated = write({}) + } if (updated) { if ('worktreeBasePath' in updates) { void prepareLocalWorktreeRootForRepo(store, updated) diff --git a/src/main/persistence-update-repo.test.ts b/src/main/persistence-update-repo.test.ts index b3d0a5039c2..cebcf5f6b14 100644 --- a/src/main/persistence-update-repo.test.ts +++ b/src/main/persistence-update-repo.test.ts @@ -8,10 +8,16 @@ import { makeProjectHostSetup } from './persistence-test-harness' import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { rmSync, mkdtempSync } from 'node:fs' +import { rmSync, mkdtempSync, mkdirSync } from 'node:fs' +import { execFileSync } from 'node:child_process' import { join } from 'node:path' import { tmpdir } from 'node:os' import { getDefaultPersistedState } from '../shared/constants' +import { + commitProvenRepoKindChange, + proveRepoKindChange, + updateProjectHostSetupWithProvenKind +} from './folder-repo-git-conversion' import { getLocalWorktreeScanGeneration, @@ -357,6 +363,55 @@ describe('Store', () => { ).toMatchObject({ kind: 'git' }) }) + // Git accepts any path inside a work tree, so only the top folder may become a Git project. + it('proves a manual folder-to-Git change against the Git root before writing it', async () => { + const store = await createStore() + const root = join(testState.dir, 'project') + mkdirSync(join(root, 'sub'), { recursive: true }) + execFileSync('git', ['init', '-q'], { cwd: root }) + store.addRepo(makeRepo({ id: 'inner', path: join(root, 'sub'), kind: 'folder' })) + store.addRepo(makeRepo({ id: 'outer', path: root, kind: 'folder' })) + const changeToGit = (setupId: string) => + updateProjectHostSetupWithProvenKind(store, (args) => store.updateProjectHostSetup(args), { + setupId, + updates: { kind: 'git' } + }) + + await expect(changeToGit('inner')).rejects.toThrow("isn't the top folder of a Git repository") + expect(store.getRepo('inner')?.kind).toBe('folder') + const converted = await changeToGit('outer') + expect(converted?.repo).toMatchObject({ + kind: 'git', + folderUpgradeGitRootPath: expect.any(String) + }) + }) + + it('keeps an SSH-owned folder with only executionHostId on the unchecked path', async () => { + const repo = makeRepo({ + id: 'remote-folder', + path: '/remote-only/folder', + kind: 'folder', + executionHostId: 'ssh:build-host' + }) + expect(await proveRepoKindChange(repo, 'git')).toEqual({}) + }) + + it('refuses a proven conversion if the selected folder path changed before commit', async () => { + const root = join(testState.dir, 'moved-project') + mkdirSync(root, { recursive: true }) + execFileSync('git', ['init', '-q'], { cwd: root }) + const repo = makeRepo({ id: 'moving-folder', path: root, kind: 'folder' }) + const write = vi.fn() + await expect( + commitProvenRepoKindChange( + { getRepos: () => [{ ...repo, path: join(root, 'new-path') }] }, + repo, + write + ) + ).rejects.toThrow('The project changed while Orca was checking it') + expect(write).not.toHaveBeenCalled() + }) + it('deletes independent project host setup records without deleting the project', async () => { const independentProject = makeProject({ id: 'cloud-project', diff --git a/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts b/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts index cd1f188a6f3..48a08d04ee5 100644 --- a/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts +++ b/src/main/runtime/orca-runtime-tests/repository-project-operations-part-02.spec.ts @@ -26,6 +26,7 @@ import { createRuntime, store } from '../orca-runtime-test-fixtures.spec' +import { normalizeRuntimePathSeparators } from '../../../shared/cross-platform-path' describe('OrcaRuntimeService', () => { it('adopts public clone repos into host-qualified project setup', async () => { @@ -349,16 +350,22 @@ describe('OrcaRuntimeService', () => { }) it('preserves existing badgeColor on runtime cloneRepo folder->git dedupe upgrade', async () => { + const destination = await mkdtemp(join(tmpdir(), 'orca-runtime-folder-upgrade-')) + const clonePath = join(destination, 'repo-badge-color') const spawnSpy = vi.spyOn(gitRunner, 'gitSpawnAfterWindowsEnvironmentReady') spawnSpy.mockImplementation(() => { const proc = new EventEmitter() as EventEmitter & { stderr: EventEmitter } proc.stderr = new EventEmitter() - setImmediate(() => proc.emit('close', 0, null)) + setImmediate(() => { + mkdirSync(clonePath, { recursive: true }) + execFileSync('git', ['init'], { cwd: clonePath, stdio: 'ignore' }) + proc.emit('close', 0, null) + }) return proc as never }) const existing = { id: 'runtime-folder-upgrade', - path: '/tmp/repo-badge-color', + path: clonePath, displayName: 'repo-badge-color', badgeColor: '#ec4899', addedAt: 1, @@ -377,14 +384,20 @@ describe('OrcaRuntimeService', () => { const runtime = new OrcaRuntimeService(colorStore as never) try { - const repo = await runtime.cloneRepo('https://example.com/repo-badge-color.git', '/tmp') - expect(updates).toEqual([{ id: existing.id, updates: { kind: 'git' } }]) + const repo = await runtime.cloneRepo('https://example.com/repo-badge-color.git', destination) + expect(updates).toEqual([ + { + id: existing.id, + updates: { kind: 'git', folderUpgradeGitRootPath: normalizeRuntimePathSeparators(existing.path) } + } + ]) expect(repo).toEqual(upgraded) expect(repo.badgeColor).toBe('#ec4899') expect(prepareLocalWorktreeRootForRepoMock).toHaveBeenCalledWith(colorStore, upgraded) expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() } finally { spawnSpy.mockRestore() + await rm(destination, { recursive: true, force: true }) } }) @@ -414,7 +427,7 @@ describe('OrcaRuntimeService', () => { expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() }) - it('prepares the runtime worktree root when repo-backed project host setup base path changes', () => { + it('prepares the runtime worktree root when repo-backed project host setup base path changes', async () => { const repo = { id: TEST_REPO_ID, path: TEST_REPO_PATH, @@ -435,12 +448,12 @@ describe('OrcaRuntimeService', () => { } const runtime = new OrcaRuntimeService(runtimeStore as never) - expect( + await expect( runtime.updateProjectHostSetup({ setupId: 'setup-1', updates: { worktreeBasePath: '../worktrees' } }) - ).toBe(result) + ).resolves.toBe(result) expect(prepareLocalWorktreeRootForRepoMock).toHaveBeenCalledWith(runtimeStore, repo) expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() 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 b90dd229205..96b09050d48 100644 --- a/src/main/runtime/rpc/methods/project-runtime-rpc-methods.ts +++ b/src/main/runtime/rpc/methods/project-runtime-rpc-methods.ts @@ -70,9 +70,9 @@ export const PROJECT_RUNTIME_METHODS = [ name: 'projectHostSetup.update', permission: 'workspace', params: ProjectHostSetupUpdate, - handler: (params, context) => ({ + handler: async (params, context) => ({ result: projectRepoResultVisibilityForClient( - context.runtime.updateProjectHostSetup(params), + await context.runtime.updateProjectHostSetup(params), context ) }) diff --git a/src/main/runtime/runtime-project-host-setup-controller.ts b/src/main/runtime/runtime-project-host-setup-controller.ts index 58b9101cf80..4f9c61e1367 100644 --- a/src/main/runtime/runtime-project-host-setup-controller.ts +++ b/src/main/runtime/runtime-project-host-setup-controller.ts @@ -28,6 +28,7 @@ import { } from '../providers/ssh-host-not-served-error' import { runWorktreeChangeInvalidators } from '../ipc/worktree-change-invalidators' import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation' +import { updateProjectHostSetupWithProvenKind } from '../folder-repo-git-conversion' import type { RuntimeStore } from './runtime-store-contract' import { readAllWorktreeMetaForRepo } from '../persistence/host-qualified-worktree-meta' import { @@ -142,13 +143,31 @@ export class RuntimeProjectHostSetupController { ) } - updateSetup(args: ProjectHostSetupUpdateArgs): ProjectHostSetupUpdateResult { + // Why sync up front: availability and the WSL move fence refuse before any Git probe starts. + updateSetup(args: ProjectHostSetupUpdateArgs): Promise { const store = this.deps.getStore() - if (!store?.updateProjectHostSetup) { + const updateProjectHostSetup = store?.updateProjectHostSetup?.bind(store) + if (!store || !updateProjectHostSetup) { throw new Error('runtime_unavailable') } assertWslMoveHostSetupIdWritable(store, args.setupId) - const result = store.updateProjectHostSetup(args) + return this.finishSetupUpdate(store, updateProjectHostSetup, args) + } + + private async finishSetupUpdate( + store: RuntimeStore, + updateProjectHostSetup: NonNullable, + args: ProjectHostSetupUpdateArgs + ): Promise { + const result = await updateProjectHostSetupWithProvenKind( + store, + (setupArgs) => { + assertWslMoveHostSetupIdWritable(store, setupArgs.setupId) + return updateProjectHostSetup(setupArgs) + }, + args, + true + ) if (!result) { throw new Error(`Project host setup not found: ${args.setupId}`) } diff --git a/src/main/runtime/runtime-project-host-setup-publication.test.ts b/src/main/runtime/runtime-project-host-setup-publication.test.ts index 3592f17d5b4..e0f05ab6711 100644 --- a/src/main/runtime/runtime-project-host-setup-publication.test.ts +++ b/src/main/runtime/runtime-project-host-setup-publication.test.ts @@ -59,10 +59,10 @@ describe('RuntimeProjectHostSetupController publication', () => { expect(notifyReposChanged).toHaveBeenCalledTimes(1) }) - it('publishes a rename without retiring cached worktree scans', () => { + it('publishes a rename without retiring cached worktree scans', async () => { const { controller, notifyReposChanged, invalidateResolvedWorktrees, retired } = makeController() - controller.updateSetup({ setupId: setup.id, updates: { displayName: 'renamed' } }) + await controller.updateSetup({ setupId: setup.id, updates: { displayName: 'renamed' } }) expect(notifyReposChanged).toHaveBeenCalledTimes(1) expect(invalidateResolvedWorktrees).toHaveBeenCalled() expect(retired).toEqual([]) @@ -70,9 +70,9 @@ describe('RuntimeProjectHostSetupController publication', () => { it.each([{ kind: 'folder' as const }, { path: '/srv/moved' }])( 'retires cached worktree scans when a setup changes %o', - (updates) => { + async (updates) => { const { controller, notifyReposChanged, retired } = makeController() - controller.updateSetup({ setupId: setup.id, updates }) + await controller.updateSetup({ setupId: setup.id, updates }) expect(retired).toEqual(['repo-1']) expect(notifyReposChanged).toHaveBeenCalledTimes(1) } diff --git a/src/main/runtime/runtime-repository-clone-controller.ts b/src/main/runtime/runtime-repository-clone-controller.ts index 74b6d715be8..05010bdf967 100644 --- a/src/main/runtime/runtime-repository-clone-controller.ts +++ b/src/main/runtime/runtime-repository-clone-controller.ts @@ -7,6 +7,7 @@ import { type ExecutionHostId } from '../../shared/execution-host' import type { Repo } from '../../shared/repo-types' +import { isFolderRepo } from '../../shared/repo-kind' import { getGitCloneFailureMessage } from '../../shared/git-clone-failure-message' import { cleanupClaimedCloneTarget, @@ -18,12 +19,12 @@ import { gitSpawnAfterWindowsEnvironmentReady, promptGuardGitEnv } from '../git/ import { reuseSavedCloneTarget } from '../git/saved-clone-target' import { runWithGitReadCacheInvalidation } from '../git/status' import { invalidateAuthorizedRootsCache } from '../ipc/filesystem-auth' -import { isFolderRepo } from '../../shared/repo-kind' import { detectRepoIconAndUpstream } from '../repo-icon-autodetect' import { getRepoName } from '../git/repo' import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation' import type { RuntimeStore } from './runtime-store-contract' import { runtimeRepoMatchesExecutionHost } from './runtime-worktree-selection' +import { resolveClonedFolderConversion } from '../folder-repo-git-conversion' import { findExtraFolderWorkspaceIds } from '../folder-workspace-meta-presence' type RuntimeRepositoryCloneDependencies = { @@ -155,12 +156,17 @@ export class RuntimeRepositoryCloneController { const existing = findSaved() if (existing) { - // A folder project with other workspaces stays a folder project: a Git project can't show them. + const conversion = await resolveClonedFolderConversion(store, existing) + const current = findSaved() if ( - isFolderRepo(existing) && - findExtraFolderWorkspaceIds(existing, store.getAllWorktreeMeta()).length === 0 + current && + conversion && + findExtraFolderWorkspaceIds(current, store.getAllWorktreeMeta()).length === 0 && + current.path === existing.path && + isFolderRepo(current) && + runtimeRepoMatchesExecutionHost(current, executionHostId) ) { - const updated = store.updateRepo(existing.id, { kind: 'git' }) + const updated = store.updateRepo(current.id, { kind: 'git', ...conversion }) if (updated) { await prepareLocalWorktreeRootForRepo(store, updated) invalidateAuthorizedRootsCache() @@ -170,10 +176,12 @@ export class RuntimeRepositoryCloneController { } // Why: git re-created this project's folder, so its worktree root is gone with it and the // authorized-roots cache still holds the answers from before the folder came back. - await prepareLocalWorktreeRootForRepo(store, existing) - invalidateAuthorizedRootsCache() - this.invalidate(existing.id) - return existing + if (current) { + await prepareLocalWorktreeRootForRepo(store, current) + invalidateAuthorizedRootsCache() + this.invalidate(current.id) + return current + } } // `cloneRepo` ran `git clone` in this process (see `assertCloneHostIsSupported`), so the // checkout is here regardless of the host id stamped on the row. diff --git a/src/main/runtime/runtime-repository-settings-controller.ts b/src/main/runtime/runtime-repository-settings-controller.ts index ce7384e2297..d6abd6aeb18 100644 --- a/src/main/runtime/runtime-repository-settings-controller.ts +++ b/src/main/runtime/runtime-repository-settings-controller.ts @@ -1,5 +1,6 @@ import { getRepoExecutionHostId } from '../../shared/execution-host' import { isFolderRepo } from '../../shared/repo-kind' +import { commitProvenRepoKindChange } from '../folder-repo-git-conversion' import type { Repo } from '../../shared/repo-types' import type { GhAccountBinding } from '../../shared/github/account-binding' import { invalidateAuthorizedRootsCache } from '../ipc/filesystem-auth' @@ -91,7 +92,14 @@ export class RuntimeRepositorySettingsController { if ('sourceControlAi' in updates && updates.sourceControlAi === null) { sanitizedUpdates.sourceControlAi = null } - const updated = store.updateRepo(repo.id, sanitizedUpdates) + const write = (proven: Pick): Repo | null => { + assertWslMoveRepoWritable(store, repo) + return store.updateRepo(repo.id, { ...sanitizedUpdates, ...proven }) + } + const updated = + sanitizedUpdates.kind === 'git' && isFolderRepo(repo) + ? await commitProvenRepoKindChange(store, repo, write, true) + : write({}) if (!updated) { throw new Error('repo_not_found') }