From e82dce43827010e30d3d0c6229d55cf74e1c8be0 Mon Sep 17 00:00:00 2001 From: Vladislav Meshkorudnyj Date: Tue, 30 Jun 2026 05:38:11 +0200 Subject: [PATCH] Fix/remote skill install floating terminal (#6816) * fix(terminal): scope skill-install terminals to the floating runtime selector (#6789) Inline setup/onboarding terminals (skill installers, feature tips) create a PTY under a synthetic per-panel worktree id. On a remote runtime the remote PTY transport sent that id as `id:` to `terminal.create`, which the runtime cannot resolve, so installing a skill via Settings failed with selector_not_found. Locally it worked because the IPC spawn uses cwd directly and never resolves the selector. These terminals are ephemeral floating terminals with no backing worktree, so brand their id and resolve it to the floating-terminal selector (global-floating-terminal), which every runtime already maps to the home dir. Local tab isolation keeps the distinct per-panel id; only the runtime terminal selector changes. The server is unchanged. * docs(terminal): add JSDoc for functions touched by the skill-install terminal fix Document the new ephemeral setup-terminal id helpers, the runtime terminal selector, and the onboarding/remote-transport entry points so the changed functions carry contract-level docstrings. --------- Co-authored-by: vladmesh --- .../OnboardingInlineCommandTerminal.tsx | 13 +++++++- .../remote-runtime-pty-transport.test.ts | 24 ++++++++++++++ .../remote-runtime-pty-transport.ts | 12 +++++-- .../runtime/runtime-worktree-selector.test.ts | 21 +++++++++++- .../src/runtime/runtime-worktree-selector.ts | 16 +++++++++ ...hemeral-setup-terminal-worktree-id.test.ts | 33 +++++++++++++++++++ .../ephemeral-setup-terminal-worktree-id.ts | 19 +++++++++++ 7 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 src/shared/ephemeral-setup-terminal-worktree-id.test.ts create mode 100644 src/shared/ephemeral-setup-terminal-worktree-id.ts diff --git a/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx b/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx index cf1cf53aa..9f7c8da13 100644 --- a/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx +++ b/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx @@ -5,6 +5,7 @@ import { PASTE_TERMINAL_TEXT_EVENT, type PasteTerminalTextDetail } from '@/const import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { useAppStore } from '@/store' import { translate } from '@/i18n/i18n' +import { brandEphemeralSetupTerminalWorktreeId } from '../../../../shared/ephemeral-setup-terminal-worktree-id' const ONBOARDING_INLINE_TERMINAL_WORKTREE_ID = 'onboarding-inline-terminal' const AUTO_INSERT_DELAY_MS = 250 @@ -30,6 +31,10 @@ type OnboardingInlineCommandTerminalProps = { onTerminalExit?: () => void } +/** + * Inline pane that runs a one-off setup command (skill install, feature tip) in an + * ephemeral floating-scoped terminal, auto-inserting the command once the PTY is ready. + */ export function OnboardingInlineCommandTerminal({ command, title, @@ -39,12 +44,18 @@ export function OnboardingInlineCommandTerminal({ terminalTopMarginPx = 20, descriptionPaddingClassName = 'px-4 py-3', autoScrollIntoView = true, - worktreeId = ONBOARDING_INLINE_TERMINAL_WORKTREE_ID, + worktreeId: worktreeIdProp = ONBOARDING_INLINE_TERMINAL_WORKTREE_ID, shellOverride, onOpened, onInteracted, onTerminalExit }: OnboardingInlineCommandTerminalProps): React.JSX.Element { + // Why: brand the id so a remote runtime scopes this ephemeral terminal to the + // floating terminal instead of rejecting the synthetic id. + const worktreeId = useMemo( + () => brandEphemeralSetupTerminalWorktreeId(worktreeIdProp), + [worktreeIdProp] + ) const createTab = useAppStore((s) => s.createTab) const closeTab = useAppStore((s) => s.closeTab) const setActiveTabForWorktree = useAppStore((s) => s.setActiveTabForWorktree) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index fd8cc3bc7..d11d07afe 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -550,6 +550,30 @@ describe('createRemoteRuntimePtyTransport', () => { ) }) + it('scopes ephemeral setup terminals to the floating-terminal selector (#6789)', async () => { + const { brandEphemeralSetupTerminalWorktreeId } = await import( + '../../../../shared/ephemeral-setup-terminal-worktree-id' + ) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: brandEphemeralSetupTerminalWorktreeId('feature-wall-orchestration-skill-terminal'), + tabId: 'tab-1', + leafId: 'pane:1' + }) + + await transport.connect({ url: '', callbacks: {} }) + + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ + selector: 'env-1', + method: 'terminal.create', + params: expect.objectContaining({ + worktree: 'id:global-floating-terminal' + }) + }) + ) + }) + it('passes startup command delivery when creating the remote runtime terminal', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const transport = createRemoteRuntimePtyTransport('env-1', { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 71cffb568..44f7745b7 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -22,7 +22,10 @@ import { getRemoteRuntimeTerminalMultiplexer, type RemoteRuntimeMultiplexedTerminal } from '../../runtime/remote-runtime-terminal-multiplexer' -import { toRuntimeWorktreeSelector } from '../../runtime/runtime-worktree-selector' +import { + toRuntimeTerminalWorktreeSelector, + toRuntimeWorktreeSelector +} from '../../runtime/runtime-worktree-selector' import { createRemoteRuntimePtyTextBatcher, createRemoteRuntimeViewportBatcher @@ -45,6 +48,11 @@ function isRemoteTerminalGoneMessage(message: string): boolean { ) } +/** + * PTY transport backing a renderer terminal pane with a terminal on a remote Orca + * runtime, over runtime RPC plus the multiplexed stream (create, subscribe, input, + * resize, close, reattach). + */ export function createRemoteRuntimePtyTransport( runtimeEnvironmentId: string, opts: IpcPtyTransportOptions = {} @@ -467,7 +475,7 @@ export function createRemoteRuntimePtyTransport( const launchTokenToSend = options.launchToken ?? launchToken const launchAgentToSend = options.launchAgent ?? launchAgent const created = await callRuntime<{ terminal: RuntimeTerminalCreate }>('terminal.create', { - worktree: toRuntimeWorktreeSelector(worktreeId), + worktree: toRuntimeTerminalWorktreeSelector(worktreeId), ...(commandToSend !== undefined ? { command: commandToSend } : {}), ...(startupCommandDeliveryToSend !== undefined ? { startupCommandDelivery: startupCommandDeliveryToSend } diff --git a/src/renderer/src/runtime/runtime-worktree-selector.test.ts b/src/renderer/src/runtime/runtime-worktree-selector.test.ts index 42817b820..0aee6d057 100644 --- a/src/renderer/src/runtime/runtime-worktree-selector.test.ts +++ b/src/renderer/src/runtime/runtime-worktree-selector.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { toRuntimeWorktreeSelector } from './runtime-worktree-selector' +import { + toRuntimeTerminalWorktreeSelector, + toRuntimeWorktreeSelector +} from './runtime-worktree-selector' +import { brandEphemeralSetupTerminalWorktreeId } from '../../../shared/ephemeral-setup-terminal-worktree-id' describe('toRuntimeWorktreeSelector', () => { it('addresses raw worktree IDs as runtime ID selectors', () => { @@ -14,3 +18,18 @@ describe('toRuntimeWorktreeSelector', () => { expect(toRuntimeWorktreeSelector('')).toBe('') }) }) + +describe('toRuntimeTerminalWorktreeSelector', () => { + it('resolves ephemeral setup terminals to the floating-terminal scope', () => { + expect( + toRuntimeTerminalWorktreeSelector( + brandEphemeralSetupTerminalWorktreeId('feature-wall-orchestration-skill-terminal') + ) + ).toBe('id:global-floating-terminal') + }) + + it('addresses real worktree ids like the base selector', () => { + expect(toRuntimeTerminalWorktreeSelector('wt-1')).toBe('id:wt-1') + expect(toRuntimeTerminalWorktreeSelector('id:wt-1')).toBe('id:wt-1') + }) +}) diff --git a/src/renderer/src/runtime/runtime-worktree-selector.ts b/src/renderer/src/runtime/runtime-worktree-selector.ts index 2467e0dfa..0a33843ee 100644 --- a/src/renderer/src/runtime/runtime-worktree-selector.ts +++ b/src/renderer/src/runtime/runtime-worktree-selector.ts @@ -1,5 +1,9 @@ +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' +import { isEphemeralSetupTerminalWorktreeId } from '../../../shared/ephemeral-setup-terminal-worktree-id' + const RUNTIME_WORKTREE_ID_SELECTOR_PREFIX = 'id:' +/** Address a raw worktree id as a runtime `id:` selector; passes through empty or already-prefixed values. */ export function toRuntimeWorktreeSelector(worktreeId: string): string { const trimmed = worktreeId.trim() if (!trimmed || trimmed.startsWith(RUNTIME_WORKTREE_ID_SELECTOR_PREFIX)) { @@ -7,3 +11,15 @@ export function toRuntimeWorktreeSelector(worktreeId: string): string { } return `${RUNTIME_WORKTREE_ID_SELECTOR_PREFIX}${trimmed}` } + +/** + * Runtime selector for a terminal's worktree id. Ephemeral setup terminals have no + * worktree on the runtime, so resolve them to the floating-terminal scope (home dir) + * every runtime understands; other ids map to their own `id:` selector. + */ +export function toRuntimeTerminalWorktreeSelector(worktreeId: string): string { + if (isEphemeralSetupTerminalWorktreeId(worktreeId.trim())) { + return toRuntimeWorktreeSelector(FLOATING_TERMINAL_WORKTREE_ID) + } + return toRuntimeWorktreeSelector(worktreeId) +} diff --git a/src/shared/ephemeral-setup-terminal-worktree-id.test.ts b/src/shared/ephemeral-setup-terminal-worktree-id.test.ts new file mode 100644 index 000000000..c93c869ac --- /dev/null +++ b/src/shared/ephemeral-setup-terminal-worktree-id.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { + EPHEMERAL_SETUP_TERMINAL_WORKTREE_ID_PREFIX, + brandEphemeralSetupTerminalWorktreeId, + isEphemeralSetupTerminalWorktreeId +} from './ephemeral-setup-terminal-worktree-id' + +describe('ephemeral setup terminal worktree id', () => { + it('brands a panel id with the ephemeral prefix', () => { + expect(brandEphemeralSetupTerminalWorktreeId('feature-wall-orchestration-skill-terminal')).toBe( + `${EPHEMERAL_SETUP_TERMINAL_WORKTREE_ID_PREFIX}feature-wall-orchestration-skill-terminal` + ) + }) + + it('is idempotent for already-branded ids', () => { + const branded = brandEphemeralSetupTerminalWorktreeId('settings-orchestration-skill-terminal') + expect(brandEphemeralSetupTerminalWorktreeId(branded)).toBe(branded) + }) + + it('recognizes branded ids and rejects real worktree ids', () => { + expect( + isEphemeralSetupTerminalWorktreeId( + brandEphemeralSetupTerminalWorktreeId('feature-tip-cli-skills-terminal') + ) + ).toBe(true) + expect(isEphemeralSetupTerminalWorktreeId('repo-1::/work/orca/wt')).toBe(false) + expect(isEphemeralSetupTerminalWorktreeId('global-floating-terminal')).toBe(false) + }) + + it('does not introduce the `::` worktree id separator', () => { + expect(brandEphemeralSetupTerminalWorktreeId('onboarding-inline-terminal')).not.toContain('::') + }) +}) diff --git a/src/shared/ephemeral-setup-terminal-worktree-id.ts b/src/shared/ephemeral-setup-terminal-worktree-id.ts new file mode 100644 index 000000000..005659b7c --- /dev/null +++ b/src/shared/ephemeral-setup-terminal-worktree-id.ts @@ -0,0 +1,19 @@ +// Inline setup/onboarding terminals have no backing worktree. Branding their +// per-panel id lets the terminal RPC layer scope them to the floating terminal, +// instead of leaking an unresolvable selector to a remote runtime (#6789). +export const EPHEMERAL_SETUP_TERMINAL_WORKTREE_ID_PREFIX = 'ephemeral-setup-terminal:' + +/** + * Brand a per-panel setup-terminal id so the terminal RPC layer routes it to the + * floating-terminal scope on a runtime. Idempotent for already-branded ids. + */ +export function brandEphemeralSetupTerminalWorktreeId(panelId: string): string { + return isEphemeralSetupTerminalWorktreeId(panelId) + ? panelId + : `${EPHEMERAL_SETUP_TERMINAL_WORKTREE_ID_PREFIX}${panelId}` +} + +/** Whether `worktreeId` is a branded ephemeral setup-terminal id. */ +export function isEphemeralSetupTerminalWorktreeId(worktreeId: string): boolean { + return worktreeId.startsWith(EPHEMERAL_SETUP_TERMINAL_WORKTREE_ID_PREFIX) +}