From 720a1d4ea5ba3fcb419d79aed4e69d51a582cbf9 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 12 Jun 2026 00:34:38 -0700 Subject: [PATCH] Fix remote project setup UX gaps Co-authored-by: Orca --- src/main/ipc/repos-remote.test.ts | 2 ++ src/main/ipc/repos.ts | 7 +++++++ .../AddRepoSteps.default-checkout.test.ts | 10 ++++++++++ .../src/components/sidebar/AddRepoSteps.tsx | 9 ++------- .../sidebar/add-repo-store-upsert.ts | 19 +++++++++++++++++++ .../sidebar/useAddRepoCloneFlow.test.ts | 12 +++++++++++- .../components/sidebar/useAddRepoCloneFlow.ts | 12 ++---------- .../useCreateRepo.default-checkout.test.ts | 10 ++++++++++ .../src/components/sidebar/useCreateRepo.ts | 11 ++--------- src/renderer/src/hooks/useComposerState.ts | 9 ++++++++- 10 files changed, 73 insertions(+), 28 deletions(-) create mode 100644 src/renderer/src/components/sidebar/add-repo-store-upsert.ts diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index 7e6dbd2ec6f..515c59afe07 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -54,6 +54,7 @@ const { readDir: vi.fn().mockResolvedValue([]), readFile: vi.fn().mockRejectedValue(new Error('not found')), stat: vi.fn().mockRejectedValue(new Error('not found')), + createDir: vi.fn().mockResolvedValue(undefined), createDirNoClobber: vi.fn().mockResolvedValue(undefined), deletePath: vi.fn().mockResolvedValue(undefined) }, @@ -913,6 +914,7 @@ describe('repos:addRemote', () => { destination: '/home/user' }) + expect(mockFilesystemProvider.createDir).toHaveBeenCalledWith('/home/user') expect(mockGitProvider.clone).toHaveBeenCalledWith( ['clone', '--progress', '--', 'https://github.com/stablyai/orca.git', 'orca'], '/home/user', diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index c96bb713656..bfc64243237 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -329,6 +329,10 @@ async function cloneRemoteRepo( if (!gitProvider) { throw new Error(`SSH connection "${args.connectionId}" not found or not connected`) } + const fsProvider = getSshFilesystemProvider(args.connectionId) + if (!fsProvider) { + throw new Error(`SSH connection "${args.connectionId}" not found or not connected`) + } const host = gitProvider.getHostPlatform?.() if (!host) { throw new Error('SSH host platform is unavailable. Reconnect the SSH target before cloning.') @@ -367,6 +371,9 @@ async function cloneRemoteRepo( activeRemoteClone = metadata remoteCloneInFlightByPath.add(remoteCloneKey) try { + // Why: local clone creates the typed parent before spawning git. SSH clone + // must match that behavior or a fresh remote parent surfaces as spawn ENOENT. + await fsProvider.createDir(trimmedDestination) // Why: the SSH relay exposes argv-based git execution, not a shell. Use // the repo folder name as the target so git creates it inside the chosen // parent, and keep the same flag separator safety as local clone. diff --git a/src/renderer/src/components/sidebar/AddRepoSteps.default-checkout.test.ts b/src/renderer/src/components/sidebar/AddRepoSteps.default-checkout.test.ts index 3dab5029ef8..db874556e9d 100644 --- a/src/renderer/src/components/sidebar/AddRepoSteps.default-checkout.test.ts +++ b/src/renderer/src/components/sidebar/AddRepoSteps.default-checkout.test.ts @@ -8,6 +8,8 @@ const mocks = vi.hoisted(() => ({ stateIndex: 0, storeState: { repos: [] as Repo[], + projects: [], + projectHostSetups: [], clearOrcaHookTrustForRepo: vi.fn(), openModal: vi.fn(), cancelNestedRepoScan: vi.fn() @@ -92,6 +94,8 @@ describe('useRemoteRepo default-checkout handoff', () => { mocks.stateSetters = [] mocks.stateValues = [[], 'ssh-1', '/srv/repo', null, false, null] mocks.storeState.repos = [] + mocks.storeState.projects = [] + mocks.storeState.projectHostSetups = [] mocks.listTargets.mockResolvedValue([ { id: 'ssh-1', label: 'Builder 1' }, { id: 'ssh-2', label: 'Builder 2' } @@ -133,6 +137,12 @@ describe('useRemoteRepo default-checkout handoff', () => { expect(mocks.fetchWorktrees).toHaveBeenCalledWith(repo.id, { requireAuthoritative: true }) + expect(mocks.storeState.projects).toEqual( + expect.arrayContaining([expect.objectContaining({ sourceRepoIds: [repo.id] })]) + ) + expect(mocks.storeState.projectHostSetups).toEqual( + expect.arrayContaining([expect.objectContaining({ repoId: repo.id, path: repo.path })]) + ) expect(mocks.onGitRepoReady).toHaveBeenCalledWith(repo.id) }) diff --git a/src/renderer/src/components/sidebar/AddRepoSteps.tsx b/src/renderer/src/components/sidebar/AddRepoSteps.tsx index f5947a642a3..cb1bb949010 100644 --- a/src/renderer/src/components/sidebar/AddRepoSteps.tsx +++ b/src/renderer/src/components/sidebar/AddRepoSteps.tsx @@ -7,6 +7,7 @@ import type { SshTarget, SshConnectionState } from '../../../../shared/ssh-types import { createNestedRepoTelemetryAttemptId } from '../../../../shared/nested-repo-telemetry' import { translate } from '@/i18n/i18n' import { extractIpcErrorMessage } from '@/lib/ipc-error' +import { upsertAddedRepoWithProjectHostSetup } from './add-repo-store-upsert' // ── Remote project hook ───────────────────────────────────────────── @@ -188,13 +189,7 @@ export function useRemoteRepo( if (existingIdx !== -1) { state.clearOrcaHookTrustForRepo(repo.id) } - if (existingIdx === -1) { - useAppStore.setState({ repos: [...state.repos, repo] }) - } else { - const updated = [...state.repos] - updated[existingIdx] = repo - useAppStore.setState({ repos: updated }) - } + upsertAddedRepoWithProjectHostSetup(repo) if (!mountedRef.current || gen !== remoteGenRef.current) { return diff --git a/src/renderer/src/components/sidebar/add-repo-store-upsert.ts b/src/renderer/src/components/sidebar/add-repo-store-upsert.ts new file mode 100644 index 00000000000..664d5e3e93e --- /dev/null +++ b/src/renderer/src/components/sidebar/add-repo-store-upsert.ts @@ -0,0 +1,19 @@ +import { projectHostSetupProjectionFromRepos } from '../../../../shared/project-host-setup-projection' +import type { Repo } from '../../../../shared/types' +import { useAppStore } from '@/store' + +export function upsertAddedRepoWithProjectHostSetup(repo: Repo): void { + const state = useAppStore.getState() + const repos = state.repos.some((entry) => entry.id === repo.id) + ? state.repos.map((entry) => (entry.id === repo.id ? repo : entry)) + : [...state.repos, repo] + const projection = projectHostSetupProjectionFromRepos(repos) + + // Why: these Add Project flows call IPC directly, bypassing the repo slice + // action that normally keeps the project-first compatibility model synced. + useAppStore.setState({ + repos, + projects: projection.projects, + projectHostSetups: projection.setups + }) +} diff --git a/src/renderer/src/components/sidebar/useAddRepoCloneFlow.test.ts b/src/renderer/src/components/sidebar/useAddRepoCloneFlow.test.ts index 40d0147de1b..23747c0c00f 100644 --- a/src/renderer/src/components/sidebar/useAddRepoCloneFlow.test.ts +++ b/src/renderer/src/components/sidebar/useAddRepoCloneFlow.test.ts @@ -10,7 +10,9 @@ const mocks = vi.hoisted(() => ({ refIndex: 0, storeState: { settings: { activeRuntimeEnvironmentId: null as string | null }, - repos: [] as Repo[] + repos: [] as Repo[], + projects: [], + projectHostSetups: [] }, cloneRemote: vi.fn(), cloneLocal: vi.fn(), @@ -96,6 +98,8 @@ describe('useAddRepoCloneFlow', () => { mocks.refValues = [] mocks.stateValues = ['https://github.com/stablyai/orca.git', '/srv', false, null, null] mocks.storeState.repos = [] + mocks.storeState.projects = [] + mocks.storeState.projectHostSetups = [] vi.stubGlobal('window', { api: { repos: { @@ -134,6 +138,12 @@ describe('useAddRepoCloneFlow', () => { expect(mocks.fetchWorktrees).toHaveBeenCalledWith(repo.id, { requireAuthoritative: true }) + expect(mocks.storeState.projects).toEqual( + expect.arrayContaining([expect.objectContaining({ sourceRepoIds: [repo.id] })]) + ) + expect(mocks.storeState.projectHostSetups).toEqual( + expect.arrayContaining([expect.objectContaining({ repoId: repo.id, path: repo.path })]) + ) expect(mocks.onGitRepoReady).toHaveBeenCalledWith(repo.id, 'clone_url') }) diff --git a/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts b/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts index e5ea4335d79..2f2bc83b4e9 100644 --- a/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts +++ b/src/renderer/src/components/sidebar/useAddRepoCloneFlow.ts @@ -8,6 +8,7 @@ import { getCloneDestinationAutoFill } from './clone-defaults' import type { AddRepoDialogStep } from './add-repo-dialog-types' import { translate } from '@/i18n/i18n' import { extractIpcErrorMessage } from '@/lib/ipc-error' +import { upsertAddedRepoWithProjectHostSetup } from './add-repo-store-upsert' export function useAddRepoCloneFlow({ step, @@ -151,16 +152,7 @@ export function useAddRepoCloneFlow({ translate('auto.components.sidebar.useAddRepoCloneFlow.4d0013cc93', 'Repository cloned'), { description: repo.displayName } ) - // Why: eagerly upsert so step 2 finds the repo before the IPC event. - const state = useAppStore.getState() - const existingIdx = state.repos.findIndex((r) => r.id === repo.id) - if (existingIdx === -1) { - useAppStore.setState({ repos: [...state.repos, repo] }) - } else { - const updated = [...state.repos] - updated[existingIdx] = repo - useAppStore.setState({ repos: updated }) - } + upsertAddedRepoWithProjectHostSetup(repo) // Why: once the repo exists, a transient non-authoritative refresh // should fall through to project reveal instead of leaving the add flow open. await fetchWorktrees(repo.id, { requireAuthoritative: true }) diff --git a/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts b/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts index e26e901c4d1..116d97ffad7 100644 --- a/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts +++ b/src/renderer/src/components/sidebar/useCreateRepo.default-checkout.test.ts @@ -9,6 +9,8 @@ const mocks = vi.hoisted(() => ({ storeState: { settings: { activeRuntimeEnvironmentId: null as string | null }, repos: [] as Repo[], + projects: [], + projectHostSetups: [], worktreesByRepo: {} as Record }, createRepo: vi.fn(), @@ -98,6 +100,8 @@ describe('useCreateRepo default-checkout handoff', () => { mocks.stateSetters = [] mocks.stateValues = ['created', '/projects', 'git', null, false] mocks.storeState.repos = [] + mocks.storeState.projects = [] + mocks.storeState.projectHostSetups = [] mocks.storeState.worktreesByRepo = {} mocks.createRepo.mockReset() mocks.createRemoteRepo.mockReset() @@ -130,6 +134,12 @@ describe('useCreateRepo default-checkout handoff', () => { expect(mocks.fetchWorktrees).toHaveBeenCalledWith(repo.id, { requireAuthoritative: true }) + expect(mocks.storeState.projects).toEqual( + expect.arrayContaining([expect.objectContaining({ sourceRepoIds: [repo.id] })]) + ) + expect(mocks.storeState.projectHostSetups).toEqual( + expect.arrayContaining([expect.objectContaining({ repoId: repo.id, path: repo.path })]) + ) expect(mocks.onGitRepoReady).toHaveBeenCalledWith(repo.id) }) diff --git a/src/renderer/src/components/sidebar/useCreateRepo.ts b/src/renderer/src/components/sidebar/useCreateRepo.ts index 57e41ff088c..71ff106367c 100644 --- a/src/renderer/src/components/sidebar/useCreateRepo.ts +++ b/src/renderer/src/components/sidebar/useCreateRepo.ts @@ -12,6 +12,7 @@ import type { Repo } from '../../../../shared/types' import { translate } from '@/i18n/i18n' import type { RepoKind } from './create-project-defaults' import { extractIpcErrorMessage } from '@/lib/ipc-error' +import { upsertAddedRepoWithProjectHostSetup } from './add-repo-store-upsert' export function useCreateRepo( fetchWorktrees: ( @@ -137,8 +138,6 @@ export function useCreateRepo( return } const repo = result.repo - // Upsert into the store before the repos:changed event round-trips, - // so the next step can find the repo immediately. const state = useAppStore.getState() const existingIdx = state.repos.findIndex((r) => r.id === repo.id) // Why: the IPC handler dedupes by path (see repos:create) and returns @@ -146,13 +145,7 @@ export function useCreateRepo( // handler took the dedup path — no new project was created, so don't // claim one was. const wasDeduped = existingIdx !== -1 - if (existingIdx === -1) { - useAppStore.setState({ repos: [...state.repos, repo] }) - } else { - const updated = [...state.repos] - updated[existingIdx] = repo - useAppStore.setState({ repos: updated }) - } + upsertAddedRepoWithProjectHostSetup(repo) if (wasDeduped) { toast.info( translate( diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index dc973289fef..76147302e10 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -25,6 +25,7 @@ import { import { tuiAgentToAgentKind } from '@/lib/telemetry' import { isGitRepoKind } from '../../../shared/repo-kind' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' +import { getRuntimeRepoBaseRefDefault } from '@/runtime/runtime-repo-client' import type { GitHubWorkItem, GitHubPrStartPoint, @@ -2423,6 +2424,11 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS workspaceName, preserveWorkspaceNameEdits: branchNameOverridePreservesNameEdits }) + const submitBaseBranch = + selectedRepoIsGit && !baseBranch + ? ((await getRuntimeRepoBaseRefDefault(selectedRepoSettings, repoId).catch(() => null)) + ?.defaultBaseRef ?? undefined) + : baseBranch const createDisplayName = smartGitHubResolution?.displayName ?? (nameIsAutoManaged ? submitTitleName?.displayName : undefined) @@ -2503,7 +2509,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS repoId, name: workspaceName, ...(createDisplayName ? { displayName: createDisplayName } : {}), - ...(selectedRepoIsGit && baseBranch ? { baseBranch } : {}), + ...(selectedRepoIsGit && submitBaseBranch ? { baseBranch: submitBaseBranch } : {}), setupDecision: effectiveSetupDecision, ...(selectedRepoIsGit && sparseEnabled ? { @@ -2581,6 +2587,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS resolvedInitialWorkspaceStatus, selectedRepo, selectedRepoIsGit, + selectedRepoSettings, selectedRepoRequiresConnection, showProjectRequiredError, settings?.agentCmdOverrides,