mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(fork-sync): address runtime repos by main worktree id, not repo id (#16876)
Repo-level fork sync (Safe Auto and the Sync Now button) passed `repo.id` as the runtime worktree selector, so runtime-hosted repos always failed with `worktree_id_requires_full_path`. Compose the repo's main worktree id (`<repoId>::<repo.path>`) via a new shared `getRepoMainWorktreeId`. Fixes #16447
This commit is contained in:
@@ -0,0 +1,54 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import { RepositoryForkSyncSection } from './RepositoryForkSyncSection'
|
||||
|
||||
const syncRuntimeGitForkDefaultBranch = vi.fn()
|
||||
|
||||
vi.mock('../../runtime/runtime-git-client', () => ({
|
||||
syncRuntimeGitForkDefaultBranch: (...args: unknown[]) => syncRuntimeGitForkDefaultBranch(...args)
|
||||
}))
|
||||
|
||||
vi.mock('../../store', () => ({
|
||||
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
|
||||
selector({ settingsSearchQuery: '', settings: { activeRuntimeEnvironmentId: 'env-1' } })
|
||||
}))
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: Object.assign(vi.fn(), { message: vi.fn(), success: vi.fn(), error: vi.fn() })
|
||||
}))
|
||||
|
||||
const RUNTIME_REPO: Repo = {
|
||||
id: 'repo-1',
|
||||
path: '/srv/repo-1',
|
||||
displayName: 'repo-1',
|
||||
badgeColor: '#000000',
|
||||
addedAt: 0,
|
||||
kind: 'git',
|
||||
executionHostId: 'runtime:env-1',
|
||||
forkSyncMode: 'ask',
|
||||
upstream: { owner: 'up', repo: 'r' }
|
||||
}
|
||||
|
||||
describe('RepositoryForkSyncSection', () => {
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
syncRuntimeGitForkDefaultBranch.mockReset()
|
||||
})
|
||||
|
||||
it('syncs a runtime-hosted repo by its main worktree id, not the bare repo id', () => {
|
||||
// Why: the runtime rejects `id:<repo-id>` with worktree_id_requires_full_path (#16447).
|
||||
syncRuntimeGitForkDefaultBranch.mockResolvedValue({ status: 'up-to-date', behind: 0 })
|
||||
render(
|
||||
<RepositoryForkSyncSection repo={RUNTIME_REPO} updateRepo={vi.fn()} forceVisible={true} />
|
||||
)
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: /sync now/i }))
|
||||
|
||||
expect(syncRuntimeGitForkDefaultBranch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ worktreeId: 'repo-1::/srv/repo-1', worktreePath: '/srv/repo-1' }),
|
||||
{ owner: 'up', repo: 'r' }
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -3,6 +3,7 @@ import { RefreshCw } from 'lucide-react'
|
||||
import { toast } from 'sonner'
|
||||
import type { ForkSyncMode, GitForkSyncResult } from '../../../../shared/git-fork-sync'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import { getRepoMainWorktreeId } from '../../../../shared/worktree/id'
|
||||
import { Button } from '../ui/button'
|
||||
import { SearchableSetting } from './SearchableSetting'
|
||||
import { SettingsSegmentedControl } from './SettingsFormControls'
|
||||
@@ -128,7 +129,7 @@ export function RepositoryForkSyncSection({
|
||||
const result = await syncRuntimeGitForkDefaultBranch(
|
||||
{
|
||||
settings: getRepoOwnerRoutedSettings(settings, repo),
|
||||
worktreeId: repo.id,
|
||||
worktreeId: getRepoMainWorktreeId(repo),
|
||||
worktreePath: repo.path,
|
||||
connectionId: repo.connectionId ?? undefined
|
||||
},
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { AppState } from '../types'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import {
|
||||
createCompatibleRuntimeStatusResponseIfNeeded,
|
||||
type RuntimeEnvironmentCallRequest
|
||||
} from '../../runtime/runtime-compatibility-test-fixture'
|
||||
import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client'
|
||||
import { safeAutoForkSyncAttempts, scheduleSafeAutoForkSync } from './safe-auto-fork-sync'
|
||||
|
||||
const runtimeEnvironmentCall = vi.fn()
|
||||
const runtimeEnvironmentTransportCall = vi.fn()
|
||||
const gitSyncFork = vi.fn()
|
||||
|
||||
const RUNTIME_REPO: Repo = {
|
||||
id: 'repo-1',
|
||||
path: '/srv/repo-1',
|
||||
displayName: 'repo-1',
|
||||
badgeColor: '#000000',
|
||||
addedAt: 0,
|
||||
kind: 'git',
|
||||
executionHostId: 'runtime:env-1',
|
||||
forkSyncMode: 'safe-auto',
|
||||
upstream: { owner: 'up', repo: 'r' }
|
||||
}
|
||||
|
||||
function stateWith(repo: Repo): AppState {
|
||||
return {
|
||||
repos: [repo],
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' }
|
||||
} as unknown as AppState
|
||||
}
|
||||
|
||||
async function flushScheduledSyncs(): Promise<void> {
|
||||
await Promise.all([...safeAutoForkSyncAttempts.values()].map((attempt) => attempt.promise))
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
clearRuntimeCompatibilityCacheForTests()
|
||||
safeAutoForkSyncAttempts.clear()
|
||||
runtimeEnvironmentCall.mockReset()
|
||||
runtimeEnvironmentTransportCall.mockReset()
|
||||
gitSyncFork.mockReset()
|
||||
gitSyncFork.mockResolvedValue({ status: 'up-to-date', behind: 0 })
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-1',
|
||||
ok: true,
|
||||
result: { status: 'up-to-date', behind: 0 },
|
||||
_meta: { runtimeId: 'remote-runtime' }
|
||||
})
|
||||
runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => {
|
||||
return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args)
|
||||
})
|
||||
vi.stubGlobal('window', {
|
||||
api: {
|
||||
git: { syncFork: gitSyncFork },
|
||||
runtimeEnvironments: { call: runtimeEnvironmentTransportCall }
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('scheduleSafeAutoForkSync', () => {
|
||||
it('addresses a runtime-hosted repo by its main worktree id, not the bare repo id', async () => {
|
||||
// Why: the runtime rejects `id:<repo-id>` with worktree_id_requires_full_path (#16447).
|
||||
scheduleSafeAutoForkSync(() => stateWith(RUNTIME_REPO), [RUNTIME_REPO])
|
||||
await flushScheduledSyncs()
|
||||
|
||||
expect(runtimeEnvironmentCall).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
method: 'git.forkSync',
|
||||
params: expect.objectContaining({ worktree: 'id:repo-1::/srv/repo-1' })
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('still runs local repos against the repo path over local git IPC', async () => {
|
||||
const localRepo: Repo = {
|
||||
...RUNTIME_REPO,
|
||||
id: 'repo-2',
|
||||
path: '/home/me/repo-2',
|
||||
executionHostId: 'local'
|
||||
}
|
||||
|
||||
scheduleSafeAutoForkSync(
|
||||
() =>
|
||||
({ ...stateWith(localRepo), settings: { activeRuntimeEnvironmentId: null } }) as AppState,
|
||||
[localRepo]
|
||||
)
|
||||
await flushScheduledSyncs()
|
||||
|
||||
expect(gitSyncFork).toHaveBeenCalledWith({
|
||||
worktreePath: '/home/me/repo-2',
|
||||
connectionId: undefined,
|
||||
expectedUpstream: { owner: 'up', repo: 'r' }
|
||||
})
|
||||
expect(runtimeEnvironmentCall).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -2,6 +2,7 @@ import type { AppState } from '../types'
|
||||
import type { Repo } from '../../../../shared/repo-types'
|
||||
import { syncRuntimeGitForkDefaultBranch } from '../../runtime/runtime-git-client'
|
||||
import { getRepoExecutionHostId } from '../../../../shared/execution-host'
|
||||
import { getRepoMainWorktreeId } from '../../../../shared/worktree/id'
|
||||
import { settingsForRepoOwner } from './owner-routing'
|
||||
|
||||
export const SAFE_AUTO_FORK_SYNC_COOLDOWN_MS = 10 * 60 * 1000
|
||||
@@ -32,7 +33,7 @@ export function scheduleSafeAutoForkSync(get: () => AppState, repos: readonly Re
|
||||
const promise = syncRuntimeGitForkDefaultBranch(
|
||||
{
|
||||
settings: settingsForRepoOwner(get(), repo.id),
|
||||
worktreeId: repo.id,
|
||||
worktreeId: getRepoMainWorktreeId(repo),
|
||||
worktreePath: repo.path,
|
||||
connectionId: repo.connectionId ?? undefined
|
||||
},
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
WORKTREE_ID_SEPARATOR,
|
||||
getRepoIdFromWorktreeId,
|
||||
getRepoMainWorktreeId,
|
||||
getWorktreePathBasenameFromId,
|
||||
splitWorktreeId,
|
||||
splitWorktreeIdForFilesystem,
|
||||
@@ -14,6 +15,19 @@ describe('WORKTREE_ID_SEPARATOR', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('getRepoMainWorktreeId', () => {
|
||||
it('round-trips through the id parsers on posix and Windows paths', () => {
|
||||
for (const repo of [
|
||||
{ id: 'repo-123', path: '/abs/path' },
|
||||
{ id: 'repo-123', path: 'C:\\Users\\me\\repo' }
|
||||
]) {
|
||||
const worktreeId = getRepoMainWorktreeId(repo)
|
||||
expect(worktreeId).toBe(`${repo.id}${WORKTREE_ID_SEPARATOR}${repo.path}`)
|
||||
expect(splitWorktreeId(worktreeId)).toEqual({ repoId: repo.id, worktreePath: repo.path })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('getRepoIdFromWorktreeId', () => {
|
||||
it('returns the repo id for a canonical worktree id', () => {
|
||||
expect(getRepoIdFromWorktreeId('repo-123::/abs/path')).toBe('repo-123')
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { normalizeRuntimePathForComparison } from '../cross-platform-path'
|
||||
import { WORKTREE_ID_SEPARATOR } from '../pty-session-id-format'
|
||||
import type { Repo } from '../repo-types'
|
||||
|
||||
export { WORKTREE_ID_SEPARATOR } from '../pty-session-id-format'
|
||||
|
||||
@@ -13,6 +14,14 @@ const FOLDER_WORKSPACE_INSTANCE_SUFFIX = new RegExp(
|
||||
`${FOLDER_WORKSPACE_INSTANCE_SEPARATOR.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}[0-9a-f-]{36}$`
|
||||
)
|
||||
|
||||
/**
|
||||
* Worktree id of the repo's own checkout. A bare repo id is never a valid worktree id —
|
||||
* runtimes reject it with `worktree_id_requires_full_path` (#16447).
|
||||
*/
|
||||
export function getRepoMainWorktreeId(repo: Pick<Repo, 'id' | 'path'>): string {
|
||||
return `${repo.id}${WORKTREE_ID_SEPARATOR}${repo.path}`
|
||||
}
|
||||
|
||||
export function getRepoIdFromWorktreeId(worktreeId: string): string {
|
||||
const separatorIdx = worktreeId.indexOf(WORKTREE_ID_SEPARATOR)
|
||||
return separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx)
|
||||
|
||||
Reference in New Issue
Block a user