mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 08:02:02 +00:00
fix(repos): stop the retry advice contradicting itself, and cover the runtime lane
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<string, unknown>[] = []
|
||||
const colorStore = {
|
||||
|
||||
Reference in New Issue
Block a user