mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(runtime): new local projects never dedupe onto an SSH project at the same path (#31055)
* fix(runtime): project registration dedupes only against rows on this machine Codex review 6 #7: create() matched paths across hosts, so creating a local project at a path an SSH project used returned the SSH row and made no directory. add() with no host had the same gap. One predicate, runtimeRepoIsRegistrationCandidate, now backs add, create and clone. * test(runtime): cover nested-SSH rows and clone reuse in host-qualified dedupe Review follow-up (Opus P3s): runtime:<env> rows carrying a nested SSH connectionId, add() for a runtime host, the clone controller, and a stricter create() assertion. * test(runtime): clone dedupe fixture commits without signing
This commit is contained in:
@@ -1,11 +1,7 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { mkdir } from 'node:fs/promises'
|
||||
import { DEFAULT_REPO_BADGE_COLOR } from '../../shared/constants'
|
||||
import {
|
||||
getRepoSshConnectionId,
|
||||
LOCAL_EXECUTION_HOST_ID,
|
||||
type ExecutionHostId
|
||||
} from '../../shared/execution-host'
|
||||
import { LOCAL_EXECUTION_HOST_ID, 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'
|
||||
@@ -23,7 +19,7 @@ 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 { runtimeRepoIsRegistrationCandidate } from './runtime-worktree-selection'
|
||||
import { resolveClonedFolderConversion } from '../folder-repo-git-conversion'
|
||||
import { findExtraFolderWorkspaceIds } from '../folder-workspace-meta-presence'
|
||||
|
||||
@@ -99,8 +95,7 @@ export class RuntimeRepositoryCloneController {
|
||||
store.getRepos().find((repo) => {
|
||||
return (
|
||||
getClonePathComparisonKey(repo.path) === clonePathKey &&
|
||||
runtimeRepoMatchesExecutionHost(repo, executionHostId) &&
|
||||
!getRepoSshConnectionId(repo)
|
||||
runtimeRepoIsRegistrationCandidate(repo, executionHostId)
|
||||
)
|
||||
})
|
||||
const reused = await reuseSavedCloneTarget(findSaved, trimmedUrl, LOCAL_EXECUTION_HOST_ID)
|
||||
@@ -164,7 +159,7 @@ export class RuntimeRepositoryCloneController {
|
||||
findExtraFolderWorkspaceIds(current, store.getAllWorktreeMeta()).length === 0 &&
|
||||
current.path === existing.path &&
|
||||
isFolderRepo(current) &&
|
||||
runtimeRepoMatchesExecutionHost(current, executionHostId)
|
||||
runtimeRepoIsRegistrationCandidate(current, executionHostId)
|
||||
) {
|
||||
const updated = store.updateRepo(current, { kind: 'git', ...conversion })
|
||||
if (updated) {
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
import { runProcessSync } from '@orca/process-host'
|
||||
import { mkdtemp, realpath, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import { RuntimeRepositoryCloneController } from './runtime-repository-clone-controller'
|
||||
import type { RuntimeStore } from './runtime-store-contract'
|
||||
|
||||
vi.mock('../repo-icon-autodetect', () => ({
|
||||
detectRepoIconAndUpstream: vi.fn().mockResolvedValue({})
|
||||
}))
|
||||
vi.mock('../worktree-root-preparation', () => ({
|
||||
prepareLocalWorktreeRootForRepo: vi.fn().mockResolvedValue(undefined)
|
||||
}))
|
||||
|
||||
function runGit(args: string[]): void {
|
||||
const result = runProcessSync({ program: 'git', args })
|
||||
if (result.code !== 0) {
|
||||
throw new Error(`git ${args.join(' ')} failed: ${result.stderr}`)
|
||||
}
|
||||
}
|
||||
|
||||
function createController(seed: Repo[]): {
|
||||
controller: RuntimeRepositoryCloneController
|
||||
repos: Repo[]
|
||||
} {
|
||||
const repos = [...seed]
|
||||
const store = {
|
||||
getRepos: () => repos,
|
||||
getRepo: (id: string) => repos.find((repo) => repo.id === id),
|
||||
getAllWorktreeMeta: () => ({}),
|
||||
addRepo: (repo: Repo) => {
|
||||
repos.push(repo)
|
||||
return repo
|
||||
},
|
||||
updateRepo: (target: Repo, updates: Partial<Repo>) => Object.assign(target, updates)
|
||||
}
|
||||
const controller = new RuntimeRepositoryCloneController({
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: clone() only uses the methods above.
|
||||
getStore: () => store as unknown as RuntimeStore,
|
||||
invalidateResolvedWorktrees: () => {},
|
||||
invalidateWorktreeScan: () => {},
|
||||
notifyReposChanged: () => {}
|
||||
})
|
||||
return { controller, repos }
|
||||
}
|
||||
|
||||
function folderRow(path: string, fields: Pick<Repo, 'connectionId' | 'executionHostId'>): Repo {
|
||||
return {
|
||||
id: 'existing-row',
|
||||
path,
|
||||
displayName: 'source',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
kind: 'folder',
|
||||
...fields
|
||||
}
|
||||
}
|
||||
|
||||
describe('clone reuses only a project on this machine', () => {
|
||||
let root: string
|
||||
let sourceUrl: string
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await realpath(await mkdtemp(join(tmpdir(), 'orca-clone-host-dedupe-')))
|
||||
const source = join(root, 'origin', 'source')
|
||||
runGit(['init', '-q', source])
|
||||
runGit([
|
||||
'-C',
|
||||
source,
|
||||
'-c',
|
||||
'user.name=t',
|
||||
'-c',
|
||||
'user.email=t@t',
|
||||
'-c',
|
||||
'commit.gpgsign=false',
|
||||
'commit',
|
||||
'-q',
|
||||
'--allow-empty',
|
||||
'-m',
|
||||
'init'
|
||||
])
|
||||
sourceUrl = pathToFileURL(source).href
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(root, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it.each([
|
||||
['executionHostId', { executionHostId: 'ssh:gpu' as const }],
|
||||
['legacy connectionId', { connectionId: 'gpu' }],
|
||||
[
|
||||
'nested SSH on a runtime host',
|
||||
{ executionHostId: 'runtime:env' as const, connectionId: 'gpu' }
|
||||
]
|
||||
])(
|
||||
'adds a new project where an SSH folder project uses the path (%s)',
|
||||
async (_label, fields) => {
|
||||
const destination = join(root, 'dest')
|
||||
const remote = folderRow(join(destination, 'source'), fields)
|
||||
const { controller, repos } = createController([remote])
|
||||
|
||||
const cloned = await controller.clone(sourceUrl, destination)
|
||||
|
||||
expect(cloned.id).not.toBe('existing-row')
|
||||
expect(repos).toHaveLength(2)
|
||||
expect(remote.kind).toBe('folder')
|
||||
}
|
||||
)
|
||||
|
||||
it('converts the local folder project already at the clone path', async () => {
|
||||
const destination = join(root, 'dest')
|
||||
const { controller, repos } = createController([folderRow(join(destination, 'source'), {})])
|
||||
|
||||
const cloned = await controller.clone(sourceUrl, destination)
|
||||
|
||||
expect(cloned.id).toBe('existing-row')
|
||||
expect(repos).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
@@ -3,7 +3,6 @@ import { mkdir, readdir, rm, stat } from 'node:fs/promises'
|
||||
import { isAbsolute, join } from 'node:path'
|
||||
import { DEFAULT_REPO_BADGE_COLOR } from '../../shared/constants'
|
||||
import {
|
||||
getRepoExecutionHostId,
|
||||
LOCAL_EXECUTION_HOST_ID,
|
||||
parseExecutionHostId,
|
||||
type ExecutionHostId
|
||||
@@ -18,7 +17,7 @@ import { detectRepoIconAndUpstream } from '../repo-icon-autodetect'
|
||||
import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation'
|
||||
import type { RuntimeStore } from './runtime-store-contract'
|
||||
import { runtimePathsEqual } from './runtime-worktree-path-identity'
|
||||
import { runtimeRepoMatchesExecutionHost } from './runtime-worktree-selection'
|
||||
import { runtimeRepoIsRegistrationCandidate } from './runtime-worktree-selection'
|
||||
|
||||
type RuntimeRepositoryRegistrationDependencies = {
|
||||
getStore: () => RuntimeStore | null
|
||||
@@ -52,19 +51,12 @@ export class RuntimeRepositoryRegistrationController {
|
||||
const repoPath = gitInfo?.rootPath ?? path
|
||||
const sameHostRepos = store
|
||||
.getRepos()
|
||||
.filter((repo) => runtimeRepoMatchesExecutionHost(repo, executionHostId))
|
||||
.filter((repo) => runtimeRepoIsRegistrationCandidate(repo, executionHostId))
|
||||
// Why: git's toplevel is already resolved through symlinks, so an alias of a tracked checkout finds it.
|
||||
const existing =
|
||||
sameHostRepos.find(
|
||||
(repo) => runtimePathsEqual(repo.path, path) || runtimePathsEqual(repo.path, repoPath)
|
||||
) ??
|
||||
(await findProjectAtSameRealPath(
|
||||
// Why: SSH rows name paths on another machine; resolving them here could match an unrelated local dir.
|
||||
sameHostRepos.filter(
|
||||
(repo) => parseExecutionHostId(getRepoExecutionHostId(repo))?.kind !== 'ssh'
|
||||
),
|
||||
repoPath
|
||||
))
|
||||
) ?? (await findProjectAtSameRealPath(sameHostRepos, repoPath))
|
||||
if (existing) {
|
||||
if (
|
||||
existing.executionHostId == null &&
|
||||
@@ -125,7 +117,15 @@ export class RuntimeRepositoryRegistrationController {
|
||||
return { error: 'Parent directory must be an absolute path' }
|
||||
}
|
||||
const targetPath = join(trimmedParentPath, trimmedName)
|
||||
const existing = store.getRepos().find((repo) => runtimePathsEqual(repo.path, targetPath))
|
||||
// Why host-qualified: an SSH project at this path string is another machine's directory.
|
||||
const findLocalAtTarget = (): Repo | undefined =>
|
||||
store
|
||||
.getRepos()
|
||||
.find(
|
||||
(repo) =>
|
||||
runtimeRepoIsRegistrationCandidate(repo) && runtimePathsEqual(repo.path, targetPath)
|
||||
)
|
||||
const existing = findLocalAtTarget()
|
||||
if (existing) {
|
||||
return { repo: existing }
|
||||
}
|
||||
@@ -161,7 +161,7 @@ export class RuntimeRepositoryRegistrationController {
|
||||
return { error }
|
||||
}
|
||||
}
|
||||
const raceWinner = store.getRepos().find((repo) => runtimePathsEqual(repo.path, targetPath))
|
||||
const raceWinner = findLocalAtTarget()
|
||||
if (raceWinner) {
|
||||
return { repo: raceWinner }
|
||||
}
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
import { mkdir, mkdtemp, realpath, rm, stat } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import { RuntimeRepositoryRegistrationController } from './runtime-repository-registration-controller'
|
||||
import type { RuntimeStore } from './runtime-store-contract'
|
||||
|
||||
vi.mock('../repo-icon-autodetect', () => ({
|
||||
detectRepoIconAndUpstream: vi.fn().mockResolvedValue({})
|
||||
}))
|
||||
vi.mock('../worktree-root-preparation', () => ({
|
||||
prepareLocalWorktreeRootForRepo: vi.fn().mockResolvedValue(undefined)
|
||||
}))
|
||||
|
||||
function createController(seed: Repo[]): {
|
||||
controller: RuntimeRepositoryRegistrationController
|
||||
repos: Repo[]
|
||||
} {
|
||||
const repos = [...seed]
|
||||
const store = {
|
||||
getRepos: () => repos,
|
||||
getRepo: (id: string) => repos.find((repo) => repo.id === id),
|
||||
addRepo: (repo: Repo) => {
|
||||
repos.push(repo)
|
||||
return repo
|
||||
},
|
||||
updateRepo: () => null
|
||||
}
|
||||
const controller = new RuntimeRepositoryRegistrationController({
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: add()/create() only use getRepos/getRepo/addRepo/updateRepo.
|
||||
getStore: () => store as unknown as RuntimeStore,
|
||||
invalidateResolvedWorktrees: () => {},
|
||||
invalidateWorktreeScan: () => {},
|
||||
notifyReposChanged: () => {}
|
||||
})
|
||||
return { controller, repos }
|
||||
}
|
||||
|
||||
function remoteRow(path: string, fields: Pick<Repo, 'connectionId' | 'executionHostId'>): Repo {
|
||||
return {
|
||||
id: 'ssh-row',
|
||||
path,
|
||||
displayName: 'remote',
|
||||
badgeColor: 'blue',
|
||||
addedAt: 1,
|
||||
kind: 'folder',
|
||||
...fields
|
||||
}
|
||||
}
|
||||
|
||||
// Why the third: a runtime host's nested SSH target appears only in `connectionId`.
|
||||
const SSH_SPELLINGS = [
|
||||
['executionHostId', { executionHostId: 'ssh:gpu' as const }],
|
||||
['legacy connectionId', { connectionId: 'gpu' }],
|
||||
['nested SSH on a runtime host', { executionHostId: 'runtime:env' as const, connectionId: 'gpu' }]
|
||||
] as const
|
||||
|
||||
describe('project registration dedupes only against rows on the same host', () => {
|
||||
let root: string
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await realpath(await mkdtemp(join(tmpdir(), 'orca-runtime-host-dedupe-')))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(root, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it.each(SSH_SPELLINGS)(
|
||||
'create makes a local project where an SSH project uses the same path (%s)',
|
||||
async (_label, fields) => {
|
||||
const targetPath = join(root, 'notes')
|
||||
const { controller, repos } = createController([remoteRow(targetPath, fields)])
|
||||
|
||||
const result = await controller.create(root, 'notes', 'folder')
|
||||
|
||||
if (!('repo' in result)) {
|
||||
throw new Error(result.error)
|
||||
}
|
||||
expect(result.repo.id).not.toBe('ssh-row')
|
||||
expect(result.repo.path).toBe(targetPath)
|
||||
expect(repos).toHaveLength(2)
|
||||
expect((await stat(targetPath)).isDirectory()).toBe(true)
|
||||
}
|
||||
)
|
||||
|
||||
it('create still returns the local project already at that path', async () => {
|
||||
const targetPath = join(root, 'notes')
|
||||
await mkdir(targetPath)
|
||||
const local: Repo = { ...remoteRow(targetPath, {}), id: 'local-row' }
|
||||
const { controller, repos } = createController([local])
|
||||
|
||||
const result = await controller.create(root, 'notes', 'folder')
|
||||
|
||||
if (!('repo' in result)) {
|
||||
throw new Error(result.error)
|
||||
}
|
||||
expect(result.repo.id).toBe('local-row')
|
||||
expect(repos).toHaveLength(1)
|
||||
})
|
||||
|
||||
it.each(SSH_SPELLINGS)(
|
||||
'add registers a local folder where an SSH project uses the same path (%s)',
|
||||
async (_label, fields) => {
|
||||
const folderPath = join(root, 'notes')
|
||||
await mkdir(folderPath)
|
||||
const { controller, repos } = createController([remoteRow(folderPath, fields)])
|
||||
|
||||
const added = await controller.add(folderPath, 'folder')
|
||||
|
||||
expect(added.id).not.toBe('ssh-row')
|
||||
expect(repos).toHaveLength(2)
|
||||
}
|
||||
)
|
||||
|
||||
it("add for a runtime host skips that host's nested SSH project at the same path", async () => {
|
||||
const folderPath = join(root, 'notes')
|
||||
await mkdir(folderPath)
|
||||
const nested = remoteRow(folderPath, { executionHostId: 'runtime:env', connectionId: 'gpu' })
|
||||
const { controller, repos } = createController([nested])
|
||||
|
||||
const added = await controller.add(folderPath, 'folder', 'runtime:env')
|
||||
|
||||
expect(added.id).not.toBe('ssh-row')
|
||||
expect(repos).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('add for a runtime host still returns its own project at the same path', async () => {
|
||||
const folderPath = join(root, 'notes')
|
||||
await mkdir(folderPath)
|
||||
const own: Repo = { ...remoteRow(folderPath, { executionHostId: 'runtime:env' }), id: 'own' }
|
||||
const { controller, repos } = createController([own])
|
||||
|
||||
const added = await controller.add(folderPath, 'folder', 'runtime:env')
|
||||
|
||||
expect(added.id).toBe('own')
|
||||
expect(repos).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,9 @@
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
|
||||
import {
|
||||
getRepoExecutionHostId,
|
||||
getRepoSshConnectionId,
|
||||
type ExecutionHostId
|
||||
} from '../../shared/execution-host'
|
||||
import { splitWorktreeId } from '../../shared/worktree/id'
|
||||
import type { GitPushTarget } from '../../shared/worktree/types'
|
||||
|
||||
@@ -60,6 +64,17 @@ export function runtimeRepoMatchesExecutionHost(
|
||||
return getRepoExecutionHostId(repo) === executionHostId
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `repo` can be the row a registration in this process dedupes onto. Registration reads its
|
||||
* path on this machine, so an SSH row at the same path string is another machine's project.
|
||||
*/
|
||||
export function runtimeRepoIsRegistrationCandidate(
|
||||
repo: Pick<Repo, 'connectionId' | 'executionHostId'>,
|
||||
executionHostId?: ExecutionHostId | null
|
||||
): boolean {
|
||||
return runtimeRepoMatchesExecutionHost(repo, executionHostId) && !getRepoSshConnectionId(repo)
|
||||
}
|
||||
|
||||
export function parseExactWorktreeIdSelector(
|
||||
selector: string
|
||||
): RuntimeWorktreeRemovalTarget | null {
|
||||
|
||||
Reference in New Issue
Block a user