diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 0708d09d993..39fbcf45af1 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 36m + + downloads: 37m @@ -15,7 +15,7 @@ downloads downloads - 36m - 36m + 37m + 37m diff --git a/src/main/ipc/pty-restore-record-seeding.test.ts b/src/main/ipc/pty-restore-record-seeding.test.ts index 0434be17e67..0f1646e7812 100644 --- a/src/main/ipc/pty-restore-record-seeding.test.ts +++ b/src/main/ipc/pty-restore-record-seeding.test.ts @@ -12,6 +12,9 @@ import { setupPtyIpcSuite } from './pty-ipc-test-harness' import { getDefaultWorkspaceSession } from '../../shared/constants' import { makePaneKey } from '../../shared/stable-pane-id' import { OrcaRuntimeService } from '../runtime/orca-runtime' +import type { RuntimeResolvedWorktreeCache } from '../runtime/runtime-resolved-worktree-cache' +import type { ResolvedWorktree } from '../runtime/runtime-worktree-path-identity' +import { getWorktreeScanMutationRevision } from '../local-worktree-scan-generation' import { registerPtyHandlers, clearProviderPtyState, @@ -359,15 +362,22 @@ describe('registerPtyHandlers', () => { } as never) // Why: selector resolution shells out to git for real repos; prime the // resolved-worktree cache so this headless fixture resolves offline. + // + // Why through getSnapshot and not a hand-written `resolved` entry: the cache decides freshness + // from fields it stamps itself, so a literal that mirrors them is a second copy of that + // contract and goes stale the moment a field is added. Let the cache stamp its own entry. const worktreeResolutionInternals = runtime as unknown as { - buildResolvedWorktreeFromId(id: string): unknown - resolvedWorktrees: object + buildResolvedWorktreeFromId(id: string): ResolvedWorktree + resolvedWorktrees: RuntimeResolvedWorktreeCache } - Reflect.set(worktreeResolutionInternals.resolvedWorktrees, 'resolved', { - worktrees: [worktreeResolutionInternals.buildResolvedWorktreeFromId(worktreeId)], - platformByRepoId: new Map([[repo.id, process.platform]]), - expiresAt: Date.now() + 60_000 - }) + await worktreeResolutionInternals.resolvedWorktrees.getSnapshot( + async () => ({ + worktrees: [worktreeResolutionInternals.buildResolvedWorktreeFromId(worktreeId)], + platformByRepoId: new Map([[repo.id, process.platform]]) + }), + 60_000, + getWorktreeScanMutationRevision() + ) setLocalPtyProvider({ spawn: vi.fn(async () => ({ id: ptyId, diff --git a/src/main/local-worktree-scan-generation.ts b/src/main/local-worktree-scan-generation.ts index c8cc3bd0ff9..a2c86afcc33 100644 --- a/src/main/local-worktree-scan-generation.ts +++ b/src/main/local-worktree-scan-generation.ts @@ -1,5 +1,6 @@ const generationByRepoId = new Map() let generationSequence = 0 +let mutationRevision = 0 export function getLocalWorktreeScanGeneration(repoId: string): number { const existing = generationByRepoId.get(repoId) @@ -13,6 +14,22 @@ export function getLocalWorktreeScanGeneration(repoId: string): number { export function bumpLocalWorktreeScanGeneration(repoId: string): void { generationByRepoId.set(repoId, ++generationSequence) + mutationRevision += 1 +} + +/** + * Advances on every event above that can change what a worktree scan would find — repo add, + * removal, update, and scan-cache invalidation — and on nothing else. A cache that must not answer + * for repos it never saw compares this in O(1) instead of walking the repo list. + * + * Why not `generationSequence`: that also advances when `getLocalWorktreeScanGeneration` mints a key + * for a repo id nothing has scanned yet, which is a read. Keying a snapshot on it would let a read + * path discard a snapshot that is still perfectly valid. + * + * Ordering-only: the value means nothing outside a same-process comparison. + */ +export function getWorktreeScanMutationRevision(): number { + return mutationRevision } export function isLocalWorktreeScanGenerationCurrent(repoId: string, generation: number): boolean { @@ -21,5 +38,6 @@ export function isLocalWorktreeScanGenerationCurrent(repoId: string, generation: export function resetLocalWorktreeScanGenerationsForTests(): void { generationSequence += 1 + mutationRevision += 1 generationByRepoId.clear() } diff --git a/src/main/providers/execution-host-provider-dispatch.test.ts b/src/main/providers/execution-host-provider-dispatch.test.ts new file mode 100644 index 00000000000..fd68deea787 --- /dev/null +++ b/src/main/providers/execution-host-provider-dispatch.test.ts @@ -0,0 +1,105 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { + ExecutionHostNotDispatchableError, + requireFilesystemProviderForHost, + requireGitProviderForHost, + resolveFilesystemRouteForHost, + resolveGitRouteForHost, + UnresolvableExecutionHostError +} from './execution-host-provider-dispatch' +import { registerSshGitProvider, unregisterSshGitProvider } from './ssh-git-dispatch' +import { + registerSshFilesystemProvider, + unregisterSshFilesystemProvider +} from './ssh-filesystem-dispatch' + +const connectionId = 'host-dispatch-target' +const gitProvider = { listWorktrees: async () => [] } as never +const filesystemProvider = { readDir: async () => [] } as never + +describe('execution host provider dispatch', () => { + afterEach(() => { + unregisterSshGitProvider(connectionId) + unregisterSshFilesystemProvider(connectionId) + }) + + it('routes `local` to the local entry rather than to a provider', () => { + expect(resolveGitRouteForHost('local')).toEqual({ kind: 'local', hostId: 'local' }) + expect(resolveFilesystemRouteForHost('local')).toEqual({ kind: 'local', hostId: 'local' }) + }) + + it('routes an ssh host to its registered provider', () => { + registerSshGitProvider(connectionId, gitProvider) + registerSshFilesystemProvider(connectionId, filesystemProvider) + + expect(resolveGitRouteForHost(`ssh:${connectionId}`)).toEqual({ + kind: 'ssh', + hostId: `ssh:${connectionId}`, + connectionId, + provider: gitProvider + }) + expect(resolveFilesystemRouteForHost(`ssh:${connectionId}`)).toEqual({ + kind: 'ssh', + hostId: `ssh:${connectionId}`, + connectionId, + provider: filesystemProvider + }) + expect(requireGitProviderForHost(`ssh:${connectionId}`)).toBe(gitProvider) + expect(requireFilesystemProviderForHost(`ssh:${connectionId}`)).toBe(filesystemProvider) + }) + + it('answers `unreachable`, not `local`, for an ssh host with no registered provider', () => { + const route = resolveGitRouteForHost(`ssh:${connectionId}`) + + // The distinction the old `connectionId ? ssh : local` shape could not spell. + expect(route.kind).toBe('ssh') + expect(route.kind === 'ssh' && route.provider).toBeNull() + expect(() => requireGitProviderForHost(`ssh:${connectionId}`)).toThrow( + /Remote connection dropped/ + ) + expect(() => requireFilesystemProviderForHost(`ssh:${connectionId}`)).toThrow( + /Remote connection dropped/ + ) + }) + + it('routes a runtime host to its own entry instead of collapsing it into local', () => { + expect(resolveGitRouteForHost('runtime:env-7')).toEqual({ + kind: 'runtime', + hostId: 'runtime:env-7', + environmentId: 'env-7' + }) + expect(resolveFilesystemRouteForHost('runtime:env-7')).toEqual({ + kind: 'runtime', + hostId: 'runtime:env-7', + environmentId: 'env-7' + }) + }) + + it('refuses to hand a runtime host to this process’s ssh table', () => { + // A runtime repo row carries the *server's* nested target id. Dialling it here would reach a + // same-named target in this client's namespace. + registerSshGitProvider(connectionId, gitProvider) + + expect(() => requireGitProviderForHost('runtime:env-7')).toThrow( + ExecutionHostNotDispatchableError + ) + expect(() => requireFilesystemProviderForHost('runtime:env-7')).toThrow( + ExecutionHostNotDispatchableError + ) + }) + + it('refuses to serve a local host from the remote-only accessor', () => { + expect(() => requireGitProviderForHost('local')).toThrow(ExecutionHostNotDispatchableError) + expect(() => requireFilesystemProviderForHost('local')).toThrow( + ExecutionHostNotDispatchableError + ) + }) + + it.each([null, undefined, '', 'nonsense', 'ssh:', 'runtime:', 'ssh:a|b'])( + 'throws instead of answering local for the unresolvable host %p', + (hostId) => { + expect(() => resolveGitRouteForHost(hostId)).toThrow(UnresolvableExecutionHostError) + expect(() => resolveFilesystemRouteForHost(hostId)).toThrow(UnresolvableExecutionHostError) + } + ) +}) diff --git a/src/main/providers/execution-host-provider-dispatch.ts b/src/main/providers/execution-host-provider-dispatch.ts new file mode 100644 index 00000000000..1034ceac140 --- /dev/null +++ b/src/main/providers/execution-host-provider-dispatch.ts @@ -0,0 +1,155 @@ +/** + * Host-keyed provider dispatch: one entry per execution host kind, with `local` among them. + * + * The incumbent spelling across main is `const c = repo.connectionId; c ? sshProvider(c) : local()`, + * where `null` means *both* "resolved: this is local" and "could not resolve". Every path that + * cannot determine the host therefore answers "local" and runs remote work on the client — the + * #11163 defect class, which has produced a reproduced cross-host leak (an `ssh:` worktree + * resolving to another target) and near-misses where a transcript that exists only on a remote host + * would have been read locally. The shape also cannot express a `runtime:` host at all. + * + * This module removes that spelling. Its input is an `ExecutionHostId`, which is never null, and an + * id that names no host throws instead of degrading. `getRepoExecutionHostId` / + * `getWorktreeExecutionHostId` / `resolveWorktreeExecutionHost` are the resolution layer that feeds + * it; the last one already answers `unresolved` as a distinct verdict rather than "local". + * + * Why a route union rather than a uniform `getGitProviderForHost(): IGitProvider`, which is the + * VS Code shape (`registerProvider(Schemas.file, …)` symmetric with `Schemas.vscodeRemote`, and + * `ENOPRO` when nothing matches). Two properties of this process, not style preferences: + * + * - `local` git and filesystem work is free functions taking per-worktree execution options + * (`wslDistro`, `sharedLinkPaths`, admission tier), not an `IGitProvider`. There is no local + * provider object to register, and a stateless one would silently drop WSL routing. + * - `runtime:` is not executed in this process *at all*. It is forwarded over the + * environment's transport (`runtimeEnvironments:call`) and the receiving server normalizes it to + * its own `local`. A repo row on a runtime host carries the server's *nested* SSH target in + * `connectionId`; that id is addressable only as the pair (environmentId, targetId). Handing it + * to this client's SSH table would dial a same-named target in the wrong namespace — turning a + * silent-local bug into a silent-wrong-host bug. `host-repo-catalog-snapshot` and + * `host-qualified-worktree-listing` already reject runtime hosts for the same reason. + * + * So the answer is Zed's shape — an enum on the owner (`Local { fs }` vs `Remote { … }`) — and the + * three kinds are symmetric variants of it. Callers switch exhaustively, so `runtime` can no longer + * collapse into `local` by omission. + * + * Note the deliberate second distinction inside the `ssh` variant: `provider: null` means "this host + * is remote and currently unreachable", which is not the same answer as "this host is local" and can + * no longer be spelled the same way. That mirrors the `live` / `unverifiable` / `exited` rule in + * docs/reference/ssh-execution-boundary.md — loss of contact is never evidence of locality. + */ + +import { + parseExecutionHostId, + type ExecutionHostId, + type LOCAL_EXECUTION_HOST_ID, + type ParsedExecutionHost +} from '../../shared/execution-host' +import { getSshGitProvider, SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE } from './ssh-git-dispatch' +import { + getSshFilesystemProvider, + SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE +} from './ssh-filesystem-dispatch' +import type { IFilesystemProvider, IGitProvider } from './types' + +/** An id that names no execution host. Never degrade to local — that is the whole defect class. */ +export class UnresolvableExecutionHostError extends Error { + constructor(readonly hostId: string | null | undefined) { + super( + `Cannot route work: ${JSON.stringify(hostId ?? null)} names no execution host. ` + + 'Refusing to fall back to this machine.' + ) + this.name = 'UnresolvableExecutionHostError' + } +} + +/** Asking this process for a host it does not execute is a routing mistake, not a fallback. */ +export class ExecutionHostNotDispatchableError extends Error { + constructor(readonly hostId: ExecutionHostId) { + super(`Execution host ${hostId} is not dispatched by this process.`) + this.name = 'ExecutionHostNotDispatchableError' + } +} + +type LocalRoute = { kind: 'local'; hostId: typeof LOCAL_EXECUTION_HOST_ID } +type RuntimeRoute = { kind: 'runtime'; hostId: `runtime:${string}`; environmentId: string } +type SshRoute = { + kind: 'ssh' + hostId: `ssh:${string}` + connectionId: string + /** `null` is "remote, currently unreachable" — never "local". */ + provider: TProvider | null +} + +export type ExecutionHostGitRoute = LocalRoute | RuntimeRoute | SshRoute +export type ExecutionHostFilesystemRoute = LocalRoute | RuntimeRoute | SshRoute + +// Takes an unvalidated string rather than `ExecutionHostId`: validating is the point, and host +// ids also arrive from persistence and IPC where the compiler cannot vouch for them. +function parseRoutableHost(hostId: string | null | undefined): ParsedExecutionHost { + const parsed = parseExecutionHostId(hostId) + if (!parsed) { + throw new UnresolvableExecutionHostError(hostId) + } + return parsed +} + +export function resolveGitRouteForHost(hostId: string | null | undefined): ExecutionHostGitRoute { + const parsed = parseRoutableHost(hostId) + switch (parsed.kind) { + case 'local': + return { kind: 'local', hostId: parsed.id } + case 'ssh': + return { + kind: 'ssh', + hostId: parsed.id, + connectionId: parsed.targetId, + provider: getSshGitProvider(parsed.targetId) ?? null + } + case 'runtime': + return { kind: 'runtime', hostId: parsed.id, environmentId: parsed.environmentId } + } +} + +export function resolveFilesystemRouteForHost( + hostId: string | null | undefined +): ExecutionHostFilesystemRoute { + const parsed = parseRoutableHost(hostId) + switch (parsed.kind) { + case 'local': + return { kind: 'local', hostId: parsed.id } + case 'ssh': + return { + kind: 'ssh', + hostId: parsed.id, + connectionId: parsed.targetId, + provider: getSshFilesystemProvider(parsed.targetId) ?? null + } + case 'runtime': + return { kind: 'runtime', hostId: parsed.id, environmentId: parsed.environmentId } + } +} + +/** For call sites that are structurally remote-only: local and runtime are both routing errors. */ +export function requireGitProviderForHost(hostId: string | null | undefined): IGitProvider { + const route = resolveGitRouteForHost(hostId) + if (route.kind !== 'ssh') { + throw new ExecutionHostNotDispatchableError(route.hostId) + } + if (!route.provider) { + throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) + } + return route.provider +} + +export function requireFilesystemProviderForHost( + hostId: string | null | undefined +): IFilesystemProvider { + const route = resolveFilesystemRouteForHost(hostId) + if (route.kind !== 'ssh') { + throw new ExecutionHostNotDispatchableError(route.hostId) + } + if (!route.provider) { + throw new Error(SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE) + } + return route.provider +} diff --git a/src/main/repo-worktrees.test.ts b/src/main/repo-worktrees.test.ts index d1c3935c1e1..a6b20c5445f 100644 --- a/src/main/repo-worktrees.test.ts +++ b/src/main/repo-worktrees.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { listWorktreeGraphMock, listWorktreesMock, listWorktreesStrictMock } = vi.hoisted(() => ({ listWorktreeGraphMock: vi.fn(), @@ -19,6 +19,8 @@ import { listRepoWorktreeGraph, listRepoWorktrees } from './repo-worktrees' +import { registerSshGitProvider, unregisterSshGitProvider } from './providers/ssh-git-dispatch' +import { WorktreeCatalogUnavailableError } from '../shared/worktree/worktree-catalog-availability' describe('repo-worktrees', () => { beforeEach(() => { @@ -196,6 +198,63 @@ describe('repo-worktrees', () => { expect(listWorktreesStrictMock).not.toHaveBeenCalled() }) + // #11163: a row may spell its owner only as `executionHostId`. Reading `connectionId` answers + // "local" for it and runs the listing against a same-named path on this machine. + describe('rows that spell their owner only as executionHostId', () => { + const sshOnlyRepo = { + id: 'repo-1', + path: '/srv/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + kind: 'git' as const, + executionHostId: 'ssh:host-a' as const + } + + afterEach(() => { + unregisterSshGitProvider('host-a') + unregisterSshGitProvider('nested-target') + }) + + it('never lists an ssh-owned row with local git', async () => { + await expect(listRepoWorktrees(sshOnlyRepo)).rejects.toThrow(WorktreeCatalogUnavailableError) + expect(listWorktreesMock).not.toHaveBeenCalled() + }) + + it('lists an ssh-owned row through its registered provider', async () => { + const listWorktrees = vi.fn().mockResolvedValue([{ path: '/srv/repo' }]) + registerSshGitProvider('host-a', { listWorktrees } as never) + + await expect(listRepoWorktrees(sshOnlyRepo)).resolves.toEqual([{ path: '/srv/repo' }]) + expect(listWorktrees).toHaveBeenCalledWith('/srv/repo') + expect(listWorktreesMock).not.toHaveBeenCalled() + }) + + it('keeps an ssh-owned root out of the local repo-root match', () => { + expect(isRepoRoot([sshOnlyRepo], '/srv/repo')).toBe(false) + }) + + it('rejects strict local listing for an ssh-owned row', async () => { + await expect(listLocalRepoWorktreesStrict(sshOnlyRepo)).rejects.toThrow('remote repository') + expect(listWorktreesStrictMock).not.toHaveBeenCalled() + }) + + it('refuses to answer a runtime-owned row from a same-named local target', async () => { + const listWorktrees = vi.fn().mockResolvedValue([{ path: '/wrong/host' }]) + registerSshGitProvider('nested-target', { listWorktrees } as never) + + await expect( + listRepoWorktrees({ + ...sshOnlyRepo, + executionHostId: 'runtime:env-7', + connectionId: 'nested-target' + }) + ).rejects.toThrow(WorktreeCatalogUnavailableError) + expect(listWorktrees).not.toHaveBeenCalled() + expect(listWorktreesMock).not.toHaveBeenCalled() + }) + }) + it('treats Windows repo root casing differences as the same local root', () => { const repos = [ { diff --git a/src/main/repo-worktrees.ts b/src/main/repo-worktrees.ts index b7c6e16f91a..228f303bfa2 100644 --- a/src/main/repo-worktrees.ts +++ b/src/main/repo-worktrees.ts @@ -2,7 +2,8 @@ import type { Repo } from '../shared/repo-types' import type { GitWorktreeInfo } from '../shared/worktree/types' import { listWorktreeGraph, listWorktrees, listWorktreesStrict } from './git/worktree' import { isFolderRepo } from '../shared/repo-kind' -import { getSshGitProvider } from './providers/ssh-git-dispatch' +import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../shared/execution-host' +import { resolveGitRouteForHost } from './providers/execution-host-provider-dispatch' import { areWorktreePathsEqual } from './ipc/worktree-logic' import { WorktreeCatalogUnavailableError } from '../shared/worktree/worktree-catalog-availability' @@ -16,8 +17,12 @@ function hasLocalRepoWorktreeListOptions(options: LocalRepoWorktreeListOptions | } export function isRepoRoot(repos: Repo[], resolvedTarget: string): boolean { + // Why: `!repo.connectionId` matched a remote path against a local one for a row that spells its + // owner only as `executionHostId: 'ssh:'`. Resolve the host instead of reading one field. return repos.some( - (repo) => !repo.connectionId && areWorktreePathsEqual(repo.path, resolvedTarget) + (repo) => + getRepoExecutionHostId(repo) === LOCAL_EXECUTION_HOST_ID && + areWorktreePathsEqual(repo.path, resolvedTarget) ) } @@ -41,17 +46,24 @@ export async function listRepoWorktrees( if (isFolderRepo(repo)) { return [createFolderWorktree(repo)] } - if (repo.connectionId) { - const provider = getSshGitProvider(repo.connectionId) + const route = resolveGitRouteForHost(getRepoExecutionHostId(repo)) + if (route.kind === 'runtime') { + // A runtime row's `connectionId` names a target in the *server's* namespace, not one this + // client may dial. Reading it here would answer from a same-named local target. + throw new WorktreeCatalogUnavailableError( + `Worktree catalog unavailable for ${repo.path}: host ${route.hostId} is not reachable from this process.` + ) + } + if (route.kind === 'ssh') { // Why: runtime worktree resolution can run before SSH providers have reattached during startup. // Never fall back to local git against a server path, and never report the unreachable host as an // empty catalog (#14004) — callers treat a resolved listing as authoritative. - if (!provider) { + if (!route.provider) { throw new WorktreeCatalogUnavailableError( - `Worktree catalog unavailable for ${repo.path}: SSH connection "${repo.connectionId}" is not connected.` + `Worktree catalog unavailable for ${repo.path}: SSH connection "${route.connectionId}" is not connected.` ) } - return await provider.listWorktrees(repo.path) + return await route.provider.listWorktrees(repo.path) } return hasLocalRepoWorktreeListOptions(options) ? await listWorktrees(repo.path, options) @@ -72,9 +84,15 @@ export async function listRepoWorktreeGraph( if (isFolderRepo(repo)) { return [createFolderWorktree(repo)] } - if (repo.connectionId) { - const provider = getSshGitProvider(repo.connectionId) - return provider ? await provider.listWorktrees(repo.path) : [] + const route = resolveGitRouteForHost(getRepoExecutionHostId(repo)) + // An unreachable remote host answers `[]` here, unlike listRepoWorktrees above, which throws. + // Preserved as-is: this call site's callers treat the graph as best-effort. The inconsistency is + // real but is a separate behavior decision from resolving the host correctly. + if (route.kind === 'runtime') { + return [] + } + if (route.kind === 'ssh') { + return route.provider ? await route.provider.listWorktrees(repo.path) : [] } return hasLocalRepoWorktreeListOptions(options) ? await listWorktreeGraph(repo.path, options) @@ -85,7 +103,7 @@ export async function listLocalRepoWorktreesStrict( repo: Repo, options?: LocalRepoWorktreeListOptions ): Promise { - if (repo.connectionId) { + if (getRepoExecutionHostId(repo) !== LOCAL_EXECUTION_HOST_ID) { throw new Error('Cannot list worktrees for a remote repository') } if (isFolderRepo(repo)) { diff --git a/src/main/runtime/orca-runtime-agent-session-operation.test.ts b/src/main/runtime/orca-runtime-agent-session-operation.test.ts index ac64dfaa6b7..e99560c7b3a 100644 --- a/src/main/runtime/orca-runtime-agent-session-operation.test.ts +++ b/src/main/runtime/orca-runtime-agent-session-operation.test.ts @@ -149,6 +149,41 @@ describe('agent-session create operation ledger', () => { expect(createTerminal).toHaveBeenCalledOnce() }) + it('shapes the launch for the route it resolved, not a repo row on another host', async () => { + // `scope.repo` is display metadata and can be a row from a different host than the worktree + // names (#11163). Reading it made a locally-routed launch emit the SSH relay shim name. + const runtime = createRuntime({ + supportsAgentSessionClaims: () => true, + supportsAgentSessionCreateOperations: () => true + }) + const internal = runtime as unknown as { + resolveTerminalWorkspaceLaunchScope: ReturnType + } + internal.resolveTerminalWorkspaceLaunchScope.mockResolvedValue({ + id: 'worktree-1', + path: '/repo/worktree-1', + connectionId: null, + // The rival row names openclaw while the worktree resolved to no SSH route at all. + repo: { + id: 'repo-1', + connectionId: 'openclaw', + executionHostId: null, + path: '/srv/openclaw' + }, + folderWorkspace: null + }) + const createTerminal = vi.spyOn(runtime, 'createTerminal').mockResolvedValue(terminal()) + + await runtime.createAgentSession( + request(operationId(), { agent: 'claude-agent-teams', prompt: '' }) + ) + + expect(createTerminal).toHaveBeenCalledWith( + 'id:worktree-1', + expect.objectContaining({ command: expect.stringContaining('orca-ide claude-teams') }) + ) + }) + it('requests exact client legacy fallback before nested SSH side effects', async () => { const runtime = createRuntime() const internal = runtime as unknown as { diff --git a/src/main/runtime/orca-runtime-create-agent-session.ts b/src/main/runtime/orca-runtime-create-agent-session.ts index db2b71a0adc..2ac34f4a920 100644 --- a/src/main/runtime/orca-runtime-create-agent-session.ts +++ b/src/main/runtime/orca-runtime-create-agent-session.ts @@ -16,7 +16,6 @@ import { AGENT_SESSION_OPERATION_PER_CLIENT_LIMIT } from './orca-runtime-core' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' -import { repoIsRemote } from '../../shared/agent-launch-remote' import { resolveLocalWindowsAgentStartupShell } from '../../shared/windows-terminal-shell' import { resolveTuiAgentLaunchArgs, @@ -152,9 +151,9 @@ export class OrcaRuntimeWithCreateAgentSession extends OrcaRuntimeWithGetAgentSe throw new Error('Selected agent is disabled. Choose an enabled agent before creating.') } const platform = this.getAgentLaunchPlatformForWorkspace(workspace) - const isRemote = workspace.repo - ? repoIsRemote(workspace.repo) - : Boolean(workspace.connectionId) + // Why: `workspace.repo` is display metadata and may be a row from another host; the launch + // shape must match the PTY route this scope already resolved. + const isRemote = Boolean(workspace.connectionId) const shell = resolveLocalWindowsAgentStartupShell({ platform, isRemote, diff --git a/src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts b/src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts index 2b7568cc75d..f70cf033718 100644 --- a/src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts +++ b/src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts @@ -11,7 +11,6 @@ import type { } from '../../shared/agent-session-host-authority' import { canonicalizeAgentSessionIdentity } from './agent-session-claim-identity' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' -import { repoIsRemote } from '../../shared/agent-launch-remote' import { resolveLocalWindowsAgentStartupShell } from '../../shared/windows-terminal-shell' import { buildAgentResumeStartupPlan } from '../../shared/tui-agent-startup' import { @@ -131,7 +130,9 @@ export class OrcaRuntimeWithGetAgentSessionExecutionNamespace extends OrcaRuntim throw new Error('Selected agent is disabled. Choose an enabled agent before resuming.') } const platform = this.getAgentLaunchPlatformForWorkspace(workspace) - const isRemote = workspace.repo ? repoIsRemote(workspace.repo) : Boolean(workspace.connectionId) + // Why: `workspace.repo` is display metadata and may be a row from another host; the launch + // shape must match the PTY route this scope already resolved. + const isRemote = Boolean(workspace.connectionId) const shell = resolveLocalWindowsAgentStartupShell({ platform, isRemote, diff --git a/src/main/runtime/orca-runtime-list-known-resolved-worktrees-for-explicit-target.ts b/src/main/runtime/orca-runtime-list-known-resolved-worktrees-for-explicit-target.ts index 1f0dbe591a3..152ef547889 100644 --- a/src/main/runtime/orca-runtime-list-known-resolved-worktrees-for-explicit-target.ts +++ b/src/main/runtime/orca-runtime-list-known-resolved-worktrees-for-explicit-target.ts @@ -5,6 +5,7 @@ import { splitWorktreeIdForFilesystem } from '../../shared/worktree/id' import { isPathInsideOrEqual } from '../../shared/cross-platform-path' import type { ResolvedWorktreeSnapshot } from './runtime-resolved-worktree-cache' import { RESOLVED_WORKTREE_CACHE_TTL_MS } from './orca-runtime-postlude' +import { getWorktreeScanMutationRevision } from '../local-worktree-scan-generation' import { resolveLocalProjectRuntimeForRepo, resolveLocalProjectRuntimesForRepos @@ -65,8 +66,7 @@ export class OrcaRuntimeWithListKnownResolvedWorktreesForExplicitTarget extends /** A warm fleet snapshot already answers any selector for free, so scoped scanning must yield to it. */ protected hasFreshResolvedWorktreeCache(): boolean { - const cached = this.resolvedWorktrees.peek() - return Boolean(cached && cached.expiresAt > Date.now()) + return this.resolvedWorktrees.isFresh(getWorktreeScanMutationRevision()) } protected async listResolvedWorktrees(): Promise { @@ -79,7 +79,8 @@ export class OrcaRuntimeWithListKnownResolvedWorktreesForExplicitTarget extends } return this.resolvedWorktrees.getSnapshot( () => this.computeResolvedWorktrees(), - RESOLVED_WORKTREE_CACHE_TTL_MS + RESOLVED_WORKTREE_CACHE_TTL_MS, + getWorktreeScanMutationRevision() ) } diff --git a/src/main/runtime/orca-runtime-resolve-mobile-session-terminal-command.ts b/src/main/runtime/orca-runtime-resolve-mobile-session-terminal-command.ts index ab63b16ee53..db30b7e2d11 100644 --- a/src/main/runtime/orca-runtime-resolve-mobile-session-terminal-command.ts +++ b/src/main/runtime/orca-runtime-resolve-mobile-session-terminal-command.ts @@ -5,7 +5,6 @@ import type { WorktreeStartupLaunch } from '../../shared/worktree/launch-types' import type { TuiAgent } from '../../shared/tui-agent' import type { SleepingAgentLaunchConfig } from '../../shared/agent-session-resume' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' -import { repoIsRemote } from '../../shared/agent-launch-remote' import { resolveLocalWindowsAgentStartupShell } from '../../shared/windows-terminal-shell' import { buildAgentStartupPlan } from '../../shared/tui-agent-startup' import { @@ -54,7 +53,7 @@ export class OrcaRuntimeWithResolveMobileSessionTerminalCommand extends OrcaRunt // Why: mobile may be iOS while the shell host is Windows/macOS/Linux or SSH Linux; quote for the host shell. const platform = this.getAgentLaunchPlatformForWorkspace(workspace) // Why: SSH runs the CLI through the relay shim (plain `orca`), so the Linux-only `orca-ide` rename must not apply. - const isRemote = workspace.repo ? repoIsRemote(workspace.repo) : repoIsRemote(workspace) + const isRemote = Boolean(workspace.connectionId) const queuedShell = resolveLocalWindowsAgentStartupShell({ platform, isRemote, diff --git a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts index 58fd6231fc7..0d934548332 100644 --- a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts +++ b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts @@ -14,7 +14,6 @@ import type { ForceDeleteWorktreeBranchResult } from '../../shared/worktree/crea import type { RuntimeTerminalRename } from '../../shared/runtime-types' import type { TerminalWorkspaceLaunchScope } from './runtime-legacy-worker-terminal-recovery-types' import type { TerminalCreateOptions } from './runtime-terminal-contracts' -import { repoIsRemote } from '../../shared/agent-launch-remote' import { resolveLocalWindowsAgentStartupShell } from '../../shared/windows-terminal-shell' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' import { resolveBareAgentLaunchCommand } from './runtime-agent-launch-resolution' @@ -175,7 +174,9 @@ export class OrcaRuntimeWithResolveWorktreeRemovalTarget extends OrcaRuntimeWith const settings = store.getSettings() const platform = this.getAgentLaunchPlatformForWorkspace(workspace) - const isRemote = workspace.repo ? repoIsRemote(workspace.repo) : Boolean(workspace.connectionId) + // Why: `workspace.repo` is display metadata and may be a row from another host; the launch + // shape must match the PTY route this scope already resolved. + const isRemote = Boolean(workspace.connectionId) const queuedShell = resolveLocalWindowsAgentStartupShell({ platform, isRemote, diff --git a/src/main/runtime/orca-runtime-tests/worktree-scan-cache-ttl.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-scan-cache-ttl.spec.ts index 8dc8d7db3a2..f8ec37f7c2c 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-scan-cache-ttl.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-scan-cache-ttl.spec.ts @@ -5,6 +5,7 @@ import { resolveWorktreeScanCacheTtlMs } from '../orca-runtime-test-mocks.spec' import { store } from '../orca-runtime-test-fixtures.spec' +import { bumpLocalWorktreeScanGeneration } from '../../local-worktree-scan-generation' describe('resolveWorktreeScanCacheTtlMs', () => { const BASE_TTL_MS = 30_000 @@ -84,4 +85,34 @@ describe('resolveWorktreeScanCacheTtlMs', () => { vi.useRealTimers() } }) + + it('scans a repo registered after the last snapshot instead of answering from it', async () => { + // Why: the fleet snapshot only covers the repos that existed when it ran, so for a full TTL it + // reported a just-connected SSH host as having no worktrees at all — and callers that resolve a + // workspace through it turned that gap into `skill-install-workspace-not-found`. + vi.mocked(listWorktrees).mockClear() + const addedPath = '/tmp/repo-registered-later' + const repos = [ + { id: 'repo-1', path: '/tmp/repo', displayName: 'repo', badgeColor: 'blue', addedAt: 1 } + ] + const runtime = new OrcaRuntimeService({ ...store, getRepos: () => repos } as never) + const internals = runtime as unknown as { listResolvedWorktrees: () => Promise } + const scanCallsFor = (path: string): number => + vi.mocked(listWorktrees).mock.calls.filter((call) => call[0] === path).length + + await internals.listResolvedWorktrees() + expect(scanCallsFor(addedPath)).toBe(0) + + repos.push({ + id: 'repo-added', + path: addedPath, + displayName: 'added', + badgeColor: 'blue', + addedAt: 2 + }) + bumpLocalWorktreeScanGeneration('repo-added') + + await internals.listResolvedWorktrees() + expect(scanCallsFor(addedPath)).toBe(1) + }) }) diff --git a/src/main/runtime/runtime-resolved-worktree-cache.test.ts b/src/main/runtime/runtime-resolved-worktree-cache.test.ts new file mode 100644 index 00000000000..31cc4fa2b60 --- /dev/null +++ b/src/main/runtime/runtime-resolved-worktree-cache.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest' +import { RuntimeResolvedWorktreeCache } from './runtime-resolved-worktree-cache' +import type { ResolvedWorktreeSnapshot } from './runtime-resolved-worktree-cache' +import { + bumpLocalWorktreeScanGeneration, + getLocalWorktreeScanGeneration, + getWorktreeScanMutationRevision +} from '../local-worktree-scan-generation' + +function snapshotOf(ids: string[]): ResolvedWorktreeSnapshot { + return { + worktrees: ids.map((id) => ({ id }) as ResolvedWorktreeSnapshot['worktrees'][number]), + platformByRepoId: new Map() + } +} + +describe('RuntimeResolvedWorktreeCache', () => { + it('reuses a snapshot inside the TTL while the repo inventory is unchanged', async () => { + const cache = new RuntimeResolvedWorktreeCache() + let computes = 0 + const compute = async (): Promise => { + computes += 1 + return snapshotOf(['repo-1::/a']) + } + + await cache.getSnapshot(compute, 60_000, 7) + const second = await cache.getSnapshot(compute, 60_000, 7) + + expect(computes).toBe(1) + expect(second.worktrees.map((worktree) => worktree.id)).toEqual(['repo-1::/a']) + }) + + it('recomputes when the repo inventory moved, even well inside the TTL', async () => { + // Why: this is the whole point. A snapshot taken before a repo was registered cannot testify + // that the repo's worktrees are absent — callers read the gap as "workspace not found". + const cache = new RuntimeResolvedWorktreeCache() + const results = [snapshotOf(['repo-1::/a']), snapshotOf(['repo-1::/a', 'repo-2::/b'])] + let computes = 0 + const compute = async (): Promise => results[computes++] + + await cache.getSnapshot(compute, 60_000, 7) + const afterRegistration = await cache.getSnapshot(compute, 60_000, 8) + + expect(computes).toBe(2) + expect(afterRegistration.worktrees.map((worktree) => worktree.id)).toEqual([ + 'repo-1::/a', + 'repo-2::/b' + ]) + }) + + it('does not join an in-flight compute that started under a stale inventory', async () => { + const cache = new RuntimeResolvedWorktreeCache() + const computed: number[] = [] + const compute = async (): Promise => { + computed.push(computed.length) + return snapshotOf([]) + } + + const first = cache.getSnapshot(compute, 60_000, 7) + const second = cache.getSnapshot(compute, 60_000, 8) + await Promise.all([first, second]) + + expect(computed).toHaveLength(2) + }) + + it('reports freshness against the inventory the snapshot was computed under', async () => { + const cache = new RuntimeResolvedWorktreeCache() + await cache.getSnapshot(async () => snapshotOf([]), 60_000, 7) + + expect(cache.isFresh(7)).toBe(true) + expect(cache.isFresh(8)).toBe(false) + cache.invalidateResolved() + expect(cache.isFresh(7)).toBe(false) + }) + + it('keeps a primed snapshot servable when nothing mutated', async () => { + // Why: the headless-reattach fixtures prime this cache once and then resolve a selector off it + // without any git available. Losing freshness for a reason other than a mutation strands them + // on a real scan, which is the failure this pairs with — a lookup that finds nothing because + // the snapshot was dropped, not because the worktree is gone. + const cache = new RuntimeResolvedWorktreeCache() + let computes = 0 + const prime = async (): Promise => { + computes += 1 + return snapshotOf(['repo-restore::/tmp/restore-records']) + } + await cache.getSnapshot(prime, 60_000, getWorktreeScanMutationRevision()) + + // A read that mints a scan generation for a repo nothing has scanned yet is not a mutation. + getLocalWorktreeScanGeneration(`repo-never-scanned-${Math.random()}`) + + expect(cache.isFresh(getWorktreeScanMutationRevision())).toBe(true) + const served = await cache.getSnapshot(prime, 60_000, getWorktreeScanMutationRevision()) + expect(computes).toBe(1) + expect(served.worktrees.map((worktree) => worktree.id)).toEqual([ + 'repo-restore::/tmp/restore-records' + ]) + }) +}) + +describe('getWorktreeScanMutationRevision', () => { + it('advances on a repo mutation and not on a first-seen generation read', () => { + const repoId = `repo-${Math.random()}` + const before = getWorktreeScanMutationRevision() + + getLocalWorktreeScanGeneration(repoId) + expect(getWorktreeScanMutationRevision()).toBe(before) + + bumpLocalWorktreeScanGeneration(repoId) + expect(getWorktreeScanMutationRevision()).toBe(before + 1) + }) +}) diff --git a/src/main/runtime/runtime-resolved-worktree-cache.ts b/src/main/runtime/runtime-resolved-worktree-cache.ts index b7ce735eefe..ef7532a6e91 100644 --- a/src/main/runtime/runtime-resolved-worktree-cache.ts +++ b/src/main/runtime/runtime-resolved-worktree-cache.ts @@ -5,9 +5,10 @@ export type ResolvedWorktreeSnapshot = { platformByRepoId: ReadonlyMap } -type ResolvedCache = ResolvedWorktreeSnapshot & { expiresAt: number } +type ResolvedCache = ResolvedWorktreeSnapshot & { expiresAt: number; inventoryRevision: number } type ResolvedInFlight = { generation: number + inventoryRevision: number promise: Promise } export class RuntimeResolvedWorktreeCache { @@ -19,25 +20,43 @@ export class RuntimeResolvedWorktreeCache { return this.resolved } + /** + * Why the revision and not the TTL alone: a snapshot only answers for the repos that were + * registered when it ran. A repo added afterwards — a remote host the user just connected — + * is missing from it for reasons that have nothing to do with what exists on that host, and + * callers read the gap as a verdict that the worktree does not exist. + */ + isFresh(inventoryRevision: number, now = Date.now()): boolean { + return Boolean( + this.resolved && + this.resolved.inventoryRevision === inventoryRevision && + this.resolved.expiresAt > now + ) + } + async getSnapshot( compute: () => Promise, - ttlMs: number + ttlMs: number, + inventoryRevision: number ): Promise { - if (this.resolved && this.resolved.expiresAt > Date.now()) { + if (this.resolved && this.isFresh(inventoryRevision)) { return this.resolved } const generation = this.resolvedGeneration - if (this.resolvedInFlight?.generation === generation) { + if ( + this.resolvedInFlight?.generation === generation && + this.resolvedInFlight.inventoryRevision === inventoryRevision + ) { return this.resolvedInFlight.promise } const promise = compute() - this.resolvedInFlight = { generation, promise } + this.resolvedInFlight = { generation, inventoryRevision, promise } try { const result = await promise if (generation === this.resolvedGeneration) { // Why stamped on completion, not entry: a compute that spent longer than the TTL would // otherwise publish an already-expired entry, so the next poll recomputes the same slow path. - this.resolved = { ...result, expiresAt: Date.now() + ttlMs } + this.resolved = { ...result, inventoryRevision, expiresAt: Date.now() + ttlMs } } return result } finally { diff --git a/src/main/runtime/runtime-worktree-agent-startup.test.ts b/src/main/runtime/runtime-worktree-agent-startup.test.ts index e276595c4dd..87fcfd9dd58 100644 --- a/src/main/runtime/runtime-worktree-agent-startup.test.ts +++ b/src/main/runtime/runtime-worktree-agent-startup.test.ts @@ -1,14 +1,119 @@ import { describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/repo-types' const mocks = vi.hoisted(() => ({ markCodexProjectTrusted: vi.fn(), markCopilotFolderTrusted: vi.fn(), - markCursorWorkspaceTrusted: vi.fn() + markCursorWorkspaceTrusted: vi.fn(), + detectRemoteAgents: vi.fn(), + detectInstalledAgentsWithShellPathHydration: vi.fn() })) -vi.mock('../agent-trust-presets', () => mocks) +vi.mock('../agent-trust-presets', () => ({ + markCodexProjectTrusted: mocks.markCodexProjectTrusted, + markCopilotFolderTrusted: mocks.markCopilotFolderTrusted, + markCursorWorkspaceTrusted: mocks.markCursorWorkspaceTrusted +})) -import { markLocalWorktreeTrusted } from './runtime-worktree-agent-startup' +vi.mock('../preflight/agent-detection', () => ({ + detectRemoteAgents: mocks.detectRemoteAgents, + detectInstalledAgentsWithShellPathHydration: mocks.detectInstalledAgentsWithShellPathHydration +})) + +import { + buildWorktreeStartupForAgent, + buildWorktreeStartupForDraft, + markLocalWorktreeTrusted +} from './runtime-worktree-agent-startup' + +function makeRepo(fields: Partial): Repo { + return { + id: 'repo-1', + name: 'repo', + path: '/srv/repo', + connectionId: null, + executionHostId: null, + ...fields + } as Repo +} + +const settings = { + agentCmdOverrides: {}, + agentDefaultArgs: {}, + agentDefaultEnv: {}, + disabledTuiAgents: [], + defaultTuiAgent: undefined, + terminalWindowsShell: null +} as never + +/** The launched CLI name is the whole decision: `orca` is the relay shim, `orca-ide` is local. */ +function launchCliNameFor(repo: Repo): string { + return buildWorktreeStartupForAgent({ + repo, + settings, + agent: 'claude-agent-teams', + getLaunchPlatform: () => 'linux', + toSessionOptions: () => undefined + }).startup.command.split(' ')[0]! +} + +describe('buildWorktreeStartupForAgent host resolution', () => { + // Why two hosts: one SSH fixture passes even when the launch shape is resolved off another + // host's row, which is the shape of the `ssh:m4air` -> openclaw leak. + it('drops the Linux-only rename for both spellings of SSH ownership on two hosts', () => { + expect(launchCliNameFor(makeRepo({ connectionId: 'm4air' }))).toBe('orca') + expect(launchCliNameFor(makeRepo({ executionHostId: 'ssh:openclaw' }))).toBe('orca') + }) + + it('keeps the Linux rename for a local row carrying a stale connection', () => { + expect(launchCliNameFor(makeRepo({ connectionId: 'm4air', executionHostId: 'local' }))).toBe( + 'orca-ide' + ) + }) + + it('drops the rename for a runtime host reaching a nested SSH target', () => { + expect( + launchCliNameFor(makeRepo({ connectionId: 'nested', executionHostId: 'runtime:vm-1' })) + ).toBe('orca') + }) + + it('keeps the rename for a runtime host with no nested SSH target', () => { + expect(launchCliNameFor(makeRepo({ executionHostId: 'runtime:vm-1' }))).toBe('orca-ide') + }) +}) + +describe('buildWorktreeStartupForDraft agent detection', () => { + it('probes the SSH host named only by executionHostId instead of this client', async () => { + mocks.detectRemoteAgents.mockResolvedValueOnce(['claude']) + mocks.detectInstalledAgentsWithShellPathHydration.mockResolvedValue([]) + + const result = await buildWorktreeStartupForDraft({ + repo: makeRepo({ executionHostId: 'ssh:openclaw' }), + settings, + draft: 'ship it', + getLaunchPlatform: () => 'linux' + }) + + expect(mocks.detectRemoteAgents).toHaveBeenCalledWith({ connectionId: 'openclaw' }) + expect(mocks.detectInstalledAgentsWithShellPathHydration).not.toHaveBeenCalled() + expect(result?.agent).toBe('claude') + }) + + it('probes this client for a local row carrying a stale connection', async () => { + mocks.detectRemoteAgents.mockClear() + mocks.detectInstalledAgentsWithShellPathHydration.mockResolvedValueOnce(['claude']) + + const result = await buildWorktreeStartupForDraft({ + repo: makeRepo({ connectionId: 'm4air', executionHostId: 'local' }), + settings, + draft: 'ship it', + getLaunchPlatform: () => 'linux' + }) + + expect(mocks.detectRemoteAgents).not.toHaveBeenCalled() + expect(result?.agent).toBe('claude') + }) +}) describe('markLocalWorktreeTrusted', () => { it('waits for the Codex trust write before resolving', async () => { diff --git a/src/main/runtime/runtime-worktree-agent-startup.ts b/src/main/runtime/runtime-worktree-agent-startup.ts index 7c662d633e2..8663771e998 100644 --- a/src/main/runtime/runtime-worktree-agent-startup.ts +++ b/src/main/runtime/runtime-worktree-agent-startup.ts @@ -3,6 +3,7 @@ import type { Repo } from '../../shared/repo-types' import type { TuiAgent } from '../../shared/tui-agent' import type { WorktreeStartupLaunch } from '../../shared/worktree/launch-types' import { repoIsRemote } from '../../shared/agent-launch-remote' +import { getRepoSshConnectionId } from '../../shared/execution-host' import { isTuiAgent, TUI_AGENT_CONFIG } from '../../shared/tui-agent-config' import { isTuiAgentEnabled, pickTuiAgent } from '../../shared/tui-agent-selection' import { @@ -55,10 +56,13 @@ export async function buildWorktreeStartupForDraft( : null if (!agent) { let detected: string[] = [] + // Why: detection has to run on the machine that will run the agent, and SSH ownership has two + // spellings — the raw field probes this client for an `executionHostId: 'ssh:*'`-only repo. + const sshConnectionId = getRepoSshConnectionId(repo) try { // Why: startup-draft fallback can run from sparse runtime launch envs too. - detected = repo.connectionId - ? await detectRemoteAgents({ connectionId: repo.connectionId }) + detected = sshConnectionId + ? await detectRemoteAgents({ connectionId: sshConnectionId }) : await detectInstalledAgentsWithShellPathHydration() } catch { detected = [] diff --git a/src/main/workspace-space-repo-scan.ts b/src/main/workspace-space-repo-scan.ts index 9a0bc316403..3bf3c6cf1f4 100644 --- a/src/main/workspace-space-repo-scan.ts +++ b/src/main/workspace-space-repo-scan.ts @@ -9,11 +9,13 @@ import type { WorkspaceSpaceWorktree } from '../shared/workspace-space-types' import { mapWithConcurrency } from '../shared/map-with-concurrency' -import { getRepoExecutionHostId } from '../shared/execution-host' +import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../shared/execution-host' import { readWorktreeMetaForHost } from './persistence/host-qualified-worktree-meta' import { getRepoOwnedWorktreeMeta } from './worktree-metadata-ownership' -import { getSshFilesystemProvider } from './providers/ssh-filesystem-dispatch' -import { getSshGitProvider } from './providers/ssh-git-dispatch' +import { + resolveFilesystemRouteForHost, + resolveGitRouteForHost +} from './providers/execution-host-provider-dispatch' import { createFolderWorktree, listRepoWorktrees } from './repo-worktrees' import { mergeWorktree } from './ipc/worktree-logic' import { getLocalProjectWorktreeGitOptions } from './project-runtime-git-options' @@ -89,16 +91,25 @@ async function listWorktreesForSpaceScan( if (isFolderRepo(repo)) { return { ok: true, worktrees: [createFolderWorktree(repo)] } } - if (repo.connectionId) { - const provider = getSshGitProvider(repo.connectionId) - if (!provider) { + // Why: the raw `connectionId` field answers "local" for a row that spells its owner only as + // `executionHostId: 'ssh:'`, which sizes a same-named path on this machine instead. + const route = resolveGitRouteForHost(getRepoExecutionHostId(repo)) + if (route.kind === 'runtime') { + return { + ok: false, + status: 'unavailable', + error: `Host ${route.hostId} is not reachable from this process.` + } + } + if (route.kind === 'ssh') { + if (!route.provider) { return { ok: false, status: 'unavailable', - error: `SSH connection "${repo.connectionId}" is not connected.` + error: `SSH connection "${route.connectionId}" is not connected.` } } - const worktrees = await provider.listWorktrees(repo.path, { signal }) + const worktrees = await route.provider.listWorktrees(repo.path, { signal }) throwIfWorkspaceSpaceScanAborted(signal) return { ok: true, worktrees } } @@ -175,7 +186,7 @@ export async function scanWorkspaceSpaceRepo(args: { executionHostId: getRepoExecutionHostId(repo), displayName: repo.displayName, path: repo.path, - isRemote: Boolean(repo.connectionId), + isRemote: getRepoExecutionHostId(repo) !== LOCAL_EXECUTION_HOST_ID, worktreeCount: 0, scannedWorktreeCount: 0, unavailableWorktreeCount: 1, @@ -193,7 +204,7 @@ export async function scanWorkspaceSpaceRepo(args: { { totalWorktreeCount: progress.totalWorktreeCount + worktrees.length }, options.onProgress ) - const remoteProvider = repo.connectionId ? getSshFilesystemProvider(repo.connectionId) : undefined + const filesystemRoute = resolveFilesystemRouteForHost(getRepoExecutionHostId(repo)) const rows = await mapWithConcurrency(worktrees, WORKTREE_SCAN_CONCURRENCY, async (worktree) => { throwIfWorkspaceSpaceScanAborted(options.signal) reportProgress( @@ -204,33 +215,36 @@ export async function scanWorkspaceSpaceRepo(args: { }, options.onProgress ) - const row = repo.connectionId - ? remoteProvider - ? await scanRemoteWorkspaceSpaceWorktree( - repo, - worktree, - scannedAt, - remoteProvider, - limiters.remoteFallbackTraversal, - options.signal + const row = + filesystemRoute.kind !== 'local' + ? filesystemRoute.kind === 'ssh' && filesystemRoute.provider + ? await scanRemoteWorkspaceSpaceWorktree( + repo, + worktree, + scannedAt, + filesystemRoute.provider, + limiters.remoteFallbackTraversal, + options.signal + ) + : createUnavailableWorkspaceSpaceRow( + repo, + worktree, + scannedAt, + 'unavailable', + filesystemRoute.kind === 'ssh' + ? `SSH filesystem for "${filesystemRoute.connectionId}" is not connected.` + : `Host ${filesystemRoute.hostId} is not reachable from this process.` + ) + : await limiters.localWorktree(() => + scanLocalWorkspaceSpaceWorktree( + repo, + worktree, + scannedAt, + args.readLocalDuDepthOne, + args.normalizeLocalDuPath, + options.signal + ) ) - : createUnavailableWorkspaceSpaceRow( - repo, - worktree, - scannedAt, - 'unavailable', - `SSH filesystem for "${repo.connectionId}" is not connected.` - ) - : await limiters.localWorktree(() => - scanLocalWorkspaceSpaceWorktree( - repo, - worktree, - scannedAt, - args.readLocalDuDepthOne, - args.normalizeLocalDuPath, - options.signal - ) - ) reportProgress( progress, { @@ -265,7 +279,7 @@ export async function scanWorkspaceSpaceRepo(args: { executionHostId: getRepoExecutionHostId(repo), displayName: repo.displayName, path: repo.path, - isRemote: Boolean(repo.connectionId), + isRemote: getRepoExecutionHostId(repo) !== LOCAL_EXECUTION_HOST_ID, worktreeCount: rows.length, ...summary, error: null diff --git a/src/renderer/src/lib/agent-background-session-launch-host.test.ts b/src/renderer/src/lib/agent-background-session-launch-host.test.ts index 86fd8577fae..ef00efa5145 100644 --- a/src/renderer/src/lib/agent-background-session-launch-host.test.ts +++ b/src/renderer/src/lib/agent-background-session-launch-host.test.ts @@ -71,6 +71,80 @@ describe('resolveAgentBackgroundLaunchHost', () => { ).toThrow('unavailable or ambiguous') }) + // Why two hosts: a single-SSH fixture passes even when the route is read off another host's + // row, which is the shape of the `ssh:m4air` -> openclaw leak. + it('routes both spellings of SSH ownership to their own host', () => { + const legacy = resolveAgentBackgroundLaunchHost({ + store: makeFolderHostState({ connectionId: null, folderPath: '/project' }) as never, + worktreeId: 'repo-1::/srv/repo', + worktreePath: '/srv/repo', + repo: { + id: 'repo-1', + connectionId: 'm4air', + executionHostId: null, + path: '/srv/repo' + } as never + }) + const unified = resolveAgentBackgroundLaunchHost({ + store: makeFolderHostState({ connectionId: null, folderPath: '/project' }) as never, + worktreeId: 'repo-1::/srv/repo', + worktreePath: '/srv/repo', + repo: { + id: 'repo-1', + connectionId: null, + executionHostId: 'ssh:openclaw', + path: '/srv/repo' + } as never + }) + + expect(legacy).toMatchObject({ + connectionId: 'm4air', + isRemote: true, + expectedConnectionId: 'm4air' + }) + expect(unified).toMatchObject({ + connectionId: 'openclaw', + isRemote: true, + expectedConnectionId: 'openclaw' + }) + }) + + it('keeps a local row with a stale connection off the SSH route', () => { + const host = resolveAgentBackgroundLaunchHost({ + store: makeFolderHostState({ connectionId: null, folderPath: '/project' }) as never, + worktreeId: 'repo-1::/srv/repo', + worktreePath: '/srv/repo', + repo: { + id: 'repo-1', + connectionId: 'm4air', + executionHostId: 'local', + path: '/srv/repo' + } as never + }) + + expect(host).toMatchObject({ + connectionId: null, + isRemote: false, + expectedConnectionId: null + }) + }) + + it('keeps a runtime host reaching a nested SSH target remote', () => { + const host = resolveAgentBackgroundLaunchHost({ + store: makeFolderHostState({ connectionId: null, folderPath: '/project' }) as never, + worktreeId: 'repo-1::/srv/repo', + worktreePath: '/srv/repo', + repo: { + id: 'repo-1', + connectionId: 'nested', + executionHostId: 'runtime:vm-1', + path: '/srv/repo' + } as never + }) + + expect(host).toMatchObject({ connectionId: 'nested', isRemote: true }) + }) + it('uses Linux startup quoting for a local WSL folder', () => { const folderPath = '\\\\wsl.localhost\\Ubuntu\\home\\me\\project' const host = resolveAgentBackgroundLaunchHost({ diff --git a/src/renderer/src/lib/agent-background-session-launch-host.ts b/src/renderer/src/lib/agent-background-session-launch-host.ts index 7919f9f3af5..300dec7f6ff 100644 --- a/src/renderer/src/lib/agent-background-session-launch-host.ts +++ b/src/renderer/src/lib/agent-background-session-launch-host.ts @@ -6,6 +6,7 @@ import { getFolderWorkspaceConnectionId } from '@/lib/folder-workspace-connectio import { parseWorkspaceKey } from '../../../shared/workspace-scope' import { isWindowsAbsolutePathLike } from '../../../shared/cross-platform-path' import { repoIsRemote } from '../../../shared/agent-launch-remote' +import { getRepoSshConnectionId } from '../../../shared/execution-host' import { isWslUncPath } from '../../../shared/wsl-paths' type LaunchStore = ReturnType @@ -41,14 +42,18 @@ export function resolveAgentBackgroundLaunchHost(args: { }): AgentBackgroundLaunchHost { const { store, worktreeId, worktreePath, repo } = args if (repo) { + // Why: SSH ownership has two spellings, so the raw field spawns an `executionHostId: 'ssh:*'`-only + // repo on the client with a remote path. One resolution feeds the route, the trust write and the + // launch shape, which must not disagree about the host. + const sshConnectionId = getRepoSshConnectionId(repo) return { - connectionId: repo.connectionId ?? null, + connectionId: sshConnectionId, platform: getAgentLaunchPlatformForRepo( repo, - repo.connectionId ? undefined : getLocalProjectExecutionRuntimeContext(store, worktreeId) + sshConnectionId ? undefined : getLocalProjectExecutionRuntimeContext(store, worktreeId) ), isRemote: repoIsRemote(repo), - expectedConnectionId: repo.connectionId ?? null + expectedConnectionId: sshConnectionId } } const folderWorkspaceConnectionId = resolveFolderWorkspaceConnectionIdForLaunch(store, worktreeId) diff --git a/src/renderer/src/lib/launch-agent-in-new-tab-host-resolution.test.ts b/src/renderer/src/lib/launch-agent-in-new-tab-host-resolution.test.ts new file mode 100644 index 00000000000..bcb09c1b17c --- /dev/null +++ b/src/renderer/src/lib/launch-agent-in-new-tab-host-resolution.test.ts @@ -0,0 +1,167 @@ +// Execution-host coverage for launchAgentInNewTab, split from launch-agent-in-new-tab.test.ts to +// keep both files within the lines budget. + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockCreateTab = vi.fn() +const mockQueueTabStartupCommand = vi.fn() + +type StoreRepo = { + id: string + connectionId: string | null + executionHostId?: string | null + path: string +} + +type StoreWorktree = { + id: string + repoId: string + projectId: string + hostId?: string | null + path: string + displayName: string +} + +const store = { + activeRepoId: 'repo-1', + activeWorktreeId: 'wt-1', + settings: { + agentCmdOverrides: {} as Record, + agentDefaultArgs: {} as Record, + agentDefaultEnv: {} as Record>, + activeRuntimeEnvironmentId: null as string | null + }, + projects: [{ id: 'repo-1', localWindowsRuntimePreference: { kind: 'inherit-global' as const } }], + repos: [] as StoreRepo[], + folderWorkspaces: [] as unknown[], + projectGroups: [] as unknown[], + sshConnectionStates: new Map(), + transientClearedAgentStatusConnectionIds: {} as Record, + worktreesByRepo: {} as Record, + allWorktrees: vi.fn(() => store.worktreesByRepo['repo-1'] ?? []), + tabsByWorktree: { 'wt-1': [{ id: 'tab-1' }] }, + openFiles: [] as { id: string; worktreeId: string }[], + browserTabsByWorktree: {} as Record, + tabBarOrderByWorktree: {} as Record, + terminalLayoutsByTabId: {} as Record< + string, + { activeLeafId: string | null; ptyIdsByLeafId?: Record } + >, + ptyIdsByTabId: {} as Record, + createTab: mockCreateTab, + closeTab: vi.fn(), + queueTabStartupCommand: mockQueueTabStartupCommand, + setActiveTabType: vi.fn(), + setTabBarOrder: vi.fn(), + setAgentStatus: vi.fn(), + seedNativeChatLaunchPrompt: vi.fn(), + seedNativeChatLaunchDraft: vi.fn(), + markNativeChatLaunchPromptFailed: vi.fn() +} + +vi.mock('@/store', () => ({ useAppStore: { getState: () => store } })) + +vi.mock('sonner', () => ({ toast: { message: vi.fn(), error: vi.fn() } })) + +vi.mock('@/components/tab-bar/reconcile-order', () => ({ + reconcileTabOrder: vi.fn( + (_stored, termIds: string[], editorIds: string[], browserIds: string[]) => [ + ...termIds, + ...editorIds, + ...browserIds + ] + ) +})) + +vi.mock('@/lib/agent-paste-draft', () => ({ pasteDraftWhenAgentReady: vi.fn() })) + +vi.mock('@/lib/telemetry', () => ({ + track: vi.fn(), + tuiAgentToAgentKind: (agent: string) => agent +})) + +vi.mock('@/runtime/web-runtime-session', () => ({ + createWebRuntimeSessionTerminal: vi.fn(), + isWebRuntimeSessionActive: vi.fn(() => false), + isWebTerminalSurfaceTabId: vi.fn(() => false) +})) + +function worktreeOn(hostId: string, path: string): StoreWorktree { + return { id: 'wt-1', repoId: 'repo-1', projectId: 'repo-1', hostId, path, displayName: 'main' } +} + +async function launchOnLinux(): Promise { + const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') + launchAgentInNewTab({ agent: 'claude-agent-teams', worktreeId: 'wt-1', launchPlatform: 'linux' }) +} + +function queuedCommand(): string { + return mockQueueTabStartupCommand.mock.calls[0]?.[1]?.command +} + +describe('launchAgentInNewTab execution host resolution', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCreateTab.mockReturnValue({ id: 'tab-1' }) + store.settings = { + agentCmdOverrides: {}, + agentDefaultArgs: {}, + agentDefaultEnv: {}, + activeRuntimeEnvironmentId: null + } + store.tabsByWorktree = { 'wt-1': [{ id: 'tab-1' }] } + store.openFiles = [] + store.browserTabsByWorktree = {} + store.tabBarOrderByWorktree = {} + store.terminalLayoutsByTabId = {} + store.ptyIdsByTabId = {} + }) + + it('shapes the launch from the worktree host, not a rival repo row on another SSH host', async () => { + // `store.repos.find` is host-blind, so a worktree that names its own host could be shaped by + // an `ssh:openclaw` row it has nothing to do with (#11163). + store.repos = [ + { id: 'repo-1', connectionId: 'openclaw', path: '/srv/openclaw' }, + { id: 'repo-1', connectionId: null, executionHostId: 'local', path: '/repo' } + ] + store.worktreesByRepo = { 'repo-1': [worktreeOn('local', '/repo/worktree')] } + + await launchOnLinux() + + expect(queuedCommand()).toBe("orca-ide claude-teams '--dangerously-skip-permissions'") + }) + + it('keeps a worktree on one SSH host remote while a rival row names another', async () => { + store.repos = [ + { id: 'repo-1', connectionId: 'openclaw', path: '/srv/openclaw' }, + { id: 'repo-1', connectionId: null, executionHostId: 'ssh:m4air', path: '/srv/m4air' } + ] + store.worktreesByRepo = { 'repo-1': [worktreeOn('ssh:m4air', '/srv/m4air/worktree')] } + + await launchOnLinux() + + expect(queuedCommand()).toBe("orca claude-teams '--dangerously-skip-permissions'") + }) + + it('keeps a runtime host reaching a nested SSH target on the relay shim name', async () => { + store.repos = [ + { id: 'repo-1', connectionId: 'nested', executionHostId: 'runtime:vm-1', path: '/srv/vm' } + ] + store.worktreesByRepo = { 'repo-1': [worktreeOn('runtime:vm-1', '/srv/vm/worktree')] } + + await launchOnLinux() + + expect(queuedCommand()).toBe("orca claude-teams '--dangerously-skip-permissions'") + }) + + it('keeps a runtime host with no nested SSH target on the local CLI name', async () => { + store.repos = [ + { id: 'repo-1', connectionId: null, executionHostId: 'runtime:vm-1', path: '/srv/vm' } + ] + store.worktreesByRepo = { 'repo-1': [worktreeOn('runtime:vm-1', '/srv/vm/worktree')] } + + await launchOnLinux() + + expect(queuedCommand()).toBe("orca-ide claude-teams '--dangerously-skip-permissions'") + }) +}) diff --git a/src/renderer/src/lib/launch-agent-in-new-tab.ts b/src/renderer/src/lib/launch-agent-in-new-tab.ts index b6cbbb736d8..bf4eda09890 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab.ts @@ -22,7 +22,6 @@ import { } from '../../../shared/tui-agent-launch-defaults' import { resolveLocalWindowsAgentStartupShell } from '../../../shared/windows-terminal-shell' import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' -import { repoIsRemote } from '../../../shared/agent-launch-remote' import { seedCommandCodeSubmittedPromptStatus } from '@/lib/command-code-prompt-status-seed' import type { TuiAgent } from '../../../shared/tui-agent' import type { LaunchSource } from '../../../shared/telemetry-events' @@ -96,16 +95,23 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI const store = useAppStore.getState() const worktree = store.allWorktrees?.().find((entry: { id: string }) => entry.id === worktreeId) const repo = worktree ? store.repos?.find((entry) => entry.id === worktree.repoId) : null + // Why: `store.repos.find` is host-blind and the same repo id can exist on local, SSH and runtime + // hosts, so the row it returns can belong to a different host than the worktree names (#11163). + // The shared resolver answers from the worktree's own host; `undefined` (rival rows disagree) is + // not evidence of a remote, and main rejects that launch anyway. + const worktreeSshConnectionId = getConnectionIdFromState(store, worktreeId) const resolvedLaunchPlatform = launchPlatform ?? (repo ? getAgentLaunchPlatformForRepo( repo, - repo.connectionId ? undefined : getLocalProjectExecutionRuntimeContext(store, worktreeId) + worktreeSshConnectionId + ? undefined + : getLocalProjectExecutionRuntimeContext(store, worktreeId) ) : CLIENT_PLATFORM) // Why: SSH remotes deploy the shim as plain `orca`, so skip the Linux-only `orca-ide` rename for remote launches. - const isRemote = repo ? repoIsRemote(repo) : false + const isRemote = Boolean(worktreeSshConnectionId) const queuedShell = resolveLocalWindowsAgentStartupShell({ platform: resolvedLaunchPlatform, isRemote, @@ -127,9 +133,8 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI agent, promptDelivery: viewModePromptDelivery, launchDraftText: trimmedPrompt, - nativeChatTranscriptIsLocalReadable: isNativeChatTranscriptLocalReadable( - getConnectionIdFromState(store, worktreeId) - ) + nativeChatTranscriptIsLocalReadable: + isNativeChatTranscriptLocalReadable(worktreeSshConnectionId) } const initialViewModeProps = initialAgentTabViewModeProps(store.settings, initialViewModeOptions) const startupPlanBase = { diff --git a/src/shared/agent-launch-remote.test.ts b/src/shared/agent-launch-remote.test.ts new file mode 100644 index 00000000000..4656dab39fc --- /dev/null +++ b/src/shared/agent-launch-remote.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { repoIsRemote } from './agent-launch-remote' + +describe('repoIsRemote', () => { + it('reads both spellings of SSH ownership on two different hosts', () => { + // Why two hosts: a single-host fixture passes even when the predicate answers from the wrong + // row, which is how the `ssh:m4air` -> openclaw leak survived review. + expect(repoIsRemote({ connectionId: 'm4air', executionHostId: null })).toBe(true) + expect(repoIsRemote({ connectionId: null, executionHostId: 'ssh:openclaw' })).toBe(true) + expect(repoIsRemote({ connectionId: 'm4air', executionHostId: 'ssh:m4air' })).toBe(true) + }) + + it('answers local for a row that declares itself local with a stale connection', () => { + expect(repoIsRemote({ connectionId: 'm4air', executionHostId: 'local' })).toBe(false) + }) + + it('keeps a runtime host with a nested SSH target remote', () => { + expect(repoIsRemote({ connectionId: 'nested-target', executionHostId: 'runtime:vm-1' })).toBe( + true + ) + }) + + it('keeps a runtime host with no nested SSH target local-shaped', () => { + // A runtime with no nested target is a full Orca install, not a relay shim, so it keeps the + // platform CLI name. + expect(repoIsRemote({ connectionId: null, executionHostId: 'runtime:vm-1' })).toBe(false) + }) + + it('keeps plain local and WSL rows local', () => { + expect(repoIsRemote({ connectionId: null, executionHostId: null })).toBe(false) + expect(repoIsRemote({ connectionId: null, executionHostId: 'local' })).toBe(false) + }) +}) diff --git a/src/shared/agent-launch-remote.ts b/src/shared/agent-launch-remote.ts index 08482815859..bec5aae49b9 100644 --- a/src/shared/agent-launch-remote.ts +++ b/src/shared/agent-launch-remote.ts @@ -1,11 +1,26 @@ +import type { Repo } from './repo-types' +import { getRepoSshConnectionId } from './execution-host' + /** - * Why: a repo reached over SSH runs the Orca CLI through the relay shim, which - * is always deployed as plain `orca` (Unix) / `orca.cmd` (Windows). The - * Linux-only `orca-ide` rename — which exists solely to avoid shadowing the - * GNOME Orca screen reader on a local desktop — must not be applied to those - * remotes, or `orca-ide claude-teams` lands on a PATH where it does not exist. - * `connectionId` is the SSH signal; WSL and local stay false. + * Why: a repo reached over SSH runs the Orca CLI through the relay shim, which is always deployed + * as plain `orca` (Unix) / `orca.cmd` (Windows). The Linux-only `orca-ide` rename — which exists + * solely to avoid shadowing the GNOME Orca screen reader on a local desktop — must not be applied + * to those remotes, or `orca-ide claude-teams` lands on a PATH where it does not exist. + * + * The question is "does an SSH target hold this row's files", not "what may this client dial", so + * it resolves the execution host instead of reading the raw `connectionId` field. SSH ownership has + * two spellings and the raw read is wrong in both directions: + * + * - a row carrying only `executionHostId: 'ssh:'` reads as local and gets the `orca-ide` + * rename it cannot resolve on the remote; + * - a row that declares itself `local` while a stale `connectionId` survives reads as remote and + * loses the rename it needs on a Linux desktop. + * + * `runtime:` keeps its nested SSH target (that machine reaches the files through its own relay + * shim), while a runtime host with no nested target is a full Orca install and stays false — as do + * WSL and local. Callers routing a client-local PTY want `getSshTargetIdForExecutionHost` instead; + * callers that already hold a resolved launch connection should read that, not re-derive here. */ -export function repoIsRemote(repo: { connectionId?: string | null }): boolean { - return Boolean(repo.connectionId) +export function repoIsRemote(repo: Pick): boolean { + return getRepoSshConnectionId(repo) !== null }