From e61d062ef19aea3eb2903de79db924cb61407d7d Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:45:36 -0700 Subject: [PATCH] Revert HTML preview by default for the release (#31575) (#31893) * Revert "Preview HTML files by default" (#31575) Restore source/diff opening for general HTML file-opening routes for the release. This reverts commit 524fa0039b48253f102f7cbac2bfc8c79f5722ac, retaining the published files.readDocPreview capability for mixed-version clients and removing an unnecessary restored test assertion to satisfy the current lint gate. Existing explicit previews and terminal HTML links retain their previous behavior; the underlying local-file preview exposure remains tracked in STA-11670. * Preserve file ownership and symlink guards during HTML preview rollback Keep the existing owner checks after file-drop uploads and tab-entry stats so an ownership change cannot reopen a file through a stale host. Preserve local preview source checks so symlinks outside the workspace keep user-file access. The race and symlink tests reproduce failures on the mechanical revert before these guards are retained. Cover HTML source opening for folder workspaces, Explorer, and Source Control without restoring preview-by-default. * fix: split tab entry browser requests to satisfy lint limit --- .../QuickOpen.mount-gating.test.tsx | 2 - .../assemble-chrome/browser-page-toolbar.tsx | 25 +- .../host-guest/browser-page-webview.ts | 4 - .../navigate/browser-file-editor-open.test.ts | 31 +- .../local-html-preview-freshness.test.ts | 190 ---------- .../navigate/local-html-preview-freshness.ts | 85 ----- .../doc-preview-document-actions.test.ts | 10 +- .../doc-preview-document-actions.ts | 27 +- ...or-dropped-file-open.escaping-link.test.ts | 6 +- .../editor/editor-dropped-file-open.ts | 53 +-- ...itor-group-file-drop-owner.remote.test.tsx | 113 ++---- .../use-editor-group-file-drop-owner.test.tsx | 4 + .../quick-open-file-navigation.test.ts | 66 +--- .../components/quick-open-file-navigation.ts | 38 +- .../SourceControl.preview-open.test.tsx | 355 +----------------- .../file-explorer-drag-scroll-marker.test.tsx | 2 +- .../file-explorer-instant-dir-toggle.test.ts | 12 +- ...ce-control-branch-section-heading.test.tsx | 2 +- .../listing/active-open-file-keys.ts | 47 --- .../listing/branch-entry-row.tsx | 2 - .../source-control/listing/branch-section.tsx | 8 +- .../listing/open-uncommitted-entry.test.ts | 2 +- .../listing/open-uncommitted-entry.ts} | 2 +- .../listing/section-action-buttons.test.tsx | 2 +- .../listing/section-file-list.tsx | 8 +- .../listing/uncommitted-sections.tsx | 4 +- .../listing/use-file-listing.ts | 8 +- .../source-control/listing/use-row-opening.ts | 76 ++-- .../source-control/listing/use-selection.ts | 16 +- .../source-control/notes/use-note-opening.ts | 26 +- .../source-control/panel/panel-content.tsx | 8 +- .../source-control/panel/use-panel-model.ts | 4 + .../use-file-explorer-tree-pane-state.ts | 4 +- .../useFileExplorerHandlers.test.ts | 128 +++---- .../right-sidebar/useFileExplorerHandlers.ts | 39 +- .../workspace-space-open-change.test.ts | 19 +- .../status-bar/workspace-space-open-change.ts | 15 +- .../tab-bar/tab-create-entry-absolute-file.ts | 16 +- .../tab-bar/tab-create-entry-action.test.ts | 142 ++----- .../tab-bar/tab-create-entry-action.ts | 143 +++---- .../tab-create-entry-browser-request.ts | 34 ++ .../tab-create-entry-local-path.test.ts | 81 ++-- .../tab-bar/tab-create-entry-preview.test.ts | 3 +- .../terminal-file-link-editor-open.ts} | 2 +- .../terminal-file-open-routing.ts | 77 +++- .../terminal-link-file-open-routing.test.ts | 21 +- .../terminal-link-handlers-test-fixtures.ts | 21 +- .../terminal-link-handlers-test-harness.ts | 9 +- .../terminal-pane/terminal-link-handlers.ts | 6 +- .../terminal-link-open-beside-source.test.ts | 19 +- ...minal-link-remote-runtime-ssh-open.test.ts | 9 +- src/renderer/src/lib/file-preview-tab.ts | 69 ---- src/renderer/src/lib/file-preview.test.ts | 124 +----- src/renderer/src/lib/file-preview.ts | 124 +++--- .../src/lib/open-html-file-preview.test.ts | 53 +++ .../src/lib/open-html-file-preview.ts | 29 ++ .../src/lib/workspace-file-open.test.ts | 350 ----------------- src/renderer/src/lib/workspace-file-open.ts | 233 ------------ .../src/lib/workspace-file-preview-page.ts | 28 -- src/shared/html-file-url.ts | 6 +- tests/e2e/html-file-url-opening.spec.ts | 146 ++----- tests/e2e/html-source-control-preview.spec.ts | 109 ------ 62 files changed, 841 insertions(+), 2456 deletions(-) delete mode 100644 src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.test.ts delete mode 100644 src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.ts rename src/renderer/src/{lib/workspace-file-changes-open.ts => components/right-sidebar/source-control/listing/open-uncommitted-entry.ts} (93%) create mode 100644 src/renderer/src/components/tab-bar/tab-create-entry-browser-request.ts rename src/renderer/src/{lib/editor-file-link-open.ts => components/terminal-pane/terminal-file-link-editor-open.ts} (97%) delete mode 100644 src/renderer/src/lib/file-preview-tab.ts create mode 100644 src/renderer/src/lib/open-html-file-preview.test.ts create mode 100644 src/renderer/src/lib/open-html-file-preview.ts delete mode 100644 src/renderer/src/lib/workspace-file-open.test.ts delete mode 100644 src/renderer/src/lib/workspace-file-open.ts delete mode 100644 src/renderer/src/lib/workspace-file-preview-page.ts delete mode 100644 tests/e2e/html-source-control-preview.spec.ts diff --git a/src/renderer/src/components/QuickOpen.mount-gating.test.tsx b/src/renderer/src/components/QuickOpen.mount-gating.test.tsx index 8622cedf6a0..2da4f0a97e8 100644 --- a/src/renderer/src/components/QuickOpen.mount-gating.test.tsx +++ b/src/renderer/src/components/QuickOpen.mount-gating.test.tsx @@ -37,8 +37,6 @@ vi.mock('@/hooks/useModalReturnFocus', () => ({ useModalReturnFocus: () => ({ captureReturnFocus: vi.fn(), skipReturnFocus: vi.fn() }) })) -vi.mock('./quick-open-file-navigation', () => ({ openQuickOpenFile: vi.fn() })) - vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-toolbar.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-toolbar.tsx index b12aff9eb14..85c527aae65 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-toolbar.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-toolbar.tsx @@ -224,24 +224,13 @@ export function BrowserPageToolbar({ } : undefined } - viewSource={ - openSourceFile - ? { - onSelect: openSourceFile, - label: translate( - 'auto.components.editor.HtmlDocPreview.openSourceControl', - 'Open source file' - ), - alreadyInOverflowMenu: true - } - : { - onSelect: () => void window.api.browser.openDevTools({ browserPageId }), - label: translate( - 'auto.components.browser.pane.BrowserPane.ec75d0c412', - 'Open browser devtools' - ) - } - } + viewSource={{ + onSelect: () => void window.api.browser.openDevTools({ browserPageId }), + label: translate( + 'auto.components.browser.pane.BrowserPane.ec75d0c412', + 'Open browser devtools' + ) + }} openExternal={{ onSelect: () => { if (!externalUrl) { diff --git a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview.ts b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview.ts index 401151166c2..fc345a8ee1b 100644 --- a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview.ts +++ b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview.ts @@ -6,7 +6,6 @@ import { replacePersistentWebview, webviewRegistry } from './webview-registry' -import { recordLocalHtmlPreviewNavigation } from '../navigate/local-html-preview-freshness' export function setBrowserPageWebviewInputLock( webview: Electron.WebviewTag, @@ -71,9 +70,6 @@ export function ensureBrowserPageWebview({ setBrowserPageWebviewInputLock(webview, inputLocked) webview.style.background = 'var(--background)' const guest = webview - guest.addEventListener('did-start-navigation', (event) => { - recordLocalHtmlPreviewNavigation(guest, event) - }) // A committed synthetic blank document belongs to New Tab, including while its first URL waits. guest.addEventListener('load-commit', (event) => { if (event.isMainFrame) { diff --git a/src/renderer/src/components/browser-pane/navigate/browser-file-editor-open.test.ts b/src/renderer/src/components/browser-pane/navigate/browser-file-editor-open.test.ts index c499035fcc2..0545972c2e3 100644 --- a/src/renderer/src/components/browser-pane/navigate/browser-file-editor-open.test.ts +++ b/src/renderer/src/components/browser-pane/navigate/browser-file-editor-open.test.ts @@ -1,4 +1,4 @@ -import { expect, it, vi } from 'vitest' +import { afterEach, expect, it, vi } from 'vitest' import type * as ConnectionOwnerModule from '@/lib/connection-owner-resolution' // A workspace this computer owns. @@ -13,6 +13,8 @@ vi.mock('@/lib/user-opened-local-path', () => ({ import { useAppStore } from '@/store' import { editorTabFileAccess } from '@/lib/local-file-access' +import { statUserOpenedPath } from '@/lib/user-opened-local-path' +import { makeWorktree } from '@/store/slices/worktrees-slice-test-fixtures' import { getShareableBrowserArtifactFile } from '../describe-page/browser-artifact-upload' import { canOpenBrowserFileInWorkspaceEditor, @@ -20,6 +22,12 @@ import { } from './browser-file-editor-open' const PATH = '/Users/me/orca/chat-scratch/0123456789abcdef0123456789abcdef/usage.html' +const initialState = useAppStore.getInitialState() + +afterEach(() => { + useAppStore.setState(initialState, true) + vi.clearAllMocks() +}) it("opens a local chat visual's page as an editable source tab, read and saved in place", async () => { const openFile = vi.fn() @@ -33,3 +41,24 @@ it("opens a local chat visual's page as an editable source tab, read and saved i // What its editor reads and saves with: the user-named file itself, outside any project. expect(editorTabFileAccess(useAppStore.getState(), opened)).toEqual({ kind: 'user-file' }) }) + +it('keeps user-file access when a previewed symlink escapes the workspace', async () => { + const filePath = '/repo/docs-link/report.html' + const openFile = vi.fn() + useAppStore.setState({ + openFile, + worktreesByRepo: { + repo: [makeWorktree({ id: 'wt-1', repoId: 'repo', path: '/repo', hostId: 'local' })] + } + }) + vi.mocked(statUserOpenedPath).mockResolvedValueOnce({ + isDirectory: false, + escapesWorktree: true + }) + + await openBrowserFileSource('wt-1', filePath) + + const opened = openFile.mock.calls[0][0] + expect(opened).toMatchObject({ filePath, relativePath: filePath, mode: 'edit' }) + expect(editorTabFileAccess(useAppStore.getState(), opened)).toEqual({ kind: 'user-file' }) +}) diff --git a/src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.test.ts b/src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.test.ts deleted file mode 100644 index 7916ab4306b..00000000000 --- a/src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.test.ts +++ /dev/null @@ -1,190 +0,0 @@ -// @vitest-environment happy-dom - -import { afterEach, beforeEach, expect, it, vi } from 'vitest' -import { - recordLocalHtmlPreviewNavigation, - refreshLocalHtmlPreview -} from './local-html-preview-freshness' - -const stat = vi.fn() - -afterEach(() => vi.unstubAllGlobals()) - -beforeEach(() => { - stat.mockReset() - stat.mockResolvedValue({ size: 100, mtime: 1, isDirectory: false }) - vi.stubGlobal('window', { api: { fs: { stat } } }) -}) - -function guest() { - const reloadIgnoringCache = vi.fn() - const webview = { - getWebContentsId: () => 1, - reloadIgnoringCache - } - return { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these tests use only the guest liveness and reload methods. - webview: webview as unknown as Electron.WebviewTag, - reloadIgnoringCache - } -} - -function navigate(webview: Electron.WebviewTag, url = 'file:///repo/report.html') { - recordLocalHtmlPreviewNavigation(webview, { isMainFrame: true, isInPlace: false, url }) -} - -it('preserves an unchanged page across repeated file opens and in-page navigation', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - recordLocalHtmlPreviewNavigation(webview, { - isMainFrame: true, - isInPlace: true, - url: 'file:///repo/report.html#section' - }) - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - await refreshLocalHtmlPreview(webview, '/repo/report.html') - - expect(reloadIgnoringCache).not.toHaveBeenCalled() -}) - -it.each([ - { size: 100, mtime: 2 }, - { size: 120, mtime: 1 } -])('reloads changed disk content once, including regeneration: %j', async (revision) => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - stat.mockResolvedValue({ ...revision, isDirectory: false }) - - await Promise.all([ - refreshLocalHtmlPreview(webview, '/repo/report.html'), - refreshLocalHtmlPreview(webview, '/repo/report.html') - ]) - await refreshLocalHtmlPreview(webview, '/repo/report.html') - - expect(reloadIgnoringCache).toHaveBeenCalledTimes(1) -}) - -it('records the new disk revision after a manual reload', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - stat.mockResolvedValue({ size: 100, mtime: 2, isDirectory: false }) - navigate(webview) - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - - expect(reloadIgnoringCache).not.toHaveBeenCalled() -}) - -it('does not reload another page after navigation during the disk check', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - let finishStat = (_value: { size: number; mtime: number; isDirectory: boolean }): void => { - throw new Error('Disk check did not start') - } - stat.mockImplementationOnce( - () => - new Promise((resolve) => { - finishStat = resolve - }) - ) - const refresh = refreshLocalHtmlPreview(webview, '/repo/report.html') - navigate(webview, 'https://example.com') - finishStat({ size: 100, mtime: 2, isDirectory: false }) - await refresh - - expect(reloadIgnoringCache).not.toHaveBeenCalled() -}) - -it('keeps the loaded revision after a failed stat so a later successful open can refresh', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - stat.mockRejectedValueOnce(new Error('Filesystem unavailable')) - await refreshLocalHtmlPreview(webview, '/repo/report.html') - expect(reloadIgnoringCache).not.toHaveBeenCalled() - - stat.mockResolvedValue({ size: 100, mtime: 2, isDirectory: false }) - await refreshLocalHtmlPreview(webview, '/repo/report.html') - expect(reloadIgnoringCache).toHaveBeenCalledOnce() -}) - -it('refreshes when the initial disk revision was unavailable but a later reopen can read it', async () => { - const { webview, reloadIgnoringCache } = guest() - stat.mockRejectedValueOnce(new Error('Filesystem unavailable')) - navigate(webview) - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - await refreshLocalHtmlPreview(webview, '/repo/report.html') - - expect(reloadIgnoringCache).toHaveBeenCalledOnce() -}) - -it('refreshes a directly linked file through explicit access and retains its revision across reloads', async () => { - const { webview, reloadIgnoringCache } = guest() - let mtime = 1 - stat.mockImplementation(async ({ access }: { access: { kind: string } }) => { - if (access.kind === 'document-resource') { - throw new Error('Symlink target outside the document folder') - } - return { size: 100, mtime, isDirectory: false } - }) - reloadIgnoringCache.mockImplementation(() => navigate(webview)) - navigate(webview) - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - await refreshLocalHtmlPreview(webview, '/repo/report.html') - expect(reloadIgnoringCache).toHaveBeenCalledTimes(1) - - mtime = 2 - await refreshLocalHtmlPreview(webview, '/repo/report.html') - await refreshLocalHtmlPreview(webview, '/repo/report.html') - expect(reloadIgnoringCache).toHaveBeenCalledTimes(2) -}) - -it('retains a changed revision for retry when the guest is temporarily missing', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - stat.mockResolvedValue({ size: 100, mtime: 2, isDirectory: false }) - vi.spyOn(webview, 'getWebContentsId').mockImplementationOnce(() => { - throw new Error('Guest missing') - }) - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - expect(reloadIgnoringCache).not.toHaveBeenCalled() - await refreshLocalHtmlPreview(webview, '/repo/report.html') - expect(reloadIgnoringCache).toHaveBeenCalledOnce() -}) - -it('retries a changed file after its guest was not ready to reload', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview) - stat.mockResolvedValue({ size: 100, mtime: 2, isDirectory: false }) - reloadIgnoringCache.mockImplementationOnce(() => { - throw new Error('Guest not ready') - }) - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - await refreshLocalHtmlPreview(webview, '/repo/report.html') - - expect(reloadIgnoringCache).toHaveBeenCalledTimes(2) -}) - -it('compares Windows file paths with encoded characters and fragments', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview, 'file:///C:/repo/report%20one.html#section') - stat.mockResolvedValue({ size: 100, mtime: 2, isDirectory: false }) - - await refreshLocalHtmlPreview(webview, 'C:\\repo\\report one.html') - - expect(reloadIgnoringCache).toHaveBeenCalledOnce() -}) - -it('does not stat ordinary web pages or reload a guest without a loaded-file revision', async () => { - const { webview, reloadIgnoringCache } = guest() - navigate(webview, 'https://example.com/report.html') - - await refreshLocalHtmlPreview(webview, '/repo/report.html') - - expect(stat).not.toHaveBeenCalled() - expect(reloadIgnoringCache).not.toHaveBeenCalled() -}) diff --git a/src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.ts b/src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.ts deleted file mode 100644 index 41e7f9c8421..00000000000 --- a/src/renderer/src/components/browser-pane/navigate/local-html-preview-freshness.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { documentResourceAccess, userNamedFileAccess } from '@/lib/local-file-access' -import { normalizeRuntimePathForComparison } from '../../../../../shared/cross-platform-path' -import { isHtmlFilePath } from '../../../../../shared/html-file-url' -import type { LocalFileAccess } from '../../../../../shared/local-file-access' -import { browserFileUrlToAbsolutePath } from '../describe-page/browser-artifact-upload' -import { reloadBrowserPageWebview } from './browser-reload-action' - -type FileRevision = { size: number; mtime: number } -type LoadedFile = { filePath: string; revision: Promise } - -const loadedFiles = new WeakMap() - -async function readFileRevision( - filePath: string, - access: LocalFileAccess = documentResourceAccess(filePath) -): Promise { - try { - const stat = await window.api.fs.stat({ - filePath, - access - }) - return stat.isDirectory ? null : { size: stat.size, mtime: stat.mtime } - } catch { - return null - } -} - -export function recordLocalHtmlPreviewNavigation( - webview: Electron.WebviewTag, - event: Pick -): void { - if (!event.isMainFrame || event.isInPlace) { - return - } - const filePath = browserFileUrlToAbsolutePath(event.url) - if (!filePath || !isHtmlFilePath(filePath)) { - loadedFiles.delete(webview) - return - } - const previous = loadedFiles.get(webview) - const previousRevision = - previous && - normalizeRuntimePathForComparison(previous.filePath) === - normalizeRuntimePathForComparison(filePath) - ? previous.revision - : null - loadedFiles.set(webview, { - filePath, - revision: readFileRevision(filePath).then((revision) => revision ?? previousRevision) - }) -} - -/** Explicit file opens refresh changed disk contents while unchanged pages keep their UI state. */ -export async function refreshLocalHtmlPreview( - webview: Electron.WebviewTag, - filePath: string -): Promise { - const loaded = loadedFiles.get(webview) - if ( - !loaded || - normalizeRuntimePathForComparison(loaded.filePath) !== - normalizeRuntimePathForComparison(filePath) - ) { - return - } - const [previous, current] = await Promise.all([ - loaded.revision, - readFileRevision(filePath, userNamedFileAccess()) - ]) - if ( - loadedFiles.get(webview) !== loaded || - !current || - (previous && previous.size === current.size && previous.mtime === current.mtime) - ) { - return - } - const refreshed = { filePath, revision: Promise.resolve(current) } - loadedFiles.set(webview, refreshed) - if ( - reloadBrowserPageWebview(webview, { ignoreCache: true }) !== 'reloaded' && - loadedFiles.get(webview) === refreshed - ) { - loadedFiles.set(webview, loaded) - } -} diff --git a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.test.ts b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.test.ts index a60f2dcc7de..c6abc1901b9 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.test.ts +++ b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.test.ts @@ -1,5 +1,4 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type * as I18nModule from '@/i18n/i18n' const mocks = vi.hoisted(() => ({ /** What the per-file resolver answers: null local, a string remote, undefined unresolved. */ @@ -18,8 +17,7 @@ const mocks = vi.hoisted(() => ({ })) vi.mock('sonner', () => ({ toast: { error: mocks.toastError } })) -vi.mock('@/i18n/i18n', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string, params?: Record) => fallback.replace('{{value0}}', params?.value0 ?? '') })) @@ -105,8 +103,7 @@ describe('openDocPreviewSource', () => { worktreeId: 'wt-1', language: 'html', mode: 'edit' - }), - undefined + }) ) }) @@ -116,8 +113,7 @@ describe('openDocPreviewSource', () => { openDocPreviewSource({ ...REMOTE_DOCUMENT, externalSshTargetId: 'ssh-7' }) expect(mocks.openFile).toHaveBeenCalledWith( - expect.objectContaining({ externalSshTargetId: 'ssh-7' }), - undefined + expect.objectContaining({ externalSshTargetId: 'ssh-7' }) ) }) diff --git a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.ts b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.ts index 097062aa0e4..534c5d0db94 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.ts +++ b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.ts @@ -10,7 +10,6 @@ import { canClientOsOpenWorkspaceFile } from '@/lib/workspace-file-host-routing' import { useAppStore } from '@/store' -import { openWorkspaceFile } from '@/lib/workspace-file-open' import { downloadAndOpenRemoteTerminalFile } from '@/components/terminal-pane/terminal-remote-file-download-open' export type DocPreviewDocument = { @@ -31,22 +30,16 @@ export type DocPreviewDocument = { * preview keeps its own id, so the reader can leave the rendered page where it was. */ export function openDocPreviewSource(document: DocPreviewDocument): void { - openWorkspaceFile({ - kind: 'file', - intent: 'source', - file: { - filePath: document.filePath, - relativePath: document.relativePath, - worktreeId: document.worktreeId, - // Why not the preview tab's language: it is pinned to 'html' for the preview itself, and the - // source tab needs the editor's own detection to pick a highlighter. - language: detectLanguage(document.filePath), - runtimeEnvironmentId: document.runtimeEnvironmentId, - ...(document.externalSshTargetId - ? { externalSshTargetId: document.externalSshTargetId } - : {}), - mode: 'edit' - } + useAppStore.getState().openFile({ + filePath: document.filePath, + relativePath: document.relativePath, + worktreeId: document.worktreeId, + // Why not the preview tab's language: it is pinned to 'html' for the preview itself, and the + // source tab needs the editor's own detection to pick a highlighter. + language: detectLanguage(document.filePath), + runtimeEnvironmentId: document.runtimeEnvironmentId, + ...(document.externalSshTargetId ? { externalSshTargetId: document.externalSshTargetId } : {}), + mode: 'edit' }) } diff --git a/src/renderer/src/components/editor/editor-dropped-file-open.escaping-link.test.ts b/src/renderer/src/components/editor/editor-dropped-file-open.escaping-link.test.ts index 306384392d2..6f04f5e23ab 100644 --- a/src/renderer/src/components/editor/editor-dropped-file-open.escaping-link.test.ts +++ b/src/renderer/src/components/editor/editor-dropped-file-open.escaping-link.test.ts @@ -60,8 +60,7 @@ describe('dropping a project file on the tab strip', () => { expect.objectContaining({ filePath: '/repo/docs-link/secret.md', relativePath: '/repo/docs-link/secret.md' - }), - {} + }) ) }) @@ -71,8 +70,7 @@ describe('dropping a project file on the tab strip', () => { await dropOnTabStrip('/repo/docs/plain.md') expect(mocks.openFile).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '/repo/docs/plain.md', relativePath: 'docs/plain.md' }), - {} + expect.objectContaining({ filePath: '/repo/docs/plain.md', relativePath: 'docs/plain.md' }) ) }) }) diff --git a/src/renderer/src/components/editor/editor-dropped-file-open.ts b/src/renderer/src/components/editor/editor-dropped-file-open.ts index bc0f60951e7..4f138111882 100644 --- a/src/renderer/src/components/editor/editor-dropped-file-open.ts +++ b/src/renderer/src/components/editor/editor-dropped-file-open.ts @@ -13,8 +13,8 @@ import { translate } from '@/i18n/i18n' import { captureWorktreeSshMutationExpectation } from '@/lib/ssh-mutation-expectation' import { statUserOpenedPath } from '@/lib/user-opened-local-path' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { openDocumentInFloatingWorkspace } from '@/lib/open-document-in-floating-workspace' import { requireRuntimeTargetForFileOwner } from '@/lib/file-owner-runtime-target' -import { openWorkspaceFile } from '@/lib/workspace-file-open' export function shouldUploadRemoteEditorFileDrop( target: RuntimeClientTarget, @@ -142,6 +142,7 @@ async function openEditorFileDropPaths( paths: readonly string[] ): Promise { const { fileContext, worktreePath, connectionId } = context + const store = useAppStore.getState() const { worktreeId, groupId } = destination const groupOptions = groupId ? { targetGroupId: groupId } : null const runtimeEnvironmentId = runtimeTargetEnvironmentId(fileContext.target) @@ -165,6 +166,9 @@ async function openEditorFileDropPaths( destinationDir, { ensureDestinationDir: true, nameExtensionlessImages: true } ) + if (!canOpenEditorFileDrop(context, destination)) { + return + } for (const result of results) { if (!canOpenEditorFileDrop(context, destination)) { return @@ -173,9 +177,9 @@ async function openEditorFileDropPaths( continue } const maybeRelative = toWorktreeRelativePath(result.destPath, worktreePath) - openWorkspaceFile({ - kind: 'file', - file: { + store.setActiveTabType('editor', worktreeId) + store.openFile( + { filePath: result.destPath, relativePath: maybeRelative ?? result.destPath, worktreeId, @@ -183,8 +187,8 @@ async function openEditorFileDropPaths( language: detectLanguage(result.destPath), mode: 'edit' }, - options: { suppressActiveRuntimeFallback: runtimeEnvironmentId === null, ...groupOptions } - }) + { suppressActiveRuntimeFallback: runtimeEnvironmentId === null, ...groupOptions } + ) } if (results.some((result) => result.status !== 'imported')) { toast.error( @@ -221,22 +225,27 @@ async function openEditorFileDropPaths( relativePath = maybeRelative } } - const floating = worktreeId === FLOATING_TERMINAL_WORKTREE_ID - openWorkspaceFile({ - kind: 'file', - file: { - filePath, - relativePath, - worktreeId, - language: detectLanguage(filePath), - mode: 'edit', - ...(floating ? { runtimeEnvironmentId: null } : {}) - }, - options: { - ...groupOptions, - ...(floating ? { preview: false, suppressActiveRuntimeFallback: true } : {}) - } - }) + store.setActiveTabType('editor', worktreeId) + if (worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + openDocumentInFloatingWorkspace( + store.openFile, + { filePath, relativePath }, + groupOptions ?? {} + ) + continue + } + const file = { + filePath, + relativePath, + worktreeId, + language: detectLanguage(filePath), + mode: 'edit' as const + } + if (groupOptions) { + store.openFile(file, groupOptions) + } else { + store.openFile(file) + } } catch { // Ignore files that cannot be stat'd. } diff --git a/src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx b/src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx index 8276b546754..23b1e3e8d9e 100644 --- a/src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx +++ b/src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx @@ -11,7 +11,6 @@ import { } from '@/lib/remote-workspace-drop-test-fixtures' import { joinPath } from '@/lib/path' import { useEditorGroupFileDropOwner } from './use-editor-group-file-drop-owner' -import { createCompatibleRuntimeStatusResponse } from '@/runtime/runtime-compatibility-test-fixture' const mocks = vi.hoisted(() => ({ importPaths: vi.fn(), @@ -42,20 +41,7 @@ function dropFile(target: Element, name = 'shot.png'): void { beforeEach(() => { seedRemoteDropWorkspaces() - const response = createCompatibleRuntimeStatusResponse('owner-runtime') - if (!response.ok) { - throw new Error('Expected a compatible runtime fixture') - } - useAppStore.setState({ - openFile: mocks.openFile, - setActiveTabType: mocks.setActiveTabType, - browserTabsByWorktree: {}, - browserPagesByWorkspace: {}, - unifiedTabsByWorktree: {}, - runtimeStatusByEnvironmentId: new Map([ - ['owner-runtime', { status: response.result, checkedAt: Date.now() }] - ]) - }) + useAppStore.setState({ openFile: mocks.openFile, setActiveTabType: mocks.setActiveTabType }) mocks.prepare.mockImplementation(async ({ paths }: { paths: string[] }) => ({ paths, failures: [] @@ -122,79 +108,40 @@ describe('editor group OS file drops on remote workspaces', () => { ) }) - it.each(['wt-ssh', 'wt-runtime'])( - 'previews uploaded HTML in its destination group in %s', - async (worktreeId) => { - const groupId = worktreeId === 'wt-ssh' ? 'group-ssh' : 'group-runtime' - const root = worktreeId === 'wt-ssh' ? SSH_PATH : RUNTIME_PATH - const view = render() - dropFile(view.getByTestId('strip'), 'report.html') - await waitFor(() => - expect(useAppStore.getState().browserTabsByWorktree[worktreeId]).toHaveLength(1) + it.each(['report.html', 'notes.txt'])( + 'refuses opening %s when its workspace changes hosts during upload', + async (name) => { + let finish: (result: { + results: { status: 'imported'; kind: 'file'; destPath: string }[] + }) => void = () => undefined + mocks.importPaths.mockImplementationOnce(() => new Promise((resolve) => (finish = resolve))) + const view = render() + dropFile(view.getByTestId('strip'), name) + await waitFor(() => expect(mocks.importPaths).toHaveBeenCalledTimes(1)) + useAppStore.setState((state) => ({ + repos: state.repos.map((repo) => + repo.id === 'repo-rt' ? { ...repo, executionHostId: 'runtime:new-owner' as const } : repo + ) + })) + await act(async () => + finish({ + results: [ + { + status: 'imported', + kind: 'file', + destPath: joinPath(RUNTIME_PATH, `.orca/drops/${name}`) + } + ] + }) ) - const state = useAppStore.getState() - const workspace = state.browserTabsByWorktree[worktreeId][0] - expect(state.browserPagesByWorkspace[workspace.id][0].docLocation).toEqual({ - kind: 'workspace-doc', - worktreeId, - filePath: joinPath(root, '.orca/drops/report.html') - }) - expect(state.unifiedTabsByWorktree[worktreeId]).toEqual([ - expect.objectContaining({ contentType: 'browser', entityId: workspace.id, groupId }) - ]) expect(mocks.openFile).not.toHaveBeenCalled() + expect(mocks.setActiveTabType).not.toHaveBeenCalled() + expect(mocks.toastError).toHaveBeenCalledWith( + expect.stringMatching(/Couldn't verify which host owns/) + ) } ) - it('opens uploaded HTML source when the runtime cannot serve scoped previews', async () => { - useAppStore.setState({ runtimeStatusByEnvironmentId: new Map() }) - const view = render() - - dropFile(view.getByTestId('strip'), 'report.html') - - await waitFor(() => expect(mocks.openFile).toHaveBeenCalledTimes(1)) - expect(mocks.openFile).toHaveBeenCalledWith( - expect.objectContaining({ - filePath: joinPath(RUNTIME_PATH, '.orca/drops/report.html'), - worktreeId: 'wt-runtime', - runtimeEnvironmentId: 'owner-runtime' - }), - { suppressActiveRuntimeFallback: false, targetGroupId: 'group-runtime' } - ) - expect(useAppStore.getState().browserTabsByWorktree['wt-runtime'] ?? []).toHaveLength(0) - }) - - it('refuses an HTML preview when its workspace changes hosts during upload', async () => { - let finish: (result: { - results: { status: 'imported'; kind: 'file'; destPath: string }[] - }) => void = () => undefined - mocks.importPaths.mockImplementationOnce(() => new Promise((resolve) => (finish = resolve))) - const view = render() - dropFile(view.getByTestId('strip'), 'report.html') - await waitFor(() => expect(mocks.importPaths).toHaveBeenCalledTimes(1)) - useAppStore.setState((state) => ({ - repos: state.repos.map((repo) => - repo.id === 'repo-rt' ? { ...repo, executionHostId: 'runtime:new-owner' as const } : repo - ) - })) - await act(async () => - finish({ - results: [ - { - status: 'imported', - kind: 'file', - destPath: joinPath(RUNTIME_PATH, '.orca/drops/report.html') - } - ] - }) - ) - expect(useAppStore.getState().browserTabsByWorktree['wt-runtime'] ?? []).toHaveLength(0) - expect(mocks.openFile).not.toHaveBeenCalled() - expect(mocks.toastError).toHaveBeenCalledWith( - expect.stringMatching(/Couldn't verify which host owns/) - ) - }) - it.each(['wt-ssh', 'wt-runtime'])( 'opens nothing when the group closes during upload in %s and keeps preparation feedback', async (worktreeId) => { diff --git a/src/renderer/src/components/editor/use-editor-group-file-drop-owner.test.tsx b/src/renderer/src/components/editor/use-editor-group-file-drop-owner.test.tsx index 323a7478d55..55fa5d0b2d3 100644 --- a/src/renderer/src/components/editor/use-editor-group-file-drop-owner.test.tsx +++ b/src/renderer/src/components/editor/use-editor-group-file-drop-owner.test.tsx @@ -12,6 +12,7 @@ const mocks = vi.hoisted(() => { const groupsByWorktree: Record = {} return { openFile: vi.fn(), + setActiveTabType: vi.fn(), stat: vi.fn(), prepare: vi.fn(), toastError: vi.fn(), @@ -55,6 +56,7 @@ vi.mock('@/store', () => ({ activeWorktreeId: 'wt-active', groupsByWorktree: mocks.groupsByWorktree, getKnownWorktreeById: (id: string) => ({ id, path: `/repos/${id}` }), + setActiveTabType: mocks.setActiveTabType, openFile: mocks.openFile }) } @@ -147,6 +149,7 @@ describe('editor group OS file drops', () => { }), { targetGroupId: 'group-b' } ) + expect(mocks.setActiveTabType).toHaveBeenCalledWith('editor', 'wt-b') } ) @@ -259,6 +262,7 @@ describe('editor group OS file drops', () => { } await act(async () => finish({ isDirectory: false, escapesWorktree: false })) expect(mocks.openFile).not.toHaveBeenCalled() + expect(mocks.setActiveTabType).not.toHaveBeenCalled() expect(mocks.toastError).toHaveBeenCalledTimes(1) } ) diff --git a/src/renderer/src/components/quick-open-file-navigation.test.ts b/src/renderer/src/components/quick-open-file-navigation.test.ts index 5586dde0b04..c3ddc6f1aee 100644 --- a/src/renderer/src/components/quick-open-file-navigation.test.ts +++ b/src/renderer/src/components/quick-open-file-navigation.test.ts @@ -11,7 +11,6 @@ const mocks = vi.hoisted(() => { authorize: vi.fn(), open: vi.fn(() => 'owner-file-id'), source: vi.fn(), - preview: vi.fn(), reveal: vi.fn(), assertCurrent: vi.fn(), rpc: vi.fn(), @@ -27,10 +26,6 @@ vi.mock('@/store', () => ({ }) } })) -vi.mock('@/lib/file-preview', () => ({ - openFileInBrowserTab: mocks.preview, - getWorkspaceFilePreviewPlan: () => ({ status: 'browser-tab' }) -})) vi.mock('@/runtime/runtime-file-metadata-client', () => ({ statRuntimePath: mocks.stat, isMissingRuntimePathError: (error: Error) => error.message.includes('ENOENT') @@ -57,36 +52,9 @@ beforeEach(() => { mocks.route.current = { target: { kind: 'local' }, connectionId: undefined } mocks.stat.mockResolvedValue({ isDirectory: false }) mocks.assertCurrent.mockReset() - mocks.preview.mockReturnValue({ status: 'unsupported' }) vi.stubGlobal('window', { api: { fs: { authorizeExternalPath: mocks.authorize } } }) }) -it('previews an HTML file selected without a line destination', async () => { - mocks.preview.mockReturnValue({ status: 'browser-tab' }) - await openQuickOpenFile('report.html', 'wt', '/repo', { pathQuery: 'report.html' }) - expect(mocks.preview).toHaveBeenCalledWith({ filePath: '/repo/report.html', worktreeId: 'wt' }) - expect(mocks.open).not.toHaveBeenCalled() - expect(mocks.reveal).not.toHaveBeenCalled() -}) - -it('opens HTML at the requested source line even when a preview is available', async () => { - mocks.preview.mockReturnValue({ status: 'browser-tab' }) - await openQuickOpenFile('report.html', 'wt', '/repo', { - pathQuery: 'report.html', - line: 12, - column: 3 - }) - expect(mocks.preview).not.toHaveBeenCalled() - expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ language: 'html' }), undefined) - expect(mocks.reveal).toHaveBeenCalledWith( - expect.any(Function), - '/repo/report.html', - 12, - 3, - 'owner-file-id' - ) -}) - it('validates pasted local paths with user-named access and uses the owner-qualified reveal', async () => { await openQuickOpenFile('/external/guide.md', 'wt', '/repo', { pathQuery: '/external/guide.md', @@ -101,8 +69,7 @@ it('validates pasted local paths with user-named access and uses the owner-quali filePath: '/external/guide.md', relativePath: '/external/guide.md', runtimeEnvironmentId: null - }), - undefined + }) ) expect(mocks.source).toHaveBeenCalledWith('owner-file-id', 'source') expect(mocks.reveal).toHaveBeenCalledWith( @@ -122,10 +89,7 @@ it('keeps a literal colon filename when it exists instead of applying its appare { pathQuery: '/repo/report', line: 12 }, '/repo/report:12' ) - expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '/repo/report:12' }), - undefined - ) + expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ filePath: '/repo/report:12' })) expect(mocks.reveal).not.toHaveBeenCalled() }) @@ -138,10 +102,7 @@ it('falls back from a missing literal colon filename to line navigation', async { pathQuery: '/repo/file.ts', line: 12 }, '/repo/file.ts:12' ) - expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '/repo/file.ts' }), - undefined - ) + expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ filePath: '/repo/file.ts' })) expect(mocks.reveal).toHaveBeenCalledWith( expect.any(Function), '/repo/file.ts', @@ -160,10 +121,7 @@ it('probes SSH paths only on their owning host', async () => { '/external/file.ts', { kind: 'user-file' } ) - expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ externalSshTargetId: 'ssh-a' }), - undefined - ) + expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ externalSshTargetId: 'ssh-a' })) }) it('resolves paired paths on the host and never falls back to client filesystem access', async () => { @@ -178,8 +136,7 @@ it('resolves paired paths on the host and never falls back to client filesystem expect(mocks.stat).not.toHaveBeenCalled() expect(mocks.authorize).not.toHaveBeenCalled() expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ runtimeEnvironmentId: 'env-a', relativePath: 'file.ts' }), - undefined + expect.objectContaining({ runtimeEnvironmentId: 'env-a', relativePath: 'file.ts' }) ) mocks.rpc.mockResolvedValue({ exists: true, @@ -243,10 +200,7 @@ it('resolves literal colon names on a paired host before treating the suffix as '/repo/report:12' ) expect(mocks.rpc).toHaveBeenCalledOnce() - expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '/repo/report:12' }), - undefined - ) + expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ filePath: '/repo/report:12' })) expect(mocks.reveal).not.toHaveBeenCalled() expect(mocks.stat).not.toHaveBeenCalled() }) @@ -266,10 +220,7 @@ it('keeps a pasted POSIX file on the local workspace WSL distro before any files expect.anything(), '\\\\wsl.localhost\\Ubuntu\\home\\repo\\file.ts' ) - expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ relativePath: 'file.ts' }), - undefined - ) + expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ relativePath: 'file.ts' })) mocks.route.current.connectionId = 'ssh-a' await openQuickOpenFile('/home/repo/file.ts', 'wt', '\\\\wsl.localhost\\Ubuntu\\home\\repo', { pathQuery: '/home/repo/file.ts' @@ -293,8 +244,7 @@ it.each(['literal:12', 'nested/literal:12', 'literal:12:3'])( query ) expect(mocks.open).toHaveBeenCalledWith( - expect.objectContaining({ filePath: `/repo/${selected}` }), - undefined + expect.objectContaining({ filePath: `/repo/${selected}` }) ) expect(mocks.reveal).not.toHaveBeenCalled() } diff --git a/src/renderer/src/components/quick-open-file-navigation.ts b/src/renderer/src/components/quick-open-file-navigation.ts index 1d0bb2380bb..d0eb76c96b1 100644 --- a/src/renderer/src/components/quick-open-file-navigation.ts +++ b/src/renderer/src/components/quick-open-file-navigation.ts @@ -1,7 +1,6 @@ import { prepareQuickOpenFiles, rankQuickOpenFiles } from '../../../shared/quick-open-path-search' import { statUserOpenedPath } from '@/lib/user-opened-local-path' import { detectLanguage } from '@/lib/language-detect' -import { openWorkspaceFile } from '@/lib/workspace-file-open' import { joinPath, getRelativePathInsideRoot } from '@/lib/path' import { useAppStore } from '@/store' import { isMissingRuntimePathError } from '@/runtime/runtime-file-metadata-client' @@ -121,27 +120,28 @@ export async function openQuickOpenFile( } } assertCurrent() - const revealLine = literalSelected ? undefined : navigation.line - const opened = openWorkspaceFile({ - kind: 'file', - intent: revealLine === undefined ? 'default' : 'source', - file: { - filePath, - relativePath, - worktreeId, - runtimeEnvironmentId: target.kind === 'environment' ? target.environmentId : null, - ...(route.connectionId && getRelativePathInsideRoot(filePath, root) === null - ? { externalSshTargetId: route.connectionId } - : {}), - language: detectLanguage(filePath), - mode: 'edit' - } + const store = useAppStore.getState() + const fileId = store.openFile({ + filePath, + relativePath, + worktreeId, + runtimeEnvironmentId: target.kind === 'environment' ? target.environmentId : null, + ...(route.connectionId && getRelativePathInsideRoot(filePath, root) === null + ? { externalSshTargetId: route.connectionId } + : {}), + language: detectLanguage(filePath), + mode: 'edit' }) - if (revealLine !== undefined && opened.surface === 'editor') { - const fileId = opened.fileId + if (!literalSelected && navigation.line !== undefined) { if (detectLanguage(filePath) === 'markdown') { useAppStore.getState().setMarkdownViewMode(fileId, 'source') } - scheduleEditorLineReveal(useAppStore.getState, filePath, revealLine, navigation.column, fileId) + scheduleEditorLineReveal( + useAppStore.getState, + filePath, + navigation.line, + navigation.column, + fileId + ) } } diff --git a/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx index c724cc1643b..3f8793166f7 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx @@ -41,7 +41,6 @@ const mocks = vi.hoisted(() => { lastActivityAt: 0 } const calls = { - createBrowserTab: vi.fn(), openDiff: vi.fn(), openFile: vi.fn(), openConflictFile: vi.fn(), @@ -150,10 +149,6 @@ function resetState(overrides: Partial> = {}): void { activeWorktreeId: mocks.activeWorktree.id, activeGroupIdByWorktree: { [mocks.activeWorktree.id]: 'group-1' }, groupsByWorktree: { [mocks.activeWorktree.id]: [{ id: 'group-1', activeTabId: null }] }, - browserTabsByWorktree: {}, - browserPagesByWorkspace: {}, - createBrowserTab: mocks.calls.createBrowserTab, - getKnownWorktreeById: () => mocks.activeWorktree, repos: [mocks.activeRepo], worktreesByRepo: { [mocks.activeRepo.id]: [mocks.activeWorktree] }, sshConnectionStates: new Map(), @@ -216,7 +211,6 @@ function resetState(overrides: Partial> = {}): void { clearDiffComments: noopAsync(true), clearDiffCommentsForFile: noopAsync(true), setScrollToDiffCommentId: vi.fn(), - closeAgentSendPopoverTargetMode: vi.fn(), setRightSidebarOpen: vi.fn(), setRightSidebarTab: vi.fn(), allocateCommitMessageGenerationRequestId: vi.fn(() => 'commit-generation-1'), @@ -273,10 +267,11 @@ function doubleClickUncommitted(path: string): void { }) } -function clickBranchRow(path = 'src/branch.ts', init: MouseEventInit = {}): void { - const row = container.querySelector( - `[data-source-control-area="branch"][data-source-control-path="${path}"]` +function clickBranchRow(init: MouseEventInit = {}): void { + const label = [...container.querySelectorAll('span')].find( + (candidate) => candidate.textContent === 'branch.ts' ) + const row = label?.closest('div') expect(row).not.toBeNull() act(() => { row?.dispatchEvent(new MouseEvent('click', { bubbles: true, ...init })) @@ -284,287 +279,37 @@ function clickBranchRow(path = 'src/branch.ts', init: MouseEventInit = {}): void } describe('SourceControl preview row opens', () => { - it.each(['untracked', 'unstaged', 'staged'] as const)( - 'keeps the current-file indicator on an HTML browser preview in %s', - (area) => { - resetState({ - gitStatusByWorktree: { - [mocks.activeWorktree.id]: [ - gitEntry({ - path: 'report one.html', - area, - status: area === 'untracked' ? area : 'modified' - }) - ] - }, - activeTabTypeByWorktree: { [mocks.activeWorktree.id]: 'browser' }, - activeBrowserTabIdByWorktree: { [mocks.activeWorktree.id]: 'browser-1' }, - browserTabsByWorktree: { - [mocks.activeWorktree.id]: [{ id: 'browser-1', activePageId: 'page-html' }] - }, - browserPagesByWorkspace: { - 'browser-1': [ - { id: 'page-web', url: 'https://example.com' }, - { id: 'page-html', url: 'file:///repo/wt/report%20one.html#section' } - ] - } - }) - renderSourceControl() - - expect( - container - .querySelector('[data-source-control-path="report one.html"]') - ?.getAttribute('data-current') - ).toBe('true') - } - ) - - it.each([mocks.activeWorktree.id, 'other-workspace'])( - 'matches a workspace document only to its owning workspace: %s', - (documentWorktreeId) => { - resetState({ - repos: [{ ...mocks.activeRepo, connectionId: 'ssh-1' }], - gitStatusByWorktree: { [mocks.activeWorktree.id]: [gitEntry({ path: 'test.html' })] }, - activeTabTypeByWorktree: { [mocks.activeWorktree.id]: 'browser' }, - activeBrowserTabIdByWorktree: { [mocks.activeWorktree.id]: 'browser-1' }, - browserTabsByWorktree: { - [mocks.activeWorktree.id]: [{ id: 'browser-1', activePageId: 'page-html' }] - }, - browserPagesByWorkspace: { - 'browser-1': [ - { - id: 'page-html', - url: 'data:text/html,', - docLocation: { - kind: 'workspace-doc', - worktreeId: documentWorktreeId, - filePath: '/repo/wt/test.html' - } - } - ] - } - }) - renderSourceControl() - - expect( - container - .querySelector('[data-source-control-path="test.html"]') - ?.getAttribute('data-current') - ).toBe(documentWorktreeId === mocks.activeWorktree.id ? 'true' : null) - } - ) - - it.each(['untracked', 'unstaged', 'staged'] as const)( - 'renders HTML when opening a row in %s', - (area) => { - resetState({ - gitStatusByWorktree: { - [mocks.activeWorktree.id]: [ - gitEntry({ path: 'test.html', area, status: area === 'untracked' ? area : 'modified' }) - ] - } - }) - renderSourceControl() - - clickUncommitted('test.html') - - expect(mocks.calls.createBrowserTab).toHaveBeenCalledWith( - mocks.activeWorktree.id, - 'file:///repo/wt/test.html', - { title: 'test.html', targetGroupId: undefined, activate: true } - ) - expect(mocks.calls.openDiff).not.toHaveBeenCalled() - expect(mocks.calls.openFile).not.toHaveBeenCalled() - } - ) - - it('targets the requested split when previewing HTML', () => { - resetState({ - gitStatusByWorktree: { [mocks.activeWorktree.id]: [gitEntry({ path: 'test.html' })] } - }) - renderSourceControl() - - clickUncommitted('test.html', { ctrlKey: true }) - - expect(mocks.calls.createBrowserTab).toHaveBeenCalledWith( - mocks.activeWorktree.id, - 'file:///repo/wt/test.html', - { title: 'test.html', targetGroupId: 'group-2', activate: true } - ) - expect(mocks.calls.openDiff).not.toHaveBeenCalled() - }) - - it('previews HTML through its SSH owner', () => { - resetState({ - repos: [{ ...mocks.activeRepo, connectionId: 'ssh-1' }], - gitStatusByWorktree: { [mocks.activeWorktree.id]: [gitEntry({ path: 'test.html' })] } - }) - renderSourceControl() - - clickUncommitted('test.html') - - expect(mocks.calls.createBrowserTab).toHaveBeenCalledWith( - mocks.activeWorktree.id, - 'data:text/html,', - expect.objectContaining({ - docLocation: { - kind: 'workspace-doc', - worktreeId: mocks.activeWorktree.id, - filePath: '/repo/wt/test.html' - }, - browserRuntimeEnvironmentId: null, - activate: true - }) - ) - expect(mocks.calls.openDiff).not.toHaveBeenCalled() - }) - - it('keeps deleted HTML and HTML conflicts in their diff surfaces', () => { + it.each([ + ['src/file.ts', 'src/staged.ts', 'typescript'], + ['report.html', 'staged.html', 'html'], + ['REPORT.HTM', 'STAGED.HTM', 'html'] + ])('opens %s and %s as preview diff tabs on plain clicks', (path, stagedPath, language) => { resetState({ gitStatusByWorktree: { [mocks.activeWorktree.id]: [ - gitEntry({ path: 'deleted.html', status: 'deleted' }), - gitEntry({ - path: 'conflict.html', - conflictKind: 'both_modified', - conflictStatus: 'unresolved' - }) + gitEntry({ path }), + gitEntry({ path: stagedPath, area: 'staged' }) ] } }) renderSourceControl() - clickUncommitted('deleted.html') - clickUncommitted('conflict.html') - - expect(mocks.calls.createBrowserTab).not.toHaveBeenCalled() - expect(mocks.calls.openDiff).toHaveBeenCalledWith( - mocks.activeWorktree.id, - '/repo/wt/deleted.html', - 'deleted.html', - 'html', - false, - { targetGroupId: undefined, preview: true } - ) - expect(mocks.calls.openConflictFile).toHaveBeenCalledWith( - mocks.activeWorktree.id, - '/repo/wt', - expect.objectContaining({ path: 'conflict.html' }), - 'html', - { targetGroupId: undefined, preview: true } - ) - }) - - it('falls back to the diff when this client cannot preview HTML', () => { - vi.stubGlobal('__ORCA_WEB_CLIENT__', true) - onTestFinished(() => { - vi.unstubAllGlobals() - }) - resetState({ - gitStatusByWorktree: { [mocks.activeWorktree.id]: [gitEntry({ path: 'test.html' })] } - }) - renderSourceControl() - - clickUncommitted('test.html') - - expect(mocks.calls.createBrowserTab).not.toHaveBeenCalled() - expect(mocks.calls.openDiff).toHaveBeenCalledWith( - mocks.activeWorktree.id, - '/repo/wt/test.html', - 'test.html', - 'html', - false, - { targetGroupId: undefined, preview: true } - ) - }) - - it.each(['uncommitted', 'branch'] as const)('opens a %s HTML review note in the diff', (area) => { - resetState({ - worktreesByRepo: { - [mocks.activeRepo.id]: [ - { - ...mocks.activeWorktree, - diffComments: [ - { - id: 'html-note', - worktreeId: mocks.activeWorktree.id, - filePath: 'test.html', - lineNumber: 1, - side: 'modified', - body: 'Review this heading', - createdAt: 1 - } - ] - } - ] - }, - gitStatusByWorktree: { - [mocks.activeWorktree.id]: area === 'uncommitted' ? [gitEntry({ path: 'test.html' })] : [] - }, - gitBranchChangesByWorktree: { - [mocks.activeWorktree.id]: area === 'branch' ? [branchEntry({ path: 'test.html' })] : [] - }, - gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: branchSummary() } - }) - renderSourceControl() - - const expandNotes = container.querySelector('button[title="Expand notes"]') - expect(expandNotes).not.toBeNull() - act(() => expandNotes?.click()) - const note = container.querySelector('button[aria-label^="Open note on"]') - expect(note?.textContent).toContain('Review this heading') - act(() => note?.click()) - - expect(mocks.calls.createBrowserTab).not.toHaveBeenCalled() - if (area === 'uncommitted') { - expect(mocks.calls.openDiff).toHaveBeenCalledWith( - mocks.activeWorktree.id, - '/repo/wt/test.html', - 'test.html', - 'html', - false, - { preview: true } - ) - } else { - expect(mocks.calls.openBranchDiff).toHaveBeenCalledWith( - mocks.activeWorktree.id, - '/repo/wt', - expect.objectContaining({ path: 'test.html' }), - branchSummary(), - 'html', - { preview: true } - ) - } - expect(mocks.state.setScrollToDiffCommentId).toHaveBeenLastCalledWith('html-note') - }) - - it('passes preview=true when plain uncommitted row clicks open diff tabs', () => { - resetState({ - gitStatusByWorktree: { - [mocks.activeWorktree.id]: [ - gitEntry({ path: 'src/file.ts' }), - gitEntry({ path: 'src/staged.ts', area: 'staged' }) - ] - } - }) - renderSourceControl() - - clickUncommitted('src/file.ts') - clickUncommitted('src/staged.ts') + clickUncommitted(path) + clickUncommitted(stagedPath) expect(mocks.calls.openDiff).toHaveBeenCalledWith( mocks.activeWorktree.id, - '/repo/wt/src/file.ts', - 'src/file.ts', - 'typescript', + `/repo/wt/${path}`, + path, + language, false, { targetGroupId: undefined, preview: true } ) expect(mocks.calls.openDiff).toHaveBeenCalledWith( mocks.activeWorktree.id, - '/repo/wt/src/staged.ts', - 'src/staged.ts', - 'typescript', + `/repo/wt/${stagedPath}`, + stagedPath, + language, true, { targetGroupId: undefined, preview: true } ) @@ -790,68 +535,6 @@ describe('SourceControl preview row opens', () => { ) }) - it.each([false, true])('previews committed HTML from a branch row (split: %s)', (split) => { - resetState({ - gitBranchChangesByWorktree: { - [mocks.activeWorktree.id]: [branchEntry({ path: 'test.html' })] - }, - gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: branchSummary() } - }) - renderSourceControl() - - clickBranchRow('test.html', { ctrlKey: split }) - - expect(mocks.calls.createBrowserTab).toHaveBeenCalledWith( - mocks.activeWorktree.id, - 'file:///repo/wt/test.html', - { title: 'test.html', targetGroupId: split ? 'group-2' : undefined, activate: true } - ) - expect(mocks.calls.openBranchDiff).not.toHaveBeenCalled() - }) - - it.each(['branch-deleted', 'worktree-deleted', 'worktree-conflict'] as const)( - 'keeps a %s HTML branch row in the diff', - (kind) => { - resetState({ - gitBranchChangesByWorktree: { - [mocks.activeWorktree.id]: [ - branchEntry({ - path: 'test.html', - status: kind === 'branch-deleted' ? 'deleted' : 'added' - }) - ] - }, - gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: branchSummary() }, - gitStatusByWorktree: { - [mocks.activeWorktree.id]: - kind === 'branch-deleted' - ? [] - : [ - gitEntry({ - path: 'test.html', - ...(kind === 'worktree-deleted' - ? { status: 'deleted' } - : { conflictKind: 'both_modified', conflictStatus: 'unresolved' }) - }) - ] - } - }) - renderSourceControl() - - clickBranchRow('test.html') - - expect(mocks.calls.createBrowserTab).not.toHaveBeenCalled() - expect(mocks.calls.openBranchDiff).toHaveBeenCalledWith( - mocks.activeWorktree.id, - '/repo/wt', - expect.objectContaining({ path: 'test.html' }), - branchSummary(), - 'html', - { targetGroupId: undefined, preview: true } - ) - } - ) - it('passes preview=true when a plain branch row click opens a branch diff tab', () => { resetState({ gitBranchChangesByWorktree: { [mocks.activeWorktree.id]: [branchEntry()] }, diff --git a/src/renderer/src/components/right-sidebar/file-explorer-drag-scroll-marker.test.tsx b/src/renderer/src/components/right-sidebar/file-explorer-drag-scroll-marker.test.tsx index 9f4b2856e18..6930a374a97 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-drag-scroll-marker.test.tsx +++ b/src/renderer/src/components/right-sidebar/file-explorer-drag-scroll-marker.test.tsx @@ -280,7 +280,7 @@ function HandlersProbe({ scrollRef }: { scrollRef: React.RefObject void) { return { activeWorktreeId: 'wt-1', - openWorkspaceFile: vi.fn(), + openFile: vi.fn(), makePreviewFilePermanent: vi.fn(), toggleDir, loadDir: vi.fn().mockResolvedValue(true), @@ -62,11 +62,7 @@ function createHandlerParams(toggleDir: (worktreeId: string, dirPath: string) => function renderHandlers(toggleDir: (worktreeId: string, dirPath: string) => void) { const params = createHandlerParams(toggleDir) const hook = renderHook(() => useFileExplorerHandlers(params)) - return { - ...hook, - openWorkspaceFile: params.openWorkspaceFile, - setSelectedPath: params.setSelectedPath - } + return { ...hook, openFile: params.openFile, setSelectedPath: params.setSelectedPath } } // Wires a row like the explorer does so clicks resolve timing in handleRowClick. @@ -187,7 +183,7 @@ describe('instant directory toggle', () => { it('keeps opening files on the second click, which is not a directory toggle', async () => { const toggleDir = vi.fn() - const { result, openWorkspaceFile } = renderHandlers(toggleDir) + const { result, openFile } = renderHandlers(toggleDir) await act(async () => { result.current.handleClick(fileNode, 'skip') @@ -195,6 +191,6 @@ describe('instant directory toggle', () => { }) expect(toggleDir).not.toHaveBeenCalled() - expect(openWorkspaceFile).toHaveBeenCalledTimes(1) + expect(openFile).toHaveBeenCalledTimes(1) }) }) diff --git a/src/renderer/src/components/right-sidebar/source-control-branch-section-heading.test.tsx b/src/renderer/src/components/right-sidebar/source-control-branch-section-heading.test.tsx index c669eedd623..d4136c63271 100644 --- a/src/renderer/src/components/right-sidebar/source-control-branch-section-heading.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control-branch-section-heading.test.tsx @@ -41,7 +41,7 @@ function render( worktreePath={join(tmpdir(), 'wt-1')} revealInExplorer={vi.fn()} activeConnectionId={null} - openBranchFile={vi.fn()} + openCommittedDiff={vi.fn()} openBranchAllDiffs={vi.fn()} diffCommentCountByPath={new Map()} /> diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/active-open-file-keys.ts b/src/renderer/src/components/right-sidebar/source-control/listing/active-open-file-keys.ts index d27f42d73d5..42f14951f44 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/active-open-file-keys.ts +++ b/src/renderer/src/components/right-sidebar/source-control/listing/active-open-file-keys.ts @@ -1,56 +1,9 @@ import type { DiffSource } from '@/store/slices/editor' -import type { AppState } from '@/store/types' -import { getRelativePathInsideRoot } from '@/lib/path' -import { runtimeEnvironmentIdUnlessTwin } from '@/lib/resolve-owner' -import { getBrowserPageRuntimeEnvironmentId } from '@/components/browser-pane/describe-page/browser-page-url-display' -import { browserFileUrlToAbsolutePath } from '@/components/browser-pane/describe-page/browser-artifact-upload' -import { canOpenBrowserFileInWorkspaceEditor } from '@/components/browser-pane/navigate/browser-file-editor-open' const EMPTY_OPEN_ROW_KEYS: ReadonlySet = new Set() const SIGNATURE_SEPARATOR = '::' -export function getActiveOpenFileSignature( - state: AppState, - worktreeId: string | null, - worktreePath: string | null -): string | null { - if (!worktreeId) { - return null - } - if (state.activeTabTypeByWorktree?.[worktreeId] === 'editor') { - const activeFileId = state.activeFileIdByWorktree?.[worktreeId] - const activeFile = state.openFiles?.find( - (file) => file.id === activeFileId && file.worktreeId === worktreeId - ) - return activeFile - ? buildActiveOpenFileSignature(activeFile.diffSource, activeFile.relativePath) - : null - } - if (state.activeTabTypeByWorktree?.[worktreeId] !== 'browser') { - return null - } - const workspaceId = state.activeBrowserTabIdByWorktree?.[worktreeId] - const workspace = state.browserTabsByWorktree?.[worktreeId]?.find((tab) => tab.id === workspaceId) - const pages = workspaceId ? state.browserPagesByWorkspace?.[workspaceId] : undefined - const page = pages?.find((candidate) => candidate.id === workspace?.activePageId) ?? pages?.[0] - if (!page) { - return null - } - let filePath: string | null = null - if (page.docLocation?.worktreeId === worktreeId) { - filePath = page.docLocation.filePath - } else if ( - !page.docLocation && - !getBrowserPageRuntimeEnvironmentId(page, runtimeEnvironmentIdUnlessTwin(state, worktreeId)) && - canOpenBrowserFileInWorkspaceEditor(state, worktreeId) - ) { - filePath = browserFileUrlToAbsolutePath(page.url) - } - const relativePath = filePath ? getRelativePathInsideRoot(filePath, worktreePath) : null - return relativePath ? buildActiveOpenFileSignature(undefined, relativePath) : null -} - /** * Build the stable string that identifies the active editor file for highlight * matching: `${diffSource}::${relativePath}`. Kept as a primitive so the zustand diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/branch-entry-row.tsx b/src/renderer/src/components/right-sidebar/source-control/listing/branch-entry-row.tsx index 13367929e53..de5a4148369 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/branch-entry-row.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/listing/branch-entry-row.tsx @@ -48,8 +48,6 @@ export function BranchEntryRow({ onRevealInExplorer={onRevealInExplorer} >
void activeConnectionId: string | null - openBranchFile: (entry: GitBranchChangeEntry, event?: SourceControlRowOpenEvent) => void + openCommittedDiff: (entry: GitBranchChangeEntry, event?: SourceControlRowOpenEvent) => void openBranchAllDiffs: ( worktreeId: string, worktreePath: string, @@ -130,7 +130,7 @@ export function SourceControlBranchSection({ depth={node.depth} onRevealInExplorer={revealInExplorer} connectionId={activeConnectionId} - onOpen={(event) => openBranchFile(node.entry, event)} + onOpen={(event) => openCommittedDiff(node.entry, event)} commentCount={diffCommentCountByPath.get(node.entry.path) ?? 0} showPathHint={false} /> @@ -150,7 +150,7 @@ export function SourceControlBranchSection({ worktreePath={worktreePath} onRevealInExplorer={revealInExplorer} connectionId={activeConnectionId} - onOpen={(event) => openBranchFile(entry, event)} + onOpen={(event) => openCommittedDiff(entry, event)} commentCount={diffCommentCountByPath.get(entry.path) ?? 0} /> )} diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.test.ts b/src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.test.ts index 6d564219c1a..ea92fc97491 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { getEditorPanelRenderModel } from '@/components/editor/editor-panel-render-model' import { createEditorStore } from '@/store/slices/editor-slice-test-harness' import type { GitStatusEntry } from '../../../../../../shared/git-status-types' -import { openUncommittedEntry } from '@/lib/workspace-file-changes-open' +import { openUncommittedEntry } from './open-uncommitted-entry' vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) vi.mock('@/runtime/close-mirrored-editor-tab', () => ({ diff --git a/src/renderer/src/lib/workspace-file-changes-open.ts b/src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.ts similarity index 93% rename from src/renderer/src/lib/workspace-file-changes-open.ts rename to src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.ts index 43f909c9b20..67ceb4bfb1d 100644 --- a/src/renderer/src/lib/workspace-file-changes-open.ts +++ b/src/renderer/src/components/right-sidebar/source-control/listing/open-uncommitted-entry.ts @@ -1,4 +1,4 @@ -import type { GitStatusEntry } from '../../../shared/git-status-types' +import type { GitStatusEntry } from '../../../../../../shared/git-status-types' import type { AppState } from '@/store/types' import { detectLanguage } from '@/lib/language-detect' import { joinPath } from '@/lib/path' diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/section-action-buttons.test.tsx b/src/renderer/src/components/right-sidebar/source-control/listing/section-action-buttons.test.tsx index 2a315f3c9f2..9942072cfbf 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/section-action-buttons.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/listing/section-action-buttons.test.tsx @@ -62,7 +62,7 @@ function renderUncommittedSections({ handleContextMenu={vi.fn()} revealInExplorer={vi.fn()} activeConnectionId={null} - openUncommittedFile={vi.fn()} + handleOpenDiff={vi.fn()} handleStage={vi.fn()} handleUnstage={vi.fn()} requestDiscardEntry={vi.fn()} diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/section-file-list.tsx b/src/renderer/src/components/right-sidebar/source-control/listing/section-file-list.tsx index 77ccd9a2588..2d71a640259 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/section-file-list.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/listing/section-file-list.tsx @@ -37,7 +37,7 @@ export function SourceControlSectionFileList({ handleContextMenu, revealInExplorer, activeConnectionId, - openUncommittedFile, + handleOpenDiff, handleStage, handleUnstage, requestDiscardEntry, @@ -64,7 +64,7 @@ export function SourceControlSectionFileList({ handleContextMenu: (key: string) => void revealInExplorer: (worktreeId: string, absolutePath: string) => void activeConnectionId: string | null - openUncommittedFile: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void + handleOpenDiff: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void handleStage: (path: string) => Promise handleUnstage: (path: string, oldPath?: string) => Promise requestDiscardEntry: (entry: GitStatusEntry) => void @@ -122,7 +122,7 @@ export function SourceControlSectionFileList({ onContextMenu={handleContextMenu} onRevealInExplorer={revealInExplorer} connectionId={activeConnectionId} - onOpen={openUncommittedFile} + onOpen={handleOpenDiff} onStage={handleStage} onUnstage={handleUnstage} onDiscard={requestDiscardEntry} @@ -173,7 +173,7 @@ export function SourceControlSectionFileList({ onContextMenu={handleContextMenu} onRevealInExplorer={revealInExplorer} connectionId={activeConnectionId} - onOpen={openUncommittedFile} + onOpen={handleOpenDiff} onStage={handleStage} onUnstage={handleUnstage} onDiscard={requestDiscardEntry} diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/uncommitted-sections.tsx b/src/renderer/src/components/right-sidebar/source-control/listing/uncommitted-sections.tsx index 63cb339dec3..8c1c0101bee 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/uncommitted-sections.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/listing/uncommitted-sections.tsx @@ -81,7 +81,7 @@ export function SourceControlUncommittedSections(props: { handleContextMenu: (key: string) => void revealInExplorer: (worktreeId: string, absolutePath: string) => void activeConnectionId: string | null - openUncommittedFile: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void + handleOpenDiff: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void handleStage: (path: string) => Promise handleUnstage: (path: string, oldPath?: string) => Promise requestDiscardEntry: (entry: GitStatusEntry) => void @@ -199,7 +199,7 @@ export function SourceControlUncommittedSections(props: { handleContextMenu={props.handleContextMenu} revealInExplorer={props.revealInExplorer} activeConnectionId={props.activeConnectionId} - openUncommittedFile={props.openUncommittedFile} + handleOpenDiff={props.handleOpenDiff} handleStage={props.handleStage} handleUnstage={props.handleUnstage} requestDiscardEntry={props.requestDiscardEntry} diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/use-file-listing.ts b/src/renderer/src/components/right-sidebar/source-control/listing/use-file-listing.ts index f1cd0be902c..1f24bd69f19 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/use-file-listing.ts +++ b/src/renderer/src/components/right-sidebar/source-control/listing/use-file-listing.ts @@ -114,7 +114,7 @@ export function useSourceControlFileListing({ }) // Why: modifier-click keeps the current pane intact by opening the file in a fresh split to the right. - const { resolveSplitTargetGroupId, activeOpenRowKeys, openUncommittedFile, openBranchFile } = + const { resolveSplitTargetGroupId, activeOpenRowKeys, handleOpenDiff, openCommittedDiff } = useSourceControlRowOpening({ isMac, activeWorktreeId, @@ -130,7 +130,7 @@ export function useSourceControlFileListing({ const { selectedKeys, handleSelect, handleContextMenu, clearSelection } = useSourceControlSelection({ flatEntries: visibleSelectionEntries, - onOpenFile: openUncommittedFile, + onOpenDiff: handleOpenDiff, shouldOpenAsSplit, containerRef: sourceControlRef }) @@ -210,7 +210,7 @@ export function useSourceControlFileListing({ handleBulkStage, handleBulkUnstage, handleContextMenu, - openUncommittedFile, + handleOpenDiff, handleSelect, handleStageAllPaths, handleStageAllPrimary, @@ -220,7 +220,7 @@ export function useSourceControlFileListing({ isExecutingBulk, isGitHistoryVisible, normalizedFilter, - openBranchFile, + openCommittedDiff, pushRecovery, refreshGitHistory, refreshGitHistoryRef, diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/use-row-opening.ts b/src/renderer/src/components/right-sidebar/source-control/listing/use-row-opening.ts index eb6d424896c..0c305e4b665 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/use-row-opening.ts +++ b/src/renderer/src/components/right-sidebar/source-control/listing/use-row-opening.ts @@ -1,12 +1,13 @@ import { useCallback, useMemo } from 'react' -import { openWorkspaceFile } from '@/lib/workspace-file-open' +import { detectLanguage } from '@/lib/language-detect' +import { openUncommittedEntry } from './open-uncommitted-entry' import { useAppStore } from '@/store' import type { GitBranchChangeEntry, GitBranchCompareSummary } from '../../../../../../shared/git-diff-compare-types' import type { GitStatusEntry } from '../../../../../../shared/git-status-types' -import { getActiveOpenFileSignature, buildActiveOpenRowKeys } from './active-open-file-keys' +import { buildActiveOpenFileSignature, buildActiveOpenRowKeys } from './active-open-file-keys' import type { FlatEntry } from './use-selection' import { isSourceControlSplitOpenModifier, @@ -30,6 +31,11 @@ export function useSourceControlRowOpening({ const activeGroupIdByWorktree = useAppStore((s) => s.activeGroupIdByWorktree) const groupsByWorktree = useAppStore((s) => s.groupsByWorktree) const createEmptySplitGroup = useAppStore((s) => s.createEmptySplitGroup) + const trackConflictPath = useAppStore((s) => s.trackConflictPath) + const openConflictFile = useAppStore((s) => s.openConflictFile) + const openDiff = useAppStore((s) => s.openDiff) + const openFile = useAppStore((s) => s.openFile) + const openBranchDiff = useAppStore((s) => s.openBranchDiff) // Why: modifier-click keeps the current pane intact by opening the file in a fresh split to the right. const resolveSplitTargetGroupId = useCallback( @@ -47,9 +53,22 @@ export function useSourceControlRowOpening({ [activeGroupIdByWorktree, activeWorktreeId, createEmptySplitGroup, groupsByWorktree, isMac] ) - const activeOpenFileSignature = useAppStore((s) => - getActiveOpenFileSignature(s, activeWorktreeId, worktreePath) - ) + // Why: a stable string signature keeps this selector referentially stable so the panel re-renders only when the active editor file changes; null when the tab isn't an editor. + const activeOpenFileSignature = useAppStore((s) => { + if (!activeWorktreeId || s.activeTabTypeByWorktree?.[activeWorktreeId] !== 'editor') { + return null + } + const activeFileId = s.activeFileIdByWorktree?.[activeWorktreeId] + if (!activeFileId) { + return null + } + const activeFile = s.openFiles?.find( + (file) => file.id === activeFileId && file.worktreeId === activeWorktreeId + ) + return activeFile + ? buildActiveOpenFileSignature(activeFile.diffSource, activeFile.relativePath) + : null + }) const activeOpenAvailableRowKeys = useMemo(() => { const keys = new Set() for (const entry of visibleSelectionEntries) { @@ -62,27 +81,33 @@ export function useSourceControlRowOpening({ [activeOpenAvailableRowKeys, activeOpenFileSignature] ) - const openUncommittedFile = useCallback( + const handleOpenDiff = useCallback( (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => { if (!activeWorktreeId || !worktreePath) { return } const targetGroupId = resolveSplitTargetGroupId(event) - openWorkspaceFile({ - kind: 'uncommitted', - worktreeId: activeWorktreeId, + const openAsPreview = shouldOpenSourceControlRowAsPreview(event, targetGroupId) + openUncommittedEntry( + { trackConflictPath, openConflictFile, openFile, openDiff }, + activeWorktreeId, worktreePath, entry, - options: { - targetGroupId, - preview: shouldOpenSourceControlRowAsPreview(event, targetGroupId) - } - }) + { targetGroupId, preview: openAsPreview } + ) }, - [activeWorktreeId, worktreePath, resolveSplitTargetGroupId] + [ + activeWorktreeId, + worktreePath, + resolveSplitTargetGroupId, + trackConflictPath, + openConflictFile, + openDiff, + openFile + ] ) - const openBranchFile = useCallback( + const openCommittedDiff = useCallback( (entry: GitBranchChangeEntry, event?: SourceControlRowOpenEvent) => { if ( !activeWorktreeId || @@ -93,20 +118,17 @@ export function useSourceControlRowOpening({ return } const targetGroupId = resolveSplitTargetGroupId(event) - openWorkspaceFile({ - kind: 'branch', - worktreeId: activeWorktreeId, + openBranchDiff( + activeWorktreeId, worktreePath, entry, - compare: branchSummary, - options: { - targetGroupId, - preview: shouldOpenSourceControlRowAsPreview(event, targetGroupId) - } - }) + branchSummary, + detectLanguage(entry.path), + { targetGroupId, preview: shouldOpenSourceControlRowAsPreview(event, targetGroupId) } + ) }, - [activeWorktreeId, branchSummary, resolveSplitTargetGroupId, worktreePath] + [activeWorktreeId, branchSummary, openBranchDiff, resolveSplitTargetGroupId, worktreePath] ) - return { resolveSplitTargetGroupId, activeOpenRowKeys, openUncommittedFile, openBranchFile } + return { resolveSplitTargetGroupId, activeOpenRowKeys, handleOpenDiff, openCommittedDiff } } diff --git a/src/renderer/src/components/right-sidebar/source-control/listing/use-selection.ts b/src/renderer/src/components/right-sidebar/source-control/listing/use-selection.ts index 4d202288d00..3c493a1b5e4 100644 --- a/src/renderer/src/components/right-sidebar/source-control/listing/use-selection.ts +++ b/src/renderer/src/components/right-sidebar/source-control/listing/use-selection.ts @@ -62,12 +62,12 @@ export function getSelectionRangeKeys( export function useSourceControlSelection({ flatEntries, - onOpenFile, + onOpenDiff, shouldOpenAsSplit, containerRef }: { flatEntries: FlatEntry[] - onOpenFile: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void + onOpenDiff: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void shouldOpenAsSplit?: (event: SourceControlRowOpenEvent) => boolean containerRef: RefObject }) { @@ -76,7 +76,7 @@ export function useSourceControlSelection({ const flatEntriesRef = useRef(flatEntries) const anchorKeyRef = useRef(anchorKey) const selectedKeysRef = useRef(selectedKeys) - const onOpenFileRef = useRef(onOpenFile) + const onOpenDiffRef = useRef(onOpenDiff) const shouldOpenAsSplitRef = useRef(shouldOpenAsSplit) useEffect(() => { @@ -92,8 +92,8 @@ export function useSourceControlSelection({ }, [selectedKeys]) useEffect(() => { - onOpenFileRef.current = onOpenFile - }, [onOpenFile]) + onOpenDiffRef.current = onOpenDiff + }, [onOpenDiff]) useEffect(() => { shouldOpenAsSplitRef.current = shouldOpenAsSplit @@ -118,7 +118,7 @@ export function useSourceControlSelection({ if (shouldOpenAsSplitRef.current?.(e)) { setSelectedKeys((prev) => (prev.size > 0 ? new Set() : prev)) setAnchorKey(null) - onOpenFileRef.current(entry, e) + onOpenDiffRef.current(entry, e) return } @@ -138,7 +138,7 @@ export function useSourceControlSelection({ // stale invisible anchor. setSelectedKeys(new Set()) setAnchorKey(key) - onOpenFileRef.current(entry) + onOpenDiffRef.current(entry) } else if (isCmdOrCtrl) { if (selectedKeysRef.current.has(key)) { setSelectedKeys((prev) => { @@ -163,7 +163,7 @@ export function useSourceControlSelection({ return prev }) setAnchorKey(key) - onOpenFileRef.current(entry) + onOpenDiffRef.current(entry) } }, []) diff --git a/src/renderer/src/components/right-sidebar/source-control/notes/use-note-opening.ts b/src/renderer/src/components/right-sidebar/source-control/notes/use-note-opening.ts index 396ef70ae5d..ef616d4b7a7 100644 --- a/src/renderer/src/components/right-sidebar/source-control/notes/use-note-opening.ts +++ b/src/renderer/src/components/right-sidebar/source-control/notes/use-note-opening.ts @@ -9,7 +9,6 @@ import type { } from '../../../../../../shared/git-diff-compare-types' import type { DiffComment } from '../../../../../../shared/diff-comment-types' import type { GitStatusEntry } from '../../../../../../shared/git-status-types' -import { openWorkspaceFile } from '@/lib/workspace-file-open' import { cancelSourceControlEditorRevealFrames, requestSourceControlEditorRevealFrame @@ -26,6 +25,8 @@ export function useSourceControlNoteOpening({ entries, branchEntries, branchSummary, + handleOpenDiff, + openCommittedDiff, sourceControlRef }: { activeWorktreeId: string | null @@ -33,6 +34,8 @@ export function useSourceControlNoteOpening({ entries: GitStatusEntry[] branchEntries: GitBranchChangeEntry[] branchSummary: GitBranchCompareSummary | null + handleOpenDiff: (entry: GitStatusEntry) => void + openCommittedDiff: (entry: GitBranchChangeEntry) => void sourceControlRef: RefObject }): SourceControlNoteOpening { const openFile = useAppStore((s) => s.openFile) @@ -95,14 +98,7 @@ export function useSourceControlNoteOpening({ matches.find((e) => e.area === 'untracked') ?? matches[0] if (uncommitted) { - openWorkspaceFile({ - kind: 'uncommitted', - intent: 'diff', - worktreeId: activeWorktreeId, - worktreePath, - entry: uncommitted, - options: { preview: true } - }) + handleOpenDiff(uncommitted) if (commentId) { setScrollToDiffCommentId(commentId) } @@ -110,15 +106,7 @@ export function useSourceControlNoteOpening({ } const branchEntry = branchEntries.find((e) => e.path === filePath) if (branchEntry && branchSummary?.status === 'ready') { - openWorkspaceFile({ - kind: 'branch', - intent: 'diff', - worktreeId: activeWorktreeId, - worktreePath, - entry: branchEntry, - compare: branchSummary, - options: { preview: true } - }) + openCommittedDiff(branchEntry) if (commentId) { setScrollToDiffCommentId(commentId) } @@ -144,6 +132,8 @@ export function useSourceControlNoteOpening({ branchEntries, branchSummary, entries, + handleOpenDiff, + openCommittedDiff, openFile, setEditorViewMode, setScrollToDiffCommentId, diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx b/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx index 6ca79cc432e..569edeebc9f 100644 --- a/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/panel/panel-content.tsx @@ -31,7 +31,7 @@ export function SourceControlPanelContent(props: SourceControlPanelReadyProps) { handleAbortOperationForConflict, handleCommitAction, handleContextMenu, - openUncommittedFile, + handleOpenDiff, handleResolveConflictsWithAI, handleSelect, handleStage, @@ -50,7 +50,7 @@ export function SourceControlPanelContent(props: SourceControlPanelReadyProps) { openAllDiffs, openBranchAllDiffs, openCommitFile, - openBranchFile, + openCommittedDiff, openConflictReview, openHistoryCommitDiff, refreshActiveGitStatus, @@ -167,7 +167,7 @@ export function SourceControlPanelContent(props: SourceControlPanelReadyProps) { handleContextMenu={handleContextMenu} revealInExplorer={revealInExplorer} activeConnectionId={activeConnectionId} - openUncommittedFile={openUncommittedFile} + handleOpenDiff={handleOpenDiff} handleStage={handleStage} handleUnstage={handleUnstage} requestDiscardEntry={requestDiscardEntry} @@ -204,7 +204,7 @@ export function SourceControlPanelContent(props: SourceControlPanelReadyProps) { worktreePath={worktreePath} revealInExplorer={revealInExplorer} activeConnectionId={activeConnectionId} - openBranchFile={openBranchFile} + openCommittedDiff={openCommittedDiff} openBranchAllDiffs={openBranchAllDiffs} diffCommentCountByPath={diffCommentCountByPath} /> diff --git a/src/renderer/src/components/right-sidebar/source-control/panel/use-panel-model.ts b/src/renderer/src/components/right-sidebar/source-control/panel/use-panel-model.ts index ec59f52c9a2..24980954fed 100644 --- a/src/renderer/src/components/right-sidebar/source-control/panel/use-panel-model.ts +++ b/src/renderer/src/components/right-sidebar/source-control/panel/use-panel-model.ts @@ -40,6 +40,7 @@ export function useSourceControlPanelModel() { entries, fetchUpstreamStatus, grouped, + handleOpenDiff, handleStageAllPrimary, hostedReview, hostedReviewCreateCopy, @@ -58,6 +59,7 @@ export function useSourceControlPanelModel() { isHostedReviewStateLoading, isRemoteOperationActive, openModal, + openCommittedDiff, refreshActiveGitStatusAfterMutation, remoteStatus, remoteStatusForActions, @@ -153,6 +155,8 @@ export function useSourceControlPanelModel() { entries, branchEntries, branchSummary, + handleOpenDiff, + openCommittedDiff, sourceControlRef }) const entryMutations = useSourceControlEntryMutations({ diff --git a/src/renderer/src/components/right-sidebar/use-file-explorer-tree-pane-state.ts b/src/renderer/src/components/right-sidebar/use-file-explorer-tree-pane-state.ts index abf594a6d65..b82a9764387 100644 --- a/src/renderer/src/components/right-sidebar/use-file-explorer-tree-pane-state.ts +++ b/src/renderer/src/components/right-sidebar/use-file-explorer-tree-pane-state.ts @@ -3,7 +3,6 @@ import { useEffect, useMemo, useRef } from 'react' import { useAppStore } from '@/store' import { runtimeEnvironmentIdOrLocalUnlessTwin } from '@/lib/worktree-runtime-owner' import { useWorkspaceFileBrowserActionPredicate } from '@/lib/file-preview' -import { openWorkspaceFile } from '@/lib/workspace-file-open' import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client-types' import type { OpenFile } from '@/store/slices/editor' import type { Repo } from '../../../../shared/repo-types' @@ -133,6 +132,7 @@ export function useFileExplorerTreePaneState({ runtimeEnvironmentIdOrLocalUnlessTwin(s, activeWorktreeId) ) const toggleDir = useAppStore((s) => s.toggleDir) + const openFile = useAppStore((s) => s.openFile) const makePreviewFilePermanent = useAppStore((s) => s.makePreviewFilePermanent) const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) const closeFile = useAppStore((s) => s.closeFile) @@ -251,7 +251,7 @@ export function useFileExplorerTreePaneState({ const handlers = useFileExplorerHandlers({ activeWorktreeId, runtimeEnvironmentId: ownerRuntimeEnvironmentId, - openWorkspaceFile, + openFile, makePreviewFilePermanent, toggleDir: hasNameFilter ? handleToggleNameFilterDir : toggleDir, loadDir, diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.test.ts b/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.test.ts index aa43a7c6684..cba904d8097 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.test.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.test.ts @@ -35,7 +35,7 @@ describe('activateFileExplorerNode', () => { await activateFileExplorerNode({ node: directoryNode, activeWorktreeId: 'wt-1', - openWorkspaceFile: vi.fn(), + openFile: vi.fn(), toggleDir, canToggleDirectories: false, loadDir: vi.fn(), @@ -52,12 +52,12 @@ describe('activateFileExplorerNode', () => { const loadDir = vi.fn().mockResolvedValue(true) const markPathAsDirectory = vi.fn() const toggleDir = vi.fn() - const openWorkspaceFile = vi.fn() + const openFile = vi.fn() await activateFileExplorerNode({ node: symlinkNode, activeWorktreeId: 'wt-1', - openWorkspaceFile, + openFile, toggleDir, loadDir, statPath: vi.fn().mockResolvedValue({ isDirectory: true }), @@ -72,17 +72,17 @@ describe('activateFileExplorerNode', () => { }) expect(markPathAsDirectory).toHaveBeenCalledWith('/repo/linked-docs') expect(toggleDir).toHaveBeenCalledWith('wt-1', '/repo/linked-docs') - expect(openWorkspaceFile).not.toHaveBeenCalled() + expect(openFile).not.toHaveBeenCalled() }) it('does not follow a folder link that leads out of the project', async () => { const loadDir = vi.fn() - const openWorkspaceFile = vi.fn() + const openFile = vi.fn() await activateFileExplorerNode({ node: { ...symlinkNode, operationOwner: { kind: 'local' } }, activeWorktreeId: 'wt-1', - openWorkspaceFile, + openFile, toggleDir: vi.fn(), loadDir, statPath: vi.fn().mockResolvedValue({ isDirectory: true, escapesWorktree: true }), @@ -91,14 +91,14 @@ describe('activateFileExplorerNode', () => { }) expect(loadDir).not.toHaveBeenCalled() - expect(openWorkspaceFile).not.toHaveBeenCalled() + expect(openFile).not.toHaveBeenCalled() expect(toastError).toHaveBeenCalledWith( "This folder links outside the project, so it can't be opened here." ) }) it('opens a file link that leads out of the project by its absolute path', async () => { - const openWorkspaceFile = vi.fn() + const openFile = vi.fn() useAppStore.setState({ worktreesByRepo: { 'repo-1': [{ id: 'wt-1', repoId: 'repo-1', path: '/repo', hostId: 'local' } as never] @@ -109,7 +109,7 @@ describe('activateFileExplorerNode', () => { node: { ...symlinkNode, operationOwner: { kind: 'local' } }, activeWorktreeId: 'wt-1', runtimeEnvironmentId: null, - openWorkspaceFile, + openFile, toggleDir: vi.fn(), loadDir: vi.fn(), statPath: vi.fn().mockResolvedValue({ isDirectory: false, escapesWorktree: true }), @@ -118,21 +118,18 @@ describe('activateFileExplorerNode', () => { }) // The absolute relativePath marks the tab as user-named, which survives a restart. - expect(openWorkspaceFile).toHaveBeenCalledWith( + expect(openFile).toHaveBeenCalledWith( expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/repo/linked-docs', - relativePath: '/repo/linked-docs', - worktreeId: 'wt-1' - }), - options: expect.anything() - }) + filePath: '/repo/linked-docs', + relativePath: '/repo/linked-docs', + worktreeId: 'wt-1' + }), + expect.anything() ) }) it('opens a symlink as a file when target stat fails', async () => { - const openWorkspaceFile = vi.fn() + const openFile = vi.fn() useAppStore.setState({ worktreesByRepo: { 'repo-1': [ @@ -150,7 +147,7 @@ describe('activateFileExplorerNode', () => { node: symlinkNode, activeWorktreeId: 'wt-1', runtimeEnvironmentId: 'runtime-env-1', - openWorkspaceFile, + openFile, toggleDir: vi.fn(), loadDir: vi.fn(), statPath: vi.fn().mockRejectedValue(new Error('stat failed')), @@ -158,59 +155,56 @@ describe('activateFileExplorerNode', () => { setSelectedPath: vi.fn() }) - expect(openWorkspaceFile).toHaveBeenCalledWith( - expect.objectContaining({ - kind: 'file', - file: { - filePath: '/repo/linked-docs', - relativePath: 'linked-docs', - worktreeId: 'wt-1', - runtimeEnvironmentId: 'runtime-env-1', - language: expect.any(String), - mode: 'edit' - }, - options: { preview: true, focusEditor: true, suppressActiveRuntimeFallback: false } - }) + expect(openFile).toHaveBeenCalledWith( + { + filePath: '/repo/linked-docs', + relativePath: 'linked-docs', + worktreeId: 'wt-1', + runtimeEnvironmentId: 'runtime-env-1', + language: expect.any(String), + mode: 'edit' + }, + { preview: true, focusEditor: true, suppressActiveRuntimeFallback: false } ) }) - it('opens local files without runtime fallback when no runtime owner is set', async () => { - const fileNode: TreeNode = { - name: 'README.md', - path: '/repo/README.md', - relativePath: 'README.md', - isDirectory: false, - depth: 0, - operationOwner: { kind: 'local' } - } - const openWorkspaceFile = vi.fn() - useAppStore.setState({ - worktreesByRepo: { - 'repo-1': [{ id: 'wt-1', repoId: 'repo-1', path: '/repo', hostId: 'local' } as never] + it.each(['README.md', 'report.html', 'REPORT.HTM'])( + 'opens %s in the editor without runtime fallback when no runtime owner is set', + async (name) => { + const fileNode: TreeNode = { + name, + path: `/repo/${name}`, + relativePath: name, + isDirectory: false, + depth: 0, + operationOwner: { kind: 'local' } } - }) + const openFile = vi.fn() + useAppStore.setState({ + worktreesByRepo: { + 'repo-1': [{ id: 'wt-1', repoId: 'repo-1', path: '/repo', hostId: 'local' } as never] + } + }) - await activateFileExplorerNode({ - node: fileNode, - activeWorktreeId: 'wt-1', - runtimeEnvironmentId: null, - openWorkspaceFile, - toggleDir: vi.fn(), - loadDir: vi.fn(), - statPath: vi.fn(), - markPathAsDirectory: vi.fn(), - setSelectedPath: vi.fn() - }) + await activateFileExplorerNode({ + node: fileNode, + activeWorktreeId: 'wt-1', + runtimeEnvironmentId: null, + openFile, + toggleDir: vi.fn(), + loadDir: vi.fn(), + statPath: vi.fn(), + markPathAsDirectory: vi.fn(), + setSelectedPath: vi.fn() + }) - expect(openWorkspaceFile).toHaveBeenCalledWith( - expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/repo/README.md', + expect(openFile).toHaveBeenCalledWith( + expect.objectContaining({ + filePath: `/repo/${name}`, runtimeEnvironmentId: undefined }), - options: { preview: true, focusEditor: true, suppressActiveRuntimeFallback: true } - }) - ) - }) + { preview: true, focusEditor: true, suppressActiveRuntimeFallback: true } + ) + } + ) }) diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.ts b/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.ts index 8e64f0dae01..5ae97cf1248 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerHandlers.ts @@ -12,12 +12,25 @@ import { requireMatchingFileExplorerOperationRoute } from './file-explorer-operation-owner' import { runtimeTargetEnvironmentId } from '@/runtime/runtime-client-target' -import type { WorkspaceFileOpener } from '@/lib/workspace-file-open' type UseFileExplorerHandlersParams = { activeWorktreeId: string | null runtimeEnvironmentId?: string | null - openWorkspaceFile: WorkspaceFileOpener + openFile: ( + params: { + filePath: string + relativePath: string + worktreeId: string + language: string + mode: 'edit' + runtimeEnvironmentId?: string | null + }, + options?: { + preview?: boolean + suppressActiveRuntimeFallback?: boolean + focusEditor?: boolean + } + ) => void makePreviewFilePermanent: (filePath: string) => void toggleDir: (worktreeId: string, dirPath: string) => void canToggleDirectories?: boolean @@ -38,11 +51,14 @@ type UseFileExplorerHandlersReturn = { handleWheelCapture: (e: React.WheelEvent) => void } +type OpenFileParams = Parameters[0] +type OpenFileOptions = Parameters[1] + export async function activateFileExplorerNode(args: { node: TreeNode activeWorktreeId: string | null runtimeEnvironmentId?: string | null - openWorkspaceFile: WorkspaceFileOpener + openFile: (params: OpenFileParams, options?: OpenFileOptions) => void toggleDir: (worktreeId: string, dirPath: string) => void canToggleDirectories?: boolean loadDir: UseFileExplorerHandlersParams['loadDir'] @@ -53,7 +69,7 @@ export async function activateFileExplorerNode(args: { const { node, activeWorktreeId, - openWorkspaceFile, + openFile, toggleDir, canToggleDirectories = true, loadDir, @@ -123,9 +139,8 @@ export async function activateFileExplorerNode(args: { toast.error(getFileExplorerOwnerUnresolvedMessage()) return } - openWorkspaceFile({ - kind: 'file', - file: { + openFile( + { filePath: node.path, // Why: a file link out of the project opens by its absolute path, as a file the user named, // so it reads the same before and after a restart instead of being refused as a project file. @@ -135,7 +150,7 @@ export async function activateFileExplorerNode(args: { language: detectLanguage(node.name), mode: 'edit' }, - options: { + { preview: true, // Why: activating an Explorer file is a focus handoff even if the rich // editor finishes mounting after the row receives browser focus. @@ -144,13 +159,13 @@ export async function activateFileExplorerNode(args: { // encode "no runtime owner" via the fallback-suppression option. suppressActiveRuntimeFallback: fileRuntimeEnvironmentId === null } - }) + ) } export function useFileExplorerHandlers({ activeWorktreeId, runtimeEnvironmentId, - openWorkspaceFile, + openFile, makePreviewFilePermanent, toggleDir, canToggleDirectories = true, @@ -172,7 +187,7 @@ export function useFileExplorerHandlers({ node, activeWorktreeId, runtimeEnvironmentId, - openWorkspaceFile, + openFile, toggleDir, canToggleDirectories, loadDir, @@ -187,7 +202,7 @@ export function useFileExplorerHandlers({ canToggleDirectories, loadDir, markPathAsDirectory, - openWorkspaceFile, + openFile, statPath, toggleDir, setSelectedPath diff --git a/src/renderer/src/components/status-bar/workspace-space-open-change.test.ts b/src/renderer/src/components/status-bar/workspace-space-open-change.test.ts index 879d1c5d123..827ae0dad17 100644 --- a/src/renderer/src/components/status-bar/workspace-space-open-change.test.ts +++ b/src/renderer/src/components/status-bar/workspace-space-open-change.test.ts @@ -21,8 +21,8 @@ vi.mock('@/store', () => ({ useAppStore: { getState: () => mocks.state } })) vi.mock('@/store/selectors', () => ({ getWorktreeOnHostFromState: mocks.getWorktree })) vi.mock('@/lib/worktree-activation', () => ({ activateAndRevealWorktree: mocks.activate })) vi.mock('@/runtime/runtime-git-status-client', () => ({ getRuntimeGitStatus: mocks.gitStatus })) -vi.mock('@/lib/workspace-file-open', () => ({ - openWorkspaceFile: mocks.openEntry +vi.mock('../right-sidebar/source-control/listing/open-uncommitted-entry', () => ({ + openUncommittedEntry: mocks.openEntry })) vi.mock('sonner', () => ({ toast: { error: mocks.error } })) @@ -61,14 +61,13 @@ describe('Space change inspection', () => { providesInitialSurface: true, executionHostId: hostId }) - expect(mocks.openEntry).toHaveBeenCalledWith({ - kind: 'uncommitted', - intent: 'diff', - worktreeId: 'same-id', - worktreePath: '/server/workspace', + expect(mocks.openEntry).toHaveBeenCalledWith( + mocks.state, + 'same-id', + '/server/workspace', entry, - options: { preview: true, runtimeEnvironmentId } - }) + { preview: true, runtimeEnvironmentId } + ) } ) @@ -89,7 +88,7 @@ describe('Space change inspection', () => { const conflict = { ...entry, conflictKind: 'both_deleted', conflictStatus: 'unresolved' } mocks.gitStatus.mockResolvedValue({ entries: [conflict] }) await revealWorkspaceSpaceChange('same-id', 'local', entry) - expect(mocks.openEntry.mock.calls[0][0].entry).toBe(conflict) + expect(mocks.openEntry.mock.calls[0][3]).toBe(conflict) }) it('reports a stale change without navigating or opening a file', async () => { diff --git a/src/renderer/src/components/status-bar/workspace-space-open-change.ts b/src/renderer/src/components/status-bar/workspace-space-open-change.ts index 7b3ae7ce788..289e25ea3f5 100644 --- a/src/renderer/src/components/status-bar/workspace-space-open-change.ts +++ b/src/renderer/src/components/status-bar/workspace-space-open-change.ts @@ -9,7 +9,7 @@ import { repoMatchesHostIdentity } from '@/store/slices/repo-host-identity' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { getRuntimeGitStatus } from '@/runtime/runtime-git-status-client' import { ipcOrRuntimeTargetForHostId } from '@/runtime/runtime-client-target' -import { openWorkspaceFile } from '@/lib/workspace-file-open' +import { openUncommittedEntry } from '../right-sidebar/source-control/listing/open-uncommitted-entry' import { translate } from '@/i18n/i18n' export function openWorkspaceSpaceChange( @@ -77,16 +77,9 @@ export async function revealWorkspaceSpaceChange( if (activated === false) { return } - openWorkspaceFile({ - kind: 'uncommitted', - intent: 'diff', - worktreeId, - worktreePath: worktree.path, - entry, - options: { - preview: true, - runtimeEnvironmentId: target.kind === 'environment' ? target.environmentId : null - } + openUncommittedEntry(useAppStore.getState(), worktreeId, worktree.path, entry, { + preview: true, + runtimeEnvironmentId: target.kind === 'environment' ? target.environmentId : null }) } catch (error) { toast.error(translate('workspaceSpace.changes.openFailed', 'Could not open changes'), { diff --git a/src/renderer/src/components/tab-bar/tab-create-entry-absolute-file.ts b/src/renderer/src/components/tab-bar/tab-create-entry-absolute-file.ts index f1be7eed856..7755c1f1303 100644 --- a/src/renderer/src/components/tab-bar/tab-create-entry-absolute-file.ts +++ b/src/renderer/src/components/tab-bar/tab-create-entry-absolute-file.ts @@ -1,16 +1,19 @@ import { detectLanguage } from '@/lib/language-detect' import { toWorktreeRelativePath } from '@/lib/terminal-links' import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client-types' +import type { OpenFile } from '@/store/slices/editor' import { validateNewTabEntryAbsolutePath, type TabEntryLocalPlatform } from './tab-create-entry-path-validation' import type { statUserOpenedPath, UserOpenedPathStat } from '@/lib/user-opened-local-path' -import type { openWorkspaceFile } from '@/lib/workspace-file-open' type AbsoluteFileOperations = { assertAbsolutePathAllowed: () => void - openWorkspaceFile: typeof openWorkspaceFile + openFile: ( + file: Omit, + options?: { preview?: boolean; targetGroupId?: string } + ) => void statUserOpenedPath: typeof statUserOpenedPath } @@ -36,9 +39,8 @@ export async function openAbsoluteTabEntryFile(args: { } args.operations.assertAbsolutePathAllowed() - args.operations.openWorkspaceFile({ - kind: 'file', - file: { + args.operations.openFile( + { filePath, // Why: a project link out of the project keeps its absolute path, so it reads as user-named. relativePath: stat.escapesWorktree @@ -48,6 +50,6 @@ export async function openAbsoluteTabEntryFile(args: { language: detectLanguage(filePath), mode: 'edit' }, - options: { preview: false, targetGroupId: args.groupId } - }) + { preview: false, targetGroupId: args.groupId } + ) } diff --git a/src/renderer/src/components/tab-bar/tab-create-entry-action.test.ts b/src/renderer/src/components/tab-bar/tab-create-entry-action.test.ts index 868f22976ab..dbecfa265dd 100644 --- a/src/renderer/src/components/tab-bar/tab-create-entry-action.test.ts +++ b/src/renderer/src/components/tab-bar/tab-create-entry-action.test.ts @@ -27,7 +27,8 @@ describe('openTabEntryWithOperations', () => { return { createRuntimePath: vi.fn().mockResolvedValue(undefined), openWorkspaceBrowserTab: vi.fn().mockResolvedValue(undefined), - openWorkspaceFile: vi.fn(), + openHtmlFilePreview: vi.fn(), + openFile: vi.fn(), statRuntimePath: vi.fn().mockResolvedValue({ size: 1, isDirectory: false, mtime: 1 }), statUserOpenedPath: vi.fn().mockResolvedValue({ isDirectory: false, escapesWorktree: false }), assertAbsolutePathAllowed: vi.fn(), @@ -50,44 +51,6 @@ describe('openTabEntryWithOperations', () => { searchEngine: 'google' as const } - it.each(['report.html', '/repo/report.html'])( - 'requests the default file view for an existing HTML file: %s', - async (query) => { - const operations = makeOperations() - await openTabEntryWithOperations({ - ...baseArgs, - fileList: readyFiles(['report.html']), - query, - operations - }) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( - expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ filePath: '/repo/report.html' }), - options: { preview: false, targetGroupId: 'group-1' } - }) - ) - expect(operations.createRuntimePath).not.toHaveBeenCalled() - } - ) - - it('requests source for newly created HTML so it can be edited', async () => { - const operations = makeOperations() - await openTabEntryWithOperations({ ...baseArgs, query: 'new.html', operations }) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith({ - kind: 'file', - intent: 'source', - file: { - filePath: '/repo/new.html', - relativePath: 'new.html', - worktreeId: 'wt-1', - language: 'html', - mode: 'edit' - }, - options: { preview: false, targetGroupId: 'group-1' } - }) - }) - it.each([true, false])( 'previews HTML file URLs without creating files (local paths: %s)', async (allowAbsolutePaths) => { @@ -101,8 +64,7 @@ describe('openTabEntryWithOperations', () => { query: 'file:///repo/docs/report%20%E4%B8%AD.html', operations }) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith({ - kind: 'preview', + expect(operations.openHtmlFilePreview).toHaveBeenCalledWith({ filePath: '/repo/docs/report 中.html', worktreeId: 'wt-1', targetGroupId: 'group-1' @@ -121,7 +83,7 @@ describe('openTabEntryWithOperations', () => { await expect( openTabEntryWithOperations({ ...baseArgs, query: 'src/index.ts', operations }) ).rejects.toThrow('Cannot open a directory') - expect(operations.openWorkspaceFile).not.toHaveBeenCalled() + expect(operations.openFile).not.toHaveBeenCalled() }) it('creates new files and opens them in the target group', async () => { @@ -139,16 +101,13 @@ describe('openTabEntryWithOperations', () => { '/repo/docs/new.md', 'file' ) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( + expect(operations.openFile).toHaveBeenCalledWith( expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/repo/docs/new.md', - relativePath: 'docs/new.md', - worktreeId: 'wt-1' - }), - options: { preview: false, targetGroupId: 'group-1' } - }) + filePath: '/repo/docs/new.md', + relativePath: 'docs/new.md', + worktreeId: 'wt-1' + }), + { preview: false, targetGroupId: 'group-1' } ) }) @@ -168,12 +127,9 @@ describe('openTabEntryWithOperations', () => { }) expect(operations.createRuntimePath).not.toHaveBeenCalled() - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( - expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ relativePath: 'README.md' }), - options: { preview: false, targetGroupId: 'group-1' } - }) + expect(operations.openFile).toHaveBeenCalledWith( + expect.objectContaining({ relativePath: 'README.md' }), + { preview: false, targetGroupId: 'group-1' } ) }) @@ -204,15 +160,12 @@ describe('openTabEntryWithOperations', () => { '/repo/.tmp/direct-entry-validation/created.md', 'file' ) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( + expect(operations.openFile).toHaveBeenCalledWith( expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/repo/.tmp/direct-entry-validation/created.md', - relativePath: '.tmp/direct-entry-validation/created.md' - }), - options: { preview: false, targetGroupId: 'group-1' } - }) + filePath: '/repo/.tmp/direct-entry-validation/created.md', + relativePath: '.tmp/direct-entry-validation/created.md' + }), + { preview: false, targetGroupId: 'group-1' } ) }) @@ -236,7 +189,7 @@ describe('openTabEntryWithOperations', () => { '/repo/docs/new.md', 'file' ) - expect(operations.openWorkspaceFile).toHaveBeenCalled() + expect(operations.openFile).toHaveBeenCalled() }) it('rejects invalid new file paths before creating parent directories', async () => { @@ -247,7 +200,7 @@ describe('openTabEntryWithOperations', () => { ).rejects.toThrow('File paths cannot contain . or .. segments.') expect(operations.createRuntimePath).not.toHaveBeenCalled() - expect(operations.openWorkspaceFile).not.toHaveBeenCalled() + expect(operations.openFile).not.toHaveBeenCalled() }) it('stats and opens when create loses an EEXIST race to a file', async () => { @@ -261,7 +214,7 @@ describe('openTabEntryWithOperations', () => { await openTabEntryWithOperations({ ...baseArgs, query: 'docs/race.md', operations }) expect(operations.statRuntimePath).toHaveBeenCalledWith(runtimeContext, '/repo/docs/race.md') - expect(operations.openWorkspaceFile).toHaveBeenCalled() + expect(operations.openFile).toHaveBeenCalled() }) it('routes URL classifications through the workspace browser opener', async () => { @@ -349,16 +302,13 @@ describe('openTabEntryWithOperations', () => { }) expect(operations.statUserOpenedPath).toHaveBeenCalledWith(runtimeContext, '/tmp/notes.md') - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( + expect(operations.openFile).toHaveBeenCalledWith( expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/tmp/notes.md', - relativePath: '/tmp/notes.md', - worktreeId: 'wt-1' - }), - options: { preview: false, targetGroupId: 'group-1' } - }) + filePath: '/tmp/notes.md', + relativePath: '/tmp/notes.md', + worktreeId: 'wt-1' + }), + { preview: false, targetGroupId: 'group-1' } ) }) @@ -372,15 +322,12 @@ describe('openTabEntryWithOperations', () => { operations }) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( + expect(operations.openFile).toHaveBeenCalledWith( expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/repo/src/index.ts', - relativePath: 'src/index.ts' - }), - options: { preview: false, targetGroupId: 'group-1' } - }) + filePath: '/repo/src/index.ts', + relativePath: 'src/index.ts' + }), + { preview: false, targetGroupId: 'group-1' } ) }) @@ -396,15 +343,12 @@ describe('openTabEntryWithOperations', () => { operations }) - expect(operations.openWorkspaceFile).toHaveBeenCalledWith( + expect(operations.openFile).toHaveBeenCalledWith( expect.objectContaining({ - kind: 'file', - file: expect.objectContaining({ - filePath: '/repo/docs-link/secret.md', - relativePath: '/repo/docs-link/secret.md' - }), - options: { preview: false, targetGroupId: 'group-1' } - }) + filePath: '/repo/docs-link/secret.md', + relativePath: '/repo/docs-link/secret.md' + }), + { preview: false, targetGroupId: 'group-1' } ) }) @@ -423,7 +367,7 @@ describe('openTabEntryWithOperations', () => { expect(operations.statUserOpenedPath).not.toHaveBeenCalled() expect(operations.createRuntimePath).not.toHaveBeenCalled() - expect(operations.openWorkspaceFile).not.toHaveBeenCalled() + expect(operations.openFile).not.toHaveBeenCalled() }) it('rejects Windows path syntax before touching a native POSIX path', async () => { @@ -439,7 +383,7 @@ describe('openTabEntryWithOperations', () => { ).rejects.toThrow('Enter an absolute path for this computer.') expect(operations.statUserOpenedPath).not.toHaveBeenCalled() - expect(operations.openWorkspaceFile).not.toHaveBeenCalled() + expect(operations.openFile).not.toHaveBeenCalled() }) it('normalizes and opens Windows drive paths on Windows', async () => { @@ -448,11 +392,7 @@ describe('openTabEntryWithOperations', () => { await openTabEntryWithOperations({ ...baseArgs, localPlatform: 'windows', - getFileContext: () => ({ - worktreePath: 'C:/repo', - runtimeContext, - assertCurrent: () => {} - }), + getFileContext: () => ({ worktreePath: 'C:/repo', runtimeContext, assertCurrent: () => {} }), classification: { kind: 'absolute-file', filePath: 'C:\\tmp\\notes.md' }, query: 'C:\\tmp\\notes.md', operations @@ -492,6 +432,6 @@ describe('openTabEntryWithOperations', () => { releaseStat?.() await rejection - expect(operations.openWorkspaceFile).not.toHaveBeenCalled() + expect(operations.openFile).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/tab-bar/tab-create-entry-action.ts b/src/renderer/src/components/tab-bar/tab-create-entry-action.ts index 9d32a8fae6b..7ffd8a41f02 100644 --- a/src/renderer/src/components/tab-bar/tab-create-entry-action.ts +++ b/src/renderer/src/components/tab-bar/tab-create-entry-action.ts @@ -5,17 +5,14 @@ import { createRuntimePath } from '@/runtime/runtime-file-mutation-client' import { statRuntimePath } from '@/runtime/runtime-file-metadata-client' import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client-types' import { useAppStore } from '@/store' +import type { OpenFile } from '@/store/slices/editor' import { - buildSearchUrl, DEFAULT_SEARCH_ENGINE, type SearchEngine, type SearchUrlOptions } from '../../../../shared/browser-url' import type { RuntimeFileListState } from '../quick-open-file-list' -import { - openWorkspaceBrowserTab, - type OpenWorkspaceBrowserTabRequest -} from '@/lib/workspace-browser-tab-open' +import { openWorkspaceBrowserTab } from '@/lib/workspace-browser-tab-open' import { classifyTabEntryQuery, TAB_ENTRY_ABSOLUTE_PATH_REMOTE_BLOCKED_MESSAGE, @@ -23,6 +20,7 @@ import { type TabEntryOptionsContext } from './tab-create-entry-classifier' import { openAbsoluteTabEntryFile } from './tab-create-entry-absolute-file' +import { getNetworkTabRequest, isNetworkTabEntry } from './tab-create-entry-browser-request' import { getTabEntryAllowAbsolutePaths, getTabEntryFileOperationContext @@ -46,7 +44,7 @@ export { isTabEntryAbsolutePathAllowed } from './tab-create-entry-local-path' import { statUserOpenedPath } from '@/lib/user-opened-local-path' -import { openWorkspaceFile } from '@/lib/workspace-file-open' +import { openHtmlFilePreview } from '@/lib/open-html-file-preview' import { assertEditorFileOperationCurrent, captureEditorFileOperationProvenance @@ -63,7 +61,11 @@ export type TabCreateEntryArgs = { export type TabEntryOperations = { createRuntimePath: typeof createRuntimePath openWorkspaceBrowserTab: typeof openWorkspaceBrowserTab - openWorkspaceFile: typeof openWorkspaceFile + openHtmlFilePreview: typeof openHtmlFilePreview + openFile: ( + file: Omit, + options?: { preview?: boolean; targetGroupId?: string } + ) => void statRuntimePath: typeof statRuntimePath statUserOpenedPath: typeof statUserOpenedPath assertAbsolutePathAllowed: () => void @@ -119,31 +121,36 @@ async function createParentDirectoriesForNewFile(args: { } } -type NetworkTabEntryClassification = Extract< - TabEntryActionClassification, - { kind: 'explicit-url' | 'host-url' | 'search' } -> - -function isNetworkTabEntry( - entry: TabEntryActionClassification -): entry is NetworkTabEntryClassification { - return entry.kind === 'explicit-url' || entry.kind === 'host-url' || entry.kind === 'search' -} - -function getNetworkTabRequest( - classification: NetworkTabEntryClassification, - { worktreeId, groupId }: Pick, - searchUrlOptions?: SearchUrlOptions -): OpenWorkspaceBrowserTabRequest { - const target = { workspaceId: worktreeId, targetGroupId: groupId } - if (classification.kind === 'search') { - return { - ...target, - url: buildSearchUrl(classification.query, classification.engine, searchUrlOptions), - intent: { kind: 'search', engine: classification.engine } - } +async function openExistingFile(args: { + assertCurrent: () => void + context: RuntimeFileOperationArgs + groupId: string + operations: TabEntryOperations + relativePath: string + worktreeId: string + worktreePath: string +}): Promise { + const filePath = joinPath(args.worktreePath, args.relativePath) + let stat: Awaited> + try { + stat = await args.operations.statRuntimePath(args.context, filePath) + } catch { + throw new Error(`File no longer exists: ${args.relativePath}`) } - return { ...target, url: classification.url, intent: { kind: 'url' } } + args.assertCurrent() + if (stat.isDirectory) { + throw new Error(`Cannot open a directory: ${args.relativePath}`) + } + args.operations.openFile( + { + filePath, + relativePath: args.relativePath, + worktreeId: args.worktreeId, + language: detectLanguage(args.relativePath), + mode: 'edit' + }, + { preview: false, targetGroupId: args.groupId } + ) } export async function openTabEntryWithOperations({ @@ -167,8 +174,7 @@ export async function openTabEntryWithOperations({ } if (classification.kind === 'html-file-url') { - operations.openWorkspaceFile({ - kind: 'preview', + operations.openHtmlFilePreview({ filePath: classification.filePath, worktreeId, targetGroupId: groupId @@ -201,43 +207,41 @@ export async function openTabEntryWithOperations({ return } - const filePath = joinPath(worktreePath, classification.relativePath) - if (classification.kind === 'new-file') { - try { - await createParentDirectoriesForNewFile({ - context: runtimeContext, - operations, - relativePath: classification.relativePath, - worktreePath - }) - await operations.createRuntimePath(runtimeContext, filePath, 'file') - } catch (error) { - if (!isExistsError(error)) { - throw error - } - } - } - let stat: Awaited> - try { - stat = await operations.statRuntimePath(runtimeContext, filePath) - } catch { - throw new Error(`File no longer exists: ${classification.relativePath}`) - } - assertCurrent() - if (stat.isDirectory) { - throw new Error(`Cannot open a directory: ${classification.relativePath}`) - } - operations.openWorkspaceFile({ - kind: 'file', - intent: classification.kind === 'new-file' ? 'source' : 'default', - file: { - filePath, + if (classification.kind === 'existing-file') { + await openExistingFile({ + assertCurrent, + context: runtimeContext, + groupId, + operations, relativePath: classification.relativePath, worktreeId, - language: detectLanguage(classification.relativePath), - mode: 'edit' - }, - options: { preview: false, targetGroupId: groupId } + worktreePath + }) + return + } + + const filePath = joinPath(worktreePath, classification.relativePath) + try { + await createParentDirectoriesForNewFile({ + context: runtimeContext, + operations, + relativePath: classification.relativePath, + worktreePath + }) + await operations.createRuntimePath(runtimeContext, filePath, 'file') + } catch (error) { + if (!isExistsError(error)) { + throw error + } + } + await openExistingFile({ + assertCurrent, + context: runtimeContext, + groupId, + operations, + relativePath: classification.relativePath, + worktreeId, + worktreePath }) } @@ -278,7 +282,8 @@ export async function openTabBarEntry(args: TabCreateEntryArgs): Promise { operations: { createRuntimePath, openWorkspaceBrowserTab, - openWorkspaceFile, + openHtmlFilePreview, + openFile: state.openFile, statRuntimePath, statUserOpenedPath, assertAbsolutePathAllowed: () => { diff --git a/src/renderer/src/components/tab-bar/tab-create-entry-browser-request.ts b/src/renderer/src/components/tab-bar/tab-create-entry-browser-request.ts new file mode 100644 index 00000000000..40c0c255904 --- /dev/null +++ b/src/renderer/src/components/tab-bar/tab-create-entry-browser-request.ts @@ -0,0 +1,34 @@ +import { buildSearchUrl, type SearchUrlOptions } from '../../../../shared/browser-url' +import type { OpenWorkspaceBrowserTabRequest } from '@/lib/workspace-browser-tab-open' +import type { TabEntryActionClassification } from './tab-create-entry-classifier' + +type NetworkTabEntryClassification = Extract< + TabEntryActionClassification, + { kind: 'explicit-url' | 'host-url' | 'search' } +> + +export function isNetworkTabEntry( + classification: TabEntryActionClassification +): classification is NetworkTabEntryClassification { + return ( + classification.kind === 'explicit-url' || + classification.kind === 'host-url' || + classification.kind === 'search' + ) +} + +export function getNetworkTabRequest( + classification: NetworkTabEntryClassification, + { worktreeId, groupId }: { worktreeId: string; groupId: string }, + searchUrlOptions?: SearchUrlOptions +): OpenWorkspaceBrowserTabRequest { + const target = { workspaceId: worktreeId, targetGroupId: groupId } + if (classification.kind === 'search') { + return { + ...target, + url: buildSearchUrl(classification.query, classification.engine, searchUrlOptions), + intent: { kind: 'search', engine: classification.engine } + } + } + return { ...target, url: classification.url, intent: { kind: 'url' } } +} diff --git a/src/renderer/src/components/tab-bar/tab-create-entry-local-path.test.ts b/src/renderer/src/components/tab-bar/tab-create-entry-local-path.test.ts index e5678e913de..266915ac8cf 100644 --- a/src/renderer/src/components/tab-bar/tab-create-entry-local-path.test.ts +++ b/src/renderer/src/components/tab-bar/tab-create-entry-local-path.test.ts @@ -67,43 +67,45 @@ describe('getTabEntryAllowAbsolutePaths', () => { useAppStore.setState(initialState, true) }) - it('refuses an HTML open if its workspace changes hosts during the file check', async () => { - const worktree = makeWorktree({ - id: localWorktreeId, - repoId: 'repo-local', - path: '/Users/me/repo', - hostId: 'local' - }) - useAppStore.setState({ - repos: [makeRepo({ id: 'repo-local', executionHostId: 'local' })], - worktreesByRepo: { 'repo-local': [worktree] }, - runtimeEnvironmentCatalogHydrated: true - }) - vi.spyOn(metadata, 'statRuntimePath').mockImplementationOnce(async () => { + it.each(['report.html', 'notes.txt'])( + 'refuses opening %s if its workspace changes hosts during the file check', + async (name) => { + const worktree = makeWorktree({ + id: localWorktreeId, + repoId: 'repo-local', + path: '/Users/me/repo', + hostId: 'local' + }) useAppStore.setState({ - repos: [makeRepo({ id: 'repo-local', executionHostId: 'runtime:hub-a' })], - worktreesByRepo: { - 'repo-local': [ - { ...worktree, hostId: 'runtime:hub-a', runtimeOwnerEnvironmentId: 'hub-a' } - ] - } + repos: [makeRepo({ id: 'repo-local', executionHostId: 'local' })], + worktreesByRepo: { 'repo-local': [worktree] }, + runtimeEnvironmentCatalogHydrated: true }) - return { size: 1, isDirectory: false, mtime: 1 } - }) - - await expect( - openTabBarEntry({ - query: 'report.html', - worktreeId: localWorktreeId, - groupId: 'group-local', - fileList: { files: ['report.html'], loading: false, loadError: null } + vi.spyOn(metadata, 'statRuntimePath').mockImplementationOnce(async () => { + useAppStore.setState({ + repos: [makeRepo({ id: 'repo-local', executionHostId: 'runtime:hub-a' })], + worktreesByRepo: { + 'repo-local': [ + { ...worktree, hostId: 'runtime:hub-a', runtimeOwnerEnvironmentId: 'hub-a' } + ] + } + }) + return { size: 1, isDirectory: false, mtime: 1 } }) - ).rejects.toThrow("Couldn't verify which host owns this file") - expect(useAppStore.getState().browserTabsByWorktree[localWorktreeId] ?? []).toHaveLength(0) - expect(useAppStore.getState().openFiles).toHaveLength(0) - }) + await expect( + openTabBarEntry({ + query: name, + worktreeId: localWorktreeId, + groupId: 'group-local', + fileList: { files: [name], loading: false, loadError: null } + }) + ).rejects.toThrow("Couldn't verify which host owns this file") + expect(useAppStore.getState().browserTabsByWorktree[localWorktreeId] ?? []).toHaveLength(0) + expect(useAppStore.getState().openFiles).toHaveLength(0) + } + ) - it('previews an existing HTML file in a local folder workspace', async () => { + it('opens existing HTML as source in a local folder workspace', async () => { const folder = makeFolderWorkspace() const worktreeId = folderWorkspaceKey(folder.id) useAppStore.setState({ @@ -123,10 +125,17 @@ describe('getTabEntryAllowAbsolutePaths', () => { groupId: 'group-local', fileList: { files: ['report.html'], loading: false, loadError: null } }) - expect(useAppStore.getState().browserTabsByWorktree[worktreeId]).toEqual([ - expect.objectContaining({ url: 'file:///Users/me/folder/report.html' }) + + expect(useAppStore.getState().openFiles).toEqual([ + expect.objectContaining({ + filePath: '/Users/me/folder/report.html', + relativePath: 'report.html', + worktreeId, + language: 'html', + mode: 'edit' + }) ]) - expect(useAppStore.getState().openFiles).toHaveLength(0) + expect(useAppStore.getState().browserTabsByWorktree[worktreeId] ?? []).toHaveLength(0) }) it('allows absolute paths for a known local worktree', () => { diff --git a/src/renderer/src/components/tab-bar/tab-create-entry-preview.test.ts b/src/renderer/src/components/tab-bar/tab-create-entry-preview.test.ts index c7777802288..f1c443e2647 100644 --- a/src/renderer/src/components/tab-bar/tab-create-entry-preview.test.ts +++ b/src/renderer/src/components/tab-bar/tab-create-entry-preview.test.ts @@ -2,7 +2,7 @@ import { expect, it, vi } from 'vitest' import { openTabBarEntry } from './tab-create-entry-action' const preview = vi.hoisted(() => vi.fn()) -vi.mock('@/lib/workspace-file-open', () => ({ openWorkspaceFile: preview })) +vi.mock('@/lib/open-html-file-preview', () => ({ openHtmlFilePreview: preview })) vi.mock('@/lib/editor-file-operation-owner', () => ({ getEditorFileOperationContext: () => { throw new Error('A preview must not require file mutation authorization') @@ -29,7 +29,6 @@ it.each([true, false])( }) expect(preview).toHaveBeenCalledTimes(1) expect(preview).toHaveBeenCalledWith({ - kind: 'preview', filePath: '/repo/report.html', worktreeId: 'remote', targetGroupId: 'group-2' diff --git a/src/renderer/src/lib/editor-file-link-open.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-editor-open.ts similarity index 97% rename from src/renderer/src/lib/editor-file-link-open.ts rename to src/renderer/src/components/terminal-pane/terminal-file-link-editor-open.ts index 369216519c1..3e7663ede34 100644 --- a/src/renderer/src/lib/editor-file-link-open.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-editor-open.ts @@ -7,7 +7,7 @@ import type { EditorOpenLinkIntent, EditorOpenResult } from '@/store/slices/editor/types/editor-open-link-intent' -import type { ExecutionHostId } from '../../../shared/execution-host' +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( 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 761c0bd218e..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,11 +1,12 @@ -import { openWorkspaceFile } from '@/lib/workspace-file-open' +import { absolutePathToFileUri } from '@/components/editor/markdown-internal-links' +import { getWorkspaceFilePreviewPlan, openFileInBrowserTab } from '@/lib/file-preview' import { downloadAndOpenRemoteTerminalFile } from './terminal-remote-file-download-open' import { findWorkspaceFileRoute } from '@/lib/runtime-workspace-file-route' import { isPathInsideWorktree, toWorktreeRelativePath } from '@/lib/terminal-links' import { canClientOsOpenWorkspaceFile } from '@/lib/workspace-file-host-routing' import { isMissingRuntimePathError } from '@/runtime/runtime-file-metadata-client' import { useAppStore } from '@/store' -import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { activateAndRevealWorkspace, activateAndRevealWorktree } from '@/lib/worktree-activation' import { resolveKnownWorktreeRootPathLink } from './terminal-worktree-path-link' import { getTerminalFileContext, @@ -28,7 +29,7 @@ import { isFloatingWorkspaceId } from '../../../../shared/floating-workspace-wor 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 } from '@/lib/editor-file-link-open' +import { isExplicitEditorScopeKept, openLinkedFileInEditor } from './terminal-file-link-editor-open' export { getTerminalFileContext, @@ -61,6 +62,28 @@ type TerminalFileOpenDeps = { openInSourceGroup?: boolean } +export function isHtmlFilePath(filePath: string): boolean { + return /\.html?$/i.test(filePath) +} + +function openHtmlFileInBrowser(filePath: string, worktreeId: string, clientLocal: boolean): void { + const store = useAppStore.getState() + if (worktreeId) { + // Why: following an HTML file link changes which worktree is foregrounded, + // so it must record a history visit before opening the browser tab — but the + // browser tab is the surface, so an emptied workspace must not gain a shell. + activateAndRevealWorktree(worktreeId, { providesInitialSurface: true }) + } + const fileUrl = absolutePathToFileUri(filePath) + const title = filePath.split(/[/\\]/).pop() ?? filePath + store.createBrowserTab(worktreeId, fileUrl, { + title, + activate: true, + // Why: the file is on this computer, so the workspace's paired-server runtime must not adopt the page. + ...(clientLocal ? { browserRuntimeEnvironmentId: null } : {}) + }) +} + export function shouldOpenTerminalFileWithSystemDefault(fileContext: TerminalFileContext): boolean { // Why: an unresolved owner has no connectionId either, which would otherwise read as local. return fileContext.sourceHostResolved && canClientOsOpenWorkspaceFile(fileContext) @@ -213,6 +236,30 @@ export function openDetectedFilePath( return } + // Why: local HTML files render in Orca's browser for ordinary Cmd/Ctrl-click, + // 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 + } + // Why: the same gesture renders remote HTML too, through the doc preview; only an + // 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 + } + } + const store = useAppStore.getState() let targetWorktreeId = worktreeId const runtimeOwnerId = runtimeTargetEnvironmentId(fileContext.target) ?? sourceRuntimeOwnerId @@ -254,11 +301,17 @@ export function openDetectedFilePath( return } - const opened = openWorkspaceFile({ - kind: 'link', - executionHostId: targetExecutionHostId, - clientLocal, - file: { + 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, + executionHostId: targetExecutionHostId + }) + } + + const opened = openLinkedFileInEditor( + { filePath: mappedFilePath, relativePath, worktreeId: targetWorktreeId || '', @@ -276,10 +329,10 @@ export function openDetectedFilePath( : {}) }, linkIntent, - position: { line, column }, - isCurrentRequest: () => requestId === latestOpenDetectedFilePathRequestId - }) - if (opened.surface === 'refused') { + { line, column }, + () => requestId === latestOpenDetectedFilePathRequestId + ) + if (!opened) { refuseExplicitNonEditorRoute() } })() 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 91b89e8a09a..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 @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { activateAndRevealWorkspace } from '@/lib/worktree-activation' +import { activateAndRevealWorkspace, activateAndRevealWorktree } from '@/lib/worktree-activation' import { openDetectedFilePath } from './terminal-link-handlers' import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' import { @@ -67,9 +67,8 @@ describe('handleOscLink', () => { expect(openFilePathMock).not.toHaveBeenCalled() // Why: the browser tab is the surface — activation must not re-seed a shell into a // workspace whose last terminal the user closed. - expect(activateAndRevealWorkspace).toHaveBeenCalledWith('wt-1', { - providesInitialSurface: true, - executionHostId: 'local' + expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { + providesInitialSurface: true }) }) @@ -89,20 +88,6 @@ describe('handleOscLink', () => { expect(openFilePathMock).not.toHaveBeenCalled() }) - it('opens an HTML line link in source and reveals the requested line', async () => { - openDetectedFilePath('/tmp/report.html', 12, 3, deps) - await flushAsyncWork() - await flushDoubleRaf() - expect(createBrowserTabMock).not.toHaveBeenCalled() - expect(openFileMock).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '/tmp/report.html', mode: 'edit' }), - { forceContentReload: true } - ) - expect(setPendingEditorRevealMock).toHaveBeenLastCalledWith( - expect.objectContaining({ line: 12, column: 3, filePath: '/tmp/report.html' }) - ) - }) - it('opens local file paths in Orca and reveals default column 1 with the platform modifier', async () => { setPlatform('Macintosh') diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-fixtures.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-fixtures.ts index 57158e78b6a..47a1bc8d6f9 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,7 +1,6 @@ import { vi, type Mock } from 'vitest' import type { ExecutionHostId } from '../../../../shared/execution-host' import type { Tab, TabGroup } from '../../../../shared/tab-types' -import type { BrowserPage } from '../../../../shared/browser-workspace-types' export type TerminalLinkStoreSettings = { openLinksInApp?: boolean @@ -36,17 +35,6 @@ export type TerminalLinkStoreState = { }[] > folderWorkspaces: [] - repos: { - id: string - displayName: string - connectionId?: string | null - executionHostId?: ExecutionHostId - }[] - browserTabsByWorktree: Record - browserPagesByWorkspace: Record - getKnownWorktreeById: ( - id: string - ) => TerminalLinkStoreState['worktreesByRepo'][string][number] | undefined } export type TerminalLinkTestDoubles = { @@ -121,14 +109,7 @@ export function createTerminalLinkTestDoubles(): TerminalLinkTestDoubles { unifiedTabsByWorktree: {}, groupsByWorktree: {}, worktreesByRepo: {}, - folderWorkspaces: [], - repos: [], - browserTabsByWorktree: {}, - browserPagesByWorkspace: {}, - getKnownWorktreeById: (id) => - Object.values(storeState.worktreesByRepo) - .flat() - .find((worktree) => worktree.id === id) + folderWorkspaces: [] } return { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-harness.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers-test-harness.ts index e953de7e405..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 @@ -60,14 +60,7 @@ export function installTerminalLinkTestEnvironment(doubles: TerminalLinkTestDoub storeState.openFiles = [] storeState.unifiedTabsByWorktree = {} storeState.groupsByWorktree = {} - storeState.worktreesByRepo = { - 'repo-1': [{ id: 'wt-1', repoId: 'repo-1', path: '/tmp', hostId: 'local' }] - } - storeState.repos = [ - { id: 'repo-1', displayName: 'Repo', connectionId: null, executionHostId: 'local' } - ] - storeState.browserTabsByWorktree = {} - storeState.browserPagesByWorkspace = {} + storeState.worktreesByRepo = {} registerHttpLinkStoreAccessor(() => storeState) vi.stubGlobal('window', { dispatchEvent: vi.fn(), 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 8ed0886f564..1dad5737515 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -8,8 +8,10 @@ import { buildCandidateLogicalLinesForBufferPosition, openFilePathLinkAtBufferPosition } from './terminal-file-link-hit-testing' -import { isHtmlFilePath } from '../../../../shared/html-file-url' -import { shouldOpenTerminalFileWithSystemDefault } from './terminal-file-open-routing' +import { + isHtmlFilePath, + shouldOpenTerminalFileWithSystemDefault +} from './terminal-file-open-routing' import { fileLinkTargetExists, resolveFileLinkTarget } from './terminal-file-link-target' import { buildHardWrappedPathLogicalLineCandidates, 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 index 8a839570cdf..a4da27e7050 100644 --- 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 @@ -32,6 +32,10 @@ vi.mock('@/store', () => ({ vi.mock('sonner', () => ({ toast: { error: toastErrorMock } })) +vi.mock('@/lib/language-detect', () => ({ + detectLanguage: () => 'plaintext' +})) + vi.mock('@/lib/worktree-activation', () => ({ activateAndRevealWorkspace: vi.fn(), activateAndRevealWorktree: vi.fn() @@ -106,7 +110,7 @@ describe('terminal file links carry their source tab', () => { ) }) - it('opens HTML source in the explicitly requested source group', async () => { + it('refuses an explicit request that resolves to Orca browser HTML', async () => { setPlatform('Macintosh') seedSourceTab() @@ -114,17 +118,8 @@ describe('terminal file links carry their source tab', () => { await flushAsyncWork() expect(createBrowserTabMock).not.toHaveBeenCalled() - expect(openFileForLinkMock).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '/tmp/report.html', language: 'html', mode: 'edit' }), - { forceContentReload: true }, - expect.objectContaining({ - workspaceId: 'wt-1', - hostId: null, - targetGroupId: 'group-a' - }), - expect.any(Function) - ) - expect(toastErrorMock).not.toHaveBeenCalled() + expect(openFileForLinkMock).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledTimes(1) }) it('refuses a closed explicit group before activating its workspace', async () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts index 2bc01752026..524f04b119e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-remote-runtime-ssh-open.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { openDetectedFilePath } from './terminal-link-handlers' import { getConnectionId } from '@/lib/connection-context' import { getWorkspaceFilePreviewPlan, openFileInBrowserTab } from '@/lib/file-preview' -import { activateAndRevealWorkspace } from '@/lib/worktree-activation' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { downloadAndOpenRemoteTerminalFile } from './terminal-remote-file-download-open' import { createTerminalLinkTestDoubles } from './terminal-link-handlers-test-fixtures' import { @@ -47,7 +47,7 @@ vi.mock('./terminal-remote-file-download-open', () => ({ vi.mock('@/lib/file-preview', () => ({ getWorkspaceFilePreviewPlan: vi.fn(() => ({ status: 'doc-preview' })), - openFileInBrowserTab: vi.fn(() => ({ status: 'doc-preview' })) + openFileInBrowserTab: vi.fn() })) installTerminalLinkTestEnvironment(doubles) @@ -254,9 +254,8 @@ describe('handleOscLink', () => { }) // Why: the preview tab is the surface — activation must not re-seed a shell into a // workspace whose last terminal the user closed. - expect(activateAndRevealWorkspace).toHaveBeenCalledWith('wt-1', { - providesInitialSurface: true, - executionHostId: 'ssh:ssh-1' + expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-1', { + providesInitialSurface: true }) }) diff --git a/src/renderer/src/lib/file-preview-tab.ts b/src/renderer/src/lib/file-preview-tab.ts deleted file mode 100644 index 2fc8986829e..00000000000 --- a/src/renderer/src/lib/file-preview-tab.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { basename } from '@/lib/path' -import { activateBrowserWorkspaceTab } from '@/lib/browser-workspace-tab-activation' -import { runtimeEnvironmentIdUnlessTwin } from '@/lib/resolve-owner' -import type { AppState } from '@/store/types' -import type { CreateBrowserTabOptions } from '@/store/slices/browser/browser-slice-contract' -import { browserPageDocLocationsEqual } from '../../../shared/browser-page-doc-location' -import type { BrowserPageDocLocation } from '../../../shared/browser-workspace-types' -import { ORCA_BROWSER_BLANK_URL } from '../../../shared/constants' -import { findWorkspaceShowingPage } from './workspace-file-preview-page' -import { getBrowserPageRuntimeEnvironmentId } from '@/components/browser-pane/describe-page/browser-page-url-display' -import { refreshLocalHtmlPreview } from '@/components/browser-pane/navigate/local-html-preview-freshness' -import { webviewRegistry } from '@/components/browser-pane/host-guest/webview-registry' -import type { WorkspaceFilePreviewPlan } from './file-preview' - -export function openFilePreviewTab( - state: AppState, - params: { filePath: string; worktreeId: string; activate: boolean; clientLocal?: boolean }, - plan: Exclude, - placement?: Pick -): void { - const docLocation: BrowserPageDocLocation | null = - plan.status === 'doc-preview' - ? { kind: 'workspace-doc', worktreeId: params.worktreeId, filePath: params.filePath } - : null - const inferredRuntimeEnvironmentId = runtimeEnvironmentIdUnlessTwin(state, params.worktreeId) - const previewRuntimeEnvironmentId = params.clientLocal ? null : inferredRuntimeEnvironmentId - const existing = findWorkspaceShowingPage( - state, - params.worktreeId, - (page) => - plan.status === 'browser-tab' - ? !page.docLocation && - page.url === plan.url && - getBrowserPageRuntimeEnvironmentId(page, inferredRuntimeEnvironmentId) === - previewRuntimeEnvironmentId - : browserPageDocLocationsEqual(page.docLocation ?? null, docLocation), - placement?.targetGroupId - ) - if (existing) { - const webview = plan.status === 'browser-tab' ? webviewRegistry.get(existing.pageId) : null - if (webview) { - void refreshLocalHtmlPreview(webview, params.filePath) - } - if ( - !params.activate || - !activateBrowserWorkspaceTab({ - worktreeId: params.worktreeId, - ...existing - }) - ) { - state.setActiveBrowserTab(existing.workspaceId) - state.setActiveBrowserPage(existing.workspaceId, existing.pageId) - } - return - } - state.createBrowserTab( - params.worktreeId, - plan.status === 'browser-tab' ? plan.url : ORCA_BROWSER_BLANK_URL, - { - title: - plan.status === 'browser-tab' ? plan.title : basename(params.filePath) || params.filePath, - ...placement, - // Document grants belong to this desktop even when the workspace is remote. - ...(docLocation ? { docLocation, browserRuntimeEnvironmentId: null } : {}), - ...(params.clientLocal ? { browserRuntimeEnvironmentId: null } : {}), - activate: params.activate - } - ) -} diff --git a/src/renderer/src/lib/file-preview.test.ts b/src/renderer/src/lib/file-preview.test.ts index 02fe5d55cac..48bdfff2188 100644 --- a/src/renderer/src/lib/file-preview.test.ts +++ b/src/renderer/src/lib/file-preview.test.ts @@ -76,15 +76,7 @@ vi.mock('@/store', () => ({ ...(mocks.twin ? localAndSshTwinRows('repo-1', 'wt-1') : { - repos: [ - { - id: 'repo-1', - connectionId: mocks.connectionId, - ...(mocks.environmentId - ? { executionHostId: `runtime:${mocks.environmentId}` } - : {}) - } - ], + repos: [{ id: 'repo-1', connectionId: mocks.connectionId }], worktreesByRepo: { 'repo-1': [{ id: 'wt-1', repoId: 'repo-1' }] } }) }) @@ -132,110 +124,6 @@ function mockOpenDocument(filePath: string): void { } describe('openFileInBrowserTab', () => { - it.each(['remote-host', undefined])( - 'does not reuse a server-owned file URL for a client-local open: %s', - (browserRuntimeEnvironmentId) => { - mocks.environmentId = 'remote-host' - mocks.browserTabsByWorktree = { 'wt-1': [{ id: 'browser-remote' }] } - mocks.browserPagesByWorkspace = { - 'browser-remote': [ - { id: 'page-remote', url: 'file:///tmp/report.html', browserRuntimeEnvironmentId } - ] - } - - openFileInBrowserTab({ - filePath: '/tmp/report.html', - worktreeId: 'wt-1', - clientLocal: true - }) - - expect(mocks.createBrowserTab).toHaveBeenCalledWith('wt-1', 'file:///tmp/report.html', { - title: 'report.html', - targetGroupId: undefined, - browserRuntimeEnvironmentId: null, - activate: true - }) - expect(mocks.setActiveBrowserPage).not.toHaveBeenCalled() - } - ) - - it('reuses an explicitly local page in a paired workspace', () => { - mocks.environmentId = 'remote-host' - mocks.browserTabsByWorktree = { 'wt-1': [{ id: 'browser-local' }] } - mocks.browserPagesByWorkspace = { - 'browser-local': [ - { id: 'page-local', url: 'file:///tmp/report.html', browserRuntimeEnvironmentId: null } - ] - } - - openFileInBrowserTab({ - filePath: '/tmp/report.html', - worktreeId: 'wt-1', - clientLocal: true - }) - - expect(mocks.setActiveBrowserPage).toHaveBeenCalledWith('browser-local', 'page-local') - expect(mocks.createBrowserTab).not.toHaveBeenCalled() - }) - - it('does not reuse a remotely pinned page in a local workspace', () => { - mocks.browserTabsByWorktree = { 'wt-1': [{ id: 'browser-remote' }] } - mocks.browserPagesByWorkspace = { - 'browser-remote': [ - { - id: 'page-remote', - url: 'file:///tmp/report.html', - browserRuntimeEnvironmentId: 'remote-host' - } - ] - } - - openFileInBrowserTab({ filePath: '/tmp/report.html', worktreeId: 'wt-1' }) - - expect(mocks.createBrowserTab).toHaveBeenCalledOnce() - expect(mocks.setActiveBrowserPage).not.toHaveBeenCalled() - }) - - it.each([undefined, 'group-2', 'group-3'])( - 'reuses a local file on an inactive page within the requested group: %s', - (targetGroupId) => { - mocks.browserTabsByWorktree = { 'wt-1': [{ id: 'browser-9' }] } - mocks.browserPagesByWorkspace = { - 'browser-9': [ - { id: 'page-web', url: 'https://example.com' }, - { id: 'page-html', url: 'file:///repo/report%20one.html' } - ] - } - mocks.unifiedTabsByWorktree = { - 'wt-1': [ - { - id: 'tab-html', - worktreeId: 'wt-1', - contentType: 'browser', - entityId: 'browser-9', - groupId: 'group-2' - } - ] - } - - openFileInBrowserTab({ filePath: '/repo/report one.html', worktreeId: 'wt-1', targetGroupId }) - - if (targetGroupId === 'group-3') { - expect(mocks.createBrowserTab).toHaveBeenCalledWith( - 'wt-1', - 'file:///repo/report%20one.html', - { title: 'report one.html', activate: true, targetGroupId } - ) - expect(mocks.focusGroup).not.toHaveBeenCalled() - return - } - - expect(mocks.focusGroup).toHaveBeenCalledWith('wt-1', 'group-2') - expect(mocks.activateTab).toHaveBeenCalledWith('tab-html', { worktreeId: 'wt-1' }) - expect(mocks.setActiveBrowserPage).toHaveBeenCalledWith('browser-9', 'page-html') - expect(mocks.createBrowserTab).not.toHaveBeenCalled() - } - ) it('reuses a document on an inactive page instead of creating a duplicate preview', () => { mocks.connectionId = 'ssh-1' mocks.browserTabsByWorktree = { 'wt-1': [{ id: 'browser-9', docLocation: null }] } @@ -261,10 +149,6 @@ describe('openFileInBrowserTab', () => { }) it('keeps a remote document preview in the requested group', () => { mocks.environmentId = 'env-1' - mockOpenDocument('/srv/repo/docs/report.html') - mocks.unifiedTabsByWorktree = { - 'wt-1': [{ contentType: 'browser', entityId: 'browser-9', groupId: 'group-1' }] - } openFileInBrowserTab({ filePath: '/srv/repo/docs/report.html', worktreeId: 'wt-1', @@ -445,11 +329,7 @@ describe('openFileInBrowserTab', () => { ) }) - it('creates a local side preview in a right-hand split even when the file is already open', () => { - mocks.browserTabsByWorktree = { 'wt-1': [{ id: 'browser-9' }] } - mocks.browserPagesByWorkspace = { - 'browser-9': [{ id: 'page-html', url: 'file:///tmp/example.html' }] - } + it('creates local side previews in an activated right-hand split', () => { openFilePreviewToSide({ language: 'html', filePath: '/tmp/example.html', diff --git a/src/renderer/src/lib/file-preview.ts b/src/renderer/src/lib/file-preview.ts index 45029639a2d..ba6eff762ba 100644 --- a/src/renderer/src/lib/file-preview.ts +++ b/src/renderer/src/lib/file-preview.ts @@ -18,14 +18,14 @@ import { translate } from '@/i18n/i18n' import { useAppStore } from '@/store' import type { AppState } from '@/store/types' import { findSiblingGroupId } from '@/store/slices/tabs' +import type { CreateBrowserTabOptions } from '@/store/slices/browser/browser-slice-contract' import { browserPageDocLocationsEqual } from '../../../shared/browser-page-doc-location' import type { BrowserPageDocLocation } from '../../../shared/browser-workspace-types' import { findPage } from '@/store/slices/browser-page-records' import type { BrowserPageConversionLeg } from '@/store/slices/browser-page-conversion' +import { ORCA_BROWSER_BLANK_URL } from '../../../shared/constants' import { chatScratchFileAddress } from '@/runtime/runtime-chat-scratch-file-address' import { selectFolderWorkspaces, selectWorktreesByRepo } from '@/store/workspace-record-selectors' -import { findWorkspaceShowingPage } from './workspace-file-preview-page' -import { openFilePreviewTab } from './file-preview-tab' export type PreviewableLanguage = 'html' @@ -51,31 +51,11 @@ export type WorkspaceFilePreviewPlan = | { status: 'doc-preview' } | { status: 'unsupported'; message: string; reason: 'no-channel' | 'outside-worktree' } -function getLocalFilePreviewPlan( +export function getWorkspaceFilePreviewPlan( state: AppState, worktreeId: string, filePath: string ): WorkspaceFilePreviewPlan { - const availability = getClientCreationActionPolicy(state, worktreeId)['managed-browser'] - if (availability.state !== 'enabled') { - return { status: 'unsupported', message: availability.reason, reason: 'no-channel' } - } - return { - status: 'browser-tab', - url: absolutePathToFileUri(filePath), - title: basename(filePath) || filePath - } -} - -export function getWorkspaceFilePreviewPlan( - state: AppState, - worktreeId: string, - filePath: string, - clientLocal = false -): WorkspaceFilePreviewPlan { - if (clientLocal) { - return getLocalFilePreviewPlan(state, worktreeId, filePath) - } // Why: a twin reads as "no server" below, which would open this machine's copy of the path. if (isTwinWorkspace(state, worktreeId)) { return { @@ -113,7 +93,15 @@ export function getWorkspaceFilePreviewPlan( } return webClientDocPreviewRefusal() ?? { status: 'doc-preview' } } - return getLocalFilePreviewPlan(state, worktreeId, filePath) + const availability = getClientCreationActionPolicy(state, worktreeId)['managed-browser'] + if (availability.state !== 'enabled') { + return { status: 'unsupported', message: availability.reason, reason: 'no-channel' } + } + return { + status: 'browser-tab', + url: absolutePathToFileUri(filePath), + title: basename(filePath) || filePath + } } export function canShowWorkspaceFileBrowserAction( @@ -157,28 +145,85 @@ export function useWorkspaceFileBrowserActionPredicate( ) } +function openDocPreviewTab( + state: AppState, + params: { filePath: string; worktreeId: string; activate: boolean }, + placement?: Pick +): void { + const docLocation = { + kind: 'workspace-doc' as const, + worktreeId: params.worktreeId, + filePath: params.filePath + } + // Why reuse and not a second tab: previewing a document already on screen is a request to look at + // it, and two tabs of one document would each hold their own grant on the same file. + const existing = findWorkspaceShowingDoc(state, docLocation) + if (existing) { + if ( + !params.activate || + !activateBrowserWorkspaceTab({ + worktreeId: params.worktreeId, + ...existing + }) + ) { + state.setActiveBrowserTab(existing.workspaceId) + state.setActiveBrowserPage(existing.workspaceId, existing.pageId) + } + return + } + state.createBrowserTab(params.worktreeId, ORCA_BROWSER_BLANK_URL, { + docLocation, + title: basename(params.filePath) || params.filePath, + ...placement, + // Why explicitly client-local: the document is read through a grant this desktop mints, so the + // page never belongs to a remote runtime even when the worktree does. + browserRuntimeEnvironmentId: null, + // Why the caller decides: opening a file is a request to look at it, while a preview opened to + // the side belongs beside the source the reader is still working in. + activate: params.activate + }) +} + export function openFileInBrowserTab(params: { filePath: string worktreeId: string targetGroupId?: string - clientLocal?: boolean }): WorkspaceFilePreviewPlan { const state = useAppStore.getState() - const plan = getWorkspaceFilePreviewPlan( - state, - params.worktreeId, - params.filePath, - params.clientLocal - ) + const plan = getWorkspaceFilePreviewPlan(state, params.worktreeId, params.filePath) if (plan.status === 'unsupported') { return plan } - openFilePreviewTab(state, { ...params, activate: true }, plan, { - targetGroupId: params.targetGroupId + if (plan.status === 'doc-preview') { + openDocPreviewTab(state, { ...params, activate: true }, { targetGroupId: params.targetGroupId }) + return plan + } + + state.createBrowserTab(params.worktreeId, plan.url, { + title: plan.title, + ...(params.targetGroupId ? { targetGroupId: params.targetGroupId } : {}), + activate: true }) return plan } +/** The tab a document is already open in, found by every PAGE's docLocation and not just the + * workspace mirror — a mixed workspace whose doc page is inactive mirrors null. */ +function findWorkspaceShowingDoc( + state: AppState, + docLocation: BrowserPageDocLocation +): { workspaceId: string; pageId: string } | null { + for (const tab of state.browserTabsByWorktree[docLocation.worktreeId] ?? []) { + const page = (state.browserPagesByWorkspace[tab.id] ?? []).find((candidate) => + browserPageDocLocationsEqual(candidate.docLocation ?? null, docLocation) + ) + if (page) { + return { workspaceId: tab.id, pageId: page.id } + } + } + return null +} + /** * The address bar's way into a workspace document: reuse before converting. A document already on * screen is a request to look at it — two tabs of one document would each hold their own grant on @@ -201,11 +246,7 @@ export function convertBrowserPageToWorkspaceDoc( // history could jump there forever. A history leg's document was this tab's own, so converting // in place is right. const isHistoryLeg = options?.leg !== undefined - const existing = isHistoryLeg - ? null - : findWorkspaceShowingPage(state, docLocation.worktreeId, (page) => - browserPageDocLocationsEqual(page.docLocation ?? null, docLocation) - ) + const existing = isHistoryLeg ? null : findWorkspaceShowingDoc(state, docLocation) if (existing) { // Why the worktree switches first: activation is deliberately scoped to the active worktree, // so without the switch a cross-worktree reuse would happen entirely out of sight. @@ -298,12 +339,7 @@ export function openFilePreviewToSide(params: { : { split: { sourceGroupId, splitDirection: 'right' as const, activate } } if (plan.status === 'doc-preview') { - openFilePreviewTab( - state, - { filePath: params.filePath, worktreeId, activate: false }, - plan, - placement - ) + openDocPreviewTab(state, { filePath: params.filePath, worktreeId, activate: false }, placement) return } diff --git a/src/renderer/src/lib/open-html-file-preview.test.ts b/src/renderer/src/lib/open-html-file-preview.test.ts new file mode 100644 index 00000000000..d9eed126ebe --- /dev/null +++ b/src/renderer/src/lib/open-html-file-preview.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, expect, it, vi } from 'vitest' +import { openHtmlFilePreview } from './open-html-file-preview' + +const mocks = vi.hoisted(() => ({ + resolve: vi.fn(), + open: vi.fn(), + select: vi.fn(), + state: { activeWorktreeId: 'current' } +})) +vi.mock('@/store', () => ({ useAppStore: { getState: () => mocks.state } })) +vi.mock('./workspace-doc-address-input', () => ({ + resolveWorkspaceDocAddressTarget: mocks.resolve +})) +vi.mock('./file-preview', () => ({ openFileInBrowserTab: mocks.open })) +vi.mock('./workspace-surface-reveal', () => ({ selectWorkspaceSurface: mocks.select })) + +beforeEach(() => { + vi.clearAllMocks() + mocks.resolve.mockReturnValue({ status: 'not-a-workspace-doc' }) + mocks.open.mockReturnValue({ status: 'browser-tab' }) +}) + +const request = { + filePath: '/repo/report one.html', + worktreeId: 'current', + targetGroupId: 'group-2' +} + +it('keeps a local HTML preview in the requested group', () => { + openHtmlFilePreview(request) + expect(mocks.resolve).toHaveBeenCalledWith(mocks.state, 'current', request.filePath) + expect(mocks.open).toHaveBeenCalledWith(request) + expect(mocks.select).not.toHaveBeenCalled() +}) + +it('opens a sibling remote document in its owning workspace', () => { + mocks.resolve.mockReturnValue({ status: 'workspace-doc', docLocation: { worktreeId: 'sibling' } }) + mocks.open.mockReturnValue({ status: 'doc-preview' }) + openHtmlFilePreview(request) + expect(mocks.select).toHaveBeenCalledWith(mocks.state, 'sibling') + expect(mocks.open).toHaveBeenCalledWith({ filePath: request.filePath, worktreeId: 'sibling' }) +}) + +it('reports a refused host without creating a preview', () => { + mocks.resolve.mockReturnValue({ status: 'unsupported', message: 'outside workspace' }) + expect(() => openHtmlFilePreview(request)).toThrow('outside workspace') + expect(mocks.open).not.toHaveBeenCalled() +}) + +it('reports a preview plan refusal', () => { + mocks.open.mockReturnValue({ status: 'unsupported', message: 'unknown host' }) + expect(() => openHtmlFilePreview(request)).toThrow('unknown host') +}) diff --git a/src/renderer/src/lib/open-html-file-preview.ts b/src/renderer/src/lib/open-html-file-preview.ts new file mode 100644 index 00000000000..3715e919313 --- /dev/null +++ b/src/renderer/src/lib/open-html-file-preview.ts @@ -0,0 +1,29 @@ +import { openFileInBrowserTab } from './file-preview' +import { resolveWorkspaceDocAddressTarget } from './workspace-doc-address-input' +import { selectWorkspaceSurface } from './workspace-surface-reveal' +import { useAppStore } from '@/store' + +export function openHtmlFilePreview(params: { + filePath: string + worktreeId: string + targetGroupId: string +}): void { + const state = useAppStore.getState() + const target = resolveWorkspaceDocAddressTarget(state, params.worktreeId, params.filePath) + if (target.status === 'unsupported') { + throw new Error(target.message) + } + const worktreeId = + target.status === 'workspace-doc' ? target.docLocation.worktreeId : params.worktreeId + if (state.activeWorktreeId !== worktreeId) { + selectWorkspaceSurface(state, worktreeId) + } + const plan = openFileInBrowserTab({ + filePath: params.filePath, + worktreeId, + ...(worktreeId === params.worktreeId ? { targetGroupId: params.targetGroupId } : {}) + }) + if (plan.status === 'unsupported') { + throw new Error(plan.message) + } +} diff --git a/src/renderer/src/lib/workspace-file-open.test.ts b/src/renderer/src/lib/workspace-file-open.test.ts deleted file mode 100644 index 7d505647075..00000000000 --- a/src/renderer/src/lib/workspace-file-open.test.ts +++ /dev/null @@ -1,350 +0,0 @@ -import { beforeEach, expect, it, vi } from 'vitest' -import { openWorkspaceFile, type WorkspaceFileOpenRequest } from './workspace-file-open' -import type { GitStatusEntry } from '../../../shared/git-status-types' - -const mocks = vi.hoisted(() => { - const gitStatusByWorktree: Record = {} - return { - resolve: vi.fn(), - plan: vi.fn(), - open: vi.fn(), - select: vi.fn(), - activate: vi.fn(), - environmentId: null as string | null, - state: { - activeWorktreeId: 'current', - gitStatusByWorktree, - runtimeStatusByEnvironmentId: new Map(), - openBranchDiff: vi.fn(), - openDiff: vi.fn(), - openFileForLink: vi.fn(), - openFile: vi.fn((file: { filePath: string }) => file.filePath) - } - } -}) -vi.mock('@/store', () => ({ useAppStore: { getState: () => mocks.state } })) -vi.mock('./workspace-doc-address-input', () => ({ - resolveWorkspaceDocAddressTarget: mocks.resolve -})) -vi.mock('./file-preview', () => ({ - openFileInBrowserTab: mocks.open, - getWorkspaceFilePreviewPlan: mocks.plan -})) -vi.mock('./workspace-surface-reveal', () => ({ selectWorkspaceSurface: mocks.select })) -vi.mock('./worktree-activation', () => ({ activateAndRevealWorkspace: mocks.activate })) -vi.mock('./resolve-owner', () => ({ - runtimeEnvironmentIdUnlessTwin: () => mocks.environmentId -})) - -beforeEach(() => { - vi.clearAllMocks() - mocks.state.gitStatusByWorktree = {} - mocks.environmentId = null - mocks.state.runtimeStatusByEnvironmentId.clear() - mocks.resolve.mockReturnValue({ status: 'not-a-workspace-doc' }) - mocks.open.mockReturnValue({ status: 'browser-tab' }) - mocks.plan.mockReturnValue({ status: 'browser-tab' }) - mocks.state.openFileForLink.mockReturnValue({ - fileId: 'html-source', - viewStateId: 'html-view', - groupId: 'group-2' - }) -}) - -const request = { - kind: 'preview' as const, - filePath: '/repo/report one.html', - worktreeId: 'current', - targetGroupId: 'group-2' -} - -function sourceFile(filePath: string) { - return { - filePath, - relativePath: filePath, - worktreeId: 'current', - language: 'plaintext', - mode: 'edit' as const - } -} - -it.each(['/repo/report.html', '/repo/report.HTM', 'C:\\repo\\report.HTML'])( - 'opens the default HTML surface for %s', - (filePath) => { - expect( - openWorkspaceFile({ - kind: 'file', - file: sourceFile(filePath), - options: { targetGroupId: 'group-2' } - }) - ).toEqual({ surface: 'preview' }) - expect(mocks.open).toHaveBeenCalledWith({ - filePath, - worktreeId: 'current', - targetGroupId: 'group-2' - }) - expect(mocks.state.openFile).not.toHaveBeenCalled() - } -) - -it.each(['/repo/index.ts', '/repo/page.jsp', '/repo/page.html.liquid'])( - 'opens non-HTML files in the editor: %s', - (filePath) => { - const file = sourceFile(filePath) - openWorkspaceFile({ kind: 'file', file }) - expect(mocks.state.openFile).toHaveBeenCalledWith(file, undefined) - expect(mocks.open).not.toHaveBeenCalled() - } -) - -it('keeps an explicit source request in the editor', () => { - const file = sourceFile('/repo/report.html') - openWorkspaceFile({ kind: 'file', intent: 'source', file, options: { preview: false } }) - expect(mocks.state.openFile).toHaveBeenCalledWith(file, { preview: false }) - expect(mocks.plan).not.toHaveBeenCalled() - expect(mocks.open).not.toHaveBeenCalled() -}) - -it('keeps a host-granted HTML artifact on its authorized editor path', () => { - const file = { ...sourceFile('/outside/report.html'), terminalArtifactGrantId: 'grant-1' } - openWorkspaceFile({ kind: 'file', file }) - expect(mocks.state.openFile).toHaveBeenCalledWith(file, undefined) - expect(mocks.open).not.toHaveBeenCalled() -}) - -it('keeps an external SSH HTML file on its owning editor path', () => { - const file = { ...sourceFile('/remote/report.html'), externalSshTargetId: 'ssh-target' } - expect(openWorkspaceFile({ kind: 'file', file })).toEqual({ - surface: 'editor', - fileId: file.filePath - }) - expect(mocks.state.openFile).toHaveBeenCalledWith(file, undefined) - expect(mocks.plan).not.toHaveBeenCalled() - expect(mocks.open).not.toHaveBeenCalled() -}) - -const htmlLink = { - kind: 'link', - file: { - filePath: '/repo/report.html', - relativePath: 'report.html', - worktreeId: 'folder:docs' - }, - linkIntent: null, - position: { line: null, column: null }, - isCurrentRequest: () => true, - executionHostId: 'runtime:host-1', - clientLocal: false -} satisfies WorkspaceFileOpenRequest - -it('activates an HTML link workspace with its execution host before previewing', () => { - expect(openWorkspaceFile(htmlLink)).toEqual({ surface: 'preview' }) - expect(mocks.activate).toHaveBeenCalledWith('folder:docs', { - providesInitialSurface: true, - executionHostId: 'runtime:host-1' - }) - expect(mocks.activate.mock.invocationCallOrder[0]).toBeLessThan( - mocks.open.mock.invocationCallOrder[0] - ) - expect(mocks.state.openFileForLink).not.toHaveBeenCalled() -}) - -it.each([ - { - ...htmlLink, - linkIntent: { - workspaceId: 'folder:docs', - sourceTabId: 'source-tab', - hostId: 'runtime:host-1', - targetGroupId: 'group-2' - } - }, - { ...htmlLink, file: { ...htmlLink.file, externalSshTargetId: 'ssh-target' } } -] satisfies WorkspaceFileOpenRequest[])( - 'opens HTML links with an explicit editor group or external SSH owner in the editor: %j', - (request) => { - expect(openWorkspaceFile(request)).toEqual({ surface: 'editor', fileId: 'html-source' }) - expect(mocks.state.openFileForLink).toHaveBeenCalledWith( - expect.objectContaining(request.file), - { forceContentReload: true }, - request.linkIntent, - expect.any(Function) - ) - expect(mocks.plan).not.toHaveBeenCalled() - expect(mocks.open).not.toHaveBeenCalled() - } -) - -it('falls back to the editor when the owning host cannot preview HTML', () => { - mocks.plan.mockReturnValue({ status: 'unsupported', reason: 'no-channel' }) - const file = sourceFile('/repo/report.html') - openWorkspaceFile({ kind: 'file', file }) - expect(mocks.state.openFile).toHaveBeenCalledWith(file, undefined) - expect(mocks.open).not.toHaveBeenCalled() -}) - -it.each([null, false])( - 'keeps source available when the paired host has no known scoped preview support: %s', - (supported) => { - mocks.environmentId = 'paired-host' - mocks.plan.mockReturnValue({ status: 'doc-preview' }) - if (supported === false) { - mocks.state.runtimeStatusByEnvironmentId.set('paired-host', { status: { capabilities: [] } }) - } - const file = sourceFile('/repo/report.html') - - expect(openWorkspaceFile({ kind: 'file', file })).toEqual({ - surface: 'editor', - fileId: file.filePath - }) - expect(mocks.state.openFile).toHaveBeenCalledWith(file, undefined) - expect(mocks.open).not.toHaveBeenCalled() - expect( - openWorkspaceFile({ ...htmlLink, file: { ...htmlLink.file, worktreeId: 'current' } }) - ).toEqual({ surface: 'editor', fileId: 'html-source' }) - expect(mocks.open).not.toHaveBeenCalled() - expect( - openWorkspaceFile({ - kind: 'uncommitted', - worktreeId: 'current', - worktreePath: '/repo', - entry: { path: 'report.html', status: 'modified', area: 'unstaged' } - }) - ).toEqual({ surface: 'diff' }) - expect(mocks.state.openDiff).toHaveBeenCalled() - } -) - -it('defaults to scoped previews only when the paired host advertises support', () => { - mocks.environmentId = 'paired-host' - mocks.plan.mockReturnValue({ status: 'doc-preview' }) - mocks.state.runtimeStatusByEnvironmentId.set('paired-host', { - status: { capabilities: ['files.readDocPreview'] } - }) - - expect(openWorkspaceFile({ kind: 'file', file: sourceFile('/repo/report.html') })).toEqual({ - surface: 'preview' - }) - expect(mocks.open).toHaveBeenCalledOnce() - expect(mocks.state.openFile).not.toHaveBeenCalled() -}) - -it('retains explicit preview behavior on an older paired host', () => { - mocks.environmentId = 'paired-host' - mocks.open.mockReturnValue({ status: 'doc-preview' }) - - expect(openWorkspaceFile(request)).toEqual({ surface: 'preview' }) - expect(mocks.state.openFile).not.toHaveBeenCalled() -}) - -const branchRequest = { - kind: 'branch', - worktreeId: 'current', - worktreePath: '/repo', - entry: { path: 'report.html', status: 'modified' }, - compare: { - baseRef: 'main', - baseOid: 'base', - compareRef: 'HEAD', - headOid: 'head', - mergeBase: 'base' - } -} as const - -it.each([ - [{ path: 'report.html', status: 'deleted', area: 'staged' }], - [{ path: 'report.html', status: 'deleted', area: 'unstaged' }], - [{ path: 'renamed.html', oldPath: 'report.html', status: 'renamed', area: 'staged' }] -] satisfies GitStatusEntry[][])( - 'opens a committed diff when the working file is absent: %j', - (...changes) => { - mocks.state.gitStatusByWorktree.current = changes - - expect(openWorkspaceFile(branchRequest)).toEqual({ surface: 'diff' }) - expect(mocks.state.openBranchDiff).toHaveBeenCalledWith( - 'current', - '/repo', - branchRequest.entry, - branchRequest.compare, - 'html', - undefined - ) - expect(mocks.open).not.toHaveBeenCalled() - } -) - -it.each([ - { path: 'report.html', status: 'deleted', area: 'staged' }, - { path: 'renamed.html', oldPath: 'report.html', status: 'renamed', area: 'staged' } -] satisfies GitStatusEntry[])( - 'previews a recreated working file after a staged $status', - (stagedChange) => { - const entry: GitStatusEntry = { path: 'report.html', status: 'untracked', area: 'untracked' } - mocks.state.gitStatusByWorktree.current = [stagedChange, entry] - - expect(openWorkspaceFile(branchRequest)).toEqual({ surface: 'preview' }) - expect( - openWorkspaceFile({ - kind: 'uncommitted', - worktreeId: 'current', - worktreePath: '/repo', - entry - }) - ).toEqual({ surface: 'preview' }) - expect(mocks.open).toHaveBeenCalledWith({ - filePath: '/repo/report.html', - worktreeId: 'current', - targetGroupId: undefined - }) - expect(mocks.state.openBranchDiff).not.toHaveBeenCalled() - expect(mocks.state.openDiff).not.toHaveBeenCalled() - } -) - -it.each([ - { path: 'report.html', oldPath: 'before.html', status: 'renamed', area: 'staged' }, - { path: 'copy.html', oldPath: 'report.html', status: 'copied', area: 'staged' } -] satisfies GitStatusEntry[])( - 'previews the existing file after a staged $status to $path', - (change) => { - mocks.state.gitStatusByWorktree.current = [change] - - expect(openWorkspaceFile(branchRequest)).toEqual({ surface: 'preview' }) - expect(mocks.open).toHaveBeenCalledWith({ - filePath: '/repo/report.html', - worktreeId: 'current', - targetGroupId: undefined - }) - expect(mocks.state.openBranchDiff).not.toHaveBeenCalled() - } -) - -it('keeps a local HTML preview in the requested group', () => { - openWorkspaceFile(request) - expect(mocks.resolve).toHaveBeenCalledWith(mocks.state, 'current', request.filePath) - expect(mocks.open).toHaveBeenCalledWith({ - filePath: request.filePath, - worktreeId: request.worktreeId, - targetGroupId: request.targetGroupId - }) - expect(mocks.select).not.toHaveBeenCalled() -}) - -it('opens a sibling remote document in its owning workspace', () => { - mocks.resolve.mockReturnValue({ status: 'workspace-doc', docLocation: { worktreeId: 'sibling' } }) - mocks.open.mockReturnValue({ status: 'doc-preview' }) - openWorkspaceFile(request) - expect(mocks.select).toHaveBeenCalledWith(mocks.state, 'sibling') - expect(mocks.open).toHaveBeenCalledWith({ filePath: request.filePath, worktreeId: 'sibling' }) -}) - -it('reports a refused host without creating a preview', () => { - mocks.resolve.mockReturnValue({ status: 'unsupported', message: 'outside workspace' }) - expect(() => openWorkspaceFile(request)).toThrow('outside workspace') - expect(mocks.open).not.toHaveBeenCalled() -}) - -it('reports a preview plan refusal', () => { - mocks.open.mockReturnValue({ status: 'unsupported', message: 'unknown host' }) - expect(() => openWorkspaceFile(request)).toThrow('unknown host') -}) diff --git a/src/renderer/src/lib/workspace-file-open.ts b/src/renderer/src/lib/workspace-file-open.ts deleted file mode 100644 index d33e9ae4a89..00000000000 --- a/src/renderer/src/lib/workspace-file-open.ts +++ /dev/null @@ -1,233 +0,0 @@ -import { useAppStore } from '@/store' -import type { AppState } from '@/store/types' -import type { - OpenFile, - BranchCompareLike, - EditorOpenTargetOptions -} from '@/store/slices/editor/types/open-file' -import type { OpenFileOptions } from '@/store/slices/editor/types/editor-files-slice' -import type { GitStatusEntry } from '../../../shared/git-status-types' -import type { GitBranchChangeEntry } from '../../../shared/git-diff-compare-types' -import { isHtmlFilePath } from '../../../shared/html-file-url' -import { detectLanguage } from './language-detect' -import { joinPath } from './path' -import { getWorkspaceFilePreviewPlan, openFileInBrowserTab } from './file-preview' -import { openUncommittedEntry } from './workspace-file-changes-open' -import { resolveWorkspaceDocAddressTarget } from './workspace-doc-address-input' -import { selectWorkspaceSurface } from './workspace-surface-reveal' -import { openLinkedFileInEditor } from './editor-file-link-open' -import type { EditorOpenLinkIntent } from '@/store/slices/editor/types/editor-open-link-intent' -import type { ExecutionHostId } from '../../../shared/execution-host' -import { activateAndRevealWorkspace } from './worktree-activation' -import { runtimeEnvironmentIdUnlessTwin } from './resolve-owner' -import { FILE_DOC_PREVIEW_RUNTIME_CAPABILITY } from '../../../shared/files-runtime-capabilities' - -type ChangeOpenRequest = { - worktreeId: string - worktreePath: string - intent?: 'default' | 'diff' - options?: EditorOpenTargetOptions -} & ( - | { kind: 'uncommitted'; entry: GitStatusEntry } - | { kind: 'branch'; entry: GitBranchChangeEntry; compare: BranchCompareLike } -) - -export type WorkspaceFileOpenRequest = - | { - kind: 'file' - file: Omit - intent?: 'default' | 'source' - options?: OpenFileOptions - } - | { kind: 'preview'; filePath: string; worktreeId: string; targetGroupId?: string } - | { - kind: 'link' - file: Omit - intent?: 'default' | 'source' - linkIntent: EditorOpenLinkIntent | null - position: { line: number | null; column: number | null } - isCurrentRequest: () => boolean - executionHostId: ExecutionHostId - clientLocal: boolean - } - | ChangeOpenRequest - -export type WorkspaceFileOpenResult = - | { surface: 'preview' } - | { surface: 'editor'; fileId: string } - | { surface: 'diff' } - | { surface: 'refused' } - -export type WorkspaceFileOpener = typeof openWorkspaceFile - -function hasWorkingTreeFile( - entry: Pick -): boolean { - return entry.status !== 'deleted' && !entry.conflictStatus && !entry.submodule -} - -function hasCurrentWorkingTreeFile(path: string, changes: GitStatusEntry[]): boolean { - const workingTreeChange = changes.find( - (change) => change.path === path && change.area !== 'staged' - ) - if (workingTreeChange) { - return hasWorkingTreeFile(workingTreeChange) - } - return !changes.some( - (change) => - (change.path === path && !hasWorkingTreeFile(change)) || - (change.status === 'renamed' && change.oldPath === path) - ) -} - -function openRequestedPreview( - request: Extract -): WorkspaceFileOpenResult { - const state = useAppStore.getState() - const target = resolveWorkspaceDocAddressTarget(state, request.worktreeId, request.filePath) - if (target.status === 'unsupported') { - throw new Error(target.message) - } - const worktreeId = - target.status === 'workspace-doc' ? target.docLocation.worktreeId : request.worktreeId - if (state.activeWorktreeId !== worktreeId) { - selectWorkspaceSurface(state, worktreeId) - } - const plan = openFileInBrowserTab({ - filePath: request.filePath, - worktreeId, - ...(worktreeId === request.worktreeId ? { targetGroupId: request.targetGroupId } : {}) - }) - if (plan.status === 'unsupported') { - throw new Error(plan.message) - } - return { surface: 'preview' } -} - -function defaultPreviewTarget( - request: Exclude, - state: Pick -): Parameters[0] | null { - if (request.intent === 'source' || request.intent === 'diff') { - return null - } - const filePath = - 'file' in request ? request.file.filePath : joinPath(request.worktreePath, request.entry.path) - if (!isHtmlFilePath(filePath)) { - return null - } - switch (request.kind) { - case 'file': - return request.file.mode === 'edit' && - !request.file.terminalArtifactGrantId && - !request.file.externalSshTargetId - ? { - filePath, - worktreeId: request.file.worktreeId, - targetGroupId: request.options?.targetGroupId - } - : null - case 'link': - return request.position.line === null && - !request.file.terminalArtifactGrantId && - !request.file.externalSshTargetId && - !request.linkIntent?.targetGroupId - ? { - filePath, - worktreeId: request.file.worktreeId, - ...(request.clientLocal ? { clientLocal: true } : {}) - } - : null - case 'uncommitted': - case 'branch': - if ( - !hasWorkingTreeFile(request.entry) || - !hasCurrentWorkingTreeFile( - request.entry.path, - state.gitStatusByWorktree[request.worktreeId] ?? [] - ) - ) { - return null - } - return { - filePath, - worktreeId: request.worktreeId, - targetGroupId: request.options?.targetGroupId - } - } -} - -export function openWorkspaceFile(request: WorkspaceFileOpenRequest): WorkspaceFileOpenResult { - if (request.kind === 'preview') { - return openRequestedPreview(request) - } - const state = useAppStore.getState() - const target = defaultPreviewTarget(request, state) - if (target) { - const plan = getWorkspaceFilePreviewPlan( - state, - target.worktreeId, - target.filePath, - target.clientLocal - ) - const previewEnvironmentId = - plan.status === 'doc-preview' - ? runtimeEnvironmentIdUnlessTwin(state, target.worktreeId) - : null - if ( - plan.status !== 'unsupported' && - (!previewEnvironmentId || - state.runtimeStatusByEnvironmentId - ?.get(previewEnvironmentId) - ?.status?.capabilities?.includes(FILE_DOC_PREVIEW_RUNTIME_CAPABILITY) === true) - ) { - if (request.kind === 'link') { - activateAndRevealWorkspace(target.worktreeId, { - providesInitialSurface: true, - executionHostId: request.executionHostId - }) - } - if (openFileInBrowserTab(target).status !== 'unsupported') { - return { surface: 'preview' } - } - } - } - if (request.kind === 'link') { - const worktreeId = request.file.worktreeId - if (worktreeId) { - activateAndRevealWorkspace(worktreeId, { - providesInitialSurface: true, - executionHostId: request.executionHostId - }) - } - const opened = openLinkedFileInEditor( - request.file, - request.linkIntent, - request.position, - request.isCurrentRequest - ) - return opened ? { surface: 'editor', fileId: opened.fileId } : { surface: 'refused' } - } - if (request.kind === 'file') { - return { surface: 'editor', fileId: state.openFile(request.file, request.options) } - } - if (request.kind === 'uncommitted') { - openUncommittedEntry( - state, - request.worktreeId, - request.worktreePath, - request.entry, - request.options - ) - } else { - state.openBranchDiff( - request.worktreeId, - request.worktreePath, - request.entry, - request.compare, - detectLanguage(request.entry.path), - request.options - ) - } - return { surface: 'diff' } -} diff --git a/src/renderer/src/lib/workspace-file-preview-page.ts b/src/renderer/src/lib/workspace-file-preview-page.ts deleted file mode 100644 index 5b0cf5f6802..00000000000 --- a/src/renderer/src/lib/workspace-file-preview-page.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { AppState } from '@/store/types' -import type { BrowserPage } from '../../../shared/browser-workspace-types' - -export function findWorkspaceShowingPage( - state: AppState, - worktreeId: string, - matches: (page: BrowserPage) => boolean, - targetGroupId?: string -): { workspaceId: string; pageId: string } | null { - for (const tab of state.browserTabsByWorktree[worktreeId] ?? []) { - if ( - targetGroupId && - !state.unifiedTabsByWorktree[worktreeId]?.some( - (candidate) => - candidate.contentType === 'browser' && - candidate.entityId === tab.id && - candidate.groupId === targetGroupId - ) - ) { - continue - } - const page = (state.browserPagesByWorkspace[tab.id] ?? []).find(matches) - if (page) { - return { workspaceId: tab.id, pageId: page.id } - } - } - return null -} diff --git a/src/shared/html-file-url.ts b/src/shared/html-file-url.ts index 9eb3fcbb39b..21a5a597cae 100644 --- a/src/shared/html-file-url.ts +++ b/src/shared/html-file-url.ts @@ -1,9 +1,5 @@ import { fileUriToFilesystemPath } from './file-uri-path' -export function isHtmlFilePath(filePath: string): boolean { - return /\.html?$/i.test(filePath) -} - /** File authorities name this machine, never an Orca SSH target. */ export function htmlFilePathFromUrl(input: string): string | null { if (!/^file:\/\//i.test(input) || /[?#\p{Cc}]|%2f|%5c/iu.test(input)) { @@ -15,7 +11,7 @@ export function htmlFilePathFromUrl(input: string): string | null { return null } const filePath = fileUriToFilesystemPath(url) - return filePath && isHtmlFilePath(filePath) && !/\p{Cc}/u.test(filePath) ? filePath : null + return filePath && /\.html?$/i.test(filePath) && !/\p{Cc}/u.test(filePath) ? filePath : null } catch { return null } diff --git a/tests/e2e/html-file-url-opening.spec.ts b/tests/e2e/html-file-url-opening.spec.ts index 77885793503..321864f8af7 100644 --- a/tests/e2e/html-file-url-opening.spec.ts +++ b/tests/e2e/html-file-url-opening.spec.ts @@ -1,4 +1,4 @@ -import { mkdirSync, readFileSync, symlinkSync, writeFileSync } from 'node:fs' +import { mkdirSync, writeFileSync } from 'node:fs' import path from 'node:path' import { pathToFileURL } from 'node:url' import { expect, test } from './helpers/orca-app' @@ -9,35 +9,24 @@ import { import { readDocPreviewRenderedText } from './helpers/paired-html-preview-inventory' import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' -test('opens HTML from Explorer as a preview and lets the user edit its source', async ({ +test('opens a local HTML file URL with encoded characters from the new-tab menu', async ({ electronApp, - orcaPage + orcaPage, + testRepoPath }, testInfo) => { - const name = 'default-preview.html' + const filePath = path.join(testRepoPath, 'report 中%20.html') + writeFileSync(filePath, 'Local file URL

Local HTML rendered

') + const url = pathToFileURL(filePath).href await waitForSessionReady(orcaPage) const worktreeId = await waitForActiveWorktree(orcaPage) - const root = await orcaPage.evaluate( - (id) => window.__store?.getState().getKnownWorktreeById(id)?.path, - worktreeId - ) - if (!root) { - throw new Error('Active workspace missing') - } - const filePath = path.join(root, name) - const url = pathToFileURL(filePath).href - writeFileSync(filePath, 'HTML default

Rendered by default

') - await orcaPage.evaluate(() => { - window.localStorage.setItem('orca.browser.markup-draw-hint-seen', 'true') - const state = window.__store?.getState() - state?.setRightSidebarOpen(true) - state?.setRightSidebarTab('explorer') + await orcaPage.getByRole('button', { name: 'New tab', exact: true }).first().click() + const input = orcaPage.getByRole('combobox', { + name: 'Search open tabs, history, files, URLs, agents…', + exact: true }) - await orcaPage.getByRole('button', { name: 'Refresh Explorer', exact: true }).click() - const row = orcaPage.locator('[data-file-explorer-row]').filter({ - has: orcaPage.locator('[data-file-explorer-row-name]').getByText(name, { exact: true }) - }) - await expect(row).toBeVisible() - await row.click() + await input.fill(url) + await expect(orcaPage.getByRole('option', { name: /Open file/ })).toBeVisible() + await input.press('Enter') await expect .poll(() => electronApp.evaluate(async ({ webContents }, fileUrl) => { @@ -47,106 +36,19 @@ test('opens HTML from Explorer as a preview and lets the user edit its source', return guest?.executeJavaScript('document.querySelector("h1")?.textContent ?? null') }, url) ) - .toBe('Rendered by default') - await electronApp.evaluate(async ({ webContents }, fileUrl) => { - const guest = webContents.getAllWebContents().find((contents) => contents.getURL() === fileUrl) - await guest?.executeJavaScript( - 'const input = document.createElement("input"); input.id = "preview-state"; input.value = "Keep my input"; document.body.append(input)' + .toBe('Local HTML rendered') + expect( + await orcaPage.evaluate( + ({ worktreeId, url }) => + window.__store + ?.getState() + .browserTabsByWorktree[worktreeId]?.some((tab) => tab.url === url && !tab.docLocation), + { worktreeId, url } ) - }, url) - await row.click() - await expect - .poll(() => - electronApp.evaluate(async ({ webContents }, fileUrl) => { - const guest = webContents - .getAllWebContents() - .find((contents) => contents.getURL() === fileUrl) - return guest?.executeJavaScript('document.querySelector("#preview-state")?.value ?? null') - }, url) - ) - .toBe('Keep my input') - await orcaPage.screenshot({ path: testInfo.outputPath('html-default-preview.png') }) - await orcaPage.getByRole('button', { name: 'Open source file', exact: true }).click() - await expect(orcaPage.locator('.editor-header-path')).toContainText(name) - await expect(orcaPage.locator('.monaco-editor').first()).toBeVisible() - await expect(orcaPage.locator('.monaco-editor').first()).toContainText('Rendered by default') - await orcaPage.locator('.monaco-editor').first().click() - await orcaPage.keyboard.press('ControlOrMeta+End') - await orcaPage.keyboard.press('Enter') - await orcaPage.keyboard.type('

Source updated

') - await orcaPage.keyboard.press('ControlOrMeta+S') - await expect.poll(() => readFileSync(filePath, 'utf8')).toContain('

Source updated

') - await row.click() - await expect( - orcaPage.getByRole('button', { name: 'Open source file', exact: true }) - ).toBeVisible() - await expect - .poll(() => - electronApp.evaluate(async ({ webContents }, fileUrl) => { - const guest = webContents - .getAllWebContents() - .find((contents) => contents.getURL() === fileUrl) - return guest?.executeJavaScript('document.body.textContent ?? ""') - }, url) - ) - .toContain('Source updated') - const previewTab = orcaPage.locator('[data-tab-id]').filter({ hasText: 'HTML default' }) - await expect(previewTab).toHaveCount(1) - await expect(previewTab).toBeVisible() + ).toBe(true) + await orcaPage.screenshot({ path: testInfo.outputPath('local-html-file-url.png') }) }) -for (const inputKind of ['file URL', 'absolute path', 'linked path'] as const) { - test(`opens a local HTML ${inputKind} with encoded characters from the new-tab menu`, async ({ - electronApp, - orcaPage, - testRepoPath - }, testInfo) => { - let directory = testRepoPath - if (inputKind === 'linked path') { - const target = testInfo.outputPath('external-source') - mkdirSync(target, { recursive: true }) - directory = path.join(testRepoPath, 'linked-source') - symlinkSync(target, directory, process.platform === 'win32' ? 'junction' : 'dir') - } - const filePath = path.join(directory, 'report 中%20.html') - writeFileSync(filePath, 'Local file URL

Local HTML rendered

') - const url = pathToFileURL(filePath).href - await waitForSessionReady(orcaPage) - await waitForActiveWorktree(orcaPage) - await orcaPage.getByRole('button', { name: 'New tab', exact: true }).first().click() - const input = orcaPage.getByRole('combobox', { - name: 'Search open tabs, history, files, URLs, agents…', - exact: true - }) - await input.fill(inputKind === 'file URL' ? url : filePath) - await expect(orcaPage.getByRole('option', { name: /Open file/ })).toBeVisible() - await input.press('Enter') - await expect - .poll(() => - electronApp.evaluate(async ({ webContents }, fileUrl) => { - const guest = webContents - .getAllWebContents() - .find((contents) => contents.getURL() === fileUrl) - return guest?.executeJavaScript('document.querySelector("h1")?.textContent ?? null') - }, url) - ) - .toBe('Local HTML rendered') - await expect( - orcaPage.getByRole('button', { name: 'Open source file', exact: true }) - ).toBeVisible() - await orcaPage.screenshot({ path: testInfo.outputPath('local-html-file-url.png') }) - await orcaPage.getByRole('button', { name: 'Open source file', exact: true }).click() - const editor = orcaPage.locator('.monaco-editor').first() - await expect(editor).toContainText('Local HTML rendered') - await editor.click() - await orcaPage.keyboard.press('ControlOrMeta+End') - await orcaPage.keyboard.press('Enter') - await orcaPage.keyboard.type('

Source saved

') - await orcaPage.keyboard.press('ControlOrMeta+S') - await expect.poll(() => readFileSync(filePath, 'utf8')).toContain('

Source saved

') - }) -} - test('opens a pasted HTML file URL from a paired host and reloads its assets', async ({ orcaPage, testRepoPath diff --git a/tests/e2e/html-source-control-preview.spec.ts b/tests/e2e/html-source-control-preview.spec.ts deleted file mode 100644 index fbf4b6cd373..00000000000 --- a/tests/e2e/html-source-control-preview.spec.ts +++ /dev/null @@ -1,109 +0,0 @@ -import { writeFileSync } from 'node:fs' -import path from 'node:path' -import { pathToFileURL } from 'node:url' -import { runProcessSync } from '@orca/process-host' -import { expect, test } from './helpers/orca-app' -import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' -import { - cleanupGoldenWorktree, - createGoldenWorktree, - openGoldenSourceControl -} from './helpers/golden-source-control' - -test('previews untracked HTML clicked in Source Control', async ({ - electronApp, - orcaPage -}, testInfo) => { - await waitForSessionReady(orcaPage) - const worktreeId = await waitForActiveWorktree(orcaPage) - const root = await orcaPage.evaluate( - (id) => window.__store?.getState().getKnownWorktreeById(id)?.path, - worktreeId - ) - if (!root) { - throw new Error('Active workspace missing') - } - const name = 'source-control-preview.html' - const filePath = path.join(root, name) - const url = pathToFileURL(filePath).href - writeFileSync(filePath, 'HTML preview

Rendered from Source Control

') - await orcaPage.evaluate(() => { - window.localStorage.setItem('orca.browser.markup-draw-hint-seen', 'true') - const state = window.__store?.getState() - state?.setRightSidebarOpen(true) - state?.setRightSidebarTab('source-control') - }) - const row = orcaPage.locator(`[data-source-control-path="${name}"]`) - await expect(row).toBeVisible({ timeout: 30_000 }) - await row.click() - - await expect( - orcaPage.getByRole('button', { name: 'Open source file', exact: true }) - ).toBeVisible() - await expect(orcaPage.locator('.monaco-diff-editor')).toHaveCount(0) - await expect - .poll(() => - electronApp.evaluate(async ({ webContents }, fileUrl) => { - const guest = webContents - .getAllWebContents() - .find((contents) => contents.getURL() === fileUrl) - return guest?.executeJavaScript('document.querySelector("h1")?.textContent ?? null') - }, url) - ) - .toBe('Rendered from Source Control') - await orcaPage.screenshot({ path: testInfo.outputPath('html-source-control-preview.png') }) -}) - -test('previews HTML clicked under Committed on branch', async ({ - electronApp, - orcaPage, - testRepoPath, - registerPostElectronShutdownCleanup -}, testInfo) => { - const fixture = createGoldenWorktree(testRepoPath, 'html-branch-preview') - registerPostElectronShutdownCleanup(async () => cleanupGoldenWorktree(testRepoPath, fixture)) - const name = 'committed-preview.html' - const filePath = path.join(fixture.worktreePath, name) - const url = pathToFileURL(filePath).href - writeFileSync(filePath, 'Committed HTML

Rendered from branch changes

') - for (const args of [ - ['add', '--', name], - ['commit', '-m', 'Add HTML preview fixture'] - ]) { - const result = runProcessSync({ - program: 'git', - args, - cwd: fixture.worktreePath, - stdio: 'pipe' - }) - if (result.code !== 0) { - throw new Error(result.stderr || 'Failed to prepare committed HTML fixture') - } - } - await waitForSessionReady(orcaPage) - await openGoldenSourceControl(orcaPage, testRepoPath, fixture) - await orcaPage.evaluate(() => { - window.localStorage.setItem('orca.browser.markup-draw-hint-seen', 'true') - }) - const row = orcaPage.locator( - `[data-source-control-area="branch"][data-source-control-path="${name}"]` - ) - await expect(row).toBeVisible({ timeout: 30_000 }) - await row.click() - - await expect( - orcaPage.getByRole('button', { name: 'Open source file', exact: true }) - ).toBeVisible() - await expect(orcaPage.locator('.monaco-diff-editor')).toHaveCount(0) - await expect - .poll(() => - electronApp.evaluate(async ({ webContents }, fileUrl) => { - const guest = webContents - .getAllWebContents() - .find((contents) => contents.getURL() === fileUrl) - return guest?.executeJavaScript('document.querySelector("h1")?.textContent ?? null') - }, url) - ) - .toBe('Rendered from branch changes') - await orcaPage.screenshot({ path: testInfo.outputPath('html-committed-branch-preview.png') }) -})