mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
fix(host-routing): resolve both sides of the execution host through one rule
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.
This commit is contained in:
@@ -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 })
|
||||
})
|
||||
|
||||
@@ -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<Repo, 'id' | 'connectionId' | 'executionHostId'>
|
||||
@@ -14,41 +13,31 @@ export type WorktreeLaunchHostResolution<T extends LaunchHostRepo> =
|
||||
| { 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<T extends LaunchHostRepo>(
|
||||
export function resolveWorktreeLaunchHost<T extends LaunchHostRepo & ExecutionHostOwnerRow>(
|
||||
repos: readonly T[],
|
||||
worktree: { repoId: string; hostId?: ExecutionHostId | null }
|
||||
worktree: { repoId: string; hostId?: string | null }
|
||||
): WorktreeLaunchHostResolution<T> {
|
||||
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<ExecutionHostId>(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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,6 +27,27 @@ function makeRepo(overrides: Partial<Repo> & { id: string }): Repo {
|
||||
}
|
||||
}
|
||||
|
||||
function makeWorktree(overrides: Partial<Worktree> & { 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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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:<target>`, 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<Repo, 'connectionId' | 'executionHostId'>
|
||||
): 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:<target>`, 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:<env>` 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<Repo, 'connectionId' | 'executionHostId'>
|
||||
): 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<Worktree, 'hostId'>,
|
||||
repo: Pick<Repo, 'connectionId' | 'executionHostId'> | undefined,
|
||||
|
||||
@@ -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<typeof resolveWorktreeExecutionHost> =>
|
||||
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'
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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<Repo, 'id' | 'connectionId' | 'executionHostId'>
|
||||
|
||||
export type ExecutionHostOwnerMatch<T> =
|
||||
| { 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<T extends ExecutionHostOwnerRow> = {
|
||||
/** The row for `repoId`, or `ambiguous` when rival rows disagree about the owning host. */
|
||||
byId: (repoId: string) => ExecutionHostOwnerMatch<T>
|
||||
/** 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<T extends ExecutionHostOwnerRow> =
|
||||
| {
|
||||
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<T extends ExecutionHostOwnerRow>(
|
||||
lookup: ExecutionHostOwnerLookup<T>,
|
||||
worktree: { repoId: string; hostId?: string | null }
|
||||
): WorktreeExecutionHostResolution<T> {
|
||||
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<T extends ExecutionHostOwnerRow>(
|
||||
repos: readonly T[]
|
||||
): ExecutionHostOwnerLookup<T> {
|
||||
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
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user