From 5d8532f6d3eb2afdc2fb7dc7ec910e15535f189b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:27:13 -0700 Subject: [PATCH] fix(worktrees): resolve the execution host at both worktree-create entry points (#18545) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two entry points create the same workspace and disagreed about how to read its host. `orca-runtime-create-managed-worktree.ts:63` resolved through `getRepoSshConnectionId` and then normalized the row; the `worktrees:create` IPC handler branched on raw `repo.connectionId` (`register-worktree-create-handlers.ts:66-69`). So a repo naming its owner only as `executionHostId: 'ssh:'` created remotely through the runtime and ran `git worktree add` on the client against a remote path through IPC (#11163). Same repo, two entry points, different answers. Both now take one route, resolved through the existing layer (`getRepoExecutionHostId` -> #18296's `resolveGitRouteForHost`). No new resolver. The row normalization on the `ssh` variant is kept, and it is a **workaround, not the pattern**. `createRemoteWorktree` and its callees re-read `repo.connectionId!` at five depths in `ipc/worktree-remote.ts` (1627, 1847, 1848, 1865, 2029), so the resolved connection has to reach them through the field they already read. It travels only as far as that object does — anything downstream that re-reads the row from the store still sees the unnormalized one, and it cannot express the `runtime:` refusal on its own. Proper fix, deliberately not done here: give that pipeline an explicit connection parameter and delete `repo.connectionId!` from it so every reader becomes a compile error, the technique #18307/#18325 used. That is a change inside a 2800-line module plus its callers, and it wants its own PR. Three answers that used to collapse into one, now distinct at both entry points: - `executionHostId: 'ssh:*'` with no `connectionId` -> that SSH host (IPC used to create locally); - `executionHostId: 'local'` with a surviving `connectionId` -> local, since a local row cannot nest an SSH namespace. This is what `getRepoSshConnectionId` and therefore the runtime sibling already answered; IPC used to go remote; - `runtime:` -> refused. Its worktree is created by that environment's own server and the SSH target on its repo row is that server's nested one, addressable only as (environmentId, targetId). The renderer already routes runtime-environment creates over `worktree.create` RPC rather than this IPC channel, so reaching either entry point with one is a routing mistake. Matches `workspace-cleanup-git-route` and `runtime-git-command-target`. Folder-workspace creation is untouched on both sides: it is a registration, not a filesystem create, so the route is resolved after that branch on the IPC side, and on the runtime side only the agent trust write consumes it — where a `runtime:` host now yields `null` instead of the nested target, so the write stops going to a same-named target in this client's table. No wire or persistence change: the normalized row is a local value passed to the create pipeline, never stored, and `CreateWorktreeResult` is untouched. --- ...rees-create-execution-host-routing.test.ts | 229 ++++++++++++++++++ .../register-worktree-create-handlers.ts | 19 +- .../orca-runtime-create-managed-worktree.ts | 44 ++-- .../worktree-create-execution-host.spec.ts | 61 +++++ src/main/runtime/orca-runtime.test.ts | 1 + ...rktree-create-execution-host-route.test.ts | 106 ++++++++ .../worktree-create-execution-host-route.ts | 69 ++++++ 7 files changed, 505 insertions(+), 24 deletions(-) create mode 100644 src/main/ipc/worktrees-create-execution-host-routing.test.ts create mode 100644 src/main/runtime/orca-runtime-tests/worktree-create-execution-host.spec.ts create mode 100644 src/main/worktree-create-execution-host-route.test.ts create mode 100644 src/main/worktree-create-execution-host-route.ts diff --git a/src/main/ipc/worktrees-create-execution-host-routing.test.ts b/src/main/ipc/worktrees-create-execution-host-routing.test.ts new file mode 100644 index 00000000000..36f1e816ea6 --- /dev/null +++ b/src/main/ipc/worktrees-create-execution-host-routing.test.ts @@ -0,0 +1,229 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + addWorktreeMock, + getActiveMultiplexerMock, + getSshGitProviderMock, + listWorktreesMock +} from './worktrees-test-module-mocks' +import { handlers, setupWorktreeHandlers, store } from './worktrees-test-harness' + +vi.mock('electron', async () => + (await import('./worktrees-test-module-mocks')).electronModuleMock() +) +vi.mock('../git/worktree', async () => + (await import('./worktrees-test-module-mocks')).gitWorktreeModuleMock() +) +vi.mock('../git/runner', async () => + (await import('./worktrees-test-module-mocks')).gitRunnerModuleMock() +) +vi.mock('../git/repo', async () => + (await import('./worktrees-test-module-mocks')).gitRepoModuleMock() +) +vi.mock('../git/git-username', async (importOriginal) => ({ + ...(await importOriginal>()), + resolveLocalGitUsername: (await import('./worktrees-test-module-mocks')) + .resolveLocalGitUsernameMock +})) +vi.mock('../github/client', async () => + (await import('./worktrees-test-module-mocks')).githubClientModuleMock() +) +vi.mock('../source-control/hosted-review', async () => + (await import('./worktrees-test-module-mocks')).hostedReviewModuleMock() +) +vi.mock('../providers/ssh-git-dispatch', async () => + (await import('./worktrees-test-module-mocks')).sshGitDispatchModuleMock() +) +vi.mock('../providers/ssh-filesystem-dispatch', async () => + (await import('./worktrees-test-module-mocks')).sshFilesystemDispatchModuleMock() +) +vi.mock('./worktree-symlinks', async () => + (await import('./worktrees-test-module-mocks')).worktreeSymlinksModuleMock() +) +vi.mock('./ssh', async () => (await import('./worktrees-test-module-mocks')).sshModuleMock()) +vi.mock('../ssh/ssh-target-registry', async () => + (await import('./worktrees-test-module-mocks')).sshTargetRegistryModuleMock() +) +vi.mock('../hooks', async () => (await import('./worktrees-test-module-mocks')).hooksModuleMock()) +vi.mock('../setup-runner-script-text', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).setupRunnerScriptTextModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../worktree-runner-script', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).worktreeRunnerScriptModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../effective-hook-config', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).effectiveHookConfigModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../setup-hook-env-vars', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).setupHookEnvVarsModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('./worktree-logic', async (importOriginal) => + (await import('./worktrees-test-module-mocks')).worktreeLogicModuleMock( + (await importOriginal()) as Record + ) +) +vi.mock('../terminal-history-deletion', async () => + (await import('./worktrees-test-module-mocks')).terminalHistoryDeletionModuleMock() +) +vi.mock('../ports/advertised-url-watcher', async () => + (await import('./worktrees-test-module-mocks')).advertisedUrlWatcherModuleMock() +) +vi.mock('../workspace-cleanup-scan-snapshot', async () => + (await import('./worktrees-test-module-mocks')).workspaceCleanupScanSnapshotModuleMock() +) +vi.mock('../workspace-space-analysis-snapshot', async () => + (await import('./worktrees-test-module-mocks')).workspaceSpaceAnalysisSnapshotModuleMock() +) +vi.mock('../workspace-cleanup-removal-snapshot-prune', async () => + (await import('./worktrees-test-module-mocks')).workspaceCleanupRemovalSnapshotPruneModuleMock() +) +vi.mock('../runtime/worktree-teardown', async () => + (await import('./worktrees-test-module-mocks')).worktreeTeardownModuleMock() +) +vi.mock('./pty', async () => (await import('./worktrees-test-module-mocks')).ptyModuleMock()) + +const REMOTE_REPO_PATH = '/remote/repo' + +function makeRepo(fields: Record) { + return { + id: 'repo-1', + path: REMOTE_REPO_PATH, + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + worktreeBaseRef: 'origin/main', + ...fields + } +} + +function makeProvider(worktreePath: string) { + return { + exec: vi.fn().mockImplementation(async (args: string[]) => { + if (args[0] === 'remote') { + return { stdout: 'origin\n', stderr: '' } + } + if (args[0] === 'show-ref') { + // A hit here reads as "branch already exists"; the create loop would then rename. + throw Object.assign(new Error('missing exact ref'), { code: 1 }) + } + return { stdout: '', stderr: '' } + }), + fetchRemoteTrackingRef: vi.fn().mockResolvedValue(undefined), + addWorktree: vi.fn().mockResolvedValue(undefined), + listWorktrees: vi.fn().mockResolvedValue([ + { + path: worktreePath, + head: 'abc123', + branch: 'refs/heads/wt', + isBare: false, + isMainWorktree: false + } + ]) + } +} + +function useRepo(repo: ReturnType): void { + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + store.setWorktreeMeta.mockImplementation((_worktreeId: string, meta: unknown) => meta) + getActiveMultiplexerMock.mockReturnValue({ + request: vi.fn().mockResolvedValue(undefined), + notify: vi.fn() + }) +} + +describe('worktrees:create execution host routing', () => { + beforeEach(() => { + setupWorktreeHandlers() + }) + + it('creates on the SSH host for a row that names it only as executionHostId', async () => { + // No `connectionId`: the raw read answered "local" and ran `git worktree add` on the client + // against `/remote/repo`. The runtime sibling already resolved this row remotely. + useRepo(makeRepo({ executionHostId: 'ssh:target-a' })) + const provider = makeProvider('/remote/repo-wt') + getSshGitProviderMock.mockImplementation((connectionId: string) => + connectionId === 'target-a' ? provider : undefined + ) + + await handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'wt' }) + + expect(provider.addWorktree).toHaveBeenCalledTimes(1) + expect(addWorktreeMock).not.toHaveBeenCalled() + }) + + it('keeps two simultaneously registered SSH hosts apart', async () => { + useRepo(makeRepo({ executionHostId: 'ssh:target-b' })) + const providerA = makeProvider('/remote/repo-wt-a') + const providerB = makeProvider('/remote/repo-wt-b') + getSshGitProviderMock.mockImplementation((connectionId: string) => + connectionId === 'target-a' ? providerA : connectionId === 'target-b' ? providerB : undefined + ) + + await handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'wt' }) + + expect(providerB.addWorktree).toHaveBeenCalledTimes(1) + expect(providerA.addWorktree).not.toHaveBeenCalled() + expect(addWorktreeMock).not.toHaveBeenCalled() + }) + + it('refuses a runtime row with no nested SSH target instead of creating locally', async () => { + useRepo(makeRepo({ executionHostId: 'runtime:env-1' })) + + await expect( + handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'wt' }) + ).rejects.toThrow('not dispatched by this process') + + expect(addWorktreeMock).not.toHaveBeenCalled() + }) + + it('refuses a runtime row whose nested SSH target is dialable in this namespace', async () => { + // `target-a` names a target inside env-1. A same-named one registered here is another machine, + // so creating through it lands the checkout on the wrong host. + useRepo(makeRepo({ executionHostId: 'runtime:env-1', connectionId: 'target-a' })) + const provider = makeProvider('/remote/repo-wt') + getSshGitProviderMock.mockImplementation((connectionId: string) => + connectionId === 'target-a' ? provider : undefined + ) + + await expect( + handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'wt' }) + ).rejects.toThrow('not dispatched by this process') + + expect(provider.addWorktree).not.toHaveBeenCalled() + expect(addWorktreeMock).not.toHaveBeenCalled() + }) + + it('answers local for a row that declares itself local while carrying a connection', async () => { + // A contradictory row: `getRepoSshConnectionId` lets `local` win, and the runtime sibling has + // always read it that way. The raw field sent it remote, so the two entry points disagreed. + useRepo( + makeRepo({ path: '/workspace/repo', executionHostId: 'local', connectionId: 'target-a' }) + ) + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/wt', + head: 'abc123', + branch: 'wt', + isBare: false, + isMainWorktree: false + } + ]) + const provider = makeProvider('/remote/repo-wt') + getSshGitProviderMock.mockImplementation((connectionId: string) => + connectionId === 'target-a' ? provider : undefined + ) + + await handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'wt' }) + + expect(addWorktreeMock).toHaveBeenCalledTimes(1) + expect(provider.addWorktree).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/worktrees/create/register-worktree-create-handlers.ts b/src/main/ipc/worktrees/create/register-worktree-create-handlers.ts index f371529963c..1f94598208b 100644 --- a/src/main/ipc/worktrees/create/register-worktree-create-handlers.ts +++ b/src/main/ipc/worktrees/create/register-worktree-create-handlers.ts @@ -29,6 +29,7 @@ import { normalizeLinkedWorkItemFields } from '../ipc-context-schemas' import type { CreateWorktreeArgsWithSystemProvenance } from '../ipc-context-schemas' import { createFolderWorkspace } from './folder-workspace-creation' import { findExactRepoOwner, isCapturedRepoCurrent } from '../listing/worktree-host-ownership' +import { requireWorktreeCreateRoute } from '../../../worktree-create-execution-host-route' import type { WorktreeIpcContext } from '../worktree-ipc-context' export function registerWorktreeCreateHandlers(context: WorktreeIpcContext): void { @@ -62,11 +63,19 @@ export function registerWorktreeCreateHandlers(context: WorktreeIpcContext): voi let result: CreateWorktreeResult try { // Why: wrap only the helpers; the pre-validation throws above are IPC-shape bugs, not the git/filesystem failures the funnel tracks. - result = isFolderRepo(repo) - ? createFolderWorkspace(createArgs, repo, store) - : repo.connectionId - ? await createRemoteWorktree(createArgs, repo, store, mainWindow) - : await createLocalWorktree(createArgs, repo, store, mainWindow, runtime) + if (isFolderRepo(repo)) { + // A folder workspace is a registration, not a filesystem create, so it is host-agnostic. + result = createFolderWorkspace(createArgs, repo, store) + } else { + // Resolve the host rather than reading the raw field: an `executionHostId: 'ssh:*'`-only + // row read as local here and ran `git worktree add` on the client against a remote path, + // while the runtime sibling on the same repo already resolved. + const createRoute = requireWorktreeCreateRoute(repo) + result = + createRoute.kind === 'ssh' + ? await createRemoteWorktree(createArgs, createRoute.repo, store, mainWindow) + : await createLocalWorktree(createArgs, repo, store, mainWindow, runtime) + } } catch (error) { releaseAutomationWorkspaceProvenanceRequest(args.automationProvenanceRequest) track('workspace_create_failed', { diff --git a/src/main/runtime/orca-runtime-create-managed-worktree.ts b/src/main/runtime/orca-runtime-create-managed-worktree.ts index f9116f73405..f17a2285b83 100644 --- a/src/main/runtime/orca-runtime-create-managed-worktree.ts +++ b/src/main/runtime/orca-runtime-create-managed-worktree.ts @@ -4,7 +4,8 @@ import type { RuntimeManagedWorktreeCreateArgs } from './runtime-managed-worktre import type { CreateWorktreeResult } from '../../shared/worktree/create-types' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' import { isFolderRepo } from '../../shared/repo-kind' -import { getRepoSshConnectionId } from '../../shared/execution-host' +import { resolveWorktreeCreateRoute } from '../worktree-create-execution-host-route' +import { ExecutionHostNotDispatchableError } from '../providers/execution-host-provider-dispatch' import { createRuntimeFolderWorktree } from './runtime-folder-worktree-create' import { createRuntimeLocalManagedWorktree } from './runtime-local-worktree-create' import { prepareRuntimeLocalWorktreeSetup } from './runtime-local-worktree-setup' @@ -57,10 +58,14 @@ export class OrcaRuntimeWithCreateManagedWorktree extends OrcaRuntimeWithGetWork draftStartup?.agent ?? (requestedAgentEnabled ? requestedAgent : undefined)) const effectiveDraftPaste = args.startupDraftPaste ?? draftStartup?.draftPaste - // Resolve the execution host once: SSH ownership has two spellings, and reading the raw - // `connectionId` field routes an `executionHostId: 'ssh:*'`-only repo down the local path, - // which runs `git worktree add` on the client against a remote path. - const sshConnectionId = getRepoSshConnectionId(repo) + // Resolve the execution host once, shared with the `worktrees:create` IPC entry point so the + // two cannot answer differently for the same repo. Reading the raw `connectionId` field routes + // an `executionHostId: 'ssh:*'`-only repo down the local path, which runs `git worktree add` on + // the client against a remote path. + const createRoute = resolveWorktreeCreateRoute(repo) + // `null` on a `runtime:` host is deliberate: its nested target is addressable only inside that + // environment, so the trust write must not go to a same-named target in this client's table. + const sshConnectionId = createRoute.kind === 'ssh' ? createRoute.connectionId : null if (isFolderRepo(repo)) { // A folder workspace is a registration, not a filesystem create, so it is host-agnostic — // except for the agent trust write, which must land on the host that will run the agent. @@ -97,20 +102,21 @@ export class OrcaRuntimeWithCreateManagedWorktree extends OrcaRuntimeWithGetWork const lineageInput = args.lineage || args.comment ? { ...args.lineage, comment: args.comment } : undefined const lineageResolution = await this.resolveLineageForWorktreeCreate(lineageInput) - if (sshConnectionId) { - // Why normalize the row: the remote-create pipeline reads `repo.connectionId!` at every - // depth, so hand it the connection the resolved host actually names. - const result = await this.createManagedRemoteWorktree( - { ...repo, connectionId: sshConnectionId }, - { - ...args, - activate: args.activate, - ...(effectiveStartup ? { startup: effectiveStartup } : {}), - ...(effectiveStartupFollowup ? { startupFollowup: effectiveStartupFollowup } : {}), - ...(effectiveCreatedWithAgent ? { createdWithAgent: effectiveCreatedWithAgent } : {}), - ...(effectiveDraftPaste ? { startupDraftPaste: effectiveDraftPaste } : {}) - } - ) + if (createRoute.kind === 'runtime') { + throw new ExecutionHostNotDispatchableError(createRoute.hostId) + } + if (createRoute.kind === 'ssh') { + // `createRoute.repo` carries the resolved connection in `connectionId`, because the + // remote-create pipeline still reads `repo.connectionId!` at every depth. See the workaround + // note in worktree-create-execution-host-route.ts. + const result = await this.createManagedRemoteWorktree(createRoute.repo, { + ...args, + activate: args.activate, + ...(effectiveStartup ? { startup: effectiveStartup } : {}), + ...(effectiveStartupFollowup ? { startupFollowup: effectiveStartupFollowup } : {}), + ...(effectiveCreatedWithAgent ? { createdWithAgent: effectiveCreatedWithAgent } : {}), + ...(effectiveDraftPaste ? { startupDraftPaste: effectiveDraftPaste } : {}) + }) const recordedLineage = this.recordCreatedWorktreeLineage(result.worktree, lineageResolution) this.emitWorktreeLifecycle({ kind: 'created', diff --git a/src/main/runtime/orca-runtime-tests/worktree-create-execution-host.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-create-execution-host.spec.ts new file mode 100644 index 00000000000..981e9860463 --- /dev/null +++ b/src/main/runtime/orca-runtime-tests/worktree-create-execution-host.spec.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + OrcaRuntimeService, + addWorktree, + registerSshGitProvider, + unregisterSshGitProvider +} from '../orca-runtime-test-mocks.spec' +import { store } from '../orca-runtime-test-fixtures.spec' + +const RUNTIME_REPO_PATH = '/remote/repo' + +function makeRuntimeHostedStore(extraRepoFields: Record = {}) { + const repo = { + ...store.getRepos()[0]!, + path: RUNTIME_REPO_PATH, + executionHostId: 'runtime:env-1', + ...extraRepoFields + } + return { + ...store, + getRepos: () => [repo], + getRepo: (id: string) => (id === repo.id ? repo : undefined) + } +} + +describe('OrcaRuntimeService worktree create execution host', () => { + beforeEach(() => { + vi.mocked(addWorktree).mockClear() + }) + + it('refuses to create for a runtime-hosted repo with no nested SSH target', async () => { + const runtime = new OrcaRuntimeService(makeRuntimeHostedStore() as never) + + await expect( + runtime.createManagedWorktree({ repoSelector: 'id:repo-1', name: 'wt' }) + ).rejects.toThrow('not dispatched by this process') + + expect(addWorktree).not.toHaveBeenCalled() + }) + + it('refuses a runtime-hosted repo whose nested SSH target is dialable in this namespace', async () => { + // `target-a` names a target inside env-1. The same-named one registered here is another + // machine, so creating through it would put the checkout on the wrong host. + const provider = { exec: vi.fn(), addWorktree: vi.fn(), listWorktrees: vi.fn() } + registerSshGitProvider('target-a', provider as never) + const runtime = new OrcaRuntimeService( + makeRuntimeHostedStore({ connectionId: 'target-a' }) as never + ) + + try { + await expect( + runtime.createManagedWorktree({ repoSelector: 'id:repo-1', name: 'wt' }) + ).rejects.toThrow('not dispatched by this process') + + expect(provider.addWorktree).not.toHaveBeenCalled() + expect(addWorktree).not.toHaveBeenCalled() + } finally { + unregisterSshGitProvider('target-a') + } + }) +}) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 573c5ba979d..9d223233ef2 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -18,6 +18,7 @@ await import('./orca-runtime-tests/terminal-listing.spec') await import('./orca-runtime-tests/worktree-selector-resolution.spec') await import('./orca-runtime-tests/local-worktree-creation.spec') await import('./orca-runtime-tests/local-worktree-creation-part-02.spec') +await import('./orca-runtime-tests/worktree-create-execution-host.spec') await import('./orca-runtime-tests/ssh-worktree-lifecycle.spec') await import('./orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec') await import('./orca-runtime-tests/ssh-worktree-lifecycle-part-03.spec') diff --git a/src/main/worktree-create-execution-host-route.test.ts b/src/main/worktree-create-execution-host-route.test.ts new file mode 100644 index 00000000000..ec4ecd2899e --- /dev/null +++ b/src/main/worktree-create-execution-host-route.test.ts @@ -0,0 +1,106 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { registerSshGitProvider, unregisterSshGitProvider } from './providers/ssh-git-dispatch' +import { ExecutionHostNotDispatchableError } from './providers/execution-host-provider-dispatch' +import type { Repo } from '../shared/repo-types' +import { + requireWorktreeCreateRoute, + resolveWorktreeCreateRoute +} from './worktree-create-execution-host-route' + +const HOST_A = 'target-a' +const HOST_B = 'target-b' + +function repoRow(fields: Partial): Repo { + return { + id: 'repo-1', + path: '/remote/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + ...fields + } as Repo +} + +afterEach(() => { + unregisterSshGitProvider(HOST_A) + unregisterSshGitProvider(HOST_B) +}) + +describe('resolveWorktreeCreateRoute', () => { + it('routes a row that names its host only as executionHostId to that SSH target', () => { + registerSshGitProvider(HOST_A, { name: 'git-a' } as never) + + expect(resolveWorktreeCreateRoute(repoRow({ executionHostId: 'ssh:target-a' }))).toMatchObject({ + kind: 'ssh', + hostId: 'ssh:target-a', + connectionId: HOST_A, + repo: { connectionId: HOST_A } + }) + }) + + it('normalizes the row for a legacy connectionId-only repo without changing its answer', () => { + expect(resolveWorktreeCreateRoute(repoRow({ connectionId: HOST_A }))).toMatchObject({ + kind: 'ssh', + connectionId: HOST_A, + repo: { connectionId: HOST_A } + }) + }) + + it('keeps two simultaneously registered SSH hosts on their own connections', () => { + registerSshGitProvider(HOST_A, { name: 'git-a' } as never) + registerSshGitProvider(HOST_B, { name: 'git-b' } as never) + + expect(resolveWorktreeCreateRoute(repoRow({ executionHostId: 'ssh:target-a' }))).toMatchObject({ + connectionId: HOST_A, + repo: { connectionId: HOST_A } + }) + expect(resolveWorktreeCreateRoute(repoRow({ executionHostId: 'ssh:target-b' }))).toMatchObject({ + connectionId: HOST_B, + repo: { connectionId: HOST_B } + }) + }) + + it('lets an explicit local host win over a surviving connectionId', () => { + // A contradictory row. `getRepoExecutionHostId` answers `local`, which is what the runtime + // create sibling has always done; the raw read sent it remote. + expect( + resolveWorktreeCreateRoute(repoRow({ executionHostId: 'local', connectionId: HOST_A })) + ).toEqual({ kind: 'local', hostId: 'local' }) + }) + + it('answers runtime for a runtime row with no nested SSH target', () => { + expect(resolveWorktreeCreateRoute(repoRow({ executionHostId: 'runtime:env-1' }))).toEqual({ + kind: 'runtime', + hostId: 'runtime:env-1', + environmentId: 'env-1' + }) + }) + + it('answers runtime for a runtime row whose nested target is dialable here', () => { + registerSshGitProvider(HOST_A, { name: 'git-a' } as never) + + expect( + resolveWorktreeCreateRoute( + repoRow({ executionHostId: 'runtime:env-1', connectionId: HOST_A }) + ) + ).toMatchObject({ kind: 'runtime', environmentId: 'env-1' }) + }) +}) + +describe('requireWorktreeCreateRoute', () => { + it('refuses a runtime host rather than creating through this client', () => { + expect(() => requireWorktreeCreateRoute(repoRow({ executionHostId: 'runtime:env-1' }))).toThrow( + ExecutionHostNotDispatchableError + ) + }) + + it('passes local and SSH hosts through unchanged', () => { + registerSshGitProvider(HOST_A, { name: 'git-a' } as never) + + expect(requireWorktreeCreateRoute(repoRow({}))).toEqual({ kind: 'local', hostId: 'local' }) + expect(requireWorktreeCreateRoute(repoRow({ executionHostId: 'ssh:target-a' }))).toMatchObject({ + kind: 'ssh', + connectionId: HOST_A + }) + }) +}) diff --git a/src/main/worktree-create-execution-host-route.ts b/src/main/worktree-create-execution-host-route.ts new file mode 100644 index 00000000000..2831a2fcbeb --- /dev/null +++ b/src/main/worktree-create-execution-host-route.ts @@ -0,0 +1,69 @@ +/** + * Which execution host a worktree create runs on. + * + * Two entry points create the same workspace and disagreed about how to read its host. The runtime + * path resolved (`orca-runtime-create-managed-worktree.ts`) and then normalized the row; the IPC + * handler branched on raw `repo.connectionId`, so a row naming its owner only as + * `executionHostId: 'ssh:'` ran `git worktree add` on the client against a remote path + * (#11163). Same repo, two entry points, two answers. + * + * Both now take this one route. + * + * The `repo` on the `ssh` variant is a normalization, and it is a workaround rather than the + * pattern: `createRemoteWorktree` and its callees re-read `repo.connectionId!` at five depths + * (`ipc/worktree-remote.ts`), so the resolved connection has to be handed to them through the field + * they already read. It travels only as far as this object does — anything downstream that re-reads + * the row from the store still sees the unnormalized one. The real fix is to give that pipeline an + * explicit connection parameter and delete `repo.connectionId!` from it, which is a separate change. + */ + +import { getRepoExecutionHostId, type LOCAL_EXECUTION_HOST_ID } from '../shared/execution-host' +import type { Repo } from '../shared/repo-types' +import { + ExecutionHostNotDispatchableError, + resolveGitRouteForHost +} from './providers/execution-host-provider-dispatch' + +export type WorktreeCreateRoute = + | { kind: 'local'; hostId: typeof LOCAL_EXECUTION_HOST_ID } + | { + kind: 'ssh' + hostId: `ssh:${string}` + connectionId: string + /** The row with `connectionId` set to the resolved target; see the workaround note above. */ + repo: Repo + } + | { kind: 'runtime'; hostId: `runtime:${string}`; environmentId: string } + +export function resolveWorktreeCreateRoute(repo: Repo): WorktreeCreateRoute { + const route = resolveGitRouteForHost(getRepoExecutionHostId(repo)) + switch (route.kind) { + case 'local': + return { kind: 'local', hostId: route.hostId } + case 'ssh': + return { + kind: 'ssh', + hostId: route.hostId, + connectionId: route.connectionId, + repo: { ...repo, connectionId: route.connectionId } + } + case 'runtime': + return { kind: 'runtime', hostId: route.hostId, environmentId: route.environmentId } + } +} + +/** + * For the two create forks that put files on a host. `runtime:` is not one of them: the + * environment's own server creates the worktree, and the SSH target on its repo row is that + * server's nested one, addressable only as (environmentId, targetId). Creating through this + * client's SSH table would `git worktree add` on a same-named target on the wrong machine. + */ +export function requireWorktreeCreateRoute( + repo: Repo +): Exclude { + const route = resolveWorktreeCreateRoute(repo) + if (route.kind === 'runtime') { + throw new ExecutionHostNotDispatchableError(route.hostId) + } + return route +}