From adb8ac58251d26eee97e3bbd2b8d20a0d311fd52 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 11 Oct 2026 23:04:56 -0400 Subject: [PATCH] fix(mobile): keep the New Workspace sheet on a hidden repo's project A last-visited per-workspace-environment repo is hidden, so the sheet fell back to the first listed project. It now maps it to its project's eligible copy with the desktop's resolver, moved to src/shared. The phone's mirror of the seeding rule is gone, since the hook imports the shared one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../NewWorktreeModal.server-ssh.test.tsx | 32 ++++++++--- .../use-new-workspace-execution-target.ts | 2 +- .../use-new-workspace-repositories.ts | 29 ++++++---- ...ew-workspace-dialog-repo-selection.test.ts | 55 +------------------ .../new-workspace-dialog-repo-selection.ts | 46 +--------------- .../lib/new-workspace-composer-repo.test.ts | 40 +------------- .../src/lib/new-workspace-composer-repo.ts | 21 +------ src/shared/new-workspace-dialog-repo.test.ts | 39 +++++++++++++ src/shared/new-workspace-dialog-repo.ts | 32 ++++++++++- 9 files changed, 119 insertions(+), 177 deletions(-) diff --git a/mobile/src/components/NewWorktreeModal.server-ssh.test.tsx b/mobile/src/components/NewWorktreeModal.server-ssh.test.tsx index 70144e2d36b..d3a7022ea4c 100644 --- a/mobile/src/components/NewWorktreeModal.server-ssh.test.tsx +++ b/mobile/src/components/NewWorktreeModal.server-ssh.test.tsx @@ -266,10 +266,13 @@ describe('NewWorktreeModal on a server SSH repo', () => { expect(runTargetPicker(renderer).props.selectedId).toBe('runtime:env-1') expect(server).toHaveBeenCalledWith('ssh.getState', { targetId: 'devbox' }) expect(gate()).toMatchObject({ status: null, requiresConnection: true }) - expect(server).not.toHaveBeenCalledWith('worktree.create', expect.anything(), expect.anything()) + act(() => sourceInputs(renderer)[0]!.props.onChangeText('same-id-wt')) + expect( + renderer.root.find((node) => typeof node.props.onCreate === 'function').props.canCreate + ).toBe(false) }) - it("never offers a server's per-workspace-environment SSH repo, even last visited", async () => { + it("never offers a server's per-workspace-environment SSH repo, and maps it to its project", async () => { const vmRepo = { ...repos[0]!, id: 'vm-repo', @@ -277,6 +280,13 @@ describe('NewWorktreeModal on a server SSH repo', () => { connectionId: 'runtime-ssh-vm-1' } const serverRepo = { ...repos[0]!, id: 'server-repo', path: '/srv/orca' } + const otherProject = { + ...repos[0]!, + id: 'noqa', + displayName: 'noqa', + path: '/srv/noqa', + upstream: { owner: 'stablyai', repo: 'noqa' } + } asyncStorage.getItem.mockResolvedValueOnce( JSON.stringify({ hostId: 'host-vm', worktreeId: 'vm-repo::/home/dev/vm-orca/wt' }) ) @@ -292,8 +302,9 @@ describe('NewWorktreeModal on a server SSH repo', () => { createElement(NewWorktreeModal, { visible: true, client: requestsOnly(listing([])), - // Listed first, it would otherwise win the server's row. - serverClients: serversOf(requestsOnly(listing([vmRepo, serverRepo]))), + // Listed before its project's copy, it would otherwise win the server's row; the last + // visited, it picks its project's copy rather than the first project listed. + serverClients: serversOf(requestsOnly(listing([otherProject, vmRepo, serverRepo]))), hostId: 'host-vm', onCreated: () => {}, openExternalUrl: () => {}, @@ -340,7 +351,7 @@ describe('NewWorktreeModal on a server SSH repo', () => { ]) }) - it("drops a connect reply that arrives after the server's target changed", async () => { + it('drops a late connect reply, keeping the newly picked target connected', async () => { const sshRepo = (id: string, connectionId: string) => ({ ...repos[0]!, id, @@ -362,7 +373,8 @@ describe('NewWorktreeModal on a server SSH repo', () => { }) } if (method === 'ssh.getState') { - return Promise.resolve(state(params!.targetId!, 'disconnected')) + const targetId = params!.targetId! + return Promise.resolve(state(targetId, targetId === 'box-b' ? 'connected' : 'disconnected')) } if (method === 'ssh.connect') { return new Promise((resolve) => { @@ -404,11 +416,15 @@ describe('NewWorktreeModal on a server SSH repo', () => { const connectable = renderer.root.find((node) => typeof node.props.onConnect === 'function') await act(async () => connectable.props.onConnect()) await pickProject('beta') - expect(gate().status).toBe('disconnected') + const canCreate = () => + renderer.root.find((node) => typeof node.props.onCreate === 'function').props.canCreate + expect(gate().status).toBe('connected') + expect(canCreate()).toBe(true) await act(async () => finishConnect!(state('box-a', 'connected'))) await flushUpdates() - expect(gate()).toMatchObject({ status: 'disconnected', requiresConnection: true }) + expect(gate()).toMatchObject({ status: 'connected', requiresConnection: false }) + expect(canCreate()).toBe(true) }) }) diff --git a/mobile/src/components/use-new-workspace-execution-target.ts b/mobile/src/components/use-new-workspace-execution-target.ts index c87c75e5b18..e248bcbffa9 100644 --- a/mobile/src/components/use-new-workspace-execution-target.ts +++ b/mobile/src/components/use-new-workspace-execution-target.ts @@ -47,7 +47,7 @@ export function useNewWorkspaceExecutionTarget(args: { const target = workspaceSshTargetKey(server, connectionId) const [sshState, setSshEntry] = useState(null) const setSshState = (record: WorkspaceSshRecord) => setSshEntry({ target, record }) - // A connect reply lands only while its target is still the one shown, so it never overwrites another's. + // Lands only if no other target's state replaced it. const settleSshState = (record: WorkspaceSshRecord) => setSshEntry((current) => (current?.target === target ? { target, record } : current)) const [connectingTarget, setConnectingTarget] = useState(null) diff --git a/mobile/src/components/use-new-workspace-repositories.ts b/mobile/src/components/use-new-workspace-repositories.ts index b1d56e8b201..3e928c547fb 100644 --- a/mobile/src/components/use-new-workspace-repositories.ts +++ b/mobile/src/components/use-new-workspace-repositories.ts @@ -1,14 +1,15 @@ import { useEffect, useMemo, useState } from 'react' import type { ExecutionHostId } from '../../../src/shared/execution-host' -import { getNewWorkspaceDialogEligibleRepos } from '../../../src/shared/new-workspace-dialog-repo' +import { + getNewWorkspaceDialogEligibleRepos, + resolveNewWorkspaceDialogActiveRepoId, + resolveNewWorkspaceDialogRepoId +} from '../../../src/shared/new-workspace-dialog-repo' import type { RpcClient } from '../transport/rpc-client' import { nativeChatRepoListRead } from '../session/mobile-session-read-operations' import { getCachedRepos, setCachedRepos } from '../cache/repo-cache' import { useLastVisitedWorktreeRepoId } from '../worktree/use-last-visited-worktree-repo' -import { - refreshMobileNewWorkspaceDialogSelectedRepo, - resolveMobileNewWorkspaceDialogRepoId -} from '../worktree/new-workspace-dialog-repo-selection' +import { refreshMobileNewWorkspaceDialogSelectedRepo } from '../worktree/new-workspace-dialog-repo-selection' import type { MobileWorkspaceRepo } from './new-worktree-modal-types' const NO_SERVER_CLIENTS: ReadonlyMap = new Map() @@ -58,12 +59,12 @@ export function useNewWorkspaceRepositories(args: { const [pickedRepo, setSelectedRepo] = useState(null) const [loading, setLoading] = useState(initialRepos == null) const lastVisitedRepo = useLastVisitedWorktreeRepoId(hostId, visible) - // The desktop's rule: a per-workspace-environment SSH repo is plumbing, never a pick. - const repos = useMemo( - () => - getNewWorkspaceDialogEligibleRepos([...desktopRepos, ...[...serverRepos.values()].flat()]), + const listedRepos = useMemo( + () => [...desktopRepos, ...[...serverRepos.values()].flat()], [desktopRepos, serverRepos] ) + // The desktop's rule: a per-workspace-environment SSH repo is plumbing, never a pick. + const repos = useMemo(() => getNewWorkspaceDialogEligibleRepos(listedRepos), [listedRepos]) // Why derived: the pick follows its repo's latest listing, and goes once its host stops listing it. const selectedRepo = useMemo( () => refreshMobileNewWorkspaceDialogSelectedRepo(repos, pickedRepo), @@ -74,15 +75,19 @@ export function useNewWorkspaceRepositories(args: { if (!visible || !lastVisitedRepo.loaded || selectedRepo || repos.length === 0) { return } - const preferredRepoId = resolveMobileNewWorkspaceDialogRepoId({ + const preferredRepoId = resolveNewWorkspaceDialogRepoId({ eligibleRepos: repos, - activeRepoId: lastVisitedRepo.repoId + activeRepoId: resolveNewWorkspaceDialogActiveRepoId( + listedRepos, + repos, + lastVisitedRepo.repoId + ) }) const preferredRepo = repos.find((repo) => repo.id === preferredRepoId) ?? null if (preferredRepo) { setSelectedRepo(preferredRepo) } - }, [lastVisitedRepo.loaded, lastVisitedRepo.repoId, repos, selectedRepo, visible]) + }, [lastVisitedRepo.loaded, lastVisitedRepo.repoId, listedRepos, repos, selectedRepo, visible]) useEffect(() => { if (!visible || !client) { diff --git a/mobile/src/worktree/new-workspace-dialog-repo-selection.test.ts b/mobile/src/worktree/new-workspace-dialog-repo-selection.test.ts index a0d1aaefbe7..e0ee09f18ea 100644 --- a/mobile/src/worktree/new-workspace-dialog-repo-selection.test.ts +++ b/mobile/src/worktree/new-workspace-dialog-repo-selection.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it } from 'vitest' -import { - refreshMobileNewWorkspaceDialogSelectedRepo, - resolveMobileNewWorkspaceDialogRepoId -} from './new-workspace-dialog-repo-selection' +import { refreshMobileNewWorkspaceDialogSelectedRepo } from './new-workspace-dialog-repo-selection' type Repo = { id: string @@ -20,56 +17,6 @@ function makeRepo(id: string, overrides: Partial = {}): Repo { } describe('mobile new workspace dialog repo selection', () => { - it('matches the dialog repo priority order', () => { - const eligibleRepos = [ - makeRepo('first'), - makeRepo('active'), - makeRepo('initial'), - makeRepo('draft') - ] - - expect( - resolveMobileNewWorkspaceDialogRepoId({ - eligibleRepos, - draftRepoId: 'draft', - initialRepoId: 'initial', - activeRepoId: 'active' - }) - ).toBe('draft') - }) - - it('uses the last visited repo as the active repo before falling back', () => { - const eligibleRepos = [makeRepo('first'), makeRepo('active')] - - expect(resolveMobileNewWorkspaceDialogRepoId({ eligibleRepos, activeRepoId: 'active' })).toBe( - 'active' - ) - expect(resolveMobileNewWorkspaceDialogRepoId({ eligibleRepos, activeRepoId: 'missing' })).toBe( - 'first' - ) - }) - - it('defaults to a repo on the focused host when no explicit repo is chosen', () => { - const eligibleRepos = [ - makeRepo('local-repo'), - makeRepo('ssh-repo', { connectionId: 'win-vm' }), - makeRepo('runtime-repo', { executionHostId: 'runtime:env-1' }) - ] - - expect( - resolveMobileNewWorkspaceDialogRepoId({ eligibleRepos, focusedHostScope: 'ssh:win-vm' }) - ).toBe('ssh-repo') - expect( - resolveMobileNewWorkspaceDialogRepoId({ - eligibleRepos, - focusedHostScope: 'runtime:env-1' - }) - ).toBe('runtime-repo') - expect( - resolveMobileNewWorkspaceDialogRepoId({ eligibleRepos, focusedHostScope: 'local' }) - ).toBe('local-repo') - }) - it('refreshes the selected repo from the fresh repo list', () => { const staleSelectedRepo = makeRepo('repo', { path: '/cached/repo' }) const freshRepo = makeRepo('repo', { path: '/fresh/repo' }) diff --git a/mobile/src/worktree/new-workspace-dialog-repo-selection.ts b/mobile/src/worktree/new-workspace-dialog-repo-selection.ts index a4c43172885..e15b7a5304b 100644 --- a/mobile/src/worktree/new-workspace-dialog-repo-selection.ts +++ b/mobile/src/worktree/new-workspace-dialog-repo-selection.ts @@ -1,49 +1,9 @@ -type ExecutionHostScope = 'all' | 'local' | `ssh:${string}` | `runtime:${string}` +import { getRepoExecutionHostId } from '../../../src/shared/execution-host' type MobileNewWorkspaceDialogRepo = { id: string - path: string connectionId?: string | null - executionHostId?: 'local' | `ssh:${string}` | `runtime:${string}` | null -} - -function getMobileRepoExecutionHostId( - repo: MobileNewWorkspaceDialogRepo -): Exclude { - if (repo.executionHostId) { - return repo.executionHostId - } - return repo.connectionId ? `ssh:${encodeURIComponent(repo.connectionId)}` : 'local' -} - -export function resolveMobileNewWorkspaceDialogRepoId({ - eligibleRepos, - draftRepoId, - initialRepoId, - activeRepoId, - focusedHostScope -}: { - eligibleRepos: readonly MobileNewWorkspaceDialogRepo[] - draftRepoId?: string | null - initialRepoId?: string | null - activeRepoId?: string | null - focusedHostScope?: ExecutionHostScope | null -}): string { - // Why: Metro cannot bundle runtime imports from the root shared tree today. - // Keep this mirror in sync with src/shared/new-workspace-dialog-repo.ts. - const focusedHostRepo = - focusedHostScope && focusedHostScope !== 'all' - ? eligibleRepos.find((repo) => getMobileRepoExecutionHostId(repo) === focusedHostScope) - : undefined - - const resolvedRepo = - (draftRepoId && eligibleRepos.find((repo) => repo.id === draftRepoId)) || - (initialRepoId && eligibleRepos.find((repo) => repo.id === initialRepoId)) || - (activeRepoId && eligibleRepos.find((repo) => repo.id === activeRepoId)) || - focusedHostRepo || - eligibleRepos[0] - - return resolvedRepo?.id ?? '' + executionHostId?: string | null } /** Whether two listed repos are the same one: a repo id names one repo per host, not overall. */ @@ -51,7 +11,7 @@ export function isSameMobileWorkspaceRepo( a: MobileNewWorkspaceDialogRepo, b: MobileNewWorkspaceDialogRepo ): boolean { - return a.id === b.id && getMobileRepoExecutionHostId(a) === getMobileRepoExecutionHostId(b) + return a.id === b.id && getRepoExecutionHostId(a) === getRepoExecutionHostId(b) } export function refreshMobileNewWorkspaceDialogSelectedRepo( diff --git a/src/renderer/src/lib/new-workspace-composer-repo.test.ts b/src/renderer/src/lib/new-workspace-composer-repo.test.ts index 4d2d1e58094..7850b664766 100644 --- a/src/renderer/src/lib/new-workspace-composer-repo.test.ts +++ b/src/renderer/src/lib/new-workspace-composer-repo.test.ts @@ -1,10 +1,6 @@ import { describe, expect, it } from 'vitest' import type { Repo } from '../../../shared/repo-types' -import { - getComposerEligibleRepos, - resolveComposerActiveRepoId, - resolveComposerRepoId -} from './new-workspace-composer-repo' +import { resolveComposerRepoId } from './new-workspace-composer-repo' function makeRepo(id: string, overrides: Partial = {}): Repo { return { @@ -25,38 +21,4 @@ describe('new-workspace-composer-repo', () => { 'local-repo' ) }) - - describe('resolveComposerActiveRepoId', () => { - const localOrca = makeRepo('local-orca', { upstream: { owner: 'stablyai', repo: 'orca' } }) - const runtimeOrca = makeRepo('runtime-orca', { - connectionId: 'runtime-ssh-orca-1', - upstream: { owner: 'stablyai', repo: 'orca' } - }) - const otherProject = makeRepo('noqa', { upstream: { owner: 'stablyai', repo: 'noqa' } }) - const repos = [otherProject, localOrca, runtimeOrca] - const eligibleRepos = getComposerEligibleRepos(repos) - - it('maps an active runtime-owned SSH repo to its local same-project sibling', () => { - expect(resolveComposerActiveRepoId(repos, eligibleRepos, 'runtime-orca')).toBe('local-orca') - }) - - it('leaves a normal active repo unchanged', () => { - expect(resolveComposerActiveRepoId(repos, eligibleRepos, 'local-orca')).toBe('local-orca') - }) - - it('keeps the runtime repo id when no same-project sibling is eligible', () => { - const onlyRuntime = [runtimeOrca] - expect( - resolveComposerActiveRepoId( - onlyRuntime, - getComposerEligibleRepos(onlyRuntime), - 'runtime-orca' - ) - ).toBe('runtime-orca') - }) - - it('passes through null/undefined active repo', () => { - expect(resolveComposerActiveRepoId(repos, eligibleRepos, null)).toBeNull() - }) - }) }) diff --git a/src/renderer/src/lib/new-workspace-composer-repo.ts b/src/renderer/src/lib/new-workspace-composer-repo.ts index 8e32e1e4bca..07395053a74 100644 --- a/src/renderer/src/lib/new-workspace-composer-repo.ts +++ b/src/renderer/src/lib/new-workspace-composer-repo.ts @@ -1,37 +1,22 @@ -import { isRuntimeOwnedSshTargetId, type ExecutionHostScope } from '../../../shared/execution-host' +import type { ExecutionHostScope } from '../../../shared/execution-host' import { getNewWorkspaceDialogEligibleRepos, + resolveNewWorkspaceDialogActiveRepoId, resolveNewWorkspaceDialogGitRepoId, resolveNewWorkspaceDialogRepoId } from '../../../shared/new-workspace-dialog-repo' -import { getProjectIdentityKey } from '../../../shared/project-host-setup-projection' import type { Repo } from '../../../shared/repo-types' export function getComposerEligibleRepos(repos: readonly Repo[]): Repo[] { return getNewWorkspaceDialogEligibleRepos(repos) } -/** - * After creating a per-workspace-env, its runtime-owned SSH repo becomes the active repo — but it's - * excluded from the composer's eligible repos (hidden plumbing). Without this, the composer can't - * match `activeRepoId`, so it falls back to the first eligible repo (a different project). Map the - * active runtime repo to its local sibling in the same project so the composer stays on that project. - */ export function resolveComposerActiveRepoId( repos: readonly Repo[], eligibleRepos: readonly Repo[], activeRepoId: string | null | undefined ): string | null { - if (!activeRepoId) { - return activeRepoId ?? null - } - const activeRepo = repos.find((repo) => repo.id === activeRepoId) - if (!activeRepo || !isRuntimeOwnedSshTargetId(activeRepo.connectionId)) { - return activeRepoId - } - const projectKey = getProjectIdentityKey(activeRepo) - const sibling = eligibleRepos.find((repo) => getProjectIdentityKey(repo) === projectKey) - return sibling?.id ?? activeRepoId + return resolveNewWorkspaceDialogActiveRepoId(repos, eligibleRepos, activeRepoId) } export function resolveComposerRepoId({ diff --git a/src/shared/new-workspace-dialog-repo.test.ts b/src/shared/new-workspace-dialog-repo.test.ts index e14116f915e..57529c338a3 100644 --- a/src/shared/new-workspace-dialog-repo.test.ts +++ b/src/shared/new-workspace-dialog-repo.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import type { Repo } from './repo-types' import { getNewWorkspaceDialogEligibleRepos, + resolveNewWorkspaceDialogActiveRepoId, resolveNewWorkspaceDialogGitRepoId, resolveNewWorkspaceDialogRepoId } from './new-workspace-dialog-repo' @@ -106,4 +107,42 @@ describe('new workspace dialog repo selection', () => { 'local-repo' ) }) + + describe('resolveNewWorkspaceDialogActiveRepoId', () => { + const localOrca = makeRepo('local-orca', { upstream: { owner: 'stablyai', repo: 'orca' } }) + const runtimeOrca = makeRepo('runtime-orca', { + connectionId: 'runtime-ssh-orca-1', + upstream: { owner: 'stablyai', repo: 'orca' } + }) + const otherProject = makeRepo('noqa', { upstream: { owner: 'stablyai', repo: 'noqa' } }) + const repos = [otherProject, localOrca, runtimeOrca] + const eligibleRepos = getNewWorkspaceDialogEligibleRepos(repos) + + it('maps an active runtime-owned SSH repo to its local same-project sibling', () => { + expect(resolveNewWorkspaceDialogActiveRepoId(repos, eligibleRepos, 'runtime-orca')).toBe( + 'local-orca' + ) + }) + + it('leaves a normal active repo unchanged', () => { + expect(resolveNewWorkspaceDialogActiveRepoId(repos, eligibleRepos, 'local-orca')).toBe( + 'local-orca' + ) + }) + + it('keeps the runtime repo id when no same-project sibling is eligible', () => { + const onlyRuntime = [runtimeOrca] + expect( + resolveNewWorkspaceDialogActiveRepoId( + onlyRuntime, + getNewWorkspaceDialogEligibleRepos(onlyRuntime), + 'runtime-orca' + ) + ).toBe('runtime-orca') + }) + + it('passes through null/undefined active repo', () => { + expect(resolveNewWorkspaceDialogActiveRepoId(repos, eligibleRepos, null)).toBeNull() + }) + }) }) diff --git a/src/shared/new-workspace-dialog-repo.ts b/src/shared/new-workspace-dialog-repo.ts index ae037513ca8..21ab9a71100 100644 --- a/src/shared/new-workspace-dialog-repo.ts +++ b/src/shared/new-workspace-dialog-repo.ts @@ -4,6 +4,7 @@ import { isRuntimeOwnedSshTargetId, type ExecutionHostScope } from './execution-host' +import { getProjectIdentityKey } from './project-host-setup-projection' import { isGitRepoKind } from './repo-kind' import type { Repo } from './repo-types' @@ -22,6 +23,34 @@ export function getNewWorkspaceDialogEligibleRepos Boolean(repo.path) && !isRuntimeOwnedSshTargetId(repo.connectionId)) } +type ActiveRepoCandidate = Pick< + Repo, + 'id' | 'connectionId' | 'upstream' | 'repoIcon' | 'gitRemoteIdentity' +> + +/** + * After creating a per-workspace-env, its runtime-owned SSH repo becomes the active repo — but it's + * excluded from the eligible repos (hidden plumbing). Without this, the dialog can't match + * `activeRepoId`, so it falls back to the first eligible repo (a different project). Map the active + * runtime repo to its sibling in the same project so the dialog stays on that project. + */ +export function resolveNewWorkspaceDialogActiveRepoId( + repos: readonly ActiveRepoCandidate[], + eligibleRepos: readonly ActiveRepoCandidate[], + activeRepoId: string | null | undefined +): string | null { + if (!activeRepoId) { + return activeRepoId ?? null + } + const activeRepo = repos.find((repo) => repo.id === activeRepoId) + if (!activeRepo || !isRuntimeOwnedSshTargetId(activeRepo.connectionId)) { + return activeRepoId + } + const projectKey = getProjectIdentityKey(activeRepo) + const sibling = eligibleRepos.find((repo) => getProjectIdentityKey(repo) === projectKey) + return sibling?.id ?? activeRepoId +} + export function resolveNewWorkspaceDialogRepoId({ eligibleRepos, draftRepoId, @@ -35,8 +64,7 @@ export function resolveNewWorkspaceDialogRepoId({ activeRepoId?: string | null focusedHostScope?: ExecutionHostScope | null }): string { - // Why: every new-workspace dialog should seed the repo the same way. Mobile - // mirrors this locally because Metro cannot bundle root shared runtime modules. + // Why: every new-workspace dialog, desktop and mobile, seeds the repo the same way. const focusedHostRepo = focusedHostScope && focusedHostScope !== ALL_EXECUTION_HOSTS_SCOPE ? eligibleRepos.find((repo) => getRepoExecutionHostId(repo) === focusedHostScope)