diff --git a/src/main/ipc/remote-workspace-patch-queue.test.ts b/src/main/ipc/remote-workspace-patch-queue.test.ts index 30ff0af7761..6b5d342b3f6 100644 --- a/src/main/ipc/remote-workspace-patch-queue.test.ts +++ b/src/main/ipc/remote-workspace-patch-queue.test.ts @@ -77,8 +77,11 @@ describe('remoteWorkspace:setForConnectedTargets patch queue', () => { const handlers = new Map unknown>() const muxByTargetId = new Map }>() const getRepoMock = vi.fn() + // Ownership resolution reads the catalog, not one id-keyed row, so the fake has to project one. + const KNOWN_REPO_IDS = ['repo-target-1', 'repo-target-2', 'repo-reset', 'repo-newer'] const store = { - getRepo: getRepoMock + getRepo: getRepoMock, + getRepos: () => KNOWN_REPO_IDS.map((repoId) => getRepoMock(repoId)).filter(Boolean) } as unknown as Store const target: SshTarget = { diff --git a/src/main/ipc/remote-workspace.test.ts b/src/main/ipc/remote-workspace.test.ts index 434f8c7dec4..56eb4804a82 100644 --- a/src/main/ipc/remote-workspace.test.ts +++ b/src/main/ipc/remote-workspace.test.ts @@ -153,8 +153,10 @@ describe('remoteWorkspace:setForConnectedTargets', () => { const muxByTargetId = new Map }>() const getRepoMock = vi.fn() const getWorkspaceSessionMock = vi.fn() + // Ownership resolution reads the catalog, not one id-keyed row, so the fake has to project one. const store = { getRepo: getRepoMock, + getRepos: () => [getRepoMock('repo-target-1')].filter(Boolean), getWorkspaceSession: getWorkspaceSessionMock } as unknown as Store diff --git a/src/main/ipc/remote-workspace.ts b/src/main/ipc/remote-workspace.ts index 74339e8f694..f9479f15ce9 100644 --- a/src/main/ipc/remote-workspace.ts +++ b/src/main/ipc/remote-workspace.ts @@ -12,7 +12,10 @@ import { } from '../../shared/remote-workspace-types' import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' import { getRepoIdFromWorktreeId } from '../../shared/worktree/id' -import { parseExecutionHostId } from '../../shared/execution-host' +import { + createRepoRowExecutionHostLookup, + resolveWorktreeExecutionHost +} from '../../shared/worktree-execution-host-resolution' import { getRemoteWorkspaceNamespace } from './remote-workspace-namespace' import { registerRemoteWorkspaceNotificationHandler } from './remote-workspace-events' import { CLIENT_ID } from './remote-workspace-client-identity' @@ -108,12 +111,15 @@ function targetForWorktree( worktreeId: string, executionHostId?: string ): string | null { - const parsedHostId = parseExecutionHostId(executionHostId) - if (parsedHostId?.kind === 'ssh') { - return parsedHostId.targetId - } - const repoId = getRepoIdFromWorktreeId(worktreeId) - return store.getRepo(repoId)?.connectionId ?? null + // Why: this decides which SSH target a workspace session is exported to. The old fallback read + // `getRepo(id)?.connectionId`, which is host-blind — the same repo id can name rows on several + // hosts, so a session could be published to a machine that never owned the worktree (#11163). + // Unresolvable ownership exports to nobody rather than guessing. + const resolution = resolveWorktreeExecutionHost( + createRepoRowExecutionHostLookup(store.getRepos()), + { repoId: getRepoIdFromWorktreeId(worktreeId), hostId: executionHostId ?? null } + ) + return resolution.kind === 'resolved' ? resolution.connectionId : null } function exportSessionForTarget( diff --git a/src/main/ipc/repos/remote-repo-registration.test.ts b/src/main/ipc/repos/remote-repo-registration.test.ts new file mode 100644 index 00000000000..0a7069b1f63 --- /dev/null +++ b/src/main/ipc/repos/remote-repo-registration.test.ts @@ -0,0 +1,124 @@ +// Registration is now the runtime's SSH path too (`projectHostSetup.setupExistingFolder --host +// ssh:*`), so what it stamps decides what every downstream host resolver can read. It minted +// `connectionId`-only rows, leaving the unified spelling permanently empty, and deduped by raw +// `connectionId`, which cannot see a row stamped `executionHostId: 'ssh:*'`. +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../shared/repo-types' + +const getSshGitProviderMock = vi.hoisted(() => vi.fn()) +vi.mock('../../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: getSshGitProviderMock +})) + +vi.mock('../../repo-icon-autodetect', () => ({ + detectRepoIconAndUpstream: vi.fn(async () => ({})) +})) + +vi.mock('../../ssh/ssh-target-registry', () => ({ + getActiveMultiplexer: vi.fn(() => null) +})) + +vi.mock('./remote-home-path', () => ({ + resolveRemoteHomePath: vi.fn(async (_connectionId: string, path: string) => path) +})) + +import { addRemoteRepoFromPath } from './remote-repo-registration' + +function makeStore(repos: Repo[]) { + return { + getRepos: () => repos, + getSshTarget: () => undefined, + addRepo: (repo: Repo) => { + repos.push(repo) + } + } +} + +describe('addRemoteRepoFromPath', () => { + beforeEach(() => { + getSshGitProviderMock.mockReset() + getSshGitProviderMock.mockReturnValue({ + isGitRepoAsync: vi.fn(async () => ({ isRepo: true, rootPath: '/srv/app' })) + }) + }) + + it('stamps the unified execution-host spelling alongside the legacy connection id', async () => { + const repos: Repo[] = [] + const result = await addRemoteRepoFromPath(makeStore(repos) as never, { + connectionId: 'm4air', + remotePath: '/srv/app' + }) + + expect('error' in result).toBe(false) + const repo = (result as { repo: Repo }).repo + expect(repo.connectionId).toBe('m4air') + expect(repo.executionHostId).toBe('ssh:m4air') + }) + + it('dedupes against a row that names the host in the unified spelling only', async () => { + const existing = { + id: 'existing', + path: '/srv/app', + displayName: 'app', + badgeColor: '#000', + addedAt: 0, + executionHostId: 'ssh:m4air' + } as Repo + const repos: Repo[] = [existing] + + const result = await addRemoteRepoFromPath(makeStore(repos) as never, { + connectionId: 'm4air', + remotePath: '/srv/app' + }) + + expect(result).toEqual({ repo: existing, alreadyExisted: true }) + expect(repos).toHaveLength(1) + }) + + it('does not dedupe onto a row on a different SSH host at the same path', async () => { + // Two hosts can both hold /srv/app. Matching on path alone registers one host's repo as the + // other's — the mirror image of the id-only lookup this change removes. + const repos: Repo[] = [ + { + id: 'openclaw-row', + path: '/srv/app', + displayName: 'app', + badgeColor: '#000', + addedAt: 0, + connectionId: 'openclaw' + } as Repo + ] + + const result = await addRemoteRepoFromPath(makeStore(repos) as never, { + connectionId: 'm4air', + remotePath: '/srv/app' + }) + + expect((result as { alreadyExisted: boolean }).alreadyExisted).toBe(false) + expect((result as { repo: Repo }).repo.executionHostId).toBe('ssh:m4air') + expect(repos).toHaveLength(2) + }) + + it('does not dedupe onto a local row that carries a stale connection id', async () => { + // The pullfrog case: a row declaring itself local must not answer as an SSH host. + const repos: Repo[] = [ + { + id: 'local-row', + path: '/srv/app', + displayName: 'app', + badgeColor: '#000', + addedAt: 0, + executionHostId: 'local', + connectionId: 'develop' + } as Repo + ] + + const result = await addRemoteRepoFromPath(makeStore(repos) as never, { + connectionId: 'develop', + remotePath: '/srv/app' + }) + + expect((result as { alreadyExisted: boolean }).alreadyExisted).toBe(false) + expect(repos).toHaveLength(2) + }) +}) diff --git a/src/main/ipc/repos/remote-repo-registration.ts b/src/main/ipc/repos/remote-repo-registration.ts index 110c027bc5a..35ab72056ae 100644 --- a/src/main/ipc/repos/remote-repo-registration.ts +++ b/src/main/ipc/repos/remote-repo-registration.ts @@ -3,7 +3,7 @@ import type { Store } from '../../persistence' import type { Repo } from '../../../shared/repo-types' import { DEFAULT_REPO_BADGE_COLOR } from '../../../shared/constants' import { normalizeRuntimePathForComparison } from '../../../shared/cross-platform-path' -import { getRepoSshConnectionId } from '../../../shared/execution-host' +import { getRepoSshConnectionId, toSshExecutionHostId } from '../../../shared/execution-host' import { getSshGitProvider } from '../../providers/ssh-git-dispatch' import { detectRepoIconAndUpstream } from '../../repo-icon-autodetect' import { getActiveMultiplexer } from '../../ssh/ssh-target-registry' @@ -95,6 +95,9 @@ export async function addRemoteRepoFromPath( addedAt: Date.now(), kind: repoKind, connectionId: args.connectionId, + // Stamp the unified spelling at creation: this is now the runtime's SSH registration path too, + // and minting `connectionId`-only rows leaves every host-resolving reader on the legacy field. + executionHostId: toSshExecutionHostId(args.connectionId), ...(repoKind === 'git' ? { externalWorktreeVisibilityLegacy: false, diff --git a/src/main/runtime/agent-terminal-launch-trust-host.test.ts b/src/main/runtime/agent-terminal-launch-trust-host.test.ts new file mode 100644 index 00000000000..949b0c545fc --- /dev/null +++ b/src/main/runtime/agent-terminal-launch-trust-host.test.ts @@ -0,0 +1,107 @@ +// launchAgentTerminal read `store.getRepo(worktree.repoId)?.connectionId` for the trust write — +// host-blind, so the same repo id on two hosts wrote a remote path into the client's agent config +// and the agent on the host never saw the trust (#11163). Every sibling call site already passes +// the resolved `workspace.connectionId`; this was the last one that did not. +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ + BrowserWindow: { fromId: vi.fn(() => null) }, + webContents: { fromId: vi.fn(() => null) }, + ipcMain: { on: vi.fn(), removeListener: vi.fn() }, + app: { getPath: vi.fn(() => '/tmp'), isPackaged: false } +})) + +import { OrcaRuntimeService } from './orca-runtime' + +const REMOTE_PATH = '/srv/app-feature' + +type RuntimeInternals = { + resolveWorktreeSelector: (selector: string) => Promise + buildStartupForAgent: (repo: unknown, agent: unknown, prompt: string) => unknown + markWorkspaceTrustedForAgent: ( + agent: unknown, + connectionId: string | null | undefined, + path: string + ) => Promise + createTerminal: (selector: string, opts: unknown) => Promise +} + +function makeRuntime(repos: readonly Record[], hostId?: string) { + const store = { + getSettings: () => ({ disabledTuiAgents: [], workspaceDir: '/tmp/workspaces' }), + getProjectHostSetups: () => [], + getRepos: () => repos, + getRepo: (id: string) => repos.find((repo) => repo.id === id) + } + const runtime = new OrcaRuntimeService(store as never) + const internals = runtime as unknown as RuntimeInternals + vi.spyOn(internals, 'resolveWorktreeSelector').mockResolvedValue({ + id: 'repo-shared::/srv/app-feature', + repoId: 'repo-shared', + path: REMOTE_PATH, + ...(hostId ? { hostId } : {}) + }) + vi.spyOn(internals, 'buildStartupForAgent').mockReturnValue({ + agent: 'codex', + startup: { command: 'codex', env: {}, startupCommandDelivery: 'none', telemetry: {} } + }) + const markTrusted = vi.fn(async () => {}) + vi.spyOn(internals, 'markWorkspaceTrustedForAgent').mockImplementation(markTrusted) + vi.spyOn(internals, 'createTerminal').mockResolvedValue({ id: 'pty-1' }) + return { runtime, markTrusted } +} + +describe('launchAgentTerminal trust write', () => { + beforeEach(() => { + vi.restoreAllMocks() + }) + + it('writes trust on the host the worktree names, not on a rival row', async () => { + // Two SSH hosts publish the same repo id; the worktree is on m4air. + const { runtime, markTrusted } = makeRuntime( + [ + { id: 'repo-shared', path: '/home/me/app', connectionId: 'openclaw' }, + { id: 'repo-shared', path: '/srv/app', connectionId: 'm4air' } + ], + 'ssh:m4air' + ) + + await runtime.launchAgentTerminal('id:repo-shared::/srv/app-feature', { + agent: 'codex', + prompt: 'go' + } as never) + + expect(markTrusted).toHaveBeenCalledWith('codex', 'm4air', REMOTE_PATH) + }) + + it('writes trust locally for a local worktree even when a remote row shares the id', async () => { + const { runtime, markTrusted } = makeRuntime( + [ + { id: 'repo-shared', path: '/srv/app', connectionId: 'm4air' }, + { id: 'repo-shared', path: '/home/me/app' } + ], + 'local' + ) + + await runtime.launchAgentTerminal('id:repo-shared::/srv/app-feature', { + agent: 'codex', + prompt: 'go' + } as never) + + expect(markTrusted).toHaveBeenCalledWith('codex', null, REMOTE_PATH) + }) + + it('refuses rather than guessing when rival rows disagree and the worktree names no host', async () => { + const { runtime } = makeRuntime([ + { id: 'repo-shared', path: '/srv/app', connectionId: 'm4air' }, + { id: 'repo-shared', path: '/home/me/app' } + ]) + + await expect( + runtime.launchAgentTerminal('id:repo-shared::/srv/app-feature', { + agent: 'codex', + prompt: 'go' + } as never) + ).rejects.toThrow('worktree_execution_host_unresolved') + }) +}) diff --git a/src/main/runtime/orca-runtime-terminal-create-deduplication.ts b/src/main/runtime/orca-runtime-terminal-create-deduplication.ts index a1743a855ce..cdb24994fce 100644 --- a/src/main/runtime/orca-runtime-terminal-create-deduplication.ts +++ b/src/main/runtime/orca-runtime-terminal-create-deduplication.ts @@ -8,6 +8,7 @@ import { PTY_CONTROLLER_LIST_TIMEOUT_MS } from './orca-runtime-postlude' import { inferWorktreeIdFromPtyId } from './runtime-worktree-path-identity' import { getRegisteredSshState } from '../ssh/ssh-target-registry' import { LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId } from '../../shared/execution-host' +import { resolveWorktreeLaunchHost } from './worktree-launch-host-repo' import type { TuiAgent } from '../../shared/tui-agent' export class OrcaRuntimeWithTerminalCreateDeduplication extends OrcaRuntimeWithCreateAgentSession { @@ -143,12 +144,20 @@ export class OrcaRuntimeWithTerminalCreateDeduplication extends OrcaRuntimeWithC opts: { agent: TuiAgent; prompt: string; title?: string } ): Promise { const worktree = await this.resolveWorktreeSelector(worktreeSelector) - const repo = this.store?.getRepo(worktree.repoId) + // Why: the trust write lands in an agent's config on the machine that runs it, keyed by the + // workspace path. `getRepo(id)` is host-blind, so reading `connectionId` off it wrote a remote + // path into the *client's* config — the agent on the host never sees the trust (#11163). + // Same shape as the folder-create trust write fixed alongside this; the agent-launch half. + const resolution = resolveWorktreeLaunchHost(this.store?.getRepos() ?? [], worktree) + if (resolution.kind === 'ambiguous') { + throw new Error('worktree_execution_host_unresolved') + } + const repo = resolution.repo ?? this.store?.getRepo(worktree.repoId) if (!repo) { throw new Error('Repository for the selected workspace is no longer available.') } const startup = this.buildStartupForAgent(repo, opts.agent, opts.prompt) - await this.markWorkspaceTrustedForAgent(opts.agent, repo.connectionId, worktree.path) + await this.markWorkspaceTrustedForAgent(opts.agent, resolution.connectionId, worktree.path) return await this.createTerminal(`id:${worktree.id}`, { command: startup.startup.command, env: startup.startup.env,