diff --git a/src/renderer/src/components/native-chat/native-chat-file-link.test.ts b/src/renderer/src/components/native-chat/native-chat-file-link.test.ts index 5f3aa7daa8e..b14fb0f653e 100644 --- a/src/renderer/src/components/native-chat/native-chat-file-link.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-file-link.test.ts @@ -65,7 +65,8 @@ describe('resolveNativeChatFileLinkContext', () => { ).toEqual({ worktreeId: 'wt-1', worktreePath: '/repo/worktree', - runtimeEnvironmentId: 'env-1' + runtimeEnvironmentId: 'env-1', + sourceTabId: 'tab-1' }) }) @@ -97,7 +98,7 @@ describe('resolveNativeChatFileLinkContext', () => { }), structuredTab.id ) - ).toEqual(context) + ).toEqual({ ...context, sourceTabId: structuredTab.id }) }) it('resolves a detected-only workspace from the supplied snapshot catalog', () => { @@ -114,7 +115,8 @@ describe('resolveNativeChatFileLinkContext', () => { ).toEqual({ worktreeId: 'wt-1', worktreePath: '/repo/detected', - runtimeEnvironmentId: null + runtimeEnvironmentId: null, + sourceTabId: 'tab-1' }) }) @@ -134,7 +136,8 @@ describe('resolveNativeChatFileLinkContext', () => { ).toEqual({ worktreeId: folderKey, worktreePath: '/workspace/platform', - runtimeEnvironmentId: null + runtimeEnvironmentId: null, + sourceTabId: folderTab.id }) }) }) @@ -184,7 +187,8 @@ describe('floating workspace native chat', () => { expect(resolveNativeChatFileLinkContext(pinned, floatingTab.id)).toEqual({ worktreeId: FLOATING_TERMINAL_WORKTREE_ID, worktreePath: '/home/me/pinned', - runtimeEnvironmentId: null + runtimeEnvironmentId: null, + sourceTabId: floatingTab.id }) }) diff --git a/src/renderer/src/components/native-chat/native-chat-file-link.ts b/src/renderer/src/components/native-chat/native-chat-file-link.ts index 38b5adbe5ac..b7bdef5e7ee 100644 --- a/src/renderer/src/components/native-chat/native-chat-file-link.ts +++ b/src/renderer/src/components/native-chat/native-chat-file-link.ts @@ -12,6 +12,8 @@ export type NativeChatFileLinkContext = { worktreeId: string worktreePath: string runtimeEnvironmentId: string | null + /** The chat's own tab (terminal or agent-session); editor opens place beside its group. */ + sourceTabId?: string } export type NativeChatResolvedFileLink = { @@ -120,7 +122,8 @@ function resolveNativeChatFileLinkContextForOwner( return { worktreeId, worktreePath, - runtimeEnvironmentId: getRuntimeEnvironmentIdForWorktree(state, worktreeId) + runtimeEnvironmentId: getRuntimeEnvironmentIdForWorktree(state, worktreeId), + sourceTabId: terminalTabId } } diff --git a/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts b/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts index 0f81196e2c4..9212ec09da6 100644 --- a/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts +++ b/src/renderer/src/components/native-chat/native-chat-link-target-actions.ts @@ -52,6 +52,7 @@ export function nativeChatFileOpenDeps( worktreeId: context.worktreeId, worktreePath: context.worktreePath, runtimeEnvironmentId: context.runtimeEnvironmentId, + sourceTabId: context.sourceTabId, onOpenFailure: (failure) => failure.verdict === 'unverifiable' ? showFileLinkUnverifiableToast(target.absolutePath, failure.error) diff --git a/src/renderer/src/components/native-chat/native-chat-owner-selectors.test.tsx b/src/renderer/src/components/native-chat/native-chat-owner-selectors.test.tsx index 93fce970819..ec4fb32da68 100644 --- a/src/renderer/src/components/native-chat/native-chat-owner-selectors.test.tsx +++ b/src/renderer/src/components/native-chat/native-chat-owner-selectors.test.tsx @@ -208,7 +208,8 @@ describe('native chat ownership selectors', () => { expect(select(current)).toEqual({ worktreeId: 'chat-owner', worktreePath: '/after', - runtimeEnvironmentId: 'owning-runtime' + runtimeEnvironmentId: 'owning-runtime', + sourceTabId: 'chat' }) useAppStore.setState(current) expect(select(initial)?.worktreePath).toBe('/before') diff --git a/src/renderer/src/components/terminal-pane/floating-file-link-selection.test.ts b/src/renderer/src/components/terminal-pane/floating-file-link-selection.test.ts index 3c7e9e3f102..cbddce1037f 100644 --- a/src/renderer/src/components/terminal-pane/floating-file-link-selection.test.ts +++ b/src/renderer/src/components/terminal-pane/floating-file-link-selection.test.ts @@ -24,7 +24,7 @@ const deps = { runtimeEnvironmentId: null } const createBrowserTab = vi.fn() -const openFile = vi.fn() +const openFileForLink = vi.fn(() => null) const markWorktreeVisited = vi.fn() const recordWorktreeVisit = vi.fn() const revealWorktreeInSidebar = vi.fn() @@ -68,7 +68,7 @@ beforeEach(() => { activeBrowserTabId: 'main-browser', rightSidebarTab: 'explorer', createBrowserTab, - openFile, + openFileForLink, markWorktreeVisited, recordWorktreeVisit, revealWorktreeInSidebar @@ -141,9 +141,10 @@ describe('floating file links preserve main-window selection', () => { await buildFileLinkActions(filePath, null, null, deps, { kind: 'local' }).primary.run() await flushOpen() - expect(openFile).toHaveBeenCalledWith( + expect(openFileForLink).toHaveBeenCalledWith( expect.objectContaining({ filePath, worktreeId: FLOATING_TERMINAL_WORKTREE_ID }), - { forceContentReload: true } + { forceContentReload: true }, + null ) expect(mocks.findWorkspaceFileRoute).not.toHaveBeenCalled() expect(mainSelection()).toEqual(before) @@ -153,9 +154,10 @@ describe('floating file links preserve main-window selection', () => { const before = mainSelection() openDetectedFilePath('/Users/me/floating/app.ts', null, null, deps) await flushOpen() - expect(openFile).toHaveBeenCalledWith( + expect(openFileForLink).toHaveBeenCalledWith( expect.objectContaining({ worktreeId: FLOATING_TERMINAL_WORKTREE_ID }), - { forceContentReload: true } + { forceContentReload: true }, + null ) expect(mainSelection()).toEqual(before) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-file-link-actions.test.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-actions.test.ts index 6b2caedfe6e..55e900c30dd 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-link-actions.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-actions.test.ts @@ -5,16 +5,18 @@ const mocks = vi.hoisted(() => { const settings: { activeRuntimeEnvironmentId: string | null } = { activeRuntimeEnvironmentId: null } + const tabGroups: Record = {} return { canOpenWithSystemDefault: true, openDetectedFilePath: vi.fn(), settings, - worktreeRoot: false + worktreeRoot: false, + tabGroups } }) vi.mock('@/store', () => ({ - useAppStore: { getState: () => ({ settings: mocks.settings }) } + useAppStore: { getState: () => ({ settings: mocks.settings, ...mocks.tabGroups }) } })) vi.mock('./terminal-file-open-routing', () => ({ @@ -72,6 +74,7 @@ beforeEach(() => { mocks.canOpenWithSystemDefault = true mocks.settings = { activeRuntimeEnvironmentId: null } mocks.worktreeRoot = false + mocks.tabGroups = {} vi.clearAllMocks() }) @@ -291,3 +294,82 @@ describe('terminal file link actions', () => { }) }) }) + +describe('open in this group', () => { + function seedGroups(groupIds: string[]): void { + const tabs = groupIds.map((groupId) => ({ + id: `${groupId}-tab`, + entityId: `${groupId}-terminal`, + groupId, + contentType: 'terminal' + })) + mocks.tabGroups = { + unifiedTabsByWorktree: { 'wt-1': tabs }, + groupsByWorktree: { + 'wt-1': groupIds.map((id) => ({ id, activeTabId: `${id}-tab`, tabOrder: [`${id}-tab`] })) + }, + layoutByWorktree: { + 'wt-1': + groupIds.length === 1 + ? { type: 'leaf', groupId: groupIds[0] } + : { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', groupId: groupIds[0] }, + second: { type: 'leaf', groupId: groupIds[1] } + } + } + } + } + + function rowLabels(request: ReturnType): string[] { + const actionRequest = request.mock.calls[0][0] + return (actionRequest.secondaryActions ?? []).map((row: { label: string }) => row.label) + } + + it('pins the source group when the source has a neighbor', () => { + seedGroups(['A', 'B']) + const request = vi.fn() + const sourceDeps = { ...deps, sourceTabId: 'A-terminal' } + handleTerminalFileLink('/repo/src/main.ts', 3, null, plainEvent(), sourceDeps, context(request)) + + const rows = request.mock.calls[0][0].secondaryActions + expect(rowLabels(request)).toContain('Open in this group') + rows.find((row: { label: string }) => row.label === 'Open in this group').run() + expect(mocks.openDetectedFilePath).toHaveBeenCalledWith('/repo/src/main.ts', 3, null, { + ...sourceDeps, + openInSourceGroup: true + }) + }) + + it('omits the row for a single group, an unknown source, or a workspace switch', () => { + seedGroups(['A']) + const single = vi.fn() + handleTerminalFileLink( + '/repo/a.ts', + null, + null, + plainEvent(), + { ...deps, sourceTabId: 'A-terminal' }, + context(single) + ) + expect(rowLabels(single)).not.toContain('Open in this group') + + seedGroups(['A', 'B']) + const unknown = vi.fn() + handleTerminalFileLink('/repo/a.ts', null, null, plainEvent(), deps, context(unknown)) + expect(rowLabels(unknown)).not.toContain('Open in this group') + + mocks.worktreeRoot = true + const root = vi.fn() + handleTerminalFileLink( + '/repo', + null, + null, + plainEvent(), + { ...deps, sourceTabId: 'A-terminal' }, + context(root) + ) + expect(rowLabels(root)).not.toContain('Open in this group') + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-file-link-actions.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-actions.ts index 4337adb3689..7416f77d0f3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-link-actions.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-actions.ts @@ -18,6 +18,7 @@ import { translate } from '@/i18n/i18n' import { getRevealInFileManagerLabel, revealInFileManager } from '@/lib/reveal-in-file-manager' import { useAppStore } from '@/store' import { getLocalPathOpenOwnerForRoute, isLocalPathOpenBlocked } from '@/lib/local-path-open-guard' +import { canOpenLinkInSourceGroup } from '@/store/slices/editor/tabs/editor-open-target-group' export type TerminalFileLinkActionDeps = { worktreeId: string @@ -27,6 +28,8 @@ export type TerminalFileLinkActionDeps = { /** The paired-server terminal that printed the link; lets its host grant a file outside workspaces. */ terminalHandle?: string | null onOpenFailure?: (failure: FileOpenFailure) => void + /** The tab that owns the link; editor opens place themselves relative to its group. */ + sourceTabId?: string | null } export type FileLinkActions = Pick< @@ -148,6 +151,21 @@ export function buildFileLinkActions( run: () => revealInFileManager(mappedPath, revealOwner) } : null + // Why: eligibility is unknown until the path resolves, so the row may still be refused then. + const openInGroupRow = + !worktreeRoot && + fileContext.sourceHostResolved && + canOpenLinkInSourceGroup(useAppStore.getState(), deps.worktreeId, deps.sourceTabId) + ? { + label: translate( + 'auto.components.terminal.pane.TerminalLinkActionPopover.openInThisGroup', + 'Open in this group' + ), + run: () => + openDetectedFilePath(filePath, line, column, { ...deps, openInSourceGroup: true }) + } + : null + const secondaryActions = [openInGroupRow, revealRow].filter((row) => row !== null) return { destination: mappedPath, kind: worktreeRoot ? 'workspace' : 'file', @@ -164,6 +182,6 @@ export function buildFileLinkActions( run: () => openDetectedFilePath(filePath, line, column, deps) }, ...(systemDefaultRow ? { alternate: systemDefaultRow } : {}), - ...(revealRow ? { secondaryActions: [revealRow] } : {}) + ...(secondaryActions.length > 0 ? { secondaryActions } : {}) } } diff --git a/src/renderer/src/components/terminal-pane/terminal-file-link-editor-open.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-editor-open.ts new file mode 100644 index 00000000000..d33d1e90bee --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-editor-open.ts @@ -0,0 +1,70 @@ +import { detectLanguage } from '@/lib/language-detect' +import { useAppStore } from '@/store' +import { scheduleEditorLineReveal } from '@/store/slices/editor/focus/editor-focus-reveal' +import { isLiveEditorOpenGroup } from '@/store/slices/editor/tabs/editor-open-target-group' +import type { OpenFile } from '@/store/slices/editor/types/open-file' +import type { + EditorOpenLinkIntent, + EditorOpenResult +} from '@/store/slices/editor/types/editor-open-link-intent' +import type { ExecutionHostId } from '../../../../shared/execution-host' + +/** An explicit group request may only end in an editor in the scope it was captured from. */ +export function isExplicitEditorScopeKept( + intent: EditorOpenLinkIntent | null, + worktreeId: string, + executionHostId: ExecutionHostId, + clientLocal: boolean +): boolean { + if (!intent?.targetGroupId || intent.workspaceId !== worktreeId || clientLocal) { + return false + } + return ( + (intent.hostId === null || executionHostId === intent.hostId) && + isLiveEditorOpenGroup(useAppStore.getState(), worktreeId, intent.targetGroupId) + ) +} + +/** The returned view still exists where the open put it, showing the same document. */ +function isOpenedEditorViewCurrent(worktreeId: string, opened: EditorOpenResult): boolean { + return (useAppStore.getState().unifiedTabsByWorktree?.[worktreeId] ?? []).some( + (tab) => + tab.id === opened.viewStateId && + tab.groupId === opened.groupId && + tab.entityId === opened.fileId + ) +} + +/** Opens a resolved link target beside its source, then reveals its line in exactly that view. */ +export function openLinkedFileInEditor( + file: Omit, + intent: EditorOpenLinkIntent | null, + position: { line: number | null; column: number | null }, + isCurrentRequest: () => boolean +): EditorOpenResult | null { + const language = detectLanguage(file.filePath) + const opened = useAppStore + .getState() + .openFileForLink({ ...file, language, mode: 'edit' }, { forceContentReload: true }, intent) + if (!opened || position.line === null) { + return opened + } + if (language === 'markdown') { + // Why: rich Markdown has no line-based reveal consumer; line links must mount Monaco. + useAppStore.getState().setMarkdownViewMode(opened.fileId, 'source') + } + // Why: another view of the same document keeps its cursor, and a newer click (even one + // without a line) supersedes this reveal until the editor consumes it. + scheduleEditorLineReveal( + useAppStore.getState, + file.filePath, + position.line, + position.column ?? 1, + opened.fileId, + { + viewStateId: opened.viewStateId, + canApply: () => isCurrentRequest() && isOpenedEditorViewCurrent(file.worktreeId, opened) + } + ) + return opened +} diff --git a/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts index 7715e80c8d8..9f0997816eb 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts @@ -19,6 +19,7 @@ type FileLinkHitTestDeps = { terminalHandle?: string | null pathExistsCache?: Map openWithSystemDefault?: boolean + sourceTabId?: string | null } export function openFilePathLinkAtBufferPosition( diff --git a/src/renderer/src/components/terminal-pane/terminal-file-open-in-group-toast.ts b/src/renderer/src/components/terminal-pane/terminal-file-open-in-group-toast.ts new file mode 100644 index 00000000000..ca1fd2f8e50 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-file-open-in-group-toast.ts @@ -0,0 +1,13 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' + +/** The file resolved to something other than an editor in the captured group, so nothing opened. */ +export function showOpenInGroupRefusedToast(filePath: string): void { + toast.error( + translate( + 'auto.components.terminal.pane.TerminalLinkActionPopover.openInThisGroupRefused', + "Couldn't open {{value0}} in this group", + { value0: filePath } + ) + ) +} 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 c751cbee02a..3ac5e36d11f 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,7 +1,6 @@ import { absolutePathToFileUri } from '@/components/editor/markdown-internal-links' import { getWorkspaceFilePreviewPlan, openFileInBrowserTab } from '@/lib/file-preview' import { downloadAndOpenRemoteTerminalFile } from './terminal-remote-file-download-open' -import { detectLanguage } from '@/lib/language-detect' import { findWorkspaceFileRoute } from '@/lib/runtime-workspace-file-route' import { isPathInsideWorktree, toWorktreeRelativePath } from '@/lib/terminal-links' import { canClientOsOpenWorkspaceFile } from '@/lib/workspace-file-host-routing' @@ -18,8 +17,7 @@ import { import { LOCAL_EXECUTION_HOST_ID, getConnectionExecutionHostId, - toRuntimeExecutionHostId, - type ExecutionHostId + toRuntimeExecutionHostId } from '../../../../shared/execution-host' import { statUserOpenedPath } from '@/lib/user-opened-local-path' import { @@ -28,6 +26,10 @@ import { type HostWorkspaceFile } from './terminal-host-workspace-file' import { isFloatingWorkspaceId } from '../../../../shared/floating-workspace-worktree' +import { captureEditorOpenLinkIntent } from '@/store/slices/editor/tabs/editor-open-target-group' +import { cancelPendingEditorLineRevealFrames } from '@/store/slices/editor/focus/editor-focus-reveal' +import { showOpenInGroupRefusedToast } from './terminal-file-open-in-group-toast' +import { isExplicitEditorScopeKept, openLinkedFileInEditor } from './terminal-file-link-editor-open' export { getTerminalFileContext, @@ -54,6 +56,10 @@ type TerminalFileOpenDeps = { terminalHandle?: string | null /** Reports a path that could not be verified before opening; skipped once a later open supersedes it. */ onOpenFailure?: (failure: FileOpenFailure) => void + /** The tab that owns the link; editor opens place themselves relative to its group. */ + sourceTabId?: string | null + /** "Open in this group": pins the source tab's group at activation, or refuses. */ + openInSourceGroup?: boolean } export function isHtmlFilePath(filePath: string): boolean { @@ -84,33 +90,6 @@ export function shouldOpenTerminalFileWithSystemDefault(fileContext: TerminalFil } let latestOpenDetectedFilePathRequestId = 0 -let pendingEditorRevealFrameIds: number[] = [] - -function cancelPendingEditorRevealFrames(): void { - if (typeof cancelAnimationFrame === 'function') { - for (const frameId of pendingEditorRevealFrameIds) { - cancelAnimationFrame(frameId) - } - } - pendingEditorRevealFrameIds = [] -} - -function schedulePendingEditorReveal(callback: () => void): void { - cancelPendingEditorRevealFrames() - const firstFrameId = requestAnimationFrame(() => { - pendingEditorRevealFrameIds = pendingEditorRevealFrameIds.filter( - (frameId) => frameId !== firstFrameId - ) - const secondFrameId = requestAnimationFrame(() => { - pendingEditorRevealFrameIds = pendingEditorRevealFrameIds.filter( - (frameId) => frameId !== secondFrameId - ) - callback() - }) - pendingEditorRevealFrameIds.push(secondFrameId) - }) - pendingEditorRevealFrameIds.push(firstFrameId) -} export function openDetectedFilePath( filePath: string, @@ -126,11 +105,30 @@ export function openDetectedFilePath( terminalLinkWslDistro(deps.wslDistro, runtimeEnvironmentId) ) const requestId = ++latestOpenDetectedFilePathRequestId - cancelPendingEditorRevealFrames() + cancelPendingEditorLineRevealFrames() + // Why: capture at activation; focus may move while the host resolves the path. + const linkIntent = captureEditorOpenLinkIntent( + useAppStore.getState(), + deps.worktreeId, + deps.sourceTabId, + { explicitSourceGroup: deps.openInSourceGroup === true } + ) + const explicitGroupRequest = deps.openInSourceGroup === true + // Why: an explicit group request never substitutes a browser, system app, or download. + const refuseExplicitNonEditorRoute = (): boolean => { + if (!explicitGroupRequest) { + return false + } + if (requestId === latestOpenDetectedFilePathRequestId) { + showOpenInGroupRefusedToast(mappedFilePath) + } + return true + } void (async () => { let statResult let fileContext = getTerminalFileContext(worktreeId, worktreePath, runtimeEnvironmentId) + const sourceRuntimeOwnerId = runtimeTargetEnvironmentId(fileContext.target) let hostWorkspaceFile: HostWorkspaceFile | null = null let hostGrantId: string | null = null let clientLocal = false @@ -149,6 +147,9 @@ export function openDetectedFilePath( fileContext ) if (worktreeRootLink) { + if (refuseExplicitNonEditorRoute()) { + return + } // Why: root workspace switching must work for SSH/runtime paths without // local auth/stat, while still coalescing provider + fallback clicks. await Promise.resolve() @@ -218,6 +219,9 @@ export function openDetectedFilePath( } if (statResult.isDirectory) { + if (refuseExplicitNonEditorRoute()) { + return + } if (canOpenWithSystemDefault) { await window.api.shell.openFilePath(mappedFilePath, LOCAL_EXECUTION_HOST_ID) } @@ -236,6 +240,10 @@ export function openDetectedFilePath( // and remain the fallback if Shift+Cmd/Ctrl cannot launch the OS default. if (isHtmlFilePath(mappedFilePath) && hostGrantId === null) { if (canOpenWithSystemDefault) { + // Why: Orca's browser has no group placement in v1. + if (refuseExplicitNonEditorRoute()) { + return + } openHtmlFileInBrowser(mappedFilePath, worktreeId, clientLocal) return } @@ -243,6 +251,9 @@ export function openDetectedFilePath( // unsupported plan (e.g. a paired doc outside the worktree) falls back to source. const plan = getWorkspaceFilePreviewPlan(useAppStore.getState(), worktreeId, mappedFilePath) if (plan.status === 'doc-preview') { + if (refuseExplicitNonEditorRoute()) { + return + } activateAndRevealWorktree(worktreeId, { providesInitialSurface: true }) openFileInBrowserTab({ filePath: mappedFilePath, worktreeId }) return @@ -251,7 +262,12 @@ export function openDetectedFilePath( const store = useAppStore.getState() let targetWorktreeId = worktreeId - let targetExecutionHostId: ExecutionHostId | undefined = hostWorkspaceFile?.executionHostId + const runtimeOwnerId = runtimeTargetEnvironmentId(fileContext.target) ?? sourceRuntimeOwnerId + let targetExecutionHostId = + hostWorkspaceFile?.executionHostId ?? + (runtimeOwnerId + ? toRuntimeExecutionHostId(runtimeOwnerId) + : getConnectionExecutionHostId(fileContext.connectionId)) let relativePath = mappedFilePath if (worktreePath && isPathInsideWorktree(mappedFilePath, worktreePath)) { const maybeRelative = toWorktreeRelativePath(mappedFilePath, worktreePath) @@ -268,11 +284,7 @@ export function openDetectedFilePath( ) ) { // Why: early resolution is only needed to avoid an existing sibling-tab collision. - const runtimeOwnerId = runtimeTargetEnvironmentId(fileContext.target) - const executionHostId = runtimeOwnerId - ? toRuntimeExecutionHostId(runtimeOwnerId) - : getConnectionExecutionHostId(fileContext.connectionId) - const siblingRoute = findWorkspaceFileRoute(store, executionHostId, mappedFilePath) + const siblingRoute = findWorkspaceFileRoute(store, targetExecutionHostId, mappedFilePath) if (siblingRoute) { targetWorktreeId = siblingRoute.worktreeId targetExecutionHostId = siblingRoute.executionHostId @@ -280,23 +292,29 @@ export function openDetectedFilePath( } } + if ( + explicitGroupRequest && + !isExplicitEditorScopeKept(linkIntent, targetWorktreeId, targetExecutionHostId, clientLocal) + ) { + // Why: refuse before activating the destination or touching editor state. + refuseExplicitNonEditorRoute() + return + } + 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 } : {}) + executionHostId: targetExecutionHostId }) } - const language = detectLanguage(mappedFilePath) - store.openFile( + const opened = openLinkedFileInEditor( { filePath: mappedFilePath, relativePath, worktreeId: targetWorktreeId || '', - language, - mode: 'edit', runtimeEnvironmentId: clientLocal ? null : runtimeEnvironmentId, // Why: a tab's owner must match its workspace's to be editable; this one is this computer's. ...(clientLocal && targetWorktreeId === worktreeId ? { readOnly: true } : {}), @@ -310,32 +328,12 @@ export function openDetectedFilePath( ? { externalSshTargetId: fileContext.connectionId } : {}) }, - { forceContentReload: true } + linkIntent, + { line, column }, + () => requestId === latestOpenDetectedFilePathRequestId ) - - if (line !== null) { - 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[targetWorktreeId] ?? mappedFilePath - if (language === 'markdown') { - // Why: rich Markdown has no line-based reveal consumer; line links must mount Monaco. - openedStore.setMarkdownViewMode(fileId, 'source') - } - const targetColumn = column ?? 1 - store.setPendingEditorReveal(null) - schedulePendingEditorReveal(() => { - if (requestId !== latestOpenDetectedFilePathRequestId) { - return - } - store.setPendingEditorReveal({ - filePath: mappedFilePath, - fileId, - line, - column: targetColumn, - matchLength: 0 - }) - }) + if (!opened) { + refuseExplicitNonEditorRoute() } })() } 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 e1cd60603d9..778dc801ac0 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 @@ -116,13 +116,16 @@ describe('createFilePathLinkProvider range bounds', () => { }), { forceContentReload: true } ) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', - fileId: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', - line: 5, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', + fileId: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', + line: 5, + column: 3, + matchLength: 0 + }) + ) }) it('opens a single-row file path with the system default from shift modifier fallback', async () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-client-local-file.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-client-local-file.test.ts index e6d516d1491..6560e3aca91 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-client-local-file.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-client-local-file.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest' +import { activateAndRevealWorkspace } from '@/lib/worktree-activation' import { openDetectedFilePath } from './terminal-link-handlers' import { downloadAndOpenRemoteTerminalFile } from './terminal-remote-file-download-open' import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' @@ -78,6 +79,10 @@ describe('a link to this computer printed in a paired-server terminal', () => { access: { kind: 'user-file' } }) ) + expect(activateAndRevealWorkspace).toHaveBeenCalledWith('wt-feat', { + providesInitialSurface: true, + executionHostId: 'runtime:env-1' + }) expect(openFileMock).toHaveBeenCalledWith( expect.objectContaining({ filePath: '/Users/me/notes.md', 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 219f7547021..79546d1a4a8 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 @@ -100,17 +100,21 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '/tmp/src/main.ts', - fileId: '/tmp/src/main.ts', - line: 42, - column: 1, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '/tmp/src/main.ts', + fileId: '/tmp/src/main.ts', + line: 42, + column: 1, + matchLength: 0 + }) + ) expect(openFilePathMock).not.toHaveBeenCalled() // Why: the editor file is the surface — the cross-worktree jump must not add a shell. expect(activateAndRevealWorkspace).toHaveBeenCalledWith('wt-1', { - providesInitialSurface: true + providesInitialSurface: true, + executionHostId: 'local' }) }) @@ -168,13 +172,16 @@ describe('handleOscLink', () => { await flushDoubleRaf() expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '/tmp/src/main.ts', - fileId: '/tmp/src/main.ts', - line: 42, - column: 7, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '/tmp/src/main.ts', + fileId: '/tmp/src/main.ts', + line: 42, + column: 7, + matchLength: 0 + }) + ) expect(openFilePathMock).not.toHaveBeenCalled() }) @@ -191,13 +198,15 @@ describe('handleOscLink', () => { await flushDoubleRaf() expect(setMarkdownViewModeMock).toHaveBeenCalledWith(fileId, 'source') - expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith({ - filePath, - fileId, - line: 230, - column: 1, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + filePath, + fileId, + line: 230, + column: 1, + matchLength: 0 + }) + ) }) it('scopes non-Markdown line reveals to the owner-qualified editor tab', async () => { @@ -213,13 +222,15 @@ describe('handleOscLink', () => { await flushDoubleRaf() expect(setMarkdownViewModeMock).not.toHaveBeenCalled() - expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith({ - filePath, - fileId, - line: 42, - column: 7, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + filePath, + fileId, + line: 42, + column: 7, + matchLength: 0 + }) + ) }) it('uses the system default app for shift+cmd/ctrl-click file paths', async () => { @@ -253,13 +264,16 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '/tmp/src/main.ts', - fileId: '/tmp/src/main.ts', - line: 42, - column: 7, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '/tmp/src/main.ts', + fileId: '/tmp/src/main.ts', + line: 42, + column: 7, + matchLength: 0 + }) + ) }) it('cancels a pending Monaco reveal frame when another file open starts', async () => { @@ -306,12 +320,15 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '/tmp/src/second.ts', - fileId: '/tmp/src/second.ts', - line: 20, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '/tmp/src/second.ts', + fileId: '/tmp/src/second.ts', + line: 20, + column: 3, + matchLength: 0 + }) + ) }) }) 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 2353ba68c0c..90aeb68c060 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 @@ -1,5 +1,6 @@ import { vi, type Mock } from 'vitest' import type { ExecutionHostId } from '../../../../shared/execution-host' +import type { Tab, TabGroup } from '../../../../shared/tab-types' export type TerminalLinkStoreSettings = { openLinksInApp?: boolean @@ -12,10 +13,17 @@ export type TerminalLinkStoreState = { setActiveWorktree: Mock createBrowserTab: Mock openFile: Mock + openFileForLink: Mock setPendingEditorReveal: Mock setMarkdownViewMode: Mock activeFileIdByWorktree: Record openFiles: { filePath: string; worktreeId: string }[] + groupsByWorktree: Record + unifiedTabsByWorktree: Record< + string, + (Pick & + Partial>)[] + > worktreesByRepo: Record< string, { @@ -34,6 +42,7 @@ export type TerminalLinkTestDoubles = { openFileUriMock: Mock openFilePathMock: Mock openFileMock: Mock + openFileForLinkMock: Mock statMock: Mock fsPathExistsMock: Mock runtimeEnvironmentCallMock: Mock @@ -62,15 +71,35 @@ export function createTerminalLinkTestDoubles(): TerminalLinkTestDoubles { const setMarkdownViewModeMock = vi.fn() const deps = { worktreeId: 'wt-1', worktreePath: '/tmp' } + // Why: specs assert the legacy openFile(file, options) shape; the link entry point delegates to it + // and records the view it "opened" so the guarded reveal can find it. + const openFileForLinkMock = vi.fn( + (file: { filePath: string; worktreeId: string }, options: unknown) => { + const returned: unknown = openFileMock(file, options) + const fileId = + typeof returned === 'string' + ? returned + : (storeState.activeFileIdByWorktree[file.worktreeId] ?? file.filePath) + const view = { id: `view:${fileId}`, entityId: fileId, groupId: 'group-1' } + storeState.unifiedTabsByWorktree = { + ...storeState.unifiedTabsByWorktree, + [file.worktreeId]: [...(storeState.unifiedTabsByWorktree[file.worktreeId] ?? []), view] + } + return { fileId, viewStateId: view.id, groupId: view.groupId } + } + ) const storeState: TerminalLinkStoreState = { settings: undefined as TerminalLinkStoreSettings | undefined, setActiveWorktree: setActiveWorktreeMock, createBrowserTab: createBrowserTabMock, openFile: openFileMock, + openFileForLink: openFileForLinkMock, setPendingEditorReveal: setPendingEditorRevealMock, setMarkdownViewMode: setMarkdownViewModeMock, activeFileIdByWorktree: {} as Record, openFiles: [] as { filePath: string; worktreeId: string }[], + unifiedTabsByWorktree: {}, + groupsByWorktree: {}, worktreesByRepo: {}, folderWorkspaces: [] as [] } @@ -80,6 +109,7 @@ export function createTerminalLinkTestDoubles(): TerminalLinkTestDoubles { openFileUriMock, openFilePathMock, openFileMock, + openFileForLinkMock, statMock, fsPathExistsMock, runtimeEnvironmentCallMock, 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 43fd7a05a6d..17b60add654 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,8 @@ export function installTerminalLinkTestEnvironment(doubles: TerminalLinkTestDoub storeState.settings = undefined storeState.activeFileIdByWorktree = {} storeState.openFiles = [] + storeState.unifiedTabsByWorktree = {} + storeState.groupsByWorktree = {} storeState.worktreesByRepo = {} registerHttpLinkStoreAccessor(() => storeState) vi.stubGlobal('window', { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts index b8cfd5c54ba..24741756e12 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -52,6 +52,8 @@ export type LinkHandlerDeps = { /** The pane's terminal on its paired server, which may grant files it printed. */ getRuntimeTerminalHandleForPane?: (paneId: number) => string | null getLinkActionContext?: (paneId: number) => TerminalLinkActionContext | null + /** The terminal tab whose panes print these links; editor opens place beside its group. */ + sourceTabId?: string } type ProvidedFileLink = { @@ -154,7 +156,8 @@ export function createFilePathLinkProvider( worktreePath, runtimeEnvironmentId, wslDistro: deps.wslDistro, - terminalHandle + terminalHandle, + sourceTabId: deps.sourceTabId }, deps.getLinkActionContext?.(paneId) ) @@ -249,7 +252,8 @@ export function installFilePathLinkClickFallback( wslDistro: deps.wslDistro, terminalHandle: deps.getRuntimeTerminalHandleForPane?.(paneId) ?? null, pathExistsCache: deps.pathExistsCache, - openWithSystemDefault: Boolean(event.shiftKey) + openWithSystemDefault: Boolean(event.shiftKey), + sourceTabId: deps.sourceTabId } ) if (opened) { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-open-beside-source.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-open-beside-source.test.ts new file mode 100644 index 00000000000..c80aed9db6e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-link-open-beside-source.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, it, vi } from 'vitest' +import { activateAndRevealWorkspace } from '@/lib/worktree-activation' +import { openDetectedFilePath } from './terminal-link-handlers' +import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' +import { + createDeferred, + flushAsyncWork, + flushDoubleRaf, + installTerminalLinkTestEnvironment, + setPlatform +} from './terminal-link-handlers-test-harness' +import type { PendingEditorReveal } from '@/store/slices/editor/types/pending-editor-reveal' + +const findWorkspaceFileRouteMock = vi.hoisted(() => vi.fn()) +const toastErrorMock = vi.hoisted(() => vi.fn()) +const doubles = createTerminalLinkTestDoubles() +const { + storeState, + deps, + openFileForLinkMock, + createBrowserTabMock, + openFilePathMock, + setPendingEditorRevealMock, + statMock +} = doubles + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => storeState + } +})) + +vi.mock('sonner', () => ({ toast: { error: toastErrorMock } })) + +vi.mock('@/lib/language-detect', () => ({ + detectLanguage: () => 'plaintext' +})) + +vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: vi.fn(), + activateAndRevealWorktree: vi.fn() +})) + +vi.mock('@/lib/connection-context', () => ({ + getConnectionId: vi.fn(() => null) +})) + +vi.mock('@/lib/runtime-workspace-file-route', () => ({ + findWorkspaceFileRoute: findWorkspaceFileRouteMock +})) + +installTerminalLinkTestEnvironment(doubles) + +function seedSourceTab(): void { + storeState.groupsByWorktree = { + 'wt-1': [ + { id: 'group-a', worktreeId: 'wt-1', activeTabId: 'unified-term', tabOrder: ['unified-term'] } + ] + } + storeState.unifiedTabsByWorktree = { + 'wt-1': [ + { id: 'unified-term', entityId: 'term-1', groupId: 'group-a', contentType: 'terminal' } + ] + } +} + +const sourceDeps = { ...deps, sourceTabId: 'term-1' } + +function lastReveal(): PendingEditorReveal { + return setPendingEditorRevealMock.mock.calls.at(-1)?.[0] +} + +describe('terminal file links carry their source tab', () => { + it('captures the source tab at activation and passes it to the open', async () => { + setPlatform('Macintosh') + seedSourceTab() + const stat = createDeferred<{ isDirectory: boolean }>() + statMock.mockImplementationOnce(() => stat.promise) + + openDetectedFilePath('/tmp/src/a.ts', null, null, sourceDeps) + // Why: a move after the click changes where ranking starts, not which tab was captured. + storeState.unifiedTabsByWorktree = {} + stat.resolve({ isDirectory: false }) + await flushAsyncWork() + + expect(openFileForLinkMock).toHaveBeenCalledWith( + expect.objectContaining({ filePath: '/tmp/src/a.ts', worktreeId: 'wt-1' }), + { forceContentReload: true }, + { sourceTabId: 'unified-term', workspaceId: 'wt-1', hostId: null } + ) + }) + + it('pins an explicit request to the source group', async () => { + setPlatform('Macintosh') + seedSourceTab() + + openDetectedFilePath('/tmp/src/a.ts', null, null, { ...sourceDeps, openInSourceGroup: true }) + await flushAsyncWork() + + expect(openFileForLinkMock).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ + workspaceId: 'wt-1', + hostId: null, + targetGroupId: 'group-a' + }) + ) + }) + + it('refuses an explicit request that resolves to Orca browser HTML', async () => { + setPlatform('Macintosh') + seedSourceTab() + + openDetectedFilePath('/tmp/report.html', null, null, { ...sourceDeps, openInSourceGroup: true }) + await flushAsyncWork() + + expect(createBrowserTabMock).not.toHaveBeenCalled() + expect(openFileForLinkMock).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledTimes(1) + }) + + it('refuses a closed explicit group before activating its workspace', async () => { + setPlatform('Macintosh') + seedSourceTab() + const stat = createDeferred<{ isDirectory: boolean }>() + statMock.mockImplementationOnce(() => stat.promise) + + openDetectedFilePath('/tmp/src/a.ts', null, null, { ...sourceDeps, openInSourceGroup: true }) + storeState.groupsByWorktree = {} + stat.resolve({ isDirectory: false }) + await flushAsyncWork() + + expect(activateAndRevealWorkspace).not.toHaveBeenCalled() + expect(openFileForLinkMock).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledTimes(1) + }) + + it('checks the resolved host even when no sibling-workspace route is needed', async () => { + setPlatform('Macintosh') + seedSourceTab() + storeState.unifiedTabsByWorktree['wt-1'][0].executionHostId = 'ssh:other-host' + + openDetectedFilePath('/tmp/src/a.ts', null, null, { ...sourceDeps, openInSourceGroup: true }) + await flushAsyncWork() + + expect(activateAndRevealWorkspace).not.toHaveBeenCalled() + expect(openFileForLinkMock).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledTimes(1) + }) + + it('refuses an explicit request that resolves to a directory', async () => { + setPlatform('Macintosh') + seedSourceTab() + statMock.mockResolvedValueOnce({ isDirectory: true }) + + openDetectedFilePath('/tmp/src', null, null, { ...sourceDeps, openInSourceGroup: true }) + await flushAsyncWork() + + expect(openFilePathMock).not.toHaveBeenCalled() + expect(openFileForLinkMock).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledTimes(1) + }) + + it('refuses an explicit request routed to another workspace before activating it', async () => { + setPlatform('Macintosh') + seedSourceTab() + storeState.openFiles = [{ filePath: '/other/x.ts', worktreeId: 'wt-2' }] + findWorkspaceFileRouteMock.mockReturnValueOnce({ + worktreeId: 'wt-2', + executionHostId: 'local', + relativePath: 'x.ts' + }) + + openDetectedFilePath('/other/x.ts', null, null, { ...sourceDeps, openInSourceGroup: true }) + await flushAsyncWork() + + expect(activateAndRevealWorkspace).not.toHaveBeenCalled() + expect(openFileForLinkMock).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledTimes(1) + }) + + it('keeps automatic HTML opens on their existing browser route', async () => { + setPlatform('Macintosh') + seedSourceTab() + + openDetectedFilePath('/tmp/report.html', null, null, sourceDeps) + await flushAsyncWork() + + expect(createBrowserTabMock).toHaveBeenCalledTimes(1) + expect(toastErrorMock).not.toHaveBeenCalled() + }) +}) + +describe('terminal file link reveals target the opened view', () => { + it('scopes the reveal to the returned view', async () => { + setPlatform('Macintosh') + + openDetectedFilePath('/tmp/src/a.ts', 12, 3, deps) + await flushAsyncWork() + await flushDoubleRaf() + + const reveal = lastReveal() + expect(reveal).toMatchObject({ + fileId: '/tmp/src/a.ts', + viewStateId: 'view:/tmp/src/a.ts', + line: 12, + column: 3 + }) + expect(reveal.canApply?.()).toBe(true) + }) + + it('invalidates a published reveal when a newer click has no line', async () => { + setPlatform('Macintosh') + + openDetectedFilePath('/tmp/src/a.ts', 12, 3, deps) + await flushAsyncWork() + await flushDoubleRaf() + const reveal = lastReveal() + + openDetectedFilePath('/tmp/src/b.ts', null, null, deps) + + expect(reveal.canApply?.()).toBe(false) + }) + + it('invalidates a reveal whose view closed before the editor consumed it', async () => { + setPlatform('Macintosh') + + openDetectedFilePath('/tmp/src/a.ts', 12, 3, deps) + await flushAsyncWork() + await flushDoubleRaf() + const reveal = lastReveal() + + storeState.unifiedTabsByWorktree = { 'wt-1': [] } + + expect(reveal.canApply?.()).toBe(false) + }) + + it('skips the reveal when the open was refused', async () => { + setPlatform('Macintosh') + openFileForLinkMock.mockReturnValueOnce(null) + + openDetectedFilePath('/tmp/src/a.ts', 12, 3, deps) + await flushAsyncWork() + await flushDoubleRaf() + + expect(setPendingEditorRevealMock).not.toHaveBeenCalled() + }) +}) 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 bf5381ce9ef..d9c73dce2fd 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 @@ -89,13 +89,16 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: 'C:/repo/src/index.ts', - fileId: 'C:/repo/src/index.ts', - line: 12, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: 'C:/repo/src/index.ts', + fileId: 'C:/repo/src/index.ts', + line: 12, + column: 3, + matchLength: 0 + }) + ) }) it('opens Windows UNC file URL links from Windows worktrees', async () => { @@ -150,13 +153,16 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '/tmp/test.txt', - fileId: '/tmp/test.txt', - line: 42, - column: 1, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '/tmp/test.txt', + fileId: '/tmp/test.txt', + line: 42, + column: 1, + matchLength: 0 + }) + ) }) it('opens file URL links with the system default app for shift+cmd/ctrl-click', async () => { @@ -193,13 +199,16 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '/tmp/test.txt', - fileId: '/tmp/test.txt', - line: 42, - column: 7, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '/tmp/test.txt', + fileId: '/tmp/test.txt', + line: 42, + column: 7, + matchLength: 0 + }) + ) }) it('opens UNC file URL links with line and column anchors', async () => { @@ -227,13 +236,16 @@ describe('handleOscLink', () => { { forceContentReload: true } ) expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(1, null) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '//server/Share/Repo/src/app.ts', - fileId: '//server/Share/Repo/src/app.ts', - line: 12, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '//server/Share/Repo/src/app.ts', + fileId: '//server/Share/Repo/src/app.ts', + line: 12, + column: 3, + matchLength: 0 + }) + ) }) it('opens relative OSC file links against the terminal cwd', async () => { @@ -290,13 +302,16 @@ describe('handleOscLink', () => { }), { forceContentReload: true } ) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', - fileId: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', - line: 5, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', + fileId: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', + line: 5, + column: 3, + matchLength: 0 + }) + ) }) it('maps file URL OSC links for a WSL worktree before opening them', async () => { @@ -325,13 +340,16 @@ describe('handleOscLink', () => { }), { forceContentReload: true } ) - expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith(2, { - filePath: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', - fileId: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', - line: 5, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + filePath: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', + fileId: '\\\\wsl.localhost\\Ubuntu\\root\\workspace\\myrepo\\README.md', + line: 5, + column: 3, + matchLength: 0 + }) + ) }) it('opens tilde OSC file links against explicit terminal home when cwd is outside home', async () => { 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 744cd12f988..82f2abcad81 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 @@ -72,13 +72,15 @@ describe('createFilePathLinkProvider range bounds', () => { expect(openFileMock).toHaveBeenCalledWith(expect.objectContaining({ filePath: mappedPath }), { forceContentReload: true }) - expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith({ - filePath: mappedPath, - fileId: mappedPath, - line: 5, - column: 3, - matchLength: 0 - }) + expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith( + expect.objectContaining({ + filePath: mappedPath, + fileId: mappedPath, + line: 5, + column: 3, + matchLength: 0 + }) + ) }) it('resolves relative POSIX terminal links against the pane cwd before mapping', async () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-osc-link-routing.ts b/src/renderer/src/components/terminal-pane/terminal-osc-link-routing.ts index b041cb65f0d..22c3db57a8b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-osc-link-routing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-osc-link-routing.ts @@ -48,7 +48,7 @@ export function handleOscLink( Partial< Pick< LinkHandlerDeps, - 'runtimeEnvironmentId' | 'startupCwd' | 'terminalHomePath' | 'wslDistro' + 'runtimeEnvironmentId' | 'startupCwd' | 'terminalHomePath' | 'wslDistro' | 'sourceTabId' > > & { sourceOwner?: HttpLinkSourceOwner diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts b/src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts index 82984b64c8f..2f3bf3ce648 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-mount-preparation.ts @@ -172,7 +172,8 @@ export function prepareTerminalPaneMount( return sourceOwner.kind === 'runtime' ? sourceOwner.runtimeEnvironmentId : null }, getRuntimeTerminalHandleForPane: (id) => resolveTerminalRuntimeHandle(paneTransports.get(id)), - getLinkActionContext + getLinkActionContext, + sourceTabId: deps.tabId } let resizeRaf: number | null = null const queueResizeAll = (focusActive: boolean): void => { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 5b437bfd2dd..b65dcaf6dc4 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -3521,7 +3521,9 @@ "copiedLink": "Copied link", "copyLinkFailed": "Failed to copy link", "downloadOpenWithDefaultApp": "Download & open with default app", - "downloadOpenFailed": "Failed to download '{{value0}}'." + "downloadOpenFailed": "Failed to download '{{value0}}'.", + "openInThisGroup": "Open in this group", + "openInThisGroupRefused": "Couldn't open {{value0}} in this group" }, "TerminalSessionStateSaveFailureDialog": { "6bee0c8f17": "Open Disk Space Analyzer", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index c18acc13531..88f8b780700 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -3427,7 +3427,9 @@ "copiedLink": "Enlace copiado", "copyLinkFailed": "No se pudo copiar el enlace", "downloadOpenWithDefaultApp": "Descargar y abrir con la aplicación predeterminada", - "downloadOpenFailed": "No se pudo descargar '{{value0}}'." + "downloadOpenFailed": "No se pudo descargar '{{value0}}'.", + "openInThisGroup": "Abrir en este grupo", + "openInThisGroupRefused": "No se pudo abrir {{value0}} en este grupo" }, "TerminalQuickCommandsSubmenu": { "3ccc7981bb": "Anfitrión no disponible", diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 9b4039ed242..a501c973fbb 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -3401,7 +3401,9 @@ "copiedLink": "Lien copié", "copyLinkFailed": "Échec de la copie du lien", "downloadOpenWithDefaultApp": "Téléchargez et ouvrez avec l'application par défaut", - "downloadOpenFailed": "Échec du téléchargement de « {{value0}} »." + "downloadOpenFailed": "Échec du téléchargement de « {{value0}} ».", + "openInThisGroup": "Ouvrir dans ce groupe", + "openInThisGroupRefused": "Impossible d'ouvrir {{value0}} dans ce groupe" }, "TerminalSessionStateSaveFailureDialog": { "6bee0c8f17": "Ouvrir l'Analyseur d'espace disque", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index ee50bc24967..33a7758d01e 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -3429,7 +3429,9 @@ "copiedLink": "コピーされたリンク", "copyLinkFailed": "リンクのコピーに失敗しました", "downloadOpenWithDefaultApp": "ダウンロードしてデフォルトのアプリで開く", - "downloadOpenFailed": "「{{value0}}」のダウンロードに失敗しました。" + "downloadOpenFailed": "「{{value0}}」のダウンロードに失敗しました。", + "openInThisGroup": "このグループで開く", + "openInThisGroupRefused": "{{value0}} をこのグループで開けませんでした" }, "TerminalQuickCommandsSubmenu": { "3ccc7981bb": "ホストが利用不可", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 2587ba6f7dc..a573b8eea8b 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -3429,7 +3429,9 @@ "copiedLink": "복사된 링크", "copyLinkFailed": "링크를 복사하지 못했습니다.", "downloadOpenWithDefaultApp": "기본 앱 다운로드 및 열기", - "downloadOpenFailed": "'{{value0}}'을(를) 다운로드하지 못했습니다." + "downloadOpenFailed": "'{{value0}}'을(를) 다운로드하지 못했습니다.", + "openInThisGroup": "이 그룹에서 열기", + "openInThisGroupRefused": "이 그룹에서 {{value0}}을(를) 열 수 없습니다" }, "TerminalQuickCommandsSubmenu": { "3ccc7981bb": "호스트를 사용할 수 없음", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 9170f34a78e..55209715352 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -3429,7 +3429,9 @@ "copiedLink": "已复制链接", "copyLinkFailed": "复制链接失败", "downloadOpenWithDefaultApp": "下载并使用默认应用程序打开", - "downloadOpenFailed": "无法下载“{{value0}}”。" + "downloadOpenFailed": "无法下载“{{value0}}”。", + "openInThisGroup": "在此分组中打开", + "openInThisGroupRefused": "无法在此分组中打开 {{value0}}" }, "TerminalQuickCommandsSubmenu": { "3ccc7981bb": "主机不可用", diff --git a/src/renderer/src/store/slices/editor-open-link-placement.test.ts b/src/renderer/src/store/slices/editor-open-link-placement.test.ts new file mode 100644 index 00000000000..bb432f29505 --- /dev/null +++ b/src/renderer/src/store/slices/editor-open-link-placement.test.ts @@ -0,0 +1,323 @@ +import type { StoreApi } from 'zustand/vanilla' +import { describe, expect, it, vi } from 'vitest' +import { createEditorTabsStore } from './editor-slice-test-harness' +import type { AppState } from '../types' +import type { OpenFile } from './editor/types/open-file' +import { captureEditorOpenLinkIntent } from './editor/tabs/editor-open-target-group' +import { removeGroupLayoutLeaf } from '../../../../shared/workspace-layout/tab-group-layout-tree' + +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) + +function fileInput(filePath: string, worktreeId = 'wt-1'): Omit { + return { + filePath, + relativePath: filePath.replace('/repo/', ''), + worktreeId, + language: 'typescript', + mode: 'edit' + } +} + +function splitRight(store: StoreApi, groupId: string): string { + const created = store.getState().createEmptySplitGroup('wt-1', groupId, 'right') + if (!created) { + throw new Error('Expected split group') + } + return created +} + +function addTab( + store: StoreApi, + contentType: 'terminal' | 'agent-session', + id: string, + groupId?: string +): void { + store.getState().createUnifiedTab('wt-1', contentType, { + id, + entityId: `${id}-entity`, + label: id, + ...(groupId ? { targetGroupId: groupId } : {}) + }) +} + +function addEditor(store: StoreApi, filePath: string, groupId: string): void { + store.getState().openFile(fileInput(filePath), { targetGroupId: groupId }) +} + +/** Drops a tab the way a close would, without the terminal slice the harness lacks. */ +function removeTab(store: StoreApi, tabId: string): void { + const state = store.getState() + store.setState({ + unifiedTabsByWorktree: { + 'wt-1': state.unifiedTabsByWorktree['wt-1']!.filter((tab) => tab.id !== tabId) + }, + groupsByWorktree: { + 'wt-1': state.groupsByWorktree['wt-1']!.map((group) => { + const tabOrder = group.tabOrder.filter((id) => id !== tabId) + return { ...group, tabOrder, activeTabId: tabOrder.at(-1) ?? null } + }) + } + }) +} + +function removeGroup(store: StoreApi, groupId: string): void { + const state = store.getState() + store.setState({ + groupsByWorktree: { + 'wt-1': state.groupsByWorktree['wt-1']!.filter((group) => group.id !== groupId) + }, + layoutByWorktree: { + 'wt-1': removeGroupLayoutLeaf(state.layoutByWorktree['wt-1']!, groupId)! + } + }) +} + +function focus(store: StoreApi, groupId: string): void { + store.setState({ activeGroupIdByWorktree: { 'wt-1': groupId } }) +} + +function groupOfFile(store: StoreApi, filePath: string): string[] { + return (store.getState().unifiedTabsByWorktree['wt-1'] ?? []) + .filter((tab) => tab.entityId === filePath) + .map((tab) => tab.groupId) +} + +/** Three columns A–B–C with the source tab in A. */ +function seedColumns( + store: StoreApi, + source: 'terminal' | 'agent-session', + middle: 'terminal' | 'editor', + right: 'terminal' | 'editor' +): { a: string; b: string; c: string } { + addTab(store, source, 'source') + const a = store.getState().groupsByWorktree['wt-1']![0]!.id + const b = splitRight(store, a) + const c = splitRight(store, b) + for (const [groupId, kind] of [ + [b, middle], + [c, right] + ] as const) { + if (kind === 'editor') { + addEditor(store, `/repo/${groupId}-seed.ts`, groupId) + } else { + addTab(store, 'terminal', `${groupId}-terminal`, groupId) + } + } + focus(store, a) + return { a, b, c } +} + +function linkIntent(store: StoreApi, explicit = false) { + return captureEditorOpenLinkIntent(store.getState(), 'wt-1', 'source', { + explicitSourceGroup: explicit + }) +} + +describe('file links open beside their source', () => { + it('captures the source only within the workspace that owns the link', () => { + const store = createEditorTabsStore() + store.getState().createUnifiedTab('wt-2', 'terminal', { id: 'source' }) + addTab(store, 'terminal', 'source') + + expect(linkIntent(store)?.workspaceId).toBe('wt-1') + expect(captureEditorOpenLinkIntent(store.getState(), 'missing', 'source')).toBeNull() + }) + + it('moves a native-chat link from the chat group to the neighboring editor', () => { + const store = createEditorTabsStore() + addTab(store, 'agent-session', 'source') + const a = store.getState().groupsByWorktree['wt-1']![0]!.id + const b = splitRight(store, a) + addEditor(store, '/repo/seed.ts', b) + focus(store, a) + + store.getState().openFile(fileInput('/repo/plain.ts')) + const opened = store + .getState() + .openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store)) + + // Why: the plain open documents the unchanged baseline for callers without link intent. + expect(groupOfFile(store, '/repo/plain.ts')).toEqual([a]) + expect(groupOfFile(store, '/repo/link.ts')).toEqual([b]) + expect(opened).toMatchObject({ fileId: '/repo/link.ts', groupId: b }) + }) + + it('covers the adjacent terminal instead of jumping to a nonadjacent editor', () => { + const store = createEditorTabsStore() + const { a, b, c } = seedColumns(store, 'terminal', 'terminal', 'editor') + + store.getState().openFile(fileInput('/repo/plain.ts')) + focus(store, a) + store.getState().openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store)) + + expect(groupOfFile(store, '/repo/plain.ts')).toEqual([c]) + expect(groupOfFile(store, '/repo/link.ts')).toEqual([b]) + }) + + it('selects an existing unselected view of the file over a nearby editor', () => { + const store = createEditorTabsStore() + const { a, b, c } = seedColumns(store, 'terminal', 'editor', 'terminal') + addEditor(store, '/repo/link.ts', c) + store.getState().activateTab(`${c}-terminal`) + focus(store, a) + const existing = store + .getState() + .unifiedTabsByWorktree['wt-1']!.find((tab) => tab.entityId === '/repo/link.ts')! + + const opened = store + .getState() + .openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store)) + + expect(groupOfFile(store, '/repo/link.ts')).toEqual([c]) + expect(opened).toEqual({ fileId: '/repo/link.ts', viewStateId: existing.id, groupId: c }) + expect(store.getState().groupsByWorktree['wt-1']!.find((g) => g.id === c)?.activeTabId).toBe( + existing.id + ) + expect(b).not.toBe(c) + }) + + it('opens a second view in the neighbor when only the source holds the file', () => { + const store = createEditorTabsStore() + const { a, b } = seedColumns(store, 'terminal', 'terminal', 'terminal') + addEditor(store, '/repo/link.ts', a) + store.getState().activateTab('source') + + const opened = store + .getState() + .openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store)) + + expect(groupOfFile(store, '/repo/link.ts').sort()).toEqual([a, b].sort()) + expect(store.getState().openFiles.filter((f) => f.filePath === '/repo/link.ts')).toHaveLength(1) + expect(opened?.groupId).toBe(b) + }) + + it('keeps placement on the captured source when focus moves during resolution', () => { + const store = createEditorTabsStore() + const { b, c } = seedColumns(store, 'terminal', 'terminal', 'terminal') + const intent = linkIntent(store) + focus(store, c) + + store.getState().openFileForLink(fileInput('/repo/link.ts'), {}, intent) + + expect(groupOfFile(store, '/repo/link.ts')).toEqual([b]) + }) + + it('follows a source tab that moved to another group while its old group survives', () => { + const store = createEditorTabsStore() + const { a, b, c } = seedColumns(store, 'terminal', 'terminal', 'terminal') + addTab(store, 'terminal', 'keeps-a-alive', a) + const intent = linkIntent(store) + store.getState().moveUnifiedTabToGroup(intent!.sourceTabId, b) + + store.getState().openFileForLink(fileInput('/repo/link.ts'), {}, intent) + + // From B the preferred neighbor is C; a stale source in A would choose B. + expect(groupOfFile(store, '/repo/link.ts')).toEqual([c]) + }) + + it('uses default placement once the source tab has closed', () => { + const store = createEditorTabsStore() + const { a, c } = seedColumns(store, 'terminal', 'terminal', 'editor') + addTab(store, 'terminal', 'keeps-a-alive', a) + const intent = linkIntent(store) + removeTab(store, intent!.sourceTabId) + focus(store, a) + + store.getState().openFileForLink(fileInput('/repo/link.ts'), {}, intent) + + // The focused terminal's existing preference sends the file to the visible editor. + expect(groupOfFile(store, '/repo/link.ts')).toEqual([c]) + }) + + it('uses destination defaults when resolution routes to another workspace', () => { + const store = createEditorTabsStore() + seedColumns(store, 'terminal', 'terminal', 'terminal') + + const opened = store + .getState() + .openFileForLink(fileInput('/other/link.ts', 'wt-2'), {}, linkIntent(store)) + + expect(opened?.groupId).toBe(store.getState().groupsByWorktree['wt-2']?.[0]?.id) + }) + + it('pins an explicit request to the captured group even when another group has the file', () => { + const store = createEditorTabsStore() + const { a, c } = seedColumns(store, 'terminal', 'terminal', 'terminal') + addEditor(store, '/repo/link.ts', c) + store.getState().activateTab('source') + + const opened = store + .getState() + .openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store, true)) + + expect(opened?.groupId).toBe(a) + expect(groupOfFile(store, '/repo/link.ts').sort()).toEqual([a, c].sort()) + }) + + it('refuses an explicit request whose group closed, without touching editor state', () => { + const store = createEditorTabsStore() + const { b } = seedColumns(store, 'terminal', 'terminal', 'terminal') + store.getState().activateTab(`${b}-terminal`) + const intent = captureEditorOpenLinkIntent(store.getState(), 'wt-1', `${b}-terminal`, { + explicitSourceGroup: true + }) + removeGroup(store, b) + const before = store.getState().openFiles + + const opened = store.getState().openFileForLink(fileInput('/repo/link.ts'), {}, intent) + + expect(opened).toBeNull() + expect(store.getState().openFiles).toBe(before) + expect(store.getState().groupsByWorktree['wt-1']!.some((g) => g.id === b)).toBe(false) + }) + + it('refuses when the chosen group disappears at the mutation seam', () => { + const store = createEditorTabsStore() + const { b } = seedColumns(store, 'terminal', 'terminal', 'terminal') + const intent = linkIntent(store) + const unsubscribe = store.subscribe((state, previous) => { + if (state.openFiles !== previous.openFiles) { + unsubscribe() + removeGroup(store, b) + } + }) + + const opened = store.getState().openFileForLink(fileInput('/repo/link.ts'), {}, intent) + + expect(opened).toBeNull() + expect(groupOfFile(store, '/repo/link.ts')).toEqual([]) + expect(store.getState().groupsByWorktree['wt-1']!.some((g) => g.id === b)).toBe(false) + }) + + it('returns the created view and focuses it', () => { + const store = createEditorTabsStore() + const { b } = seedColumns(store, 'terminal', 'terminal', 'terminal') + + const opened = store + .getState() + .openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store)) + const view = store + .getState() + .unifiedTabsByWorktree['wt-1']!.find((tab) => tab.entityId === '/repo/link.ts') + + expect(opened).toEqual({ fileId: '/repo/link.ts', viewStateId: view?.id, groupId: b }) + expect(store.getState().activeGroupIdByWorktree['wt-1']).toBe(b) + expect(store.getState().pendingEditorFocusRequest).toMatchObject({ + fileId: '/repo/link.ts', + viewStateId: view?.id + }) + }) + + it('keeps the source in place when it is the only group', () => { + const store = createEditorTabsStore() + addTab(store, 'terminal', 'source') + const a = store.getState().groupsByWorktree['wt-1']![0]!.id + + const opened = store + .getState() + .openFileForLink(fileInput('/repo/link.ts'), {}, linkIntent(store)) + + expect(opened?.groupId).toBe(a) + expect(store.getState().groupsByWorktree['wt-1']).toHaveLength(1) + }) +}) diff --git a/src/renderer/src/store/slices/editor/actions/open-file-action.ts b/src/renderer/src/store/slices/editor/actions/open-file-action.ts index a6d22a490c6..d32fa8943e6 100644 --- a/src/renderer/src/store/slices/editor/actions/open-file-action.ts +++ b/src/renderer/src/store/slices/editor/actions/open-file-action.ts @@ -1,64 +1,98 @@ import type { EditorGet, EditorSet } from '../types/editor-set-get' import type { EditorSlice } from '../types/editor-slice' +import type { OpenFile } from '../types/open-file' +import type { OpenFileOptions } from '../types/editor-files-slice' +import type { EditorOpenLinkIntent, EditorOpenResult } from '../types/editor-open-link-intent' import { openWorkspaceEditorItem } from '../tabs/workspace-editor-item' -import type { EditorTabContentType } from '../tabs/editor-tab-content-type' +import { editorTabContentTypeForMode } from '../tabs/editor-tab-content-type' +import { isLiveEditorOpenGroup } from '../tabs/editor-open-target-group' import { EDITOR_FOCUS_REQUEST_TTL_MS, takeNextEditorFocusRequestToken } from '../focus/editor-focus-reveal' -import { applyOpenFileToState } from './open-file-apply' +import { applyOpenFileToState, type OpenFileApplyScratch } from './open-file-apply' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../../shared/layout-owner-switch' import { recordReopenedEditorFileInLayoutOwner } from '../tabs/editor-tab-layout-owner' export function createOpenFileAction( set: EditorSet, get: EditorGet -): Pick { - return { - openFile: (file, options) => { - const editorItemWorktreeId = file.worktreeId - const editorItemLabel = file.relativePath - const editorItemContentType: EditorTabContentType = - file.mode === 'conflict-review' || file.mode === 'check-details' || file.mode === 'diff' - ? file.mode - : 'editor' - const scratch = { - editorItemFileId: file.filePath, - editorItemTargetGroupId: options?.targetGroupId, - editorItemIsPreview: false - } - const before = ORCA_LAYOUT_OWNER_IS_MAIN ? get() : null - set((s) => applyOpenFileToState(s, file, options, scratch)) - const editorItemViewStateId = openWorkspaceEditorItem( +): Pick { + const open = ( + file: Omit, + options: OpenFileOptions | undefined, + linkIntent: EditorOpenLinkIntent | null + ): { fileId: string; result: EditorOpenResult | null } => { + const editorItemWorktreeId = file.worktreeId + const scratch: OpenFileApplyScratch = { + editorItemFileId: file.filePath, + editorItemTargetGroupId: options?.targetGroupId, + editorItemIsPreview: false, + refused: false + } + const before = ORCA_LAYOUT_OWNER_IS_MAIN ? get() : null + set((s) => applyOpenFileToState(s, file, options, scratch, linkIntent)) + if (scratch.refused) { + return { fileId: scratch.editorItemFileId, result: null } + } + // Why: a reentrant update may remove the chosen group after selection; never recreate it. + if ( + linkIntent && + scratch.editorItemTargetGroupId && + !isLiveEditorOpenGroup(get(), editorItemWorktreeId, scratch.editorItemTargetGroupId) + ) { + return { fileId: scratch.editorItemFileId, result: null } + } + const editorItemViewStateId = openWorkspaceEditorItem( + get(), + scratch.editorItemFileId, + editorItemWorktreeId, + file.relativePath, + editorTabContentTypeForMode(file.mode), + scratch.editorItemIsPreview, + scratch.editorItemTargetGroupId, + options?.selection + ) + if (ORCA_LAYOUT_OWNER_IS_MAIN && before) { + recordReopenedEditorFileInLayoutOwner( + before, get(), - scratch.editorItemFileId, editorItemWorktreeId, - editorItemLabel, - editorItemContentType, - scratch.editorItemIsPreview, - scratch.editorItemTargetGroupId, - options?.selection + editorItemViewStateId ) - if (ORCA_LAYOUT_OWNER_IS_MAIN && before) { - recordReopenedEditorFileInLayoutOwner( - before, - get(), - editorItemWorktreeId, - editorItemViewStateId - ) + } + if (options?.focusEditor) { + set({ + pendingEditorFocusRequest: { + fileId: scratch.editorItemFileId, + worktreeId: editorItemWorktreeId, + viewStateId: editorItemViewStateId, + expiresAt: Date.now() + EDITOR_FOCUS_REQUEST_TTL_MS, + token: takeNextEditorFocusRequestToken() + } + }) + } + // Why: openWorkspaceEditorItem falls back to the file id when no tab exists; that is not a view. + const view = (get().unifiedTabsByWorktree?.[editorItemWorktreeId] ?? []).find( + (tab) => tab.id === editorItemViewStateId && tab.entityId === scratch.editorItemFileId + ) + return { + fileId: scratch.editorItemFileId, + result: view + ? { fileId: scratch.editorItemFileId, viewStateId: view.id, groupId: view.groupId } + : null + } + } + + return { + openFile: (file, options) => open(file, options, null).fileId, + openFileForLink: (file, options, intent) => { + const { result } = open(file, { ...options, focusEditor: true }, intent) + // Why: created and reused views must land identically; creating in another group leaves focus behind. + if (result) { + get().focusGroup?.(file.worktreeId, result.groupId) } - if (options?.focusEditor) { - set({ - pendingEditorFocusRequest: { - fileId: scratch.editorItemFileId, - worktreeId: editorItemWorktreeId, - viewStateId: editorItemViewStateId, - expiresAt: Date.now() + EDITOR_FOCUS_REQUEST_TTL_MS, - token: takeNextEditorFocusRequestToken() - } - }) - } - return scratch.editorItemFileId + return result } } } diff --git a/src/renderer/src/store/slices/editor/actions/open-file-apply.ts b/src/renderer/src/store/slices/editor/actions/open-file-apply.ts index 80ebbabde55..f6c38b57f38 100644 --- a/src/renderer/src/store/slices/editor/actions/open-file-apply.ts +++ b/src/renderer/src/store/slices/editor/actions/open-file-apply.ts @@ -5,7 +5,6 @@ import { getRecentlyClosedTabPosition, pushRecentlyClosedTabKind } from '../../r import type { AppState } from '../../../types' import { type ClosedEditorTabSnapshot, - type EditorTabSelection, MAX_RECENT_CLOSED_EDITOR_TABS, type OpenFile } from '../types/open-file' @@ -19,8 +18,11 @@ import { } from '../file-ids/editor-file-ids' import { buildEditorActiveResult, + resolveEditorLinkOpenPlacement, resolveEditorOpenTargetGroupId } from '../tabs/editor-open-target-group' +import type { OpenFileOptions } from '../types/editor-files-slice' +import type { EditorOpenLinkIntent } from '../types/editor-open-link-intent' import { resolveEditorPreviewIntent } from '../tabs/editor-preview-tab-setting' import { getReplaceablePreviewFileId, @@ -32,24 +34,16 @@ export type OpenFileApplyScratch = { editorItemTargetGroupId: string | undefined /** Resolved against the setting inside the reducer so the tab can't disagree with its OpenFile. */ editorItemIsPreview: boolean + /** Set when a link's explicit target failed revalidation; state was left untouched. */ + refused: boolean } export function applyOpenFileToState( s: AppState, file: Omit, - options: - | { - preview?: boolean - targetGroupId?: string - recordReplacedPreview?: boolean - suppressActiveRuntimeFallback?: boolean - forceContentReload?: boolean - focusEditor?: boolean - reopenId?: string - selection?: EditorTabSelection - } - | undefined, - scratch: OpenFileApplyScratch + options: OpenFileOptions | undefined, + scratch: OpenFileApplyScratch, + linkIntent: EditorOpenLinkIntent | null = null ): Partial | AppState { const worktreeId = file.worktreeId let operationProvenance = file.operationProvenance @@ -93,13 +87,20 @@ export function applyOpenFileToState( runtimeEnvironmentId, reusableOpenFileModes ) + const linkPlacement = + linkIntent && resolveEditorLinkOpenPlacement(s, worktreeId, linkIntent, id, file.mode) + if (linkPlacement?.kind === 'refused') { + scratch.refused = true + return s + } scratch.editorItemFileId = id const isPreview = resolveEditorPreviewIntent(s, options?.preview) scratch.editorItemIsPreview = isPreview const recordReplacedPreview = options?.recordReplacedPreview ?? false // Why: resolve the target group up-front so preview replacement is scoped to it (group B open must not evict group A's preview). - const targetGroupId = - resolveEditorOpenTargetGroupId(s, worktreeId, options?.targetGroupId) ?? undefined + const requestedGroupId = + linkPlacement?.kind === 'group' ? linkPlacement.groupId : options?.targetGroupId + const targetGroupId = resolveEditorOpenTargetGroupId(s, worktreeId, requestedGroupId) ?? undefined scratch.editorItemTargetGroupId = targetGroupId const activeResult = options?.selection === 'none' ? {} : buildEditorActiveResult(s, worktreeId, id) diff --git a/src/renderer/src/store/slices/editor/tabs/editor-open-adjacent-group.test.ts b/src/renderer/src/store/slices/editor/tabs/editor-open-adjacent-group.test.ts new file mode 100644 index 00000000000..841cae97da3 --- /dev/null +++ b/src/renderer/src/store/slices/editor/tabs/editor-open-adjacent-group.test.ts @@ -0,0 +1,227 @@ +import { describe, expect, it } from 'vitest' +import type { Tab, TabGroup, TabGroupLayoutNode } from '../../../../../../shared/tab-types' +import { findNeighbor, pickAdjacentEditorOpenGroupId } from './editor-open-adjacent-group' + +const FILE_ID = '/repo/src/main.ts' + +const leaf = (groupId: string): TabGroupLayoutNode => ({ type: 'leaf', groupId }) +const columns = ( + first: TabGroupLayoutNode, + second: TabGroupLayoutNode, + ratio?: number +): TabGroupLayoutNode => ({ type: 'split', direction: 'horizontal', first, second, ratio }) +const rows = ( + first: TabGroupLayoutNode, + second: TabGroupLayoutNode, + ratio?: number +): TabGroupLayoutNode => ({ type: 'split', direction: 'vertical', first, second, ratio }) + +function tab(id: string, groupId: string, contentType: Tab['contentType'], entityId = id): Tab { + return { + id, + entityId, + groupId, + worktreeId: 'wt-1', + contentType, + label: id, + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} + +type Fixture = { + /** Groups whose selected tab is an editor (other than the file); the rest show a terminal. */ + editors?: string[] + /** Groups holding an unselected tab of the file. */ + withFile?: string[] + /** Groups holding an unselected, unrelated editor tab behind their terminal. */ + inactiveEditors?: string[] +} + +function collectGroupIds(node: TabGroupLayoutNode): string[] { + return node.type === 'leaf' + ? [node.groupId] + : [...collectGroupIds(node.first), ...collectGroupIds(node.second)] +} + +function buildState(layout: TabGroupLayoutNode, fixture: Fixture = {}) { + const groups: TabGroup[] = [] + const tabs: Tab[] = [] + for (const groupId of collectGroupIds(layout)) { + const active = fixture.editors?.includes(groupId) + ? tab(`${groupId}-editor`, groupId, 'editor') + : tab(`${groupId}-terminal`, groupId, 'terminal') + tabs.push(active) + if (fixture.withFile?.includes(groupId)) { + tabs.push(tab(`${groupId}-file`, groupId, 'editor', FILE_ID)) + } + if (fixture.inactiveEditors?.includes(groupId)) { + tabs.push(tab(`${groupId}-old-editor`, groupId, 'editor')) + } + groups.push({ + id: groupId, + worktreeId: 'wt-1', + activeTabId: active.id, + tabOrder: tabs.filter((t) => t.groupId === groupId).map((t) => t.id) + }) + } + return { groups, tabs } +} + +function pick(layout: TabGroupLayoutNode, sourceGroupId: string, fixture?: Fixture) { + const { groups, tabs } = buildState(layout, fixture) + return pickAdjacentEditorOpenGroupId({ + layout, + groups, + tabs, + sourceGroupId, + fileId: FILE_ID, + contentType: 'editor' + }) +} + +describe('pickAdjacentEditorOpenGroupId', () => { + it('opens across a two-column split in both directions', () => { + const layout = columns(leaf('A'), leaf('B')) + expect(pick(layout, 'A')).toBe('B') + expect(pick(layout, 'B')).toBe('A') + }) + + it('opens across a two-row split in both directions', () => { + const layout = rows(leaf('A'), leaf('B')) + expect(pick(layout, 'A')).toBe('B') + expect(pick(layout, 'B')).toBe('A') + }) + + it('prefers the right neighbor over the left in three columns', () => { + expect(pick(columns(leaf('A'), columns(leaf('B'), leaf('C'))), 'B')).toBe('C') + }) + + it('ignores the diagonal group in a four-way grid', () => { + const grid = columns(rows(leaf('TL'), leaf('BL')), rows(leaf('TR'), leaf('BR'))) + expect(pick(grid, 'BR')).toBe('BL') + // Why: corner-only contact is not adjacency, so TL's editor earns no preference. + expect(pick(grid, 'BR', { editors: ['TL'] })).toBe('BL') + }) + + it('picks the upper-right group beside a tall source regardless of their height ratio', () => { + for (const ratio of [0.2, 0.5, 0.8]) { + expect(pick(columns(leaf('A'), rows(leaf('B'), leaf('C'), ratio)), 'A')).toBe('B') + } + }) + + it('lets a neighbor showing an editor beat side order', () => { + expect(pick(columns(leaf('A'), rows(leaf('B'), leaf('C'))), 'A', { editors: ['C'] })).toBe('C') + }) + + it('orders neighbors below a wide source left to right', () => { + expect(pick(rows(leaf('A'), columns(leaf('L'), leaf('R'))), 'A')).toBe('L') + }) + + it('compares equal-priority editors right, left, below, above', () => { + const editors = ['L', 'R', 'T', 'B'] + const cross = rows( + leaf('T'), + rows(columns(leaf('L'), columns(leaf('S'), leaf('R'))), leaf('B')) + ) + expect(pick(cross, 'S', { editors })).toBe('R') + expect(pick(cross, 'S', { editors: ['L', 'T', 'B'] })).toBe('L') + expect(pick(cross, 'S', { editors: ['T', 'B'] })).toBe('B') + expect(pick(cross, 'S', { editors: ['T'] })).toBe('T') + }) + + it('reuses a nonadjacent visible group that already holds the file', () => { + expect(pick(columns(leaf('A'), columns(leaf('B'), leaf('C'))), 'A', { withFile: ['C'] })).toBe( + 'C' + ) + }) + + it('prefers a neighboring view when the file has several', () => { + const layout = columns(leaf('A'), columns(leaf('B'), columns(leaf('C'), leaf('D')))) + expect(pick(layout, 'A', { withFile: ['B', 'D'] })).toBe('B') + expect(pick(layout, 'A', { withFile: ['C', 'D'] })).toBe('C') + }) + + it('opens a second view beside the source when only the source has the file', () => { + expect(pick(columns(leaf('A'), leaf('B')), 'A', { withFile: ['A'] })).toBe('B') + }) + + it('keeps a single group in place', () => { + expect(pick(leaf('A'), 'A')).toBe('A') + }) + + it('covers an adjacent agent before a nonadjacent editor without the file', () => { + expect(pick(columns(leaf('A'), columns(leaf('B'), leaf('C'))), 'A', { editors: ['C'] })).toBe( + 'B' + ) + }) + + it('selects an old unselected matching tab over a nearby editor', () => { + expect( + pick(columns(leaf('A'), columns(leaf('B'), leaf('C'))), 'A', { + editors: ['B'], + withFile: ['C'] + }) + ).toBe('C') + }) + + it('does not treat an inactive editor tab as showing an editor', () => { + const layout = columns(leaf('L'), columns(leaf('S'), leaf('R'))) + expect(pick(layout, 'S', { editors: ['L'], inactiveEditors: ['R'] })).toBe('L') + }) + + it('does not match a diff of the same file as a reusable view', () => { + const layout = columns(leaf('A'), columns(leaf('B'), leaf('C'))) + const { groups, tabs } = buildState(layout) + tabs.push(tab('C-diff', 'C', 'diff', FILE_ID)) + expect( + pickAdjacentEditorOpenGroupId({ + layout, + groups, + tabs, + sourceGroupId: 'A', + fileId: FILE_ID, + contentType: 'editor' + }) + ).toBe('B') + }) + + it('is invariant to candidate order', () => { + const layout = columns(leaf('A'), rows(leaf('B'), leaf('C'))) + const { groups, tabs } = buildState(layout, { editors: ['B', 'C'] }) + const input = { layout, sourceGroupId: 'A', fileId: FILE_ID, contentType: 'editor' as const } + const forward = pickAdjacentEditorOpenGroupId({ ...input, groups, tabs }) + const reversed = pickAdjacentEditorOpenGroupId({ + ...input, + groups: groups.toReversed(), + tabs: tabs.toReversed() + }) + expect(forward).toBe('B') + expect(reversed).toBe(forward) + }) + + it('agrees across split histories that draw the same grid', () => { + const byColumns = columns(rows(leaf('TL'), leaf('BL')), rows(leaf('TR'), leaf('BR'))) + const byRows = rows(columns(leaf('TL'), leaf('TR')), columns(leaf('BL'), leaf('BR'))) + for (const source of ['TL', 'TR', 'BL', 'BR']) { + expect(pick(byRows, source)).toBe(pick(byColumns, source)) + } + }) + + it('returns null when the source is not a live leaf', () => { + expect(pick(columns(leaf('A'), leaf('B')), 'gone')).toBeNull() + }) +}) + +describe('findNeighbor', () => { + it('requires positive overlap along the shared edge', () => { + const source = { left: 0, top: 0, right: 0.5, bottom: 0.5 } + expect(findNeighbor(source, { left: 0.5, top: 0.5, right: 1, bottom: 1 })).toBeNull() + expect(findNeighbor(source, { left: 0.5, top: 0.25, right: 1, bottom: 1 })).toEqual({ + side: 'right', + position: 0.25 + }) + }) +}) diff --git a/src/renderer/src/store/slices/editor/tabs/editor-open-adjacent-group.ts b/src/renderer/src/store/slices/editor/tabs/editor-open-adjacent-group.ts new file mode 100644 index 00000000000..c468e0705ed --- /dev/null +++ b/src/renderer/src/store/slices/editor/tabs/editor-open-adjacent-group.ts @@ -0,0 +1,150 @@ +import type { Tab, TabGroup, TabGroupLayoutNode } from '../../../../../../shared/tab-types' +import { isEditorTabContentType, type EditorTabContentType } from './editor-tab-content-type' + +type LayoutRect = { left: number; top: number; right: number; bottom: number } +type LayoutLeaf = { groupId: string; rect: LayoutRect; layoutOrder: number } +type NeighborSide = 'right' | 'left' | 'below' | 'above' +type Neighbor = { side: NeighborSide; position: number } + +type Candidate = { + id: string + neighbor: Neighbor | null + hasMatchingFile: boolean + showsEditor: boolean + layoutOrder: number +} +type Rank = readonly [number, number, number, number] + +const EPSILON = 1e-9 +const SIDE_ORDER = { right: 0, left: 1, below: 2, above: 3 } as const + +function clampRatio(ratio: number | undefined): number { + return typeof ratio === 'number' && Number.isFinite(ratio) && ratio > 0 && ratio < 1 ? ratio : 0.5 +} + +/** Leaves in stable tree order, each projected into the unit square by its split ratios. */ +function projectLayoutLeaves(layout: TabGroupLayoutNode): LayoutLeaf[] { + const leaves: LayoutLeaf[] = [] + const visit = (node: TabGroupLayoutNode, rect: LayoutRect): void => { + if (node.type === 'leaf') { + if (rect.right - rect.left > EPSILON && rect.bottom - rect.top > EPSILON) { + leaves.push({ groupId: node.groupId, rect, layoutOrder: leaves.length }) + } + return + } + const ratio = clampRatio(node.ratio) + if (node.direction === 'horizontal') { + const split = rect.left + (rect.right - rect.left) * ratio + visit(node.first, { ...rect, right: split }) + visit(node.second, { ...rect, left: split }) + } else { + const split = rect.top + (rect.bottom - rect.top) * ratio + visit(node.first, { ...rect, bottom: split }) + visit(node.second, { ...rect, top: split }) + } + } + visit(layout, { left: 0, top: 0, right: 1, bottom: 1 }) + return leaves +} + +function overlaps(a0: number, a1: number, b0: number, b1: number): boolean { + return Math.min(a1, b1) - Math.max(a0, b0) > EPSILON +} + +function touches(a: number, b: number): boolean { + return Math.abs(a - b) <= EPSILON +} + +/** Shared edge with positive overlap; corner-only contact is not adjacency. */ +export function findNeighbor(source: LayoutRect, target: LayoutRect): Neighbor | null { + const verticalOverlap = overlaps(source.top, source.bottom, target.top, target.bottom) + const horizontalOverlap = overlaps(source.left, source.right, target.left, target.right) + if (verticalOverlap && touches(source.right, target.left)) { + return { side: 'right', position: target.top } + } + if (verticalOverlap && touches(source.left, target.right)) { + return { side: 'left', position: target.top } + } + if (horizontalOverlap && touches(source.bottom, target.top)) { + return { side: 'below', position: target.left } + } + if (horizontalOverlap && touches(source.top, target.bottom)) { + return { side: 'above', position: target.left } + } + return null +} + +function rankCandidate(candidate: Candidate, sourceGroupId: string): Rank | null { + const { neighbor, hasMatchingFile, showsEditor, layoutOrder } = candidate + if (candidate.id === sourceGroupId) { + return [4, 0, 0, layoutOrder] + } + if (!neighbor) { + return hasMatchingFile ? [1, 0, 0, layoutOrder] : null + } + const priority = hasMatchingFile ? 0 : showsEditor ? 2 : 3 + return [priority, SIDE_ORDER[neighbor.side], neighbor.position, layoutOrder] +} + +function compareRanks(a: Rank, b: Rank): number { + return a[0] - b[0] || a[1] - b[1] || a[2] - b[2] || a[3] - b[3] +} + +type AdjacentEditorOpenInput = { + layout: TabGroupLayoutNode + groups: readonly TabGroup[] + tabs: readonly Tab[] + sourceGroupId: string + /** File identity already resolved by the open path; never re-derived from paths here. */ + fileId: string + contentType: EditorTabContentType +} + +/** + * Minimum-rank destination for a file link: neighbor with the file, any group with the + * file, neighbor showing an editor, any neighbor, then the source. Null when the source + * is not a live leaf of this layout. + */ +export function pickAdjacentEditorOpenGroupId(input: AdjacentEditorOpenInput): string | null { + const leaves = projectLayoutLeaves(input.layout) + const source = leaves.find((leaf) => leaf.groupId === input.sourceGroupId) + const liveGroups = new Map(input.groups.map((group) => [group.id, group])) + if (!source || !liveGroups.has(source.groupId)) { + return null + } + const groupsWithFile = new Set() + const tabsById = new Map() + for (const tab of input.tabs) { + tabsById.set(tab.id, tab) + if (tab.entityId === input.fileId && tab.contentType === input.contentType) { + groupsWithFile.add(tab.groupId) + } + } + // Why: the source keeps its conversation visible, so a source-only copy earns no preference. + groupsWithFile.delete(source.groupId) + + let bestId: string | null = null + let bestRank: Rank | null = null + for (const leaf of leaves) { + const group = liveGroups.get(leaf.groupId) + if (!group) { + continue + } + const activeTab = group.activeTabId ? tabsById.get(group.activeTabId) : undefined + const rank = rankCandidate( + { + id: leaf.groupId, + neighbor: leaf.groupId === source.groupId ? null : findNeighbor(source.rect, leaf.rect), + hasMatchingFile: groupsWithFile.has(leaf.groupId), + showsEditor: activeTab ? isEditorTabContentType(activeTab.contentType) : false, + layoutOrder: leaf.layoutOrder + }, + source.groupId + ) + if (rank && (!bestRank || compareRanks(rank, bestRank) < 0)) { + bestRank = rank + bestId = leaf.groupId + } + } + return bestId +} diff --git a/src/renderer/src/store/slices/editor/tabs/editor-open-target-group.ts b/src/renderer/src/store/slices/editor/tabs/editor-open-target-group.ts index c3ecf03edc6..7a1ff3b80fe 100644 --- a/src/renderer/src/store/slices/editor/tabs/editor-open-target-group.ts +++ b/src/renderer/src/store/slices/editor/tabs/editor-open-target-group.ts @@ -1,7 +1,11 @@ import type { AppState } from '../../../types' import type { Tab, TabGroup, WorkspaceVisibleTabType } from '../../../../../../shared/tab-types' -import { isEditorTabContentType } from './editor-tab-content-type' +import { editorTabContentTypeForMode, isEditorTabContentType } from './editor-tab-content-type' +import type { OpenFile } from '../types/open-file' import { ownsGlobalSelection } from '../../../global-selection-owner' +import type { EditorOpenLinkIntent } from '../types/editor-open-link-intent' +import { pickAdjacentEditorOpenGroupId } from './editor-open-adjacent-group' +import { collectTabGroupLayoutGroupIds } from '../../../../../../shared/workspace-layout/tab-group-moves' export function getGroupActiveTab(group: TabGroup, tabsById: Map): Tab | null { return group.activeTabId ? (tabsById.get(group.activeTabId) ?? null) : null @@ -79,6 +83,118 @@ export function resolveEditorOpenTargetGroupId( return recentEditorGroup?.id ?? activeGroup.id } +type LinkPlacementState = Pick< + AppState, + 'groupsByWorktree' | 'unifiedTabsByWorktree' | 'layoutByWorktree' +> + +/** The unified tab a link came from; a terminal surface may know only its backing terminal id. */ +function findLinkSourceTab(tabs: readonly Tab[], sourceTabId: string): Tab | undefined { + return ( + tabs.find((tab) => tab.id === sourceTabId) ?? + tabs.find((tab) => tab.contentType === 'terminal' && tab.entityId === sourceTabId) + ) +} + +/** Captures a link activation's source at click time, so later focus changes cannot redirect it. */ +export function captureEditorOpenLinkIntent( + state: Pick, + workspaceId: string, + sourceTabId: string | null | undefined, + options?: { explicitSourceGroup?: boolean } +): EditorOpenLinkIntent | null { + if (!sourceTabId) { + return null + } + const tab = findLinkSourceTab(state.unifiedTabsByWorktree?.[workspaceId] ?? [], sourceTabId) + if (!tab) { + return null + } + return { + sourceTabId: tab.id, + workspaceId, + hostId: tab.executionHostId ?? null, + ...(options?.explicitSourceGroup ? { targetGroupId: tab.groupId } : {}) + } +} + +function liveLayoutGroupIds(state: LinkPlacementState, worktreeId: string): Set { + const layout = state.layoutByWorktree?.[worktreeId] + const groupIds = new Set((state.groupsByWorktree?.[worktreeId] ?? []).map((group) => group.id)) + if (!layout) { + return groupIds + } + return new Set([...collectTabGroupLayoutGroupIds(layout)].filter((id) => groupIds.has(id))) +} + +export function isLiveEditorOpenGroup( + state: LinkPlacementState, + worktreeId: string, + groupId: string +): boolean { + return liveLayoutGroupIds(state, worktreeId).has(groupId) +} + +/** "Open in this group" only differs from automatic placement when the source has company. */ +export function canOpenLinkInSourceGroup( + state: LinkPlacementState, + workspaceId: string, + sourceTabId: string | null | undefined +): boolean { + const tab = sourceTabId + ? findLinkSourceTab(state.unifiedTabsByWorktree?.[workspaceId] ?? [], sourceTabId) + : undefined + if (!tab) { + return false + } + const groupIds = liveLayoutGroupIds(state, workspaceId) + return groupIds.has(tab.groupId) && groupIds.size > 1 +} + +export type EditorLinkOpenPlacement = + | { kind: 'group'; groupId: string } + | { kind: 'default' } + | { kind: 'refused' } + +/** + * Link-intent extension of resolveEditorOpenTargetGroupId: an explicit target is honored + * or refused as captured; an automatic open ranks groups around the source's current group. + */ +export function resolveEditorLinkOpenPlacement( + state: LinkPlacementState, + worktreeId: string, + intent: EditorOpenLinkIntent, + fileId: string, + mode: OpenFile['mode'] +): EditorLinkOpenPlacement { + if (intent.targetGroupId) { + return intent.workspaceId === worktreeId && + isLiveEditorOpenGroup(state, worktreeId, intent.targetGroupId) + ? { kind: 'group', groupId: intent.targetGroupId } + : { kind: 'refused' } + } + // Why: groups in another workspace's layout have no spatial relationship to the source. + if (intent.workspaceId !== worktreeId) { + return { kind: 'default' } + } + const layout = state.layoutByWorktree?.[worktreeId] + const tabs = state.unifiedTabsByWorktree?.[worktreeId] ?? [] + const source = tabs.find((tab) => tab.id === intent.sourceTabId) + // Why: a closed or re-hosted source falls back to defaults, never to its stale initial group. + if (!layout || !source || (source.executionHostId ?? null) !== intent.hostId) { + return { kind: 'default' } + } + const groupId = pickAdjacentEditorOpenGroupId({ + layout, + groups: state.groupsByWorktree?.[worktreeId] ?? [], + tabs, + sourceGroupId: source.groupId, + fileId, + contentType: editorTabContentTypeForMode(mode) + }) + return groupId ? { kind: 'group', groupId } : { kind: 'default' } +} + export function buildEditorActiveResult( state: Pick, worktreeId: string, diff --git a/src/renderer/src/store/slices/editor/tabs/editor-tab-content-type.ts b/src/renderer/src/store/slices/editor/tabs/editor-tab-content-type.ts index 3f4a3c065f6..1427ad1de2c 100644 --- a/src/renderer/src/store/slices/editor/tabs/editor-tab-content-type.ts +++ b/src/renderer/src/store/slices/editor/tabs/editor-tab-content-type.ts @@ -21,3 +21,7 @@ export function isEditorTabContentType( export function isVirtualEditorFile(file: Pick): boolean { return file.mode === 'check-details' } + +export function editorTabContentTypeForMode(mode: OpenFile['mode']): EditorTabContentType { + return mode === 'conflict-review' || mode === 'check-details' || mode === 'diff' ? mode : 'editor' +} diff --git a/src/renderer/src/store/slices/editor/types/editor-files-slice.ts b/src/renderer/src/store/slices/editor/types/editor-files-slice.ts index be6893b7a64..34297d11b40 100644 --- a/src/renderer/src/store/slices/editor/types/editor-files-slice.ts +++ b/src/renderer/src/store/slices/editor/types/editor-files-slice.ts @@ -21,12 +21,24 @@ import type { EditorTabSelection, OpenFile } from './open-file' +import type { EditorOpenLinkIntent, EditorOpenResult } from './editor-open-link-intent' import type { OpenFilePathRekey, RekeyOpenFilesResult } from './open-file-path-rekey' import type { RestoredEditorOwnerMigration, RestoredEditorOwnerResult } from './restored-editor-owner' +export type OpenFileOptions = { + preview?: boolean + targetGroupId?: string + recordReplacedPreview?: boolean + suppressActiveRuntimeFallback?: boolean + forceContentReload?: boolean + focusEditor?: boolean + reopenId?: string + selection?: EditorTabSelection +} + export type EditorFilesSlice = { // Open files / editor tabs openFiles: OpenFile[] @@ -35,19 +47,13 @@ export type EditorFilesSlice = { activeTabTypeByWorktree: Record // worktreeId -> last active tab type activeTabType: WorkspaceVisibleTabType setActiveTabType: (type: WorkspaceVisibleTabType, worktreeId: string | null) => void - openFile: ( + openFile: (file: Omit, options?: OpenFileOptions) => string + /** openFile for a captured link activation; returns the actual view, or null when refused. */ + openFileForLink: ( file: Omit, - options?: { - preview?: boolean - targetGroupId?: string - recordReplacedPreview?: boolean - suppressActiveRuntimeFallback?: boolean - forceContentReload?: boolean - focusEditor?: boolean - reopenId?: string - selection?: EditorTabSelection - } - ) => string + options: Omit | undefined, + intent: EditorOpenLinkIntent | null + ) => EditorOpenResult | null openNewMarkdownInActiveWorkspace: (groupId: string) => Promise // Why: sequences openFile/setMarkdownViewMode/reveal around an async Monaco remount. See docs/markdown-internal-link-opening-design.md. activateMarkdownLink: ( diff --git a/src/renderer/src/store/slices/editor/types/editor-open-link-intent.ts b/src/renderer/src/store/slices/editor/types/editor-open-link-intent.ts new file mode 100644 index 00000000000..5344e74445e --- /dev/null +++ b/src/renderer/src/store/slices/editor/types/editor-open-link-intent.ts @@ -0,0 +1,18 @@ +import type { ExecutionHostId } from '../../../../../../shared/execution-host' + +/** A link's source, captured before async path resolution. */ +export type EditorOpenLinkIntent = { + workspaceId: string + sourceTabId: string + /** The source tab's owning host at capture; null when the tab carries none. */ + hostId: ExecutionHostId | null + /** Set only for "Open in this group"; otherwise follow the source tab's current group. */ + targetGroupId?: string +} + +/** The view an open actually created or reused; the file id alone names only the document. */ +export type EditorOpenResult = { + fileId: string + viewStateId: string + groupId: string +} diff --git a/src/renderer/src/store/slices/tabs/tabs-reconciliation-empty-groups.test.ts b/src/renderer/src/store/slices/tabs/tabs-reconciliation-empty-groups.test.ts new file mode 100644 index 00000000000..2493c7a98f4 --- /dev/null +++ b/src/renderer/src/store/slices/tabs/tabs-reconciliation-empty-groups.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it, vi } from 'vitest' +import { createTestStore, makeTab } from '../store-test-helpers' +import { createTabsSliceMockApi } from '../tabs-slice-test-harness' +import { captureEditorOpenLinkIntent } from '../editor/tabs/editor-open-target-group' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) + +createTabsSliceMockApi() + +function splitWorkspace(workspaceId: string, source: 'terminal' | 'agent-session' = 'terminal') { + const store = createTestStore() + store.getState().createUnifiedTab(workspaceId, source, { id: 'source', entityId: 'source' }) + if (source === 'terminal') { + store.setState({ + tabsByWorktree: { + [workspaceId]: [makeTab({ id: 'source', worktreeId: workspaceId, ptyId: 'pty-1' })] + }, + ptyIdsByTabId: { source: ['pty-1'] } + }) + } + const sourceGroupId = store.getState().groupsByWorktree[workspaceId][0].id + const emptyGroupId = store.getState().createEmptySplitGroup(workspaceId, sourceGroupId, 'right') + if (!emptyGroupId) { + throw new Error('Expected an empty split group') + } + return { store, sourceGroupId, emptyGroupId } +} + +describe.each(['repo1::/tmp/feature', 'folder:folder-1'])('empty groups in %s', (workspaceId) => { + it.each(['terminal', 'agent-session'] as const)( + 'preserves an empty neighbor through reconciliation before opening a %s link', + (source) => { + const { store, sourceGroupId, emptyGroupId } = splitWorkspace(workspaceId, source) + const before = store.getState() + const intent = captureEditorOpenLinkIntent(before, workspaceId, 'source') + expect(intent).not.toBeNull() + expect(before.activeGroupIdByWorktree[workspaceId]).toBe(emptyGroupId) + + for (let pass = 0; pass < 2; pass++) { + expect(store.getState().reconcileWorktreeTabModel(workspaceId).renderableTabCount).toBe(1) + const state = store.getState() + expect(state.groupsByWorktree[workspaceId]).toBe(before.groupsByWorktree[workspaceId]) + expect(state.layoutByWorktree[workspaceId]).toBe(before.layoutByWorktree[workspaceId]) + expect(state.activeGroupIdByWorktree[workspaceId]).toBe(emptyGroupId) + } + + const opened = store.getState().openFileForLink( + { + filePath: '/tmp/feature/link.ts', + relativePath: 'link.ts', + worktreeId: workspaceId, + language: 'typescript', + mode: 'edit' + }, + {}, + intent + ) + + expect(opened?.groupId).toBe(emptyGroupId) + const state = store.getState() + expect(state.unifiedTabsByWorktree[workspaceId]).toContainEqual( + expect.objectContaining({ id: opened?.viewStateId, groupId: emptyGroupId }) + ) + expect( + state.groupsByWorktree[workspaceId].find((group) => group.id === sourceGroupId) + ).toMatchObject({ activeTabId: 'source', tabOrder: ['source'] }) + } + ) + + it('removes a group emptied by stale-tab cleanup while preserving an already empty group', () => { + const { store, sourceGroupId, emptyGroupId } = splitWorkspace(workspaceId) + const layoutWithEmptyNeighbor = store.getState().layoutByWorktree[workspaceId] + const staleGroupId = store.getState().createEmptySplitGroup(workspaceId, emptyGroupId, 'right') + if (!staleGroupId) { + throw new Error('Expected a stale split group') + } + store.getState().createUnifiedTab(workspaceId, 'browser', { + id: 'stale-browser', + entityId: 'missing-browser', + targetGroupId: staleGroupId + }) + + store.getState().reconcileWorktreeTabModel(workspaceId) + + const state = store.getState() + expect(state.groupsByWorktree[workspaceId].map((group) => group.id)).toEqual([ + sourceGroupId, + emptyGroupId + ]) + expect(state.layoutByWorktree[workspaceId]).toEqual(layoutWithEmptyNeighbor) + expect(state.activeGroupIdByWorktree[workspaceId]).toBe(sourceGroupId) + expect(state.unifiedTabsByWorktree[workspaceId].map((tab) => tab.id)).toEqual(['source']) + }) +}) diff --git a/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts b/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts index 561a1483705..22466065de7 100644 --- a/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts +++ b/src/renderer/src/store/slices/tabs/tabs-reconciliation.ts @@ -157,7 +157,11 @@ export function projectWorktreeTabModelReconciliation( }) const prunedGroups = validTabs.length > 0 - ? nextGroupsWithEmpty.filter((group) => group.tabOrder.length > 0) + ? nextGroupsWithEmpty.filter( + // Preserve intentional empty panes; only collapse groups emptied by stale-tab cleanup. + (group, index) => + group.tabOrder.length > 0 || reconciledGroups[index].tabOrder.length === 0 + ) : nextGroupsWithEmpty const groupsChanged = prunedGroups.length !== groups.length ||