mirror of
https://github.com/stablyai/orca.git
synced 2026-11-02 01:57:57 +00:00
fix(cli): worktree create sends the host --project/--host selected (#31066)
* fix(cli): worktree create sends the host --project/--host selected Codex review 6 #6: the CLI picked the right setup but sent only its repo id, so a repo id set up on two hosts failed as selector_ambiguous; and --project-host-setup ignored --host when setup ids repeated. worktree.create gains optional repoHostId (older hosts drop it and keep the old behaviour); the runtime narrows the repo selector to that host. * refactor(runtime): the repo selector carries its host through every lookup Review follow-up (Codex + Opus on #31066): a separate repoHostId was dropped by the startup-agent lookup, the CLI's GitLab link check and the create dedupe key. RuntimeRepoSelector now carries the host with the selector, so showRepo/resolveRepoSelector and startup-agent resolution keep it; the create dedupe key includes it; repo.show gains the same optional repoHostId; the CLI spreads one CliRepoTarget into every repo lookup. Handler-level tests cover worktree.create (plain, startupAgent, startupDraft), agent.launch, repo.show and the dedupe key. * test(runtime): drop a needless cast in the agent.launch host test; fold an import
This commit is contained in:
@@ -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<string, string | boolean>,
|
||||
client: RuntimeClient,
|
||||
target: { repo: string } | { worktree: string }
|
||||
target: CliRepoTarget | { worktree: string }
|
||||
): Promise<void> {
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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<string, CommandHandler> = {
|
||||
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<string, CommandHandler> = {
|
||||
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',
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -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',
|
||||
|
||||
@@ -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<string, string |
|
||||
}
|
||||
}
|
||||
|
||||
export async function resolveProjectCreateRepoSelector(
|
||||
flags: Map<string, string | boolean>,
|
||||
client: RuntimeClient
|
||||
): Promise<string | undefined> {
|
||||
return (await resolveProjectCreateTarget(flags, client))?.repoSelector
|
||||
}
|
||||
|
||||
// Why: the routed runtime can hold both an exact `runtime:<id>` 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<string, string | boolean>,
|
||||
cwdParentWorktree: { worktreeId: string } | undefined,
|
||||
client: RuntimeClient
|
||||
): Promise<string> {
|
||||
const projectRepoSelector = await resolveProjectCreateRepoSelector(flags, client)
|
||||
if (projectRepoSelector) {
|
||||
return projectRepoSelector
|
||||
): Promise<CliRepoTarget> {
|
||||
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',
|
||||
|
||||
@@ -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<string, (event: unknown, args: unknown) => 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(),
|
||||
|
||||
@@ -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<Repo>
|
||||
protected abstract resolveRepoSelector(selector: RuntimeRepoSelector): Promise<Repo>
|
||||
protected abstract selectReposBySelector(selector: string): Repo[]
|
||||
protected abstract listRepoWorktreesForListing(
|
||||
repo: Repo,
|
||||
|
||||
@@ -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<ResolvedWorktree>
|
||||
|
||||
protected abstract resolveRepoSelector(selector: string): Promise<Repo>
|
||||
protected abstract resolveRepoSelector(selector: RuntimeRepoSelector): Promise<Repo>
|
||||
|
||||
protected readonly fileCommands = new RuntimeFileCommands({
|
||||
getRuntimeId: () => this.runtimeId,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<typeof WorktreeCreate>
|
||||
@@ -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<WorktreeCreateParams, 'repo' | 'repoHostId'>
|
||||
): 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 } : {}),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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<Repo>
|
||||
resolveRepo: (selector: RuntimeRepoSelector) => Promise<Repo>
|
||||
forgetTerminalTopology: (repoId: string) => void
|
||||
/** Optional so test doubles and older wiring keep removing without it. */
|
||||
stopProjectTerminals?: (
|
||||
@@ -62,7 +63,7 @@ function omitUndefined<T extends Record<string, unknown>>(value: T): Partial<T>
|
||||
export class RuntimeRepositorySettingsController {
|
||||
constructor(private readonly deps: RuntimeRepositorySettingsDependencies) {}
|
||||
|
||||
async show(repoSelector: string): Promise<Repo> {
|
||||
async show(repoSelector: RuntimeRepoSelector): Promise<Repo> {
|
||||
return await this.deps.resolveRepo(repoSelector)
|
||||
}
|
||||
|
||||
|
||||
@@ -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<never>
|
||||
},
|
||||
'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)
|
||||
})
|
||||
})
|
||||
@@ -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<T>(
|
||||
repoSelector: string,
|
||||
target: string | { repo: string; repoHostId?: ExecutionHostId },
|
||||
clientMutationId: string | undefined,
|
||||
run: () => Promise<T>
|
||||
): Promise<T> {
|
||||
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.
|
||||
|
||||
@@ -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<Repo> {
|
||||
protected async resolveRepoSelector(selector: RuntimeRepoSelector): Promise<Repo> {
|
||||
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]
|
||||
|
||||
@@ -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'),
|
||||
|
||||
+2
-2
@@ -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,
|
||||
|
||||
@@ -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. */
|
||||
|
||||
Reference in New Issue
Block a user