From 60317bc149da4399a110d8fb714d50e77882d7cc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:46:20 -0700 Subject: [PATCH] fix(hosts): resolve a folder workspace's SSH host from the repo's host, not its raw connectionId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveFolderWorkspaceHost` inferred a workspace's host by reading `repo.connectionId` directly. SSH ownership has two spellings on a repo row, and a row carrying only `executionHostId: 'ssh:'` has no `connectionId` to read — so it counted as a local repo and the workspace resolved `{ kind: 'local' }`. That is an execute-here answer for a workspace whose files are on an SSH host, the #11163 class, and it fires on a well-formed row. Resolve the host first, then read the target off it. Every other row keeps its existing contribution, including a `runtime:` row's nested SSH target: that target is not this client's to dial, but narrowing it here would be a second behaviour change riding on this one. The runtime branch above still answers `local`, and now says so — `FolderWorkspaceHost` has no runtime variant, and widening the type is its own change, not an oversight to be silently corrected. Three smaller items that stand on their own: - `resolveWorktreeExecutionHost` gains a `malformed` reason distinct from `unknown`. `unknown` (nothing carries the id) is a verdict the launch path may legitimately dispose of as a plain local folder; `malformed` (the row named a host that cannot be parsed) must fail closed. One word for two situations is the shape that lost the distinction in #18006. The strict read is private to that module: `getRepoExecutionHostId` stays the answer everywhere else, since its fall-through to `local` is harmless for the grouping, label and index callers that are nearly all of its ~340 call sites. - `readAllWorktreeMetaForRepo` / `readWorktreeMetaForRepo` replace four open-coded copies of the same host-qualified read (the F7/F8 lockstep shape). - `getExecutionHostLabel` answers 'Unknown host' rather than 'All hosts' for an id that names no host. Showing one unroutable row as though it were on every host is wrong on its own terms. Plain English like every other label in that module, none of which resolve through the renderer's i18n catalog. --- .../listing/detected-provider-listing.ts | 7 +- .../register-worktree-catalog-handlers.ts | 11 +- .../listing/ssh-worktree-fallback.ts | 4 +- .../listing/worktree-discovery-metadata.ts | 4 +- .../host-qualified-worktree-meta.ts | 23 +++- src/main/runtime/worktree-launch-host-repo.ts | 10 +- src/shared/execution-host.test.ts | 13 +++ src/shared/execution-host.ts | 9 +- .../folder-workspace-execution-host.test.ts | 110 ++++++++++++++++++ src/shared/folder-workspace-execution-host.ts | 20 +++- ...worktree-execution-host-resolution.test.ts | 34 ++++++ .../worktree-execution-host-resolution.ts | 38 +++++- 12 files changed, 257 insertions(+), 26 deletions(-) diff --git a/src/main/ipc/worktrees/listing/detected-provider-listing.ts b/src/main/ipc/worktrees/listing/detected-provider-listing.ts index 25c08d262fb..388c825530f 100644 --- a/src/main/ipc/worktrees/listing/detected-provider-listing.ts +++ b/src/main/ipc/worktrees/listing/detected-provider-listing.ts @@ -26,8 +26,7 @@ import { type DetectedWorktreeSideEffectToken } from './detected-worktree-scan-cache' import { loggedWorktreeListFailures, warnOnce } from './worktree-listing-diagnostics' -import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' -import { getRepoExecutionHostId } from '../../../../shared/execution-host' +import { readAllWorktreeMetaForRepo } from '../../../persistence/host-qualified-worktree-meta' export async function listDetectedWorktreesForCapturedRepo( store: Store, @@ -40,9 +39,7 @@ export async function listDetectedWorktreesForCapturedRepo( providerAbort?.signal.aborted ? ({ providerAbortStatus: providerAbort.status() } as const) : undefined - const allMeta = isFolderRepo(repo) - ? undefined - : readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) + const allMeta = isFolderRepo(repo) ? undefined : readAllWorktreeMetaForRepo(store, repo) // Why: only the disconnected fallbacks read this, so keep parseWorktreeId over the whole host snapshot // off the connected path entirely. let cachedSshWorktreeMetaIndex: SshWorktreeMetaIndex | undefined diff --git a/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts b/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts index d684461a381..4f3055c63a2 100644 --- a/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts +++ b/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts @@ -23,7 +23,10 @@ import { warnOnce } from './worktree-listing-diagnostics' import type { WorktreeIpcContext } from '../worktree-ipc-context' -import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' +import { + readAllWorktreeMetaForHost, + readAllWorktreeMetaForRepo +} from '../../../persistence/host-qualified-worktree-meta' import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' const WORKTREE_LIST_ALL_CONCURRENCY = 8 @@ -174,9 +177,7 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo if (!repo) { return [] } - const allMeta = repo.connectionId - ? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) - : undefined + const allMeta = repo.connectionId ? readAllWorktreeMetaForRepo(store, repo) : undefined const sshWorktreeMetaIndex = repo.connectionId ? createSshWorktreeMetaIndex(Object.entries(allMeta ?? {})) : new Map() @@ -226,7 +227,7 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo }) } loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) - const metadata = allMeta ?? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) + const metadata = allMeta ?? readAllWorktreeMetaForRepo(store, repo) return buildDetectedGitWorktrees(store, repo, gitWorktrees, metadata) .filter((worktree) => worktree.visible) .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata)) diff --git a/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts b/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts index 5c734d8bcd9..ecf8ab3abc1 100644 --- a/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts +++ b/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts @@ -9,7 +9,7 @@ import type { GitWorktreeInfo, DetectedWorktree, Worktree } from '../../../../sh import type { Store } from '../../../persistence/loading-store/store' import { getRepoExecutionHostId } from '../../../../shared/execution-host' import { - readWorktreeMetaForHost, + readWorktreeMetaForRepo, writeWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' import { getRepoOwnedWorktreeMeta } from '../../../worktree-metadata-ownership' @@ -159,7 +159,7 @@ export function buildDetectedGitWorktrees( const legacyMeta = allMeta === undefined ? store.getWorktreeMeta?.(worktreeId) : undefined const metaById = allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {}) const meta = - readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo)) ?? + readWorktreeMetaForRepo(store, worktreeId, repo) ?? getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) const worktree = mergeWorktree(repo.id, gitWorktree, meta, repo.displayName) const detected = toDetectedWorktree({ diff --git a/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts b/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts index b17677ddfcc..3edb8efc1d7 100644 --- a/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts +++ b/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts @@ -4,7 +4,7 @@ import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' import { getProjectHostSetupWorktreeMeta } from '../../../../shared/project-host-setup-lookup' import { getRepoExecutionHostId } from '../../../../shared/execution-host' import { - readWorktreeMetaForHost, + readWorktreeMetaForRepo, writeWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' import { getRepoOwnedWorktreeMeta } from '../../../worktree-metadata-ownership' @@ -44,7 +44,7 @@ export function resolveWorktreeMetaWithDiscoveryBackfill( // Why: the locator-keyed row is only a stand-in for a missing snapshot, so don't read it when we have one. const legacyMeta = allMeta === undefined ? store.getWorktreeMeta?.(worktreeId) : undefined const existing = - readWorktreeMetaForHost(store, worktreeId, executionHostId) ?? + readWorktreeMetaForRepo(store, worktreeId, repo) ?? getRepoOwnedWorktreeMeta( repo, worktreeId, diff --git a/src/main/persistence/host-qualified-worktree-meta.ts b/src/main/persistence/host-qualified-worktree-meta.ts index a9d1c0e8fb1..6267311f471 100644 --- a/src/main/persistence/host-qualified-worktree-meta.ts +++ b/src/main/persistence/host-qualified-worktree-meta.ts @@ -1,4 +1,5 @@ -import type { ExecutionHostId } from '../../shared/execution-host' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' +import type { Repo } from '../../shared/repo-types' import type { WorktreeMeta } from '../../shared/worktree/meta-types' /** @@ -52,6 +53,26 @@ export function readWorktreeMetaForHost( return store.getWorktreeMetaForHost?.(worktreeId, executionHostId) } +/** + * The same two reads keyed off a repo row, so the resolve-then-read pair lives in one place. Four + * call sites had open-coded it identically, which is the shape that lets one copy drift from the + * rest (F7/F8). + */ +export function readAllWorktreeMetaForRepo( + store: Pick, + repo: Pick +): Record { + return readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) +} + +export function readWorktreeMetaForRepo( + store: Pick, + worktreeId: string, + repo: Pick +): WorktreeMeta | undefined { + return readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo)) +} + export function writeWorktreeMetaForHost( store: Pick, worktreeId: string, diff --git a/src/main/runtime/worktree-launch-host-repo.ts b/src/main/runtime/worktree-launch-host-repo.ts index 4decb7acb56..7db9f4dae18 100644 --- a/src/main/runtime/worktree-launch-host-repo.ts +++ b/src/main/runtime/worktree-launch-host-repo.ts @@ -17,7 +17,10 @@ export type WorktreeHostRouting = | { kind: 'resolved'; hostId: ExecutionHostId; repo: T | null } /** No row carries this repo id and the worktree names no host — nothing ever named a host. */ | { kind: 'unowned' } - /** Rival rows disagree about the host; guessing one is the cross-host leak. */ + /** + * No single trustworthy host: rival rows disagree, or the resolved row named one that cannot be + * parsed. Guessing is the cross-host leak in both cases. + */ | { kind: 'ambiguous' } /** @@ -33,7 +36,10 @@ export function resolveWorktreeHostRouting { const resolution = resolveWorktreeExecutionHost(createRepoRowExecutionHostLookup(repos), worktree) if (resolution.kind === 'unresolved') { - return resolution.reason === 'ambiguous' ? { kind: 'ambiguous' } : { kind: 'unowned' } + // Only `unknown` — nothing anywhere carries the id — becomes `unowned`, which callers dispose of + // as a plain local folder. `malformed` is a row that declared a host and named an unparseable + // one, so it joins `ambiguous`: guessing is the cross-host leak either way. + return resolution.reason === 'unknown' ? { kind: 'unowned' } : { kind: 'ambiguous' } } return { kind: 'resolved', hostId: resolution.hostId, repo: resolution.owner } } diff --git a/src/shared/execution-host.test.ts b/src/shared/execution-host.test.ts index 9905fc5fe2a..fba1d1fee8d 100644 --- a/src/shared/execution-host.test.ts +++ b/src/shared/execution-host.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { ALL_EXECUTION_HOSTS_SCOPE, LOCAL_EXECUTION_HOST_ID, + getExecutionHostLabel, getLocalExecutionHostLabel, getRepoExecutionHostId, getRepoSshConnectionId, @@ -169,4 +170,16 @@ describe('execution host id delimiter invariant', () => { targetId: 'a|b' }) }) + + // "All hosts" is the everything-scope. Answering with it for an id that names no host shows one + // unroutable row as though it were on every host, which is the opposite of what it is. + it('labels an id that names no host as one unknown host, not as every host', () => { + for (const id of ['ssh:', 'ssh:a|b', 'ssh:%zz', 'runtime:', 'quantum:box'] as const) { + expect(getExecutionHostLabel(id as never)).toBe('Unknown host') + } + expect(getExecutionHostLabel(null)).toBe('Unknown host') + expect(getExecutionHostLabel(ALL_EXECUTION_HOSTS_SCOPE)).toBe('All hosts') + expect(getExecutionHostLabel('ssh:box')).toBe('box') + expect(getExecutionHostLabel('runtime:env-1')).toBe('env-1') + }) }) diff --git a/src/shared/execution-host.ts b/src/shared/execution-host.ts index a77d02b3882..bbe55aea1a0 100644 --- a/src/shared/execution-host.ts +++ b/src/shared/execution-host.ts @@ -226,13 +226,18 @@ export function getSettingsFocusedExecutionHostId( : LOCAL_EXECUTION_HOST_ID } -export function getExecutionHostLabel(id: ExecutionHostScope): string { +export function getExecutionHostLabel(id: ExecutionHostScope | null | undefined): string { if (id === ALL_EXECUTION_HOSTS_SCOPE) { return 'All hosts' } const parsed = parseExecutionHostId(id) if (!parsed) { - return 'All hosts' + // Not "All hosts": an id that names no host is one *unknown* host, and answering with the + // everything-scope label shows an unroutable row as though it were on every host. + // Plain English like every other label in this module (`Local Mac`, `This computer`, + // `All hosts`) — none of them resolve through the renderer's i18n catalog, so a lone + // translated string here would read inconsistently. + return 'Unknown host' } switch (parsed.kind) { case 'local': diff --git a/src/shared/folder-workspace-execution-host.test.ts b/src/shared/folder-workspace-execution-host.test.ts index 2ccced49907..40351788031 100644 --- a/src/shared/folder-workspace-execution-host.test.ts +++ b/src/shared/folder-workspace-execution-host.test.ts @@ -174,6 +174,116 @@ describe('folder workspace execution host', () => { expect(resolveFolderWorkspaceHost(state({ repos: [] }), 'fw-1')).toEqual({ kind: 'local' }) }) + // SSH ownership has two spellings on a repo row. A row carrying only `executionHostId: 'ssh:*'` + // has no `connectionId`, and reading the raw field counted it as a local repo — so a workspace + // whose files live on an SSH host resolved `local`, which is an execute-here answer for a remote + // path. These fire on well-formed rows; nothing malformed is involved. + it('resolves a repo that names its SSH host only through executionHostId', () => { + const resolved = resolveFolderWorkspaceHost( + state({ + repos: [ + repo({ + id: 'repo-1', + path: '/work/app/a', + projectGroupId: 'group-1', + executionHostId: 'ssh:box' + }) + ] + }), + 'fw-1' + ) + + expect(resolved).toEqual({ kind: 'ssh', targetId: 'box' }) + }) + + it('mixes such a repo with a local one as ambiguous rather than local', () => { + const resolved = resolveFolderWorkspaceHost( + state({ + repos: [ + repo({ id: 'repo-1', path: '/work/app/a', projectGroupId: 'group-1' }), + repo({ + id: 'repo-2', + path: '/work/app/b', + projectGroupId: 'group-1', + executionHostId: 'ssh:box' + }) + ] + }), + 'fw-1' + ) + + expect(resolved).toEqual({ kind: 'ambiguous' }) + }) + + it('matches a scope connection against such a repo instead of calling it ambiguous', () => { + const resolved = resolveFolderWorkspaceHost( + state({ + folderWorkspaces: [workspace({ connectionId: 'box' })], + repos: [ + repo({ + id: 'repo-1', + path: '/work/app/a', + projectGroupId: 'group-1', + executionHostId: 'ssh:box' + }) + ] + }), + 'fw-1' + ) + + expect(resolved).toEqual({ kind: 'ssh', targetId: 'box' }) + }) + + it('reads the target off the host, so a percent-encoded id decodes', () => { + const resolved = resolveFolderWorkspaceHost( + state({ + repos: [ + repo({ + id: 'repo-1', + path: '/work/app/a', + projectGroupId: 'group-1', + executionHostId: `ssh:${encodeURIComponent('box 1')}` + }) + ] + }), + 'fw-1' + ) + + expect(resolved).toEqual({ kind: 'ssh', targetId: 'box 1' }) + }) + + // Deliberately unchanged: a `runtime:` row's nested SSH target is not this client's to dial, but + // narrowing that here would be a second behaviour change riding on the SSH fix. + it('leaves a runtime row contributing its nested connection exactly as before', () => { + const resolved = resolveFolderWorkspaceHost( + state({ + repos: [ + repo({ + id: 'repo-1', + path: '/work/app/a', + projectGroupId: 'group-1', + executionHostId: 'runtime:env-1', + connectionId: 'nested-box' + }) + ] + }), + 'fw-1' + ) + + expect(resolved).toEqual({ kind: 'ssh', targetId: 'nested-box' }) + }) + + it('still answers local for a runtime pin, which the type cannot express otherwise', () => { + const resolved = resolveFolderWorkspaceHost( + state({ + folderWorkspaces: [workspace({ executionHostId: 'runtime:env-1' })] + }), + 'fw-1' + ) + + expect(resolved).toEqual({ kind: 'local' }) + }) + it('reads each repository membership once while collecting candidates', () => { let membershipReads = 0 const repos = Array.from({ length: 32 }, (_, index) => { diff --git a/src/shared/folder-workspace-execution-host.ts b/src/shared/folder-workspace-execution-host.ts index dc4dc80c51f..c2f44f0a4b0 100644 --- a/src/shared/folder-workspace-execution-host.ts +++ b/src/shared/folder-workspace-execution-host.ts @@ -17,7 +17,7 @@ import type { ProjectGroup } from './project-group-types' import type { Repo } from './repo-types' import { isPathInsideOrEqual } from './cross-platform-path' import { getProjectGroupSubtreeIds } from './project-groups' -import { parseExecutionHostId } from './execution-host' +import { getRepoExecutionHostId, parseExecutionHostId } from './execution-host' export type FolderWorkspaceHostState = { folderWorkspaces: readonly FolderWorkspace[] @@ -102,6 +102,12 @@ export function resolveFolderWorkspaceHost( } const explicitHost = parseExecutionHostId(workspace.executionHostId) if (explicitHost) { + // A `runtime:` workspace deliberately answers `local`, and `FolderWorkspaceHost` has no runtime + // variant to answer with instead. That omission is known: a runtime environment's own server + // normalizes its work to `local`, and the nested SSH target on such a row is addressable only as + // the pair (environmentId, targetId) — handing it to this client's SSH table would dial a + // same-named box in the wrong namespace. Widening the type is its own change, not an oversight + // here. return explicitHost.kind === 'ssh' ? { kind: 'ssh', targetId: explicitHost.targetId } : { kind: 'local' } @@ -114,7 +120,17 @@ export function resolveFolderWorkspaceHost( let hasLocalRepo = false const connectionIds = new Set() for (const repo of candidateRepos) { - const connectionId = normalizeConnectionId(repo.connectionId) + // Why not `repo.connectionId` alone: SSH ownership has two spellings on a repo row, and a row + // carrying only `executionHostId: 'ssh:'` has no `connectionId` to read. Reading the raw + // field counted it as a local repo, so a folder workspace whose files live on an SSH host + // resolved `local` — an execute-here answer for a remote path (#11163). + // + // Resolve the host first, then read the target off it. Every other row keeps its existing + // contribution, including a `runtime:` row's nested target: that is not this client's to dial, + // but narrowing it here would be a second behaviour change riding on this one. + const host = parseExecutionHostId(getRepoExecutionHostId(repo)) + const connectionId = + host?.kind === 'ssh' ? host.targetId : normalizeConnectionId(repo.connectionId) if (connectionId) { connectionIds.add(connectionId) } else { diff --git a/src/shared/worktree-execution-host-resolution.test.ts b/src/shared/worktree-execution-host-resolution.test.ts index b82cea476eb..b31281fe925 100644 --- a/src/shared/worktree-execution-host-resolution.test.ts +++ b/src/shared/worktree-execution-host-resolution.test.ts @@ -156,6 +156,40 @@ describe('resolveWorktreeExecutionHost', () => { it('reports an unknown owner distinctly from a conflicting one', () => { expect(resolve([], { repoId: 'r' })).toEqual({ kind: 'unresolved', reason: 'unknown' }) }) + + // `unknown` is a verdict the launch path disposes of as a plain local folder, so a row that + // declared a host and named an unparseable one must not share the word — it has to fail closed. + it('reports a row naming an unparseable host distinctly from an unknown one', () => { + for (const executionHostId of ['ssh:', 'ssh:a|b', 'ssh:%zz', 'runtime:', 'quantum:box']) { + expect(resolve([{ id: 'r', executionHostId }], { repoId: 'r' })).toEqual({ + kind: 'unresolved', + reason: 'malformed' + }) + } + }) + + it('does not recover a host from the connectionId such a row overrode', () => { + expect( + resolve([{ id: 'r', executionHostId: 'ssh:a|b', connectionId: 'openclaw' }], { + repoId: 'r' + }) + ).toEqual({ kind: 'unresolved', reason: 'malformed' }) + }) + + it('still resolves every row that names a parseable host', () => { + expect(resolve([{ id: 'r', executionHostId: 'ssh:box' }], { repoId: 'r' })).toMatchObject({ + kind: 'resolved', + hostId: 'ssh:box' + }) + expect(resolve([{ id: 'r', connectionId: 'box' }], { repoId: 'r' })).toMatchObject({ + kind: 'resolved', + hostId: 'ssh:box' + }) + expect(resolve([{ id: 'r' }], { repoId: 'r' })).toMatchObject({ + kind: 'resolved', + hostId: 'local' + }) + }) }) it('ignores an unparseable host id rather than treating it as a host', () => { diff --git a/src/shared/worktree-execution-host-resolution.ts b/src/shared/worktree-execution-host-resolution.ts index 00b66b7f11c..0172106aee0 100644 --- a/src/shared/worktree-execution-host-resolution.ts +++ b/src/shared/worktree-execution-host-resolution.ts @@ -54,7 +54,29 @@ export type WorktreeExecutionHostResolution = /** Display metadata only. The decisions are `hostId` / `connectionId`. */ owner: T | null } - | { kind: 'unresolved'; reason: 'ambiguous' | 'unknown' } + /** + * Three reasons, not two, and deliberately not collapsed. `unknown` (nothing carries the id) is a + * verdict the launch path may legitimately dispose of as a plain local folder; `malformed` (the + * row named a host that cannot be parsed) must fail closed. A vocabulary that cannot express the + * difference guarantees it is lost at the first caller that switches on it — the same shape as + * #18006, where one word had to stand for two liveness situations. + */ + | { kind: 'unresolved'; reason: 'ambiguous' | 'unknown' | 'malformed' } + +/** + * The owner row's host, or `null` when the row names one that cannot be parsed. + * + * Module-private and deliberately not a second exported reading of a repo row: only this resolution + * needs the distinction, because only this resolution is routing. `getRepoExecutionHostId` stays the + * answer everywhere else — its fall-through to `local` is harmless for the grouping, label and index + * callers that make up nearly all of its ~340 call sites, and is wrong only when the value decides + * where work runs. + */ +function resolveOwnerRowHostId(row: ExecutionHostOwnerRow): ExecutionHostId | null { + return row.executionHostId?.trim() + ? normalizeExecutionHostId(row.executionHostId) + : getRepoExecutionHostId(row) +} export function resolveWorktreeExecutionHost( lookup: ExecutionHostOwnerLookup, @@ -80,9 +102,13 @@ export function resolveWorktreeExecutionHost( if (match.kind !== 'resolved') { return { kind: 'unresolved', reason: match.kind === 'ambiguous' ? 'ambiguous' : 'unknown' } } + const hostId = resolveOwnerRowHostId(match.owner) + if (!hostId) { + return { kind: 'unresolved', reason: 'malformed' } + } return { kind: 'resolved', - hostId: getRepoExecutionHostId(match.owner), + hostId, connectionId: getRepoSshConnectionId(match.owner), owner: match.owner } @@ -115,12 +141,14 @@ export function createRepoRowExecutionHostLookup getRepoExecutionHostId(repo) !== ownerHostId) + const ownerHostId = resolveOwnerRowHostId(owner) + return rows.some((repo) => resolveOwnerRowHostId(repo) !== ownerHostId) ? { kind: 'ambiguous' } : { kind: 'resolved', owner } }, + // A row naming an unparseable host matches no host, which is what stops a worktree on a real + // host from adopting it. byHost: (repoId, hostId) => - rowsFor(repoId).find((repo) => getRepoExecutionHostId(repo) === hostId) ?? null + rowsFor(repoId).find((repo) => resolveOwnerRowHostId(repo) === hostId) ?? null } }