diff --git a/src/cli/handlers/worktree-gitlab-link-context.ts b/src/cli/handlers/worktree-gitlab-link-context.ts index fe1bd6b7f56..ba78726852e 100644 --- a/src/cli/handlers/worktree-gitlab-link-context.ts +++ b/src/cli/handlers/worktree-gitlab-link-context.ts @@ -3,11 +3,12 @@ import type { Repo } from '../../shared/repo-types' import type { RuntimeWorktreeRecord } from '../../shared/runtime-types' import { parseGitLabIssueOrMRLink, type ProjectSlug } from '../../shared/new-workspace/gitlab-links' import { RuntimeClientError, type RuntimeClient } from '../runtime-client' +import type { CliRepoTarget } from '../worktree-project-target' export async function assertGitLabLinkFlagProjectsMatch( flags: Map, client: RuntimeClient, - target: { repo: string } | { worktree: string } + target: CliRepoTarget | { worktree: string } ): Promise { const projects = ['gitlab-issue', 'gitlab-mr'].flatMap((name) => { const value = flags.get(name) @@ -22,7 +23,7 @@ export async function assertGitLabLinkFlagProjectsMatch( } let project: ProjectSlug | null = null - let repo: string + let repoTarget: CliRepoTarget if ('worktree' in target) { const result = await client.call<{ worktree: RuntimeWorktreeRecord }>('worktree.show', target) const context = result.result.worktree.linkedTaskSourceContext @@ -30,12 +31,12 @@ export async function assertGitLabLinkFlagProjectsMatch( if (context?.provider === 'gitlab' && identity?.provider === 'gitlab' && identity.webUrl) { project = parseRemoteProjectRefCandidate(identity.webUrl) } - repo = `id:${result.result.worktree.repoId}` + repoTarget = { repo: `id:${result.result.worktree.repoId}` } } else { - repo = target.repo + repoTarget = target } if (!project) { - const result = await client.call<{ repo: Repo }>('repo.show', { repo }) + const result = await client.call<{ repo: Repo }>('repo.show', repoTarget) const remoteUrl = result.result.repo.gitRemoteIdentity?.remoteUrl project = remoteUrl ? parseRemoteProjectRefCandidate(remoteUrl) : null } diff --git a/src/cli/handlers/worktree.ts b/src/cli/handlers/worktree.ts index 2836528193e..75bec4b5fb0 100644 --- a/src/cli/handlers/worktree.ts +++ b/src/cli/handlers/worktree.ts @@ -34,7 +34,7 @@ import { projectWorktreePsTerminalVerdict } from '../worktree-ps-terminal-verdic import { assertWorkspaceTargetFlagsCompatible, hasWorkspaceProjectTarget, - getCreateRepoSelector + getCreateRepoTarget } from '../worktree-project-target' import { assertWorktreeParentFlagsCompatible, @@ -176,8 +176,8 @@ export const WORKTREE_HANDLERS: Record = { const linearIssueLink = getOptionalLinearIssueLinkFlag(flags, 'linear-issue') const activate = flags.get('activate') === true || flags.get('run-hooks') === true const name = getRequiredStringFlag(flags, 'name') - const repo = await getCreateRepoSelector(flags, cwdParentWorktree, client) - await assertGitLabLinkFlagProjectsMatch(flags, client, { repo }) + const repoTarget = await getCreateRepoTarget(flags, cwdParentWorktree, client) + await assertGitLabLinkFlagProjectsMatch(flags, client, repoTarget) const linkedItems = referenceInputs ? await resolveReferenceInputs(context, referenceInputs) : undefined @@ -185,7 +185,8 @@ export const WORKTREE_HANDLERS: Record = { await assertReferenceWritesSupported(client) } const create = { - repo, + // Older hosts drop `repoHostId` and keep refusing an id on several hosts, as before. + ...repoTarget, name, displayName: name, displayNameKind: 'user', diff --git a/src/cli/index-worktree-create-repo-host.test.ts b/src/cli/index-worktree-create-repo-host.test.ts new file mode 100644 index 00000000000..0fbcda05b9b --- /dev/null +++ b/src/cli/index-worktree-create-repo-host.test.ts @@ -0,0 +1,253 @@ +import { describe, expect, it, vi } from 'vitest' + +const { + callMock, + runtimeClientConstructorMock, + serveOrcaAppMock, + getDefaultUserDataPathMock, + addEnvironmentFromPairingCodeMock, + listEnvironmentsMock, + spawnMock +} = vi.hoisted(() => ({ + callMock: vi.fn(), + runtimeClientConstructorMock: vi.fn(), + serveOrcaAppMock: vi.fn(), + getDefaultUserDataPathMock: vi.fn(() => '/tmp/orca-user-data'), + addEnvironmentFromPairingCodeMock: vi.fn(), + listEnvironmentsMock: vi.fn(), + spawnMock: vi.fn() +})) + +vi.mock('./runtime-client', async () => { + const { createRuntimeClientModuleMock } = await import('./index-test-harness.js') + return createRuntimeClientModuleMock({ + callMock, + runtimeClientConstructorMock, + serveOrcaAppMock, + getDefaultUserDataPathMock + }) +}) + +vi.mock('./runtime/environments', () => ({ + addEnvironmentFromPairingCode: addEnvironmentFromPairingCodeMock, + listEnvironments: listEnvironmentsMock, + removeEnvironment: vi.fn(), + resolveEnvironment: vi.fn() +})) + +vi.mock('child_process', async () => { + const { createChildProcessModuleMock } = await import('./index-test-harness.js') + return createChildProcessModuleMock(spawnMock) +}) + +import { main } from './index' +import { buildWorktree, okFixture, queueFixtures, worktreeListFixture } from './test-fixtures' +import { useWorktreeAwarenessEnvironment } from './index-test-harness' + +function buildSetup(overrides: { id: string; hostId: string; repoId: string; path: string }) { + return { + projectId: 'github:stablyai/orca', + displayName: 'Orca', + setupState: 'ready', + setupMethod: 'legacy-repo', + createdAt: 1, + updatedAt: 1, + ...overrides + } +} + +// Why one repo id on two setups: a project copied between hosts keeps its id (Codex review 6 #6). +const TWIN_SETUPS = [ + buildSetup({ id: 'setup-1', hostId: 'local', repoId: 'repo-1', path: '/tmp/orca' }), + buildSetup({ id: 'setup-1', hostId: 'runtime:gpu', repoId: 'repo-1', path: '/srv/orca' }) +] + +describe('orca worktree create carries the selected host', () => { + useWorktreeAwarenessEnvironment({ + callMock, + serveOrcaAppMock, + getDefaultUserDataPathMock, + addEnvironmentFromPairingCodeMock, + listEnvironmentsMock, + spawnMock + }) + + function queueCreate(): void { + queueFixtures( + callMock, + okFixture('req_project_setups', { setups: TWIN_SETUPS }), + okFixture('req_create', { + worktree: buildWorktree('/tmp/orca/feature', 'feature', 'abc', 'repo-1'), + lineage: null, + warnings: [] + }) + ) + } + + it('sends the host --project --host chose, not just the repo id', async () => { + queueCreate() + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + [ + 'worktree', + 'create', + '--project', + 'github:stablyai/orca', + '--host', + 'local', + '--name', + 'feature', + '--no-parent', + '--json' + ], + '/tmp/repo' + ) + + expect(callMock).toHaveBeenNthCalledWith( + 2, + 'worktree.create', + expect.objectContaining({ repo: 'id:repo-1', repoHostId: 'local' }) + ) + }) + + it('applies --host to a --project-host-setup id that repeats across hosts', async () => { + queueCreate() + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + [ + 'worktree', + 'create', + '--project-host-setup', + 'setup-1', + '--host', + 'local', + '--name', + 'feature', + '--no-parent', + '--json' + ], + '/tmp/repo' + ) + + expect(callMock).toHaveBeenNthCalledWith( + 2, + 'worktree.create', + expect.objectContaining({ repo: 'id:repo-1', repoHostId: 'local' }) + ) + }) + + it('asks for --host when a repeated --project-host-setup id names two hosts', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const priorExitCode = process.exitCode + queueCreate() + + await main( + [ + 'worktree', + 'create', + '--project-host-setup', + 'setup-1', + '--name', + 'feature', + '--no-parent', + '--json' + ], + '/tmp/repo' + ) + + expect(callMock).toHaveBeenCalledTimes(1) + const printed = [...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n') + expect(printed).toContain('Ambiguous --project-host-setup setup-1') + expect(process.exitCode).toBe(1) + process.exitCode = priorExitCode + }) + + it('checks a GitLab link against the repo on the selected host', async () => { + queueFixtures( + callMock, + okFixture('req_project_setups', { setups: TWIN_SETUPS }), + okFixture('req_repo', { + repo: { id: 'repo-1', gitRemoteIdentity: { remoteUrl: 'https://gitlab.com/acme/app.git' } } + }), + okFixture('req_create', { + worktree: buildWorktree('/tmp/orca/feature', 'feature', 'abc', 'repo-1'), + lineage: null, + warnings: [] + }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main( + [ + 'worktree', + 'create', + '--project', + 'github:stablyai/orca', + '--host', + 'local', + '--gitlab-mr', + 'https://gitlab.com/acme/app/-/merge_requests/5', + '--name', + 'feature', + '--no-parent', + '--json' + ], + '/tmp/repo' + ) + + expect(callMock).toHaveBeenNthCalledWith(2, 'repo.show', { + repo: 'id:repo-1', + repoHostId: 'local' + }) + expect(callMock).toHaveBeenNthCalledWith( + 3, + 'worktree.create', + expect.objectContaining({ repo: 'id:repo-1', repoHostId: 'local' }) + ) + }) + + it('names the host when a --project-host-setup id is missing there', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const priorExitCode = process.exitCode + queueFixtures(callMock, okFixture('req_project_setups', { setups: [TWIN_SETUPS[1]] })) + + await main( + [ + 'worktree', + 'create', + '--project-host-setup', + 'setup-1', + '--host', + 'local', + '--name', + 'feature', + '--no-parent', + '--json' + ], + '/tmp/repo' + ) + + const printed = [...logSpy.mock.calls, ...errSpy.mock.calls].flat().join('\n') + expect(printed).toContain('not ready or was not found: setup-1 on local') + process.exitCode = priorExitCode + }) + + it('keeps --repo selector-only', async () => { + queueFixtures( + callMock, + worktreeListFixture([buildWorktree('/tmp/repo', 'main', 'abc', 'repo-1')]), + okFixture('req_create', { + worktree: buildWorktree('/tmp/repo/feature', 'feature', 'abc', 'repo-1') + }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main(['worktree', 'create', '--repo', 'id:repo-1', '--name', 'f', '--json'], '/tmp/repo') + + expect(callMock.mock.calls[1]?.[1]).not.toHaveProperty('repoHostId') + }) +}) diff --git a/src/cli/index-worktree-create-target.test.ts b/src/cli/index-worktree-create-target.test.ts index 8679b2b9257..9752a29defd 100644 --- a/src/cli/index-worktree-create-target.test.ts +++ b/src/cli/index-worktree-create-target.test.ts @@ -169,6 +169,7 @@ describe('orca cli worktree awareness', () => { expect(callMock).toHaveBeenNthCalledWith(1, 'projectHostSetup.list') expect(callMock).toHaveBeenNthCalledWith(2, 'worktree.create', { repo: 'id:repo-gpu', + repoHostId: 'runtime:gpu', name: 'feature', displayName: 'feature', displayNameKind: 'user', diff --git a/src/cli/worktree-project-target.ts b/src/cli/worktree-project-target.ts index 51de5d13e28..66413289be5 100644 --- a/src/cli/worktree-project-target.ts +++ b/src/cli/worktree-project-target.ts @@ -7,6 +7,8 @@ import { RuntimeClientError } from './runtime-client' export type ProjectCreateTarget = { repoSelector: string + /** The selected setup's host; a repo id alone is ambiguous once it is set up on two hosts. */ + repoHostId: string setup: ProjectHostSetup } @@ -44,19 +46,12 @@ export function assertWorkspaceTargetFlagsCompatible(flags: Map, - client: RuntimeClient -): Promise { - return (await resolveProjectCreateTarget(flags, client))?.repoSelector -} - // Why: the routed runtime can hold both an exact `runtime:` row and a `local` row for the // same project; the exact stamp is the one the caller named, so it wins the selection. function selectSetupOnHost( candidates: readonly ProjectHostSetup[], host: ParsedExecutionHost | undefined, - projectSelector: string | undefined + selectorLabel: string ): ProjectHostSetup | undefined { if (host) { return ( @@ -67,12 +62,12 @@ function selectSetupOnHost( // Why: one host is not a choice, so demanding --host for it was ceremony. Two are a choice, and // picking one would run the command on a machine the caller never named. const hostIds = new Set(candidates.map(setupHostId)) - if (hostIds.size <= 1 || projectSelector === undefined) { + if (hostIds.size <= 1) { return candidates[0] } throw new RuntimeClientError( 'invalid_argument', - `Ambiguous --project ${projectSelector}: it is set up on ${hostIds.size} hosts. Pass --host to choose one.`, + `Ambiguous ${selectorLabel}: it is set up on ${hostIds.size} hosts. Pass --host to choose one.`, { knownProjectHostSetups: candidates.map(summarizeSetup), nextSteps: [ @@ -125,14 +120,19 @@ export async function resolveProjectCreateTarget( : (await findProjectSetups(client, result.result.setups, projectId)).filter( (candidate) => candidate.setupState === 'ready' ) + // Why --host applies to both: setup ids are per host, so the same id can name two setups. const setup = projectHostSetupId - ? ready.find((candidate) => candidate.id === projectHostSetupId) - : selectSetupOnHost(candidates, host, projectId) + ? selectSetupOnHost( + ready.filter((candidate) => candidate.id === projectHostSetupId), + host, + `--project-host-setup ${projectHostSetupId}` + ) + : selectSetupOnHost(candidates, host, `--project ${projectId}`) if (!setup) { if (projectHostSetupId) { throw new RuntimeClientError( 'invalid_argument', - `Project host setup is not ready or was not found: ${projectHostSetupId}` + `Project host setup is not ready or was not found: ${projectHostSetupId}${host ? ` on ${host.id}` : ''}` ) } // Why: blaming the host when no project matched sent callers to debug the wrong flag. @@ -162,6 +162,7 @@ export async function resolveProjectCreateTarget( } return { repoSelector: `id:${setup.repoId}`, + repoHostId: setupHostId(setup), setup } } @@ -175,22 +176,28 @@ function getRepoSelectorFromWorktreeId(worktreeId: string | undefined): string | return `id:${worktreeId.slice(0, separatorIndex)}` } -export async function getCreateRepoSelector( +/** + * The repo a create names, as RPC params: spread it whole into every call that looks the repo up, + * so the host a project target picked is never dropped. `--repo` stays selector-only. + */ +export type CliRepoTarget = { repo: string; repoHostId?: string } + +export async function getCreateRepoTarget( flags: Map, cwdParentWorktree: { worktreeId: string } | undefined, client: RuntimeClient -): Promise { - const projectRepoSelector = await resolveProjectCreateRepoSelector(flags, client) - if (projectRepoSelector) { - return projectRepoSelector +): Promise { + const projectTarget = await resolveProjectCreateTarget(flags, client) + if (projectTarget) { + return { repo: projectTarget.repoSelector, repoHostId: projectTarget.repoHostId } } const explicitRepo = getPresentStringFlag(flags, 'repo') if (explicitRepo) { - return explicitRepo + return { repo: explicitRepo } } const inferredRepo = getRepoSelectorFromWorktreeId(cwdParentWorktree?.worktreeId) if (inferredRepo) { - return inferredRepo + return { repo: inferredRepo } } throw new RuntimeClientError( 'invalid_argument', diff --git a/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts b/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts index fdc80d51e10..663305da3b1 100644 --- a/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts +++ b/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts @@ -16,6 +16,7 @@ import { import { RuntimeRepositorySettingsController } from '../../runtime/runtime-repository-settings-controller' import { registerFolderWorkspaceHandlers } from './folder-workspace-handlers' import { registerRepoCatalogHandlers } from './repo-catalog-handlers' +import { runtimeRepoSelectorText } from '../../runtime/runtime-repo-selector' const { handlers } = vi.hoisted(() => ({ handlers: new Map unknown>() @@ -133,7 +134,7 @@ describe('RuntimeRepositorySettingsController under a WSL move', () => { const notifyReposChanged = vi.fn() const controller = new RuntimeRepositorySettingsController({ getStore: () => store, - resolveRepo: async (id) => repo(id), + resolveRepo: async (selector) => repo(runtimeRepoSelectorText(selector)), forgetTerminalTopology: vi.fn(), invalidateResolvedWorktrees: vi.fn(), invalidateWorktreeScan: vi.fn(), diff --git a/src/main/runtime/construction/runtime-construction-services.ts b/src/main/runtime/construction/runtime-construction-services.ts index c7fe2510fb8..e6c73137729 100644 --- a/src/main/runtime/construction/runtime-construction-services.ts +++ b/src/main/runtime/construction/runtime-construction-services.ts @@ -41,6 +41,7 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-st import type { AgentStatusIpcPayload } from '../../../shared/agent-status-types' import type * as PtyControllerContract from '../runtime-pty-controller-contract' import type { RuntimeClientEventBus } from '../runtime-client-event-bus' +import type { RuntimeRepoSelector } from '../runtime-repo-selector' export abstract class RuntimeConstructionServices { abstract getOrchestrationDb(): OrchestrationDb @@ -179,7 +180,7 @@ export abstract class RuntimeConstructionServices { hostIds: ExecutionHostId[] omittedHostIds: ExecutionHostId[] } - protected abstract resolveRepoSelector(selector: string): Promise + protected abstract resolveRepoSelector(selector: RuntimeRepoSelector): Promise protected abstract selectReposBySelector(selector: string): Repo[] protected abstract listRepoWorktreesForListing( repo: Repo, diff --git a/src/main/runtime/construction/runtime-file-command-state.ts b/src/main/runtime/construction/runtime-file-command-state.ts index 9dd7b754d7b..6cc4666f013 100644 --- a/src/main/runtime/construction/runtime-file-command-state.ts +++ b/src/main/runtime/construction/runtime-file-command-state.ts @@ -32,6 +32,7 @@ import type { ResolvedWorktree } from '../runtime-worktree-path-identity' import type { Repo } from '../../../shared/repo-types' import { isWslMoveWorktreeBlocked } from '../../persistence/moving-wsl-projects/wsl-move-background-backoff' import { worktreeRowTarget } from '../../persistence/host-qualified-worktree-meta' +import type { RuntimeRepoSelector } from '../runtime-repo-selector' export abstract class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreservedBranchCleanup { abstract resolveRuntimeFileTarget(worktreeSelector: string): Promise<{ @@ -93,7 +94,7 @@ export abstract class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreserv protected abstract resolveWorktreeSelector(selector: string): Promise - protected abstract resolveRepoSelector(selector: string): Promise + protected abstract resolveRepoSelector(selector: RuntimeRepoSelector): Promise protected readonly fileCommands = new RuntimeFileCommands({ getRuntimeId: () => this.runtimeId, diff --git a/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts index 2e77a987d26..5194c341bfb 100644 --- a/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts +++ b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts @@ -21,7 +21,7 @@ import type { AgentLaunchWorkspaceFactory } from '../../../agent-launch/agent-la import { combineLaunchWarnings } from '../../../agent-launch/agent-launch-warnings' import type { RpcContext } from '../core' import { resolveRpcWorkspaceCreatorProvenance } from '../workspace-creator-context' -import { buildManagedWorktreeCreateArgs } from './worktree-create-args' +import { buildManagedWorktreeCreateArgs, worktreeCreateRepoSelector } from './worktree-create-args' import { toAgentLaunchPreferences } from '../../../../shared/agent-launch-preferences' import type { AgentLaunchParams } from '../../../../shared/rpc-contract/agent-launch-params' import { agentLaunchFolderWorkspaceCreator } from './agent-launch-folder-workspace-creation' @@ -61,7 +61,7 @@ export function agentLaunchWorkspaceFactory( // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: already validated by `AgentLaunch`; the executor only removed the reserved agent fields, so the rest of the payload is the parsed shape. const params = create as WorktreeCreateParams const { runtime } = context - const repo = await runtime.showRepo(params.repo) + const repo = await runtime.showRepo(worktreeCreateRepoSelector(params)) const automationProvenance = resolveAutomationWorkspaceProvenance({ authority: runtime, repoSelector: params.repo, diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts index a7a1217f43c..1f1fd09f680 100644 --- a/src/main/runtime/rpc/methods/agent-launch.ts +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -166,8 +166,9 @@ function runLegacyAgentLaunch( presentation: params.presentation }) if (params.target.kind === 'create-worktree' && params.target.create.clientMutationId) { + const { repo, repoHostId } = params.target.create return context.runtime.dedupeWorktreeCreate( - params.target.create.repo, + repoHostId ? { repo, repoHostId } : repo, `agent.launch:${params.target.create.clientMutationId}`, execute ) diff --git a/src/main/runtime/rpc/methods/repo.ts b/src/main/runtime/rpc/methods/repo.ts index 67c942ef6da..4937527a068 100644 --- a/src/main/runtime/rpc/methods/repo.ts +++ b/src/main/runtime/rpc/methods/repo.ts @@ -5,7 +5,8 @@ import { includesQualifiedSearchRefs, projectRepoSearchRefsForClient } from './repo-search-ref-projection' -import { RepoSelector } from '../../../../shared/rpc-contract/github-repo-target-params' +import { RepoSelector, RepoShow } from '../../../../shared/rpc-contract/github-repo-target-params' +import { hostQualifiedRepoSelector } from '../../runtime-repo-selector' import { projectRepoResultVisibilityForClient, projectRepoVisibilityForClient @@ -172,9 +173,12 @@ export const REPO_METHODS = [ defineMethod({ name: 'repo.show', permission: 'workspace', - params: RepoSelector, + params: RepoShow, handler: async (params, context) => ({ - repo: projectRepoVisibilityForClient(await context.runtime.showRepo(params.repo), context) + repo: projectRepoVisibilityForClient( + await context.runtime.showRepo(hostQualifiedRepoSelector(params.repo, params.repoHostId)), + context + ) }) }), defineMethod({ diff --git a/src/main/runtime/rpc/methods/worktree-create-args.ts b/src/main/runtime/rpc/methods/worktree-create-args.ts index 2ee45da5f17..dd29fbc834d 100644 --- a/src/main/runtime/rpc/methods/worktree-create-args.ts +++ b/src/main/runtime/rpc/methods/worktree-create-args.ts @@ -2,6 +2,7 @@ import { terminalImageViewerSupported } from '../../terminal-image-viewer-env' import type { z } from 'zod' import { resolveRuntimeNavigationTarget } from '../../../../shared/runtime-navigation' import type { OrcaRuntimeService } from '../../orca-runtime' +import { hostQualifiedRepoSelector, type RuntimeRepoSelector } from '../../runtime-repo-selector' import type { WorktreeCreate } from '../../../../shared/rpc-contract/worktree-create-params' type WorktreeCreateParams = z.infer @@ -11,6 +12,13 @@ type CreateProvenance = Pick< 'automationProvenance' | 'cliProvenance' | 'creatorProvenance' > +/** The create's repo, narrowed to `repoHostId` when the caller named one. */ +export function worktreeCreateRepoSelector( + params: Pick +): RuntimeRepoSelector { + return hostQualifiedRepoSelector(params.repo, params.repoHostId) +} + /** Wire params → runtime create args. Kept out of the method table so the mapping can grow with * the schema without the table becoming unreadable. */ export function buildManagedWorktreeCreateArgs( @@ -20,7 +28,7 @@ export function buildManagedWorktreeCreateArgs( ): ManagedWorktreeCreateArgs { const viewerSupported = origin && terminalImageViewerSupported(origin) return { - repoSelector: params.repo, + repoSelector: worktreeCreateRepoSelector(params), // A phone's retry key is the identity of its creation. creationId: params.creationId ?? params.clientMutationId, ...(viewerSupported !== undefined ? { terminalImageViewerSupported: viewerSupported } : {}), diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index 153824e48da..7f48b08048b 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -11,7 +11,7 @@ import { displayNameUpdatePinsLabel } from '../../../../shared/worktree/display- import { getWorkspaceStatusWarning } from '../../../../shared/workspace-status-warning' import { createWorktreeWithStartupAgent } from '../../../agent-launch/startup-agent-worktree-create' import { defineMethod } from '../core' -import { buildManagedWorktreeCreateArgs } from './worktree-create-args' +import { buildManagedWorktreeCreateArgs, worktreeCreateRepoSelector } from './worktree-create-args' import { resolvePairedCallerHostId } from './paired-caller-host-id' import { resolveRuntimeNavigationTarget } from '../../../../shared/runtime-navigation' import { resolveRpcWorkspaceCreatorProvenance } from '../workspace-creator-context' @@ -96,9 +96,9 @@ export const WORKTREE_METHODS = [ // Why: a mobile create interrupted by a connection migration is retried with // the same clientMutationId; dedupe so the host returns the in-flight/created // worktree instead of spawning a duplicate. No key (desktop/CLI) runs plainly. - context.runtime.dedupeWorktreeCreate(params.repo, params.clientMutationId, async () => { + context.runtime.dedupeWorktreeCreate(params, params.clientMutationId, async () => { const { runtime } = context - const repo = await runtime.showRepo(params.repo) + const repo = await runtime.showRepo(worktreeCreateRepoSelector(params)) const automationProvenance = resolveAutomationWorkspaceProvenance({ authority: runtime, repoSelector: params.repo, diff --git a/src/main/runtime/runtime-managed-worktree-create-types.ts b/src/main/runtime/runtime-managed-worktree-create-types.ts index 53ac465c4df..77eb0e3b6f8 100644 --- a/src/main/runtime/runtime-managed-worktree-create-types.ts +++ b/src/main/runtime/runtime-managed-worktree-create-types.ts @@ -14,9 +14,10 @@ import type { WorktreeStartupLaunch } from '../../shared/worktree/launch-types' import type { TaskSourceContext } from '../../shared/task-source-context' import type { WorktreeStartupDraftPaste } from './runtime-worktree-agent-startup' import type { RuntimeNavigationTarget } from '../../shared/runtime-navigation' +import type { RuntimeRepoSelector } from './runtime-repo-selector' export type RuntimeManagedWorktreeCreateArgs = { - repoSelector: string + repoSelector: RuntimeRepoSelector name: string nameWasGenerated?: boolean navigation?: RuntimeNavigationTarget diff --git a/src/main/runtime/runtime-repo-selector.ts b/src/main/runtime/runtime-repo-selector.ts new file mode 100644 index 00000000000..840d665a428 --- /dev/null +++ b/src/main/runtime/runtime-repo-selector.ts @@ -0,0 +1,27 @@ +import type { ExecutionHostId } from '../../shared/execution-host' + +/** + * A repo selector, optionally narrowed to one host. One repo id can be set up on several hosts, so + * the host travels with the selector: every lookup that re-resolves it keeps the caller's choice. + */ +export type RuntimeRepoSelector = + | string + | { readonly selector: string; readonly hostId: ExecutionHostId } + +/** The wire pair (`repo`, optional `repoHostId`) as one selector; absent host keeps the old string. */ +export function hostQualifiedRepoSelector( + selector: string, + hostId: ExecutionHostId | undefined +): RuntimeRepoSelector { + return hostId ? { selector, hostId } : selector +} + +export function runtimeRepoSelectorText(selector: RuntimeRepoSelector): string { + return typeof selector === 'string' ? selector : selector.selector +} + +export function runtimeRepoSelectorHostId( + selector: RuntimeRepoSelector +): ExecutionHostId | undefined { + return typeof selector === 'string' ? undefined : selector.hostId +} diff --git a/src/main/runtime/runtime-repository-settings-controller.ts b/src/main/runtime/runtime-repository-settings-controller.ts index 559200ffba1..29593d7fb51 100644 --- a/src/main/runtime/runtime-repository-settings-controller.ts +++ b/src/main/runtime/runtime-repository-settings-controller.ts @@ -1,6 +1,7 @@ import { isFolderRepo } from '../../shared/repo-kind' import { commitProvenRepoKindChange } from '../folder-repo-git-conversion' import type { Repo } from '../../shared/repo-types' +import type { RuntimeRepoSelector } from './runtime-repo-selector' import type { GhAccountBinding } from '../../shared/github/account-binding' import { invalidateAuthorizedRootsCache } from '../ipc/registered-worktree-roots-cache' import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation' @@ -12,7 +13,7 @@ const PROJECT_TERMINAL_STOP_BUDGET_MS = 10_000 type RuntimeRepositorySettingsDependencies = { getStore: () => RuntimeStore | null - resolveRepo: (selector: string) => Promise + resolveRepo: (selector: RuntimeRepoSelector) => Promise forgetTerminalTopology: (repoId: string) => void /** Optional so test doubles and older wiring keep removing without it. */ stopProjectTerminals?: ( @@ -62,7 +63,7 @@ function omitUndefined>(value: T): Partial export class RuntimeRepositorySettingsController { constructor(private readonly deps: RuntimeRepositorySettingsDependencies) {} - async show(repoSelector: string): Promise { + async show(repoSelector: RuntimeRepoSelector): Promise { return await this.deps.resolveRepo(repoSelector) } diff --git a/src/main/runtime/runtime-worktree-create-repo-host.test.ts b/src/main/runtime/runtime-worktree-create-repo-host.test.ts new file mode 100644 index 00000000000..d8e3e584432 --- /dev/null +++ b/src/main/runtime/runtime-worktree-create-repo-host.test.ts @@ -0,0 +1,121 @@ +import './rpc/unused-default-rpc-methods.test-fixture' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/repo-types' +import { RpcDispatcher } from './rpc/dispatcher' +import type { RpcContext, RpcRequest } from './rpc/core' +import { REPO_METHODS } from './rpc/methods/repo' +import { WORKTREE_METHODS } from './rpc/methods/worktree' +import { agentLaunchWorkspaceFactory } from './rpc/methods/agent-launch-worktree-creation' +import { OrcaRuntimeService } from './orca-runtime' + +// Why twins: a project copied between hosts keeps its repo id (Codex review 6 #6). +const REPOS: Repo[] = [ + { + id: 'dup', + path: '/laptop/dup', + displayName: 'Dup', + badgeColor: '#000', + addedAt: 1, + kind: 'git', + executionHostId: 'local' + }, + { + id: 'dup', + path: '/remote/dup', + displayName: 'Dup', + badgeColor: '#000', + addedAt: 2, + kind: 'git', + executionHostId: 'ssh:gpu' + } +] + +function createRuntime(): OrcaRuntimeService { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these paths stop right after repo resolution, which reads only these store methods. + return new OrcaRuntimeService({ + getRepos: () => REPOS, + getRepo: (id: string) => REPOS.find((repo) => repo.id === id) ?? null, + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => null, + setWorktreeMeta: vi.fn(), + getGitHubCache: () => null, + getSettings: () => ({}) + } as never) +} + +/** Stops every create at the step that receives the resolved repo, and records that repo. */ +function stopAtResolvedProject() { + return vi + .spyOn( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: spies on the private step that receives the resolved repo; it exists on the prototype. + OrcaRuntimeService.prototype as unknown as { + createInResolvedProject: (args: unknown, repo: Repo) => Promise + }, + 'createInResolvedProject' + ) + .mockRejectedValue(new Error('stop after resolution')) +} + +function request(method: string, params: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +describe('a repo id set up on two hosts, named with repoHostId', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('repo.show resolves the row on the named host and still refuses a bare id', async () => { + const dispatcher = new RpcDispatcher({ runtime: createRuntime(), methods: REPO_METHODS }) + + const named = await dispatcher.dispatch( + request('repo.show', { repo: 'id:dup', repoHostId: 'ssh:gpu' }) + ) + const bare = await dispatcher.dispatch(request('repo.show', { repo: 'id:dup' })) + + expect(named).toMatchObject({ ok: true, result: { repo: { path: '/remote/dup' } } }) + expect(bare).toMatchObject({ ok: false, error: { code: 'selector_ambiguous' } }) + }) + + it.each([ + ['plain', {}], + ['with a startup agent', { startupAgent: 'claude' }], + ['with a linked startup draft', { startupDraft: 'Fix the bug' }] + ])('worktree.create keeps the host through every lookup (%s)', async (_label, extra) => { + const reached = stopAtResolvedProject() + const dispatcher = new RpcDispatcher({ runtime: createRuntime(), methods: WORKTREE_METHODS }) + + await dispatcher.dispatch( + request('worktree.create', { repo: 'id:dup', repoHostId: 'local', name: 'x', ...extra }) + ) + + expect(reached).toHaveBeenCalledTimes(1) + expect(reached.mock.calls[0]?.[1]).toMatchObject({ path: '/laptop/dup' }) + }) + + it('agent.launch create-worktree keeps the host', async () => { + const reached = stopAtResolvedProject() + const runtime = createRuntime() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the create reads only `runtime` and the creator-provenance fields, which default when absent. + const factory = agentLaunchWorkspaceFactory({ runtime } as unknown as RpcContext, 'claude') + + await expect( + factory.createWorktree({ + create: { repo: 'id:dup', repoHostId: 'ssh:gpu', name: 'x' }, + startupAgent: 'claude' + }) + ).rejects.toThrow('stop after resolution') + + expect(reached.mock.calls[0]?.[1]).toMatchObject({ path: '/remote/dup' }) + }) + + it('a retried create for one host never coalesces with the same retry for another', async () => { + const runtime = createRuntime() + const run = vi.fn(async () => 'created') + + await runtime.dedupeWorktreeCreate({ repo: 'id:dup', repoHostId: 'local' }, 'retry-1', run) + await runtime.dedupeWorktreeCreate({ repo: 'id:dup', repoHostId: 'ssh:gpu' }, 'retry-1', run) + + expect(run).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/main/runtime/terminal/launch/terminal-create-deduplication.ts b/src/main/runtime/terminal/launch/terminal-create-deduplication.ts index 7298e71a792..1d77b402e00 100644 --- a/src/main/runtime/terminal/launch/terminal-create-deduplication.ts +++ b/src/main/runtime/terminal/launch/terminal-create-deduplication.ts @@ -9,7 +9,8 @@ import { getRegisteredSshState } from '../../../ssh/targets/ssh-target-registry' import { getConnectionExecutionHostId, LOCAL_EXECUTION_HOST_ID, - toSshExecutionHostId + toSshExecutionHostId, + type ExecutionHostId } from '../../../../shared/execution-host' import { resolveWorktreeLaunchHost } from '../../worktree-launch-host-repo' import type { TuiAgent } from '../../../../shared/tui-agent' @@ -196,14 +197,21 @@ export abstract class OrcaRuntimeWithTerminalCreateDeduplication extends OrcaRun // drop immediately so a genuine retry starts fresh, and successes linger // briefly so a retry whose response was lost in the cutover still reconciles. dedupeWorktreeCreate( - repoSelector: string, + target: string | { repo: string; repoHostId?: ExecutionHostId }, clientMutationId: string | undefined, run: () => Promise ): Promise { if (!clientMutationId) { return run() } - const key = `${repoSelector}\0${clientMutationId}` + // Why the host: two creates for one repo id on different hosts are different creates. + const repoKey = + typeof target === 'string' + ? target + : target.repoHostId + ? `${target.repoHostId}\0${target.repo}` + : target.repo + const key = `${repoKey}\0${clientMutationId}` const inflight = this.worktreeCreateByMutationId.get(key) if (inflight) { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Callers must use one operation/result type per repo and mutation key. diff --git a/src/main/runtime/workspaces/workspace-selectors.ts b/src/main/runtime/workspaces/workspace-selectors.ts index 57c67b33a8e..155f03f02d2 100644 --- a/src/main/runtime/workspaces/workspace-selectors.ts +++ b/src/main/runtime/workspaces/workspace-selectors.ts @@ -3,6 +3,11 @@ import type { ResolvedWorktree } from '../runtime-worktree-path-identity' import { splitWorktreeIdForFilesystem, worktreeIdentityKey } from '../../../shared/worktree/id' import { branchSelectorMatches, runtimePathsEqual } from '../runtime-worktree-path-identity' import { getRepoExecutionHostId, getWorktreeExecutionHostId } from '../../../shared/execution-host' +import { + runtimeRepoSelectorHostId, + runtimeRepoSelectorText, + type RuntimeRepoSelector +} from '../runtime-repo-selector' import type { WorktreeLineageInput, WorktreeLineageResolution @@ -172,11 +177,15 @@ export abstract class RuntimeWorkspaceSelectors extends OrcaRuntimeWithResolveBr ) } - protected async resolveRepoSelector(selector: string): Promise { + protected async resolveRepoSelector(selector: RuntimeRepoSelector): Promise { if (!this.store) { throw new Error('repo_not_found') } - const candidates = this.selectReposBySelector(selector) + const matches = this.selectReposBySelector(runtimeRepoSelectorText(selector)) + const hostId = runtimeRepoSelectorHostId(selector) + const candidates = hostId + ? matches.filter((repo) => getRepoExecutionHostId(repo) === hostId) + : matches if (candidates.length === 1) { return candidates[0] diff --git a/src/shared/rpc-contract/github-repo-target-params.ts b/src/shared/rpc-contract/github-repo-target-params.ts index 199e90ad388..c8a9135dc67 100644 --- a/src/shared/rpc-contract/github-repo-target-params.ts +++ b/src/shared/rpc-contract/github-repo-target-params.ts @@ -1,10 +1,14 @@ import { z } from 'zod' import { OptionalString, requiredString } from './rpc-param-primitives' +import { OptionalExecutionHostId } from './worktree-params' export const RepoSelector = z.object({ repo: requiredString('Missing repo selector') }) +/** `repo.show`: `repoHostId` narrows a repo id set up on several hosts; older hosts drop it. */ +export const RepoShow = RepoSelector.extend({ repoHostId: OptionalExecutionHostId }) + export const SlugRepo = z.object({ owner: requiredString('Missing owner'), repo: requiredString('Missing repo'), diff --git a/src/shared/rpc-contract/rpc-params-catalog.generated.ts b/src/shared/rpc-contract/rpc-params-catalog.generated.ts index 5052e1b4e98..e3672b9326b 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -289,7 +289,7 @@ import { UpdatePrState, UpdatePrTitle } from './github-pull-request-update-params' -import { RepoSelector, SlugRepo } from './github-repo-target-params' +import { RepoSelector, RepoShow, SlugRepo } from './github-repo-target-params' import { IssuesList, RateLimit, @@ -1252,7 +1252,7 @@ export const RPC_PARAMS_BY_METHOD = { 'repo.searchRefs': RepoSearchRefs, 'repo.setBaseRef': RepoSetBaseRef, 'repo.setupScriptImports': RepoSelector, - 'repo.show': RepoSelector, + 'repo.show': RepoShow, 'repo.sparsePresets': RepoSelector, 'repo.update': RepoUpdate, 'runtime.automaticWork.resume': null, diff --git a/src/shared/rpc-contract/worktree-create-params.ts b/src/shared/rpc-contract/worktree-create-params.ts index a492b78a0b2..d026d67980b 100644 --- a/src/shared/rpc-contract/worktree-create-params.ts +++ b/src/shared/rpc-contract/worktree-create-params.ts @@ -16,6 +16,7 @@ import { import { AutomationWorkspaceProvenanceRequest, CliWorkspaceProvenanceRequest, + OptionalExecutionHostId, OptionalTuiAgent, assertLinkedWorkItemSourceContextMatch } from './worktree-params' @@ -26,6 +27,9 @@ export const WorktreeCreate = z .unknown() .transform((v) => (typeof v === 'string' ? v : '')) .pipe(z.string().min(1, 'Missing repo selector')), + /** The host of the repo row `repo` names: one repo id can be set up on several hosts. Absent + * (older clients) keeps the selector-only lookup, which refuses an id on several hosts. */ + repoHostId: OptionalExecutionHostId, name: OptionalString, /** Set by clients that fell back to a generated creature name. Absent means user-typed, so the * host neither skips a retired candidate nor retires the name it lands on. */