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') }