mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
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.
This commit is contained in:
@@ -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]
|
||||
|
||||
@@ -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(<HookProbe activeFile={activeFile} />))
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(latestFileContents[activeFile.id]?.loadError).toBe(
|
||||
'The sibling file is already open; close one tab before restoring it.'
|
||||
)
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
+1
@@ -24,6 +24,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -43,6 +43,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
@@ -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')
|
||||
|
||||
|
||||
@@ -14,7 +14,9 @@ export type TerminalLinkStoreState = {
|
||||
setPendingEditorReveal: Mock
|
||||
setMarkdownViewMode: Mock
|
||||
activeFileIdByWorktree: Record<string, string | null>
|
||||
openFiles: { filePath: string; worktreeId: string }[]
|
||||
worktreesByRepo: Record<string, { id: string; path: string }[]>
|
||||
folderWorkspaces: []
|
||||
}
|
||||
|
||||
export type TerminalLinkTestDoubles = {
|
||||
@@ -60,7 +62,9 @@ export function createTerminalLinkTestDoubles(): TerminalLinkTestDoubles {
|
||||
setPendingEditorReveal: setPendingEditorRevealMock,
|
||||
setMarkdownViewMode: setMarkdownViewModeMock,
|
||||
activeFileIdByWorktree: {} as Record<string, string | null>,
|
||||
worktreesByRepo: {} as Record<string, { id: string; path: string }[]>
|
||||
openFiles: [] as { filePath: string; worktreeId: string }[],
|
||||
worktreesByRepo: {} as Record<string, { id: string; path: string }[]>,
|
||||
folderWorkspaces: [] as []
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -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', {
|
||||
|
||||
@@ -29,6 +29,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
@@ -27,6 +27,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
+1
@@ -34,6 +34,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
+1
@@ -31,6 +31,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
+1
@@ -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()
|
||||
}))
|
||||
|
||||
|
||||
@@ -30,6 +30,7 @@ vi.mock('@/lib/language-detect', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-activation', () => ({
|
||||
activateAndRevealWorkspace: vi.fn(),
|
||||
activateAndRevealWorktree: vi.fn()
|
||||
}))
|
||||
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user