From b21f8cd230d1608085605e56a421ea0cc24fd6b7 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 23 Jun 2026 21:00:42 -0700 Subject: [PATCH] Scope terminal handle links by runtime and focus mounted tasks directly (#6225) * Match terminal handle links using their owning runtime environment ID to prevent false-positive activation when terminal handles collide across local and remote runtimes. * Focus task-associated terminals directly in the renderer when they are already mounted to avoid redundant runtime RPC focus calls. --- .../terminal-handle-links.test.ts | 239 +++++++++++++++++- .../terminal-pane/terminal-handle-links.ts | 44 +++- .../terminal-orchestration-task-links.ts | 6 +- 3 files changed, 277 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/terminal-handle-links.test.ts b/src/renderer/src/components/terminal-pane/terminal-handle-links.test.ts index ddb842b66f1..b4d12bfb12f 100644 --- a/src/renderer/src/components/terminal-pane/terminal-handle-links.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-handle-links.test.ts @@ -294,16 +294,80 @@ describe('createTerminalHandleLinkProvider', () => { }) }) + it('uses the owning runtime for terminal links when a renderer match belongs to another runtime', async () => { + markRuntimeEnvironmentCompatible('env-1') + mocks.storeState.tabsByWorktree = { + 'wt-local': [ + { + id: 'tab-local', + worktreeId: 'wt-local', + ptyId: 'term_worker', + title: 'Local worker', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ], + 'wt-env-2': [ + { + id: 'tab-env-2', + worktreeId: 'wt-env-2', + ptyId: 'remote:env-2@@term_worker', + title: 'Other remote worker', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + mocks.storeState.ptyIdsByTabId = { + 'tab-local': ['term_worker'], + 'tab-env-2': ['remote:env-2@@term_worker'] + } + mocks.storeState.terminalLayoutsByTabId = { + 'tab-local': { root: null, activeLeafId: null, expandedLeafId: null }, + 'tab-env-2': { root: null, activeLeafId: null, expandedLeafId: null } + } + window.api.runtimeEnvironments.call = vi.fn().mockResolvedValue({ + ok: true, + result: { focus: { handle: 'term_worker', tabId: 'tab-env-1', worktreeId: 'wt-env-1' } } + }) + const links = await collectLinks([makeBufferLine('Worker: term_worker')], 1, 'env-1') + + links[0].activate( + { + metaKey: true, + ctrlKey: false, + preventDefault: vi.fn() + } as unknown as MouseEvent, + links[0].text + ) + await Promise.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(mocks.storeState.setActiveTab).not.toHaveBeenCalledWith('tab-local') + expect(mocks.storeState.setActiveTab).not.toHaveBeenCalledWith('tab-env-2') + expect(window.api.runtimeEnvironments.call).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'terminal.focus', + params: { terminal: 'term_worker' }, + timeoutMs: undefined + }) + }) + it('provides wrapped task links and focuses their dispatched terminal through runtime', async () => { const rows = [makeBufferLine('Task: task_work'), makeBufferLine('er', { isWrapped: true })] const links = await collectLinks(rows, 1) + const preventDefault = vi.fn() expect(links.map((link) => link.text)).toEqual(['task_worker']) links[0].activate( { metaKey: true, ctrlKey: false, - preventDefault: vi.fn() + preventDefault } as unknown as MouseEvent, links[0].text ) @@ -318,6 +382,179 @@ describe('createTerminalHandleLinkProvider', () => { method: 'terminal.focus', params: { terminal: 'term_worker' } }) + expect(preventDefault).toHaveBeenCalled() + }) + + it('focuses resolved task terminals directly when they are already mounted', async () => { + mocks.storeState.tabsByWorktree = { + 'wt-1': [ + { + id: 'tab-1', + worktreeId: 'wt-1', + ptyId: 'term_worker', + title: 'Worker', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + mocks.storeState.ptyIdsByTabId = { 'tab-1': ['term_worker'] } + mocks.storeState.terminalLayoutsByTabId = { + 'tab-1': { root: null, activeLeafId: null, expandedLeafId: null } + } + const links = await collectLinks([makeBufferLine('Task: task_worker')]) + + links[0].activate( + { + metaKey: true, + ctrlKey: false, + preventDefault: vi.fn() + } as unknown as MouseEvent, + links[0].text + ) + await Promise.resolve() + await Promise.resolve() + + expect(window.api.runtime.call).toHaveBeenCalledTimes(1) + expect(window.api.runtime.call).toHaveBeenCalledWith({ + method: 'orchestration.dispatchShow', + params: { task: 'task_worker' } + }) + expect(mocks.storeState.setActiveWorktree).toHaveBeenCalledWith('wt-1') + expect(mocks.storeState.markWorktreeVisited).toHaveBeenCalledWith('wt-1') + expect(mocks.storeState.setActiveView).toHaveBeenCalledWith('terminal') + expect(mocks.storeState.setActiveTabType).toHaveBeenCalledWith('terminal') + expect(mocks.storeState.revealWorktreeInSidebar).toHaveBeenCalledWith('wt-1') + expect(mocks.storeState.setActiveTab).toHaveBeenCalledWith('tab-1') + expect(mocks.focusTerminalTabSurface).toHaveBeenCalledWith('tab-1') + }) + + it('focuses mounted task terminals only when they belong to the owning runtime', async () => { + markRuntimeEnvironmentCompatible('env-1') + mocks.storeState.tabsByWorktree = { + 'wt-remote': [ + { + id: 'tab-remote', + worktreeId: 'wt-remote', + ptyId: 'remote:env-1@@term_remote', + title: 'Remote worker', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + mocks.storeState.ptyIdsByTabId = { 'tab-remote': ['remote:env-1@@term_remote'] } + mocks.storeState.terminalLayoutsByTabId = { + 'tab-remote': { root: null, activeLeafId: null, expandedLeafId: null } + } + window.api.runtimeEnvironments.call = vi.fn().mockResolvedValue({ + ok: true, + result: { dispatch: { assignee_handle: 'term_remote' } } + }) + const links = await collectLinks([makeBufferLine('Task: task_remote')], 1, 'env-1') + + links[0].activate( + { + metaKey: true, + ctrlKey: false, + preventDefault: vi.fn() + } as unknown as MouseEvent, + links[0].text + ) + await Promise.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(window.api.runtimeEnvironments.call).toHaveBeenCalledTimes(1) + expect(window.api.runtimeEnvironments.call).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'orchestration.dispatchShow', + params: { task: 'task_remote' }, + timeoutMs: undefined + }) + expect(mocks.storeState.setActiveWorktree).toHaveBeenCalledWith('wt-remote') + expect(mocks.storeState.setActiveTab).toHaveBeenCalledWith('tab-remote') + }) + + it('falls back to the owning runtime when task handles collide across runtimes', async () => { + markRuntimeEnvironmentCompatible('env-1') + mocks.storeState.tabsByWorktree = { + 'wt-local': [ + { + id: 'tab-local', + worktreeId: 'wt-local', + ptyId: 'term_worker', + title: 'Local worker', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ], + 'wt-env-2': [ + { + id: 'tab-env-2', + worktreeId: 'wt-env-2', + ptyId: 'remote:env-2@@term_worker', + title: 'Other remote worker', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + mocks.storeState.ptyIdsByTabId = { + 'tab-local': ['term_worker'], + 'tab-env-2': ['remote:env-2@@term_worker'] + } + mocks.storeState.terminalLayoutsByTabId = { + 'tab-local': { root: null, activeLeafId: null, expandedLeafId: null }, + 'tab-env-2': { root: null, activeLeafId: null, expandedLeafId: null } + } + window.api.runtimeEnvironments.call = vi.fn().mockImplementation(({ method }) => { + if (method === 'orchestration.dispatchShow') { + return Promise.resolve({ + ok: true, + result: { dispatch: { assignee_handle: 'term_worker' } } + }) + } + return Promise.resolve({ + ok: true, + result: { focus: { handle: 'term_worker', tabId: 'tab-env-1', worktreeId: 'wt-env-1' } } + }) + }) + const links = await collectLinks([makeBufferLine('Task: task_worker')], 1, 'env-1') + + links[0].activate( + { + metaKey: true, + ctrlKey: false, + preventDefault: vi.fn() + } as unknown as MouseEvent, + links[0].text + ) + await Promise.resolve() + await Promise.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(mocks.storeState.setActiveTab).not.toHaveBeenCalledWith('tab-local') + expect(mocks.storeState.setActiveTab).not.toHaveBeenCalledWith('tab-env-2') + expect(window.api.runtimeEnvironments.call).toHaveBeenNthCalledWith(1, { + selector: 'env-1', + method: 'orchestration.dispatchShow', + params: { task: 'task_worker' }, + timeoutMs: undefined + }) + expect(window.api.runtimeEnvironments.call).toHaveBeenNthCalledWith(2, { + selector: 'env-1', + method: 'terminal.focus', + params: { terminal: 'term_worker' }, + timeoutMs: undefined + }) }) it('returns task and terminal links in line order', async () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-handle-links.ts b/src/renderer/src/components/terminal-pane/terminal-handle-links.ts index d692e68b897..c11d595f611 100644 --- a/src/renderer/src/components/terminal-pane/terminal-handle-links.ts +++ b/src/renderer/src/components/terminal-pane/terminal-handle-links.ts @@ -4,7 +4,7 @@ import { useAppStore } from '@/store' import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' -import { getRemoteRuntimeTerminalHandle } from '@/runtime/runtime-terminal-stream' +import { parseRemoteRuntimePtyId } from '@/runtime/runtime-terminal-stream' import { buildWrappedLogicalLine, rangeForParsedFileLink } from './wrapped-terminal-link-ranges' import { extractOrchestrationTaskLinks, @@ -96,13 +96,14 @@ function findPrefixedTokenEnd(lineText: string, startIndex: number): number { export function findTerminalHandleTarget( handle: string, - state: TerminalHandleFocusState + state: TerminalHandleFocusState, + runtimeEnvironmentId?: string | null ): TerminalHandleTarget | null { for (const [worktreeId, tabs] of Object.entries(state.tabsByWorktree)) { for (const tab of tabs) { const layout = state.terminalLayoutsByTabId[tab.id] for (const [leafId, ptyId] of Object.entries(layout?.ptyIdsByLeafId ?? {})) { - if (ptyIdMatchesTerminalHandle(ptyId, handle)) { + if (ptyIdMatchesTerminalHandle(ptyId, handle, runtimeEnvironmentId)) { return { worktreeId, tabId: tab.id, leafId } } } @@ -110,7 +111,9 @@ export function findTerminalHandleTarget( const tabPtyIds = [tab.ptyId, ...(state.ptyIdsByTabId[tab.id] ?? [])].filter( (ptyId): ptyId is string => Boolean(ptyId) ) - if (tabPtyIds.some((ptyId) => ptyIdMatchesTerminalHandle(ptyId, handle))) { + if ( + tabPtyIds.some((ptyId) => ptyIdMatchesTerminalHandle(ptyId, handle, runtimeEnvironmentId)) + ) { return { worktreeId, tabId: tab.id, leafId: layout?.activeLeafId ?? null } } } @@ -118,9 +121,12 @@ export function findTerminalHandleTarget( return null } -export function focusRendererTerminalHandle(handle: string): boolean { +export function focusRendererTerminalHandle( + handle: string, + runtimeEnvironmentId?: string | null +): boolean { const store = useAppStore.getState() - const target = findTerminalHandleTarget(handle, store) + const target = findTerminalHandleTarget(handle, store, runtimeEnvironmentId) if (!target) { return false } @@ -211,21 +217,39 @@ async function activateParsedLink( ): Promise { try { if (parsed.kind === 'terminal') { - if (!focusRendererTerminalHandle(parsed.text)) { + if (!focusRendererTerminalHandle(parsed.text, runtimeEnvironmentId)) { await focusRuntimeTerminalHandle(parsed.text, runtimeEnvironmentId) } return } // Why: a task can be retried onto a new dispatch; runtime DB is the // authority for the latest terminal assigned to a stable task ID. - await focusRuntimeOrchestrationTask(parsed.text, runtimeEnvironmentId) + await focusRuntimeOrchestrationTask(parsed.text, runtimeEnvironmentId, (handle) => + focusRendererTerminalHandle(handle, runtimeEnvironmentId) + ) } catch (error: unknown) { console.warn('[terminal-handle-link] focus failed:', error) } } -function ptyIdMatchesTerminalHandle(ptyId: string, handle: string): boolean { - return ptyId === handle || getRemoteRuntimeTerminalHandle(ptyId) === handle +function ptyIdMatchesTerminalHandle( + ptyId: string, + handle: string, + runtimeEnvironmentId?: string | null +): boolean { + const targetEnvironmentId = runtimeEnvironmentId?.trim() || null + if (ptyId === handle) { + return targetEnvironmentId === null + } + const remotePty = parseRemoteRuntimePtyId(ptyId) + if (!remotePty || remotePty.handle !== handle) { + return false + } + const ptyEnvironmentId = remotePty.environmentId?.trim() || null + if (runtimeEnvironmentId === undefined) { + return true + } + return ptyEnvironmentId === targetEnvironmentId } function getTerminalHandleFocusHint(): string { diff --git a/src/renderer/src/components/terminal-pane/terminal-orchestration-task-links.ts b/src/renderer/src/components/terminal-pane/terminal-orchestration-task-links.ts index 99acf41f12c..fdd90fb4019 100644 --- a/src/renderer/src/components/terminal-pane/terminal-orchestration-task-links.ts +++ b/src/renderer/src/components/terminal-pane/terminal-orchestration-task-links.ts @@ -46,7 +46,8 @@ export function extractOrchestrationTaskLinks(lineText: string): ParsedOrchestra export async function focusRuntimeOrchestrationTask( taskId: string, - runtimeEnvironmentId: string | null + runtimeEnvironmentId: string | null, + focusRendererTerminal?: (handle: string) => boolean ): Promise { const environmentId = runtimeEnvironmentId?.trim() const target = environmentId @@ -59,6 +60,9 @@ export async function focusRuntimeOrchestrationTask( if (!terminal) { throw new Error(`No dispatched terminal for orchestration task ${taskId}`) } + if (focusRendererTerminal?.(terminal)) { + return + } // Why: task IDs are stable orchestration DB records, but terminal.focus owns // the app-side navigation contract for local and SSH runtime terminals. await callRuntimeRpc(target, 'terminal.focus', { terminal })