From 07b82340f34c7225afdc95e5e99ff68eba897494 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:17:59 -0700 Subject: [PATCH] Route terminal file links to sibling workspace tabs (#16544) * fix: route terminal file links to sibling workspace tabs Detect when a clicked file is already open in a sibling workspace and route to that existing tab instead of creating a duplicate. Reorganizes workspace activation to dispatch by both worktree id and execution host, allowing the same worktree name across different remotes to be disambiguated and routed correctly. * test: validate terminal file link opens in correct sibling worktree Enhance test to check both file path and active worktree ID, ensuring the linked file opens in the intended sibling workspace. --- .../editor/useEditorPanelFileContentLoader.ts | 2 +- ...torPanelRemoteSiblingContentState.test.tsx | 26 ++++++ ...terminal-hover-link-provider.repro.test.ts | 1 + .../terminal-file-open-routing.ts | 46 +++++++++-- .../terminal-link-click-fallback.test.ts | 1 + .../terminal-link-file-open-routing.test.ts | 56 ++++++++++++- .../terminal-link-handlers-test-fixtures.ts | 6 +- .../terminal-link-handlers-test-harness.ts | 1 + .../terminal-link-osc-file-targets.test.ts | 1 + .../terminal-link-osc-url-routing.test.ts | 1 + ...minal-link-provider-link-detection.test.ts | 1 + ...minal-link-remote-runtime-ssh-open.test.ts | 1 + ...inal-link-worktree-root-activation.test.ts | 1 + .../terminal-link-wsl-path-mapping.test.ts | 1 + tests/e2e/golden-terminal-file-link.spec.ts | 82 +++++++++++++++++++ 15 files changed, 215 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts b/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts index 0628719116d..9889e10ceb4 100644 --- a/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts +++ b/src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts @@ -134,7 +134,6 @@ export function useEditorPanelFileContentLoader({ route, runtimeEnvironmentId ?? null ) - fileReadGenerationRef.current[id] = ++fileReadGenerationCounterRef.current if (!migration.ok) { throw new Error( migration.reason === 'collision' @@ -142,6 +141,7 @@ export function useEditorPanelFileContentLoader({ : 'The sibling file owner changed while the tab was restoring.' ) } + fileReadGenerationRef.current[id] = ++fileReadGenerationCounterRef.current setFileContents((prev) => { const next = { ...prev } delete next[id] diff --git a/src/renderer/src/components/editor/useEditorPanelRemoteSiblingContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelRemoteSiblingContentState.test.tsx index 4576b137432..6666302d0a9 100644 --- a/src/renderer/src/components/editor/useEditorPanelRemoteSiblingContentState.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelRemoteSiblingContentState.test.tsx @@ -165,4 +165,30 @@ describe('remote sibling editor content routing', () => { expect(authorizeExternalPath).not.toHaveBeenCalled() expect(mocks.readRuntimeFileContent).not.toHaveBeenCalled() }) + + it('reports a sibling-owner collision instead of remaining in loading state', async () => { + const activeFile = createOpenFile({ + id: '/work/repo-b/docs/readme.md', + filePath: '/work/repo-b/docs/readme.md', + relativePath: '/work/repo-b/docs/readme.md', + worktreeId: 'repo-a::/work/repo-a' + }) + mocks.findWorkspaceFileRoute.mockReturnValue({ + worktreeId: 'repo-b::/work/repo-b', + relativePath: 'docs/readme.md', + executionHostId: 'runtime:runtime-1' + }) + mocks.migrateRestoredEditorFileOwner.mockResolvedValue({ + ok: false, + reason: 'collision' + }) + + await act(async () => root?.render()) + + await vi.waitFor(() => + expect(latestFileContents[activeFile.id]?.loadError).toBe( + 'The sibling file is already open; close one tab before restoring it.' + ) + ) + }) }) diff --git a/src/renderer/src/components/terminal-pane/issue-4631-terminal-hover-link-provider.repro.test.ts b/src/renderer/src/components/terminal-pane/issue-4631-terminal-hover-link-provider.repro.test.ts index 1e293297b8f..8f012d0e462 100644 --- a/src/renderer/src/components/terminal-pane/issue-4631-terminal-hover-link-provider.repro.test.ts +++ b/src/renderer/src/components/terminal-pane/issue-4631-terminal-hover-link-provider.repro.test.ts @@ -24,6 +24,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts b/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts index fbbee1f771c..cde47e4ab86 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts @@ -1,6 +1,7 @@ import { absolutePathToFileUri } from '@/components/editor/markdown-internal-links' import { getConnectionId } from '@/lib/connection-context' import { detectLanguage } from '@/lib/language-detect' +import { findWorkspaceFileRoute } from '@/lib/runtime-workspace-file-route' import { isPathInsideWorktree, toWorktreeRelativePath } from '@/lib/terminal-links' import { isRemoteRuntimeFileOperation, @@ -9,9 +10,15 @@ import { } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' import { useAppStore } from '@/store' -import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { activateAndRevealWorkspace, activateAndRevealWorktree } from '@/lib/worktree-activation' import { resolveKnownWorktreeRootPathLink } from './terminal-worktree-path-link' import { parseWslUncPath, toWindowsWslPath } from '../../../../shared/wsl-paths' +import { + LOCAL_EXECUTION_HOST_ID, + toRuntimeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../../../shared/execution-host' type TerminalFileOpenDeps = { worktreeId: string @@ -199,19 +206,42 @@ export function openDetectedFilePath( return } + const store = useAppStore.getState() + let targetWorktreeId = worktreeId + let targetExecutionHostId: ExecutionHostId | undefined let relativePath = mappedFilePath if (worktreePath && isPathInsideWorktree(mappedFilePath, worktreePath)) { const maybeRelative = toWorktreeRelativePath(mappedFilePath, worktreePath) if (maybeRelative !== null && maybeRelative.length > 0) { relativePath = maybeRelative } + } else if ( + store.openFiles.some( + (openFile) => openFile.filePath === mappedFilePath && openFile.worktreeId !== worktreeId + ) + ) { + // Why: early resolution is only needed to avoid an existing sibling-tab collision. + const runtimeOwnerId = fileContext.settings?.activeRuntimeEnvironmentId?.trim() + const executionHostId = runtimeOwnerId + ? toRuntimeExecutionHostId(runtimeOwnerId) + : fileContext.connectionId + ? toSshExecutionHostId(fileContext.connectionId) + : LOCAL_EXECUTION_HOST_ID + const siblingRoute = findWorkspaceFileRoute(store, executionHostId, mappedFilePath) + if (siblingRoute) { + targetWorktreeId = siblingRoute.worktreeId + targetExecutionHostId = siblingRoute.executionHostId + relativePath = siblingRoute.relativePath + } } - const store = useAppStore.getState() - if (worktreeId) { - // Why: cross-worktree file links share the activation history stack with sidebar and - // palette navigation, but the editor file is the surface — don't seed a shell. - activateAndRevealWorktree(worktreeId, { providesInitialSurface: true }) + if (targetWorktreeId) { + // Why: the route may name a folder-workspace key, and the same worktree id can exist + // on several hosts — dispatch by workspace shape and keep the resolved host. + activateAndRevealWorkspace(targetWorktreeId, { + providesInitialSurface: true, + ...(targetExecutionHostId ? { executionHostId: targetExecutionHostId } : {}) + }) } const language = detectLanguage(mappedFilePath) @@ -219,7 +249,7 @@ export function openDetectedFilePath( { filePath: mappedFilePath, relativePath, - worktreeId: worktreeId || '', + worktreeId: targetWorktreeId || '', language, mode: 'edit', runtimeEnvironmentId, @@ -238,7 +268,7 @@ export function openDetectedFilePath( const openedStore = useAppStore.getState() // Why: scope the reveal to the opened editor tab id so owner-qualified tabs // across local/SSH/runtime contexts get it instead of an ambiguous path key. - const fileId = openedStore.activeFileIdByWorktree[worktreeId] ?? mappedFilePath + const fileId = openedStore.activeFileIdByWorktree[targetWorktreeId] ?? mappedFilePath if (language === 'markdown') { // Why: rich Markdown has no line-based reveal consumer; line links must mount Monaco. openedStore.setMarkdownViewMode(fileId, 'source') diff --git a/src/renderer/src/components/terminal-pane/terminal-link-click-fallback.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-click-fallback.test.ts index 163b381d0bc..f7c92a460bc 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-click-fallback.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-click-fallback.test.ts @@ -43,6 +43,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-file-open-routing.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-file-open-routing.test.ts index 16203985686..61fc1e2c1ba 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-file-open-routing.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-file-open-routing.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { activateAndRevealWorkspace, activateAndRevealWorktree } from '@/lib/worktree-activation' import { openDetectedFilePath } from './terminal-link-handlers' import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' import { @@ -10,6 +10,7 @@ import { setPlatform } from './terminal-link-handlers-test-harness' +const findWorkspaceFileRouteMock = vi.hoisted(() => vi.fn()) const doubles = createTerminalLinkTestDoubles() const { storeState, @@ -33,6 +34,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) @@ -40,6 +42,10 @@ vi.mock('@/lib/connection-context', () => ({ getConnectionId: vi.fn(() => null) })) +vi.mock('@/lib/runtime-workspace-file-route', () => ({ + findWorkspaceFileRoute: findWorkspaceFileRouteMock +})) + installTerminalLinkTestEnvironment(doubles) describe('handleOscLink', () => { @@ -103,11 +109,57 @@ describe('handleOscLink', () => { }) expect(openFilePathMock).not.toHaveBeenCalled() // Why: the editor file is the surface — the cross-worktree jump must not add a shell. - expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { + expect(activateAndRevealWorkspace).toHaveBeenCalledWith('wt-1', { providesInitialSurface: true }) }) + it('opens a sibling folder-workspace path under its owning host and workspace', async () => { + const filePath = '/sibling/docs/SKILL.md' + storeState.openFiles = [{ filePath, worktreeId: 'folder:notes' }] + findWorkspaceFileRouteMock.mockReturnValueOnce({ + worktreeId: 'folder:notes', + relativePath: 'docs/SKILL.md', + executionHostId: 'local' + }) + openFileMock.mockImplementationOnce(() => { + storeState.activeFileIdByWorktree['folder:notes'] = 'owned-skill' + }) + + openDetectedFilePath(filePath, 12, null, deps) + await flushAsyncWork() + await flushDoubleRaf() + + expect(activateAndRevealWorkspace).toHaveBeenCalledWith('folder:notes', { + providesInitialSurface: true, + executionHostId: 'local' + }) + expect(openFileMock).toHaveBeenCalledWith( + expect.objectContaining({ + filePath, + relativePath: 'docs/SKILL.md', + worktreeId: 'folder:notes' + }), + { forceContentReload: true } + ) + expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith( + expect.objectContaining({ fileId: 'owned-skill' }) + ) + }) + + it('leaves ordinary external ownership lookup to the editor loader', async () => { + const filePath = '/external/docs/readme.md' + + openDetectedFilePath(filePath, null, null, deps) + await flushAsyncWork() + + expect(findWorkspaceFileRouteMock).not.toHaveBeenCalled() + expect(openFileMock).toHaveBeenCalledWith( + expect.objectContaining({ filePath, worktreeId: 'wt-1' }), + { forceContentReload: true } + ) + }) + it('preserves explicit column for Orca opens from :line:column links', async () => { setPlatform('Macintosh') diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-fixtures.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-fixtures.ts index 30abb651aed..12751c02373 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-fixtures.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-fixtures.ts @@ -14,7 +14,9 @@ export type TerminalLinkStoreState = { setPendingEditorReveal: Mock setMarkdownViewMode: Mock activeFileIdByWorktree: Record + openFiles: { filePath: string; worktreeId: string }[] worktreesByRepo: Record + folderWorkspaces: [] } export type TerminalLinkTestDoubles = { @@ -60,7 +62,9 @@ export function createTerminalLinkTestDoubles(): TerminalLinkTestDoubles { setPendingEditorReveal: setPendingEditorRevealMock, setMarkdownViewMode: setMarkdownViewModeMock, activeFileIdByWorktree: {} as Record, - worktreesByRepo: {} as Record + openFiles: [] as { filePath: string; worktreeId: string }[], + worktreesByRepo: {} as Record, + folderWorkspaces: [] as [] } return { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-harness.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-harness.ts index 319df49cb73..5230fa00ed2 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-harness.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-harness.ts @@ -58,6 +58,7 @@ export function installTerminalLinkTestEnvironment(doubles: TerminalLinkTestDoub openFilePathMock.mockResolvedValue(true) storeState.settings = undefined storeState.activeFileIdByWorktree = {} + storeState.openFiles = [] storeState.worktreesByRepo = {} registerHttpLinkStoreAccessor(() => storeState) vi.stubGlobal('window', { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-osc-file-targets.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-osc-file-targets.test.ts index 670a93fb80a..c537d57ba0f 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-osc-file-targets.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-osc-file-targets.test.ts @@ -29,6 +29,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-osc-url-routing.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-osc-url-routing.test.ts index b0346d1f79c..7fd4ffc9c6c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-osc-url-routing.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-osc-url-routing.test.ts @@ -27,6 +27,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-provider-link-detection.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-provider-link-detection.test.ts index 00a205d2605..cff3cc1a951 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-provider-link-detection.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-provider-link-detection.test.ts @@ -34,6 +34,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts index 11f5b375b63..8f8e0b7ade9 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts @@ -31,6 +31,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-worktree-root-activation.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-worktree-root-activation.test.ts index ebe6520b525..1920afc3643 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-worktree-root-activation.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-worktree-root-activation.test.ts @@ -34,6 +34,7 @@ vi.mock('@/lib/language-detect', () => ({ // unit tests. Mock it so these tests only assert on routing (browser tab vs. // openFile), not on activation internals. vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-wsl-path-mapping.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-wsl-path-mapping.test.ts index a9c5fce610b..5ef937f9ed2 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-wsl-path-mapping.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-wsl-path-mapping.test.ts @@ -30,6 +30,7 @@ vi.mock('@/lib/language-detect', () => ({ })) vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() })) diff --git a/tests/e2e/golden-terminal-file-link.spec.ts b/tests/e2e/golden-terminal-file-link.spec.ts index 56ee1c3b04b..35e6b20959a 100644 --- a/tests/e2e/golden-terminal-file-link.spec.ts +++ b/tests/e2e/golden-terminal-file-link.spec.ts @@ -189,3 +189,85 @@ test('opens a terminal file link and observes an external edit @golden', async ( writeFileSync(filePath, original) } }) + +test('reuses a terminal file link already open in a sibling workspace @golden', async ({ + orcaPage +}) => { + test.setTimeout(180_000) + await waitForSessionReady(orcaPage) + const sourceWorktreeId = await waitForActiveWorktree(orcaPage) + const sibling = await orcaPage.evaluate((sourceId) => { + const state = window.__store?.getState() + return ( + Object.values(state?.worktreesByRepo ?? {}) + .flat() + .find((worktree) => worktree.id !== sourceId) ?? null + ) + }, sourceWorktreeId) + if (!sibling) { + throw new Error('sibling worktree fixture unavailable') + } + + const filePath = path.join(sibling.path, 'package.json') + await orcaPage.evaluate( + ({ filePath, sourceWorktreeId, siblingWorktreeId }) => { + const state = window.__store?.getState() + if (!state) { + throw new Error('store unavailable') + } + state.openFile({ + filePath, + relativePath: 'package.json', + worktreeId: siblingWorktreeId, + runtimeEnvironmentId: null, + language: 'json', + mode: 'edit' + }) + state.setActiveWorktree(sourceWorktreeId) + }, + { filePath, sourceWorktreeId, siblingWorktreeId: sibling.id } + ) + + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + const ptyId = await waitForActivePanePtyId(orcaPage) + await waitForPtyShellEcho(orcaPage, ptyId, 15_000) + const printedPath = process.platform === 'win32' ? filePath.replaceAll('\\', '/') : filePath + const command = nodeTerminalCommand(['-e', `console.log(${JSON.stringify(printedPath)})`]) + await sendToTerminal(orcaPage, ptyId, `${command}\r`) + await expect + .poll(() => getTerminalContent(orcaPage, LINK_SCAN_CHAR_LIMIT), { timeout: 15_000 }) + .toContain(printedPath) + + let probe: LinkProbe | null = null + await expect + .poll( + async () => { + probe = await locateLink(orcaPage, printedPath) + return probe ? hoverLink(orcaPage, probe) : null + }, + { timeout: 10_000, message: 'sibling file path did not become clickable' } + ) + .toContain('package.json') + if (!probe) { + throw new Error('sibling file link disappeared before activation') + } + await clickLink(orcaPage, probe) + const actionPopover = orcaPage.locator('[data-terminal-link-action-popover]') + await expect(actionPopover).toBeVisible() + await actionPopover.getByRole('button', { name: /Open file/i }).click() + + const editorHeader = orcaPage.locator('.editor-header-path').first() + await expect(editorHeader).toContainText('package.json', { timeout: 20_000 }) + await expect + .poll( + () => + orcaPage.evaluate(() => ({ + filePath: window.__monacoEditorE2E?.filePath ?? null, + activeWorktreeId: window.__store?.getState()?.activeWorktreeId ?? null + })), + { timeout: 20_000, message: 'sibling workspace never rendered the linked file' } + ) + .toEqual({ filePath, activeWorktreeId: sibling.id }) + await expect(orcaPage.getByText('Loading...', { exact: true })).toHaveCount(0) +})