diff --git a/config/scripts/run-ssh-docker-e2e.mjs b/config/scripts/run-ssh-docker-e2e.mjs index 96727e837abb..1a67f56cf10c 100644 --- a/config/scripts/run-ssh-docker-e2e.mjs +++ b/config/scripts/run-ssh-docker-e2e.mjs @@ -54,6 +54,7 @@ const result = spawnSync( 'playwright', 'test', 'tests/e2e/local-ssh-browser-routing.spec.ts', + 'tests/e2e/repo-registration-async.spec.ts', 'tests/e2e/ssh-client-hosted-browser-drop-reconnect.spec.ts', 'tests/e2e/pty-input-write-queue-ssh.spec.ts', 'tests/e2e/ssh-ai-vault-session-history.spec.ts', diff --git a/src/main/git/repo-detection-async.test.ts b/src/main/git/repo-detection-async.test.ts new file mode 100644 index 000000000000..19c89216d10c --- /dev/null +++ b/src/main/git/repo-detection-async.test.ts @@ -0,0 +1,142 @@ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import * as runner from './runner' +import { runProcess } from '@orca/process-host' +import { + getGitRepoRoot, + getGitRepoRootAsync, + inspectGitRepoForRegistration, + inspectGitRepoForRegistrationAsync, + isGitRepo, + isGitRepoAsync +} from './repo-detection' + +let fixture: string +async function git(cwd: string, args: string[]): Promise { + const result = await runProcess({ program: 'git', args, cwd, timeoutMs: 15_000 }) + if (result.code !== 0) { + throw new Error(result.stderr) + } +} + +beforeEach(() => { + fixture = mkdtempSync(join(tmpdir(), 'orca-repo-async-')) +}) +afterEach(() => { + vi.restoreAllMocks() + rmSync(fixture, { recursive: true, force: true }) +}) + +async function expectSameDetection(path: string): Promise { + expect(await isGitRepoAsync(path)).toBe(isGitRepo(path)) + expect(await getGitRepoRootAsync(path)).toBe(getGitRepoRoot(path)) + expect(await inspectGitRepoForRegistrationAsync(path)).toEqual( + inspectGitRepoForRegistration(path) + ) +} + +describe('asynchronous repository detection with real Git', () => { + it('preserves normal, nested, linked-worktree, bare, and folder results', async () => { + const repo = join(fixture, 'repo with spaces') + const nested = join(repo, 'packages', 'web') + mkdirSync(nested, { recursive: true }) + await git(repo, ['init', '--quiet']) + await git(repo, [ + '-c', + 'user.name=Orca Test', + '-c', + 'user.email=orca@example.invalid', + 'commit', + '--allow-empty', + '--quiet', + '-m', + 'fixture' + ]) + const linked = join(fixture, 'linked worktree') + await git(repo, ['worktree', 'add', '--quiet', '-b', 'fixture-linked', linked]) + const bare = join(fixture, 'bare.git') + await git(fixture, ['init', '--bare', '--quiet', bare]) + const folder = join(fixture, 'folder') + mkdirSync(folder) + for (const path of [repo, nested, linked, bare, folder]) { + await expectSameDetection(path) + } + expect(await isGitRepoAsync(folder)).toBe(false) + expect(await isGitRepoAsync(bare)).toBe(true) + expect((await inspectGitRepoForRegistrationAsync(linked)).mainRepoPath).not.toBeNull() + }) + + it('preserves genuine metadata fallback when asynchronous Git cannot answer', async () => { + const repo = join(fixture, 'fallback repo') + const nested = join(repo, 'packages') + mkdirSync(nested, { recursive: true }) + await git(repo, ['init', '--quiet']) + vi.spyOn(runner, 'gitExecFileAsync').mockRejectedValue(new Error('Git unavailable')) + expect(await isGitRepoAsync(nested)).toBe(true) + expect(await getGitRepoRootAsync(nested)).toBe(repo.replaceAll('\\', '/')) + expect(await isGitRepoAsync(join(repo, '.git'))).toBe(false) + }) + + it('rejects missing directories and invalid Git markers without changing root fallback', async () => { + const invalid = join(fixture, 'invalid') + mkdirSync(invalid) + writeFileSync(join(invalid, '.git'), 'not a gitdir file') + for (const path of [invalid, join(fixture, 'missing')]) { + await expectSameDetection(path) + expect(await isGitRepoAsync(path)).toBe(false) + } + }) + + it.skipIf(process.platform === 'win32')( + 'preserves newlines, dollar signs and trailing spaces in roots and linked identity', + async () => { + const repo = join(fixture, 'repo\nwith dollar$ and trailing space ') + const nested = join(repo, 'nested\n') + mkdirSync(nested, { recursive: true }) + await git(repo, ['init', '--quiet']) + await git(repo, [ + '-c', + 'user.name=Orca Test', + '-c', + 'user.email=orca@example.invalid', + 'commit', + '--allow-empty', + '--quiet', + '-m', + 'fixture' + ]) + const linked = join(fixture, 'linked\n ') + await git(repo, ['worktree', 'add', '--quiet', '-b', 'edge-linked', linked]) + for (const path of [repo, nested, linked]) { + await expectSameDetection(path) + } + expect((await inspectGitRepoForRegistrationAsync(nested)).rootPath).toBe(realpathSync(repo)) + expect((await inspectGitRepoForRegistrationAsync(linked)).mainRepoPath).toBe( + realpathSync(repo) + ) + } + ) + + it('preserves symlink target identity and rejects its invalid nested marker without Git', async () => { + const repo = join(fixture, 'target') + const nested = join(repo, 'nested') + mkdirSync(nested, { recursive: true }) + await git(repo, ['init', '--quiet']) + const alias = join(fixture, 'alias') + symlinkSync(nested, alias, process.platform === 'win32' ? 'junction' : 'dir') + await expectSameDetection(alias) + vi.spyOn(runner, 'gitExecFileSync').mockImplementation(() => { + throw new Error('Git unavailable') + }) + vi.spyOn(runner, 'gitExecFileAsync').mockRejectedValue(new Error('Git unavailable')) + await expectSameDetection(alias) + expect((await inspectGitRepoForRegistrationAsync(alias)).rootPath).toBe( + realpathSync(repo).replaceAll('\\', '/') + ) + writeFileSync(join(nested, '.git'), 'gitdir: missing-metadata') + await expectSameDetection(alias) + expect(await isGitRepoAsync(alias)).toBe(false) + }) +}) diff --git a/src/main/git/repo-detection-command.test.ts b/src/main/git/repo-detection-command.test.ts new file mode 100644 index 000000000000..e3e1120feed0 --- /dev/null +++ b/src/main/git/repo-detection-command.test.ts @@ -0,0 +1,90 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { gitExecFileAsync, gitExecFileSync } from './runner' +import { gitRepoOutput, runGitRepoCommands, runGitRepoCommandsSync } from './repo-detection-command' + +vi.mock('./runner', () => ({ gitExecFileAsync: vi.fn(), gitExecFileSync: vi.fn() })) +afterEach(() => vi.resetAllMocks()) + +describe('repository command execution', () => { + it('lets another event-loop turn run while Git is pending, with no synchronous spawn', async () => { + let finish: (value: { stdout: string; stderr: string }) => void = () => {} + vi.mocked(gitExecFileAsync).mockImplementation( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + let settled = false + const detection = runGitRepoCommands( + gitRepoOutput(['rev-parse', '--show-toplevel'], { cwd: '/repo' }) + ).then((result) => { + settled = true + return result + }) + await new Promise((resolve) => setImmediate(resolve)) + expect(settled).toBe(false) + expect(gitExecFileSync).not.toHaveBeenCalled() + expect(gitExecFileAsync).toHaveBeenCalledExactlyOnceWith(['rev-parse', '--show-toplevel'], { + cwd: '/repo', + timeout: 15_000 + }) + finish({ stdout: '/repo\n', stderr: '' }) + expect(await detection).toBe('/repo\n') + }) + + it('delivers command failures to the shared decision tree in both execution modes', async () => { + function* withFallback() { + try { + return yield* gitRepoOutput(['rev-parse'], { cwd: '/repo' }) + } catch { + return 'marker fallback' + } + } + vi.mocked(gitExecFileAsync).mockRejectedValue(new Error('Git unavailable')) + vi.mocked(gitExecFileSync).mockImplementation(() => { + throw new Error('Git unavailable') + }) + expect(await runGitRepoCommands(withFallback())).toBe('marker fallback') + expect(runGitRepoCommandsSync(withFallback())).toBe('marker fallback') + }) + + it('preserves an exception from decision logic after a successful command', async () => { + const error = new Error('decision failed') + function* decisionFailure() { + yield* gitRepoOutput(['first'], { cwd: '/repo' }) + throw error + } + vi.mocked(gitExecFileAsync).mockResolvedValue({ stdout: 'result', stderr: '' }) + vi.mocked(gitExecFileSync).mockReturnValue('result') + await expect(runGitRepoCommands(decisionFailure())).rejects.toBe(error) + expect(() => runGitRepoCommandsSync(decisionFailure())).toThrow(error) + }) + + it('executes a fallback command yielded while handling a rejected command', async () => { + function* withFallbackCommand() { + try { + return yield* gitRepoOutput(['first'], { cwd: '/repo' }) + } catch { + return yield* gitRepoOutput(['second'], { cwd: '/repo' }) + } + } + vi.mocked(gitExecFileAsync) + .mockRejectedValueOnce(new Error('first failed')) + .mockResolvedValueOnce({ stdout: 'fallback', stderr: '' }) + vi.mocked(gitExecFileSync) + .mockImplementationOnce(() => { + throw new Error('first failed') + }) + .mockReturnValueOnce('fallback') + expect(await runGitRepoCommands(withFallbackCommand())).toBe('fallback') + expect(runGitRepoCommandsSync(withFallbackCommand())).toBe('fallback') + expect(vi.mocked(gitExecFileAsync).mock.calls.map(([args]) => args)).toEqual([ + ['first'], + ['second'] + ]) + expect(vi.mocked(gitExecFileSync).mock.calls.map(([args]) => args)).toEqual([ + ['first'], + ['second'] + ]) + }) +}) diff --git a/src/main/git/repo-detection-command.ts b/src/main/git/repo-detection-command.ts new file mode 100644 index 000000000000..778688e15477 --- /dev/null +++ b/src/main/git/repo-detection-command.ts @@ -0,0 +1,39 @@ +import { gitExecFileAsync, gitExecFileSync } from './runner' + +type GitRepoCommand = { args: string[]; cwd: string } +export type GitRepoCommands = Generator + +export function* gitRepoOutput(args: string[], options: { cwd: string }): GitRepoCommands { + return yield { args, cwd: options.cwd } +} + +// One decision tree keeps Git failures and marker fallbacks identical across both callers. +export async function runGitRepoCommands(commands: GitRepoCommands): Promise { + let step = commands.next() + while (!step.done) { + const command = step.value + try { + const { stdout } = await gitExecFileAsync(command.args, { + cwd: command.cwd, + timeout: 15_000 + }) + step = commands.next(stdout) + } catch (error) { + step = commands.throw(error) + } + } + return step.value +} + +export function runGitRepoCommandsSync(commands: GitRepoCommands): T { + let step = commands.next() + while (!step.done) { + const command = step.value + try { + step = commands.next(gitExecFileSync(command.args, { cwd: command.cwd })) + } catch (error) { + step = commands.throw(error) + } + } + return step.value +} diff --git a/src/main/git/repo-detection-failure-parity.test.ts b/src/main/git/repo-detection-failure-parity.test.ts new file mode 100644 index 000000000000..38c69253a9d0 --- /dev/null +++ b/src/main/git/repo-detection-failure-parity.test.ts @@ -0,0 +1,55 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import * as runner from './runner' +import { inspectGitRepoForRegistration, inspectGitRepoForRegistrationAsync } from './repo-detection' + +let fixture: string +beforeEach(() => { + fixture = mkdtempSync(join(tmpdir(), 'orca-repo-failure-parity-')) + mkdirSync(join(fixture, '.git', 'objects'), { recursive: true }) + mkdirSync(join(fixture, '.git', 'refs'), { recursive: true }) + writeFileSync(join(fixture, '.git', 'HEAD'), 'ref: refs/heads/main\n') + writeFileSync(join(fixture, '.git', 'config'), '[core]\n bare = false\n') +}) +afterEach(() => { + vi.restoreAllMocks() + rmSync(fixture, { recursive: true, force: true }) +}) + +it.each([0, 1, 2, 3, 4])( + 'preserves registration fallback and command order when command %i fails', + async (failAt) => { + // Ambiguous combined output requires separate root, Git directory and common directory reads. + const outputs = [ + 'true\nfalse\n', + `true\n${fixture}\nextra\n.git\n.git\n`, + `${fixture}\n`, + '.git\n', + '.git\n' + ] + const sync = vi.spyOn(runner, 'gitExecFileSync') + const async = vi.spyOn(runner, 'gitExecFileAsync') + let syncIndex = 0 + let asyncIndex = 0 + sync.mockImplementation(() => { + const index = syncIndex++ + if (index === failAt) { + throw new Error('rejected stage') + } + return outputs[index] ?? '' + }) + async.mockImplementation(async () => { + const index = asyncIndex++ + if (index === failAt) { + throw new Error('rejected stage') + } + return { stdout: outputs[index] ?? '', stderr: '' } + }) + const expected = inspectGitRepoForRegistration(fixture) + expect(expected).toEqual({ isRepo: true, rootPath: fixture, mainRepoPath: null }) + expect(await inspectGitRepoForRegistrationAsync(fixture)).toEqual(expected) + expect(async.mock.calls.map(([args]) => args)).toEqual(sync.mock.calls.map(([args]) => args)) + } +) diff --git a/src/main/git/repo-detection.test.ts b/src/main/git/repo-detection.test.ts index 83bf39391893..37e6a2ed0833 100644 --- a/src/main/git/repo-detection.test.ts +++ b/src/main/git/repo-detection.test.ts @@ -80,7 +80,7 @@ describe('isGitRepo', () => { git(realRepo, ['init', '--quiet']) withGitUnavailable(() => { - expect(getGitRepoRoot(nestedDir)).toBe(realRepo) + expect(getGitRepoRoot(nestedDir)).toBe(realRepo.replaceAll('\\', '/')) }) }) @@ -330,7 +330,7 @@ describe('isGitRepo', () => { const bareRepo = path.join(tmpDir, 'bare.git') git(tmpDir, ['init', '--bare', '--quiet', bareRepo]) - expect(getGitRepoRoot(bareRepo)).toBe(bareRepo) + expect(getGitRepoRoot(bareRepo)).toBe(bareRepo.replaceAll('\\', '/')) }) }) diff --git a/src/main/git/repo-detection.ts b/src/main/git/repo-detection.ts index 3fe75a1148d5..9be70187a793 100644 --- a/src/main/git/repo-detection.ts +++ b/src/main/git/repo-detection.ts @@ -4,7 +4,12 @@ import { normalizeRuntimePathSeparators } from '../../shared/cross-platform-path import { parseWslUncPath } from '../../shared/wsl-paths' import { toWindowsWslPath } from '../wsl' import { scanGitMarkerSync, resolveRealPathSync } from './repo-git-marker-scan' -import { gitExecFileSync } from './runner' +import { + gitRepoOutput, + runGitRepoCommands, + runGitRepoCommandsSync, + type GitRepoCommands +} from './repo-detection-command' type GitRepoProbeResult = 'repo' | 'not-repo' | 'indeterminate' type GitRepoProbe = { @@ -23,7 +28,7 @@ export type GitRepoRegistrationInfo = { let warnedMarkerFallbackThisSession = false /** Check if a path is a valid git repository (regular or bare). */ -export function isGitRepo(path: string): boolean { +function* isGitRepoCommands(path: string): GitRepoCommands { try { if (!statSync(path, { throwIfNoEntry: false })?.isDirectory()) { return false @@ -32,7 +37,7 @@ export function isGitRepo(path: string): boolean { return false } - return isGitRepoFromProbe(path, probeGitRepo(path).result) + return isGitRepoFromProbe(path, (yield* probeGitRepo(path)).result) } function isGitRepoFromProbe(path: string, result: GitRepoProbeResult): boolean { @@ -54,10 +59,10 @@ function isGitRepoFromProbe(path: string, result: GitRepoProbeResult): boolean { } /** Only a clean pair of negative Git answers is a definitive non-repo. */ -function probeGitRepo(path: string, includeLocation = false): GitRepoProbe { +function* probeGitRepo(path: string, includeLocation = false): GitRepoCommands { try { const records = readGitPathOutput( - gitExecFileSync( + yield* gitRepoOutput( [ 'rev-parse', '--is-inside-work-tree', @@ -76,7 +81,7 @@ function probeGitRepo(path: string, includeLocation = false): GitRepoProbe { : 'indeterminate' const location = includeLocation && insideWorkTree === 'true' && records.length !== 4 - ? readGitRepoDirectories(path) + ? yield* readGitRepoDirectories(path) : { gitDir, commonDir } return { result, insideWorkTree: insideWorkTree === 'true', ...location } } catch { @@ -85,7 +90,9 @@ function probeGitRepo(path: string, includeLocation = false): GitRepoProbe { } /** Reuse one repository discovery across registration's validity, root and worktree checks. */ -export function inspectGitRepoForRegistration(path: string): GitRepoRegistrationInfo { +function* inspectGitRepoForRegistrationCommands( + path: string +): GitRepoCommands { try { if (!statSync(path, { throwIfNoEntry: false })?.isDirectory()) { return { isRepo: false, rootPath: path, mainRepoPath: null } @@ -93,14 +100,14 @@ export function inspectGitRepoForRegistration(path: string): GitRepoRegistration } catch { return { isRepo: false, rootPath: path, mainRepoPath: null } } - const probe = probeGitRepo(path) + const probe = yield* probeGitRepo(path) const isRepo = isGitRepoFromProbe(path, probe.result) let rootPath = path let mainRepoPath: string | null = null if (isRepo && probe.insideWorkTree) { try { const records = readGitPathOutput( - gitExecFileSync( + yield* gitRepoOutput( [ 'rev-parse', '--is-inside-work-tree', @@ -119,9 +126,9 @@ export function inspectGitRepoForRegistration(path: string): GitRepoRegistration ? { toplevel, gitDir, commonDir } : { toplevel: readGitPathOutput( - gitExecFileSync(['rev-parse', '--show-toplevel'], { cwd: path }) + yield* gitRepoOutput(['rev-parse', '--show-toplevel'], { cwd: path }) ), - ...readGitRepoDirectories(path) + ...(yield* readGitRepoDirectories(path)) } if (location.toplevel) { rootPath = normalizeGitRepoRootForInputPath(path, location.toplevel) @@ -145,13 +152,13 @@ export function inspectGitRepoForRegistration(path: string): GitRepoRegistration return { isRepo, rootPath, mainRepoPath } } -export function getGitRepoRoot(path: string): string { +function* getGitRepoRootCommands(path: string): GitRepoCommands { try { if (!existsSync(path) || !statSync(path).isDirectory()) { return path } // A bare repo has no toplevel; keep its marker fallback separate from the boolean probes. - const output = gitExecFileSync(['rev-parse', '--is-inside-work-tree', '--show-toplevel'], { + const output = yield* gitRepoOutput(['rev-parse', '--is-inside-work-tree', '--show-toplevel'], { cwd: path }) const firstNewline = output.indexOf('\n') @@ -171,10 +178,14 @@ function readGitPathOutput(output: string): string { return output.endsWith('\n') ? output.slice(0, -1) : output } -function readGitRepoDirectories(path: string): Pick { +function* readGitRepoDirectories( + path: string +): GitRepoCommands> { return { - gitDir: readGitPathOutput(gitExecFileSync(['rev-parse', '--git-dir'], { cwd: path })), - commonDir: readGitPathOutput(gitExecFileSync(['rev-parse', '--git-common-dir'], { cwd: path })) + gitDir: readGitPathOutput(yield* gitRepoOutput(['rev-parse', '--git-dir'], { cwd: path })), + commonDir: readGitPathOutput( + yield* gitRepoOutput(['rev-parse', '--git-common-dir'], { cwd: path }) + ) } } @@ -191,13 +202,13 @@ function canonicalizeGitDirPath(path: string): string { } /** Return the main-checkout path only when `path` is a linked worktree. */ -export function getLinkedWorktreeMainRepoRoot(path: string): string | null { +function* getLinkedWorktreeMainRepoRootCommands(path: string): GitRepoCommands { try { if (!statSync(path, { throwIfNoEntry: false })?.isDirectory()) { return null } - const mainRepoPath = mainRepoPathFromProbe(path, probeGitRepo(path, true)) - return mainRepoPath ? getGitRepoRoot(mainRepoPath) : null + const mainRepoPath = mainRepoPathFromProbe(path, yield* probeGitRepo(path, true)) + return mainRepoPath ? yield* getGitRepoRootCommands(mainRepoPath) : null } catch { return null } @@ -225,3 +236,31 @@ export function normalizeGitRepoRootForInputPath(inputPath: string, rootPath: st } return normalizeRuntimePathSeparators(rootPath) } + +export function isGitRepo(path: string): boolean { + return runGitRepoCommandsSync(isGitRepoCommands(path)) +} + +export function isGitRepoAsync(path: string): Promise { + return runGitRepoCommands(isGitRepoCommands(path)) +} + +export function inspectGitRepoForRegistration(path: string): GitRepoRegistrationInfo { + return runGitRepoCommandsSync(inspectGitRepoForRegistrationCommands(path)) +} + +export function inspectGitRepoForRegistrationAsync(path: string): Promise { + return runGitRepoCommands(inspectGitRepoForRegistrationCommands(path)) +} + +export function getGitRepoRoot(path: string): string { + return runGitRepoCommandsSync(getGitRepoRootCommands(path)) +} + +export function getGitRepoRootAsync(path: string): Promise { + return runGitRepoCommands(getGitRepoRootCommands(path)) +} + +export function getLinkedWorktreeMainRepoRoot(path: string): string | null { + return runGitRepoCommandsSync(getLinkedWorktreeMainRepoRootCommands(path)) +} diff --git a/src/main/git/repo.ts b/src/main/git/repo.ts index da64c013efba..f1bb563be62e 100644 --- a/src/main/git/repo.ts +++ b/src/main/git/repo.ts @@ -11,8 +11,11 @@ import { gitExecFileAsync } from './runner' export { isGitRepo, + isGitRepoAsync, inspectGitRepoForRegistration, + inspectGitRepoForRegistrationAsync, getGitRepoRoot, + getGitRepoRootAsync, getLinkedWorktreeMainRepoRoot, normalizeGitRepoRootForInputPath } from './repo-detection' diff --git a/src/main/ipc/repos-add-linked-worktree.test.ts b/src/main/ipc/repos-add-linked-worktree.test.ts index b6ad18063b1c..1433a1e816cc 100644 --- a/src/main/ipc/repos-add-linked-worktree.test.ts +++ b/src/main/ipc/repos-add-linked-worktree.test.ts @@ -44,12 +44,14 @@ vi.mock('electron', () => ({ vi.mock('../git/repo', () => ({ isGitRepo: isGitRepoMock, - inspectGitRepoForRegistration: vi.fn((path: string) => ({ + isGitRepoAsync: isGitRepoMock, + inspectGitRepoForRegistrationAsync: vi.fn((path: string) => ({ isRepo: isGitRepoMock(path), rootPath: getGitRepoRootMock(path), mainRepoPath: getLinkedWorktreeMainRepoRootMock(path) })), getGitRepoRoot: getGitRepoRootMock, + getGitRepoRootAsync: getGitRepoRootMock, getLinkedWorktreeMainRepoRoot: getLinkedWorktreeMainRepoRootMock, getRepoName: vi.fn().mockImplementation((path: string) => path.split('/').pop()), getBaseRefDefault: vi.fn().mockResolvedValue('origin/main'), diff --git a/src/main/ipc/repos-remote-test-harness.ts b/src/main/ipc/repos-remote-test-harness.ts index 183e3a4ab7b9..eea5e7677252 100644 --- a/src/main/ipc/repos-remote-test-harness.ts +++ b/src/main/ipc/repos-remote-test-harness.ts @@ -121,8 +121,10 @@ export function gitRepoModuleMock(actual: typeof RepoModule): Record ({ + getGitRepoRootAsync: getGitRepoRoot, + inspectGitRepoForRegistrationAsync: vi.fn((path: string) => ({ isRepo: isGitRepo(path), rootPath: getGitRepoRoot(path), mainRepoPath: null diff --git a/src/main/ipc/repos/local-repo-registration.ts b/src/main/ipc/repos/local-repo-registration.ts index 43fec4637cd1..4d8c98e8c831 100644 --- a/src/main/ipc/repos/local-repo-registration.ts +++ b/src/main/ipc/repos/local-repo-registration.ts @@ -5,7 +5,11 @@ import { isFolderRepo } from '../../../shared/repo-kind' import { DEFAULT_REPO_BADGE_COLOR } from '../../../shared/constants' import { normalizeRuntimePathForComparison } from '../../../shared/cross-platform-path' import { awaitWindowsHostGitEnvironmentReady } from '../../git/runner' -import { inspectGitRepoForRegistration, getGitRepoRoot, getRepoName } from '../../git/repo' +import { + inspectGitRepoForRegistrationAsync, + getGitRepoRootAsync, + getRepoName +} from '../../git/repo' import { LOCAL_EXECUTION_HOST_ID } from '../../../shared/execution-host' import { detectRepoIconAndUpstream } from '../../repo-icon-autodetect' import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation' @@ -20,7 +24,7 @@ export async function addLocalRepoFromPath( if (repoKind === 'git') { await awaitWindowsHostGitEnvironmentReady({ cwd: path }) } - const gitInfo = repoKind === 'git' ? inspectGitRepoForRegistration(path) : null + const gitInfo = repoKind === 'git' ? await inspectGitRepoForRegistrationAsync(path) : null if (gitInfo && !gitInfo.isRepo) { return { error: `Not a valid git repository: ${path}` } } @@ -52,7 +56,9 @@ export async function addLocalRepoFromPath( // same project and host — a duplicate run-target row that resolves to a transient worktree path. let mainRepoKey: string | null = null if (repoKind === 'git') { - const mainRepoRoot = gitInfo?.mainRepoPath ? getGitRepoRoot(gitInfo.mainRepoPath) : null + const mainRepoRoot = gitInfo?.mainRepoPath + ? await getGitRepoRootAsync(gitInfo.mainRepoPath) + : null if (mainRepoRoot) { mainRepoKey = normalizeRuntimePathForComparison(mainRepoRoot) // Why !isFolderRepo: only a git-kind main checkout projects onto the same project as its diff --git a/src/main/runtime/runtime-repository-registration-controller.ts b/src/main/runtime/runtime-repository-registration-controller.ts index cc64e20a9acb..01099f34b63d 100644 --- a/src/main/runtime/runtime-repository-registration-controller.ts +++ b/src/main/runtime/runtime-repository-registration-controller.ts @@ -9,7 +9,7 @@ import { } from '../../shared/execution-host' import type { Repo } from '../../shared/repo-types' import { gitExecFileAsync, awaitWindowsHostGitEnvironmentReady } from '../git/runner' -import { getRepoName, isGitRepo } from '../git/repo' +import { getRepoName, isGitRepoAsync } from '../git/repo' import { invalidateAuthorizedRootsCache, isENOENT } from '../ipc/filesystem-auth' import { detectRepoIconAndUpstream } from '../repo-icon-autodetect' import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation' @@ -40,7 +40,7 @@ export class RuntimeRepositoryRegistrationController { if (kind === 'git') { await awaitWindowsHostGitEnvironmentReady({ cwd: path }) } - if (kind === 'git' && !isGitRepo(path)) { + if (kind === 'git' && !(await isGitRepoAsync(path))) { throw new Error(`Not a valid git repository: ${path}`) } const existing = store.getRepos().find((repo) => { diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index 139990cd5821..69fa1ec3e3a2 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -30,6 +30,7 @@ import { fastForwardLocalBaseBranch } from './worktree/local-base-branch-fast-fo import { gitChangeListArgs, parseGitChangeList } from './git-change-list' import { encodeGitPathspecs } from './git-pathspec-stdin' import { endSubprocessStdin } from './subprocess-stdin-write' +import { registerGitRepositoryRegistrationBinaryCompatibilityCase } from './git-repository-registration-binary-compatibility.test-cases' import { registerGitResolutionBinaryCompatibilityCases } from './git-resolution-binary-compatibility.test-cases' const execFileAsync = promisify(execFile) @@ -138,6 +139,8 @@ describeBinaryCompatibility('real Git binary compatibility', () => { } }) + registerGitRepositoryRegistrationBinaryCompatibilityCase(runGit, () => repoPath, Boolean(image)) + it('stages, unstages and restores NUL-delimited literal pathspecs from stdin', async () => { const fixturePath = join(repoPath, 'stdin-pathspec') await mkdir(fixturePath) diff --git a/src/shared/git-repository-registration-binary-compatibility.test-cases.ts b/src/shared/git-repository-registration-binary-compatibility.test-cases.ts new file mode 100644 index 000000000000..ff7811e6c46b --- /dev/null +++ b/src/shared/git-repository-registration-binary-compatibility.test-cases.ts @@ -0,0 +1,60 @@ +import { mkdir, rm } from 'node:fs/promises' +import { join, posix, resolve } from 'node:path' +import { expect, it } from 'vitest' + +export function registerGitRepositoryRegistrationBinaryCompatibilityCase( + runGit: (args: string[]) => Promise<{ stdout: string; stderr: string }>, + getRepoPath: () => string, + image: boolean +): void { + it('supports repository registration probes for nested, linked and bare repositories', async () => { + const repoPath = getRepoPath() + const executionRoot = image ? '/repo' : repoPath + const nested = join(repoPath, 'registration nested') + const linked = image ? '/repo/registration linked' : join(repoPath, 'registration linked') + const bare = image ? '/repo/registration bare' : join(repoPath, 'registration bare') + await mkdir(nested) + await runGit(['worktree', 'add', '-b', 'registration-linked', linked]) + await runGit(['init', '--bare', bare]) + try { + const root = (await runGit(['rev-parse', '--show-toplevel'])).stdout.trim() + const linkedRoot = ( + await runGit(['-C', linked, 'rev-parse', '--show-toplevel']) + ).stdout.trim() + for (const cwd of [executionRoot, image ? '/repo/registration nested' : nested, linked]) { + expect( + (await runGit(['-C', cwd, 'rev-parse', '--is-inside-work-tree', '--is-bare-repository'])) + .stdout + ).toBe('true\nfalse\n') + const records = ( + await runGit([ + '-C', + cwd, + 'rev-parse', + '--is-inside-work-tree', + '--show-toplevel', + '--git-dir', + '--git-common-dir' + ]) + ).stdout + .trim() + .split('\n') + expect(records).toHaveLength(4) + expect(records[0]).toBe('true') + expect(records[1]).toBe(cwd === linked ? linkedRoot : root) + const resolveDirectory = image ? posix.resolve : resolve + expect(resolveDirectory(cwd, records[2]) === resolveDirectory(cwd, records[3])).toBe( + cwd !== linked + ) + } + expect( + (await runGit(['-C', bare, 'rev-parse', '--is-inside-work-tree', '--is-bare-repository'])) + .stdout + ).toBe('false\ntrue\n') + } finally { + await runGit(['worktree', 'remove', '--force', linked]) + await rm(nested, { recursive: true, force: true }) + await rm(join(repoPath, 'registration bare'), { recursive: true, force: true }) + } + }) +} diff --git a/tests/e2e/repo-registration-async.spec.ts b/tests/e2e/repo-registration-async.spec.ts new file mode 100644 index 000000000000..01fa0bf276bf --- /dev/null +++ b/tests/e2e/repo-registration-async.spec.ts @@ -0,0 +1,180 @@ +import { mkdirSync, mkdtempSync, realpathSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' +import { runProcess } from '@orca/process-host' + +test.use({ seedTestRepo: false }) + +async function runFixtureGit(spec: Parameters[0]): Promise { + const result = await runProcess(spec) + expect(result.code, result.stderr).toBe(0) +} + +test('registration resolves nested and linked paths to one existing project', async ({ + orcaPage, + registerPostElectronShutdownCleanup +}) => { + const root = realpathSync(mkdtempSync(join(tmpdir(), 'orca-registration-'))) + const repoPath = join(root, 'repo') + const nestedPath = join(repoPath, 'nested') + const linkedPath = join(root, 'linked') + mkdirSync(nestedPath, { recursive: true }) + registerPostElectronShutdownCleanup(async () => { + const { rm } = await import('node:fs/promises') + await rm(root, { recursive: true, force: true }) + }) + await runFixtureGit({ program: 'git', args: ['init', repoPath] }) + await runFixtureGit({ + program: 'git', + args: [ + '-c', + 'user.name=E2E', + '-c', + 'user.email=e2e@test.local', + 'commit', + '--allow-empty', + '-m', + 'fixture' + ], + cwd: repoPath + }) + await runFixtureGit({ + program: 'git', + args: ['worktree', 'add', '-b', 'linked', linkedPath], + cwd: repoPath + }) + await waitForSessionReady(orcaPage) + const result = await orcaPage.evaluate( + async ({ repoPath, nestedPath }) => { + const results = [] + for (const path of [repoPath, nestedPath]) { + results.push(await window.api.repos.add({ path })) + } + if (results.some((result) => 'error' in result)) { + throw new Error(JSON.stringify(results)) + } + await window.__store!.getState().fetchRepos() + await window.__store!.getState().awaitLocalRepoCatalogSettlement() + return { + ids: results.map((result) => ('repo' in result ? result.repo.id : null)), + persistedIds: (await window.api.repos.list()).map((repo) => repo.id) + } + }, + { repoPath, nestedPath } + ) + expect(new Set(result.ids).size).toBe(1) + expect(result.persistedIds).toEqual([result.ids[0]]) + // A repos:changed refresh can supersede the explicitly awaited catalog fetch. + await expect + .poll(() => orcaPage.evaluate(() => window.__store!.getState().repos.map((repo) => repo.id))) + .toEqual(result.persistedIds) + const linked = await orcaPage.evaluate(async (path) => window.api.repos.add({ path }), linkedPath) + expect('repo' in linked && linked.repo.id).toBe(result.ids[0]) + const missing = await orcaPage.evaluate( + async (path) => window.api.repos.add({ path }), + join(root, 'missing') + ) + expect('error' in missing).toBe(true) + expect( + await orcaPage.evaluate(async () => (await window.api.repos.list()).map((repo) => repo.id)) + ).toEqual(result.persistedIds) +}) + +test('SSH registration reads the repository on its execution host', async ({ + orcaPage +}, testInfo) => { + test.skip(process.env.ORCA_E2E_SSH_DOCKER !== '1', 'Requires the existing Docker SSH fixture') + test.setTimeout(180_000) + const { + startDockerSshRelayTarget, + cleanupDockerSshRelayTarget, + execDockerSshRelayTargetCommand, + DOCKER_SSH_RELAY_REMOTE_REPO_PATH + } = await import('./helpers/docker-ssh-relay-target') + const { connectDockerSshRelayTarget, disconnectDockerSshRelayTarget } = + await import('./helpers/docker-ssh-relay-connection') + const target = startDockerSshRelayTarget(testInfo) + let targetId: string | null = null + try { + await waitForSessionReady(orcaPage) + const connected = await connectDockerSshRelayTarget(orcaPage, target, { seedInitialTab: false }) + targetId = connected.targetId + execDockerSshRelayTargetCommand(target, `mkdir -p ${DOCKER_SSH_RELAY_REMOTE_REPO_PATH}/nested`) + const nested = await orcaPage.evaluate( + async ({ connectionId, remotePath }) => + window.api.repos.addRemote({ connectionId, remotePath }), + { connectionId: targetId, remotePath: `${DOCKER_SSH_RELAY_REMOTE_REPO_PATH}/nested` } + ) + expect('repo' in nested && nested.repo.id).toBe(connected.repoId) + expect('repo' in nested && nested.repo.connectionId).toBe(targetId) + expect('repo' in nested && nested.repo.executionHostId).toBe(`ssh:${targetId}`) + const missing = await orcaPage.evaluate( + async (connectionId) => + window.api.repos.addRemote({ connectionId, remotePath: '/tmp/or60risk-missing' }), + targetId + ) + expect('error' in missing).toBe(true) + await orcaPage.screenshot({ path: testInfo.outputPath('ssh-project.png') }) + } finally { + if (targetId) { + await disconnectDockerSshRelayTarget(orcaPage, targetId) + } + cleanupDockerSshRelayTarget(target) + } +}) + +test('real WSL registration preserves literal paths and fences login output', async ({ + orcaPage +}, testInfo) => { + const distro = process.env.ORCA_REAL_WSL_DISTRO + test.skip(process.platform !== 'win32' || !distro, 'Requires a real Windows WSL distro') + const { buildWslExecArgs, buildWslCapturedLoginShellCommand, quotePosixShell } = + await import('../../src/shared/wsl-login-shell-command') + const root = `/tmp/or60risk/run-${Date.now()}` + const repoPath = `${root}/repo-$literal space` + const uncPath = `\\\\wsl.localhost\\${distro}${repoPath.replaceAll('/', '\\')}` + const setup = await runProcess({ + program: 'wsl.exe', + args: buildWslExecArgs(distro, [ + '/bin/sh', + '-c', + `mkdir -p ${quotePosixShell(`${repoPath}/nested`)} && git init ${quotePosixShell(repoPath)}` + ]), + timeoutMs: 30_000 + }) + expect(setup.code, setup.stderr).toBe(0) + try { + const capture = buildWslCapturedLoginShellCommand( + `git -C ${quotePosixShell(repoPath)} rev-parse --is-inside-work-tree` + ) + const raw = await runProcess({ + program: 'wsl.exe', + args: buildWslExecArgs(distro, [ + '/bin/sh', + '-c', + `printf 'OR60_LOGIN_NOISE\\n'; ${capture.command}` + ]), + timeoutMs: 30_000 + }) + expect(raw.code, raw.stderr).toBe(0) + expect(raw.stdout).toContain('OR60_LOGIN_NOISE') + expect(capture.readStdout(raw.stdout)).toBe('true\n') + await waitForSessionReady(orcaPage) + const added = await orcaPage.evaluate( + async (path) => window.api.repos.add({ path }), + `${uncPath}\\nested` + ) + expect('repo' in added).toBe(true) + expect('repo' in added && added.repo.path).toBe(uncPath) + await orcaPage.screenshot({ path: testInfo.outputPath('wsl-project.png') }) + } finally { + const cleanup = await runProcess({ + program: 'wsl.exe', + args: buildWslExecArgs(distro, ['/bin/sh', '-c', `rm -rf ${quotePosixShell(root)}`]), + timeoutMs: 30_000 + }) + expect(cleanup.code, cleanup.stderr).toBe(0) + } +})