diff --git a/src/renderer/src/app-shell/use-app-shell-services.ts b/src/renderer/src/app-shell/use-app-shell-services.ts index f0cd00ab6dd..c3ad37a2692 100644 --- a/src/renderer/src/app-shell/use-app-shell-services.ts +++ b/src/renderer/src/app-shell/use-app-shell-services.ts @@ -21,6 +21,7 @@ import { useHostModelCatalogSnapshotsSync } from '../runtime/host-model-catalog- import { useRemoteRuntimeRecoveryTriggers } from '../runtime/use-remote-runtime-recovery-triggers' import { useTerminalViewerColorPublication } from './use-terminal-viewer-color-publication' import { useBrowserIdentityMigrationNotice } from '../components/browser-pane/browser-user-agent-migration-notice' +import { useBrowserClientHostedDownloadNotices } from '../components/browser-pane/browser-client-hosted-download-notices' import { useCodexTerminalServerIsolationNotice } from '../components/terminal-pane/codex-terminal-server-isolation-notice' import { useCodexSharedSettingsNotice } from '../components/terminal-pane/codex-shared-settings-notice' import { useVisibleReviewRefreshReporting } from './use-visible-review-refresh-reporting' @@ -64,6 +65,7 @@ export function useAppShellServices(): void { usePrimarySelectionPaste(primarySelectionMiddleClickPaste) useOsc52ClipboardDefaultOnNotice(persistedUIReady) useBrowserIdentityMigrationNotice() + useBrowserClientHostedDownloadNotices() useCodexTerminalServerIsolationNotice() useCodexSharedSettingsNotice() } diff --git a/src/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsx b/src/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsx index a956a35a642..b1c49a502be 100644 --- a/src/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsx +++ b/src/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsx @@ -16,7 +16,6 @@ import { startBrowserClientPageMetadataPublisher } from './browser-client-page-metadata-reporting' import { attachBrowserClientPageToViewport } from './browser-client-page-renderer-installation' -import { useBrowserClientHostedDownloadNotices } from './browser-client-hosted-download-notices' import { useBrowserClientHostedPopupNotices } from './browser-client-hosted-popup-notices' import { useBrowserClientHostedPermissionNotices } from './browser-client-hosted-permission-notices' import { useClientHostedBrowserIntroTour } from './use-client-hosted-browser-intro-tour' @@ -139,7 +138,6 @@ export function ClientHostedBrowserPagePane({ onUpdatePageStateRef }) - useBrowserClientHostedDownloadNotices(browserTab.id) useBrowserClientHostedPopupNotices(browserTab.id) useBrowserClientHostedPermissionNotices(browserTab.id) // Why: the tour points at controls that cannot work yet, and recording the interaction is a diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/browser-workspace-pane.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/browser-workspace-pane.tsx index e473f442a90..6035ef1f486 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/browser-workspace-pane.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/browser-workspace-pane.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useMemo } from 'react' import { useAppStore } from '@/store' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' +import { isClientHostedBrowserPageHandle } from '@/store/slices/browser' import type { BrowserWorkspace as BrowserWorkspaceState } from '../../../../../shared/browser-workspace-types' import { destroyPersistentWebview } from '../host-guest/webview-registry' import { useBrowserAutomationVisiblePageIds } from '../host-guest/browser-automation-visibility' @@ -118,14 +119,14 @@ export default function BrowserPane({ // first frame. The host mints the placement, so keying on it would swap component and key at // adoption — a remount that replays open dropdowns and throws away focus and drafts. The pane // renders connecting until adoption fills the placement in. - const stagedClientHosted = - environmentHandle?.staged === true && environmentHandle.stagedClientHosted === true - // Why: a row restored from a previous run has no placement until the relaunched host recovers + // A row restored from a previous run has no placement until the relaunched host recovers // the page, and the streamed pane would meanwhile open a server screencast the host refuses // for a client-placed page. It mounts quiet on the client pane until adoption fills it in. - const restoredClientHosted = environmentHandle?.restoredClientHosted === true + const clientHosted = environmentHandle + ? isClientHostedBrowserPageHandle(environmentHandle) + : false return activeBrowserPage ? ( - clientPlacement || stagedClientHosted || restoredClientHosted ? ( + clientHosted ? ( ({ @@ -10,14 +10,19 @@ const toastMocks = vi.hoisted(() => ({ vi.mock('sonner', () => ({ toast: toastMocks })) +const store = vi.hoisted(() => { + const remoteBrowserPageHandlesByPageId: Record = {} + return { state: { remoteBrowserPageHandlesByPageId } } +}) +vi.mock('@/store', () => ({ useAppStore: { getState: () => store.state } })) + import type { BrowserDownloadFinishedEvent, BrowserDownloadProgressEvent, BrowserDownloadRequestedEvent } from '../../../../shared/browser-guest-events' -import { TooltipProvider } from '@/components/ui/tooltip' -import { installClientHostedPaneApi, paneChannel } from './client-hosted-browser-pane-test-rig' -import { ClientHostedBrowserPagePane } from './ClientHostedBrowserPagePane' +import { paneChannel } from './client-hosted-browser-pane-test-rig' +import { useBrowserClientHostedDownloadNotices } from './browser-client-hosted-download-notices' let requested = paneChannel() let progress = paneChannel() @@ -27,12 +32,21 @@ beforeEach(() => { requested = paneChannel() progress = paneChannel() finished = paneChannel() - // Only the preload boundary is faked: the pane subscribes through the real window.api surface. - installClientHostedPaneApi({ - browser: { - onDownloadRequested: requested.subscribe, - onDownloadProgress: progress.subscribe, - onDownloadFinished: finished.subscribe + store.state.remoteBrowserPageHandlesByPageId = { + 'page-a': { + environmentId: 'environment-a', + remotePageId: 'remote-page-a', + placement: { kind: 'client' } + } + } + Object.defineProperty(window, 'api', { + configurable: true, + value: { + browser: { + onDownloadRequested: requested.subscribe, + onDownloadProgress: progress.subscribe, + onDownloadFinished: finished.subscribe + } } }) }) @@ -42,36 +56,8 @@ afterEach(() => { vi.clearAllMocks() }) -function renderPane(browserPageId = 'page-a'): void { - render( - - - - ) +function renderNotices(): void { + renderHook(useBrowserClientHostedDownloadNotices) } function emitRequested(overrides: Partial = {}): void { @@ -112,9 +98,9 @@ function emitProgress(overrides: Partial = {}): vo act(() => progress.emit(event)) } -describe('ClientHostedBrowserPagePane download notices', () => { +describe('client-hosted browser download notices', () => { it('names the remote workspace destination when the download lands there', () => { - renderPane() + renderNotices() emitRequested() expect(toastMocks.loading).toHaveBeenCalledWith('Downloading report.pdf…', { @@ -135,7 +121,7 @@ describe('ClientHostedBrowserPagePane download notices', () => { }) it('surfaces the fail-closed remote error instead of dropping it', () => { - renderPane() + renderNotices() emitRequested() emitFinished({ @@ -149,9 +135,12 @@ describe('ClientHostedBrowserPagePane download notices', () => { ) }) - it('reports a canceled download rather than leaving the spinner running', () => { - renderPane() + // Why: the page's pane unmounts on a page switch or close while its download keeps running, and + // an unsettled loading toast has no close button. + it('settles the toast after the page that started the download is gone', () => { + renderNotices() emitRequested() + store.state.remoteBrowserPageHandlesByPageId = {} emitFinished({ status: 'canceled' }) @@ -160,11 +149,11 @@ describe('ClientHostedBrowserPagePane download notices', () => { }) }) - it('ignores downloads belonging to another page', () => { - renderPane('page-a') + it('leaves downloads from pages this desktop does not host to their own shelf', () => { + renderNotices() - emitRequested({ browserPageId: 'page-b', downloadId: 'download-2' }) - emitFinished({ browserPageId: 'page-b', downloadId: 'download-2' }) + emitRequested({ browserPageId: 'local-page', downloadId: 'download-2' }) + emitFinished({ browserPageId: 'local-page', downloadId: 'download-2' }) expect(toastMocks.loading).not.toHaveBeenCalled() expect(toastMocks.success).not.toHaveBeenCalled() @@ -173,7 +162,7 @@ describe('ClientHostedBrowserPagePane download notices', () => { // Why: bytes for a client-hosted page land on the remote workspace, so this toast is the only // place a long download shows any movement at all. it('moves the toast forward as bytes arrive', () => { - renderPane() + renderNotices() emitRequested() toastMocks.loading.mockClear() @@ -184,20 +173,8 @@ describe('ClientHostedBrowserPagePane download notices', () => { }) }) - it('keeps the plain line when the server never reported a size', () => { - renderPane() - emitRequested() - toastMocks.loading.mockClear() - - emitProgress({ totalBytes: null }) - - expect(toastMocks.loading).toHaveBeenCalledWith('Downloading report.pdf… 2.1 MB', { - id: 'browser-download:download-1' - }) - }) - - it('ignores progress for a download this pane never started', () => { - renderPane() + it('ignores progress for a download it never claimed', () => { + renderNotices() toastMocks.loading.mockClear() emitProgress() diff --git a/src/renderer/src/components/browser-pane/browser-client-hosted-download-notices.ts b/src/renderer/src/components/browser-pane/browser-client-hosted-download-notices.ts index 081549d5198..83c256ec166 100644 --- a/src/renderer/src/components/browser-pane/browser-client-hosted-download-notices.ts +++ b/src/renderer/src/components/browser-pane/browser-client-hosted-download-notices.ts @@ -1,6 +1,8 @@ -import { useEffect, useRef } from 'react' +import { useEffect } from 'react' import { toast } from 'sonner' import { translate } from '@/i18n/i18n' +import { useAppStore } from '@/store' +import { isClientHostedBrowserPageHandle } from '@/store/slices/browser' import type { BrowserDownloadFinishedEvent, BrowserDownloadProgressEvent, @@ -18,15 +20,18 @@ function downloadToastId(downloadId: string): string { * A client-hosted page has no download shelf and its bytes never land in this desktop's Downloads * folder, so these toasts are the only signal that a download started, landed on the remote * workspace, or was refused. + * + * Why app-level: the page's pane unmounts on a page switch or close while the download keeps + * running, and a loading toast whose finish nobody hears has no close button and never expires. */ -export function useBrowserClientHostedDownloadNotices(browserPageId: string): void { - const filenamesRef = useRef>(new Map()) - +export function useBrowserClientHostedDownloadNotices(): void { useEffect(() => { - const filenames = filenamesRef.current + // Claimed at request time: the finish can arrive after the page's handle is gone. + const filenames = new Map() const releaseRequested = window.api.browser.onDownloadRequested( (event: BrowserDownloadRequestedEvent) => { - if (event.browserPageId !== browserPageId) { + const handle = useAppStore.getState().remoteBrowserPageHandlesByPageId[event.browserPageId] + if (!handle || !isClientHostedBrowserPageHandle(handle)) { return } filenames.set(event.downloadId, event.filename) @@ -38,8 +43,6 @@ export function useBrowserClientHostedDownloadNotices(browserPageId: string): vo ) } ) - // Why: progress carries no page id once the guest is gone, so a filename this pane recorded is - // what proves the download is ours — an unknown id belongs to another page and is ignored. const releaseProgress = window.api.browser.onDownloadProgress( (event: BrowserDownloadProgressEvent) => { const filename = filenames.get(event.downloadId) @@ -53,10 +56,10 @@ export function useBrowserClientHostedDownloadNotices(browserPageId: string): vo ) const releaseFinished = window.api.browser.onDownloadFinished( (event: BrowserDownloadFinishedEvent) => { - if (event.browserPageId !== browserPageId) { + const filename = filenames.get(event.downloadId) + if (filename === undefined) { return } - const filename = filenames.get(event.downloadId) ?? '' filenames.delete(event.downloadId) emitBrowserClientHostedDownloadNotice(event, filename) } @@ -65,9 +68,8 @@ export function useBrowserClientHostedDownloadNotices(browserPageId: string): vo releaseRequested() releaseProgress() releaseFinished() - filenames.clear() } - }, [browserPageId]) + }, []) } /** "Downloading report.pdf… 2.1 MB / 8 MB", falling back to the plain line when size is unknown. */ diff --git a/src/renderer/src/components/browser-pane/client-hosted-browser-pane-test-rig.ts b/src/renderer/src/components/browser-pane/client-hosted-browser-pane-test-rig.ts index b5527404c29..d2b357d92dd 100644 --- a/src/renderer/src/components/browser-pane/client-hosted-browser-pane-test-rig.ts +++ b/src/renderer/src/components/browser-pane/client-hosted-browser-pane-test-rig.ts @@ -42,9 +42,6 @@ export function installClientHostedPaneApi(overrides?: { configurable: true, value: { browser: { - onDownloadRequested: inert, - onDownloadProgress: inert, - onDownloadFinished: inert, onPopup: inert, onPermissionDenied: inert, onContextMenuRequested: inert, diff --git a/src/renderer/src/store/slices/browser.ts b/src/renderer/src/store/slices/browser.ts index 424384e9a42..bb83a79f466 100644 --- a/src/renderer/src/store/slices/browser.ts +++ b/src/renderer/src/store/slices/browser.ts @@ -14,5 +14,8 @@ export type { RemoteBrowserPageHandle } from './browser/browser-slice-contract' export { createBrowserSlice } from './browser/create-browser-slice' -export { isLocalBrowserPageOwner } from './browser/browser-host-state' +export { + isClientHostedBrowserPageHandle, + isLocalBrowserPageOwner +} from './browser/browser-host-state' export { sanitizeBrowserPageAnnotation } from './browser/browser-page-annotation' diff --git a/src/renderer/src/store/slices/browser/browser-host-state.ts b/src/renderer/src/store/slices/browser/browser-host-state.ts index 7ee75ac54a1..5c1865aa1f5 100644 --- a/src/renderer/src/store/slices/browser/browser-host-state.ts +++ b/src/renderer/src/store/slices/browser/browser-host-state.ts @@ -50,6 +50,16 @@ export function buildRestoredRemoteBrowserPageHandles( return handles } +/** Whether this desktop hosts the remote page itself rather than streaming it from the host. Staged + * and restored rows count before the host confirms a placement, so they never mount as streams. */ +export function isClientHostedBrowserPageHandle(handle: RemoteBrowserPageHandle): boolean { + return ( + handle.placement?.kind === 'client' || + (handle.staged === true && handle.stagedClientHosted === true) || + handle.restoredClientHosted === true + ) +} + export function retainCookieImportExecutionHost( result: BrowserCookieImportResult, executionHostId: ExecutionHostId,