diff --git a/src/renderer/src/components/browser-pane/BrowserPane.remote-link-routing.test.ts b/src/renderer/src/components/browser-pane/BrowserPane.remote-link-routing.test.ts new file mode 100644 index 00000000000..2ea6375f123 --- /dev/null +++ b/src/renderer/src/components/browser-pane/BrowserPane.remote-link-routing.test.ts @@ -0,0 +1,19 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +describe('remote browser link routing', () => { + it('pins context-menu opens to the runtime that owns the pane', () => { + const source = readFileSync( + fileURLToPath(new URL('./BrowserPane.tsx', import.meta.url)), + 'utf8' + ) + const paneStart = source.indexOf('function RemoteBrowserPagePane') + const actionStart = source.indexOf('void openWorkspaceBrowserTab({', paneStart) + const actionEnd = source.indexOf('}).catch((error) => {', actionStart) + const openRequest = source.slice(actionStart, actionEnd) + + expect(openRequest).toContain('workspaceId: worktreeId') + expect(openRequest).toContain('expectedRuntimeEnvironmentId: runtimeEnvironmentId') + }) +}) diff --git a/src/renderer/src/components/browser-pane/BrowserPane.tsx b/src/renderer/src/components/browser-pane/BrowserPane.tsx index 825b0ceb3e8..9158962d88c 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPane.tsx @@ -2042,7 +2042,8 @@ function RemoteBrowserPagePane({ void openWorkspaceBrowserTab({ workspaceId: worktreeId, url: linkUrl, - intent: { kind: 'url' } + intent: { kind: 'url' }, + expectedRuntimeEnvironmentId: runtimeEnvironmentId }).catch((error) => { setPaneNotice({ kind: 'direct', diff --git a/src/renderer/src/lib/workspace-browser-tab-open.test.ts b/src/renderer/src/lib/workspace-browser-tab-open.test.ts index 05fd79eefb3..a6e9bd980c3 100644 --- a/src/renderer/src/lib/workspace-browser-tab-open.test.ts +++ b/src/renderer/src/lib/workspace-browser-tab-open.test.ts @@ -131,7 +131,16 @@ describe('openWorkspaceBrowserTab', () => { expect(createBrowserTab).not.toHaveBeenCalled() }) - it('fails closed instead of opening on a runtime other than the asserted owner', async () => { + it('fails closed when the workspace route swaps away from the pane runtime before opening', async () => { + mocks.state = { + ...ownerState(toRuntimeExecutionHostId('hub-a')), + ...browserCapableRuntime('hub-a'), + createBrowserTab: vi.fn(), + defaultBrowserSessionProfileId: 'client-profile', + defaultBrowserSessionProfileIdByHostId: {} + } + const paneRuntimeEnvironmentId = 'hub-a' + mocks.state = { ...ownerState(toRuntimeExecutionHostId('hub-b')), ...browserCapableRuntime('hub-b'), @@ -145,13 +154,57 @@ describe('openWorkspaceBrowserTab', () => { workspaceId: WORKSPACE_ID, url: 'https://example.com/', intent: { kind: 'url' }, - expectedRuntimeEnvironmentId: 'hub-a' + expectedRuntimeEnvironmentId: paneRuntimeEnvironmentId }) ).rejects.toThrow('Unable to open URL.') expect(mocks.createRemote).not.toHaveBeenCalled() expect(mocks.state.createBrowserTab).not.toHaveBeenCalled() }) + it('fails closed for a blank asserted runtime owner', async () => { + mocks.state = { + ...ownerState(toRuntimeExecutionHostId('hub-a')), + ...browserCapableRuntime('hub-a'), + createBrowserTab: vi.fn(), + defaultBrowserSessionProfileId: 'client-profile', + defaultBrowserSessionProfileIdByHostId: {} + } + + await expect( + openWorkspaceBrowserTab({ + workspaceId: WORKSPACE_ID, + url: 'https://example.com/', + intent: { kind: 'url' }, + expectedRuntimeEnvironmentId: ' ' + }) + ).rejects.toThrow('Unable to open URL.') + expect(mocks.createRemote).not.toHaveBeenCalled() + expect(mocks.state.createBrowserTab).not.toHaveBeenCalled() + }) + + it('routes an asserted paired-HUB SSH owner through that runtime', async () => { + const sshHost = toSshExecutionHostId('ssh-target') + mocks.state = { + ...ownerState(sshHost, 'hub-a'), + ...browserCapableRuntime('hub-a'), + createBrowserTab: vi.fn(), + defaultBrowserSessionProfileId: 'client-profile', + defaultBrowserSessionProfileIdByHostId: {} + } + + await openWorkspaceBrowserTab({ + workspaceId: WORKSPACE_ID, + url: 'http://localhost:3000/', + intent: { kind: 'url' }, + expectedRuntimeEnvironmentId: 'hub-a' + }) + + expect(mocks.createRemote).toHaveBeenCalledWith( + expect.objectContaining({ environmentId: 'hub-a', worktreeId: WORKSPACE_ID }) + ) + expect(mocks.state.createBrowserTab).not.toHaveBeenCalled() + }) + it('does not fall back to a client browser for an incapable asserted runtime', async () => { mocks.state = { ...ownerState(toRuntimeExecutionHostId('hub-a')), diff --git a/src/renderer/src/lib/workspace-browser-tab-open.ts b/src/renderer/src/lib/workspace-browser-tab-open.ts index a25f9cd77ca..cc6adbb5669 100644 --- a/src/renderer/src/lib/workspace-browser-tab-open.ts +++ b/src/renderer/src/lib/workspace-browser-tab-open.ts @@ -179,9 +179,12 @@ export async function openWorkspaceBrowserTab( throw openFailure(presentation.error, 'no active worktree route') } const environmentId = route.runtimeEnvironmentId?.trim() || null - const expectedEnvironmentId = request.expectedRuntimeEnvironmentId?.trim() || null + const expectedEnvironmentId = + request.expectedRuntimeEnvironmentId === undefined + ? null + : request.expectedRuntimeEnvironmentId.trim() if ( - expectedEnvironmentId && + expectedEnvironmentId !== null && !isExpectedRuntimeBrowserRoute( state, availability, diff --git a/tests/e2e/paired-remote-browser-link-open-routing.spec.ts b/tests/e2e/paired-remote-browser-link-open-routing.spec.ts new file mode 100644 index 00000000000..e66db6f9687 --- /dev/null +++ b/tests/e2e/paired-remote-browser-link-open-routing.spec.ts @@ -0,0 +1,326 @@ +import { createServer, type IncomingMessage, type Server, type ServerResponse } from 'node:http' +import type { AddressInfo } from 'node:net' +import type { Page } from '@stablyai/playwright-test' +import { LOCAL_EXECUTION_HOST_ID } from '../../src/shared/execution-host' +import { + launchHeadlessPairedRuntimeHost, + type HeadlessPairedRuntimeHost +} from './helpers/headless-paired-runtime-host' +import { expect, test } from './helpers/orca-app' +import { + launchPairedElectronClient, + type PairedElectronClient +} from './helpers/paired-electron-client' + +// What this proves: "Open Link In Orca Browser" inside a remote browser pane opens the link on the +// runtime that owns the pane — never on the client machine. The URL is a dev-server URL, so a +// client-local fallback would silently load a *different machine's* server while looking +// successful. The second act puts the workspace on this machine (the execution-host switch a user +// makes) while the pane still shows the runtime's page: the open must fail visibly in the pane +// instead of quietly landing here. Every verdict comes from outside the client under test — the +// host's own RPC connection, the DOM the user sees, and the fixture server's request log. + +const PANE_PATH = '/remote-pane' +const LINK_PATH = '/remote-link-target' +const LINK_MARKER = 'remote link target' + +type LinkFixtureServer = { + close: () => Promise + linkLoadCount: () => number + linkUrl: string + paneUrl: string +} + +async function startLinkFixtureServer(): Promise { + let linkLoadCount = 0 + const server: Server = createServer((request: IncomingMessage, response: ServerResponse) => { + const requestPath = request.url ?? '/' + if (requestPath.startsWith(LINK_PATH)) { + linkLoadCount += 1 + response.writeHead(200, { + 'cache-control': 'no-store', + 'content-type': 'text/html; charset=utf-8' + }) + response.end(`

${LINK_MARKER}

`) + return + } + if (requestPath.startsWith(PANE_PATH)) { + response.writeHead(200, { + 'cache-control': 'no-store', + 'content-type': 'text/html; charset=utf-8' + }) + // Why the viewport-filling anchor: the context menu reads the link with + // elementFromPoint over the screencast, so covering the viewport keeps the test off + // screencast coordinate mapping — any right-click lands on the link. + response.end( + `open me` + ) + return + } + response.writeHead(404, { 'content-type': 'text/plain' }) + response.end('not found') + }) + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', () => { + server.off('error', reject) + resolve() + }) + }) + const { port } = server.address() as AddressInfo + const origin = `http://127.0.0.1:${port}` + return { + close: () => + new Promise((resolve, reject) => { + // Keep-alive sockets from either browser would otherwise hold the close open. + server.closeAllConnections() + server.close((error) => (error ? reject(error) : resolve())) + }), + linkLoadCount: () => linkLoadCount, + linkUrl: `${origin}${LINK_PATH}`, + paneUrl: `${origin}${PANE_PATH}` + } +} + +/** + * What the host itself says it owns, asked over the host's own connection rather than proxied + * through the client under test — the client cannot talk its way out of this one. + */ +async function readHostBrowserUrls( + host: HeadlessPairedRuntimeHost, + worktreeId: string +): Promise { + const response = await host.client.call<{ tabs: { type: string; url?: string }[] }>( + 'session.tabs.list', + { worktree: `id:${worktreeId}` }, + { timeoutMs: 15_000 } + ) + return response.result.tabs.filter((tab) => tab.type === 'browser').map((tab) => tab.url ?? '') +} + +/** + * Browser pages this machine renders itself. A remote pane is a screencast image; a client-local + * tab is an Electron with the page really loaded here, so the URLs below are exactly the + * pages the client machine's own browser is showing. + */ +async function readLocalBrowserViewUrls(page: Page): Promise { + return page.evaluate(() => + Array.from(document.querySelectorAll('webview')).map( + (view) => (view as HTMLElement & { src?: string }).src || view.getAttribute('src') || '' + ) + ) +} + +async function readRemotePaneUrls(page: Page, worktreeId: string): Promise { + return page.evaluate((worktreeId) => { + const state = window.__store?.getState() + const workspaces = state?.browserTabsByWorktree[worktreeId] ?? [] + return workspaces + .flatMap((workspace) => state?.browserPagesByWorkspace[workspace.id] ?? []) + .filter( + (browserPage) => state?.remoteBrowserPageHandlesByPageId[browserPage.id]?.environmentId + ) + .map((browserPage) => browserPage.url) + }, worktreeId) +} + +/** The client mirrors host browser tabs on its own; this finds the mirrored page for one URL. */ +async function findMirroredPage( + page: Page, + worktreeId: string, + url: string +): Promise<{ handleEnvironmentId: string | null; pageId: string } | null> { + return page.evaluate( + ({ url, worktreeId }) => { + const state = window.__store?.getState() + for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { + for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { + if (browserPage.url.startsWith(url)) { + return { + handleEnvironmentId: + state?.remoteBrowserPageHandlesByPageId[browserPage.id]?.environmentId ?? null, + pageId: browserPage.id + } + } + } + } + return null + }, + { url, worktreeId } + ) +} + +async function focusMirroredPage(page: Page, worktreeId: string, pageId: string): Promise { + await page.evaluate( + ({ pageId, worktreeId }) => { + window.__store?.getState().focusBrowserTabInWorktree(worktreeId, pageId, { + surfacePane: true + }) + }, + { pageId, worktreeId } + ) +} + +type LinkOpenOutcome = 'opened on this machine' | 'pending' | 'refused' + +/** + * Collapses the click's aftermath into the one distinction that matters, using only what a user + * could see: did the pane refuse the open, or is this machine's browser now showing the URL? + */ +async function readLinkOpenOutcome(page: Page, linkUrl: string): Promise { + if ((await readLocalBrowserViewUrls(page)).some((url) => url.startsWith(linkUrl))) { + return 'opened on this machine' + } + const notice = page.getByTestId('remote-browser-stream-error') + const text = (await notice.count()) > 0 ? ((await notice.first().textContent()) ?? '') : '' + return text.includes('Unable to open URL.') ? 'refused' : 'pending' +} + +async function openLinkFromRemotePaneContextMenu(page: Page): Promise { + const remoteFrame = page.locator('[data-testid="remote-browser-frame"]:visible').first() + await expect(remoteFrame).toBeVisible({ timeout: 60_000 }) + await remoteFrame.click({ button: 'right', position: { x: 60, y: 60 }, force: true }) + await expect(page.getByTestId('remote-browser-context-menu')).toBeVisible({ timeout: 30_000 }) + // The item only renders once the remote hit-test resolves an anchor, so this wait is the + // wait for the link lookup itself. + const openInOrca = page.getByRole('menuitem', { name: 'Open Link In Orca Browser' }) + await expect(openInOrca).toBeVisible({ timeout: 30_000 }) + await openInOrca.click() +} + +test('opens a remote pane link on the pane runtime and refuses to fall back to the client', async ({ + testRepoPath +}, testInfo) => { + test.setTimeout(300_000) + const fixture = await startLinkFixtureServer() + const host: HeadlessPairedRuntimeHost = await launchHeadlessPairedRuntimeHost() + let client: PairedElectronClient | null = null + + try { + await host.client.call('repo.add', { path: testRepoPath, kind: 'git' }) + client = await launchPairedElectronClient(host.offer, testInfo, 'Remote browser link routing') + const page = client.page + const environmentId = client.environmentId + + await expect + .poll(() => page.evaluate(() => window.__store?.getState().allWorktrees().length ?? 0), { + timeout: 60_000, + message: 'paired client never saw a host worktree' + }) + .toBeGreaterThan(0) + const worktreeId = await page.evaluate( + () => window.__store?.getState().allWorktrees()[0]?.id ?? null + ) + if (!worktreeId) { + throw new Error('paired client did not receive the host worktree') + } + + // The workspace runs on the paired runtime, the way it does when the user picks that host. + await page.evaluate( + ({ environmentId, worktreeId }) => { + window.__store?.getState().setActiveWorktree(worktreeId, `runtime:${environmentId}`) + }, + { environmentId, worktreeId } + ) + + // The host opens the page on itself: this is the "remote server" whose links must stay remote. + await host.client.call( + 'browser.tabCreate', + { worktree: `id:${worktreeId}`, url: fixture.paneUrl, activate: true }, + { timeoutMs: 30_000 } + ) + await expect + .poll(() => findMirroredPage(page, worktreeId, fixture.paneUrl), { + timeout: 60_000, + message: 'the client never mirrored the host browser page' + }) + .not.toBeNull() + const pane = await findMirroredPage(page, worktreeId, fixture.paneUrl) + if (!pane) { + throw new Error('mirrored host browser page disappeared') + } + expect(pane.handleEnvironmentId).toBe(environmentId) + await focusMirroredPage(page, worktreeId, pane.pageId) + const paneCountBeforeOpen = await page.getByTestId('remote-browser-pane').count() + + // Act 1: the healthy path must land on the runtime, end to end. + await openLinkFromRemotePaneContextMenu(page) + + await expect + .poll( + async () => + (await readHostBrowserUrls(host, worktreeId)).filter((url) => + url.startsWith(fixture.linkUrl) + ).length, + { timeout: 60_000, message: 'the link never opened as a browser tab on the host runtime' } + ) + .toBe(1) + // The host's browser really fetched it; a tab record alone would not prove a load. + expect(fixture.linkLoadCount()).toBeGreaterThan(0) + // One more remote pane, and still nothing rendered by this machine's own browser. + await expect(page.getByTestId('remote-browser-pane')).toHaveCount(paneCountBeforeOpen + 1, { + timeout: 60_000 + }) + expect(await readRemotePaneUrls(page, worktreeId)).toContainEqual( + expect.stringContaining(fixture.linkUrl) + ) + expect(await readLocalBrowserViewUrls(page)).toHaveLength(0) + + // Drop every tab except the pane's, so the second act drives the pane it started with. + await page.evaluate( + ({ paneUrl, worktreeId }) => { + const state = window.__store?.getState() + for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { + const pages = state?.browserPagesByWorkspace[workspace.id] ?? [] + if (!pages.some((browserPage) => browserPage.url.startsWith(paneUrl))) { + state?.closeBrowserTab(workspace.id) + } + } + }, + { paneUrl: fixture.paneUrl, worktreeId } + ) + await expect(page.getByTestId('remote-browser-pane')).toHaveCount(paneCountBeforeOpen, { + timeout: 60_000 + }) + + // Act 2: the user moves this workspace onto their own machine while the runtime's page is + // still on screen. Opening the link must fail in the pane, not load the runtime's dev server + // here — the client has no business serving a page for a workspace it does not run. + await page.evaluate( + ({ localHostId, worktreeId }) => { + window.__store?.getState().setActiveWorktree(worktreeId, localHostId) + }, + { localHostId: LOCAL_EXECUTION_HOST_ID, worktreeId } + ) + await focusMirroredPage(page, worktreeId, pane.pageId) + const hostUrlsBefore = await readHostBrowserUrls(host, worktreeId) + const linkLoadsBefore = fixture.linkLoadCount() + + await openLinkFromRemotePaneContextMenu(page) + + // Wait for the click to produce an outcome — refusal or a local page — so the assertion below + // reports which one happened instead of racing past a fallback that lands a moment later. + await expect + .poll(() => readLinkOpenOutcome(page, fixture.linkUrl), { + timeout: 30_000, + message: 'the link open produced neither a refusal nor a page' + }) + .not.toBe('pending') + expect(await readLinkOpenOutcome(page, fixture.linkUrl)).toBe('refused') + + // The workspace must still be the local one, or the refusal above proved nothing. + expect( + await page.evaluate(() => window.__store?.getState().activeWorkspaceExecutionHostId ?? null) + ).toBe(LOCAL_EXECUTION_HOST_ID) + // Nothing rendered here, nothing new on the host, and nobody fetched the link anywhere. + expect(await readLocalBrowserViewUrls(page)).toHaveLength(0) + expect(await readHostBrowserUrls(host, worktreeId)).toEqual(hostUrlsBefore) + expect(fixture.linkLoadCount()).toBe(linkLoadsBefore) + } finally { + if (client) { + await client.dispose() + } + await host.dispose() + await fixture.close() + } +})