From 05bc9aab882f9074f98d401962272e2ec2eca293 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:28:14 -0700 Subject: [PATCH] fix(host-routing): resolve both sides of the execution host through one rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renderer resolver leaked between two different SSH hosts: a worktree on `ssh:m4air` whose only indexed repo row belonged to `openclaw` answered 'openclaw', because the host-scoped lookup missing fell through to an id-only one. Main's resolver, in the same change, answered 'm4air' — two resolvers, one right and one wrong, on identical input. Both sides now adapt one shared rule (`worktree-execution-host-resolution.ts`): the worktree's own host outranks every repo row, and a row on a different host is never evidence about this one. The renderer's WeakMap index becomes the memoizing adapter it always was; `resolveWorktreeLaunchHost` becomes main's mapping of unresolved onto its throw. Settles the rule the change previously answered two ways. `getRepoSshConnectionId` and `getSshTargetIdForExecutionHost` disagreed for a runtime host carrying a nested `connectionId`; they now compose, so the execution host is the single authority. On a `runtime:*` row that field is a paired HUB's private SSH target, spread through by `repoWithFetchedOwner` and unaddressable from this client — the project-first successor of the row nulls it for exactly that reason. That also fixes the `kind !== 'ssh'` fallback, which fired for `local`: a row declaring itself local handed out an SSH connection. --- .../runtime/worktree-launch-host-repo.test.ts | 46 ++++- src/main/runtime/worktree-launch-host-repo.ts | 59 +++---- .../src/lib/connection-context.test.ts | 117 ++++++++++++ .../src/lib/connection-owner-resolution.ts | 29 ++- src/shared/execution-host.test.ts | 37 ++++ src/shared/execution-host.ts | 49 +++-- ...worktree-execution-host-resolution.test.ts | 167 ++++++++++++++++++ .../worktree-execution-host-resolution.ts | 109 ++++++++++++ 8 files changed, 537 insertions(+), 76 deletions(-) create mode 100644 src/shared/worktree-execution-host-resolution.test.ts create mode 100644 src/shared/worktree-execution-host-resolution.ts diff --git a/src/main/runtime/worktree-launch-host-repo.test.ts b/src/main/runtime/worktree-launch-host-repo.test.ts index 2de36bb7b4c..8a81f2424d1 100644 --- a/src/main/runtime/worktree-launch-host-repo.test.ts +++ b/src/main/runtime/worktree-launch-host-repo.test.ts @@ -24,16 +24,52 @@ describe('resolveWorktreeLaunchHost', () => { ).toEqual({ kind: 'resolved', repo: localRow, connectionId: null }) }) - it('never hands a runtime-owned worktree a client SSH connection', () => { + // The settled rule: the execution host is authoritative, and a row on some *other* host is never + // evidence about this one — not for the connection, and not for the metadata row either. This is + // the question `getRepoSshConnectionId` and `getSshTargetIdForExecutionHost` once answered two + // ways; they now compose, and `execution-host.test.ts` pins the composition. + it('never hands a worktree a connection belonging to a different host', () => { const clientOwnedRow = { id: 'r', path: '/p', connectionId: 'ssh-client' } expect( resolveWorktreeLaunchHost([clientOwnedRow], { repoId: 'r', hostId: 'runtime:env-a' }) - ).toEqual({ kind: 'resolved', repo: clientOwnedRow, connectionId: null }) - // Two rows and no match: nothing names the owner, so the row is not evidence either. + ).toEqual({ kind: 'resolved', repo: null, connectionId: null }) + // Even the runtime host's *own* row contributes no PTY route: its nested target lives in that + // machine's namespace, so spawning against it here would dial the wrong box. The renderer + // reads the same resolution and does want that id — see execution-host.test.ts. + const nestedRow = { + id: 'r', + path: '/p', + connectionId: 'ssh-nested', + executionHostId: 'runtime:env-a' as const + } expect( - resolveWorktreeLaunchHost([clientOwnedRow, { id: 'r', path: '/q' }], { + resolveWorktreeLaunchHost([nestedRow], { repoId: 'r', hostId: 'runtime:env-a' }) + ).toEqual({ kind: 'resolved', repo: nestedRow, connectionId: null }) + // Two SSH hosts, one shared repo id: the worktree's own host wins outright. + expect( + resolveWorktreeLaunchHost([{ id: 'r', path: '/p', connectionId: 'openclaw' }], { repoId: 'r', - hostId: 'runtime:env-a' + hostId: 'ssh:m4air' + }) + ).toEqual({ kind: 'resolved', repo: null, connectionId: 'm4air' }) + expect( + resolveWorktreeLaunchHost( + [ + { id: 'r', path: '/p', connectionId: 'openclaw' }, + { id: 'r', path: '/q', connectionId: 'm4air' } + ], + { repoId: 'r', hostId: 'ssh:m4air' } + ) + ).toEqual({ + kind: 'resolved', + repo: { id: 'r', path: '/q', connectionId: 'm4air' }, + connectionId: 'm4air' + }) + // A row declaring itself local hands out no SSH connection, whatever `connectionId` says. + expect( + resolveWorktreeLaunchHost([{ id: 'r', path: '/p', connectionId: 'openclaw' }], { + repoId: 'r', + hostId: 'local' }) ).toEqual({ kind: 'resolved', repo: null, connectionId: null }) }) diff --git a/src/main/runtime/worktree-launch-host-repo.ts b/src/main/runtime/worktree-launch-host-repo.ts index bb78a48c0e0..fa2641655b3 100644 --- a/src/main/runtime/worktree-launch-host-repo.ts +++ b/src/main/runtime/worktree-launch-host-repo.ts @@ -1,10 +1,9 @@ import { - LOCAL_EXECUTION_HOST_ID, - getRepoExecutionHostId, - getSshTargetIdForExecutionHost, - normalizeExecutionHostId, - type ExecutionHostId -} from '../../shared/execution-host' + createRepoRowExecutionHostLookup, + resolveWorktreeExecutionHost, + type ExecutionHostOwnerRow +} from '../../shared/worktree-execution-host-resolution' +import { getSshTargetIdForExecutionHost } from '../../shared/execution-host' import type { Repo } from '../../shared/repo-types' export type LaunchHostRepo = Pick @@ -14,41 +13,31 @@ export type WorktreeLaunchHostResolution = | { kind: 'ambiguous' } /** - * Pick the repo row that owns a worktree's execution, and read the SSH connection off the - * resolved host rather than off whichever row an id-only lookup happened to return. + * Main-side adapter over the shared execution-host rule + * (`src/shared/worktree-execution-host-resolution.ts`), which the renderer's owner index answers + * with too. Two things are local to this side: * - * The same repo id can exist on local, SSH and runtime hosts at once — `setResolvedRepoGitUsername` - * already refuses id-only lookups for that reason. A host-blind `getRepo(id)` can hand a remote - * worktree the local row, whose `connectionId` reads `null`, and the PTY then spawns on the client - * with the remote cwd (#11163). Conflicting rows with nothing to disambiguate them resolve - * `ambiguous`, never "local". + * - rival rows that disagree about the host are `ambiguous` and the launch scope throws, while an + * id nobody carries stays "no repo, no connection" — the launch path's long-standing behaviour + * for a worktree whose repo row has gone; + * - the connection comes off the *host*, not the resolved row. This is a client-dialable PTY + * route, so a `runtime:` host contributes nothing: its nested SSH target belongs to that + * machine's namespace and spawning against it here would dial the wrong box. The renderer wants + * the opposite answer from the same resolution, which is why the shared type carries both. */ -export function resolveWorktreeLaunchHost( +export function resolveWorktreeLaunchHost( repos: readonly T[], - worktree: { repoId: string; hostId?: ExecutionHostId | null } + worktree: { repoId: string; hostId?: string | null } ): WorktreeLaunchHostResolution { - const rows = repos.filter((repo) => repo.id === worktree.repoId) - const worktreeHostId = normalizeExecutionHostId(worktree.hostId) - if (worktreeHostId) { - // The worktree names its own host, which outranks the repo fallback. - const match = rows.find((repo) => getRepoExecutionHostId(repo) === worktreeHostId) - return { - kind: 'resolved', - repo: match ?? (rows.length === 1 ? (rows[0] ?? null) : null), - connectionId: getSshTargetIdForExecutionHost(worktreeHostId) - } + const resolution = resolveWorktreeExecutionHost(createRepoRowExecutionHostLookup(repos), worktree) + if (resolution.kind === 'unresolved') { + return resolution.reason === 'ambiguous' + ? { kind: 'ambiguous' } + : { kind: 'resolved', repo: null, connectionId: null } } - if (rows.length === 0) { - return { kind: 'resolved', repo: null, connectionId: null } - } - const hostIds = new Set(rows.map((repo) => getRepoExecutionHostId(repo))) - if (hostIds.size > 1) { - return { kind: 'ambiguous' } - } - const hostId = [...hostIds][0] ?? LOCAL_EXECUTION_HOST_ID return { kind: 'resolved', - repo: rows[0] ?? null, - connectionId: getSshTargetIdForExecutionHost(hostId) + repo: resolution.owner, + connectionId: getSshTargetIdForExecutionHost(resolution.hostId) } } diff --git a/src/renderer/src/lib/connection-context.test.ts b/src/renderer/src/lib/connection-context.test.ts index 45eeeb00728..fae6c74cdd8 100644 --- a/src/renderer/src/lib/connection-context.test.ts +++ b/src/renderer/src/lib/connection-context.test.ts @@ -27,6 +27,27 @@ function makeRepo(overrides: Partial & { id: string }): Repo { } } +function makeWorktree(overrides: Partial & { id: string; repoId: string }): Worktree { + return { + path: '/srv/repo', + head: 'abc123', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: false, + displayName: 'Workspace', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0, + ...overrides + } +} + describe('getConnectionId', () => { afterEach(() => { useAppStore.setState(initialState, true) @@ -555,6 +576,102 @@ describe('getConnectionIdFromState', () => { expect(getConnectionIdFromState(state, 'repo-dup::/home/neil/repo-feature')).toBe('ssh-same') }) + it('never hands a worktree the SSH connection of a different host', () => { + // Why (#11163): two SSH hosts, one shared repo id. The worktree names `ssh:m4air`; the only + // indexed row belongs to `openclaw`. An id-only fallback after the host lookup misses answers + // with the wrong host's connection — "Reconnect openclaw" on an m4air pane, and file reads + // routed to a machine that never held the path. + const state: ConnectionContextState = { + folderWorkspaces: [], + projectGroups: [], + repos: [makeRepo({ id: 'repo-shared', connectionId: 'openclaw' })], + worktreesByRepo: { + 'repo-shared': [ + makeWorktree({ + id: 'repo-shared::/srv/repo', + repoId: 'repo-shared', + hostId: 'ssh:m4air' + }) + ] + } + } + + expect(getConnectionIdFromState(state, 'repo-shared::/srv/repo')).toBe('m4air') + }) + + it('never hands a runtime-hosted worktree a client-owned SSH connection', () => { + // The row is on `ssh:openclaw`, not on the runtime host, so it says nothing about this + // worktree. This is the cross-host case, not the nested-SSH one below. + const state: ConnectionContextState = { + folderWorkspaces: [], + projectGroups: [], + repos: [makeRepo({ id: 'repo-shared', connectionId: 'openclaw' })], + worktreesByRepo: { + 'repo-shared': [ + makeWorktree({ + id: 'repo-shared::/srv/repo', + repoId: 'repo-shared', + hostId: 'runtime:awin' + }) + ] + } + } + + expect(getConnectionIdFromState(state, 'repo-shared::/srv/repo')).toBeNull() + }) + + it('keeps a runtime host nested SSH target, which decides local readability', () => { + // `repoWithFetchedOwner` stamps the runtime host and spreads the nested target through. The + // pane pairs it with the environment (`selectRuntimeAwareSshStatus`) for reconnect state, and + // `isNativeChatTranscriptLocalReadable` treats a null here as "this client can read it" — so + // dropping it would send a transcript read to the wrong machine. + const state: ConnectionContextState = { + folderWorkspaces: [], + projectGroups: [], + repos: [ + makeRepo({ + id: 'repo-runtime', + connectionId: 'ssh-nested', + executionHostId: 'runtime:env-a' + }) + ], + worktreesByRepo: { + 'repo-runtime': [ + makeWorktree({ + id: 'repo-runtime::/srv/repo', + repoId: 'repo-runtime', + hostId: 'runtime:env-a', + runtimeOwnerEnvironmentId: 'env-a' + }) + ] + } + } + + expect(getConnectionIdFromState(state, 'repo-runtime::/srv/repo')).toBe('ssh-nested') + }) + + it('resolves the row on the SSH host the worktree names when both hosts carry the id', () => { + const state: ConnectionContextState = { + folderWorkspaces: [], + projectGroups: [], + repos: [ + makeRepo({ id: 'repo-shared', connectionId: 'openclaw' }), + makeRepo({ id: 'repo-shared', connectionId: 'm4air', path: '/srv/repo' }) + ], + worktreesByRepo: { + 'repo-shared': [ + makeWorktree({ + id: 'repo-shared::/srv/repo', + repoId: 'repo-shared', + hostId: 'ssh:m4air' + }) + ] + } + } + + expect(getConnectionIdFromState(state, 'repo-shared::/srv/repo')).toBe('m4air') + }) + it('indexes immutable worktree and repo snapshots once across repeated selector calls', () => { let worktreeIdReads = 0 let repoIdReads = 0 diff --git a/src/renderer/src/lib/connection-owner-resolution.ts b/src/renderer/src/lib/connection-owner-resolution.ts index 148c0993cde..f1b52607d46 100644 --- a/src/renderer/src/lib/connection-owner-resolution.ts +++ b/src/renderer/src/lib/connection-owner-resolution.ts @@ -4,7 +4,7 @@ import { resolveIndexedRepoOwner, resolveIndexedWorktreeOwner } from './worktree-runtime-owner-index' -import { getRepoSshConnectionId, normalizeExecutionHostId } from '../../../shared/execution-host' +import { resolveWorktreeExecutionHost } from '../../../shared/worktree-execution-host-resolution' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' import { parseWorkspaceKey } from '../../../shared/workspace-scope' @@ -74,23 +74,16 @@ export function getConnectionIdFromState( } const worktree = worktreeResolution.kind === 'resolved' ? worktreeResolution.owner : undefined const repoId = worktree?.repoId ?? getRepoIdFromWorktreeId(worktreeId) - // Why (#17799): take the connection off the same host record the rest of owner - // resolution used. A host-blind, last-wins repo lookup can pair a runtime owner - // with a client-owned SSH connection the runtime has never heard of. - const worktreeHostId = normalizeExecutionHostId(worktree?.hostId) - const hostScopedRepo = worktreeHostId - ? findIndexedRepoOwnerForHost(state.repos, repoId, worktreeHostId) - : null - if (hostScopedRepo) { - return getRepoSshConnectionId(hostScopedRepo) - } - const repoResolution = resolveIndexedRepoOwner(state.repos, repoId) - if (repoResolution.kind === 'ambiguous') { - return undefined - } - return repoResolution.kind === 'resolved' - ? getRepoSshConnectionId(repoResolution.owner) - : undefined + // Why (#17799, #11163): one rule, shared with main's launch scope. The renderer's contribution is + // only the memoized index — unrelated store writes must not rescan every repository. + const resolution = resolveWorktreeExecutionHost( + { + byId: (id) => resolveIndexedRepoOwner(state.repos, id), + byHost: (id, hostId) => findIndexedRepoOwnerForHost(state.repos, id, hostId) + }, + { repoId, hostId: worktree?.hostId ?? null } + ) + return resolution.kind === 'resolved' ? resolution.connectionId : undefined } export function getConnectionIdForFileFromState( diff --git a/src/shared/execution-host.test.ts b/src/shared/execution-host.test.ts index de895978f3b..9905fc5fe2a 100644 --- a/src/shared/execution-host.test.ts +++ b/src/shared/execution-host.test.ts @@ -4,7 +4,9 @@ import { LOCAL_EXECUTION_HOST_ID, getLocalExecutionHostLabel, getRepoExecutionHostId, + getRepoSshConnectionId, getSettingsFocusedExecutionHostId, + getSshTargetIdForExecutionHost, getWorktreeExecutionHostId, normalizeExecutionHostOrder, normalizeExecutionHostScope, @@ -113,6 +115,41 @@ describe('execution host identity', () => { expect(getWorktreeExecutionHostId({}, {}, 'runtime:focused-host')).toBe('runtime:focused-host') }) + // These two look interchangeable and are not: one answers "which SSH target holds this row's + // files", the other "which connection may this client dial". They agree except on a runtime + // host, where a nested target exists but is not dialable from here — so the pane that reads it + // needs one answer and the PTY route needs the other. + it('distinguishes the SSH target holding a row from the connection this client may dial', () => { + // Legacy spelling: `connectionId` alone *is* the host, so both answers agree. + expect(getRepoSshConnectionId({ connectionId: 'openclaw' })).toBe('openclaw') + expect(getSshTargetIdForExecutionHost('ssh:openclaw')).toBe('openclaw') + // Unified spelling, no legacy field. + expect(getRepoSshConnectionId({ executionHostId: 'ssh:m4air' })).toBe('m4air') + + // A row declaring itself local hands out no SSH connection, whatever the legacy field says: + // `local` has no SSH namespace to nest in, so the two spellings are contradicting each other. + expect( + getRepoSshConnectionId({ executionHostId: 'local', connectionId: 'openclaw' }) + ).toBeNull() + + // A runtime host does have its own namespace, and a nested target appears only in this field. + // Dropping it would make a nested-SSH workspace read as local — which is what decides whether + // this client tries to read the transcript itself. + expect( + getRepoSshConnectionId({ executionHostId: 'runtime:env-a', connectionId: 'ssh-nested' }) + ).toBe('ssh-nested') + // ...but that id is not dialable from this client alone, so the routing answer stays null. + expect(getSshTargetIdForExecutionHost('runtime:env-a')).toBeNull() + // A runtime host with no nested target is simply not on SSH. + expect(getRepoSshConnectionId({ executionHostId: 'runtime:env-a' })).toBeNull() + + // An ephemeral-VM target is an ordinary client-dialable target and stays an `ssh:` host. + expect(getRepoSshConnectionId({ connectionId: 'runtime-ssh-vm-1' })).toBe('runtime-ssh-vm-1') + expect(getRepoExecutionHostId({ connectionId: 'runtime-ssh-vm-1' })).toBe( + 'ssh:runtime-ssh-vm-1' + ) + }) + it('derives focused host compatibility from active runtime settings', () => { expect(getSettingsFocusedExecutionHostId(null)).toBe(LOCAL_EXECUTION_HOST_ID) expect(getSettingsFocusedExecutionHostId({ activeRuntimeEnvironmentId: 'runtime-1' })).toBe( diff --git a/src/shared/execution-host.ts b/src/shared/execution-host.ts index 2032bd9a511..a77d02b3882 100644 --- a/src/shared/execution-host.ts +++ b/src/shared/execution-host.ts @@ -166,24 +166,6 @@ export function getRepoExecutionHostId( return connectionId ? toSshExecutionHostId(connectionId) : LOCAL_EXECUTION_HOST_ID } -// Why: SSH ownership has two spellings on a repo row — the legacy `connectionId` -// field and the unified `executionHostId`. Routing that reads the raw field answers -// "local" for a row that only carries `ssh:`, which runs a remote operation -// on the client. Resolve the host first, then read the connection off it. -// -// Why the fallback: a row whose execution host is a *runtime* can still reach a nested -// SSH target, and that target only ever appears in `connectionId`. Returning null for -// those rows answers "local" for a nested-SSH worktree — the same defect in reverse. -export function getRepoSshConnectionId( - repo: Pick -): string | null { - const parsed = parseExecutionHostId(getRepoExecutionHostId(repo)) - if (parsed?.kind === 'ssh') { - return parsed.targetId - } - return normalizeHostPart(repo.connectionId) ?? null -} - export function getSshTargetIdForExecutionHost( executionHostId: string | null | undefined ): string | null { @@ -191,6 +173,37 @@ export function getSshTargetIdForExecutionHost( return parsed?.kind === 'ssh' ? parsed.targetId : null } +// Why: SSH ownership has two spellings on a repo row — the legacy `connectionId` +// field and the unified `executionHostId`. Routing that reads the raw field answers +// "local" for a row that only carries `ssh:`, which runs a remote operation +// on the client. Resolve the host first, then read the connection off it. +// +// The two hosts that are not themselves SSH are not the same case: +// +// - `local` has no SSH namespace to nest in, so a surviving `connectionId` is a row +// contradicting itself — the shape main's `resolveRepoOwnershipEvidence` calls +// `contradictory`. Answering with it hands out an SSH connection for a row that declares +// itself local. +// - `runtime:` is a different machine with its own SSH targets, and a nested one appears +// only in this field (`repoWithFetchedOwner` spreads it through). It is not dialable on its +// own, but it is addressable as the pair (environmentId, targetId) — which is how the +// renderer reads it, recovering the environment from the worktree and looking the target up +// inside it (`selectRuntimeAwareSshStatus`). Dropping it makes a nested-SSH workspace read +// as local, which is what decides whether a transcript is read on this client. +// +// So this answers "which SSH target holds this row's files", not "which connection may this +// client dial". `getSshTargetIdForExecutionHost` answers the latter; callers routing a +// client-local PTY or Git provider want that one instead. +export function getRepoSshConnectionId( + repo: Pick +): string | null { + const host = parseExecutionHostId(getRepoExecutionHostId(repo)) + if (host?.kind === 'ssh') { + return host.targetId + } + return host?.kind === 'runtime' ? normalizeHostPart(repo.connectionId) : null +} + export function getWorktreeExecutionHostId( worktree: Pick, repo: Pick | undefined, diff --git a/src/shared/worktree-execution-host-resolution.test.ts b/src/shared/worktree-execution-host-resolution.test.ts new file mode 100644 index 00000000000..70ee6d304b6 --- /dev/null +++ b/src/shared/worktree-execution-host-resolution.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it } from 'vitest' +import { + createRepoRowExecutionHostLookup, + resolveWorktreeExecutionHost +} from './worktree-execution-host-resolution' + +// Why (#11163, #17799): main's terminal launch scope and the renderer's owner index both answer +// "which host does this worktree execute on". They used to answer it separately, and disagreed — +// main derived the host from the worktree while the renderer fell back to an id-only repo lookup, +// so a pane on one SSH host was routed to another. One rule now, exercised here directly. +const resolve = ( + repos: readonly { id: string; connectionId?: string; executionHostId?: string }[], + worktree: { repoId: string; hostId?: string | null } +): ReturnType => + resolveWorktreeExecutionHost(createRepoRowExecutionHostLookup(repos as never), worktree) as never + +describe('resolveWorktreeExecutionHost', () => { + describe('the worktree names its own host', () => { + it('routes to that host even when the only row belongs to a different SSH host', () => { + // The reproduced defect: `ssh:m4air` worktree, sole row on `openclaw`. + expect( + resolve([{ id: 'r', connectionId: 'openclaw' }], { repoId: 'r', hostId: 'ssh:m4air' }) + ).toEqual({ kind: 'resolved', hostId: 'ssh:m4air', connectionId: 'm4air', owner: null }) + }) + + it('answers before the repo row hydrates, because the host is not a guess', () => { + // Deliberate change from "unresolved": #6648 blocks destructive ops while the *host* is + // unknown. A worktree naming `ssh:m4air` is not that case — the repo row adds nothing the + // host id has not already settled, and refusing here stalls a remote pane on hydration. + expect(resolve([], { repoId: 'r', hostId: 'ssh:m4air' })).toEqual({ + kind: 'resolved', + hostId: 'ssh:m4air', + connectionId: 'm4air', + owner: null + }) + }) + + it('picks the row on that host when both SSH hosts carry the id', () => { + const openclaw = { id: 'r', connectionId: 'openclaw' } + const m4air = { id: 'r', connectionId: 'm4air' } + expect(resolve([openclaw, m4air], { repoId: 'r', hostId: 'ssh:m4air' })).toEqual({ + kind: 'resolved', + hostId: 'ssh:m4air', + connectionId: 'm4air', + owner: m4air + }) + expect(resolve([openclaw, m4air], { repoId: 'r', hostId: 'ssh:openclaw' })).toEqual({ + kind: 'resolved', + hostId: 'ssh:openclaw', + connectionId: 'openclaw', + owner: openclaw + }) + }) + + it('matches a row that names the host in either spelling', () => { + const stamped = { id: 'r', executionHostId: 'ssh:m4air' } + expect(resolve([stamped], { repoId: 'r', hostId: 'ssh:m4air' })).toEqual({ + kind: 'resolved', + hostId: 'ssh:m4air', + connectionId: 'm4air', + owner: stamped + }) + }) + + it('takes no connection from a row on a different host, whatever this host is', () => { + // The row lives on `ssh:openclaw`; neither a local nor a runtime worktree may borrow it. + for (const hostId of ['local', 'runtime:env-a']) { + expect(resolve([{ id: 'r', connectionId: 'openclaw' }], { repoId: 'r', hostId })).toEqual({ + kind: 'resolved', + hostId, + connectionId: null, + owner: null + }) + } + }) + + it('reads a runtime host nested SSH target off the row on that same host', () => { + // Not a cross-host borrow: this row *is* the runtime host's row, and the nested target + // appears nowhere else. Nulling it makes the workspace read as local, which decides whether + // this client tries to read a transcript that lives on the nested host. + const nested = { id: 'r', connectionId: 'ssh-nested', executionHostId: 'runtime:env-a' } + expect(resolve([nested], { repoId: 'r', hostId: 'runtime:env-a' })).toEqual({ + kind: 'resolved', + hostId: 'runtime:env-a', + connectionId: 'ssh-nested', + owner: nested + }) + }) + + it('gives a local row no SSH connection even when it carries a stale one', () => { + const contradictory = { id: 'r', connectionId: 'openclaw', executionHostId: 'local' } + expect(resolve([contradictory], { repoId: 'r', hostId: 'local' })).toEqual({ + kind: 'resolved', + hostId: 'local', + connectionId: null, + owner: contradictory + }) + }) + }) + + describe('the worktree names no host', () => { + it('resolves from the sole row, in either spelling', () => { + const legacy = { id: 'r', connectionId: 'openclaw' } + expect(resolve([legacy], { repoId: 'r' })).toEqual({ + kind: 'resolved', + hostId: 'ssh:openclaw', + connectionId: 'openclaw', + owner: legacy + }) + const stamped = { id: 'r', executionHostId: 'ssh:m4air' } + expect(resolve([stamped], { repoId: 'r' })).toEqual({ + kind: 'resolved', + hostId: 'ssh:m4air', + connectionId: 'm4air', + owner: stamped + }) + const local = { id: 'r' } + expect(resolve([local], { repoId: 'r' })).toEqual({ + kind: 'resolved', + hostId: 'local', + connectionId: null, + owner: local + }) + }) + + it('refuses when rival rows disagree about the host, including two SSH hosts', () => { + expect( + resolve( + [ + { id: 'r', connectionId: 'openclaw' }, + { id: 'r', connectionId: 'm4air' } + ], + { + repoId: 'r' + } + ) + ).toEqual({ kind: 'unresolved', reason: 'ambiguous' }) + expect( + resolve([{ id: 'r', connectionId: 'openclaw' }, { id: 'r' }], { repoId: 'r' }) + ).toEqual({ kind: 'unresolved', reason: 'ambiguous' }) + }) + + it('treats the two spellings of one host as agreement, not conflict', () => { + expect( + resolve( + [ + { id: 'r', connectionId: 'm4air' }, + { id: 'r', executionHostId: 'ssh:m4air' } + ], + { repoId: 'r' } + ) + ).toMatchObject({ kind: 'resolved', hostId: 'ssh:m4air', connectionId: 'm4air' }) + }) + + it('reports an unknown owner distinctly from a conflicting one', () => { + expect(resolve([], { repoId: 'r' })).toEqual({ kind: 'unresolved', reason: 'unknown' }) + }) + }) + + it('ignores an unparseable host id rather than treating it as a host', () => { + const row = { id: 'r', connectionId: 'openclaw' } + expect(resolve([row], { repoId: 'r', hostId: 'ssh:' })).toMatchObject({ + kind: 'resolved', + connectionId: 'openclaw' + }) + }) +}) diff --git a/src/shared/worktree-execution-host-resolution.ts b/src/shared/worktree-execution-host-resolution.ts new file mode 100644 index 00000000000..8abe5de72cc --- /dev/null +++ b/src/shared/worktree-execution-host-resolution.ts @@ -0,0 +1,109 @@ +/** + * One rule for "which host does this worktree execute on, and what connection routes there". + * + * Main and the renderer both have to answer it — the terminal launch scope picks a PTY route from + * it, the renderer picks a file-read route and the reconnect affordance from it — so the rule lives + * here instead of being re-derived per side. Two re-derivations already disagreed: main answered + * from the worktree's own host while the renderer fell back to an id-only repo lookup, so a pane on + * `ssh:m4air` was offered "Reconnect openclaw" and read its files off openclaw (#11163). + * + * `unresolved` is a distinct answer, never "local": the same repo id can exist on a local, an SSH + * and a runtime host at once, and loss of a usable answer must fail closed rather than authorize a + * client-side read of a remote path (#6648, #17799). + */ + +import type { Repo } from './repo-types' +import { + getRepoExecutionHostId, + getRepoSshConnectionId, + getSshTargetIdForExecutionHost, + normalizeExecutionHostId, + type ExecutionHostId +} from './execution-host' + +export type ExecutionHostOwnerRow = Pick + +export type ExecutionHostOwnerMatch = + | { kind: 'resolved'; owner: T } + | { kind: 'missing' } + | { kind: 'ambiguous' } + +/** + * How a caller finds repo rows. Main scans the store array; the renderer answers from a + * WeakMap-memoized index because owner resolution runs inside retained selectors. That is a + * performance difference, not a different rule. + */ +export type ExecutionHostOwnerLookup = { + /** The row for `repoId`, or `ambiguous` when rival rows disagree about the owning host. */ + byId: (repoId: string) => ExecutionHostOwnerMatch + /** The row for `repoId` on exactly `hostId`, or null when that host carries no row. */ + byHost: (repoId: string, hostId: ExecutionHostId) => T | null +} + +export type WorktreeExecutionHostResolution = + | { + kind: 'resolved' + hostId: ExecutionHostId + /** + * The SSH target whose filesystem holds this workspace — for a `runtime:` host, its nested + * target, addressable only as the pair with `hostId`. Callers deciding what *this client* + * may dial (a PTY route, a Git provider) must use `getSshTargetIdForExecutionHost(hostId)` + * instead; this field can name a host the client cannot reach on its own. + */ + connectionId: string | null + /** Display metadata only. The decisions are `hostId` / `connectionId`. */ + owner: T | null + } + | { kind: 'unresolved'; reason: 'ambiguous' | 'unknown' } + +export function resolveWorktreeExecutionHost( + lookup: ExecutionHostOwnerLookup, + worktree: { repoId: string; hostId?: string | null } +): WorktreeExecutionHostResolution { + const worktreeHostId = normalizeExecutionHostId(worktree.hostId) + if (worktreeHostId) { + // The worktree names its own host, which outranks every repo row. A row on a *different* host + // is not evidence about this one — falling back to it is the cross-host leak: one SSH host's + // pane routed to another. A row on *this* host still is evidence, and is the only place a + // runtime's nested SSH target appears. + const owner = lookup.byHost(worktree.repoId, worktreeHostId) + return { + kind: 'resolved', + hostId: worktreeHostId, + connectionId: + getSshTargetIdForExecutionHost(worktreeHostId) ?? + (owner ? getRepoSshConnectionId(owner) : null), + owner + } + } + const match = lookup.byId(worktree.repoId) + if (match.kind !== 'resolved') { + return { kind: 'unresolved', reason: match.kind === 'ambiguous' ? 'ambiguous' : 'unknown' } + } + return { + kind: 'resolved', + hostId: getRepoExecutionHostId(match.owner), + connectionId: getRepoSshConnectionId(match.owner), + owner: match.owner + } +} + +/** Array-backed lookup for callers holding the whole repo list (main's store). */ +export function createRepoRowExecutionHostLookup( + repos: readonly T[] +): ExecutionHostOwnerLookup { + const rowsFor = (repoId: string): T[] => repos.filter((repo) => repo.id === repoId) + return { + byId: (repoId) => { + const rows = rowsFor(repoId) + if (rows.length === 0) { + return { kind: 'missing' } + } + const hostIds = new Set(rows.map((repo) => getRepoExecutionHostId(repo))) + const owner = rows[0] + return hostIds.size > 1 || !owner ? { kind: 'ambiguous' } : { kind: 'resolved', owner } + }, + byHost: (repoId, hostId) => + rowsFor(repoId).find((repo) => getRepoExecutionHostId(repo) === hostId) ?? null + } +}