From 8a23618310968bd3ab7ad37db89929414910d690 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 23 Jul 2026 02:33:27 -0700 Subject: [PATCH] fix(terminal): route host-agnostic setup and floating terminals instead of failing them closed (#10151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inline setup/onboarding terminals (skill installs, feature tips) and the floating terminal use host-agnostic synthetic worktree ids with no worktree/repo row. Since #9994, connectPanePty gated their transport on resolveWorktreeOperationRouteResult, which fails closed ('missing') for unknown ids — so the Orca CLI skill Update button, every other inline setup terminal, and the floating terminal showed "Workspace identity is ambiguous across hosts" instead of running. Route them through the shared resolveTerminalWorktreeRoute (which already exempts the floating terminal and folder workspaces), and add the missing ephemeral-setup exemption so setup terminals follow the single active runtime (remote skill installs land there) or run locally when none is focused. Genuinely unknown/stale repo-backed worktrees still fail closed, preserving #9994. Adds terminal-worktree-route unit tests and connectPanePty regression tests (proven to fail without the fix). Co-authored-by: Orca --- .../terminal-pane/pty-connection.test.ts | 84 +++++++++++++++++++ .../terminal-pane/pty-connection.ts | 16 ++-- .../src/lib/terminal-worktree-route.test.ts | 72 ++++++++++++++++ .../src/lib/terminal-worktree-route.ts | 8 ++ 4 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/lib/terminal-worktree-route.test.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 634f5f5b4..7374e99e5 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -15122,6 +15122,90 @@ describe('connectPanePty', () => { expect(createRemoteRuntimePtyTransport).toHaveBeenCalledWith('legacy-hub', expect.any(Object)) }) + it('runs an inline setup terminal locally instead of failing its host closed', async () => { + // Regression (#9994 fallout): the branded ephemeral-setup id resolves to no worktree/repo, so + // the strict owner resolver reported it unresolved and gave the pane the "Workspace identity is + // ambiguous across hosts" error transport instead of a real local PTY. + const { connectPanePty } = await import('./pty-connection') + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { createIpcPtyTransport } = await import('./pty-transport') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + const setupWorktreeId = + 'ephemeral-setup-terminal:settings-mobile-emulator-orca-cli-skill-terminal' + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { [setupWorktreeId]: [{ id: 'tab-1', ptyId: null }] }, + worktreesByRepo: { + repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1', hostId: 'local' }] + }, + repos: [{ id: 'repo1', connectionId: null, executionHostId: 'local' }] + } as StoreState + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ worktreeId: setupWorktreeId }) as never + ) + + expect(createIpcPtyTransport).toHaveBeenCalled() + expect(createRemoteRuntimePtyTransport).not.toHaveBeenCalled() + }) + + it('runs an inline setup terminal on the single active runtime for remote skill installs', async () => { + const { connectPanePty } = await import('./pty-connection') + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + const setupWorktreeId = + 'ephemeral-setup-terminal:settings-mobile-emulator-orca-cli-skill-terminal' + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { [setupWorktreeId]: [{ id: 'tab-1', ptyId: null }] }, + worktreesByRepo: { + repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1', hostId: 'local' }] + }, + repos: [{ id: 'repo1', connectionId: null, executionHostId: 'local' }], + runtimeEnvironments: [{ id: 'hub-a' }], + settings: { ...mockStoreState.settings, activeRuntimeEnvironmentId: 'hub-a' } + } as StoreState + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ worktreeId: setupWorktreeId }) as never + ) + + expect(createRemoteRuntimePtyTransport).toHaveBeenCalledWith('hub-a', expect.any(Object)) + }) + + it('keeps the floating terminal local even while a runtime is active', async () => { + const { connectPanePty } = await import('./pty-connection') + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { createIpcPtyTransport } = await import('./pty-transport') + const transport = createMockTransport() + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'global-floating-terminal': [{ id: 'tab-1', ptyId: null }] }, + worktreesByRepo: { + repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1', hostId: 'local' }] + }, + repos: [{ id: 'repo1', connectionId: null, executionHostId: 'local' }], + runtimeEnvironments: [{ id: 'hub-a' }], + settings: { ...mockStoreState.settings, activeRuntimeEnvironmentId: 'hub-a' } + } as StoreState + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ worktreeId: 'global-floating-terminal' }) as never + ) + + expect(createIpcPtyTransport).toHaveBeenCalled() + expect(createRemoteRuntimePtyTransport).not.toHaveBeenCalled() + }) + it('routes a HUB-owned SSH PTY wake hint through the HUB without direct SSH', async () => { const { connectPanePty } = await import('./pty-connection') const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 8a410c4e0..40785b1cc 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -35,7 +35,7 @@ import { createIpcPtyTransport } from './pty-transport' import { createRemoteRuntimePtyTransport } from './remote-runtime-pty-transport' import { toAgentLaunchPreferences } from '@/runtime/agent-session-create-operation' import { createUnresolvedOwnerPtyTransport } from './unresolved-owner-pty-transport' -import { resolveWorktreeOperationRouteResult } from '@/lib/worktree-operation-route' +import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route' import { getConnectionId } from '@/lib/connection-context' import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' import { @@ -3205,11 +3205,13 @@ export function connectPanePty( deps.restoredLeafId && deps.restoredPtyIdByLeafId ? (deps.restoredPtyIdByLeafId[deps.restoredLeafId] ?? null) : null - const operationRouteResolution = resolveWorktreeOperationRouteResult(state, deps.worktreeId) - const explicitRuntimeEnvironmentId = - operationRouteResolution.kind === 'resolved' - ? operationRouteResolution.route.runtimeEnvironmentId - : null + // Why: the floating terminal and inline setup/onboarding terminals are host-agnostic synthetic + // ids with no worktree/repo row, so the strict owner resolver reports them as unresolved. The + // shared terminal router scopes them to their floating owner (local for the floating terminal, + // the active runtime for setup terminals so remote skill installs land there) and returns null + // only for a genuinely unknown/stale worktree that must fail closed (#9994). + const terminalWorktreeRoute = resolveTerminalWorktreeRoute(state, deps.worktreeId) + const explicitRuntimeEnvironmentId = terminalWorktreeRoute?.runtimeEnvironmentId ?? null // Why: paired-web worktrees retain HUB execution identity; their runtime-scoped mirrored pane is the session-level transport owner. const mirroredRuntimeOwners = new Set( isWebTerminalSurfaceTabId(deps.tabId) @@ -3221,7 +3223,7 @@ export function connectPanePty( const mirroredRuntimeEnvironmentId = mirroredRuntimeOwners.values().next().value ?? null const terminalOwnerUnresolved = mirroredRuntimeOwners.size > 1 || - (operationRouteResolution.kind !== 'resolved' && !mirroredRuntimeEnvironmentId) + (terminalWorktreeRoute === null && !mirroredRuntimeEnvironmentId) const runtimeEnvironmentId = explicitRuntimeEnvironmentId ? explicitRuntimeEnvironmentId : mirroredRuntimeEnvironmentId diff --git a/src/renderer/src/lib/terminal-worktree-route.test.ts b/src/renderer/src/lib/terminal-worktree-route.test.ts new file mode 100644 index 000000000..396c3df98 --- /dev/null +++ b/src/renderer/src/lib/terminal-worktree-route.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import type { AppState } from '@/store/types' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import { brandEphemeralSetupTerminalWorktreeId } from '../../../shared/ephemeral-setup-terminal-worktree-id' +import { folderWorkspaceKey } from '../../../shared/workspace-scope' +import { resolveTerminalWorktreeRoute } from './terminal-worktree-route' + +const EPHEMERAL_ID = brandEphemeralSetupTerminalWorktreeId( + 'settings-mobile-emulator-orca-cli-skill-terminal' +) + +// A realistic local-only store: one real repo/worktree, hydrated empty runtime catalog. +function localState(overrides: Partial = {}): AppState { + return { + repos: [{ id: 'repo-1', connectionId: null, executionHostId: 'local' }], + worktreesByRepo: { 'repo-1': [{ id: 'repo-1::/w', repoId: 'repo-1', hostId: 'local' }] }, + runtimeEnvironments: [], + runtimeEnvironmentCatalogHydrated: true, + removedRuntimeEnvironmentIds: new Set(), + ...overrides + } as unknown as AppState +} + +describe('resolveTerminalWorktreeRoute', () => { + it('keeps the floating terminal local', () => { + expect(resolveTerminalWorktreeRoute(localState(), FLOATING_TERMINAL_WORKTREE_ID)).toEqual({ + runtimeEnvironmentId: null + }) + }) + + it('routes an ephemeral setup terminal locally when no runtime is active', () => { + // Regression: previously returned null (unroutable), producing the + // "Workspace identity is ambiguous across hosts" error transport (#9994 fallout). + expect(resolveTerminalWorktreeRoute(localState(), EPHEMERAL_ID)).toEqual({ + runtimeEnvironmentId: null + }) + }) + + it('scopes an ephemeral setup terminal to the single active runtime for remote skill installs', () => { + const state = localState({ + settings: { activeRuntimeEnvironmentId: 'hub-a' }, + runtimeEnvironments: [{ id: 'hub-a' }] + } as unknown as Partial) + expect(resolveTerminalWorktreeRoute(state, EPHEMERAL_ID)).toEqual({ + runtimeEnvironmentId: 'hub-a' + }) + }) + + it('does not guess a runtime for an ephemeral setup terminal when the focus is ambiguous', () => { + const state = localState({ + settings: { activeRuntimeEnvironmentId: 'hub-a' }, + runtimeEnvironments: [{ id: 'hub-a' }, { id: 'hub-b' }] + } as unknown as Partial) + expect(resolveTerminalWorktreeRoute(state, EPHEMERAL_ID)).toEqual({ + runtimeEnvironmentId: null + }) + }) + + it('resolves a known local worktree', () => { + expect(resolveTerminalWorktreeRoute(localState(), 'repo-1::/w')).toEqual({ + runtimeEnvironmentId: null + }) + }) + + it('still fails a genuinely unknown/stale worktree closed', () => { + expect(resolveTerminalWorktreeRoute(localState(), 'repo-9::/stale')).toBeNull() + }) + + it('does not treat a folder workspace as an unresolved worktree', () => { + expect(resolveTerminalWorktreeRoute(localState(), folderWorkspaceKey('abc-123'))).not.toBeNull() + }) +}) diff --git a/src/renderer/src/lib/terminal-worktree-route.ts b/src/renderer/src/lib/terminal-worktree-route.ts index 58e900e91..fc001125d 100644 --- a/src/renderer/src/lib/terminal-worktree-route.ts +++ b/src/renderer/src/lib/terminal-worktree-route.ts @@ -1,4 +1,5 @@ import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import { isEphemeralSetupTerminalWorktreeId } from '../../../shared/ephemeral-setup-terminal-worktree-id' import { parseWorkspaceKey } from '../../../shared/workspace-scope' import type { AppState } from '@/store/types' import { getRuntimeEnvironmentIdForWorktree } from './worktree-runtime-owner' @@ -22,6 +23,13 @@ export function resolveTerminalWorktreeRoute( ) { return { runtimeEnvironmentId: getRuntimeEnvironmentIdForWorktree(state, worktreeId) } } + // Why: inline setup/onboarding terminals (skill installs, feature tips) have no worktree row, + // so the strict owner resolver reports them as an unresolved cross-host worktree. Scope them to + // the active runtime — so a remote skill install lands on that runtime — falling back to local + // when none is focused, instead of failing them closed. + if (isEphemeralSetupTerminalWorktreeId(worktreeId)) { + return { runtimeEnvironmentId: getSingleFocusedRuntimeEnvironmentId(state) } + } const resolution = resolveWorktreeOperationRouteResult(state, worktreeId) if (resolution.kind === 'resolved') { return { runtimeEnvironmentId: resolution.route.runtimeEnvironmentId }