From 4875e771261fc0c942ec1296efdb7df28244fe75 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Mon, 31 Aug 2026 15:01:26 -0700 Subject: [PATCH] fix(repos): stop the retry advice contradicting itself, and cover the runtime lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Readiness review, two recommendations. The retry message contradicted itself. After the most common first-run git failure (unconfigured user.email), the first error says a leftover .git must be removed before retrying — and the second attempt then answered "Choose a different name or location", which is the wrong remedy and contradicts what the user just read. We cannot distinguish a user's own repository from a .git a failed attempt left behind, so the message now names both remedies. The runtime lane had zero coverage for either new behaviour, and it is the lane paired web and remote-desktop clients use: every existing createRepo test passes kind:'folder', which skips the git branch entirely. Added two real-filesystem tests — an existing .git directory, and the .git *pointer file* form used by linked worktrees and submodules — each asserting the refusal and that the bytes we did not create are still on disk afterwards. Mocked fs cannot prove that. Both mutation-tested: removing the runtime refusal turns both red. --- .../ipc/repos/repository-creation-messages.ts | 4 +- src/main/runtime/orca-runtime.test.ts | 46 ++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/main/ipc/repos/repository-creation-messages.ts b/src/main/ipc/repos/repository-creation-messages.ts index c3851a8ad96..1c6579afea4 100644 --- a/src/main/ipc/repos/repository-creation-messages.ts +++ b/src/main/ipc/repos/repository-creation-messages.ts @@ -6,8 +6,10 @@ export const LEFTOVER_GIT_DIR_RETRY_HINT = 'If a .git directory was left behind, remove it before retrying.' +// Why: name both remedies. We cannot tell a user's own repository from a .git a failed attempt +// left behind, and offering only "pick another location" contradicts the retry hint they just read. export function alreadyARepositoryError(name: string): string { - return `"${name}" is already a git repository. Choose a different name or location.` + return `"${name}" is already a git repository. Choose a different name or location — or, if an earlier attempt left a .git directory behind, remove it and try again.` } // Why: a probe that did not complete is not evidence of absence, so we refuse — but as a diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index e4c39a32583..9fdac2aa918 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -14,7 +14,7 @@ import { EventEmitter } from 'node:events' import { createHash, randomUUID } from 'node:crypto' import { execFileSync } from 'node:child_process' import { mkdirSync } from 'node:fs' -import { lstat, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { lstat, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { homedir, tmpdir } from 'node:os' import { basename, join, win32 } from 'node:path' import { ipcMain } from 'electron' @@ -9374,6 +9374,50 @@ describe('OrcaRuntimeService', () => { } }) + // Why real bytes: this lane serves paired web and remote-desktop clients, and the behaviour + // under test is "a .git that is not ours must still be on disk afterwards". A mocked fs cannot + // prove that. + it('refuses a runtime create into an existing repository and leaves its .git intact', async () => { + const runtime = new OrcaRuntimeService(store as never) + const parentDir = await mkdtemp('/tmp/orca-runtime-existing-repo-') + try { + const target = join(parentDir, 'existing') + await mkdir(join(target, '.git'), { recursive: true }) + await writeFile(join(target, '.git', 'HEAD'), 'ref: refs/heads/main\n') + + const result = await runtime.createRepo(parentDir, 'existing', 'git') + + expect(result).toMatchObject({ + error: expect.stringContaining('already a git repository') + }) + await expect(readFile(join(target, '.git', 'HEAD'), 'utf8')).resolves.toBe( + 'ref: refs/heads/main\n' + ) + } finally { + await rm(parentDir, { recursive: true, force: true }) + } + }) + + it('refuses a runtime create when the target holds a .git pointer file', async () => { + // Linked worktrees and submodules point with a `.git` FILE, not a directory. + const runtime = new OrcaRuntimeService(store as never) + const parentDir = await mkdtemp('/tmp/orca-runtime-gitfile-') + try { + const target = join(parentDir, 'linked') + await mkdir(target, { recursive: true }) + await writeFile(join(target, '.git'), 'gitdir: /somewhere/.git/worktrees/linked\n') + + const result = await runtime.createRepo(parentDir, 'linked', 'git') + + expect(result).toMatchObject({ + error: expect.stringContaining('already a git repository') + }) + await expect(readFile(join(target, '.git'), 'utf8')).resolves.toContain('gitdir:') + } finally { + await rm(parentDir, { recursive: true, force: true }) + } + }) + it('defaults runtime createRepo badgeColor to DEFAULT_REPO_BADGE_COLOR', async () => { const added: Record[] = [] const colorStore = {