Files
orca/src/shared/worktree-execution-host-resolution.test.ts
T
Neil fb69f00b65 fix(hosts): resolve a folder workspace's SSH host from the repo's host, not its raw connectionId (#18598)
* fix(hosts): resolve a folder workspace's SSH host from the repo's host, not its raw connectionId

`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:<target>'` 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.

* fix(hosts): resolve the host in candidate selection too, not just in resolution

The first pass fixed how a repo row is classified once it reaches
`resolveFolderWorkspaceHost`. The candidate filter decides which rows reach it at
all, and it read `repo.connectionId` raw as well — so an SSH-only row outside the
project-group subtree was dropped before the new logic could see it, and the
execute-here bug survived for the population the fix was for, via a different
path. Found in review by CodeRabbit.

Three repo-row reads had the same root cause, not one:

- the scope-connection filter, comparing a path repo's raw field against the
  workspace/group connection;
- the group-connection set, built from group repos' raw fields;
- that set's membership test against path repos' raw fields.

The last two are one comparison with the mismatch on either side, so resolving
only the path side would have reintroduced it from the other direction.

All three, plus the resolution loop, now go through one `getRepoScopeConnectionId`
helper. Non-SSH hosts still fall back to the raw field, so a `runtime:` row keeps
contributing its nested target exactly as before.

The new tests use a repo matched only by path, outside the subtree — the
population every existing test missed, which is why four passing revert-tests
did not catch this. One of them is labelled as pinning the resolver rather than
the filter: under the old raw read both rows came back connectionless and matched
each other by accident, so it survives a filter revert and must not be counted as
coverage for it.
2026-09-04 01:34:47 -07:00

267 lines
10 KiB
TypeScript

import { describe, expect, it } from 'vitest'
import {
createRepoRowExecutionHostLookup,
resolveWorktreeExecutionHost,
type ExecutionHostOwnerRow
} 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' })
})
// `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', () => {
const row = { id: 'r', connectionId: 'openclaw' }
expect(resolve([row], { repoId: 'r', hostId: 'ssh:' })).toMatchObject({
kind: 'resolved',
connectionId: 'openclaw'
})
})
})
describe('createRepoRowExecutionHostLookup', () => {
/** Rows whose `id` reads are counted, so a rescan of the repo list is observable. */
const countingRepos = (
rows: readonly ExecutionHostOwnerRow[]
): { repos: ExecutionHostOwnerRow[]; idReads: () => number } => {
let idReads = 0
const repos = rows.map(({ id, ...rest }) => ({
...rest,
get id(): string {
idReads += 1
return id
}
}))
return { repos, idReads: () => idReads }
}
it('scans the repo list once for the factory, never again per lookup', () => {
const { repos, idReads } = countingRepos([
{ id: 'a' },
{ id: 'b', connectionId: 'm4air' },
{ id: 'c' }
])
const lookup = createRepoRowExecutionHostLookup(repos)
// One grouping pass over the list — a Map get plus a set per row — and then never again.
const afterBuild = idReads()
expect(afterBuild).toBeLessThanOrEqual(repos.length * 2)
for (let i = 0; i < 50; i++) {
lookup.byId('a')
lookup.byId('missing')
lookup.byHost('b', 'ssh:m4air')
}
expect(idReads()).toBe(afterBuild)
})
it('answers missing, ambiguous and resolved exactly as a per-call scan would', () => {
expect(createRepoRowExecutionHostLookup([]).byId('r')).toEqual({ kind: 'missing' })
const openclaw = { id: 'r', connectionId: 'openclaw' }
const m4air = { id: 'r', connectionId: 'm4air' }
expect(createRepoRowExecutionHostLookup([openclaw, m4air]).byId('r')).toEqual({
kind: 'ambiguous'
})
// Two rows agreeing on one host still resolve to the first in repo-list order.
const first: ExecutionHostOwnerRow = { id: 'r', connectionId: 'm4air' }
const second: ExecutionHostOwnerRow = { id: 'r', executionHostId: 'ssh:m4air' }
expect(createRepoRowExecutionHostLookup([first, second]).byId('r')).toEqual({
kind: 'resolved',
owner: first
})
})
it('keeps byHost hits, misses and repo-list order', () => {
const openclaw = { id: 'r', connectionId: 'openclaw' }
const m4air = { id: 'r', connectionId: 'm4air' }
const lookup = createRepoRowExecutionHostLookup([openclaw, m4air])
expect(lookup.byHost('r', 'ssh:m4air')).toBe(m4air)
expect(lookup.byHost('r', 'ssh:openclaw')).toBe(openclaw)
expect(lookup.byHost('r', 'local')).toBeNull()
expect(lookup.byHost('other', 'local')).toBeNull()
})
})