From 3ed7796624c4d8ffaf82bb8e13d562c3119e00a7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:16:06 -0700 Subject: [PATCH] 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 (`::`) via a new shared `getRepoMainWorktreeId`. Fixes #16447 --- .../RepositoryForkSyncSection.test.tsx | 54 ++++++++++ .../settings/RepositoryForkSyncSection.tsx | 3 +- .../store/repos/safe-auto-fork-sync.test.ts | 98 +++++++++++++++++++ .../src/store/repos/safe-auto-fork-sync.ts | 3 +- src/shared/worktree/id.test.ts | 14 +++ src/shared/worktree/id.ts | 9 ++ 6 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx create mode 100644 src/renderer/src/store/repos/safe-auto-fork-sync.test.ts diff --git a/src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx b/src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx new file mode 100644 index 00000000000..53545eaf740 --- /dev/null +++ b/src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx @@ -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) => 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:` with worktree_id_requires_full_path (#16447). + syncRuntimeGitForkDefaultBranch.mockResolvedValue({ status: 'up-to-date', behind: 0 }) + render( + + ) + + 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' } + ) + }) +}) diff --git a/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx b/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx index 3a399f16e38..a541a82dd24 100644 --- a/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx +++ b/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx @@ -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 }, diff --git a/src/renderer/src/store/repos/safe-auto-fork-sync.test.ts b/src/renderer/src/store/repos/safe-auto-fork-sync.test.ts new file mode 100644 index 00000000000..ec30e09944f --- /dev/null +++ b/src/renderer/src/store/repos/safe-auto-fork-sync.test.ts @@ -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 { + 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:` 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() + }) +}) diff --git a/src/renderer/src/store/repos/safe-auto-fork-sync.ts b/src/renderer/src/store/repos/safe-auto-fork-sync.ts index d2167a9296a..3db51383ee5 100644 --- a/src/renderer/src/store/repos/safe-auto-fork-sync.ts +++ b/src/renderer/src/store/repos/safe-auto-fork-sync.ts @@ -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 }, diff --git a/src/shared/worktree/id.test.ts b/src/shared/worktree/id.test.ts index 726e53abe5a..156c78e9959 100644 --- a/src/shared/worktree/id.test.ts +++ b/src/shared/worktree/id.test.ts @@ -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') diff --git a/src/shared/worktree/id.ts b/src/shared/worktree/id.ts index 478ac6bfa76..82220328eb3 100644 --- a/src/shared/worktree/id.ts +++ b/src/shared/worktree/id.ts @@ -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): 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)