From a5597a7a63a7d2cecbc537c150fb8f8b8b85d977 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 09:44:51 +0800 Subject: [PATCH 01/12] fix(peer-device): open unsupported PR links in the system browser --- .../generated/remote-surface-registry.json | 4 +- .../src/remote_surface/capabilities.rs | 9 ++ src/web-ui/AGENTS.md | 10 ++ .../modern/ModernFlowChatContainer.tsx | 24 +--- .../modern/useFlowChatHttpLinkClick.ts | 41 ++++++ .../api/generated/remoteSurface.ts | 4 +- .../api/service-api/SystemAPI.test.ts | 11 ++ .../markdown/MarkdownRenderer.test.tsx | 118 ++++++++++++++++++ .../peer-device/PeerConnectionManager.test.ts | 29 +++++ .../peer-device/PeerConnectionManager.ts | 5 + .../src/infrastructure/peer-device/README.md | 9 ++ .../peerCapabilityResolution.test.ts | 19 +++ .../peer-device/peerCapabilityResolution.ts | 9 ++ 13 files changed, 272 insertions(+), 20 deletions(-) create mode 100644 src/web-ui/src/flow_chat/components/modern/useFlowChatHttpLinkClick.ts diff --git a/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json b/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json index 8e3a1f7bcf..dad0ceed77 100644 --- a/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json +++ b/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "digest": "fnv1a64:e0ac67cc604f9730", + "digest": "fnv1a64:deaa9af9ff3f916e", "retiredCommandPrefixes": [ { "prefix": "lsp_", @@ -39,6 +39,7 @@ "product_control_presentation_v1", "cancel_tool", "tool_catalog", + "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", "whole_session_copy_v1", @@ -76,6 +77,7 @@ "product_control_presentation_v1", "cancel_tool", "tool_catalog", + "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", "whole_session_copy_v1", diff --git a/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs b/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs index e2e83c1e78..c6641fab32 100644 --- a/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs +++ b/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs @@ -81,6 +81,8 @@ pub enum PeerHostCapability { CancelTool, /// Read-only `get_all_tools_info` catalog is implemented. ToolCatalog, + /// Workspace-backed pull request context and detail pages are implemented. + ReviewPlatformV1, /// Mode/workspace-scoped `get_chat_mcp_catalog` is implemented. ChatMcpCatalogV1, /// Workspace references can carry the owning host's persisted ID. @@ -126,6 +128,7 @@ impl PeerHostCapability { Self::ProductControlPresentationV1, Self::CancelTool, Self::ToolCatalog, + Self::ReviewPlatformV1, Self::ChatMcpCatalogV1, Self::WorkspaceIdReferencesV1, Self::WholeSessionCopyV1, @@ -166,6 +169,7 @@ impl PeerHostCapability { Self::ProductControlPresentationV1 => "product_control_presentation_v1", Self::CancelTool => "cancel_tool", Self::ToolCatalog => "tool_catalog", + Self::ReviewPlatformV1 => "review_platform_v1", Self::ChatMcpCatalogV1 => "chat_mcp_catalog_v1", Self::WorkspaceIdReferencesV1 => "workspace_id_references_v1", Self::WholeSessionCopyV1 => "whole_session_copy_v1", @@ -294,6 +298,11 @@ mod tests { assert!(!cli.contains_key("product_control_native_v1")); assert!(!cli.contains_key("product_control_presentation_v1")); assert!(!cli.contains_key("whole_session_copy_v1")); + assert!(!cli.contains_key("review_platform_v1")); + assert_eq!( + capability_map(PeerHostKind::Desktop).get("review_platform_v1"), + Some(&Value::Bool(true)) + ); assert_eq!( capability_map(PeerHostKind::Desktop).get("whole_session_copy_v1"), Some(&Value::Bool(true)) diff --git a/src/web-ui/AGENTS.md b/src/web-ui/AGENTS.md index 08861f7bef..85a7f44280 100644 --- a/src/web-ui/AGENTS.md +++ b/src/web-ui/AGENTS.md @@ -257,3 +257,13 @@ pnpm --dir src/web-ui run test:run src/infrastructure/markdown/MermaidBlock.test ``` The service tests use the real Mermaid parser; native SVG rendering still needs UI verification. + +For chat PR/MR link routing, peer capability negotiation, and controller browser fallback: + +```bash +pnpm --dir src/web-ui run test:run src/infrastructure/markdown/MarkdownRenderer.test.tsx src/infrastructure/peer-device/PeerConnectionManager.test.ts src/infrastructure/peer-device/peerCapabilityResolution.test.ts src/infrastructure/api/service-api/SystemAPI.test.ts +``` + +These DOM and handshake fixtures cover local, legacy Desktop, CLI and unknown peers, +device changes, and the controller-owned URL opener. They do not establish live +macOS-to-Linux Peer Device or native browser behavior. diff --git a/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx b/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx index cec20f0dae..64ac15687f 100644 --- a/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx @@ -66,8 +66,8 @@ import { useConversationViewScope } from '../../contexts/conversationViewScope'; import { useWorkspaceContext } from '@/infrastructure/contexts/WorkspaceContext'; import { flowChatSessionConfigForCurrentWorkspace } from '@/app/utils/projectSessionWorkspace'; import { createLogger } from '@/shared/utils/logger'; -import { parsePullRequestUrl } from '@/shared/utils/pullRequestLinks'; -import { createBackgroundCommandOutputTab, createReviewPlatformPullRequestDetailTab } from '@/shared/utils/tabUtils'; +import { createBackgroundCommandOutputTab } from '@/shared/utils/tabUtils'; +import { useFlowChatHttpLinkClick } from './useFlowChatHttpLinkClick'; import { isAcpFlowSession } from '../../utils/acpSession'; import { flowChatStore } from '../../store/FlowChatStore'; import { openBtwSessionInAuxPane } from '../../services/btwSessionPane'; @@ -657,22 +657,10 @@ export const ModernFlowChatContainer: React.FC = ( workspacePath, onFileViewRequest, }); - const handleHttpLinkClick = useCallback((url: string, _event: React.MouseEvent) => { - const pullRequestTarget = parsePullRequestUrl(url); - if (!pullRequestTarget) { - return false; - } - - if (!activeSession?.workspaceId) return false; - createReviewPlatformPullRequestDetailTab({ - workspaceId: activeSession.workspaceId, - workspacePath: activeSession?.workspacePath || workspacePath, - pullRequestId: pullRequestTarget.pullRequestId, - pullRequestUrl: pullRequestTarget.webUrl, - title: `PR #${pullRequestTarget.pullRequestId}`, - }); - return true; - }, [activeSession?.workspaceId, activeSession?.workspacePath, workspacePath]); + const handleHttpLinkClick = useFlowChatHttpLinkClick( + activeSession?.workspaceId, + activeSession?.workspacePath || workspacePath, + ); const { searchQuery, onSearchChange: setSearchQuery, diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatHttpLinkClick.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatHttpLinkClick.ts new file mode 100644 index 0000000000..96ffd99def --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatHttpLinkClick.ts @@ -0,0 +1,41 @@ +import { useCallback } from 'react'; +import { systemAPI } from '@/infrastructure/api/service-api/SystemAPI'; +import { usePeerDeviceModeOptional } from '@/infrastructure/peer-device/peerDeviceContextState'; +import { canViewPullRequestsOnSurface } from '@/infrastructure/peer-device/peerCapabilityResolution'; +import { createLogger } from '@/shared/utils/logger'; +import { parsePullRequestUrl } from '@/shared/utils/pullRequestLinks'; +import { createReviewPlatformPullRequestDetailTab } from '@/shared/utils/tabUtils'; + +const log = createLogger('FlowChatHttpLink'); + +export function useFlowChatHttpLinkClick(workspaceId?: string, workspacePath?: string) { + const peer = usePeerDeviceModeOptional(); + const canViewPullRequests = canViewPullRequestsOnSurface( + peer?.peerMode.active ?? false, + peer?.currentPeerCapabilities ?? null, + ); + + return useCallback((url: string, event: React.MouseEvent): boolean => { + const target = parsePullRequestUrl(url); + if (!target) return false; + + const externalModifier = event.metaKey || event.ctrlKey || event.shiftKey || event.altKey; + if (!canViewPullRequests || !workspaceId || externalModifier) { + // Consume the click: returning false opens the built-in browser. The + // system opener belongs to the controller, never the selected peer. + void systemAPI.openExternal(url).catch(error => { + log.error('Failed to open pull request URL', { url, error }); + }); + return true; + } + + createReviewPlatformPullRequestDetailTab({ + workspaceId, + workspacePath, + pullRequestId: target.pullRequestId, + pullRequestUrl: target.webUrl, + title: `PR #${target.pullRequestId}`, + }); + return true; + }, [canViewPullRequests, workspaceId, workspacePath]); +} diff --git a/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts b/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts index 25878df83d..35c7a8880a 100644 --- a/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts +++ b/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts @@ -1,6 +1,6 @@ // Generated by scripts/generate-interactive-capabilities.mjs; do not edit. // Source: openbitfun_product_domains::remote_surface (Product Operation Registry). -export const REMOTE_SURFACE_REGISTRY_DIGEST = "fnv1a64:e0ac67cc604f9730" as const; +export const REMOTE_SURFACE_REGISTRY_DIGEST = "fnv1a64:deaa9af9ff3f916e" as const; /** * Registered Tauri commands the Peer Device controller keeps on the controller @@ -200,6 +200,7 @@ export const PEER_HOST_CAPABILITY_IDS = [ "product_control_presentation_v1", "cancel_tool", "tool_catalog", + "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", "whole_session_copy_v1", @@ -244,6 +245,7 @@ export const PEER_HOST_ADVERTISED_CAPABILITIES: Readonly< "product_control_presentation_v1", "cancel_tool", "tool_catalog", + "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", "whole_session_copy_v1", diff --git a/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts index 070da9c3d5..ef352e0f77 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts @@ -5,6 +5,8 @@ const invokeMock = vi.hoisted(() => vi.fn()); const copyTextToClipboardMock = vi.hoisted(() => vi.fn()); const controllerInvoke = vi.hoisted(() => vi.fn()); const localVersion = vi.hoisted(() => vi.fn()); +const openUrlMock = vi.hoisted(() => vi.fn()); +vi.mock('@tauri-apps/plugin-opener', () => ({ openUrl: openUrlMock })); vi.mock('@tauri-apps/api/core', () => ({ invoke: controllerInvoke })); vi.mock('@tauri-apps/api/app', () => ({ getVersion: localVersion })); @@ -27,6 +29,7 @@ describe('SystemAPI', () => { copyTextToClipboardMock.mockReset(); controllerInvoke.mockReset(); localVersion.mockReset(); + openUrlMock.mockReset(); }); afterEach(() => { @@ -34,6 +37,14 @@ describe('SystemAPI', () => { vi.unstubAllGlobals(); }); + it('opens external URLs with the controller OS without invoking the product transport', async () => { + const url = 'https://github.com/GCWing/OpenBitFun/pull/3573'; + await systemAPI.openExternal(url); + expect(openUrlMock).toHaveBeenCalledWith(url); + expect(invokeMock).not.toHaveBeenCalled(); + expect(controllerInvoke).not.toHaveBeenCalled(); + }); + it.each([true, false])('allows explicit update checks with DEV=%s', async development => { vi.stubEnv('DEV', development); const response = { diff --git a/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx b/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx index 2090d6a707..e9a5d658a8 100644 --- a/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx +++ b/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx @@ -14,6 +14,9 @@ import { appManager } from '@/app/services/AppManager'; import { flowChatStore } from '@/flow_chat/store/FlowChatStore'; import type { Session } from '@/flow_chat/types/flow-chat'; import { activateSurface, getActiveSurfaceId } from '@/infrastructure/peer-device/deviceSurface'; +import { PeerDeviceContext, type PeerDeviceContextValue } from '@/infrastructure/peer-device/peerDeviceContextState'; +import type { PeerHostCapabilities } from '@/infrastructure/peer-device/PeerConnectionManager'; +import { useFlowChatHttpLinkClick } from '@/flow_chat/components/modern/useFlowChatHttpLinkClick'; /** The chat session's workspace record; content opened from chat links is routed by its ID. */ const SESSION_WORKSPACE = vi.hoisted(() => ({ @@ -48,6 +51,10 @@ vi.mock('@/infrastructure/api', () => ({ }, })); +vi.mock('@/infrastructure/api/service-api/SystemAPI', () => ({ + systemAPI: { openExternal: (...args: unknown[]) => mocks.openExternal(...args) }, +})); + vi.mock('@/infrastructure/i18n', () => ({ i18nService: { t: (key: string, options?: { defaultValue?: string }) => options?.defaultValue ?? key, @@ -120,6 +127,34 @@ vi.mock('@/infrastructure/services/business/workspaceManager', () => ({ const EXAMPLE_WORKSPACE = 'C:\\ExampleWorkspace'; const EXAMPLE_ABSOLUTE_README = 'D:\\SampleDocs\\Guides\\README.md'; +function ChatPullRequestLink({ url, workspaceId = SESSION_WORKSPACE.id }: { url: string; workspaceId?: string }) { + const onHttpLinkClick = useFlowChatHttpLinkClick(workspaceId, SESSION_WORKSPACE.rootPath); + return ; +} + +function peerContext(capabilities: Partial | null): PeerDeviceContextValue { + return { + peerMode: { active: true, deviceId: 'linux-peer', deviceName: 'Linux CLI' }, + attachments: [], + currentPeerCapabilities: capabilities === null ? null : { + idempotentDialogSubmit: false, + targetedSessionRollback: false, + tokenUsageStatistics: false, + miniAppAgentContextFilesV1: false, + miniAppAgentImagesV1: false, + cancelTool: null, + toolCatalog: null, + userQuestionResponse: null, + hostKind: null, + ...capabilities, + }, + switchToDevice: vi.fn(), + switchToLocal: vi.fn(), + disconnectDevice: vi.fn(), + disconnectAllDevices: vi.fn(), + }; +} + describe('Markdown file links', () => { let container: HTMLDivElement; let root: Root; @@ -170,6 +205,7 @@ describe('Markdown file links', () => { mocks.revealInExplorer.mockReset(); mocks.readFileContent.mockReset(); mocks.openExternal.mockReset(); + mocks.openExternal.mockResolvedValue(undefined); mocks.openFileInBestTarget.mockReset(); mocks.openHtmlFileInExternalBrowser.mockReset(); mocks.renderMath.mockReset(); @@ -479,6 +515,88 @@ Second paragraph. } }); + it.each([ + ['legacy CLI', { hostKind: 'cli' }], + ['unprobed peer', null], + ['unknown peer', { hostKind: null }], + ['explicitly unsupported Desktop', { hostKind: 'desktop', reviewPlatformV1: false }], + ] as const)('opens PR links in the system browser for %s without creating a panel', async (_label, capabilities) => { + container.className = 'openbitfun-session-scene modern-flowchat-container'; + activateSurface('linux-peer'); + openSessionHost(); + const url = 'https://github.com/GCWing/OpenBitFun/pull/3573/files?diff=split#diff-example'; + await act(async () => root.render( + + + , + )); + await act(async () => container.querySelector('a')?.click()); + + expect(mocks.openExternal).toHaveBeenCalledExactlyOnceWith(url); + expect(useAgentCanvasStore.getState().primaryGroup.tabs).toHaveLength(0); + expect(selectSessionPaneMode(sessionPaneLayoutStore.getState())).toBe('chat-only'); + expect(mocks.getCurrentWorkspacePath).not.toHaveBeenCalled(); + }); + + it.each([ + ['local', null], + ['legacy Desktop', peerContext({ hostKind: 'desktop' })], + ['capable peer', peerContext({ hostKind: 'cli', reviewPlatformV1: true })], + ] as const)('keeps supported PR links in the detail panel on %s', async (_label, peer) => { + container.className = 'openbitfun-session-scene modern-flowchat-container'; + if (peer) activateSurface('linux-peer'); + openSessionHost(); + const url = 'https://github.com/GCWing/OpenBitFun/pull/3573'; + await act(async () => root.render( + + + , + )); + await act(async () => container.querySelector('a')?.click()); + + expect(mocks.openExternal).not.toHaveBeenCalled(); + expect(useAgentCanvasStore.getState().primaryGroup.tabs).toHaveLength(1); + expect(useAgentCanvasStore.getState().primaryGroup.tabs[0].content).toMatchObject({ + type: 'review-platform-pr-detail', + data: { workspaceId: SESSION_WORKSPACE.id, pullRequestId: '3573', pullRequestUrl: url }, + }); + }); + + it('updates PR link routing when switching from a Desktop peer to a CLI peer', async () => { + container.className = 'openbitfun-session-scene modern-flowchat-container'; + const url = 'https://gitlab.example.com/team/repo/-/merge_requests/12'; + const renderLink = (hostKind: 'desktop' | 'cli') => root.render( + + + , + ); + await act(async () => renderLink('desktop')); + await act(async () => renderLink('cli')); + await act(async () => container.querySelector('a')?.click()); + expect(mocks.openExternal).toHaveBeenCalledExactlyOnceWith(url); + expect(useAgentCanvasStore.getState().primaryGroup.tabs).toHaveLength(0); + }); + + it.each(['metaKey', 'ctrlKey', 'shiftKey', 'altKey'])('honors %s on a PR link', async modifier => { + container.className = 'openbitfun-session-scene modern-flowchat-container'; + openSessionHost(); + const url = 'https://github.com/GCWing/OpenBitFun/pull/3573'; + await act(async () => root.render()); + await act(async () => container.querySelector('a')?.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true, [modifier]: true }), + )); + expect(mocks.openExternal).toHaveBeenCalledExactlyOnceWith(url); + expect(useAgentCanvasStore.getState().primaryGroup.tabs).toHaveLength(0); + }); + + it('opens PR links without a workspace in the system browser', async () => { + const url = 'https://gitee.com/team/repo/pulls/12'; + await act(async () => root.render()); + await act(async () => container.querySelector('a')?.click()); + expect(mocks.openExternal).toHaveBeenCalledExactlyOnceWith(url); + expect(useAgentCanvasStore.getState().primaryGroup.tabs).toHaveLength(0); + }); + it('opens chat http links in the built-in browser by default', async () => { container.className = 'openbitfun-session-scene modern-flowchat-container'; openSessionHost(); diff --git a/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.test.ts b/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.test.ts index d6366ab6d1..5ab817d225 100644 --- a/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.test.ts +++ b/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.test.ts @@ -87,6 +87,7 @@ describe('PeerConnectionManager attach', () => { capabilities: { cancel_tool: true, tool_catalog: true, + review_platform_v1: true, chat_mcp_catalog_v1: true, user_question_response: true, user_question_interaction_v1: true, @@ -106,6 +107,7 @@ describe('PeerConnectionManager attach', () => { const caps = connection.getState().capabilities; expect(caps.cancelTool).toBe(true); expect(caps.toolCatalog).toBe(true); + expect(caps.reviewPlatformV1).toBe(true); expect(caps.chatMcpCatalogV1).toBe(true); expect(caps.miniAppAgentContextFilesV1).toBe(true); expect(caps.miniAppAgentImagesV1).toBe(true); @@ -139,6 +141,7 @@ describe('PeerConnectionManager attach', () => { const desktop = await makeManager('desktop').connect('peer-1', 'Studio'); expect(desktop.getState().capabilities.hostKind).toBe('desktop'); + expect(desktop.getState().capabilities.reviewPlatformV1).toBeNull(); expect(desktop.getState().capabilities.inlineImageAttachmentsV1).toBe(false); expect(desktop.getState().capabilities.wholeSessionCopyV1).toBe(false); expect(desktop.getState().capabilities.btwInitialModelSelectionV1).toBe(false); @@ -147,6 +150,7 @@ describe('PeerConnectionManager attach', () => { const cli = await makeManager('cli').connect('peer-2', 'Studio'); expect(cli.getState().capabilities.hostKind).toBe('cli'); + expect(cli.getState().capabilities.reviewPlatformV1).toBeNull(); const unknown = await makeManager(undefined).connect('peer-3', 'Studio'); expect(unknown.getState().capabilities.hostKind).toBeNull(); @@ -518,6 +522,31 @@ describe('PeerConnectionManager disposal', () => { expect(snapshots.length).toBeGreaterThan(0); }); + it('publishes PR capability changes and preserves missing legacy fields', async () => { + let capabilities: Record = { review_platform_v1: true }; + const manager = createManager(async (_target: string, commandJson: string) => { + const parsed = JSON.parse(commandJson) as { command?: string }; + return JSON.stringify({ resp: 'host_invoke_result', ok: true, value: + parsed.command === 'peer_mode_ping' ? { host_type: 'desktop', capabilities } : null, + }); + }); + const connection = await manager.connect('peer-1', 'Studio'); + expect(connection.getState().capabilities.reviewPlatformV1).toBe(true); + const listener = vi.fn(); + manager.subscribe(listener); + + capabilities = { review_platform_v1: false }; + await vi.advanceTimersByTimeAsync(KEEPALIVE_MS); + expect(connection.getState().capabilities.reviewPlatformV1).toBe(false); + expect(listener).toHaveBeenCalled(); + + listener.mockClear(); + capabilities = {}; + await vi.advanceTimersByTimeAsync(KEEPALIVE_MS); + expect(connection.getState().capabilities.reviewPlatformV1).toBeNull(); + expect(listener).toHaveBeenCalled(); + }); + it('withdraws session copying from UI and transport when a ready host downgrades', async () => { let capabilities: Record = { whole_session_copy_v1: true, tool_catalog: true }; const deviceRpc = vi.fn(async (_target: string, commandJson: string) => { diff --git a/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.ts b/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.ts index d70332cb84..93401c1bad 100644 --- a/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.ts +++ b/src/web-ui/src/infrastructure/peer-device/PeerConnectionManager.ts @@ -92,6 +92,8 @@ export interface PeerHostCapabilities { * (older host); resolve via `hostKind` the same way as `cancelTool`. */ readonly toolCatalog: boolean | null; + /** Missing on older hosts: Desktop implements PR details, CLI does not. */ + readonly reviewPlatformV1?: boolean | null; /** Scoped MCP choices must be explicitly advertised by the host. */ readonly chatMcpCatalogV1?: boolean; readonly workspaceIdReferencesV1?: boolean; @@ -208,6 +210,7 @@ const NO_CAPABILITIES: PeerHostCapabilities = { // Consumers treat `null` optimistically so an unprobed host is not gated off. cancelTool: null, toolCatalog: null, + reviewPlatformV1: null, chatMcpCatalogV1: false, workspaceIdReferencesV1: false, userQuestionResponse: null, @@ -563,6 +566,7 @@ export class PeerConnectionManager { caps?.product_control_presentation_v1 === true, cancelTool, toolCatalog, + reviewPlatformV1: caps?.review_platform_v1 === undefined ? null : caps.review_platform_v1 === true, chatMcpCatalogV1: caps?.chat_mcp_catalog_v1 === true, workspaceIdReferencesV1: caps?.workspace_id_references_v1 === true, userQuestionResponse, @@ -815,6 +819,7 @@ function capabilitiesEqual( a.computerUseSharingV1 === b.computerUseSharingV1 && a.cancelTool === b.cancelTool && a.toolCatalog === b.toolCatalog && + a.reviewPlatformV1 === b.reviewPlatformV1 && a.chatMcpCatalogV1 === b.chatMcpCatalogV1 && a.workspaceIdReferencesV1 === b.workspaceIdReferencesV1 && a.userQuestionResponse === b.userQuestionResponse && diff --git a/src/web-ui/src/infrastructure/peer-device/README.md b/src/web-ui/src/infrastructure/peer-device/README.md index 6b6ba01a51..301320a9d5 100644 --- a/src/web-ui/src/infrastructure/peer-device/README.md +++ b/src/web-ui/src/infrastructure/peer-device/README.md @@ -259,3 +259,12 @@ listener alone recovers an interaction emitted before attachment. replaces the capability, including when a host restarts on an older build. Once a copy is acknowledged, a metadata-read failure reports the created copy and asks for a list refresh; it must not replay the copy mutation. + +20. **Chat PR/MR links require a supported peer review platform.** + `review_platform_v1` advertises the workspace-backed PR viewer. Older + Desktop hosts already implement it; CLI and unknown/unprobed hosts use + the controller's system URL opener. Consume that click so the ordinary + Markdown path cannot create an integrated browser or an unsupported PR + panel. This only opens the clicked URL: it never queries controller-local + repositories or credentials on behalf of a peer. Preserve URL fragments + and queries, and re-evaluate support after capability or surface changes. diff --git a/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.test.ts b/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.test.ts index 088e92a4f0..a8c4bb1e19 100644 --- a/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.test.ts +++ b/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { canCopySessionsOnSurface, + canViewPullRequestsOnSurface, canQueryToolCatalogOnSurface, canSubmitUserQuestionsOnSurface, canQuerySubscriptionQuotaOnSurface, @@ -23,6 +24,24 @@ function caps(overrides: Partial): PeerHostCapabilities { }; } +describe('canViewPullRequestsOnSurface', () => { + it('keeps local and legacy Desktop PR viewers available', () => { + expect(canViewPullRequestsOnSurface(false, null)).toBe(true); + expect(canViewPullRequestsOnSurface(true, caps({ hostKind: 'desktop' }))).toBe(true); + }); + + it('uses the external URL for CLI, unknown and unprobed peers', () => { + expect(canViewPullRequestsOnSurface(true, null)).toBe(false); + expect(canViewPullRequestsOnSurface(true, caps({ hostKind: null }))).toBe(false); + expect(canViewPullRequestsOnSurface(true, caps({ hostKind: 'cli' }))).toBe(false); + }); + + it('prefers negotiated support over host kind', () => { + expect(canViewPullRequestsOnSurface(true, caps({ hostKind: 'cli', reviewPlatformV1: true }))).toBe(true); + expect(canViewPullRequestsOnSurface(true, caps({ hostKind: 'desktop', reviewPlatformV1: false }))).toBe(false); + }); +}); + it('requires explicit host allowance-query capability and never falls back to controller credentials', () => { expect(canQuerySubscriptionQuotaOnSurface(false, null)).toBe(true); expect(canQuerySubscriptionQuotaOnSurface(true, null)).toBe(false); diff --git a/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.ts b/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.ts index c1f90fd016..221b809568 100644 --- a/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.ts +++ b/src/web-ui/src/infrastructure/peer-device/peerCapabilityResolution.ts @@ -1,5 +1,14 @@ import type { PeerHostCapabilities } from './PeerConnectionManager'; +/** Unknown peers use the URL; older Desktop hosts already support PR details. */ +export function canViewPullRequestsOnSurface( + peerActive: boolean, + capabilities: PeerHostCapabilities | null, +): boolean { + if (!peerActive) return true; + return capabilities?.reviewPlatformV1 ?? (capabilities?.hostKind === 'desktop'); +} + /** Older Desktop and CLI peer hosts do not implement the dedicated copy command. */ export function canCopySessionsOnSurface( peerActive: boolean, From 304500ba5c63aa416624532fb60ed8d89e07d2b9 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 09:55:31 +0800 Subject: [PATCH 02/12] fix(peer-device): gate PR entry points and fence device switches --- src/web-ui/AGENTS.md | 4 +- .../ReviewPlatformPanel.test.tsx | 69 ++++++++++++-- .../review-platform/ReviewPlatformPanel.tsx | 49 ++++++++-- .../components/modern/FlowChatHeader.test.tsx | 76 ++++++++++++++++ .../components/modern/FlowChatHeader.tsx | 63 +++++++++---- .../modern/useFlowChatHttpLinkClick.ts | 9 +- .../tool-cards/BuiltinToolCard.test.tsx | 90 +++++++++++++++++++ .../flow_chat/tool-cards/BuiltinToolCard.tsx | 10 ++- .../src/infrastructure/peer-device/README.md | 4 + .../peer-device/useReviewPlatformSupport.ts | 10 +++ src/web-ui/src/locales/en-US/common.json | 1 + src/web-ui/src/locales/zh-CN/common.json | 1 + src/web-ui/src/locales/zh-TW/common.json | 1 + 13 files changed, 346 insertions(+), 41 deletions(-) create mode 100644 src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx create mode 100644 src/web-ui/src/infrastructure/peer-device/useReviewPlatformSupport.ts diff --git a/src/web-ui/AGENTS.md b/src/web-ui/AGENTS.md index 85a7f44280..516f6295ac 100644 --- a/src/web-ui/AGENTS.md +++ b/src/web-ui/AGENTS.md @@ -261,9 +261,9 @@ The service tests use the real Mermaid parser; native SVG rendering still needs For chat PR/MR link routing, peer capability negotiation, and controller browser fallback: ```bash -pnpm --dir src/web-ui run test:run src/infrastructure/markdown/MarkdownRenderer.test.tsx src/infrastructure/peer-device/PeerConnectionManager.test.ts src/infrastructure/peer-device/peerCapabilityResolution.test.ts src/infrastructure/api/service-api/SystemAPI.test.ts +pnpm --dir src/web-ui run test:run src/infrastructure/markdown/MarkdownRenderer.test.tsx src/infrastructure/peer-device/PeerConnectionManager.test.ts src/infrastructure/peer-device/peerCapabilityResolution.test.ts src/infrastructure/api/service-api/SystemAPI.test.ts src/flow_chat/components/modern/FlowChatHeader.test.tsx src/flow_chat/tool-cards/BuiltinToolCard.test.tsx src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx ``` These DOM and handshake fixtures cover local, legacy Desktop, CLI and unknown peers, -device changes, and the controller-owned URL opener. They do not establish live +device changes, overview/tool-card gates, restored panels, and the controller-owned URL opener. They do not establish live macOS-to-Linux Peer Device or native browser behavior. diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx index 1974c7f3f8..a01c1068b8 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx @@ -5,9 +5,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { ReviewPlatformPullRequest, ReviewPlatformPullRequestDetailPage, ReviewPlatformWorkspaceSnapshot, ReviewRepositoryLocator } from '@/infrastructure/api/service-api/ReviewPlatformAPI'; import { ReviewPlatformPanel } from './ReviewPlatformPanel'; import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; +import type { PeerHostCapabilities } from '@/infrastructure/peer-device/PeerConnectionManager'; -const mocks = vi.hoisted(() => ({ snapshot: vi.fn(), detail: vi.fn(), openExternal: vi.fn(), t: (key: string) => key })); -vi.mock('@/infrastructure/api', () => ({ reviewPlatformAPI: { getWorkspaceSnapshot: mocks.snapshot, getPullRequestDetailPage: mocks.detail }, systemAPI: { openExternal: mocks.openExternal } })); +const mocks = vi.hoisted(() => ({ snapshot: vi.fn(), context: vi.fn(), detail: vi.fn(), openExternal: vi.fn(), t: (key: string) => key })); +const peer = vi.hoisted(() => ({ active: false, capabilities: null as Partial | null })); +vi.mock('@/infrastructure/peer-device/peerDeviceContextState', () => ({ + usePeerDeviceModeOptional: () => peer.active + ? { peerMode: { active: true }, currentPeerCapabilities: peer.capabilities } + : null, +})); +vi.mock('@/infrastructure/api', () => ({ reviewPlatformAPI: { getWorkspaceSnapshot: mocks.snapshot, getWorkspaceContext: mocks.context, getPullRequestDetailPage: mocks.detail }, systemAPI: { openExternal: mocks.openExternal } })); vi.mock('@/infrastructure/markdown', () => ({ MarkdownRenderer: () => null })); vi.mock('@/shared/notification-system', () => ({ notificationService: {} })); vi.mock('@/infrastructure/i18n', () => ({ useI18n: () => ({ t: mocks.t }), i18nService: { t: mocks.t, formatDate: () => '' } })); @@ -86,14 +93,64 @@ function statisticText(owner: Element, id: string): string | null | undefined { describe('Gitee panel state and asynchronous request ordering', () => { beforeEach(() => { Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + activateSurface('local'); + peer.active = false; + peer.capabilities = null; host = document.createElement('div'); document.body.append(host); root = createRoot(host); mocks.snapshot.mockReset().mockImplementation(snapshotFor()); + mocks.context.mockReset(); mocks.detail.mockReset().mockImplementation(() => new Promise(() => {})); mocks.openExternal.mockReset().mockResolvedValue(undefined); }); - afterEach(async () => { await act(async () => root.unmount()); host.remove(); }); + afterEach(async () => { await act(async () => root.unmount()); host.remove(); activateSurface('local'); }); + + it.each([null, { hostKind: 'cli' }, { hostKind: 'desktop', reviewPlatformV1: false }] as const)( + 'keeps unsupported restored panels out of the peer transport: %j', async capabilities => { + peer.active = true; + peer.capabilities = capabilities; + const url = 'https://github.com/GCWing/OpenBitFun/pull/3573/files#diff-example'; + await act(async () => root.render( + , + )); + expect(host.textContent).toContain('common:reviewPlatform.errors.hostUnsupported'); + expect(mocks.snapshot).not.toHaveBeenCalled(); + expect(mocks.context).not.toHaveBeenCalled(); + expect(mocks.detail).not.toHaveBeenCalled(); + await act(async () => host.querySelector('button')?.click()); + expect(mocks.openExternal).toHaveBeenCalledExactlyOnceWith(url); + }, + ); + + it('replaces a mounted panel on a device switch even when workspace identities match', async () => { + const old = deferred(); + mocks.snapshot.mockImplementationOnce(() => old.promise); + const path = await mount(); + const selected = snapshot(path); + selected.pullRequests[0].title = 'Selected peer result'; + mocks.snapshot.mockResolvedValueOnce(selected); + await act(async () => { activateSurface('other-peer'); }); + const departed = snapshot(path); + departed.pullRequests[0].title = 'Departed peer result'; + await act(async () => old.resolve(departed)); + expect(mocks.snapshot).toHaveBeenCalledTimes(2); + expect(host.textContent).toContain('Selected peer result'); + expect(host.textContent).not.toContain('Departed peer result'); + }); + + it('keeps a browser action when a supported peer cannot load the PR context', async () => { + const url = 'https://github.com/GCWing/OpenBitFun/pull/3573'; + mocks.context.mockRejectedValueOnce(new Error('Provider unavailable')); + await act(async () => root.render( + , + )); + const browser = [...host.querySelectorAll('button')] + .find(button => button.textContent === 'common:file.openInSystemBrowser'); + expect(browser).toBeDefined(); + await act(async () => browser?.click()); + expect(mocks.openExternal).toHaveBeenCalledExactlyOnceWith(url); + }); it('never serves one device\'s pull requests to another device with the same workspace ID', async () => { // Same-path local workspaces hash to one workspace ID on every device. @@ -107,17 +164,17 @@ describe('Gitee panel state and asynchronous request ordering', () => { const late = deferred(); mocks.snapshot.mockImplementationOnce(() => late.promise); await remount(); - activateSurface('peer-b'); + await act(async () => { activateSurface('peer-b'); }); await act(async () => late.resolve(snapshot(path))); await remount(); expect(mocks.snapshot).toHaveBeenCalledTimes(2); await remount(); expect(mocks.snapshot).toHaveBeenCalledTimes(2); - activateSurface('local'); + await act(async () => { activateSurface('local'); }); await remount(); expect(mocks.snapshot).toHaveBeenCalledTimes(3); } finally { - activateSurface('local'); + await act(async () => { activateSurface('local'); }); } }); diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx index cb811693d5..86817e39e2 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx @@ -41,7 +41,7 @@ import { DialogHeading, DialogTitle, } from '@openbitfun/ui'; -import React, { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useId, useMemo, useRef, useState, useSyncExternalStore } from 'react'; import { GitPullRequest, GitPullRequestClosed, KeyRound, MessageSquareText, MoreHorizontal } from 'lucide-react'; import { MarkdownRenderer } from '@/infrastructure/markdown'; import { reviewPlatformAPI, systemAPI, type ReviewPlatformAccount, type ReviewPlatformAuthChallenge, type ReviewPlatformCiItem, type ReviewPlatformCiLog, type ReviewPlatformCommit, type ReviewPlatformDetailSection, type ReviewPlatformFile, type ReviewPlatformPagination, type ReviewPlatformPullRequest, type ReviewPlatformPullRequestDetail, type ReviewPlatformPullRequestDetailPage, type ReviewPlatformRemote, type ReviewPlatformRepositoryRef, type ReviewPlatformThread, type ReviewPlatformWorkspaceSnapshot } from '@/infrastructure/api'; @@ -66,7 +66,8 @@ import { withGitRepositoryTrustRecovery, } from '@/shared/services/gitTrustService'; import type { PullRequestContext } from '@/shared/types/context'; -import { getActiveSurfaceId, onSurfaceActivated, surfaceScopedKey } from '@/infrastructure/peer-device/deviceSurface'; +import { getActiveSurfaceId, getActiveSurfaceScope, onSurfaceActivated, surfaceScopedKey } from '@/infrastructure/peer-device/deviceSurface'; +import { useReviewPlatformSupport } from '@/infrastructure/peer-device/useReviewPlatformSupport'; import { currentPullRequestReviewStatusText, effectivePullRequestReviewFreshness, @@ -712,14 +713,47 @@ function canExpandCiItem(remote: ReviewPlatformRemote | null, item: ReviewPlatfo return canLoadCiLog(remote, item) || Boolean(item.log || item.detail || item.stage || item.webUrl || item.startedAt || item.finishedAt); } -export const ReviewPlatformPanel: React.FC = ({ +export const ReviewPlatformPanel: React.FC = (props) => { + const supported = useReviewPlatformSupport(); + const surface = useSyncExternalStore(onSurfaceActivated, getActiveSurfaceScope, getActiveSurfaceScope); + const { t } = useI18n('panels/git'); + + if (!supported) { + const url = props.initialPullRequestUrl; + return ( +
+ } + description={t('common:reviewPlatform.errors.hostUnsupported')} + data-testid="review-platform-unsupported" + data-openbitfun-product-component="review-platform" + data-openbitfun-product-part="emptyState" + actions={url && parsePullRequestUrl(url) ? ( + + ) : undefined} + /> +
+ ); + } + + // The same workspace ID/path can exist on two hosts. Recreate state and + // invalidate in-flight effects even if the panel itself survives the switch. + return ; +}; + +function ReviewPlatformContent({ workspacePath, workspaceId, initialRemoteId, initialPullRequestId, initialPullRequestUrl, detailOnly = false, -}) => { +}: ReviewPlatformPanelProps) { const { t } = useI18n('panels/git'); const { t: tReview } = useI18n('flow-chat'); const authFormId = useId(); @@ -2193,6 +2227,11 @@ export const ReviewPlatformPanel: React.FC = ({ + {initialPullRequestTarget && ( + + )} {selectedRemote && selectedRemote.platform !== 'unknown' && ( - {pullRequestOverview.status === 'loading' || pullRequestOverview.status === 'idle' ? ( + {pullRequestOverview.status === 'unsupported' ? ( +
+ {pullRequestOverviewSummary} +
+ ) : pullRequestOverview.status === 'loading' || pullRequestOverview.status === 'idle' ? (
): boolean => { const target = parsePullRequestUrl(url); diff --git a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx new file mode 100644 index 0000000000..07e5fac780 --- /dev/null +++ b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx @@ -0,0 +1,90 @@ +// @vitest-environment jsdom +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { FlowToolItem, Session } from '../types/flow-chat'; +import type { PeerHostCapabilities } from '@/infrastructure/peer-device/PeerConnectionManager'; +import { BuiltinToolCard } from './BuiltinToolCard'; +import { getToolItemCardConfig } from './toolCardMetadata'; + +const state = vi.hoisted(() => ({ + peer: false, + capabilities: null as Partial | null, + sessions: new Map(), + openExternal: vi.fn(), + openPanel: vi.fn(), +})); +vi.mock('@/infrastructure/peer-device/peerDeviceContextState', () => ({ + usePeerDeviceModeOptional: () => state.peer + ? { peerMode: { active: true }, currentPeerCapabilities: state.capabilities } + : null, +})); +vi.mock('../store/FlowChatStore', () => ({ flowChatStore: { + getState: () => ({ sessions: state.sessions }), + subscribe: () => () => {}, +} })); +vi.mock('../services/sessionActivation', () => ({ openMainSession: vi.fn() })); +vi.mock('../services/btwSessionPane', () => ({ openBtwSessionInAuxPane: vi.fn() })); +vi.mock('@/infrastructure/api/service-api/SystemAPI', () => ({ systemAPI: { openExternal: state.openExternal } })); +vi.mock('@/shared/utils/tabUtils', () => ({ createReviewPlatformTab: state.openPanel })); +vi.mock('@/infrastructure/i18n/hooks/useI18n', () => ({ + useI18n: () => ({ t: (key: string) => key, formatNumber: String }), +})); + +const url = 'https://github.com/GCWing/OpenBitFun/pull/3573'; +const call: FlowToolItem = { + id: 'review-call', type: 'tool', toolName: 'ReviewPlatform', status: 'completed', timestamp: 1, + toolCall: { id: 'review-call', input: { action: 'get_pull_request', repository_path: '/workspace' } }, + toolResult: { success: true, result: { result: { number: 3573, webUrl: url } } }, +}; +let host: HTMLDivElement; +let root: Root; +beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true; + state.peer = false; + state.capabilities = null; + state.sessions = new Map([['current', { + sessionId: 'current', title: 'Session', sessionKind: 'normal', status: 'idle', config: {}, dialogTurns: [], + workspaceId: 'workspace-1', createdAt: 1, lastActiveAt: 1, error: null, + }]]); + state.openExternal.mockReset().mockResolvedValue(undefined); + state.openPanel.mockReset(); + host = document.createElement('div'); + document.body.append(host); + root = createRoot(host); +}); +afterEach(() => { act(() => root.unmount()); host.remove(); }); +function render() { + act(() => root.render()); +} +function button(label: string) { + return host.querySelector(`button[aria-label="${label}"]`); +} + +describe('ReviewPlatform tool card entry points', () => { + it('disables the CLI panel action while keeping the PR URL usable in the system browser', async () => { + state.peer = true; + state.capabilities = { hostKind: 'cli' }; + render(); + expect(button('toolCards.builtin.links.openReviewPanel')?.disabled).toBe(true); + act(() => host.querySelector('[data-openbitfun-part="surface"]')?.click()); + expect(host.textContent).toContain('common:reviewPlatform.errors.hostUnsupported'); + await act(async () => button('toolCards.builtin.links.openReview')?.click()); + expect(state.openExternal).toHaveBeenCalledExactlyOnceWith(url); + expect(state.openPanel).not.toHaveBeenCalled(); + }); + + it('keeps the local panel action and updates it when peer capabilities change', async () => { + render(); + expect(button('toolCards.builtin.links.openReviewPanel')?.disabled).toBe(false); + await act(async () => button('toolCards.builtin.links.openReviewPanel')?.click()); + expect(state.openPanel).toHaveBeenCalledWith('workspace-1', '/workspace'); + state.peer = true; + state.capabilities = { hostKind: 'cli' }; + render(); + expect(button('toolCards.builtin.links.openReviewPanel')?.disabled).toBe(true); + state.capabilities = { hostKind: 'desktop' }; + render(); + expect(button('toolCards.builtin.links.openReviewPanel')?.disabled).toBe(false); + }); +}); diff --git a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx index f1a7220397..2268fffa3a 100644 --- a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx @@ -14,6 +14,7 @@ import { notificationService } from '@/shared/notification-system'; import { copyTextToClipboard } from '@/shared/utils/textSelection'; import { lazyWithRecovery } from '@/shared/utils/lazyWithRecovery'; import { getActiveSurfaceScope, isLocalSurface, onSurfaceActivated } from '@/infrastructure/peer-device/deviceSurface'; +import { useReviewPlatformSupport } from '@/infrastructure/peer-device/useReviewPlatformSupport'; import { flowChatStore } from '../store/FlowChatStore'; import { openMainSession } from '../services/sessionActivation'; import { sessionProjectWorkspaceId } from '../utils/sessionWorkspace'; @@ -46,6 +47,7 @@ const views: Record = ({ toolItem, sessionId, onExpand, onOpenInEditor }) => { const { t, formatNumber } = useI18n('flow-chat'); + const canViewPullRequests = useReviewPlatformSupport(); const [isExpanded, setExpanded] = useToolCardDisclosure('isExpanded'); const [market, setMarket] = useState<'miniapps' | 'appearance'>(); const [marketOpen, setMarketOpen] = useState(false); @@ -108,11 +110,11 @@ export const BuiltinToolCard: React.FC = ({ toolItem, sessionId, }) }); if (link.kind === 'market') actions.push({ key, label: link.label, icon: 'store', disabled: detached, onPress: () => { setMarket(link.value === 'appearance' ? 'appearance' : 'miniapps'); setMarketOpen(true); } }); - if (link.kind === 'review') actions.push({ key, label: link.label, icon: 'git-pull-request', disabled: detached || !workspaceId, + if (link.kind === 'review') actions.push({ key, label: link.label, icon: 'git-pull-request', disabled: detached || !workspaceId || !canViewPullRequests, onPress: () => run(async () => { const scope = getActiveSurfaceScope(); const { createReviewPlatformTab } = await import('@/shared/utils/tabUtils'); - if (scope.isCurrent() && workspaceId) createReviewPlatformTab(workspaceId, link.value); + if (scope.isCurrent() && workspaceId && canViewPullRequests) createReviewPlatformTab(workspaceId, link.value); }) }); if (link.intent !== 'primary' && (link.kind === 'url' || link.kind === 'file' || link.kind === 'copy-path')) actions.push({ key: `${key}:copy`, label: t(link.kind === 'url' ? 'toolCards.builtin.links.copyAddress' : 'toolCards.builtin.links.copyPath'), icon: 'duplicate', @@ -148,7 +150,9 @@ export const BuiltinToolCard: React.FC = ({ toolItem, sessionId, sections: model.sections, emptyContent: model.emptyContent, error: model.error, notice: detached && allLinks.some(link => ['file', 'session', 'miniapp', 'review', 'market'].includes(link.kind)) ? [model.notice, t('toolCards.builtin.hostUnavailable')].filter(Boolean).join('\n') - : remoteLoopback ? [model.notice, t('toolCards.builtin.remoteLoopback')].filter(Boolean).join('\n') : model.notice, + : remoteLoopback ? [model.notice, t('toolCards.builtin.remoteLoopback')].filter(Boolean).join('\n') + : !canViewPullRequests && allLinks.some(link => link.kind === 'review') + ? [model.notice, t('common:reviewPlatform.errors.hostUnsupported')].filter(Boolean).join('\n') : model.notice, connection: model.connection, ordered: model.ordered, actions: toActions(model.links), media: isExpanded && model.family === 'image-analysis' ? model.imageSources.length > 0 ? model.imageSources.map((src, index) => {model.summary) diff --git a/src/web-ui/src/infrastructure/peer-device/README.md b/src/web-ui/src/infrastructure/peer-device/README.md index 301320a9d5..afacbb03f8 100644 --- a/src/web-ui/src/infrastructure/peer-device/README.md +++ b/src/web-ui/src/infrastructure/peer-device/README.md @@ -268,3 +268,7 @@ listener alone recovers an interaction emitted before attachment. panel. This only opens the clicked URL: it never queries controller-local repositories or credentials on behalf of a peer. Preserve URL fragments and queries, and re-evaluate support after capability or surface changes. + The session overview and tool-card panel action use the same gate; a + restored unsupported panel keeps its URL available in the system browser + without querying the host. Overview requests and panel state belong to + the surface activation even when two devices share a workspace ID/path. diff --git a/src/web-ui/src/infrastructure/peer-device/useReviewPlatformSupport.ts b/src/web-ui/src/infrastructure/peer-device/useReviewPlatformSupport.ts new file mode 100644 index 0000000000..4dd8c64a62 --- /dev/null +++ b/src/web-ui/src/infrastructure/peer-device/useReviewPlatformSupport.ts @@ -0,0 +1,10 @@ +import { usePeerDeviceModeOptional } from './peerDeviceContextState'; +import { canViewPullRequestsOnSurface } from './peerCapabilityResolution'; + +export function useReviewPlatformSupport(): boolean { + const peer = usePeerDeviceModeOptional(); + return canViewPullRequestsOnSurface( + peer?.peerMode.active ?? false, + peer?.currentPeerCapabilities ?? null, + ); +} diff --git a/src/web-ui/src/locales/en-US/common.json b/src/web-ui/src/locales/en-US/common.json index e6d258ce99..c22a564e62 100644 --- a/src/web-ui/src/locales/en-US/common.json +++ b/src/web-ui/src/locales/en-US/common.json @@ -2056,6 +2056,7 @@ "repositoryUntrusted": "Git does not trust this repository. Confirm its ownership before retrying.", "remoteNotFound": "The selected remote no longer exists. Refresh and select a remote.", "unsupportedPlatform": "This repository provider is not supported.", + "hostUnsupported": "The pull request viewer is unavailable on this device. Open pull request links in the system browser.", "providerFailed": "The repository provider could not complete the request. Retry or check the provider service.", "authenticationRequired": "Authentication is required. Sign in or update the token and retry.", "permissionDenied": "Access was denied. Check the account and token permissions.", diff --git a/src/web-ui/src/locales/zh-CN/common.json b/src/web-ui/src/locales/zh-CN/common.json index 40edda8334..1b28309167 100644 --- a/src/web-ui/src/locales/zh-CN/common.json +++ b/src/web-ui/src/locales/zh-CN/common.json @@ -2056,6 +2056,7 @@ "repositoryUntrusted": "Git 尚未信任此仓库，请确认仓库所有权后重试。", "remoteNotFound": "所选远程仓库已不存在，请刷新并重新选择。", "unsupportedPlatform": "不支持此仓库提供方。", + "hostUnsupported": "当前设备不支持内置 PR 查看，请使用系统浏览器打开 PR 链接。", "providerFailed": "仓库提供方无法完成请求，请重试或检查提供方服务。", "authenticationRequired": "需要认证，请登录或更新令牌后重试。", "permissionDenied": "访问被拒绝，请检查账号和令牌权限。", diff --git a/src/web-ui/src/locales/zh-TW/common.json b/src/web-ui/src/locales/zh-TW/common.json index d5ccd2eaf9..f710eb5804 100644 --- a/src/web-ui/src/locales/zh-TW/common.json +++ b/src/web-ui/src/locales/zh-TW/common.json @@ -2056,6 +2056,7 @@ "repositoryUntrusted": "Git 尚未信任此儲存庫，請確認儲存庫擁有權後重試。", "remoteNotFound": "所選遠端儲存庫已不存在，請重新整理並重新選取。", "unsupportedPlatform": "不支援此儲存庫提供者。", + "hostUnsupported": "目前裝置不支援內建 PR 檢視，請使用系統瀏覽器開啟 PR 連結。", "providerFailed": "儲存庫提供者無法完成請求，請重試或檢查提供者服務。", "authenticationRequired": "需要驗證，請登入或更新權杖後重試。", "permissionDenied": "存取遭拒，請檢查帳號與權杖權限。", From 4dc2a134e7394ded7672c5e7c0787f15baec42f6 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 09:55:31 +0800 Subject: [PATCH 03/12] fix(web-ui): repair model selector callback lint failures --- .../flow_chat/components/ModelSelector.tsx | 70 +++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/web-ui/src/flow_chat/components/ModelSelector.tsx b/src/web-ui/src/flow_chat/components/ModelSelector.tsx index 0d71f32045..22e0ade7ed 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelector.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelector.tsx @@ -1336,36 +1336,6 @@ export const ModelSelector: React.FC = ({ const smartView = isNativeModelSelection && routing.available && (smartPreviewScope === modelSelectionScopeKey || (!poolId && routing.mode === 'smart')); const smartModeDescription = !routing.available ? t('modelSelector.routing.notOpen') : routing.status?.issue ? t('modelSelector.routing.configurationIssue') : t('modelSelector.modes.smartSummary'); - const handleRoutingMode = async (mode: 'manual' | 'smart', keepSmartView = false) => { - try { - if (mode === 'smart' && !routing.available) return false; - // Inspect unavailable smart mode without changing the host's active strategy. - if (mode === 'smart' && (!routing.status || !routing.status.ready)) { - setSmartPreviewScope(modelSelectionScopeKey); - closePanel(true); - return false; - } - if (mode === 'smart' && poolId && !await handleSelectModel('primary')) return false; - if (mode === 'smart' && !currentModelMode) { - manualModelRef.current = { scopeKey: modelSelectionScopeKey, modelId: currentNativeModelId }; - setRecentManualModel(modelSelectionScopeKey, currentNativeModelId); - } - const selected = routing.status ? await routing.select(mode) : mode === 'manual'; - if (selected && deviceSurfaceScope.isCurrent() && modelSelectionScopeRef.current === modelSelectionScopeKey) { - setSmartPreviewScope(keepSmartView ? modelSelectionScopeKey : null); - closePanel(true); - } - return selected; - } - catch (error) { - if (deviceSurfaceScope.isCurrent()) { - notificationService.error(t('modelSelector.routing.saveFailed')); - log.warn('Failed to update model routing strategy', { error }); - void routing.refresh(); - } - return false; - } - }; const currentModelMode = isNativeModelSelection && poolId ? 'pool' : isNativeModelSelection && routing.mode === 'smart' ? 'smart' : null; const settingsModelMode = smartView ? 'smart' : currentModelMode; // Configuration previews do not change the host's active selection. @@ -1614,9 +1584,7 @@ export const ModelSelector: React.FC = ({ activeSession?.workspacePath, acpClientId, currentMode, - currentModelMode, poolId, - currentNativeModelId, defaultModels, disabled, externalSelection, @@ -1624,7 +1592,6 @@ export const ModelSelector: React.FC = ({ loading, modelSelectionScopeKey, persistSharedModeDefault, - isNativeModelSelection, routing.pending, reasoningLoading, recentPresetForModel, @@ -1635,13 +1602,46 @@ export const ModelSelector: React.FC = ({ activeSession?.workspaceId, ]); - const handleSelectModelFromMenu = async (modelId: string) => { + const handleRoutingMode = useCallback(async (mode: 'manual' | 'smart', keepSmartView = false) => { + try { + if (mode === 'smart' && !routing.available) return false; + // Inspect unavailable smart mode without changing the host's active strategy. + if (mode === 'smart' && (!routing.status || !routing.status.ready)) { + setSmartPreviewScope(modelSelectionScopeKey); + closePanel(true); + return false; + } + if (mode === 'smart' && poolId && !await handleSelectModel('primary')) return false; + if (mode === 'smart' && !currentModelMode) { + manualModelRef.current = { scopeKey: modelSelectionScopeKey, modelId: currentNativeModelId }; + setRecentManualModel(modelSelectionScopeKey, currentNativeModelId); + } + const selected = routing.status ? await routing.select(mode) : mode === 'manual'; + if (selected && deviceSurfaceScope.isCurrent() && modelSelectionScopeRef.current === modelSelectionScopeKey) { + setSmartPreviewScope(keepSmartView ? modelSelectionScopeKey : null); + closePanel(true); + } + return selected; + } + catch (error) { + if (deviceSurfaceScope.isCurrent()) { + notificationService.error(t('modelSelector.routing.saveFailed')); + log.warn('Failed to update model routing strategy', { error }); + void routing.refresh(); + } + return false; + } + }, [closePanel, currentModelMode, currentNativeModelId, deviceSurfaceScope, handleSelectModel, + modelSelectionScopeKey, poolId, routing, t]); + + const handleSelectModelFromMenu = useCallback(async (modelId: string) => { if (isNativeModelSelection && routing.mode === 'smart' && !await handleRoutingMode('manual')) return false; const selected = await handleSelectModel(modelId); if (modelSelectionScopeRef.current !== modelSelectionScopeKey) return false; if (selected && hasSettingsPanel) { setSmartPreviewScope(null); closePanel(true); } return selected; - }; + }, [closePanel, handleRoutingMode, handleSelectModel, hasSettingsPanel, isNativeModelSelection, + modelSelectionScopeKey, routing.mode]); const handleConfigureRoleModel = useCallback(async (role: ModelRole, modelId: string) => { if (!canConfigureNativeDefaults || disabled || loading || reasoningLoading From f7bfa3934ae0ec621e51215ebea0cbdcb5737353 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 10:12:46 +0800 Subject: [PATCH 04/12] fix(peer-device): isolate PR preferences and discard stale operations --- .../ReviewPlatformPanel.test.tsx | 103 ++++++++++++- .../review-platform/ReviewPlatformPanel.tsx | 135 ++++++++++++------ .../components/modern/FlowChatHeader.test.tsx | 4 +- .../components/modern/FlowChatHeader.tsx | 2 +- .../tool-cards/BuiltinToolCard.test.tsx | 2 +- .../flow_chat/tool-cards/BuiltinToolCard.tsx | 2 +- src/web-ui/src/locales/en-US/common.json | 2 +- src/web-ui/src/locales/zh-CN/common.json | 2 +- src/web-ui/src/locales/zh-TW/common.json | 2 +- 9 files changed, 196 insertions(+), 58 deletions(-) diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx index a01c1068b8..bdf0f06f82 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.test.tsx @@ -2,27 +2,33 @@ import React, { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { ReviewPlatformPullRequest, ReviewPlatformPullRequestDetailPage, ReviewPlatformWorkspaceSnapshot, ReviewRepositoryLocator } from '@/infrastructure/api/service-api/ReviewPlatformAPI'; +import type { ReviewPlatformPullRequest, ReviewPlatformPullRequestDetailPage, ReviewPlatformPullRequestReviewTarget, ReviewPlatformWorkspaceSnapshot, ReviewRepositoryLocator } from '@/infrastructure/api/service-api/ReviewPlatformAPI'; +import type { Session } from '@/flow_chat/types/flow-chat'; import { ReviewPlatformPanel } from './ReviewPlatformPanel'; import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; import type { PeerHostCapabilities } from '@/infrastructure/peer-device/PeerConnectionManager'; -const mocks = vi.hoisted(() => ({ snapshot: vi.fn(), context: vi.fn(), detail: vi.fn(), openExternal: vi.fn(), t: (key: string) => key })); +const mocks = vi.hoisted(() => ({ + snapshot: vi.fn(), context: vi.fn(), detail: vi.fn(), openExternal: vi.fn(), reviewTarget: vi.fn(), + prepareReview: vi.fn(), + state: { sessions: new Map(), activeSessionId: null as string | null }, + t: (key: string) => key, +})); const peer = vi.hoisted(() => ({ active: false, capabilities: null as Partial | null })); vi.mock('@/infrastructure/peer-device/peerDeviceContextState', () => ({ usePeerDeviceModeOptional: () => peer.active ? { peerMode: { active: true }, currentPeerCapabilities: peer.capabilities } : null, })); -vi.mock('@/infrastructure/api', () => ({ reviewPlatformAPI: { getWorkspaceSnapshot: mocks.snapshot, getWorkspaceContext: mocks.context, getPullRequestDetailPage: mocks.detail }, systemAPI: { openExternal: mocks.openExternal } })); +vi.mock('@/infrastructure/api', () => ({ reviewPlatformAPI: { getWorkspaceSnapshot: mocks.snapshot, getWorkspaceContext: mocks.context, getPullRequestDetailPage: mocks.detail, getPullRequestReviewTarget: mocks.reviewTarget }, systemAPI: { openExternal: mocks.openExternal } })); vi.mock('@/infrastructure/markdown', () => ({ MarkdownRenderer: () => null })); vi.mock('@/shared/notification-system', () => ({ notificationService: {} })); vi.mock('@/infrastructure/i18n', () => ({ useI18n: () => ({ t: mocks.t }), i18nService: { t: mocks.t, formatDate: () => '' } })); vi.mock('@/flow_chat/services/sessionActivation', () => ({ openMainSession: vi.fn() })); vi.mock('@/flow_chat/services/btwSessionPane', () => ({ openBtwSessionInAuxPane: vi.fn() })); -vi.mock('@/flow_chat/services/ReviewService', () => ({ launchPreparedReviewSession: vi.fn(), prepareReviewLaunchFromPullRequest: vi.fn() })); +vi.mock('@/flow_chat/services/ReviewService', () => ({ launchPreparedReviewSession: vi.fn(), prepareReviewLaunchFromPullRequest: mocks.prepareReview })); vi.mock('@/flow_chat/components/DeepReviewConsentDialog', () => ({ useDeepReviewConsent: () => ({ confirmDeepReviewLaunch: vi.fn(), deepReviewConsentDialog: null }) })); -vi.mock('@/flow_chat/store/FlowChatStore', () => ({ flowChatStore: { getState: () => ({ sessions: new Map(), activeSessionId: null }), subscribe: () => () => {} } })); +vi.mock('@/flow_chat/store/FlowChatStore', () => ({ flowChatStore: { getState: () => mocks.state, subscribe: () => () => {} } })); vi.mock('@/shared/stores/contextStore', () => ({ useContextStore: {} })); vi.mock('@/shared/services/ide-control', () => ({ quickActions: {} })); @@ -103,6 +109,10 @@ describe('Gitee panel state and asynchronous request ordering', () => { mocks.context.mockReset(); mocks.detail.mockReset().mockImplementation(() => new Promise(() => {})); mocks.openExternal.mockReset().mockResolvedValue(undefined); + mocks.reviewTarget.mockReset(); + mocks.prepareReview.mockReset(); + mocks.state.sessions.clear(); + mocks.state.activeSessionId = null; }); afterEach(async () => { await act(async () => root.unmount()); host.remove(); activateSurface('local'); }); @@ -114,7 +124,7 @@ describe('Gitee panel state and asynchronous request ordering', () => { await act(async () => root.render( , )); - expect(host.textContent).toContain('common:reviewPlatform.errors.hostUnsupported'); + expect(host.textContent).toContain('common:reviewPlatform.messages.hostUnsupported'); expect(mocks.snapshot).not.toHaveBeenCalled(); expect(mocks.context).not.toHaveBeenCalled(); expect(mocks.detail).not.toHaveBeenCalled(); @@ -139,6 +149,87 @@ describe('Gitee panel state and asynchronous request ordering', () => { expect(host.textContent).not.toContain('Departed peer result'); }); + it('isolates remembered remotes by device and preserves legacy local preferences', async () => { + const path = `/remote-preferences-${++testNumber}`; + const workspaceId = workspaceIdFor(path); + const legacyKey = `openbitfun:review-platform:last-remote:id:${workspaceId}`; + window.localStorage.setItem(legacyKey, 'local-origin'); + mocks.context.mockImplementation(({ repositoryPath }, remoteId) => { + const result = snapshot(repositoryPath); + result.selectedRemoteId = remoteId ?? 'peer-origin'; + return Promise.resolve(result); + }); + await act(async () => { + activateSurface('preference-peer-a'); + root.render(); + }); + expect(mocks.context).toHaveBeenLastCalledWith(locator(path), null); + await act(async () => { activateSurface('preference-peer-b'); }); + expect(mocks.context).toHaveBeenLastCalledWith(locator(path), null); + await act(async () => { activateSurface('local'); }); + expect(mocks.context).toHaveBeenLastCalledWith(locator(path), 'local-origin'); + expect(window.localStorage.getItem(legacyKey)).toBe('local-origin'); + }); + + it('migrates a legacy path preference only on the local device', async () => { + const path = `/legacy-remote-preference-${++testNumber}`; + const legacyKey = `openbitfun:review-platform:last-remote:${path}`; + window.localStorage.setItem(legacyKey, 'old-origin'); + mocks.context.mockImplementation(() => new Promise(() => {})); + await act(async () => { + activateSurface('legacy-preference-peer'); + root.render(); + }); + expect(mocks.context).toHaveBeenLastCalledWith(locator(path), null); + expect(window.localStorage.getItem(legacyKey)).toBe('old-origin'); + await act(async () => { activateSurface('local'); }); + expect(mocks.context).toHaveBeenLastCalledWith(locator(path), 'old-origin'); + expect(window.localStorage.getItem(`openbitfun:review-platform:last-remote:id:${workspaceIdFor(path)}`)).toBe('old-origin'); + expect(window.localStorage.getItem(legacyKey)).toBeNull(); + }); + + it('discards a departed device response before it writes preferences or cache aliases', async () => { + const path = `/late-remote-preference-${++testNumber}`; + const old = deferred(); + mocks.context.mockImplementationOnce(() => old.promise); + const selected = snapshot(path); + selected.selectedRemoteId = 'selected-origin'; + mocks.context.mockResolvedValue(selected); + await act(async () => { + activateSurface('late-preference-peer-a'); + root.render(); + }); + await act(async () => { activateSurface('late-preference-peer-b'); }); + const preferenceAfterSwitch = { ...window.localStorage }; + const departed = snapshot(path); + departed.selectedRemoteId = 'departed-origin'; + await act(async () => old.resolve(departed)); + expect({ ...window.localStorage }).toEqual(preferenceAfterSwitch); + // The discarded response must not become a cache hit on a later activation. + await act(async () => { activateSurface('late-preference-peer-a'); }); + expect(mocks.context).toHaveBeenCalledTimes(3); + expect(mocks.context).toHaveBeenLastCalledWith(locator(path), null); + }); + + it('does not prepare a review on a new device when the old target response arrives', async () => { + const path = `/review-launch-switch-${++testNumber}`; + mocks.state.sessions.set('chat', { + sessionId: 'chat', sessionKind: 'normal', workspaceId: workspaceIdFor(path), dialogTurns: [], + } as Session); + mocks.state.activeSessionId = 'chat'; + mocks.detail.mockImplementation(({ pullRequestId, section }) => Promise.resolve(detail(Number(pullRequestId), section))); + const target = deferred(); + mocks.reviewTarget.mockReturnValue(target.promise); + await act(async () => root.render()); + const review = [...host.querySelectorAll('button')].find(button => button.textContent === 'Review'); + expect(review?.disabled).toBe(false); + await act(async () => review!.click()); + expect(mocks.reviewTarget).toHaveBeenCalledOnce(); + await act(async () => { activateSurface('review-launch-new-peer'); }); + await act(async () => target.resolve({ pullRequest: pull(10, 'open'), files: [], omittedFileCount: 0, limitations: [] })); + expect(mocks.prepareReview).not.toHaveBeenCalled(); + }); + it('keeps a browser action when a supported peer cannot load the PR context', async () => { const url = 'https://github.com/GCWing/OpenBitFun/pull/3573'; mocks.context.mockRejectedValueOnce(new Error('Provider unavailable')); diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx index 86817e39e2..32e2ab7baa 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx @@ -66,7 +66,7 @@ import { withGitRepositoryTrustRecovery, } from '@/shared/services/gitTrustService'; import type { PullRequestContext } from '@/shared/types/context'; -import { getActiveSurfaceId, getActiveSurfaceScope, onSurfaceActivated, surfaceScopedKey } from '@/infrastructure/peer-device/deviceSurface'; +import { getActiveSurfaceId, getActiveSurfaceScope, isLocalSurface, onSurfaceActivated, surfaceScopedKey, type DeviceSurfaceId, type SurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; import { useReviewPlatformSupport } from '@/infrastructure/peer-device/useReviewPlatformSupport'; import { currentPullRequestReviewStatusText, @@ -254,8 +254,12 @@ function mergeDetailPage( }; } -function remotePreferenceKey(workspaceId: string): string { - return `${REMOTE_STORAGE_PREFIX}id:${workspaceId}`; +function remotePreferenceKey(surfaceId: DeviceSurfaceId, workspaceId: string): string { + // Keep the existing local key readable by older installs. Peer preferences + // have a separate namespace and must never inherit controller-local values. + return isLocalSurface(surfaceId) + ? `${REMOTE_STORAGE_PREFIX}id:${workspaceId}` + : `${REMOTE_STORAGE_PREFIX}surface:${surfaceScopedKey(surfaceId, workspaceId)}`; } /** Pre-ID installs stored the preference under the workspace path. */ @@ -263,16 +267,16 @@ function legacyRemotePreferenceKey(workspacePath: string): string { return `${REMOTE_STORAGE_PREFIX}${workspacePath}`; } -function readRememberedRemote(workspaceId: string, workspacePath?: string): string | null { +function readRememberedRemote(surfaceId: DeviceSurfaceId, workspaceId: string, workspacePath?: string): string | null { if (!workspaceId || typeof window === 'undefined') return null; try { - const remembered = window.localStorage.getItem(remotePreferenceKey(workspaceId)); + const remembered = window.localStorage.getItem(remotePreferenceKey(surfaceId, workspaceId)); if (remembered !== null) return remembered; - if (!workspacePath) return null; + if (!isLocalSurface(surfaceId) || !workspacePath) return null; // Migrate the legacy path-keyed preference onto the workspace ID. const legacy = window.localStorage.getItem(legacyRemotePreferenceKey(workspacePath)); if (legacy !== null) { - window.localStorage.setItem(remotePreferenceKey(workspaceId), legacy); + window.localStorage.setItem(remotePreferenceKey(surfaceId, workspaceId), legacy); window.localStorage.removeItem(legacyRemotePreferenceKey(workspacePath)); } return legacy; @@ -281,10 +285,10 @@ function readRememberedRemote(workspaceId: string, workspacePath?: string): stri } } -function rememberRemote(workspaceId: string, remoteId: string | null): void { +function rememberRemote(surfaceId: DeviceSurfaceId, workspaceId: string, remoteId: string | null): void { if (!workspaceId || typeof window === 'undefined') return; try { - const key = remotePreferenceKey(workspaceId); + const key = remotePreferenceKey(surfaceId, workspaceId); if (remoteId) { window.localStorage.setItem(key, remoteId); } else { @@ -725,7 +729,7 @@ export const ReviewPlatformPanel: React.FC = (props) = } - description={t('common:reviewPlatform.errors.hostUnsupported')} + description={t('common:reviewPlatform.messages.hostUnsupported')} data-testid="review-platform-unsupported" data-openbitfun-product-component="review-platform" data-openbitfun-product-part="emptyState" @@ -743,17 +747,18 @@ export const ReviewPlatformPanel: React.FC = (props) = // The same workspace ID/path can exist on two hosts. Recreate state and // invalidate in-flight effects even if the panel itself survives the switch. - return ; + return ; }; function ReviewPlatformContent({ + surface, workspacePath, workspaceId, initialRemoteId, initialPullRequestId, initialPullRequestUrl, detailOnly = false, -}: ReviewPlatformPanelProps) { +}: ReviewPlatformPanelProps & { surface: SurfaceScope }) { const { t } = useI18n('panels/git'); const { t: tReview } = useI18n('flow-chat'); const authFormId = useId(); @@ -767,6 +772,17 @@ function ReviewPlatformContent({ const detailRequestSeq = useRef(0); const detailSectionRequestSeq = useRef(0); const reviewLaunchInFlight = useRef(false); + const mounted = useRef(true); + const isPanelCurrent = useCallback(() => mounted.current && surface.isCurrent(), [surface]); + useEffect(() => { + mounted.current = true; + return () => { + mounted.current = false; + snapshotRequestSeq.current += 1; + detailRequestSeq.current += 1; + detailSectionRequestSeq.current += 1; + }; + }, []); const [snapshot, setSnapshot] = useState(emptySnapshot); const [selectedRemoteId, setSelectedRemoteId] = useState(null); const [listRemoteId, setListRemoteId] = useState(null); @@ -883,6 +899,7 @@ function ReviewPlatformContent({ nextRemoteId?: string | null, options?: { force?: boolean; page?: number; state?: ListStateFilter; userInitiated?: boolean }, ) => { + if (!isPanelCurrent()) return; const requestSeq = ++snapshotRequestSeq.current; detailRequestSeq.current += 1; detailSectionRequestSeq.current += 1; @@ -902,7 +919,7 @@ function ReviewPlatformContent({ const requestedRemoteId = nextRemoteId !== undefined ? nextRemoteId : detailOnly - ? readRememberedRemote(workspaceId, workspacePath) + ? readRememberedRemote(surface.surfaceId, workspaceId, workspacePath) : null; const requestedPage = Math.max(1, options?.page ?? 1); const requestedState = detailOnly ? 'all' : options?.state ?? serverStateFilter.current; @@ -936,7 +953,9 @@ function ReviewPlatformContent({ setSnapshotError(null); try { const repository = { workspaceId, repositoryPath: workspacePath }; - const fetchSnapshot = () => detailOnly + const fetchSnapshot = () => { + if (!isPanelCurrent()) throw new Error('Review panel is no longer active'); + return detailOnly ? reviewPlatformAPI.getWorkspaceContext(repository, requestedRemoteId ?? null) : reviewPlatformAPI.getWorkspaceSnapshot( repository, @@ -945,15 +964,16 @@ function ReviewPlatformContent({ PR_PAGE_SIZE, requestedState, ); + }; const next = options?.userInitiated ? await withGitRepositoryTrustRecovery(fetchSnapshot, { workspaceId, repositoryPath: workspacePath }, { userInitiated: true }) : await fetchSnapshot(); - if (snapshotRequestSeq.current !== requestSeq) return; + if (!isPanelCurrent() || snapshotRequestSeq.current !== requestSeq) return; setSnapshot(next); const remoteId = next.selectedRemoteId ?? next.remotes[0]?.id ?? null; setSelectedRemoteId(remoteId); setPageIndex(Math.max(0, (next.pagination.page || requestedPage) - 1)); - rememberRemote(workspaceId, remoteId); + rememberRemote(surface.surfaceId, workspaceId, remoteId); setSelectedPrId(detailOnly ? null : next.pullRequests[0]?.id ?? null); setDetail(null); setVerifiedDetailKey(null); @@ -964,17 +984,18 @@ function ReviewPlatformContent({ snapshotCache.set(snapshotCacheKey(workspaceId, remoteId, requestedPage, PR_PAGE_SIZE, snapshotMode, requestedState), entry); } } catch (err) { - if (snapshotRequestSeq.current !== requestSeq) return; + if (!isPanelCurrent() || snapshotRequestSeq.current !== requestSeq) return; setSnapshotError({ cause: err }); log.error('Failed to load review platform snapshot', { workspacePath, error: err }); } finally { - if (snapshotRequestSeq.current === requestSeq) { + if (isPanelCurrent() && snapshotRequestSeq.current === requestSeq) { setLoading(false); } } - }, [setDetailError, detailOnly, workspacePath, workspaceId]); + }, [setDetailError, detailOnly, isPanelCurrent, surface.surfaceId, workspacePath, workspaceId]); const loadDetail = useCallback(async (repo: ReviewPlatformRepositoryRef | null, remoteId: string, pullRequestId: string, options?: { force?: boolean }) => { + if (!isPanelCurrent()) return; const requestSeq = ++detailRequestSeq.current; detailSectionRequestSeq.current += 1; const repositoryPath = workspacePath || repo?.workspacePath || ''; @@ -1006,7 +1027,7 @@ function ReviewPlatformContent({ page: 1, perPage: 1, }); - if (detailRequestSeq.current !== requestSeq) return; + if (!isPanelCurrent() || detailRequestSeq.current !== requestSeq) return; if (cached && !samePullRequestRevisions(cached.detail, nextDetail)) { clearDetailPageCacheForPullRequest(workspaceId, remoteId, pullRequestId); } @@ -1014,18 +1035,18 @@ function ReviewPlatformContent({ detailCache.set(cacheKey, { detail: nextDetail, fetchedAt: Date.now() }); setVerifiedDetailKey(cacheKey); } catch (err) { - if (detailRequestSeq.current !== requestSeq) return; + if (!isPanelCurrent() || detailRequestSeq.current !== requestSeq) return; log.error('Failed to load pull request detail', { pullRequestId, error: err }); setDetailError(err); if (!cached) { setDetail(null); } } finally { - if (detailRequestSeq.current === requestSeq) { + if (isPanelCurrent() && detailRequestSeq.current === requestSeq) { setDetailLoading(false); } } - }, [setDetailError, workspacePath, workspaceId]); + }, [setDetailError, isPanelCurrent, workspacePath, workspaceId]); const applySectionPagination = useCallback((section: Exclude, pagination: ReviewPlatformPagination) => { if (section === 'ci') { @@ -1048,6 +1069,7 @@ function ReviewPlatformContent({ perPage: number, options?: { force?: boolean }, ) => { + if (!isPanelCurrent()) return; const repositoryPath = workspacePath || repo?.workspacePath || ''; const page = Math.max(1, pageIndex + 1); const cacheKey = detailPageCacheKey(workspaceId, remoteId, pullRequestId, section, page, perPage); @@ -1083,7 +1105,7 @@ function ReviewPlatformContent({ page, perPage, }); - if (detailSectionRequestSeq.current !== requestSeq) return; + if (!isPanelCurrent() || detailSectionRequestSeq.current !== requestSeq) return; if (!matchesVerifiedOverview(nextPage)) { clearDetailPageCacheForPullRequest(workspaceId, remoteId, pullRequestId); void loadDetail(repo, remoteId, pullRequestId, { force: true }); @@ -1093,15 +1115,15 @@ function ReviewPlatformContent({ setDetail(prev => mergeDetailPage(prev, nextPage)); applySectionPagination(section, nextPage.pagination); } catch (err) { - if (detailSectionRequestSeq.current !== requestSeq) return; + if (!isPanelCurrent() || detailSectionRequestSeq.current !== requestSeq) return; log.error('Failed to load pull request detail section', { pullRequestId, section, page, perPage, error: err }); setDetailError(err); } finally { - if (detailSectionRequestSeq.current === requestSeq) { + if (isPanelCurrent() && detailSectionRequestSeq.current === requestSeq) { setDetailLoading(false); } } - }, [setDetailError, applySectionPagination, loadDetail, workspacePath, workspaceId]); + }, [setDetailError, applySectionPagination, isPanelCurrent, loadDetail, workspacePath, workspaceId]); useEffect(() => { serverStateFilter.current = 'all'; @@ -1167,7 +1189,7 @@ function ReviewPlatformContent({ if (nextRemoteId && selectedRemoteId !== nextRemoteId) { setSelectedRemoteId(nextRemoteId); - rememberRemote(workspaceId, nextRemoteId); + rememberRemote(surface.surfaceId, workspaceId, nextRemoteId); } if (selectedPrId !== targetPullRequestId) { @@ -1178,6 +1200,7 @@ function ReviewPlatformContent({ detailOnly, initialPullRequestId, initialPullRequestTarget, + surface.surfaceId, initialPullRequestUrl, initialRemoteId, selectedPrId, @@ -1398,10 +1421,10 @@ function ReviewPlatformContent({ setStateFilter('all'); serverStateFilter.current = 'all'; setPageIndex(0); - rememberRemote(workspaceId, remoteId || null); + rememberRemote(surface.surfaceId, workspaceId, remoteId || null); setSnapshot(emptySnapshot()); void loadSnapshot(remoteId || null, { page: 1, state: 'all' }); - }, [setDetailError, loadSnapshot, workspaceId]); + }, [setDetailError, loadSnapshot, surface.surfaceId, workspaceId]); const handleStateChange = useCallback((state: ListStateFilter) => { setPanelView('list'); @@ -1494,6 +1517,7 @@ function ReviewPlatformContent({ }, []); const loadCiLog = useCallback(async (item: ReviewPlatformCiItem): Promise => { + if (!isPanelCurrent()) return null; const cached = ciLogById[item.id]; if (cached) return cached; if (!canLoadCiLog(selectedRemote, item)) { @@ -1527,20 +1551,22 @@ function ReviewPlatformContent({ ciItemId: item.id, ciItemName: item.name, }); + if (!isPanelCurrent()) return null; setCiLogById(prev => ({ ...prev, [item.id]: nextLog })); return nextLog; } catch (err) { + if (!isPanelCurrent()) return null; setCiLogErrorById(prev => ({ ...prev, [item.id]: { cause: err } })); log.error('Failed to load CI log', { itemId: item.id, error: err }); return null; } finally { - setCiLogLoadingIds(prev => { + if (isPanelCurrent()) setCiLogLoadingIds(prev => { const next = new Set(prev); next.delete(item.id); return next; }); } - }, [ciLogById, repository, selectedPrId, selectedRemote, selectedRemoteId, workspacePath, workspaceId]); + }, [isPanelCurrent, ciLogById, repository, selectedPrId, selectedRemote, selectedRemoteId, workspacePath, workspaceId]); const toggleCiExpanded = useCallback((item: ReviewPlatformCiItem) => { if (expandedCiItemIds.has(item.id)) { @@ -1568,12 +1594,14 @@ function ReviewPlatformContent({ content: string; metadata?: Record; }) => { + if (!isPanelCurrent()) return; if (!parentSession) { notificationService.warning(i18nService.t('common:reviewPlatform.messages.chatRequired'), { duration: 3500 }); return; } await openMainSession(parentSession.sessionId); + if (!isPanelCurrent()) return; const context: PullRequestContext = { id: createContextId('pr'), type: 'pull-request', @@ -1591,7 +1619,7 @@ function ReviewPlatformContent({ useContextStore.getState().addContext(context); window.dispatchEvent(new CustomEvent('insert-context-tag', { detail: { context } })); - }, [initialPullRequestUrl, parentSession, repository?.projectPath, selectedPr, selectedRemote?.id, selectedRemote?.projectPath]); + }, [isPanelCurrent, initialPullRequestUrl, parentSession, repository?.projectPath, selectedPr, selectedRemote?.id, selectedRemote?.projectPath]); const handleFillPrContext = useCallback(async () => { if (!selectedPr) return; @@ -1611,6 +1639,7 @@ function ReviewPlatformContent({ }, [addPullRequestContextToChat, detail, prFilePaths, repository, reviewItemCount, selectedPr, selectedRemote]); const handleStartReview = useCallback(async () => { + if (!isPanelCurrent()) return; if (!workspacePath || !selectedRemote || !repository || !selectedPr || !parentSession) { notificationService.warning(i18nService.t('common:reviewPlatform.messages.reviewChatRequired'), { duration: 3500, @@ -1630,6 +1659,7 @@ function ReviewPlatformContent({ selectedRemote.id, selectedPr.id, ); + if (!isPanelCurrent()) return; const freshPullRequest = reviewTarget.pullRequest; const freshIdentity = { platform: selectedRemote.platform, @@ -1682,6 +1712,7 @@ function ReviewPlatformContent({ repository, reviewTarget, }); + if (!isPanelCurrent()) return; if (prepared.mode === 'strict' && prepared.requiresConsent) { const confirmed = await confirmDeepReviewLaunch(prepared.runManifest, { sessionConcurrencyGuard: deriveDeepReviewSessionConcurrencyGuard( @@ -1689,7 +1720,7 @@ function ReviewPlatformContent({ parentSession.sessionId, ), }); - if (!confirmed) return; + if (!isPanelCurrent() || !confirmed) return; } if (runningReviewExists() || reviewLaunchesInFlight.has(sharedLaunchKey)) { return; @@ -1703,12 +1734,14 @@ function ReviewPlatformContent({ childSessionName: `Review: PR #${reviewTarget.pullRequest.number}`, prepared, }); + if (!isPanelCurrent()) return; if (launched.launchStatus === 'uncertain') { notificationService.warning(i18nService.t('common:reviewPlatform.messages.reviewUncertain'), { duration: 8000, }); } } catch (reviewError) { + if (!isPanelCurrent()) return; log.error('Failed to start pull request Review', { pullRequestId: selectedPr.id, error: reviewError, @@ -1722,9 +1755,10 @@ function ReviewPlatformContent({ reviewLaunchesInFlight.delete(sharedLaunchKey); } reviewLaunchInFlight.current = false; - setReviewLaunching(false); + if (isPanelCurrent()) setReviewLaunching(false); } }, [ + isPanelCurrent, confirmDeepReviewLaunch, tReview, latestCurrentReview?.lifecycle, @@ -1791,11 +1825,12 @@ function ReviewPlatformContent({ }, [addPullRequestContextToChat, ciLogById, loadCiLog, selectedPr]); const refreshAuthSnapshot = useCallback((remoteId: string | null) => { + if (!isPanelCurrent()) return; snapshotCache.clear(); detailCache.clear(); detailPageCache.clear(); void loadSnapshot(detailOnly ? remoteId : listRemoteId, { force: true, page: currentPageIndex + 1 }); - }, [currentPageIndex, detailOnly, listRemoteId, loadSnapshot]); + }, [isPanelCurrent, currentPageIndex, detailOnly, listRemoteId, loadSnapshot]); const handleOpenAuthModal = useCallback(() => { setAuthToken(''); @@ -1804,6 +1839,7 @@ function ReviewPlatformContent({ }, [setAuthError]); const handleSaveAuthToken = useCallback(async () => { + if (!isPanelCurrent()) return; if (!selectedRemote || selectedRemote.platform === 'unknown' || selectedRemote.platform === 'github') return; const token = authToken.trim(); if (!token) { @@ -1819,50 +1855,59 @@ function ReviewPlatformContent({ host: selectedRemote.host, token, }); + if (!isPanelCurrent()) return; setAuthModalOpen(false); setAuthToken(''); refreshAuthSnapshot(selectedRemote.id); } catch (err) { + if (!isPanelCurrent()) return; setAuthError(err, 'saveTokenFailed'); log.error('Failed to save review platform token', { error: err, host: selectedRemote.host }); } finally { - setAuthSaving(false); + if (isPanelCurrent()) setAuthSaving(false); } - }, [setAuthError, authToken, refreshAuthSnapshot, selectedRemote]); + }, [isPanelCurrent, setAuthError, authToken, refreshAuthSnapshot, selectedRemote]); const handleOpenGithubAuthTerminal = useCallback(async () => { + if (!isPanelCurrent()) return; if (!selectedRemote || selectedRemote.platform !== 'github') return; const command = `gh auth login --hostname ${selectedRemote.host}`; setAuthSaving(true); setAuthError(null); try { await systemAPI.setClipboard(command); + if (!isPanelCurrent()) return; setAuthModalOpen(false); quickActions.openTerminal(undefined, workspacePath); notificationService.success('GitHub CLI login command copied. Paste it in the terminal to continue.', { duration: 3500, }); } catch (err) { + if (!isPanelCurrent()) return; setAuthError(err, 'openAuthFailed'); log.error('Failed to prepare GitHub CLI authentication', { error: err, host: selectedRemote.host }); } finally { - setAuthSaving(false); + if (isPanelCurrent()) setAuthSaving(false); } - }, [setAuthError, selectedRemote, workspacePath]); + }, [isPanelCurrent, setAuthError, selectedRemote, workspacePath]); const handleCopyGithubAuthCommand = useCallback(async () => { + if (!isPanelCurrent()) return; if (!selectedRemote || selectedRemote.platform !== 'github') return; setAuthError(null); try { await systemAPI.setClipboard(`gh auth login --hostname ${selectedRemote.host}`); + if (!isPanelCurrent()) return; notificationService.success('GitHub CLI login command copied.', { duration: 2500 }); } catch (err) { + if (!isPanelCurrent()) return; setAuthError(err, 'copyAuthFailed'); log.error('Failed to copy GitHub CLI authentication command', { error: err, host: selectedRemote.host }); } - }, [setAuthError, selectedRemote]); + }, [isPanelCurrent, setAuthError, selectedRemote]); const handleClearAuthToken = useCallback(async () => { + if (!isPanelCurrent()) return; if (!selectedRemote || selectedRemote.platform === 'unknown') return; setAuthSaving(true); setAuthError(null); @@ -1871,15 +1916,17 @@ function ReviewPlatformContent({ platform: selectedRemote.platform, host: selectedRemote.host, }); + if (!isPanelCurrent()) return; refreshAuthSnapshot(selectedRemote.id); } catch (err) { + if (!isPanelCurrent()) return; setAuthError(err, 'clearTokenFailed'); setAuthModalOpen(true); log.error('Failed to clear review platform token', { error: err, host: selectedRemote.host }); } finally { - setAuthSaving(false); + if (isPanelCurrent()) setAuthSaving(false); } - }, [setAuthError, refreshAuthSnapshot, selectedRemote]); + }, [isPanelCurrent, setAuthError, refreshAuthSnapshot, selectedRemote]); const renderAuthGate = useCallback((mode: 'inline' | 'detail' = 'inline') => { if (!authChallenge || !selectedRemote || selectedRemote.platform === 'unknown') return null; @@ -2113,7 +2160,7 @@ function ReviewPlatformContent({ onClick={() => { if (pr.providerId && pr.providerId !== selectedRemoteId) { setSelectedRemoteId(pr.providerId); - rememberRemote(workspaceId, pr.providerId); + rememberRemote(surface.surfaceId, workspaceId, pr.providerId); } panelFocusRequested.current = true; setPanelView('detail'); diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.test.tsx b/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.test.tsx index 4ea330b677..b53b8a1d0f 100644 --- a/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.test.tsx +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.test.tsx @@ -658,7 +658,7 @@ describe('FlowChatHeader', () => { )?.click()); const section = document.querySelector('[data-testid="flowchat-header-pull-requests"]'); - expect(section?.textContent).toContain('common:reviewPlatform.errors.hostUnsupported'); + expect(section?.textContent).toContain('common:reviewPlatform.messages.hostUnsupported'); expect(section?.querySelector('button')?.disabled).toBe(true); expect(isGitRepositoryMock).not.toHaveBeenCalled(); expect(getWorkspaceSnapshotMock).not.toHaveBeenCalled(); @@ -682,7 +682,7 @@ describe('FlowChatHeader', () => { }); expect(getWorkspaceSnapshotMock).not.toHaveBeenCalled(); expect(document.querySelector('[data-testid="flowchat-header-pull-requests"]')?.textContent) - .toContain('common:reviewPlatform.errors.hostUnsupported'); + .toContain('common:reviewPlatform.messages.hostUnsupported'); }); it('reloads the same workspace ID on a device switch and ignores the previous device response', async () => { diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.tsx b/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.tsx index 0efacb4f71..7b2bf98538 100644 --- a/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.tsx +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatHeader.tsx @@ -576,7 +576,7 @@ export const FlowChatHeader: React.FC = ({ let pullRequestOverviewSummary: string; switch (pullRequestOverview.status) { case 'unsupported': - pullRequestOverviewSummary = t('common:reviewPlatform.errors.hostUnsupported'); + pullRequestOverviewSummary = t('common:reviewPlatform.messages.hostUnsupported'); break; case 'loading': pullRequestOverviewSummary = t('flowChatHeader.pullRequestLoading'); diff --git a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx index 07e5fac780..9449922514 100644 --- a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.test.tsx @@ -68,7 +68,7 @@ describe('ReviewPlatform tool card entry points', () => { render(); expect(button('toolCards.builtin.links.openReviewPanel')?.disabled).toBe(true); act(() => host.querySelector('[data-openbitfun-part="surface"]')?.click()); - expect(host.textContent).toContain('common:reviewPlatform.errors.hostUnsupported'); + expect(host.textContent).toContain('common:reviewPlatform.messages.hostUnsupported'); await act(async () => button('toolCards.builtin.links.openReview')?.click()); expect(state.openExternal).toHaveBeenCalledExactlyOnceWith(url); expect(state.openPanel).not.toHaveBeenCalled(); diff --git a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx index 2268fffa3a..afc1ff77cd 100644 --- a/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/BuiltinToolCard.tsx @@ -152,7 +152,7 @@ export const BuiltinToolCard: React.FC = ({ toolItem, sessionId, ? [model.notice, t('toolCards.builtin.hostUnavailable')].filter(Boolean).join('\n') : remoteLoopback ? [model.notice, t('toolCards.builtin.remoteLoopback')].filter(Boolean).join('\n') : !canViewPullRequests && allLinks.some(link => link.kind === 'review') - ? [model.notice, t('common:reviewPlatform.errors.hostUnsupported')].filter(Boolean).join('\n') : model.notice, + ? [model.notice, t('common:reviewPlatform.messages.hostUnsupported')].filter(Boolean).join('\n') : model.notice, connection: model.connection, ordered: model.ordered, actions: toActions(model.links), media: isExpanded && model.family === 'image-analysis' ? model.imageSources.length > 0 ? model.imageSources.map((src, index) => {model.summary) diff --git a/src/web-ui/src/locales/en-US/common.json b/src/web-ui/src/locales/en-US/common.json index c22a564e62..03ca18aea8 100644 --- a/src/web-ui/src/locales/en-US/common.json +++ b/src/web-ui/src/locales/en-US/common.json @@ -2056,7 +2056,6 @@ "repositoryUntrusted": "Git does not trust this repository. Confirm its ownership before retrying.", "remoteNotFound": "The selected remote no longer exists. Refresh and select a remote.", "unsupportedPlatform": "This repository provider is not supported.", - "hostUnsupported": "The pull request viewer is unavailable on this device. Open pull request links in the system browser.", "providerFailed": "The repository provider could not complete the request. Retry or check the provider service.", "authenticationRequired": "Authentication is required. Sign in or update the token and retry.", "permissionDenied": "Access was denied. Check the account and token permissions.", @@ -2068,6 +2067,7 @@ "targetIsPullRequest": "This item is a pull request. Open it as a pull request instead of an issue." }, "messages": { + "hostUnsupported": "The pull request viewer is unavailable on this device. Open pull request links in the system browser.", "noWorkspace": "No active workspace is available.", "unsupportedLink": "This link is not a supported pull request URL.", "detailsFailed": "Failed to load pull request details.", diff --git a/src/web-ui/src/locales/zh-CN/common.json b/src/web-ui/src/locales/zh-CN/common.json index 1b28309167..b4f4beda9d 100644 --- a/src/web-ui/src/locales/zh-CN/common.json +++ b/src/web-ui/src/locales/zh-CN/common.json @@ -2056,7 +2056,6 @@ "repositoryUntrusted": "Git 尚未信任此仓库，请确认仓库所有权后重试。", "remoteNotFound": "所选远程仓库已不存在，请刷新并重新选择。", "unsupportedPlatform": "不支持此仓库提供方。", - "hostUnsupported": "当前设备不支持内置 PR 查看，请使用系统浏览器打开 PR 链接。", "providerFailed": "仓库提供方无法完成请求，请重试或检查提供方服务。", "authenticationRequired": "需要认证，请登录或更新令牌后重试。", "permissionDenied": "访问被拒绝，请检查账号和令牌权限。", @@ -2068,6 +2067,7 @@ "targetIsPullRequest": "此条目是拉取请求，请按拉取请求打开，而不是按问题打开。" }, "messages": { + "hostUnsupported": "当前设备不支持内置 PR 查看，请使用系统浏览器打开 PR 链接。", "noWorkspace": "没有可用的活动工作区。", "unsupportedLink": "此链接不是受支持的拉取请求链接。", "detailsFailed": "加载拉取请求详情失败。", diff --git a/src/web-ui/src/locales/zh-TW/common.json b/src/web-ui/src/locales/zh-TW/common.json index f710eb5804..a3d2c5b05f 100644 --- a/src/web-ui/src/locales/zh-TW/common.json +++ b/src/web-ui/src/locales/zh-TW/common.json @@ -2056,7 +2056,6 @@ "repositoryUntrusted": "Git 尚未信任此儲存庫，請確認儲存庫擁有權後重試。", "remoteNotFound": "所選遠端儲存庫已不存在，請重新整理並重新選取。", "unsupportedPlatform": "不支援此儲存庫提供者。", - "hostUnsupported": "目前裝置不支援內建 PR 檢視，請使用系統瀏覽器開啟 PR 連結。", "providerFailed": "儲存庫提供者無法完成請求，請重試或檢查提供者服務。", "authenticationRequired": "需要驗證，請登入或更新權杖後重試。", "permissionDenied": "存取遭拒，請檢查帳號與權杖權限。", @@ -2068,6 +2067,7 @@ "targetIsPullRequest": "此項目是提取請求，請以提取請求開啟，而非以問題開啟。" }, "messages": { + "hostUnsupported": "目前裝置不支援內建 PR 檢視，請使用系統瀏覽器開啟 PR 連結。", "noWorkspace": "沒有可用的作用中工作區。", "unsupportedLink": "此連結不是支援的提取請求連結。", "detailsFailed": "載入提取請求詳細資料失敗。", From b95a9b57458e4bbf65a311aecd5f6f60235a99bc Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 10:12:56 +0800 Subject: [PATCH 05/12] fix(i18n): restore smart-mode availability search labels --- src/web-ui/src/locales/en-US/scenes/capabilities.json | 1 + src/web-ui/src/locales/zh-CN/scenes/capabilities.json | 1 + src/web-ui/src/locales/zh-TW/scenes/capabilities.json | 1 + 3 files changed, 3 insertions(+) diff --git a/src/web-ui/src/locales/en-US/scenes/capabilities.json b/src/web-ui/src/locales/en-US/scenes/capabilities.json index 0afd5d6975..14b14cf37e 100644 --- a/src/web-ui/src/locales/en-US/scenes/capabilities.json +++ b/src/web-ui/src/locales/en-US/scenes/capabilities.json @@ -469,6 +469,7 @@ "available-models": "Let an agent inspect models available under the current configuration", "model-catalog": "Inspect models.dev catalog status, refresh it manually, and reveal its local cache", "advanced-request": "Add custom headers, request-body fields, and SSL behavior per provider", + "smart-routing": "View smart-mode availability (not released)", "connection-test": "Test model connections or image generation and refresh the runtime client", "model-pools": "Manage model pools, routing strategies, and session model selection" } diff --git a/src/web-ui/src/locales/zh-CN/scenes/capabilities.json b/src/web-ui/src/locales/zh-CN/scenes/capabilities.json index 735a0037bd..8d05cc0b19 100644 --- a/src/web-ui/src/locales/zh-CN/scenes/capabilities.json +++ b/src/web-ui/src/locales/zh-CN/scenes/capabilities.json @@ -469,6 +469,7 @@ "available-models": "让智能体查看当前配置下可用的模型", "model-catalog": "查看 models.dev 目录状态、手动刷新并定位本地缓存", "advanced-request": "为供应商添加自定义 Header、请求体字段与 SSL 行为", + "smart-routing": "查看智能模式可用状态（暂未开放）", "connection-test": "测试模型连接或图像生成，并刷新运行时客户端", "model-pools": "管理模型池、路由策略与会话模型选择" } diff --git a/src/web-ui/src/locales/zh-TW/scenes/capabilities.json b/src/web-ui/src/locales/zh-TW/scenes/capabilities.json index 6e48fdc33f..9abc85ac72 100644 --- a/src/web-ui/src/locales/zh-TW/scenes/capabilities.json +++ b/src/web-ui/src/locales/zh-TW/scenes/capabilities.json @@ -469,6 +469,7 @@ "available-models": "讓智能體查看當前配置下可用的模型", "model-catalog": "查看 models.dev 目錄狀態、手動重新整理並定位本地緩存", "advanced-request": "為供應商添加自定義 Header、請求體字段與 SSL 行為", + "smart-routing": "查看智慧模式可用狀態（暫未開放）", "connection-test": "測試模型連線或圖像生成，並重新整理執行階段用戶端", "model-pools": "管理模型池、路由策略與工作階段模型選擇" } From edb5563c6a6cf0df772f3e281f4e90253f26e1f8 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 10:19:40 +0800 Subject: [PATCH 06/12] test: stabilize background routing fixture and document regression coverage --- src/crates/assembly/core/AGENTS.md | 9 ++ .../src/agentic/coordination/scheduler.rs | 92 ++++++++++--------- src/web-ui/AGENTS.md | 5 +- 3 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/crates/assembly/core/AGENTS.md b/src/crates/assembly/core/AGENTS.md index 67f6d5714c..b7ca1b0ca4 100644 --- a/src/crates/assembly/core/AGENTS.md +++ b/src/crates/assembly/core/AGENTS.md @@ -562,6 +562,15 @@ For host-owned user queue admission, cancellation, steering receipts and client cargo test --locked -p openbitfun-core --no-default-features --features remote-connect,git --lib host_queue_ ``` +For background-result delivery through the persisted logical session route: + +```bash +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git --lib background_result_ +``` + +The inline lifecycle fixture uses the existing dedicated test stack to retain its +task-local model configuration when CI exercises the full product assembly. + For Computer Use control host admission, cancellation leases, control entrypoints, permission projection and provider-neutral tool contracts: diff --git a/src/crates/assembly/core/src/agentic/coordination/scheduler.rs b/src/crates/assembly/core/src/agentic/coordination/scheduler.rs index 85cac4168b..d45af613bf 100644 --- a/src/crates/assembly/core/src/agentic/coordination/scheduler.rs +++ b/src/crates/assembly/core/src/agentic/coordination/scheduler.rs @@ -3907,51 +3907,55 @@ mod tests { assert_eq!(scheduler.queue_depth(session_id), 0); } - #[tokio::test] - async fn idle_background_result_uses_the_session_logical_agent_route() { - let (scheduler, session_manager, _, root) = test_scheduler(); - let session_id = "external-parent-session"; - let workspace = fixture_workspace_dir(root.path().join("workspace")); - session_manager - .create_session_with_id( - Some(session_id.to_string()), - "External parent".to_string(), - "Standard".to_string(), - SessionConfig { - tool_presentation_mode: Default::default(), - workspace_path: Some(workspace.to_string_lossy().into_owned()), - model_id: Some("model-original".to_string()), - ..Default::default() - }, - ) - .await - .expect("create parent session"); - - TEST_MODEL_RESOLUTION_AI_CONFIG - .scope( - AIConfig { - models: vec![AIModelConfig { - id: "model-original".to_string(), - name: "model-original".to_string(), - model_name: "model-original".to_string(), - enabled: true, + #[test] + fn idle_background_result_uses_the_session_logical_agent_route() { + // Inline admission retains the task-local model fixture and needs the + // same debug stack as the host-queue lifecycle tests with product-full. + run_host_queue_lifecycle_test(|| async { + let (scheduler, session_manager, _, root) = test_scheduler(); + let session_id = "external-parent-session"; + let workspace = fixture_workspace_dir(root.path().join("workspace")); + session_manager + .create_session_with_id( + Some(session_id.to_string()), + "External parent".to_string(), + "Standard".to_string(), + SessionConfig { + tool_presentation_mode: Default::default(), + workspace_path: Some(workspace.to_string_lossy().into_owned()), + model_id: Some("model-original".to_string()), ..Default::default() - }], - ..Default::default() - }, - scheduler.deliver_background_result( - session_id.to_string(), - "external::opencode::agentic::generation-v1".to_string(), - None, - None, - None, - "Background Bash command completed".to_string(), - None, - None, - ), - ) - .await - .expect("lifecycle delivery must follow the persisted logical session route"); + }, + ) + .await + .expect("create parent session"); + + TEST_MODEL_RESOLUTION_AI_CONFIG + .scope( + AIConfig { + models: vec![AIModelConfig { + id: "model-original".to_string(), + name: "model-original".to_string(), + model_name: "model-original".to_string(), + enabled: true, + ..Default::default() + }], + ..Default::default() + }, + scheduler.deliver_background_result( + session_id.to_string(), + "external::opencode::agentic::generation-v1".to_string(), + None, + None, + None, + "Background Bash command completed".to_string(), + None, + None, + ), + ) + .await + .expect("lifecycle delivery must follow the persisted logical session route"); + }); } #[tokio::test] diff --git a/src/web-ui/AGENTS.md b/src/web-ui/AGENTS.md index 516f6295ac..5b4dedaeb7 100644 --- a/src/web-ui/AGENTS.md +++ b/src/web-ui/AGENTS.md @@ -265,5 +265,6 @@ pnpm --dir src/web-ui run test:run src/infrastructure/markdown/MarkdownRenderer. ``` These DOM and handshake fixtures cover local, legacy Desktop, CLI and unknown peers, -device changes, overview/tool-card gates, restored panels, and the controller-owned URL opener. They do not establish live -macOS-to-Linux Peer Device or native browser behavior. +device changes, per-device remote preferences and legacy local migration, stale +review launches, overview/tool-card gates, restored panels, and the controller-owned +URL opener. They do not establish live macOS-to-Linux Peer Device or native browser behavior. From 0679e00efa710c257d2461530c072c5f57d7d9fc Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 10:44:53 +0800 Subject: [PATCH 07/12] test(runtime): preserve legacy smart selection while routing is unavailable --- src/crates/assembly/core/src/service_agent_runtime.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/crates/assembly/core/src/service_agent_runtime.rs b/src/crates/assembly/core/src/service_agent_runtime.rs index 10823fb523..2a67f0f155 100644 --- a/src/crates/assembly/core/src/service_agent_runtime.rs +++ b/src/crates/assembly/core/src/service_agent_runtime.rs @@ -4412,7 +4412,7 @@ mod tests { } #[test] - fn remote_model_strategy_guard_distinguishes_legacy_children_from_smart_main_sessions() { + fn remote_model_strategy_guard_preserves_legacy_smart_selection_and_rejects_children() { use crate::agentic::core::SessionConfig; let mut session = Session::new( @@ -4426,9 +4426,12 @@ mod tests { ); assert!(ensure_remote_model_strategy_editable(&session).is_ok()); session.config.model_routing = Some("smart".into()); - assert!(ensure_remote_model_strategy_editable(&session) - .unwrap_err() - .contains("Smart routing is active")); + // Smart mode is unavailable in this release. A persisted smart choice + // must still allow manual model edits without rewriting the old config. + assert!(!crate::agentic::execution::round_model_routing::configuration::available()); + let before = serde_json::to_value(&session.config).unwrap(); + assert!(ensure_remote_model_strategy_editable(&session).is_ok()); + assert_eq!(serde_json::to_value(&session.config).unwrap(), before); session.kind = SessionKind::Subagent; session.config.delegation_model_routing = Some("smart".into()); From 0d6a893e9dfdf254f865de7f1cd96ff4963b3e76 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 10:45:49 +0800 Subject: [PATCH 08/12] fix(cli): initialize scheduled jobs for embedded execution hosts --- src/apps/cli/AGENTS.md | 14 +++ src/apps/cli/README.md | 17 +++ src/apps/cli/src/main.rs | 37 +++++++ src/apps/cli/src/peer_host/commands/soft.rs | 6 +- src/apps/cli/src/runtime/mod.rs | 24 ++++ .../exec_cli_contracts.rs | 41 +++++++ src/apps/cli/tests/support/mod.rs | 84 +++++++++++--- .../cli/tests/terminal_process_contracts.rs | 103 ++++++++++++++++++ .../tools/implementations/cron_tool.rs | 10 +- 9 files changed, 314 insertions(+), 22 deletions(-) diff --git a/src/apps/cli/AGENTS.md b/src/apps/cli/AGENTS.md index 800b0ec412..a5ac8f2bc0 100644 --- a/src/apps/cli/AGENTS.md +++ b/src/apps/cli/AGENTS.md @@ -158,6 +158,20 @@ For streaming `exec` retry, context recovery, and final-event contracts: cargo test --locked -p openbitfun-cli --test cli_command_contracts exec_cli_contracts::stream_json_ ``` +For scheduled-job bootstrap, persisted tasks, and unattended execution after a +CLI restart (with a mock model and a real CLI process): + +```bash +cargo test --locked -p openbitfun-cli --bin openbitfun bootstrap_profile_tests:: +cargo test --locked -p openbitfun-cli --test terminal_process_contracts scheduled_jobs_ +cargo test --locked -p openbitfun-cli --test cli_command_contracts scheduled_jobs_ +``` + +Embedded Interactive/Execution bootstrap wires the existing Core Cron service, +subscribes to turn lifecycle events, and starts scheduling after host setup. +Shared TUI must not claim the host-wide scheduling lease: its workspace scope and +last-controller idle shutdown cannot own all persisted jobs. + For ACP stdio session restore, duplicate-load rejection, history replay and close: ```bash diff --git a/src/apps/cli/README.md b/src/apps/cli/README.md index 8b1833a123..5f8003b59e 100644 --- a/src/apps/cli/README.md +++ b/src/apps/cli/README.md @@ -94,6 +94,23 @@ is not automatic restart/recovery. Review the saved session and explicitly resum after an interruption. Completion still depends on the model verifying the user's requirements against real evidence; the runtime does not prove arbitrary tasks. +### Scheduled tasks + +The Agent can create, list, edit, remove, and run scheduled tasks through the +`Cron` tool in ordinary interactive CLI sessions and Peer Device sessions hosted +by that CLI, including the always-on `daemon run` host. The executing host owns +the schedule; a remote controller does not need to stay connected. Existing +workspace and permission rules still apply. + +Keep the CLI host running until tasks are due. Schedules survive process restarts, +and due tasks resume when a scheduling host starts again. `exec` also exposes +`Cron`, but exits after its turn; it does not stay alive to wait for future tasks. +Shared TUI (`--shared`), ACP, and the stdio `server` host do not start a scheduler. + +Only one process schedules tasks for a shared user data directory. If another +CLI or Desktop instance already owns scheduling, this instance can list tasks +but cannot edit or manually run them. It takes over after the owner exits. + ### Prompt continuity Unsent drafts stay with their session while the TUI remains open. Switching, diff --git a/src/apps/cli/src/main.rs b/src/apps/cli/src/main.rs index a53ca949be..b362f98007 100644 --- a/src/apps/cli/src/main.rs +++ b/src/apps/cli/src/main.rs @@ -616,6 +616,19 @@ impl BootstrapProfile { matches!(self, Self::Interactive | Self::Execution) } + const fn starts_scheduled_jobs( + self, + deployment: openbitfun_services_core::runtime_ownership::RuntimeDeployment, + ) -> bool { + // Shared TUI owns one workspace and exits after the last controller + // disconnects. It cannot own the host-wide durable scheduler lease. + matches!(self, Self::Interactive | Self::Execution) + && matches!( + deployment, + openbitfun_services_core::runtime_ownership::RuntimeDeployment::Embedded + ) + } + const fn starts_plugin_host(self) -> bool { matches!( PLUGIN_HOST_LAUNCH_POLICY, @@ -960,6 +973,19 @@ async fn initialize_core_services_for_deployment( tracing::info!("Agentic system initialized"); let workspace = create_cli_local_workspace(workspace_root).await?; + let scheduled_jobs = if bootstrap_profile.starts_scheduled_jobs(deployment) { + match runtime::initialize_scheduled_jobs(&agentic_system).await { + Ok(service) => Some(service), + Err(error) => { + tracing::warn!( + "Scheduled jobs are unavailable; persisted jobs were preserved: {error:#}" + ); + None + } + } + } else { + None + }; let runtime = std::sync::Arc::new(runtime::CliRuntimeContext::build( agentic_system, workspace, @@ -998,6 +1024,11 @@ async fn initialize_core_services_for_deployment( } } + if let Some(scheduled_jobs) = scheduled_jobs { + scheduled_jobs.start(); + tracing::info!("CLI scheduled-job service started"); + } + Ok(runtime) } @@ -1973,6 +2004,12 @@ mod bootstrap_profile_tests { ); assert_eq!(profile.starts_mcp(), starts_mcp); assert_eq!(profile.starts_plugin_host(), starts_plugin_host); + use openbitfun_services_core::runtime_ownership::RuntimeDeployment; + assert_eq!( + profile.starts_scheduled_jobs(RuntimeDeployment::Embedded), + starts_mcp + ); + assert!(!profile.starts_scheduled_jobs(RuntimeDeployment::Shared)); } } diff --git a/src/apps/cli/src/peer_host/commands/soft.rs b/src/apps/cli/src/peer_host/commands/soft.rs index 1b4f641439..7d0dfcd26a 100644 --- a/src/apps/cli/src/peer_host/commands/soft.rs +++ b/src/apps/cli/src/peer_host/commands/soft.rs @@ -2,8 +2,12 @@ use serde_json::{json, Value}; -/// Desktop cron host readiness signal — CLI has no cron host; acknowledge as ready. +/// The embedded CLI starts scheduling during bootstrap; repeated readiness +/// signals from peer controllers are idempotent. pub(crate) async fn notify_cron_host_ready() -> Result { + let service = openbitfun_core::service::cron::get_global_cron_service() + .ok_or_else(|| "Scheduled jobs are unavailable in this execution host".to_string())?; + service.start(); Ok(Value::Null) } diff --git a/src/apps/cli/src/runtime/mod.rs b/src/apps/cli/src/runtime/mod.rs index c3b34e1c4c..7fc9a55677 100644 --- a/src/apps/cli/src/runtime/mod.rs +++ b/src/apps/cli/src/runtime/mod.rs @@ -26,6 +26,30 @@ use approval::CliApprovalPolicy; const RUNTIME_EVENT_BUFFER: usize = 256; +/// Wire the existing scheduled-job owner before the CLI starts accepting turns. +/// The host starts its runner after account and transport initialization. +pub(crate) async fn initialize_scheduled_jobs( + agentic_system: &AgenticSystem, +) -> Result> { + use openbitfun_core::service::cron::{ + set_global_cron_service, CronEventSubscriber, CronService, + }; + + let cron = CronService::new( + openbitfun_core::infrastructure::try_get_path_manager_arc()?, + agentic_system.coordinator.clone(), + ensure_product_dialog_scheduler(agentic_system), + ) + .await + .context("Failed to initialize CLI scheduled jobs")?; + agentic_system.coordinator.subscribe_internal( + "cron_jobs".to_string(), + CronEventSubscriber::new(cron.clone()), + ); + set_global_cron_service(cron.clone()); + Ok(cron) +} + #[derive(Debug, Clone)] pub(crate) struct CliProductRuntimeState { plan: ProductAssemblyPlan, diff --git a/src/apps/cli/tests/cli_command_contracts/exec_cli_contracts.rs b/src/apps/cli/tests/cli_command_contracts/exec_cli_contracts.rs index dc1fb00eaa..9acc6878ef 100644 --- a/src/apps/cli/tests/cli_command_contracts/exec_cli_contracts.rs +++ b/src/apps/cli/tests/cli_command_contracts/exec_cli_contracts.rs @@ -140,6 +140,47 @@ fn cli_agent_controls_its_own_isolated_config_through_openbitfun_control() { assert_eq!(config["ai"]["tool_execution_timeout_secs"], 74); } +#[test] +fn scheduled_jobs_invalid_store_is_preserved_and_does_not_block_cli_turns() { + use serde_json::json; + let server = MockOpenAiServer::tool_sequence(vec![ + ("GetToolSpec".into(), json!({"tool_name":"Cron"})), + ( + "CallDeferredTool".into(), + json!({"tool_name":"Cron","args":{"action":"list"}}), + ), + ]); + let environment = CliTestEnvironment::new(); + environment.configure_tool_loop_mock_model(server.base_url()); + let path = environment.cron_jobs_path(); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + let unsupported_store = r#"{"version":999,"jobs":[{"futureField":"keep-me"}]}"#; + std::fs::write(&path, unsupported_store).unwrap(); + let mut command = environment.std_command(); + command.args([ + "exec", + "List scheduled jobs", + "--auto", + "--no-verify-final-changes", + "--output-format", + "json", + ]); + let output = command_output_with_timeout(&mut command, std::time::Duration::from_secs(60)); + assert!( + output.status.success(), + "{}\n{}", + stderr(&output), + stdout(&output) + ); + assert_eq!(std::fs::read_to_string(path).unwrap(), unsupported_store); + server.assert_chat_completion_requests(3); + let messages = server.chat_completion_request_bodies()[2]["messages"].to_string(); + assert!( + messages.contains("Scheduled jobs are unavailable in this execution host"), + "{messages}" + ); +} + #[test] fn exec_rejects_auto_with_legacy_confirm() { let output = run_cli(&["exec", "task", "--auto", "--confirm"]); diff --git a/src/apps/cli/tests/support/mod.rs b/src/apps/cli/tests/support/mod.rs index f0c2eb97a8..a413b4732c 100644 --- a/src/apps/cli/tests/support/mod.rs +++ b/src/apps/cli/tests/support/mod.rs @@ -158,18 +158,22 @@ impl CliTestEnvironment { } pub(crate) fn configure_product_control_mock_model(&self, server_base_url: &str) { + self.configure_tool_loop_mock_model(server_base_url); + } + + pub(crate) fn configure_tool_loop_mock_model(&self, server_base_url: &str) { self.configure_mock_model(server_base_url); let config_path = self.user_root.join("config/app.json"); let mut config: serde_json::Value = serde_json::from_slice( - &std::fs::read(&config_path).expect("read product-control model config"), + &std::fs::read(&config_path).expect("read tool-loop model config"), ) - .expect("parse product-control model config"); + .expect("parse tool-loop model config"); config["ai"]["max_rounds"] = json!(8); std::fs::write( config_path, - serde_json::to_vec_pretty(&config).expect("serialize product-control model config"), + serde_json::to_vec_pretty(&config).expect("serialize tool-loop model config"), ) - .expect("write product-control model config"); + .expect("write tool-loop model config"); } pub(crate) fn app_config(&self) -> serde_json::Value { @@ -179,6 +183,15 @@ impl CliTestEnvironment { .expect("parse CLI app config") } + pub(crate) fn cron_jobs_path(&self) -> PathBuf { + self.user_root.join("data/cron/jobs.json") + } + + pub(crate) fn cron_jobs(&self) -> serde_json::Value { + serde_json::from_slice(&std::fs::read(self.cron_jobs_path()).expect("read scheduled jobs")) + .expect("parse scheduled jobs") + } + pub(crate) fn configure_mock_image_model(&self, server_base_url: &str) { self.configure_mock_model_with_capabilities( server_base_url, @@ -317,6 +330,7 @@ enum MockModelResponse { Immediate, Gated, ProductControlLoop, + ToolSequence(Vec<(String, serde_json::Value)>), Http403 { reason: String }, DisconnectThenHttp403, MalformedSseThenImmediate, @@ -336,6 +350,10 @@ impl MockOpenAiServer { Self::spawn(MockModelResponse::ProductControlLoop) } + pub(crate) fn tool_sequence(calls: Vec<(String, serde_json::Value)>) -> Self { + Self::spawn(MockModelResponse::ToolSequence(calls)) + } + pub(crate) fn http_403(reason: impl Into) -> Self { Self::spawn(MockModelResponse::Http403 { reason: reason.into(), @@ -437,19 +455,21 @@ impl MockOpenAiServer { &disconnect_tx, ); attempt += 1; - let accepts_more_requests = - matches!( + let accepts_more_requests = matches!( + response, + MockModelResponse::Http403 { .. } + | MockModelResponse::DisconnectThenHttp403 + ) || (matches!( + response, + MockModelResponse::MalformedSseThenImmediate + ) && attempt < 2) + || (matches!( response, - MockModelResponse::Http403 { .. } - | MockModelResponse::DisconnectThenHttp403 - ) || (matches!(response, MockModelResponse::MalformedSseThenImmediate) - && attempt < 2) - || (matches!( - response, - MockModelResponse::ContextOverflowThenImmediate - ) && attempt < 3) - || (matches!(response, MockModelResponse::ProductControlLoop) - && attempt < 5); + MockModelResponse::ContextOverflowThenImmediate + ) && attempt < 3) + || (matches!(response, MockModelResponse::ProductControlLoop) + && attempt < 5) + || matches!(&response, MockModelResponse::ToolSequence(calls) if attempt <= calls.len()); if accepts_more_requests { continue; } @@ -528,6 +548,38 @@ fn serve_model_response( return; } + if let MockModelResponse::ToolSequence(calls) = response { + let (delta, finish_reason) = match calls.get(attempt) { + Some((name, arguments)) => ( + json!({"role":"assistant", "tool_calls":[{ + "index":0, "id":format!("call_sequence_{attempt}"), "type":"function", + "function":{"name":name,"arguments":arguments.to_string()} + }]}), + "tool_calls", + ), + None => ( + json!({"role":"assistant", "content":STREAM_COMPLETED_MARKER}), + "stop", + ), + }; + write_sse_chunk( + stream, + &json!({ + "id":format!("chatcmpl_sequence_{attempt}"), "object":"chat.completion.chunk", + "created":attempt, "model":"cli-e2e-model", + "choices":[{"index":0,"delta":delta,"finish_reason":finish_reason}] + }) + .to_string(), + ) + .expect("write scripted tool response"); + write_chunk(stream, b"data: [DONE]\n\n").expect("finish scripted tool SSE"); + stream + .write_all(b"0\r\n\r\n") + .expect("finish scripted response"); + stream.flush().expect("flush scripted response"); + return; + } + if matches!(response, MockModelResponse::MalformedSseThenImmediate) && attempt == 0 { write_chunk(stream, b"data: not-json\n\n").expect("write malformed SSE frame"); let _ = stream.write_all(b"0\r\n\r\n"); diff --git a/src/apps/cli/tests/terminal_process_contracts.rs b/src/apps/cli/tests/terminal_process_contracts.rs index fce7d4e8ba..747d9b22b0 100644 --- a/src/apps/cli/tests/terminal_process_contracts.rs +++ b/src/apps/cli/tests/terminal_process_contracts.rs @@ -36,6 +36,109 @@ const MULTILINE_TAIL_SENTINEL: &str = "Z9R2"; const RECOVERY_INPUT: &[u8] = b"READY_AFTER_CANCEL K4W8"; const RECOVERY_INPUT_SENTINEL: &str = "K4W8"; +#[test] +fn scheduled_jobs_survive_cli_restart_and_run_without_a_controller_turn() { + use serde_json::json; + let environment = CliTestEnvironment::new(); + let cron_spec = || ("GetToolSpec".to_string(), json!({"tool_name":"Cron"})); + let cron_call = |args| { + ( + "CallDeferredTool".to_string(), + json!({"tool_name":"Cron", "args":args}), + ) + }; + let payload = "CLI_SCHEDULED_WAKEUP_7Q9"; + let creator = MockOpenAiServer::tool_sequence(vec![ + cron_spec(), + cron_call(json!({"action":"add", "job":{ + "name":"CLI scheduled restart check", "schedule":{"kind":"at","at":"2099-01-01T00:00:00Z"}, + "payload":payload, "enabled":true + }})), + cron_call(json!({"action":"list"})), + ]); + environment.configure_tool_loop_mock_model(creator.base_url()); + let run_exec = || { + let mut command = environment.std_command(); + command.args([ + "exec", + "Manage the scheduled restart check", + "--auto", + "--no-verify-final-changes", + "--output-format", + "json", + ]); + let output = support::command_output_with_timeout(&mut command, Duration::from_secs(60)); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); + }; + run_exec(); + creator.assert_chat_completion_requests(4); + let mut jobs = environment.cron_jobs(); + assert_eq!(jobs["jobs"].as_array().unwrap().len(), 1); + assert_eq!(jobs["jobs"][0]["payload"]["text"], payload); + let job_id = jobs["jobs"][0]["id"].as_str().unwrap().to_string(); + assert!(creator.chat_completion_request_bodies()[3]["messages"] + .to_string() + .contains(&job_id)); + + let reader = + MockOpenAiServer::tool_sequence(vec![cron_spec(), cron_call(json!({"action":"list"}))]); + environment.configure_tool_loop_mock_model(reader.base_url()); + run_exec(); + reader.assert_chat_completion_requests(3); + assert!(reader.chat_completion_request_bodies()[2]["messages"] + .to_string() + .contains(&job_id)); + + // Advance the persisted deadline while no host is running. The restarted + // interactive host must restore the session and execute without UI input. + let due = chrono::Utc::now() - chrono::Duration::seconds(1); + jobs["jobs"][0]["schedule"]["at"] = json!(due.to_rfc3339()); + jobs["jobs"][0]["state"]["nextRunAtMs"] = json!(due.timestamp_millis()); + std::fs::write( + environment.cron_jobs_path(), + serde_json::to_vec_pretty(&jobs).unwrap(), + ) + .unwrap(); + let runner = MockOpenAiServer::immediate(); + environment.configure_mock_model(runner.base_url()); + let mut process = PtyProcess::spawn(environment.pty_command(), INITIAL_SIZE); + process.expect_output( + "\x1b[?2004h", + Duration::from_secs(60), + "scheduled host did not start", + ); + let deadline = Instant::now() + Duration::from_secs(60); + loop { + let persisted = environment.cron_jobs(); + let job = &persisted["jobs"][0]; + if job["state"]["lastRunStatus"] == "ok" { + assert_eq!(job["enabled"], false, "one-shot job must finish once"); + assert!(job["state"]["activeTurnId"].is_null()); + assert!(job["state"]["lastRunStartedAtMs"].is_number()); + assert!(job["state"]["lastRunFinishedAtMs"].is_number()); + break; + } + assert!( + Instant::now() < deadline, + "scheduled job did not settle: {persisted}; terminal: {}", + process.output() + ); + thread::sleep(Duration::from_millis(25)); + } + runner.assert_chat_completion_requests(1); + assert!(runner.chat_completion_request_bodies()[0]["messages"] + .to_string() + .contains(payload)); + process.write(&[0x03]); + let (status, output) = process.finish(Duration::from_secs(15)); + assert!(status.success(), "{output}"); +} + #[cfg(unix)] #[test] fn startup_bracketed_paste_attaches_an_image_path_without_rendering_the_path() { diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/cron_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/cron_tool.rs index 4221b93a29..ea6c41cac8 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/cron_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/cron_tool.rs @@ -1171,7 +1171,7 @@ Patch schema for "update": } CronAction::List => { let cron_service = get_global_cron_service().ok_or_else(|| { - OpenBitFunError::tool("cron service not initialized".to_string()) + OpenBitFunError::tool("Scheduled jobs are unavailable in this execution host. Use a host with scheduled-job support and keep it running until the job is due.".to_string()) })?; let session_id = self.resolve_effective_session_id(params.session_id.as_deref(), context)?; @@ -1241,7 +1241,7 @@ Patch schema for "update": } CronAction::Add => { let cron_service = get_global_cron_service().ok_or_else(|| { - OpenBitFunError::tool("cron service not initialized".to_string()) + OpenBitFunError::tool("Scheduled jobs are unavailable in this execution host. Use a host with scheduled-job support and keep it running until the job is due.".to_string()) })?; let session_id = self.resolve_effective_session_id(params.session_id.as_deref(), context)?; @@ -1286,7 +1286,7 @@ Patch schema for "update": } CronAction::Update => { let cron_service = get_global_cron_service().ok_or_else(|| { - OpenBitFunError::tool("cron service not initialized".to_string()) + OpenBitFunError::tool("Scheduled jobs are unavailable in this execution host. Use a host with scheduled-job support and keep it running until the job is due.".to_string()) })?; let job_id = params.job_id.ok_or_else(|| { OpenBitFunError::tool("job_id is required for update".to_string()) @@ -1338,7 +1338,7 @@ Patch schema for "update": } CronAction::Remove => { let cron_service = get_global_cron_service().ok_or_else(|| { - OpenBitFunError::tool("cron service not initialized".to_string()) + OpenBitFunError::tool("Scheduled jobs are unavailable in this execution host. Use a host with scheduled-job support and keep it running until the job is due.".to_string()) })?; let job_id = params.job_id.ok_or_else(|| { OpenBitFunError::tool("job_id is required for remove".to_string()) @@ -1365,7 +1365,7 @@ Patch schema for "update": } CronAction::Run => { let cron_service = get_global_cron_service().ok_or_else(|| { - OpenBitFunError::tool("cron service not initialized".to_string()) + OpenBitFunError::tool("Scheduled jobs are unavailable in this execution host. Use a host with scheduled-job support and keep it running until the job is due.".to_string()) })?; let job_id = params.job_id.ok_or_else(|| { OpenBitFunError::tool("job_id is required for run".to_string()) From 6312275eee841fbf6b824e943ea4db59cd8476ef Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 11:10:40 +0800 Subject: [PATCH 09/12] test(ai-adapters): repair health check fixture configuration and timing --- src/crates/adapters/ai-adapters/src/client/healthcheck.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/crates/adapters/ai-adapters/src/client/healthcheck.rs b/src/crates/adapters/ai-adapters/src/client/healthcheck.rs index 900084cfb8..aca52f532f 100644 --- a/src/crates/adapters/ai-adapters/src/client/healthcheck.rs +++ b/src/crates/adapters/ai-adapters/src/client/healthcheck.rs @@ -390,6 +390,7 @@ mod tests { format: "openai".into(), context_window: 65_536, max_tokens: Some(128), + max_images_per_request: None, temperature: None, top_p: None, inline_think_in_text: false, @@ -588,7 +589,9 @@ mod tests { assert_eq!(server.state.requests.lock().unwrap().len(), 2); } - #[tokio::test(start_paused = true)] + // This fixture uses real TCP IO. A paused clock can advance request + // timeouts before the OS delivers the response and hide a retry attempt. + #[tokio::test] async fn transient_errors_keep_the_shared_retry_policy_and_attempt_budget() { let server = ProbeServer::spawn( Some((StatusCode::SERVICE_UNAVAILABLE, "Service unavailable")), From 24250a6771c137252b7b66cc5823f825f60de8d2 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 12:26:28 +0800 Subject: [PATCH 10/12] fix(remote): complete workspace HTML previews and CLI file browsing --- docs/interactive-capabilities/README.md | 4 +- .../technical/tauri-command-map.json | 22 +- .../cli/src/peer_host/commands/filesystem.rs | 138 ++- src/apps/cli/src/peer_host/commands/mod.rs | 17 +- src/apps/desktop/AGENTS.md | 11 + src/apps/desktop/src/api/html_preview_api.rs | 809 +++++++++--------- .../desktop/src/api/html_preview_api/tests.rs | 394 +++++++++ .../remote_connect_api/relay_routes_tests.rs | 146 ++++ src/apps/desktop/src/lib.rs | 1 + src/crates/assembly/core/AGENTS.md | 6 + .../src/service/filesystem/html_preview.rs | 139 +++ .../core/src/service/filesystem/mod.rs | 2 + .../generated/remote-surface-registry.json | 28 +- .../product-domains/src/html_preview.rs | 115 +++ .../contracts/product-domains/src/lib.rs | 1 + .../src/remote_surface/capabilities.rs | 5 + .../src/remote_surface/table.rs | 9 +- .../services/services-integrations/AGENTS.md | 13 + .../src/remote_connect.rs | 1 + .../src/remote_connect/html_preview.rs | 333 +++++++ .../src/remote_connect/html_preview/tests.rs | 347 ++++++++ .../manager/tests/workspace_sftp.rs | 112 +++ .../interactive-capabilities/catalog.json | 1 + .../scenes/browser/HtmlPreviewPanel.test.tsx | 81 ++ .../app/scenes/browser/HtmlPreviewPanel.tsx | 32 +- src/web-ui/src/app/scenes/browser/README.md | 29 + .../api/generated/remoteSurface.ts | 5 +- .../infrastructure/api/htmlPreviewApi.test.ts | 50 +- .../src/infrastructure/api/htmlPreviewApi.ts | 26 +- .../src/infrastructure/peer-device/README.md | 15 + tests/e2e/AGENTS.md | 13 + tests/e2e/scripts/run-html-preview.mjs | 154 ++++ 32 files changed, 2585 insertions(+), 474 deletions(-) create mode 100644 src/apps/desktop/src/api/html_preview_api/tests.rs create mode 100644 src/crates/assembly/core/src/service/filesystem/html_preview.rs create mode 100644 src/crates/contracts/product-domains/src/html_preview.rs create mode 100644 src/crates/services/services-integrations/src/remote_connect/html_preview.rs create mode 100644 src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs create mode 100644 src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.test.tsx create mode 100644 tests/e2e/scripts/run-html-preview.mjs diff --git a/docs/interactive-capabilities/README.md b/docs/interactive-capabilities/README.md index 7f763c60de..a21d37a511 100644 --- a/docs/interactive-capabilities/README.md +++ b/docs/interactive-capabilities/README.md @@ -27,9 +27,9 @@ OpenBitFun Playbook currently contains **23 features**, **21 settings pages**, a - Generated per-item interaction audit: `docs/interactive-capabilities/technical/product-control-open-audit.json` - Generated low-level audit map: `docs/interactive-capabilities/technical/tauri-command-map.json` -说明书、网站、搜索和智能体只看“功能 + 设置 + 子能力”。每项子能力都必须引用已注册 Tauri Command 或可解析的源码标记；这些证据不会进入公开目录。当前 **705** 个 Tauri 命令只用于实现覆盖审计。产品 UI 交互源码会在生成和检查时扫描并校验，但不会保存成随普通 UI 改动频繁变化的版本化快照。 +说明书、网站、搜索和智能体只看“功能 + 设置 + 子能力”。每项子能力都必须引用已注册 Tauri Command 或可解析的源码标记；这些证据不会进入公开目录。当前 **706** 个 Tauri 命令只用于实现覆盖审计。产品 UI 交互源码会在生成和检查时扫描并校验，但不会保存成随普通 UI 改动频繁变化的版本化快照。 -Docs, website, search, and agents see only features, settings, and documented sub-capabilities. Every sub-capability must reference a registered Tauri command or a resolvable source marker; evidence is stripped from public projections. The **705** Tauri commands remain implementation-audit evidence only. Product UI interaction sources are scanned and validated during generation and checks, but are not stored as a versioned snapshot that churns with ordinary UI changes. +Docs, website, search, and agents see only features, settings, and documented sub-capabilities. Every sub-capability must reference a registered Tauri command or a resolvable source marker; evidence is stripped from public projections. The **706** Tauri commands remain implementation-audit evidence only. Product UI interaction sources are scanned and validated during generation and checks, but are not stored as a versioned snapshot that churns with ordinary UI changes. ## 控制边界 / Control boundary diff --git a/docs/interactive-capabilities/technical/tauri-command-map.json b/docs/interactive-capabilities/technical/tauri-command-map.json index 84de94e437..bb7479f679 100644 --- a/docs/interactive-capabilities/technical/tauri-command-map.json +++ b/docs/interactive-capabilities/technical/tauri-command-map.json @@ -2,12 +2,12 @@ "schemaVersion": 2, "generatedFrom": "src/shared/interactive-capabilities/catalog.json", "catalogDigest": "40fada109d5dbcfdbc1b289af574f7ac736da1620d4eee26128a40d3f0f5818c", - "commandCount": 705, + "commandCount": 706, "coverage": { - "commandCount": 705, + "commandCount": 706, "documentedCommandCount": 646, - "implementationCommandCount": 59, - "implementationDigest": "0bce9fbda3e8b517e409aec124ca0f2c3b36a7954d996400d9ccf405194dc768" + "implementationCommandCount": 60, + "implementationDigest": "5047b674e890f899e65baf9235e9df4bd912fa28f44eb10a656e079a1b89974d" }, "commands": [ { @@ -5011,6 +5011,20 @@ "signature": "fn html_preview_create( app_state: tauri::State<'_, AppState>, request: HtmlPreviewCreateRequest, ) -> Result", "remoteWorkspacePolicy": "RemoteRouted" }, + { + "id": "html_preview_read_resource", + "moduleId": "html_preview", + "capabilityId": "feature.files-editor", + "capabilityIds": [ + "feature.files-editor" + ], + "documentedItemIds": [], + "visibility": "implementation", + "rustPath": "api::html_preview_api::html_preview_read_resource", + "sourceFile": "src/apps/desktop/src/api/html_preview_api.rs", + "signature": "fn html_preview_read_resource( request: HtmlPreviewResourceRequest, ) -> HtmlPreviewResourceReply", + "remoteWorkspacePolicy": "RemoteRouted" + }, { "id": "html_preview_release", "moduleId": "html_preview", diff --git a/src/apps/cli/src/peer_host/commands/filesystem.rs b/src/apps/cli/src/peer_host/commands/filesystem.rs index 7422cef735..6a0fec7c7d 100644 --- a/src/apps/cli/src/peer_host/commands/filesystem.rs +++ b/src/apps/cli/src/peer_host/commands/filesystem.rs @@ -1,16 +1,32 @@ //! Filesystem HostInvoke handlers. +use openbitfun_core::service::{filesystem::FileSystemService, workspace::WorkspaceService}; use serde_json::{json, Value}; use crate::peer_host::args::{get_string, optional_string, request_value}; use crate::peer_host::state::PeerHostState; +pub(crate) async fn html_preview_read_resource(args: &Value) -> Result { + let request = serde_json::from_value(request_value(args).clone()) + .map_err(|error| format!("Invalid HTML preview request: {error}"))?; + let reply = + openbitfun_core::service::filesystem::html_preview::html_preview_read_resource(request) + .await; + serde_json::to_value(reply).map_err(|error| error.to_string()) +} + async fn file_connection(state: &PeerHostState, request: &Value) -> Result, String> { + directory_connection(&state.workspace_service, request).await +} + +async fn directory_connection( + workspaces: &WorkspaceService, + request: &Value, +) -> Result, String> { if let Some(id) = optional_string(request, "workspaceId").or_else(|| optional_string(request, "workspace_id")) { - let record = state - .workspace_service + let record = workspaces .require_workspace(&id) .await .map_err(|e| e.to_string())?; @@ -18,8 +34,7 @@ async fn file_connection(state: &PeerHostState, request: &Value) -> Result Result { let request = request_value(args); let path = get_string(request, "path")?; - let preferred = file_connection(state, request).await?; - let nodes = state - .filesystem_service + let preferred = directory_connection(workspaces, request).await?; + let nodes = files .get_directory_contents_on_connection(&path, preferred.as_deref()) .await .map_err(|e| format!("Failed to get directory children: {e}"))?; @@ -63,17 +78,17 @@ pub(crate) async fn get_directory_children( } pub(crate) async fn get_directory_children_paginated( - state: &PeerHostState, + files: &FileSystemService, + workspaces: &WorkspaceService, args: &Value, ) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let preferred = file_connection(state, request).await?; + let preferred = directory_connection(workspaces, request).await?; let offset = request.get("offset").and_then(|v| v.as_u64()).unwrap_or(0) as usize; let limit = request.get("limit").and_then(|v| v.as_u64()).unwrap_or(100) as usize; - let mut nodes = state - .filesystem_service + let mut nodes = files .get_directory_contents_on_connection(&path, preferred.as_deref()) .await .map_err(|e| format!("Failed to get paginated directory children: {e}"))?; @@ -95,6 +110,44 @@ pub(crate) async fn get_directory_children_paginated( })) } +pub(crate) async fn get_file_tree( + files: &FileSystemService, + workspaces: &WorkspaceService, + args: &Value, +) -> Result { + let request = request_value(args); + let path = get_string(request, "path")?; + let preferred = directory_connection(workspaces, request).await?; + let nodes = files + .build_file_tree_on_connection(&path, preferred.as_deref()) + .await + .map_err(|error| format!("Failed to build file tree: {error}"))?; + fn node_value(mut node: openbitfun_core::infrastructure::FileTreeNode) -> Value { + let children = node.children.take(); + let mut value = directory_nodes_to_json(vec![node]).pop().unwrap(); + if let Some(children) = children { + value["children"] = json!(children.into_iter().map(node_value).collect::>()); + } + value + } + let name = if preferred.as_deref().is_some_and(|id| !id.is_empty()) { + path.trim_end_matches('/') + .rsplit('/') + .next() + .unwrap_or(&path) + } else { + std::path::Path::new(&path) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(&path) + }; + Ok(json!([{ + "path": path, "name": name, "isDirectory": true, + "size": null, "extension": null, "lastModified": null, + "children": nodes.into_iter().map(node_value).collect::>() + }])) +} + pub(crate) async fn check_path_exists( state: &PeerHostState, args: &Value, @@ -257,3 +310,64 @@ pub(crate) async fn workspace_file_upload( .await?; serde_json::to_value(result).map_err(|error| error.to_string()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn explorer_lists_registered_workspace_children_pages_and_tree() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path().join("workspace"); + std::fs::create_dir_all(root.join("nested")).unwrap(); + std::fs::write(root.join("alpha.txt"), "alpha").unwrap(); + std::fs::write(root.join("zeta.html"), "page").unwrap(); + std::fs::write(root.join("nested/child.js"), "script").unwrap(); + let workspaces = + WorkspaceService::new_isolated_for_tests(temporary.path().join("user")).await; + let workspace = workspaces + .register_local_workspace_record(root.clone()) + .await + .unwrap(); + let files = FileSystemService::default(); + let args = json!({"request": {"workspaceId": workspace.id, "path": root}}); + let children = get_directory_children(&files, &workspaces, &args) + .await + .unwrap(); + let children = children.as_array().unwrap(); + assert_eq!(children.len(), 3); + assert!(children + .iter() + .any(|node| node["name"] == "nested" && node["isDirectory"] == true)); + assert!(children + .iter() + .any(|node| node["name"] == "zeta.html" && node["isDirectory"] == false)); + let mut paginated = args.clone(); + paginated["request"]["offset"] = json!(1); + paginated["request"]["limit"] = json!(1); + paginated["request"]["sortBy"] = json!("name"); + paginated["request"]["sortOrder"] = json!("asc"); + let page = get_directory_children_paginated(&files, &workspaces, &paginated) + .await + .unwrap(); + assert_eq!(page["total"], 3); + assert_eq!(page["hasMore"], true); + assert_eq!(page["children"][0]["name"], "alpha.txt"); + let tree = get_file_tree(&files, &workspaces, &args).await.unwrap(); + assert_eq!(tree[0]["name"], "workspace"); + assert_eq!(tree[0]["isDirectory"], true); + let nested = tree[0]["children"] + .as_array() + .unwrap() + .iter() + .find(|node| node["name"] == "nested") + .unwrap(); + assert_eq!(nested["children"][0]["name"], "child.js"); + let mut invalid = args; + invalid["request"]["workspaceId"] = json!("missing-workspace"); + assert!(get_directory_children(&files, &workspaces, &invalid) + .await + .is_err()); + assert!(get_file_tree(&files, &workspaces, &invalid).await.is_err()); + } +} diff --git a/src/apps/cli/src/peer_host/commands/mod.rs b/src/apps/cli/src/peer_host/commands/mod.rs index b02ec10196..ad404ea726 100644 --- a/src/apps/cli/src/peer_host/commands/mod.rs +++ b/src/apps/cli/src/peer_host/commands/mod.rs @@ -109,11 +109,15 @@ pub(crate) fn dispatch<'a>( "terminal_shutdown_all" => Box::pin(terminal::shutdown_all()), // Filesystem "workspace_file_upload" => Box::pin(filesystem::workspace_file_upload(state, args)), - "get_directory_children" | "list_files" => { - Box::pin(filesystem::get_directory_children(state, args)) + "html_preview_read_resource" => Box::pin(filesystem::html_preview_read_resource(args)), + "get_directory_children" | "explorer_get_children" | "list_files" => { + Box::pin(filesystem::get_directory_children(&state.filesystem_service, &state.workspace_service, args)) } - "get_directory_children_paginated" => { - Box::pin(filesystem::get_directory_children_paginated(state, args)) + "get_directory_children_paginated" | "explorer_get_children_paginated" => { + Box::pin(filesystem::get_directory_children_paginated(&state.filesystem_service, &state.workspace_service, args)) + } + "get_file_tree" | "explorer_get_file_tree" => { + Box::pin(filesystem::get_file_tree(&state.filesystem_service, &state.workspace_service, args)) } "check_path_exists" => Box::pin(filesystem::check_path_exists(state, args)), "create_directory" => Box::pin(filesystem::create_directory(state, args)), @@ -296,8 +300,13 @@ pub(crate) const HANDLED_COMMANDS: &[&str] = &[ "get_configs", "get_current_workspace", "workspace_file_upload", + "html_preview_read_resource", "get_directory_children", "get_directory_children_paginated", + "get_file_tree", + "explorer_get_children", + "explorer_get_children_paginated", + "explorer_get_file_tree", "get_external_source_control_snapshot", "get_external_source_discovery_snapshot", "get_external_source_snapshot", diff --git a/src/apps/desktop/AGENTS.md b/src/apps/desktop/AGENTS.md index 22240eb8d1..2dfc724dfb 100644 --- a/src/apps/desktop/AGENTS.md +++ b/src/apps/desktop/AGENTS.md @@ -348,3 +348,14 @@ The host resolves the saved model, rejects non-API authentication and custom aut and rechecks its authorization binding before publishing the result. Model-scoped requests require the separately advertised `subscription_model_quota_v1` capability; legacy requests retain the exact account-slot path. These tests do not contact billing. + +For workspace HTML gateway authentication, chunking, cancellation and peer +capability negotiation and encrypted loopback Relay transfer, run: + +```bash +cargo test --locked -p openbitfun-desktop --lib html_preview +``` + +Use `node tests/e2e/scripts/run-html-preview.mjs` after building Desktop and Web UI +to verify CSS, JavaScript modules, images and concurrent preview cookies in native +browser child views. The fixture uses isolated storage; see `tests/e2e/AGENTS.md`. diff --git a/src/apps/desktop/src/api/html_preview_api.rs b/src/apps/desktop/src/api/html_preview_api.rs index ddd2fee880..b6a94e8edb 100644 --- a/src/apps/desktop/src/api/html_preview_api.rs +++ b/src/apps/desktop/src/api/html_preview_api.rs @@ -1,36 +1,50 @@ -//! Read-only HTTP gateway used to preview workspace HTML files in the embedded browser. +//! Controller-owned HTTP gateway for workspace HTML, including SSH and peer hosts. use crate::api::app_state::AppState; -use crate::api::path_target::{resolve_desktop_path_target, DesktopPathTarget}; -use axum::body::Body; -use axum::extract::State; -use axum::http::{header, HeaderMap, HeaderValue, Method, StatusCode, Uri}; -use axum::response::Response; -use axum::routing::any; -use axum::Router; -use openbitfun_core::service::remote_ssh::RemoteFileService; +use crate::api::remote_connect_api::{ + account_context_generation, account_context_is_current, account_device_rpc, +}; +use axum::{ + body::Body, + extract::State, + http::{header, HeaderMap, Method, StatusCode, Uri}, + response::Response, + routing::any, + Router, +}; +use base64::{engine::general_purpose::STANDARD as BASE64, Engine}; +use openbitfun_core::service::filesystem::html_preview::resolve_html_preview_source; +use openbitfun_product_domains::{ + html_preview::{ + HtmlPreviewError, HtmlPreviewErrorCode as Code, HtmlPreviewResource, + HtmlPreviewResourceReply, HtmlPreviewResourceRequest, MAX_PREVIEW_BYTES, + PREVIEW_CHUNK_BYTES, + }, + remote_surface::PeerHostCapability, +}; +use openbitfun_services_integrations::remote_connect::{ + html_preview::HtmlPreviewSource, RemoteCommand, RemoteResponse, +}; use serde::{Deserialize, Serialize}; -use std::path::{Path as FsPath, PathBuf}; use std::sync::Arc; -use tokio::net::TcpListener; -use tokio::sync::{oneshot, RwLock}; +use tokio::{net::TcpListener, sync::watch}; use uuid::Uuid; -const MAX_PREVIEW_BYTES: u64 = 64 * 1024 * 1024; - -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct HtmlPreviewCreateRequest { pub file_path: String, #[serde(default)] pub workspace_id: Option, - /// Temporary pre-ID wire field; normal clients send workspaceId. + /// Temporary pre-ID wire fields; current clients send workspaceId. #[serde(default)] pub workspace_path: String, #[serde(default)] pub remote_connection_id: Option, #[serde(default)] pub peer_device_mode: bool, + #[serde(default)] + pub peer_device_id: Option, } #[derive(Debug, Serialize)] @@ -46,30 +60,132 @@ pub struct HtmlPreviewReleaseRequest { pub session_id: String, } -#[derive(Clone)] -enum PreviewRoot { - Local { root: PathBuf }, - Remote { root: String, connection_id: String }, +#[async_trait::async_trait] +trait PreviewReader: Send + Sync { + fn is_current(&self) -> bool { + true + } + async fn read( + &self, + request: &HtmlPreviewResourceRequest, + ) -> Result; +} + +#[async_trait::async_trait] +impl PreviewReader for HtmlPreviewSource { + async fn read( + &self, + request: &HtmlPreviewResourceRequest, + ) -> Result { + HtmlPreviewSource::read(self, request).await + } +} + +struct PeerPreviewReader { + device_id: String, + account_generation: u64, +} + +impl PeerPreviewReader { + async fn invoke( + &self, + command: &str, + args: serde_json::Value, + ) -> Result { + if !self.is_current() { + return Err(HtmlPreviewError::unavailable( + "HTML preview account changed; reopen the preview", + )); + } + let command = serde_json::to_string(&RemoteCommand::HostInvoke { + command: command.to_string(), + args, + }) + .map_err(|error| HtmlPreviewError::unavailable(error.to_string()))?; + let raw = account_device_rpc(self.device_id.clone(), command, None) + .await + .map_err(HtmlPreviewError::unavailable)?; + if !self.is_current() { + return Err(HtmlPreviewError::unavailable( + "HTML preview account changed; reopen the preview", + )); + } + decode_peer_result(&raw) + } + + async fn ensure_supported(&self) -> Result<(), HtmlPreviewError> { + let ping = self.invoke("peer_mode_ping", serde_json::json!({})).await?; + require_preview_capability(&ping) + } +} + +fn require_preview_capability(ping: &serde_json::Value) -> Result<(), HtmlPreviewError> { + if ping + .get("capabilities") + .and_then(|caps| caps.get(PeerHostCapability::WorkspaceHtmlPreviewV1.key())) + .and_then(serde_json::Value::as_bool) + != Some(true) + { + return Err(HtmlPreviewError::unavailable("This device does not support HTML preview resources; update OpenBitFun on the controlled device")); + } + Ok(()) +} + +fn decode_peer_result(raw: &str) -> Result { + match serde_json::from_str::(raw) + .map_err(|_| HtmlPreviewError::unavailable("Invalid peer HTML preview response"))? + { + RemoteResponse::HostInvokeResult { + ok: true, + value: Some(value), + .. + } => Ok(value), + RemoteResponse::HostInvokeResult { error, .. } => Err(HtmlPreviewError::unavailable( + error.unwrap_or_else(|| "Peer HTML preview failed".into()), + )), + RemoteResponse::Error { message } => Err(HtmlPreviewError::unavailable(message)), + _ => Err(HtmlPreviewError::unavailable( + "Invalid peer HTML preview response", + )), + } +} + +#[async_trait::async_trait] +impl PreviewReader for PeerPreviewReader { + fn is_current(&self) -> bool { + account_context_is_current(self.account_generation) + } + async fn read( + &self, + request: &HtmlPreviewResourceRequest, + ) -> Result { + let value = self + .invoke( + "html_preview_read_resource", + serde_json::json!({ "request": request }), + ) + .await?; + serde_json::from_value::(value) + .map_err(|_| HtmlPreviewError::unavailable("Invalid peer HTML preview resource"))? + .into_result() + } } #[derive(Clone)] struct PreviewState { token: String, - root: PreviewRoot, + cookie_name: String, entry_path: String, - remote_file_service: Arc>>, -} - -struct PreviewSession { - shutdown: Option>, + request: HtmlPreviewResourceRequest, + reader: Arc, + closed: watch::Receiver, } static SESSIONS: std::sync::OnceLock< - std::sync::Mutex>, + std::sync::Mutex>>, > = std::sync::OnceLock::new(); - -fn sessions() -> &'static std::sync::Mutex> { - SESSIONS.get_or_init(|| std::sync::Mutex::new(std::collections::HashMap::new())) +fn sessions() -> &'static std::sync::Mutex>> { + SESSIONS.get_or_init(Default::default) } #[tauri::command] @@ -77,116 +193,115 @@ pub async fn html_preview_create( app_state: tauri::State<'_, AppState>, request: HtmlPreviewCreateRequest, ) -> Result { - if request.peer_device_mode { - return Err("HTML preview is not supported while controlling a peer device".to_string()); - } - let record = match request.workspace_id.as_deref() { - Some(id) => app_state - .workspace_service - .require_workspace(id) - .await - .map_err(|e| e.to_string())?, - None => app_state - .workspace_service - .resolve_legacy_workspace_reference( - None, - &request.workspace_path, - request.remote_connection_id.as_deref(), - None, - ) + let is_peer = request.peer_device_mode + || request.peer_device_id.is_some() + || super::peer_host_invoke::is_peer_controller_active(); + let reader: Arc; + let workspace_id; + if is_peer { + let device_id = request + .peer_device_id + .filter(|id| !id.trim().is_empty()) + .ok_or("HTML preview requires the selected peer device identity; reopen the file")?; + workspace_id = request + .workspace_id + .filter(|id| !id.trim().is_empty()) + .ok_or("HTML preview workspace ID is required")?; + let peer = PeerPreviewReader { + device_id, + account_generation: account_context_generation(), + }; + peer.ensure_supported() .await - .map_err(|e| e.to_string())? - .ok_or("HTML preview workspace is unavailable")?, - }; - let workspace = record.root_path.to_string_lossy(); - let connection = record.filesystem_connection_id()?; - let target = resolve_desktop_path_target( - &app_state, - &request.file_path, - Some(connection.unwrap_or("")), - Some(&record.id), - ) - .await?; - let (root, entry_relative_path) = match target { - DesktopPathTarget::Local { resolved_path, .. } => { - let workspace_root = std::fs::canonicalize(workspace.as_ref()) - .map_err(|e| format!("Failed to resolve workspace root: {e}"))?; - let entry = std::fs::canonicalize(&resolved_path) - .map_err(|e| format!("Failed to resolve HTML file: {e}"))?; - if !entry.starts_with(&workspace_root) || !entry.is_file() { - return Err("HTML file is outside the workspace or is not a file".to_string()); - } - let relative = entry - .strip_prefix(&workspace_root) - .map_err(|_| "HTML file is outside the workspace".to_string())? - .to_string_lossy() - .replace('\\', "/"); - ( - PreviewRoot::Local { - root: workspace_root, - }, - relative, - ) - } - DesktopPathTarget::Remote { entry, .. } => { - let remote_root = normalize_remote_path(&workspace); - let file_path = normalize_remote_path(&request.file_path); - if !is_under_remote_root(&file_path, &remote_root) { - return Err("HTML file is outside the remote workspace".to_string()); + .map_err(|error| error.message)?; + reader = Arc::new(peer); + } else { + workspace_id = match request.workspace_id { + Some(id) => id, + None => { + app_state + .workspace_service + .resolve_legacy_workspace_reference( + None, + &request.workspace_path, + request.remote_connection_id.as_deref(), + None, + ) + .await + .map_err(|error| error.to_string())? + .ok_or("HTML preview workspace is unavailable")? + .id } - let remote_fs = app_state - .get_remote_file_service_async() + }; + reader = Arc::new( + resolve_html_preview_source(&workspace_id, &request.file_path) .await - .map_err(|e| format!("Remote file service not available: {e}"))?; - ensure_remote_file(&remote_fs, &entry.connection_id, &file_path).await?; - let relative = file_path - .strip_prefix(remote_root.trim_end_matches('/')) - .unwrap_or(&file_path) - .trim_start_matches('/') - .to_string(); - ( - PreviewRoot::Remote { - root: remote_root, - connection_id: entry.connection_id, - }, - relative, - ) - } - }; + .map_err(|error| error.message)?, + ); + } + start_preview( + reader, + HtmlPreviewResourceRequest { + workspace_id, + file_path: request.file_path, + resource_path: None, + metadata_only: true, + offset: 0, + expected_source: None, + expected_revision: None, + }, + ) + .await +} + +/// Runs on the workspace owner. Desktop and CLI use the same assembly/service path. +#[tauri::command] +pub async fn html_preview_read_resource( + request: HtmlPreviewResourceRequest, +) -> HtmlPreviewResourceReply { + openbitfun_core::service::filesystem::html_preview::html_preview_read_resource(request).await +} +async fn start_preview( + reader: Arc, + mut request: HtmlPreviewResourceRequest, +) -> Result { + let entry = reader.read(&request).await.map_err(|error| error.message)?; + validate_chunk(&entry, &request).map_err(|error| error.message)?; + request.expected_source = Some(entry.source); let token = Uuid::new_v4().simple().to_string(); let session_id = Uuid::new_v4().simple().to_string(); + let (shutdown, closed) = watch::channel(false); let state = PreviewState { token: token.clone(), - root, - entry_path: entry_relative_path.clone(), - remote_file_service: Arc::clone(&app_state.remote_file_service), + cookie_name: format!("openbitfun_html_preview_{session_id}"), + entry_path: entry.entry_path, + request, + reader, + closed: closed.clone(), }; let listener = TcpListener::bind(("127.0.0.1", 0)) .await - .map_err(|e| format!("Failed to bind HTML preview server: {e}"))?; - let port = listener.local_addr().map_err(|e| e.to_string())?.port(); - let (shutdown_tx, shutdown_rx) = oneshot::channel(); + .map_err(|error| format!("Failed to bind HTML preview server: {error}"))?; + let port = listener + .local_addr() + .map_err(|error| error.to_string())? + .port(); let router = Router::new() .fallback(any(preview_handler)) .with_state(state); + sessions() + .lock() + .map_err(|_| "HTML preview registry poisoned")? + .insert(session_id.clone(), shutdown); tokio::spawn(async move { + let mut closed = closed; let _ = axum::serve(listener, router) - .with_graceful_shutdown(async { - let _ = shutdown_rx.await; + .with_graceful_shutdown(async move { + let _ = closed.wait_for(|closed| *closed).await; }) .await; }); - sessions() - .lock() - .map_err(|_| "HTML preview registry poisoned".to_string())? - .insert( - session_id.clone(), - PreviewSession { - shutdown: Some(shutdown_tx), - }, - ); - Ok(HtmlPreviewCreateResponse { url: format!("http://127.0.0.1:{port}/__openbitfun_preview_bootstrap?previewToken={token}"), session_id, @@ -195,15 +310,75 @@ pub async fn html_preview_create( #[tauri::command] pub async fn html_preview_release(request: HtmlPreviewReleaseRequest) -> Result<(), String> { - if let Some(mut session) = sessions() + if let Some(shutdown) = sessions() .lock() - .map_err(|_| "HTML preview registry poisoned".to_string())? + .map_err(|_| "HTML preview registry poisoned")? .remove(&request.session_id) { - if let Some(shutdown) = session.shutdown.take() { - let _ = shutdown.send(()); + let _ = shutdown.send(true); + } + Ok(()) +} + +impl PreviewState { + async fn read( + &self, + request: &HtmlPreviewResourceRequest, + ) -> Result { + let mut closed = self.closed.clone(); + if *closed.borrow() || !self.reader.is_current() { + return Err(HtmlPreviewError::unavailable( + "HTML preview is no longer available", + )); + } + tokio::select! { + _ = closed.wait_for(|closed| *closed) => Err(HtmlPreviewError::unavailable("HTML preview was closed")), + result = self.reader.read(request) => { + let resource = result?; + if *self.closed.borrow() || !self.reader.is_current() { return Err(HtmlPreviewError::unavailable("HTML preview is no longer available")); } + validate_chunk(&resource, request)?; + Ok(resource) + } } } +} + +fn validate_chunk( + file: &HtmlPreviewResource, + request: &HtmlPreviewResourceRequest, +) -> Result<(), HtmlPreviewError> { + if file.size > MAX_PREVIEW_BYTES + || file.offset != request.offset + || file.offset > file.size + || file.source.is_empty() + || file.revision.is_empty() + || request + .expected_source + .as_ref() + .is_some_and(|source| source != &file.source) + || request + .expected_revision + .as_ref() + .is_some_and(|revision| revision != &file.revision) + || file.content_base64.len() > PREVIEW_CHUNK_BYTES.div_ceil(3) * 4 + { + return Err(HtmlPreviewError::unavailable( + "Invalid HTML preview resource chunk", + )); + } + let bytes = BASE64 + .decode(&file.content_base64) + .map_err(|_| HtmlPreviewError::unavailable("Invalid HTML preview resource encoding"))?; + let expected = if request.metadata_only { + 0 + } else { + (file.size - file.offset).min(PREVIEW_CHUNK_BYTES as u64) as usize + }; + if bytes.len() != expected { + return Err(HtmlPreviewError::unavailable( + "Incomplete HTML preview resource chunk", + )); + } Ok(()) } @@ -223,321 +398,129 @@ async fn preview_handler( .get(header::COOKIE) .and_then(|value| value.to_str().ok()) .is_some_and(|cookies| { - cookies.split(';').any(|cookie| { - cookie.trim().strip_prefix("openbitfun_html_preview=") == Some(state.token.as_str()) - }) + cookies + .split(';') + .filter_map(|cookie| cookie.trim().split_once('=')) + .any(|(key, value)| key == state.cookie_name && value == state.token) }); - let bootstrap_authorized = query_token == Some(state.token.as_str()); - if (!bootstrap_authorized && !cookie_authorized) + if *state.closed.borrow() + || !state.reader.is_current() || (method != Method::GET && method != Method::HEAD) { - return response( - StatusCode::NOT_FOUND, - "Not found", - "text/plain; charset=utf-8", - ); + return response(StatusCode::NOT_FOUND, "Not found"); } - if uri.path() == "/__openbitfun_preview_bootstrap" { - let location = format!("/{}", encode_url_path(&state.entry_path)); - let cookie = format!( - "openbitfun_html_preview={}; HttpOnly; SameSite=Strict; Path=/", - state.token - ); + if uri.path() == "/__openbitfun_preview_bootstrap" && query_token == Some(state.token.as_str()) + { return Response::builder() .status(StatusCode::FOUND) - .header(header::LOCATION, location) - .header(header::SET_COOKIE, cookie) + .header( + header::LOCATION, + format!("/{}", encode_url_path(&state.entry_path)), + ) + .header( + header::SET_COOKIE, + format!( + "{}={}; HttpOnly; SameSite=Strict; Path=/", + state.cookie_name, state.token + ), + ) .header(header::CACHE_CONTROL, "no-store") .header(header::REFERRER_POLICY, "no-referrer") .body(Body::empty()) - .unwrap_or_else(|_| Response::new(Body::empty())); + .unwrap_or_else(|_| { + response(StatusCode::INTERNAL_SERVER_ERROR, "Invalid preview entry") + }); + } + if !cookie_authorized { + return response(StatusCode::NOT_FOUND, "Not found"); } let relative = match urlencoding::decode(uri.path().trim_start_matches('/')) { - Ok(relative) => relative, - Err(_) => { - return response( - StatusCode::BAD_REQUEST, - "Invalid preview path", - "text/plain; charset=utf-8", - ) - } + Ok(relative) => relative.into_owned(), + Err(_) => return response(StatusCode::BAD_REQUEST, "Invalid preview path"), }; - let result = match &state.root { - PreviewRoot::Local { root } => read_local(root, &relative).await, - PreviewRoot::Remote { - root, - connection_id, - } => read_remote(&state, root, connection_id, &relative).await, + let mut request = state.request.clone(); + request.resource_path = Some(relative); + request.metadata_only = method == Method::HEAD; + let first = match state.read(&request).await { + Ok(file) => file, + Err(error) => return preview_error_response(error), }; - match result { - Ok((bytes, content_type)) => { - let mut builder = Response::builder() - .status(StatusCode::OK) - .header(header::CONTENT_TYPE, content_type) - .header(header::CACHE_CONTROL, "no-store") - .header(header::REFERRER_POLICY, "no-referrer") - .header(header::X_CONTENT_TYPE_OPTIONS, "nosniff"); - if bootstrap_authorized { - let cookie = format!( - "openbitfun_html_preview={}; HttpOnly; SameSite=Strict; Path=/", - state.token - ); - if let Ok(value) = HeaderValue::from_str(&cookie) { - builder = builder.header(header::SET_COOKIE, value); - } - } - if method == Method::HEAD { - builder = builder.header(header::CONTENT_LENGTH, bytes.len().to_string()); - builder.body(Body::empty()).unwrap_or_else(|_| { - response( - StatusCode::INTERNAL_SERVER_ERROR, - "Internal error", - "text/plain", - ) - }) - } else { - builder.body(Body::from(bytes)).unwrap_or_else(|_| { - response( - StatusCode::INTERNAL_SERVER_ERROR, - "Internal error", - "text/plain", - ) - }) - } - } - Err((status, message)) => response(status, &message, "text/plain; charset=utf-8"), - } -} - -async fn read_local( - root: &FsPath, - relative: &str, -) -> Result<(Vec, String), (StatusCode, String)> { - let requested = confined_local_path(root, relative)?; - let path = tokio::fs::canonicalize(&requested) - .await - .map_err(|_| (StatusCode::NOT_FOUND, "Not found".to_string()))?; - if !path.starts_with(root) { - return Err((StatusCode::NOT_FOUND, "Not found".to_string())); - } - let metadata = tokio::fs::symlink_metadata(&path) - .await - .map_err(|_| (StatusCode::NOT_FOUND, "Not found".to_string()))?; - if metadata.file_type().is_symlink() || !metadata.is_file() { - return Err((StatusCode::NOT_FOUND, "Not found".to_string())); - } - if metadata.len() > MAX_PREVIEW_BYTES { - return Err(( - StatusCode::PAYLOAD_TOO_LARGE, - "Preview file is too large".to_string(), - )); - } - let bytes = tokio::fs::read(&path).await.map_err(|_| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - "Failed to read preview file".to_string(), - ) - })?; - Ok((bytes, mime_for_path(relative))) -} - -async fn read_remote( - state: &PreviewState, - root: &str, - connection_id: &str, - relative: &str, -) -> Result<(Vec, String), (StatusCode, String)> { - let path = confined_remote_path(root, relative)?; - let service = state.remote_file_service.read().await.clone().ok_or(( - StatusCode::SERVICE_UNAVAILABLE, - "Remote file service unavailable".to_string(), - ))?; - let metadata = ensure_confined_remote_file(&service, connection_id, root, relative) - .await - .map_err(|e| (StatusCode::NOT_FOUND, e))?; - if metadata > MAX_PREVIEW_BYTES { - return Err(( - StatusCode::PAYLOAD_TOO_LARGE, - "Preview file is too large".to_string(), - )); - } - let bytes = service.read_file(connection_id, &path).await.map_err(|e| { - ( - StatusCode::BAD_GATEWAY, - format!("Failed to read remote preview file: {e}"), - ) - })?; - Ok((bytes, mime_for_path(relative))) -} - -async fn ensure_remote_file( - service: &RemoteFileService, - connection_id: &str, - path: &str, -) -> Result { - let metadata = service - .workspace_metadata(connection_id, path, false) - .await - .map_err(|e| e.to_string())? - .ok_or_else(|| "Not found".to_string())?; - if metadata.kind != openbitfun_runtime_ports::WorkspacePathKind::File { - return Err("Not found".to_string()); - } - Ok(metadata.size.unwrap_or(0)) -} - -async fn ensure_confined_remote_file( - service: &RemoteFileService, - connection_id: &str, - root: &str, - relative: &str, -) -> Result { - let mut current = root.trim_end_matches('/').to_string(); - let components: Vec<&str> = relative - .split('/') - .filter(|part| !part.is_empty()) - .collect(); - for (index, component) in components.iter().enumerate() { - current = if current.is_empty() || current == "/" { - format!("/{component}") - } else { - format!("{current}/{component}") - }; - let metadata = service - .workspace_metadata(connection_id, ¤t, false) - .await - .map_err(|e| e.to_string())? - .ok_or_else(|| "Not found".to_string())?; - if metadata.kind == openbitfun_runtime_ports::WorkspacePathKind::Symlink { - return Err("Symbolic links are not available in HTML preview".to_string()); - } - let is_last = index + 1 == components.len(); - if is_last { - if metadata.kind != openbitfun_runtime_ports::WorkspacePathKind::File { - return Err("Not found".to_string()); - } - return Ok(metadata.size.unwrap_or(0)); - } - if metadata.kind != openbitfun_runtime_ports::WorkspacePathKind::Directory { - return Err("Not found".to_string()); - } - } - Err("Not found".to_string()) -} - -fn confined_local_path(root: &FsPath, relative: &str) -> Result { - let mut path = root.to_path_buf(); - for component in relative.split('/') { - if component.is_empty() || component == "." { - continue; - } - if component == ".." || component.contains('\\') { - return Err((StatusCode::BAD_REQUEST, "Invalid preview path".to_string())); - } - path.push(component); - } - Ok(path) -} - -fn normalize_remote_path(path: &str) -> String { - let mut result = String::from("/"); - for component in path.split('/') { - if component.is_empty() || component == "." { - continue; - } - if component == ".." { - let parent = result - .rsplit_once('/') - .map(|(prefix, _)| prefix.to_string()); - result = parent - .filter(|prefix| !prefix.is_empty()) - .unwrap_or_else(|| "/".to_string()); - } else { - if !result.ends_with('/') { - result.push('/'); - } - result.push_str(component); - } - } - if result.len() > 1 { - result.trim_end_matches('/').to_string() + let size = first.size; + let content_type = first.mime_type.clone(); + request.expected_revision = Some(first.revision.clone()); + let body = if method == Method::HEAD { + Body::empty() } else { - result - } -} - -fn is_under_remote_root(path: &str, root: &str) -> bool { - path == root || (root == "/" && path.starts_with('/')) || path.starts_with(&format!("{root}/")) -} - -fn confined_remote_path(root: &str, relative: &str) -> Result { - if relative - .split('/') - .any(|part| part == ".." || part.contains('\\')) - { - return Err((StatusCode::BAD_REQUEST, "Invalid preview path".to_string())); - } - let path = normalize_remote_path(&format!("{root}/{relative}")); - if !is_under_remote_root(&path, root) { - return Err((StatusCode::NOT_FOUND, "Not found".to_string())); - } - Ok(path) + let stream = futures::stream::try_unfold( + (state, request, Some(first)), + move |(state, mut request, first)| async move { + let file = match first { + Some(file) => file, + None => state + .read(&request) + .await + .map_err(|error| std::io::Error::other(error.message))?, + }; + if file.offset == file.size { + return Ok::<_, std::io::Error>(None); + } + let bytes = BASE64 + .decode(file.content_base64) + .map_err(std::io::Error::other)?; + request.offset += bytes.len() as u64; + // An empty sentinel ends the stream without another RPC at EOF. + let next = (request.offset == size).then(|| HtmlPreviewResource { + offset: size, + content_base64: String::new(), + ..file + }); + Ok(Some((bytes, (state, request, next)))) + }, + ); + Body::from_stream(stream) + }; + Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, content_type) + .header(header::CONTENT_LENGTH, size) + .header(header::CACHE_CONTROL, "no-store") + .header(header::REFERRER_POLICY, "no-referrer") + .header(header::X_CONTENT_TYPE_OPTIONS, "nosniff") + .body(body) + .unwrap_or_else(|_| { + response( + StatusCode::INTERNAL_SERVER_ERROR, + "Invalid preview response", + ) + }) } -fn mime_for_path(path: &str) -> String { - let mime = mime_guess::from_path(path).first_or_octet_stream(); - if mime.type_() == mime_guess::mime::TEXT { - format!("{}; charset=utf-8", mime.essence_str()) - } else { - mime.to_string() - } +fn preview_error_response(error: HtmlPreviewError) -> Response { + let status = match error.code { + Code::InvalidPath => StatusCode::BAD_REQUEST, + Code::NotFound => StatusCode::NOT_FOUND, + Code::Forbidden => StatusCode::FORBIDDEN, + Code::TooLarge => StatusCode::PAYLOAD_TOO_LARGE, + Code::Changed => StatusCode::CONFLICT, + Code::Unavailable => StatusCode::BAD_GATEWAY, + }; + response(status, &error.message) } - fn encode_url_path(path: &str) -> String { path.split('/') .map(|component| urlencoding::encode(component).into_owned()) .collect::>() .join("/") } - -fn response(status: StatusCode, body: &str, content_type: &'static str) -> Response { +fn response(status: StatusCode, body: &str) -> Response { Response::builder() .status(status) - .header(header::CONTENT_TYPE, content_type) + .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") + .header(header::CACHE_CONTROL, "no-store") + .header(header::REFERRER_POLICY, "no-referrer") .body(Body::from(body.to_string())) .unwrap_or_else(|_| Response::new(Body::empty())) } #[cfg(test)] -mod tests { - use super::*; - #[test] - fn current_and_100_preview_requests_have_distinct_identity_inputs() { - let current: HtmlPreviewCreateRequest = serde_json::from_value(serde_json::json!({ - "workspaceId": "remote-id", "filePath": "/repo/index.html" - })) - .unwrap(); - assert_eq!(current.workspace_id.as_deref(), Some("remote-id")); - assert!(current.workspace_path.is_empty()); - assert!(current.remote_connection_id.is_none()); - let legacy: HtmlPreviewCreateRequest = serde_json::from_value(serde_json::json!({ - "workspacePath": "/repo", "filePath": "/repo/index.html", "remoteConnectionId": "ssh-1" - })) - .unwrap(); - assert!(legacy.workspace_id.is_none()); - assert_eq!(legacy.workspace_path, "/repo"); - assert_eq!(legacy.remote_connection_id.as_deref(), Some("ssh-1")); - } - #[test] - fn rejects_traversal() { - assert!(confined_remote_path("/workspace", "../secret").is_err()); - assert!(confined_local_path(FsPath::new("/workspace"), "../secret").is_err()); - } - #[test] - fn preserves_relative_resources() { - assert_eq!( - confined_remote_path("/workspace", "assets/app.css").unwrap(), - "/workspace/assets/app.css" - ); - } -} +pub(super) mod tests; diff --git a/src/apps/desktop/src/api/html_preview_api/tests.rs b/src/apps/desktop/src/api/html_preview_api/tests.rs new file mode 100644 index 0000000000..e02e538d46 --- /dev/null +++ b/src/apps/desktop/src/api/html_preview_api/tests.rs @@ -0,0 +1,394 @@ +use super::*; +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Mutex, +}; + +struct Reader { + bytes: Vec, + requests: Mutex>, + unavailable: AtomicBool, + blocked: AtomicBool, + reading: tokio::sync::Notify, + unblock: tokio::sync::Notify, +} + +#[async_trait::async_trait] +impl PreviewReader for Reader { + async fn read( + &self, + request: &HtmlPreviewResourceRequest, + ) -> Result { + self.requests.lock().unwrap().push(request.clone()); + if self.blocked.load(Ordering::SeqCst) { + self.reading.notify_one(); + self.unblock.notified().await; + } + if self.unavailable.load(Ordering::SeqCst) { + return Err(HtmlPreviewError::unavailable("Peer is offline")); + } + if request.resource_path.as_deref() == Some("missing.js") { + return Err(HtmlPreviewError::new(Code::NotFound, "Missing resource")); + } + let count = if request.metadata_only { + 0 + } else { + (self.bytes.len() - request.offset as usize).min(PREVIEW_CHUNK_BYTES) + }; + Ok(HtmlPreviewResource { + entry_path: "site/dist/index #1.html".into(), + resource_path: request + .resource_path + .clone() + .unwrap_or_else(|| "site/dist/index #1.html".into()), + source: "workspace-provider".into(), + revision: "revision-1".into(), + mime_type: "text/html; charset=utf-8".into(), + size: self.bytes.len() as u64, + offset: request.offset, + content_base64: BASE64 + .encode(&self.bytes[request.offset as usize..request.offset as usize + count]), + }) + } +} + +fn resource_request() -> HtmlPreviewResourceRequest { + HtmlPreviewResourceRequest { + workspace_id: "workspace".into(), + file_path: "/repo/site/dist/index #1.html".into(), + resource_path: None, + metadata_only: true, + offset: 0, + expected_source: None, + expected_revision: None, + } +} + +async fn fixture(bytes: Vec) -> (HtmlPreviewCreateResponse, Arc, reqwest::Client) { + crate::ensure_rustls_crypto_provider(); + let reader = Arc::new(Reader { + bytes, + requests: Mutex::new(vec![]), + unavailable: AtomicBool::new(false), + blocked: AtomicBool::new(false), + reading: Default::default(), + unblock: Default::default(), + }); + let preview = start_preview(reader.clone(), resource_request()) + .await + .unwrap(); + let client = reqwest::Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap(); + (preview, reader, client) +} + +async fn bootstrap( + client: &reqwest::Client, + preview: &HtmlPreviewCreateResponse, +) -> (String, String) { + let reply = client.get(&preview.url).send().await.unwrap(); + assert_eq!(reply.status(), StatusCode::FOUND); + let cookie = reply.headers()[header::SET_COOKIE] + .to_str() + .unwrap() + .split(';') + .next() + .unwrap() + .to_string(); + let location = reply.headers()[header::LOCATION].to_str().unwrap(); + assert_eq!(location, "/site/dist/index%20%231.html"); + let entry = reqwest::Url::parse(&preview.url) + .unwrap() + .join(location) + .unwrap() + .to_string(); + (cookie, entry) +} + +async fn release(preview: HtmlPreviewCreateResponse) { + html_preview_release(HtmlPreviewReleaseRequest { + session_id: preview.session_id, + }) + .await + .unwrap(); +} + +#[tokio::test] +async fn http_bootstrap_streams_bounded_chunks_and_preserves_head() { + let bytes = vec![71; PREVIEW_CHUNK_BYTES * 2 + 11]; + let (preview, reader, client) = fixture(bytes.clone()).await; + let (cookie, entry) = bootstrap(&client, &preview).await; + assert_eq!( + client.get(&entry).send().await.unwrap().status(), + StatusCode::NOT_FOUND + ); + let reply = client + .get(&entry) + .header(header::COOKIE, &cookie) + .send() + .await + .unwrap(); + assert_eq!(reply.status(), StatusCode::OK); + assert_eq!( + reply.headers()[header::CONTENT_LENGTH], + bytes.len().to_string() + ); + assert_eq!(reply.headers()[header::X_CONTENT_TYPE_OPTIONS], "nosniff"); + assert_eq!(reply.bytes().await.unwrap().as_ref(), &bytes); + let requests = reader.requests.lock().unwrap().clone(); + assert_eq!( + requests.len(), + 4, + "bootstrap metadata plus three bounded content reads" + ); + assert_eq!( + requests[1].resource_path.as_deref(), + Some("site/dist/index #1.html") + ); + assert_eq!(requests[2].offset, PREVIEW_CHUNK_BYTES as u64); + assert_eq!(requests[3].expected_revision.as_deref(), Some("revision-1")); + assert_eq!( + requests[3].expected_source.as_deref(), + Some("workspace-provider") + ); + let reply = client + .head(&entry) + .header(header::COOKIE, &cookie) + .send() + .await + .unwrap(); + assert_eq!( + reply.headers()[header::CONTENT_LENGTH], + bytes.len().to_string() + ); + assert!(reply.bytes().await.unwrap().is_empty()); + assert!( + reader + .requests + .lock() + .unwrap() + .last() + .unwrap() + .metadata_only + ); + assert_eq!( + client + .post(&entry) + .header(header::COOKIE, &cookie) + .send() + .await + .unwrap() + .status(), + StatusCode::NOT_FOUND + ); + release(preview).await; +} + +#[tokio::test] +async fn concurrent_preview_cookies_do_not_overwrite_or_authorize_each_other() { + let (first, _, client) = fixture(b"first".to_vec()).await; + let (second, _, _) = fixture(b"second".to_vec()).await; + let (first_cookie, first_url) = bootstrap(&client, &first).await; + let (second_cookie, second_url) = bootstrap(&client, &second).await; + assert_ne!( + first_cookie.split('=').next(), + second_cookie.split('=').next() + ); + assert_eq!( + client + .get(&first_url) + .header(header::COOKIE, &second_cookie) + .send() + .await + .unwrap() + .status(), + StatusCode::NOT_FOUND + ); + // Browsers share cookies across ports on 127.0.0.1; both cookies will be sent. + let cookies = format!("{first_cookie}; {second_cookie}"); + assert_eq!( + client + .get(&first_url) + .header(header::COOKIE, &cookies) + .send() + .await + .unwrap() + .text() + .await + .unwrap(), + "first" + ); + assert_eq!( + client + .get(&second_url) + .header(header::COOKIE, &cookies) + .send() + .await + .unwrap() + .text() + .await + .unwrap(), + "second" + ); + release(first).await; + release(second).await; +} + +#[tokio::test] +async fn gateway_reports_missing_and_offline_resources_and_recovers_on_reload() { + let (preview, reader, client) = fixture(b"remote-content".to_vec()).await; + let (cookie, entry) = bootstrap(&client, &preview).await; + let missing = reqwest::Url::parse(&entry) + .unwrap() + .join("/missing.js") + .unwrap(); + assert_eq!( + client + .get(missing) + .header(header::COOKIE, &cookie) + .send() + .await + .unwrap() + .status(), + StatusCode::NOT_FOUND + ); + reader.unavailable.store(true, Ordering::SeqCst); + let reply = client + .get(&entry) + .header(header::COOKIE, &cookie) + .send() + .await + .unwrap(); + assert_eq!(reply.status(), StatusCode::BAD_GATEWAY); + assert!(reply.text().await.unwrap().contains("Peer is offline")); + reader.unavailable.store(false, Ordering::SeqCst); + assert_eq!( + client + .get(&entry) + .header(header::COOKIE, &cookie) + .send() + .await + .unwrap() + .text() + .await + .unwrap(), + "remote-content" + ); + release(preview).await; +} + +#[tokio::test] +async fn closing_preview_cancels_in_flight_resource_read() { + let (preview, reader, client) = fixture(b"remote-content".to_vec()).await; + let (cookie, entry) = bootstrap(&client, &preview).await; + reader.blocked.store(true, Ordering::SeqCst); + let request = tokio::spawn(async move { + client + .get(entry) + .header(header::COOKIE, cookie) + .send() + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(2), reader.reading.notified()) + .await + .unwrap(); + release(preview).await; + let reply = tokio::time::timeout(std::time::Duration::from_secs(2), request) + .await + .unwrap() + .unwrap() + .unwrap(); + assert_eq!(reply.status(), StatusCode::BAD_GATEWAY); +} + +#[test] +fn current_and_legacy_preview_requests_round_trip_without_guessing_peer_identity() { + for payload in [ + serde_json::json!({"workspaceId":"remote-id", "filePath":"/repo/index.html"}), + serde_json::json!({"workspacePath":"/repo", "filePath":"/repo/index.html", "remoteConnectionId":"ssh-1"}), + serde_json::json!({"workspaceId":"remote-id", "filePath":"/repo/index.html", "peerDeviceMode":true}), + ] { + let request: HtmlPreviewCreateRequest = serde_json::from_value(payload.clone()).unwrap(); + assert!(request.peer_device_id.is_none()); + let encoded = serde_json::to_value(request).unwrap(); + for (key, value) in payload.as_object().unwrap() { + assert_eq!(&encoded[key], value); + } + let decoded: HtmlPreviewCreateRequest = serde_json::from_value(encoded).unwrap(); + assert_eq!(decoded.file_path, "/repo/index.html"); + } +} + +#[test] +fn peer_capability_is_required_even_for_desktop_and_wire_failures_are_explicit() { + for ping in [ + serde_json::json!({}), + serde_json::json!({"host_type":"desktop","capabilities":{}}), + serde_json::json!({"capabilities":{"workspace_html_preview_v1":false}}), + ] { + assert!(require_preview_capability(&ping) + .unwrap_err() + .message + .contains("update OpenBitFun")); + } + for host in ["desktop", "cli"] { + assert!(require_preview_capability( + &serde_json::json!({"host_type":host,"capabilities":{"workspace_html_preview_v1":true}}) + ) + .is_ok()); + } + assert!( + decode_peer_result(r#"{"resp":"host_invoke_result","ok":false,"error":"offline"}"#) + .is_err() + ); + assert!(decode_peer_result(r#"{"resp":"error","message":"unsupported"}"#).is_err()); + assert!(decode_peer_result(r#"{"resp":"host_invoke_result","ok":true}"#).is_err()); + assert!(decode_peer_result("invalid").is_err()); +} + +// Called from the relay fixture while it owns the isolated account context. +pub(crate) async fn verify_encrypted_peer_gateway( + device_id: &str, + request: HtmlPreviewResourceRequest, + expected: &[u8], +) { + let reader = Arc::new(PeerPreviewReader { + device_id: device_id.into(), + account_generation: account_context_generation(), + }); + reader.ensure_supported().await.unwrap(); + let preview = start_preview(reader, request).await.unwrap(); + let client = reqwest::Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap(); + let reply = client.get(&preview.url).send().await.unwrap(); + assert_eq!(reply.status(), StatusCode::FOUND); + let cookie = reply.headers()[header::SET_COOKIE] + .to_str() + .unwrap() + .split(';') + .next() + .unwrap(); + let asset = reqwest::Url::parse(&preview.url) + .unwrap() + .join("/assets/large.js") + .unwrap(); + let reply = client + .get(asset) + .header(header::COOKIE, cookie) + .send() + .await + .unwrap(); + assert_eq!(reply.status(), StatusCode::OK); + assert_eq!( + reply.headers()[header::CONTENT_TYPE], + "text/javascript; charset=utf-8" + ); + assert_eq!(reply.bytes().await.unwrap().as_ref(), expected); + release(preview).await; +} diff --git a/src/apps/desktop/src/api/remote_connect_api/relay_routes_tests.rs b/src/apps/desktop/src/api/remote_connect_api/relay_routes_tests.rs index 921ba39bff..d1d2508e59 100644 --- a/src/apps/desktop/src/api/remote_connect_api/relay_routes_tests.rs +++ b/src/apps/desktop/src/api/remote_connect_api/relay_routes_tests.rs @@ -463,3 +463,149 @@ async fn dual_relay_rpc_persistence_and_teardown_are_independent() { .is_none()); official.stop().await; } + +#[tokio::test] +async fn html_preview_streams_assets_over_encrypted_peer_rpc() { + use openbitfun_product_domains::html_preview::{ + HtmlPreviewResourceReply, HtmlPreviewResourceRequest, PREVIEW_CHUNK_BYTES, + }; + use openbitfun_services_integrations::remote_connect::{ + file_projection::SessionFileTarget, + html_preview::HtmlPreviewSource, + relay_client::{RelayClient, RelayEvent}, + RemoteCommand, RemoteResponse, + }; + let _guard = sync_state_tests::ACCOUNT_CONTEXT_TEST_LOCK.lock().await; + let host = FixtureHost::new("/preview-test").await; + let controller = host.credential("preview-controller", [11; 32]).await; + let target = host.credential("preview-peer", [12; 32]).await; + host.start(host.port, None).await.unwrap(); + let previous_context = get_account_context() + .write() + .await + .replace(AccountContextState { + session: controller, + relay_url: host.url(), + }); + let directory = tempfile::tempdir().unwrap(); + let root = directory.path(); + std::fs::create_dir_all(root.join("site/dist/assets")).unwrap(); + std::fs::write( + root.join("site/dist/index.html"), + "", + ) + .unwrap(); + let bytes = vec![b'x'; PREVIEW_CHUNK_BYTES * 2 + 19]; + std::fs::write(root.join("site/dist/assets/large.js"), &bytes).unwrap(); + let entry = root + .join("site/dist/index.html") + .to_string_lossy() + .into_owned(); + let source = HtmlPreviewSource::new( + SessionFileTarget { + fs: Arc::new(openbitfun_services_core::workspace::LocalWorkspaceFs), + root: root.to_string_lossy().into_owned(), + path: entry.clone(), + remote: false, + }, + "fixture-workspace", + ) + .await + .unwrap(); + let (client, mut events) = RelayClient::new(); + let client = Arc::new(client); + client.connect(&host.url()).await.unwrap(); + client + .connect_authenticated(&target.token, "preview-peer") + .await + .unwrap(); + let serving_client = client.clone(); + let relay_url = host.url(); + let requests = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let serving_requests = requests.clone(); + let serving = tokio::spawn(async move { + while let Some(event) = events.recv().await { + let RelayEvent::DeviceMessageReceived { + source_device_id, + correlation_id, + encrypted_data, + nonce, + } = event + else { + continue; + }; + if correlation_id.is_empty() { + continue; + } + let plaintext = target + .decrypt_from_peer(&relay_url, &source_device_id, &encrypted_data, &nonce) + .await + .unwrap(); + let RemoteCommand::HostInvoke { command, args } = + serde_json::from_str(&plaintext).unwrap() + else { + panic!("unexpected command") + }; + let value = match command.as_str() { + "peer_mode_ping" => { + serde_json::json!({"host_type":"cli", "capabilities":{"workspace_html_preview_v1":true}}) + } + "html_preview_read_resource" => { + let request: HtmlPreviewResourceRequest = + serde_json::from_value(args["request"].clone()).unwrap(); + assert_eq!(request.workspace_id, "fixture-workspace"); + serving_requests.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + serde_json::to_value(HtmlPreviewResourceReply::from( + source.read(&request).await, + )) + .unwrap() + } + _ => panic!("unexpected host invoke {command}"), + }; + let response = RemoteResponse::HostInvokeResult { + ok: true, + value: Some(value), + error: None, + }; + let (data, nonce) = target + .encrypt_for_peer( + &relay_url, + &source_device_id, + &serde_json::to_string(&response).unwrap(), + ) + .await + .unwrap(); + serving_client + .send_device_message(&source_device_id, &correlation_id, &data, &nonce) + .await + .unwrap(); + } + }); + tokio::time::timeout( + std::time::Duration::from_secs(20), + crate::api::html_preview_api::tests::verify_encrypted_peer_gateway( + "preview-peer", + HtmlPreviewResourceRequest { + workspace_id: "fixture-workspace".into(), + file_path: entry, + resource_path: None, + metadata_only: true, + offset: 0, + expected_source: None, + expected_revision: None, + }, + &bytes, + ), + ) + .await + .unwrap(); + assert_eq!( + requests.load(std::sync::atomic::Ordering::Relaxed), + 4, + "entry metadata and three encrypted chunks" + ); + client.disconnect().await; + serving.abort(); + host.stop().await; + *get_account_context().write().await = previous_context; +} diff --git a/src/apps/desktop/src/lib.rs b/src/apps/desktop/src/lib.rs index 5da7da881e..129455f32f 100644 --- a/src/apps/desktop/src/lib.rs +++ b/src/apps/desktop/src/lib.rs @@ -2006,6 +2006,7 @@ pub async fn run() { api::browser_api::browser_webview_set_agent_target_state, api::browser_api::browser_get_url, api::html_preview_api::html_preview_create, + api::html_preview_api::html_preview_read_resource, api::html_preview_api::html_preview_release, // Browser Control API (CDP-based user browser control) api::browser_control_api::browser_control_list_browsers, diff --git a/src/crates/assembly/core/AGENTS.md b/src/crates/assembly/core/AGENTS.md index d8b106ab2a..2441d2ae9c 100644 --- a/src/crates/assembly/core/AGENTS.md +++ b/src/crates/assembly/core/AGENTS.md @@ -594,3 +594,9 @@ Assistant naming, concurrent host allocation, and user-title protection: cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git --lib service::workspace::service::tests::assistant_ cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git --lib title_ ``` + +For HTML preview workspace identity and shared resource-service wiring: + +```bash +cargo test --locked -p openbitfun-core --no-default-features --features remote-connect --lib service::filesystem::html_preview::tests +``` diff --git a/src/crates/assembly/core/src/service/filesystem/html_preview.rs b/src/crates/assembly/core/src/service/filesystem/html_preview.rs new file mode 100644 index 0000000000..03fa6e158a --- /dev/null +++ b/src/crates/assembly/core/src/service/filesystem/html_preview.rs @@ -0,0 +1,139 @@ +//! Bind preview reads to the owning host's persisted workspace and existing filesystem provider. + +use openbitfun_product_domains::html_preview::{ + HtmlPreviewError, HtmlPreviewResourceReply, HtmlPreviewResourceRequest, +}; +use openbitfun_services_integrations::remote_connect::html_preview::HtmlPreviewSource; + +pub async fn resolve_html_preview_source( + workspace_id: &str, + file_path: &str, +) -> Result { + if workspace_id.trim().is_empty() { + return Err(HtmlPreviewError::unavailable( + "HTML preview workspace ID is required", + )); + } + let (target, provider) = crate::service_agent_runtime::CoreServiceAgentRuntime::scoped_remote_file_target_with_identity( + file_path, None, Some(workspace_id), None, None, + ).await.map_err(HtmlPreviewError::unavailable)?; + HtmlPreviewSource::new(target, &format!("{workspace_id}:{provider}")).await +} + +pub async fn html_preview_read_resource( + request: HtmlPreviewResourceRequest, +) -> HtmlPreviewResourceReply { + let result = async { + let source = resolve_html_preview_source(&request.workspace_id, &request.file_path).await?; + source.read(&request).await + } + .await; + result.into() +} + +#[cfg(test)] +mod tests { + use super::*; + use base64::{engine::general_purpose::STANDARD as BASE64, Engine}; + use openbitfun_product_domains::html_preview::HtmlPreviewErrorCode; + + #[tokio::test] + async fn html_preview_uses_registered_workspace_and_shared_local_resource_rules() { + let directory = tempfile::tempdir().unwrap(); + let root = directory.path(); + std::fs::create_dir_all(root.join("site/dist/assets")).unwrap(); + std::fs::write(root.join("site/dist/index.html"), b"page").unwrap(); + std::fs::write(root.join("site/dist/assets/app.js"), b"site-script").unwrap(); + std::fs::write(root.join("site/shared.css"), b"shared-style").unwrap(); + let record = + crate::service::workspace::legacy_compat::register_local_fixture(root, None).await; + let mut request = HtmlPreviewResourceRequest { + workspace_id: record.id, + file_path: root + .join("site/dist/index.html") + .to_string_lossy() + .into_owned(), + resource_path: Some("assets/app.js".into()), + metadata_only: false, + offset: 0, + expected_source: None, + expected_revision: None, + }; + let resource = html_preview_read_resource(request.clone()) + .await + .into_result() + .unwrap(); + assert_eq!( + BASE64.decode(resource.content_base64).unwrap(), + b"site-script" + ); + assert_eq!(resource.mime_type, "text/javascript; charset=utf-8"); + request.resource_path = Some("site/shared.css".into()); + assert_eq!( + BASE64 + .decode( + html_preview_read_resource(request.clone()) + .await + .into_result() + .unwrap() + .content_base64 + ) + .unwrap(), + b"shared-style" + ); + request.resource_path = Some("../secret".into()); + assert_eq!( + html_preview_read_resource(request.clone()) + .await + .into_result() + .unwrap_err() + .code, + HtmlPreviewErrorCode::InvalidPath + ); + request.workspace_id = "unknown-workspace".into(); + request.resource_path = None; + assert!( + html_preview_read_resource(request) + .await + .into_result() + .is_err(), + "Existing file paths must not bypass workspace identity" + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn html_preview_rejects_local_symlinks_outside_the_workspace() { + let workspace = tempfile::tempdir().unwrap(); + let external = tempfile::tempdir().unwrap(); + std::fs::write(workspace.path().join("index.html"), b"page").unwrap(); + std::fs::write(external.path().join("secret.js"), b"secret").unwrap(); + std::os::unix::fs::symlink(external.path(), workspace.path().join("assets")).unwrap(); + let record = crate::service::workspace::legacy_compat::register_local_fixture( + workspace.path(), + None, + ) + .await; + let request = HtmlPreviewResourceRequest { + workspace_id: record.id, + file_path: workspace + .path() + .join("index.html") + .to_string_lossy() + .into_owned(), + resource_path: Some("assets/secret.js".into()), + metadata_only: false, + offset: 0, + expected_source: None, + expected_revision: None, + }; + assert_eq!( + html_preview_read_resource(request) + .await + .into_result() + .unwrap_err() + .code, + HtmlPreviewErrorCode::Forbidden + ); + } +} diff --git a/src/crates/assembly/core/src/service/filesystem/mod.rs b/src/crates/assembly/core/src/service/filesystem/mod.rs index 078f819e0f..8150872eb8 100644 --- a/src/crates/assembly/core/src/service/filesystem/mod.rs +++ b/src/crates/assembly/core/src/service/filesystem/mod.rs @@ -3,6 +3,8 @@ //! Integrates file operations, file tree building, search, and related functionality. pub mod factory; +#[cfg(feature = "remote-connect")] +pub mod html_preview; pub mod listing; #[cfg(feature = "ssh-remote")] pub mod path_operations; diff --git a/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json b/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json index dad0ceed77..7538a8b440 100644 --- a/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json +++ b/src/crates/contracts/product-domains/src/generated/remote-surface-registry.json @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "digest": "fnv1a64:deaa9af9ff3f916e", + "digest": "fnv1a64:99d567cb0f1aa0ad", "retiredCommandPrefixes": [ { "prefix": "lsp_", @@ -42,6 +42,7 @@ "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "whole_session_copy_v1", "user_question_response", "user_question_interaction_v1" @@ -80,6 +81,7 @@ "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "whole_session_copy_v1", "user_question_response", "user_question_interaction_v1" @@ -101,6 +103,7 @@ "tool_catalog", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "user_question_response", "user_question_interaction_v1" ] @@ -2240,8 +2243,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host does not expose the remote-workspace UI surface" + "kind": "handled" } }, { @@ -2252,8 +2254,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host does not expose the remote-workspace UI surface" + "kind": "handled" } }, { @@ -2264,8 +2265,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host does not expose the remote-workspace UI surface" + "kind": "handled" } }, { @@ -2802,8 +2802,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host has no handler for this command" + "kind": "handled" } }, { @@ -3975,6 +3974,17 @@ "reason": "the controller keeps this command; peer hosts refuse it before dispatch" } }, + { + "id": "html_preview_read_resource", + "surface": "tauri_command", + "remoteWorkspace": "RemoteRouted", + "peer": { + "kind": "proxied" + }, + "cliPeer": { + "kind": "handled" + } + }, { "id": "html_preview_release", "surface": "tauri_command", diff --git a/src/crates/contracts/product-domains/src/html_preview.rs b/src/crates/contracts/product-domains/src/html_preview.rs new file mode 100644 index 0000000000..2f11f86e6a --- /dev/null +++ b/src/crates/contracts/product-domains/src/html_preview.rs @@ -0,0 +1,115 @@ +//! Versioned, read-only workspace resources for controller-owned HTML previews. + +use serde::{Deserialize, Serialize}; + +pub const MAX_PREVIEW_BYTES: u64 = 64 * 1024 * 1024; +pub const PREVIEW_CHUNK_BYTES: usize = 256 * 1024; + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct HtmlPreviewResourceRequest { + pub workspace_id: String, + pub file_path: String, + /// Decoded URL path, relative to the preview origin. None selects the entry. + #[serde(default)] + pub resource_path: Option, + #[serde(default)] + pub metadata_only: bool, + #[serde(default)] + pub offset: u64, + #[serde(default)] + pub expected_source: Option, + #[serde(default)] + pub expected_revision: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct HtmlPreviewResource { + pub entry_path: String, + pub resource_path: String, + /// Opaque workspace/provider binding; stable across transport reconnects. + pub source: String, + pub revision: String, + pub mime_type: String, + pub size: u64, + pub offset: u64, + pub content_base64: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum HtmlPreviewErrorCode { + InvalidPath, + NotFound, + Forbidden, + TooLarge, + Changed, + Unavailable, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct HtmlPreviewError { + pub code: HtmlPreviewErrorCode, + pub message: String, +} + +impl HtmlPreviewError { + pub fn new(code: HtmlPreviewErrorCode, message: impl Into) -> Self { + Self { + code, + message: message.into(), + } + } + + pub fn unavailable(message: impl Into) -> Self { + Self::new(HtmlPreviewErrorCode::Unavailable, message) + } +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(tag = "status", content = "data", rename_all = "snake_case")] +pub enum HtmlPreviewResourceReply { + File(HtmlPreviewResource), + Error(HtmlPreviewError), +} + +impl From> for HtmlPreviewResourceReply { + fn from(result: Result) -> Self { + match result { + Ok(file) => Self::File(file), + Err(error) => Self::Error(error), + } + } +} + +impl HtmlPreviewResourceReply { + pub fn into_result(self) -> Result { + match self { + Self::File(file) => Ok(file), + Self::Error(error) => Err(error), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn minimal_request_round_trip_keeps_optional_defaults() { + let request: HtmlPreviewResourceRequest = serde_json::from_value(serde_json::json!({ + "workspaceId": "workspace", "filePath": "/repo/dist/index.html", "future": true + })) + .unwrap(); + assert!(request.resource_path.is_none()); + assert_eq!(request.offset, 0); + assert!(!request.metadata_only); + assert!(request.expected_source.is_none()); + assert!(request.expected_revision.is_none()); + let round_trip: HtmlPreviewResourceRequest = + serde_json::from_value(serde_json::to_value(request).unwrap()).unwrap(); + assert_eq!(round_trip.workspace_id, "workspace"); + assert_eq!(round_trip.file_path, "/repo/dist/index.html"); + } +} diff --git a/src/crates/contracts/product-domains/src/lib.rs b/src/crates/contracts/product-domains/src/lib.rs index 408fb380ef..5d59631e0f 100644 --- a/src/crates/contracts/product-domains/src/lib.rs +++ b/src/crates/contracts/product-domains/src/lib.rs @@ -6,6 +6,7 @@ pub mod account; pub mod agent_catalog; pub mod canvas; +pub mod html_preview; pub mod mcp; pub mod native_hooks; pub mod product_control; diff --git a/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs b/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs index c6641fab32..1c5c432e9f 100644 --- a/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs +++ b/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs @@ -87,6 +87,8 @@ pub enum PeerHostCapability { ChatMcpCatalogV1, /// Workspace references can carry the owning host's persisted ID. WorkspaceIdReferencesV1, + /// Workspace-confined static resources for controller-owned HTML preview. + WorkspaceHtmlPreviewV1, /// `copy_session` copies a whole session without a caller-selected turn id. WholeSessionCopyV1, /// `submit_user_answers` is implemented for Runtime-owned questions. @@ -131,6 +133,7 @@ impl PeerHostCapability { Self::ReviewPlatformV1, Self::ChatMcpCatalogV1, Self::WorkspaceIdReferencesV1, + Self::WorkspaceHtmlPreviewV1, Self::WholeSessionCopyV1, Self::UserQuestionResponse, Self::UserQuestionInteractionV1, @@ -172,6 +175,7 @@ impl PeerHostCapability { Self::ReviewPlatformV1 => "review_platform_v1", Self::ChatMcpCatalogV1 => "chat_mcp_catalog_v1", Self::WorkspaceIdReferencesV1 => "workspace_id_references_v1", + Self::WorkspaceHtmlPreviewV1 => "workspace_html_preview_v1", Self::WholeSessionCopyV1 => "whole_session_copy_v1", Self::UserQuestionResponse => "user_question_response", Self::UserQuestionInteractionV1 => "user_question_interaction_v1", @@ -205,6 +209,7 @@ const CLI_CAPABILITIES: &[PeerHostCapability] = &[ PeerHostCapability::ToolCatalog, PeerHostCapability::ChatMcpCatalogV1, PeerHostCapability::WorkspaceIdReferencesV1, + PeerHostCapability::WorkspaceHtmlPreviewV1, PeerHostCapability::UserQuestionResponse, PeerHostCapability::UserQuestionInteractionV1, ]; diff --git a/src/crates/contracts/product-domains/src/remote_surface/table.rs b/src/crates/contracts/product-domains/src/remote_surface/table.rs index 95059cc053..aed4c41da7 100644 --- a/src/crates/contracts/product-domains/src/remote_surface/table.rs +++ b/src/crates/contracts/product-domains/src/remote_surface/table.rs @@ -264,9 +264,9 @@ pub(super) const OPERATIONS: &[OperationDefinition] = &[ op("ensure_coordinator_session", Routed, Proxied, HANDLED), op("execute_tool", Routed, Proxied, CLI_NOT_IMPLEMENTED), op("expand_external_prompt_command_command", Unsupported, Proxied, CLI_NOT_IMPLEMENTED), - op("explorer_get_children", Routed, Proxied, CLI_NO_REMOTE_WORKSPACE_UI), - op("explorer_get_children_paginated", Routed, Proxied, CLI_NO_REMOTE_WORKSPACE_UI), - op("explorer_get_file_tree", Routed, Proxied, CLI_NO_REMOTE_WORKSPACE_UI), + op("explorer_get_children", Routed, Proxied, HANDLED), + op("explorer_get_children_paginated", Routed, Proxied, HANDLED), + op("explorer_get_file_tree", Routed, Proxied, HANDLED), op("export_config", Unaudited, Proxied, CLI_NOT_IMPLEMENTED), op("export_diagnostics_bundle", Unaudited, Proxied, CLI_NOT_IMPLEMENTED), op("export_local_file_to_path", Unsupported, Proxied, CLI_NOT_IMPLEMENTED), @@ -312,7 +312,7 @@ pub(super) const OPERATIONS: &[OperationDefinition] = &[ op("get_file_diff", Unsupported, Proxied, CLI_NOT_IMPLEMENTED), op("get_file_editor_sync_hash", Unaudited, Proxied, CLI_NOT_IMPLEMENTED), op("get_file_metadata", Routed, Proxied, CLI_NOT_IMPLEMENTED), - op("get_file_tree", Routed, Proxied, CLI_NOT_IMPLEMENTED), + op("get_file_tree", Routed, Proxied, HANDLED), op("get_file_watch_status", LocalOnly, Proxied, CLI_NOT_IMPLEMENTED), op("get_frontend_update_status", LocalOnly, ControllerLocal, REFUSED), op("get_global_config_health", Unaudited, Proxied, CLI_NOT_IMPLEMENTED), @@ -416,6 +416,7 @@ pub(super) const OPERATIONS: &[OperationDefinition] = &[ op("hide_agent_companion_desktop_pet", LocalOnly, ControllerLocal, REFUSED), op("hide_main_window_after_close_request", LocalOnly, ControllerLocal, REFUSED), op("html_preview_create", Routed, ControllerLocal, REFUSED), + op("html_preview_read_resource", Routed, Proxied, HANDLED), op("html_preview_release", Agnostic, ControllerLocal, REFUSED), op("i18n_get_config", Agnostic, ControllerLocal, REFUSED), op("i18n_get_current_language", Agnostic, ControllerLocal, REFUSED), diff --git a/src/crates/services/services-integrations/AGENTS.md b/src/crates/services/services-integrations/AGENTS.md index 8b4caa0566..ee2fe4fb37 100644 --- a/src/crates/services/services-integrations/AGENTS.md +++ b/src/crates/services/services-integrations/AGENTS.md @@ -205,3 +205,16 @@ For models.dev snapshot reuse, last-valid recovery, and background refresh: ```bash cargo test --locked -p openbitfun-services-integrations --no-default-features --features models-dev --lib models_dev::tests ``` + +For workspace-confined HTML resources and revision-checked chunk reads, run: + +```bash +cargo test --locked -p openbitfun-services-integrations --no-default-features --features remote-connect --lib remote_connect::html_preview::tests +``` + +To use a real OpenSSH SFTP executable over loopback SSH, set +`OPENBITFUN_TEST_SFTP_SERVER` and run the same package with +`--features remote-connect,remote-ssh-concrete --lib openssh_html_preview_reads_nested_site_resources -- --ignored`. +This checks the production remote filesystem provider, root-relative assets, +parent-relative resources and workspace boundaries; it does not establish +multi-device relay or physical-host coverage. diff --git a/src/crates/services/services-integrations/src/remote_connect.rs b/src/crates/services/services-integrations/src/remote_connect.rs index f511e93e9a..06662793bf 100644 --- a/src/crates/services/services-integrations/src/remote_connect.rs +++ b/src/crates/services/services-integrations/src/remote_connect.rs @@ -16,6 +16,7 @@ pub mod encryption; pub mod file_projection; pub mod host_stream; pub mod host_stream_subscriber; +pub mod html_preview; mod lan; mod page_upload; pub mod pairing; diff --git a/src/crates/services/services-integrations/src/remote_connect/html_preview.rs b/src/crates/services/services-integrations/src/remote_connect/html_preview.rs new file mode 100644 index 0000000000..8a05a28573 --- /dev/null +++ b/src/crates/services/services-integrations/src/remote_connect/html_preview.rs @@ -0,0 +1,333 @@ +//! Workspace-confined static resources, shared by Desktop and CLI peer hosts. +//! The caller selects a workspace/provider; this service never falls back to another filesystem. + +use super::file_projection::SessionFileTarget; +use base64::{engine::general_purpose::STANDARD as BASE64, Engine}; +use openbitfun_product_domains::html_preview::{ + HtmlPreviewError, HtmlPreviewErrorCode as Code, HtmlPreviewResource, + HtmlPreviewResourceRequest, MAX_PREVIEW_BYTES, PREVIEW_CHUNK_BYTES, +}; +use openbitfun_runtime_ports::{WorkspaceFileSystem, WorkspaceMetadata, WorkspacePathKind}; +use sha2::{Digest, Sha256}; +use std::{path::Path, sync::Arc}; +use tokio::io::{AsyncReadExt, AsyncSeekExt}; + +pub struct HtmlPreviewSource { + fs: Arc, + root: String, + remote: bool, + entry_path: String, + source: String, +} + +type PreviewResult = Result; + +impl HtmlPreviewSource { + pub async fn new(target: SessionFileTarget, identity: &str) -> PreviewResult { + let (root, entry_path) = if target.remote { + let root = normalize_posix(&target.root)?; + let entry = normalize_posix(&target.path)?; + let prefix = format!("{}/", root.trim_end_matches('/')); + let relative = entry.strip_prefix(&prefix).ok_or_else(outside_workspace)?; + (root, relative.to_string()) + } else { + let root = tokio::fs::canonicalize(&target.root) + .await + .map_err(local_io_error)?; + let entry = tokio::fs::canonicalize(&target.path) + .await + .map_err(local_io_error)?; + let relative = entry.strip_prefix(&root).map_err(|_| outside_workspace())?; + let relative = relative + .components() + .map(|part| part.as_os_str().to_string_lossy()) + .collect::>() + .join("/"); + (root.to_string_lossy().into_owned(), relative) + }; + validate_relative(&entry_path)?; + let source = digest(&[ + identity, + &root, + &entry_path, + if target.remote { "remote" } else { "local" }, + ]); + let result = Self { + fs: target.fs, + root, + remote: target.remote, + entry_path, + source, + }; + result.checked_file(&result.entry_path).await?; + Ok(result) + } + + pub async fn read( + &self, + request: &HtmlPreviewResourceRequest, + ) -> PreviewResult { + if request + .expected_source + .as_deref() + .is_some_and(|source| source != self.source) + { + return Err(HtmlPreviewError::new( + Code::Changed, + "HTML preview workspace changed; reopen the preview", + )); + } + let candidates = match &request.resource_path { + Some(path) => resource_candidates(&self.entry_path, path)?, + None => vec![self.entry_path.clone()], + }; + let mut resolved = None; + for relative in candidates { + match self.checked_file(&relative).await { + Ok((path, metadata)) => { + resolved = Some((relative, path, metadata)); + break; + } + Err(error) if error.code == Code::NotFound => {} + Err(error) => return Err(error), + } + } + let (relative, path, before) = resolved.ok_or_else(not_found)?; + let size = before + .size + .ok_or_else(|| HtmlPreviewError::unavailable("Preview file size is unavailable"))?; + if size > MAX_PREVIEW_BYTES { + return Err(HtmlPreviewError::new( + Code::TooLarge, + "Preview file is too large", + )); + } + let modified = before + .modified + .and_then(|time| time.duration_since(std::time::UNIX_EPOCH).ok()) + .ok_or_else(|| HtmlPreviewError::unavailable("Preview file revision is unavailable"))?; + let revision = digest(&[ + &self.source, + &relative, + &size.to_string(), + &modified.as_nanos().to_string(), + ]); + if request + .expected_revision + .as_deref() + .is_some_and(|expected| expected != revision) + { + return Err(HtmlPreviewError::new( + Code::Changed, + "Preview file changed during transfer; reload the page", + )); + } + if request.offset > size { + return Err(HtmlPreviewError::new( + Code::InvalidPath, + "Invalid preview file offset", + )); + } + let mut bytes = Vec::new(); + if !request.metadata_only { + let count = (size - request.offset).min(PREVIEW_CHUNK_BYTES as u64); + let mut reader = self.fs.open_read(&path).await.map_err(provider_error)?; + reader + .seek(std::io::SeekFrom::Start(request.offset)) + .await + .map_err(provider_error)?; + reader + .take(count) + .read_to_end(&mut bytes) + .await + .map_err(provider_error)?; + let after = self + .fs + .metadata(&path, false) + .await + .map_err(provider_error)?; + if after.as_ref() != Some(&before) || bytes.len() as u64 != count { + return Err(HtmlPreviewError::new( + Code::Changed, + "Preview file changed during transfer; reload the page", + )); + } + } + Ok(HtmlPreviewResource { + entry_path: self.entry_path.clone(), + resource_path: relative, + source: self.source.clone(), + revision, + mime_type: mime_for_path(&path), + size, + offset: request.offset, + content_base64: BASE64.encode(bytes), + }) + } + + async fn checked_file(&self, relative: &str) -> PreviewResult<(String, WorkspaceMetadata)> { + validate_relative(relative)?; + let parts: Vec<_> = relative + .split('/') + .filter(|part| !part.is_empty() && *part != ".") + .collect(); + let requested = self.fs.join_path(&self.root, &parts); + let path = if self.remote { + // POSIX paths on every controller OS. Check each component with no-follow metadata. + let mut current = String::new(); + for part in requested.split('/').filter(|part| !part.is_empty()) { + current.push('/'); + current.push_str(part); + let metadata = self + .fs + .metadata(¤t, false) + .await + .map_err(provider_error)? + .ok_or_else(not_found)?; + if metadata.kind == WorkspacePathKind::Symlink { + return Err(HtmlPreviewError::new( + Code::Forbidden, + "Symbolic links are not available in remote HTML preview", + )); + } + if current != requested && metadata.kind != WorkspacePathKind::Directory { + return Err(not_found()); + } + } + requested + } else { + let canonical = tokio::fs::canonicalize(&requested) + .await + .map_err(local_io_error)?; + if !canonical.starts_with(Path::new(&self.root)) { + return Err(outside_workspace()); + } + canonical.to_string_lossy().into_owned() + }; + let metadata = self + .fs + .metadata(&path, false) + .await + .map_err(provider_error)? + .ok_or_else(not_found)?; + if metadata.kind != WorkspacePathKind::File { + return Err(not_found()); + } + Ok((path, metadata)) + } +} + +fn digest(parts: &[&str]) -> String { + let mut digest = Sha256::new(); + for part in parts { + digest.update((part.len() as u64).to_le_bytes()); + digest.update(part.as_bytes()); + } + format!("{:x}", digest.finalize()) +} + +fn normalize_posix(path: &str) -> PreviewResult { + if !path.starts_with('/') || path.contains('\0') { + return Err(outside_workspace()); + } + let mut parts = Vec::new(); + for part in path.split('/') { + match part { + "" | "." => {} + ".." => { + if parts.pop().is_none() { + return Err(outside_workspace()); + } + } + part => parts.push(part), + } + } + Ok(format!("/{}", parts.join("/"))) +} + +fn validate_relative(path: &str) -> PreviewResult<()> { + if path.contains('\0') + || path + .split('/') + .any(|part| part == ".." || part.contains('\\')) + { + return Err(HtmlPreviewError::new( + Code::InvalidPath, + "Invalid preview path", + )); + } + Ok(()) +} + +fn resource_candidates(entry: &str, path: &str) -> PreviewResult> { + validate_relative(path)?; + let relative = path + .split('/') + .filter(|part| !part.is_empty() && *part != ".") + .collect::>() + .join("/"); + let relative = if path.ends_with('/') || relative.is_empty() { + format!("{relative}/index.html") + .trim_start_matches('/') + .to_string() + } else { + relative + }; + let directory = entry + .rsplit_once('/') + .map(|(directory, _)| directory) + .unwrap_or(""); + // The entry keeps its workspace-relative URL, preserving ../ and explicitly prefixed links. + // Root-relative build assets (/assets/...) resolve beside the selected HTML first. + if directory.is_empty() || relative.starts_with(&format!("{directory}/")) { + return Ok(vec![relative]); + } + Ok(vec![format!("{directory}/{relative}"), relative]) +} + +fn mime_for_path(path: &str) -> String { + let extension = path.rsplit('.').next().unwrap_or("").to_ascii_lowercase(); + let mime = match extension.as_str() { + "cjs" => "text/javascript", + "wasm" => "application/wasm", + "woff" => "font/woff", + "woff2" => "font/woff2", + "ttf" => "font/ttf", + "otf" => "font/otf", + "webmanifest" => "application/manifest+json", + "map" => "application/json", + "mp3" => "audio/mpeg", + "wav" => "audio/wav", + "ogg" => "audio/ogg", + "webm" => "video/webm", + _ => super::detect_remote_mime_type(Path::new(path)), + }; + if mime.starts_with("text/") { + format!("{mime}; charset=utf-8") + } else { + mime.to_string() + } +} + +fn not_found() -> HtmlPreviewError { + HtmlPreviewError::new(Code::NotFound, "Preview resource not found") +} +fn outside_workspace() -> HtmlPreviewError { + HtmlPreviewError::new( + Code::Forbidden, + "HTML preview path is outside the workspace", + ) +} +fn provider_error(error: impl std::fmt::Display) -> HtmlPreviewError { + HtmlPreviewError::unavailable(format!("Failed to read preview resource: {error}")) +} +fn local_io_error(error: std::io::Error) -> HtmlPreviewError { + if error.kind() == std::io::ErrorKind::NotFound { + not_found() + } else { + provider_error(error) + } +} + +#[cfg(test)] +mod tests; diff --git a/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs b/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs new file mode 100644 index 0000000000..bff2106dff --- /dev/null +++ b/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs @@ -0,0 +1,347 @@ +use super::*; +use openbitfun_runtime_ports::{WorkspaceDirEntry, WorkspaceReader}; +use std::{ + collections::BTreeMap, + sync::{ + atomic::{AtomicBool, Ordering}, + Mutex, + }, + time::{Duration, SystemTime}, +}; + +#[derive(Clone)] +struct Node { + metadata: WorkspaceMetadata, + bytes: Vec, +} + +#[derive(Default)] +struct RemoteFs { + nodes: Mutex>, + reads: Mutex>, + offline: AtomicBool, + truncate: AtomicBool, +} + +impl RemoteFs { + fn file(&self, path: &str, bytes: &[u8]) { + let mut nodes = self.nodes.lock().unwrap(); + let mut ancestor = String::new(); + for part in path.split('/').filter(|part| !part.is_empty()) { + ancestor.push('/'); + ancestor.push_str(part); + nodes.insert( + ancestor.clone(), + Node { + metadata: WorkspaceMetadata { + kind: if ancestor == path { + WorkspacePathKind::File + } else { + WorkspacePathKind::Directory + }, + size: Some(bytes.len() as u64), + modified: Some(SystemTime::UNIX_EPOCH + Duration::from_secs(10)), + permissions: None, + }, + bytes: if ancestor == path { + bytes.to_vec() + } else { + vec![] + }, + }, + ); + } + } +} + +#[async_trait::async_trait] +impl WorkspaceFileSystem for RemoteFs { + fn join_path(&self, root: &str, components: &[&str]) -> String { + format!("{}/{}", root.trim_end_matches('/'), components.join("/")) + } + async fn metadata( + &self, + path: &str, + follow: bool, + ) -> anyhow::Result> { + assert!(!follow, "Preview must use no-follow metadata"); + anyhow::ensure!( + !self.offline.load(Ordering::SeqCst), + "SSH connection is offline" + ); + Ok(self + .nodes + .lock() + .unwrap() + .get(path) + .map(|node| node.metadata.clone())) + } + async fn open_read(&self, path: &str) -> anyhow::Result { + anyhow::ensure!( + !self.offline.load(Ordering::SeqCst), + "SSH connection is offline" + ); + self.reads.lock().unwrap().push(path.into()); + let mut bytes = self.nodes.lock().unwrap().get(path).unwrap().bytes.clone(); + if self.truncate.load(Ordering::SeqCst) { + bytes.pop(); + } + Ok(Box::new(std::io::Cursor::new(bytes))) + } + async fn read_file(&self, _: &str) -> anyhow::Result> { + anyhow::bail!("whole-file reads must not be used") + } + async fn read_file_text(&self, _: &str) -> anyhow::Result { + anyhow::bail!("unused") + } + async fn write_file(&self, _: &str, _: &[u8]) -> anyhow::Result<()> { + anyhow::bail!("read-only") + } + async fn exists(&self, _: &str) -> anyhow::Result { + anyhow::bail!("unused") + } + async fn is_file(&self, _: &str) -> anyhow::Result { + anyhow::bail!("unused") + } + async fn is_dir(&self, _: &str) -> anyhow::Result { + anyhow::bail!("unused") + } + async fn read_dir(&self, _: &str) -> anyhow::Result> { + anyhow::bail!("unused") + } +} + +fn request(resource: Option<&str>) -> HtmlPreviewResourceRequest { + HtmlPreviewResourceRequest { + workspace_id: "workspace".into(), + file_path: "/repo/site/dist/index.html".into(), + resource_path: resource.map(str::to_string), + metadata_only: false, + offset: 0, + expected_source: None, + expected_revision: None, + } +} + +async fn fixture() -> (HtmlPreviewSource, Arc) { + let fs = Arc::new(RemoteFs::default()); + fs.file("/repo/site/dist/index.html", br#""#); + fs.file( + "/repo/site/dist/assets/app.css", + b"@font-face{src:url('./font.woff2')}body{color:green}", + ); + fs.file("/repo/site/dist/assets/app.js", b"import('./chunk.mjs')"); + fs.file( + "/repo/site/dist/assets/chunk.mjs", + b"document.body.dataset.loaded='yes'", + ); + fs.file("/repo/site/dist/assets/font.woff2", &[0, 255, 1, 2]); + fs.file("/repo/site/dist/assets/pic #1.png", &[137, 80, 78, 71]); + fs.file("/repo/site/shared.css", b"shared"); + // A different project at workspace root must not shadow the selected site's assets. + fs.file("/repo/assets/app.js", b"wrong-project"); + let source = HtmlPreviewSource::new( + SessionFileTarget { + fs: fs.clone(), + path: request(None).file_path, + root: "/repo".into(), + remote: true, + }, + "workspace:ssh-1", + ) + .await + .unwrap(); + (source, fs) +} + +#[tokio::test] +async fn nested_site_serves_absolute_relative_css_imports_and_binary_assets() { + let (source, _) = fixture().await; + for (url, expected, mime) in [ + ( + "site/dist/assets/app.js", + b"import('./chunk.mjs')".as_slice(), + "text/javascript; charset=utf-8", + ), + ( + "assets/app.js", + b"import('./chunk.mjs')".as_slice(), + "text/javascript; charset=utf-8", + ), + ( + "assets/chunk.mjs", + b"document.body.dataset.loaded='yes'".as_slice(), + "text/javascript; charset=utf-8", + ), + ("assets/font.woff2", [0, 255, 1, 2].as_slice(), "font/woff2"), + ( + "assets/pic #1.png", + [137, 80, 78, 71].as_slice(), + "image/png", + ), + ( + "site/shared.css", + b"shared".as_slice(), + "text/css; charset=utf-8", + ), + ] { + let result = source.read(&request(Some(url))).await.unwrap(); + assert_eq!( + BASE64.decode(result.content_base64).unwrap(), + expected, + "{url}" + ); + assert_eq!(result.mime_type, mime); + assert_eq!(result.entry_path, "site/dist/index.html"); + } + let css = source.read(&request(Some("assets/app.css"))).await.unwrap(); + assert_eq!(css.mime_type, "text/css; charset=utf-8"); +} + +#[tokio::test] +async fn remote_preview_never_reads_same_named_controller_files_or_falls_back_offline() { + let directory = tempfile::tempdir().unwrap(); + let root = directory.path().to_string_lossy().into_owned(); + let path = format!("{root}/index.html"); + std::fs::write(&path, b"private-controller-content").unwrap(); + let fs = Arc::new(RemoteFs::default()); + fs.file(&path, b"remote-page"); + let source = HtmlPreviewSource::new( + SessionFileTarget { + fs: fs.clone(), + root, + path, + remote: true, + }, + "remote", + ) + .await + .unwrap(); + assert_eq!( + BASE64 + .decode(source.read(&request(None)).await.unwrap().content_base64) + .unwrap(), + b"remote-page" + ); + fs.offline.store(true, Ordering::SeqCst); + assert_eq!( + source.read(&request(None)).await.unwrap_err().code, + Code::Unavailable + ); + fs.offline.store(false, Ordering::SeqCst); + assert!( + source.read(&request(None)).await.is_ok(), + "A reconnected provider can be read again" + ); +} + +#[tokio::test] +async fn traversal_symlinks_and_provider_errors_do_not_trigger_fallback() { + let (source, fs) = fixture().await; + for path in [ + "../secret", + "assets/../../secret", + "assets\\secret", + "assets/\0secret", + ] { + assert_eq!( + source.read(&request(Some(path))).await.unwrap_err().code, + Code::InvalidPath + ); + } + assert_eq!( + source + .read(&request(Some("missing.js"))) + .await + .unwrap_err() + .code, + Code::NotFound + ); + fs.nodes + .lock() + .unwrap() + .get_mut("/repo/site/dist/assets") + .unwrap() + .metadata + .kind = WorkspacePathKind::Symlink; + assert_eq!( + source + .read(&request(Some("assets/app.js"))) + .await + .unwrap_err() + .code, + Code::Forbidden + ); + assert!(fs.reads.lock().unwrap().is_empty()); + let invalid = HtmlPreviewSource::new( + SessionFileTarget { + fs, + root: "/repo".into(), + path: "/repo-other/index.html".into(), + remote: true, + }, + "remote", + ) + .await; + assert!(matches!( + invalid, + Err(HtmlPreviewError { + code: Code::Forbidden, + .. + }) + )); +} + +#[tokio::test] +async fn chunks_are_bounded_and_pinned_to_file_and_workspace_revision() { + let (source, fs) = fixture().await; + let bytes = vec![42; PREVIEW_CHUNK_BYTES + 33]; + fs.file("/repo/site/dist/assets/large.js", &bytes); + let mut input = request(Some("assets/large.js")); + let first = source.read(&input).await.unwrap(); + assert_eq!( + BASE64.decode(&first.content_base64).unwrap().len(), + PREVIEW_CHUNK_BYTES + ); + input.offset = PREVIEW_CHUNK_BYTES as u64; + input.expected_source = Some(first.source.clone()); + input.expected_revision = Some(first.revision); + assert_eq!( + BASE64 + .decode(source.read(&input).await.unwrap().content_base64) + .unwrap() + .len(), + 33 + ); + input.expected_source = Some("other-device-or-provider".into()); + assert_eq!(source.read(&input).await.unwrap_err().code, Code::Changed); + input.expected_source = Some(first.source); + fs.nodes + .lock() + .unwrap() + .get_mut("/repo/site/dist/assets/large.js") + .unwrap() + .metadata + .modified = Some(SystemTime::now()); + assert_eq!(source.read(&input).await.unwrap_err().code, Code::Changed); +} + +#[tokio::test] +async fn head_skips_content_and_oversized_or_truncated_files_fail() { + let (source, fs) = fixture().await; + let mut input = request(None); + input.metadata_only = true; + assert!(source.read(&input).await.unwrap().content_base64.is_empty()); + assert!(fs.reads.lock().unwrap().is_empty()); + input.metadata_only = false; + fs.truncate.store(true, Ordering::SeqCst); + assert_eq!(source.read(&input).await.unwrap_err().code, Code::Changed); + fs.nodes + .lock() + .unwrap() + .get_mut("/repo/site/dist/index.html") + .unwrap() + .metadata + .size = Some(MAX_PREVIEW_BYTES + 1); + assert_eq!(source.read(&input).await.unwrap_err().code, Code::TooLarge); +} diff --git a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs index 478faba8b8..90c0ff6570 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs @@ -765,6 +765,118 @@ async fn openssh_real_files_over_loopback_ssh() { assert_eq!(f.state.channels.load(Ordering::SeqCst), 2); } +#[cfg(feature = "remote-connect")] +#[tokio::test] +#[ignore = "Set OPENBITFUN_TEST_SFTP_SERVER to the OpenSSH sftp-server executable"] +async fn openssh_html_preview_reads_nested_site_resources() { + use crate::remote_connect::{ + file_projection::SessionFileTarget, html_preview::HtmlPreviewSource, + }; + use crate::remote_ssh::workspace_services::RemoteWorkspaceFs; + use base64::{engine::general_purpose::STANDARD as BASE64, Engine}; + use openbitfun_product_domains::html_preview::{ + HtmlPreviewErrorCode, HtmlPreviewResourceRequest, PREVIEW_CHUNK_BYTES, + }; + let executable = std::env::var("OPENBITFUN_TEST_SFTP_SERVER").unwrap(); + let f = Fixture::with_openssh(Some(executable)).await; + let root = f._dir.path().canonicalize().unwrap(); + std::fs::create_dir_all(root.join("website/dist/assets")).unwrap(); + std::fs::write( + root.join("website/dist/index.html"), + b"", + ) + .unwrap(); + std::fs::write( + root.join("website/dist/assets/app.js"), + b"window.sshLoaded=true", + ) + .unwrap(); + std::fs::write( + root.join("website/dist/assets/app.css"), + b"body{color:green}", + ) + .unwrap(); + std::fs::write( + root.join("website/dist/assets/image.png"), + [137, 80, 78, 71, 0, 255], + ) + .unwrap(); + std::fs::write(root.join("website/shared.css"), b"parent-style").unwrap(); + let large = vec![53; PREVIEW_CHUNK_BYTES + 17]; + std::fs::write(root.join("website/dist/assets/large.js"), &large).unwrap(); + let service = crate::remote_ssh::RemoteFileService::new(Arc::new(tokio::sync::RwLock::new( + Some(f.manager.as_ref().clone()), + ))); + let entry = root + .join("website/dist/index.html") + .to_string_lossy() + .into_owned(); + let source = HtmlPreviewSource::new( + SessionFileTarget { + fs: Arc::new(RemoteWorkspaceFs::new("sftp-test".into(), service)), + root: root.to_string_lossy().into_owned(), + path: entry.clone(), + remote: true, + }, + "ssh-workspace", + ) + .await + .unwrap(); + for (path, expected) in [ + ("assets/app.js", b"window.sshLoaded=true".as_slice()), + ( + "website/dist/assets/app.css", + b"body{color:green}".as_slice(), + ), + ("assets/image.png", [137, 80, 78, 71, 0, 255].as_slice()), + ("website/shared.css", b"parent-style".as_slice()), + ] { + let file = source + .read(&HtmlPreviewResourceRequest { + workspace_id: "ssh-workspace".into(), + file_path: entry.clone(), + resource_path: Some(path.into()), + metadata_only: false, + offset: 0, + expected_source: None, + expected_revision: None, + }) + .await + .unwrap(); + assert_eq!(BASE64.decode(file.content_base64).unwrap(), expected); + } + let mut chunk_request = HtmlPreviewResourceRequest { + workspace_id: "ssh-workspace".into(), + file_path: entry, + resource_path: Some("assets/large.js".into()), + metadata_only: false, + offset: 0, + expected_source: None, + expected_revision: None, + }; + let first = source.read(&chunk_request).await.unwrap(); + let mut content = BASE64.decode(first.content_base64).unwrap(); + assert_eq!(content.len(), PREVIEW_CHUNK_BYTES); + chunk_request.offset = content.len() as u64; + chunk_request.expected_source = Some(first.source); + chunk_request.expected_revision = Some(first.revision); + content.extend( + BASE64 + .decode(source.read(&chunk_request).await.unwrap().content_base64) + .unwrap(), + ); + assert_eq!(content, large); + chunk_request.resource_path = Some("../private-file".into()); + assert_eq!( + source.read(&chunk_request).await.unwrap_err().code, + HtmlPreviewErrorCode::InvalidPath + ); + assert!( + f.state.channels.load(Ordering::SeqCst) > 0, + "Reads must traverse a real SSH/SFTP channel" + ); +} + #[cfg(unix)] #[tokio::test] #[ignore = "Set OPENBITFUN_TEST_SFTP_SERVER to the OpenSSH sftp-server executable"] diff --git a/src/shared/interactive-capabilities/catalog.json b/src/shared/interactive-capabilities/catalog.json index f3ce71670a..597065f760 100644 --- a/src/shared/interactive-capabilities/catalog.json +++ b/src/shared/interactive-capabilities/catalog.json @@ -290,6 +290,7 @@ ], "htmlPreview": [ "html_preview_create", + "html_preview_read_resource", "html_preview_release" ], "githubDeviceRouting": [ diff --git a/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.test.tsx b/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.test.tsx new file mode 100644 index 0000000000..309fd82ee0 --- /dev/null +++ b/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.test.tsx @@ -0,0 +1,81 @@ +// @vitest-environment jsdom +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import HtmlPreviewPanel from './HtmlPreviewPanel'; +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; + +const mocks = vi.hoisted(() => ({ create: vi.fn(), release: vi.fn() })); +vi.mock('@/infrastructure/api/htmlPreviewApi', () => ({ htmlPreviewApi: mocks })); +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })); +vi.mock('@openbitfun/ui', () => ({ LoadingState: ({ children }: React.PropsWithChildren) => {children} })); +vi.mock('./BrowserPanel', () => ({ default: ({ initialUrl }: { initialUrl: string }) =>
})); + +let root: Root; +let host: HTMLDivElement; +const props = { filePath: '/repo/dist/index.html', workspaceId: 'same-id', isActive: true }; +const preview = (id: string) => ({ sessionId: id, url: `http://127.0.0.1/${id}` }); +const mount = async (next = props) => { await act(async () => { root.render(); }); }; +const currentUrl = () => host.querySelector('[data-preview-url]')?.getAttribute('data-preview-url'); + +beforeEach(() => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); + activateSurface('local'); + mocks.create.mockReset().mockResolvedValue(preview('local')); + mocks.release.mockReset().mockResolvedValue(undefined); + host = document.createElement('div'); + document.body.append(host); + root = createRoot(host); +}); +afterEach(async () => { + await act(async () => root.unmount()); + host.remove(); + vi.unstubAllGlobals(); +}); + +it('recreates a preview on a device switch even when workspace and path are identical', async () => { + await mount(); + expect(currentUrl()).toBe(preview('local').url); + mocks.create.mockResolvedValueOnce(preview('peer')); + await act(async () => { activateSurface('peer'); }); + expect(mocks.release).toHaveBeenCalledWith('local'); + expect(currentUrl()).toBe(preview('peer').url); + expect(mocks.create).toHaveBeenCalledTimes(2); +}); + +it('discards and releases a delayed result after switching to another device', async () => { + let resolve!: (value: ReturnType) => void; + mocks.create.mockImplementationOnce(() => new Promise(done => { resolve = done; })); + await mount(); + mocks.create.mockResolvedValueOnce(preview('peer')); + await act(async () => { activateSurface('peer'); }); + await act(async () => { resolve(preview('stale')); }); + expect(currentUrl()).toBe(preview('peer').url); + expect(mocks.release).toHaveBeenCalledWith('stale'); +}); + +it('does not recreate or release the gateway just because the tab is hidden', async () => { + await mount(); + await mount({ ...props, isActive: false }); + expect(mocks.create).toHaveBeenCalledTimes(1); + expect(mocks.release).not.toHaveBeenCalled(); +}); + +it('replaces stale content with the failure from the selected peer', async () => { + await mount(); + mocks.create.mockRejectedValueOnce(new Error('Update the controlled device')); + await act(async () => { activateSurface('old-peer'); }); + expect(currentUrl()).toBeUndefined(); + expect(host.textContent).toContain('Update the controlled device'); + expect(mocks.release).toHaveBeenCalledWith('local'); +}); + +it('releases a late creation after its panel is closed', async () => { + let resolve!: (value: ReturnType) => void; + mocks.create.mockImplementationOnce(() => new Promise(done => { resolve = done; })); + await mount(); + await act(async () => { root.render(null); }); + await act(async () => { resolve(preview('closed')); }); + expect(mocks.release).toHaveBeenCalledWith('closed'); + expect(currentUrl()).toBeUndefined(); +}); diff --git a/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.tsx b/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.tsx index 79a1e95d17..36c6398e88 100644 --- a/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.tsx +++ b/src/web-ui/src/app/scenes/browser/HtmlPreviewPanel.tsx @@ -1,10 +1,13 @@ -import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useState, useSyncExternalStore } from 'react'; import { AlertTriangle } from 'lucide-react'; import { LoadingState } from '@openbitfun/ui'; import { useTranslation } from 'react-i18next'; import BrowserPanel from './BrowserPanel'; import { htmlPreviewApi } from '@/infrastructure/api/htmlPreviewApi'; -import { isPeerDeviceModeActive } from '@/infrastructure/peer-device/peerModeFlag'; +import { getActiveSurfaceScope, isSurfaceChangedError, onSurfaceActivated } from '@/infrastructure/peer-device/deviceSurface'; +import { createLogger } from '@/shared/utils/logger'; + +const log = createLogger('HtmlPreviewPanel'); export interface HtmlPreviewPanelProps { isActive: boolean; @@ -12,7 +15,7 @@ export interface HtmlPreviewPanelProps { workspaceId: string; } -const HtmlPreviewPanel: React.FC = ({ +const HtmlPreviewContent: React.FC = ({ isActive, filePath, workspaceId, @@ -20,37 +23,35 @@ const HtmlPreviewPanel: React.FC = ({ const { t } = useTranslation('common'); const [url, setUrl] = useState(); const [error, setError] = useState(); - const sessionIdRef = useRef(); useEffect(() => { let cancelled = false; + let sessionId: string | undefined; + const release = (id: string) => { + void htmlPreviewApi.release(id).catch(reason => log.warn('Failed to release HTML preview', reason)); + }; setUrl(undefined); setError(undefined); void htmlPreviewApi.create({ filePath, workspaceId, - peerDeviceMode: isPeerDeviceModeActive(), }) .then((result) => { if (cancelled) { - void htmlPreviewApi.release(result.sessionId); + release(result.sessionId); return; } - sessionIdRef.current = result.sessionId; + sessionId = result.sessionId; setUrl(result.url); }) .catch((reason: unknown) => { - if (!cancelled) setError(reason instanceof Error ? reason.message : String(reason)); + if (!cancelled && !isSurfaceChangedError(reason)) setError(reason instanceof Error ? reason.message : String(reason)); }); return () => { cancelled = true; - if (sessionIdRef.current) { - void htmlPreviewApi.release(sessionIdRef.current); - sessionIdRef.current = undefined; - } + if (sessionId) release(sessionId); }; - // The session is intentionally recreated only when the opened file scope changes. }, [filePath, workspaceId]); if (error) { @@ -62,4 +63,9 @@ const HtmlPreviewPanel: React.FC = ({ return ; }; +const HtmlPreviewPanel: React.FC = (props) => { + const surface = useSyncExternalStore(onSurfaceActivated, getActiveSurfaceScope, getActiveSurfaceScope); + return ; +}; + export default HtmlPreviewPanel; diff --git a/src/web-ui/src/app/scenes/browser/README.md b/src/web-ui/src/app/scenes/browser/README.md index b88c92e5d4..424925479d 100644 --- a/src/web-ui/src/app/scenes/browser/README.md +++ b/src/web-ui/src/app/scenes/browser/README.md @@ -112,3 +112,32 @@ native pages and production session controls exercise background opening, delaye replacement, concurrent reads, repeated session switches, retained document/input state, and native visibility. It does not establish remote-host or cross-platform behavior. + +## Workspace HTML previews + +`HtmlPreviewPanel` opens a controller-owned loopback HTTP gateway. The gateway +reads the selected workspace through the shared filesystem service: local files, +SSH/SFTP files, or `html_preview_read_resource` on a Desktop/CLI peer. Peer hosts +must advertise `workspace_html_preview_v1`; older hosts receive an explicit +upgrade message. The browser remains on the controller, and a peer read never +falls back to controller-local files. + +The document URL retains its workspace-relative path so `./` and `../` references +keep working. Root-relative assets such as `/assets/app.js` first resolve beside +the selected HTML, then under the workspace root. This supports a nested `dist` +without breaking workspace-prefixed links. Resources stay inside the workspace; +remote symlinks are rejected. The service streams files up to 64 MiB in 256 KiB +chunks and checks workspace identity and file revision throughout a transfer. +Each preview has a distinct authorization cookie and is released when its panel +closes or its device activation changes. A disconnect returns a resource error; +reloading after reconnect retries the same workspace. + +Focused frontend coverage: + +```bash +pnpm --dir src/web-ui run test:run src/infrastructure/api/htmlPreviewApi.test.ts src/app/scenes/browser/HtmlPreviewPanel.test.tsx +``` + +Native resource loading is covered by `node tests/e2e/scripts/run-html-preview.mjs` +after building Desktop and Web UI. See the E2E guide for isolated storage and +binary/frontend snapshot overrides. diff --git a/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts b/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts index 35c7a8880a..541035dc5f 100644 --- a/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts +++ b/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts @@ -1,6 +1,6 @@ // Generated by scripts/generate-interactive-capabilities.mjs; do not edit. // Source: openbitfun_product_domains::remote_surface (Product Operation Registry). -export const REMOTE_SURFACE_REGISTRY_DIGEST = "fnv1a64:deaa9af9ff3f916e" as const; +export const REMOTE_SURFACE_REGISTRY_DIGEST = "fnv1a64:99d567cb0f1aa0ad" as const; /** * Registered Tauri commands the Peer Device controller keeps on the controller @@ -203,6 +203,7 @@ export const PEER_HOST_CAPABILITY_IDS = [ "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "whole_session_copy_v1", "user_question_response", "user_question_interaction_v1", @@ -248,6 +249,7 @@ export const PEER_HOST_ADVERTISED_CAPABILITIES: Readonly< "review_platform_v1", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "whole_session_copy_v1", "user_question_response", "user_question_interaction_v1", @@ -269,6 +271,7 @@ export const PEER_HOST_ADVERTISED_CAPABILITIES: Readonly< "tool_catalog", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "user_question_response", "user_question_interaction_v1", ], diff --git a/src/web-ui/src/infrastructure/api/htmlPreviewApi.test.ts b/src/web-ui/src/infrastructure/api/htmlPreviewApi.test.ts index 3c96c2aeae..aaeee51a98 100644 --- a/src/web-ui/src/infrastructure/api/htmlPreviewApi.test.ts +++ b/src/web-ui/src/infrastructure/api/htmlPreviewApi.test.ts @@ -1,16 +1,50 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { htmlPreviewApi } from './htmlPreviewApi'; -const invoke = vi.hoisted(() => vi.fn(async () => ({ sessionId: 'preview-1', url: 'http://localhost/preview' }))); +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; + +const invoke = vi.hoisted(() => vi.fn()); vi.mock('./service-api/ApiClient', () => ({ api: { invoke } })); -describe('HTML preview workspace identity', () => { - it('sends the saved workspace ID alongside the file IO operand', async () => { - const request = { workspaceId: 'remote-id', filePath: '/repo/index.html', peerDeviceMode: false }; +const request = { workspaceId: 'remote-id', filePath: '/repo/dist/index.html' }; +const result = { sessionId: 'preview-1', url: 'http://127.0.0.1:1234/preview' }; + +beforeEach(() => { + activateSurface('local'); + invoke.mockReset().mockResolvedValue(result); +}); + +describe('HTML preview workspace and device identity', () => { + it('keeps local and SSH previews on the controller using the saved workspace ID', async () => { + await htmlPreviewApi.create(request); + expect(invoke).toHaveBeenCalledWith('html_preview_create', { request: { ...request, peerDeviceMode: false } }); + }); + + it('binds peer resources to the selected device without copying paths into local IO', async () => { + activateSurface('linux-cli'); await htmlPreviewApi.create(request); - expect(invoke).toHaveBeenCalledWith('html_preview_create', { request }); + expect(invoke).toHaveBeenCalledWith('html_preview_create', { request: { ...request, peerDeviceMode: true, peerDeviceId: 'linux-cli' } }); }); + + it('releases a late gateway when device activation changes, including A to B to A', async () => { + activateSurface('peer-a'); + let resolve!: (value: typeof result) => void; + invoke.mockImplementationOnce(() => new Promise(done => { resolve = done; })); + const pending = htmlPreviewApi.create(request); + activateSurface('peer-b'); + activateSurface('peer-a'); + resolve(result); + await expect(pending).rejects.toMatchObject({ isSurfaceChangedError: true }); + expect(invoke).toHaveBeenLastCalledWith('html_preview_release', { request: { sessionId: result.sessionId } }); + }); + it('rejects a missing ID before invoking the host', async () => { - invoke.mockClear(); - await expect(htmlPreviewApi.create({ workspaceId: '', filePath: '/repo/index.html', peerDeviceMode: false })).rejects.toThrow('workspace ID'); + await expect(htmlPreviewApi.create({ ...request, workspaceId: '' })).rejects.toThrow('workspace ID'); expect(invoke).not.toHaveBeenCalled(); }); + + it('preserves an explicit old-peer or offline failure without a local retry', async () => { + activateSurface('old-peer'); + invoke.mockRejectedValueOnce(new Error('Update OpenBitFun on the controlled device')); + await expect(htmlPreviewApi.create(request)).rejects.toThrow('Update OpenBitFun'); + expect(invoke).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/web-ui/src/infrastructure/api/htmlPreviewApi.ts b/src/web-ui/src/infrastructure/api/htmlPreviewApi.ts index 80d0ea342c..9a7e8f5e77 100644 --- a/src/web-ui/src/infrastructure/api/htmlPreviewApi.ts +++ b/src/web-ui/src/infrastructure/api/htmlPreviewApi.ts @@ -1,9 +1,12 @@ import { api } from './service-api/ApiClient'; +import { getActiveSurfaceScope, isLocalSurface } from '@/infrastructure/peer-device/deviceSurface'; +import { createLogger } from '@/shared/utils/logger'; + +const log = createLogger('HtmlPreviewAPI'); export interface HtmlPreviewCreateRequest { filePath: string; workspaceId: string; - peerDeviceMode: boolean; } export interface HtmlPreviewCreateResponse { @@ -12,9 +15,24 @@ export interface HtmlPreviewCreateResponse { } export const htmlPreviewApi = { - create(request: HtmlPreviewCreateRequest): Promise { - if (!request.workspaceId) return Promise.reject(new Error('HTML preview workspace ID is required')); - return api.invoke('html_preview_create', { request }); + async create(request: HtmlPreviewCreateRequest): Promise { + if (!request.workspaceId) throw new Error('HTML preview workspace ID is required'); + const scope = getActiveSurfaceScope(); + const peerDeviceMode = !isLocalSurface(scope.surfaceId); + const result = await api.invoke('html_preview_create', { + request: { + ...request, + peerDeviceMode, + ...(peerDeviceMode ? { peerDeviceId: scope.surfaceId } : {}), + }, + }); + // The gateway is controller-local, so ApiClient intentionally does not fence it. + // A late creation must still close its local listener after a surface switch. + if (!scope.isCurrent()) { + await htmlPreviewApi.release(result.sessionId).catch(error => log.warn('Failed to release stale HTML preview', error)); + scope.assertCurrent('HTML preview'); + } + return result; }, release(sessionId: string): Promise { return api.invoke('html_preview_release', { request: { sessionId } }); diff --git a/src/web-ui/src/infrastructure/peer-device/README.md b/src/web-ui/src/infrastructure/peer-device/README.md index afacbb03f8..be028e26fa 100644 --- a/src/web-ui/src/infrastructure/peer-device/README.md +++ b/src/web-ui/src/infrastructure/peer-device/README.md @@ -272,3 +272,18 @@ listener alone recovers an interaction emitted before attachment. restored unsupported panel keeps its URL available in the system browser without querying the host. Overview requests and panel state belong to the surface activation even when two devices share a workspace ID/path. + +### Workspace HTML resources + +The controller keeps HTML preview HTTP listeners and native browser views local. +The selected peer supplies workspace resources over the existing encrypted +HostInvoke channel, negotiated with `workspace_html_preview_v1` on Desktop and +CLI. Requests carry the peer's workspace ID, entry path, source binding, revision +and chunk offset; controller paths are never reused on the peer. Releasing a +preview cancels in-flight HTTP reads. Device activation changes release old +listeners and ignore late creates. Older peers must be upgraded to supply these +resources; there is no local filesystem fallback. + +CLI file browsing handles both current `explorer_get_*` commands and the legacy +`get_directory_children*` / `get_file_tree` names through the same workspace-aware +filesystem service. Registry closure tests keep the advertised handlers in sync. diff --git a/tests/e2e/AGENTS.md b/tests/e2e/AGENTS.md index a025ca95aa..9431548335 100644 --- a/tests/e2e/AGENTS.md +++ b/tests/e2e/AGENTS.md @@ -155,3 +155,16 @@ after overlay rendering settles, not from a recreated mock page. The runner records the copied binary SHA-256 and frontend revision, then removes only its copied binary/frontend after stopping the app. Logs, result and the isolated profile remain; set `OPENBITFUN_E2E_KEEP_SNAPSHOT=1` to retain the copies. + +For workspace HTML preview resource loading, build Desktop and Web UI, then run: + +```bash +node tests/e2e/scripts/run-html-preview.mjs +``` + +The native browser fixture uses isolated product storage and completed binary / +frontend snapshots. It verifies nested build output with root-relative CSS, +relative JavaScript modules and dynamic imports, parent-relative styles, images, +encoded filenames, and reloading one preview after opening another. Inspect its +`result.json`; this is native local browser coverage. The services-integrations +OpenSSH fixture separately covers the SSH/SFTP resource provider. diff --git a/tests/e2e/scripts/run-html-preview.mjs b/tests/e2e/scripts/run-html-preview.mjs new file mode 100644 index 0000000000..3e1b935235 --- /dev/null +++ b/tests/e2e/scripts/run-html-preview.mjs @@ -0,0 +1,154 @@ +/** Native regression for workspace HTML resources in actual native browser child WebViews. */ +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { once } from 'node:events'; +import { cp, mkdir, mkdtemp, open, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { createServer } from 'node:net'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const repo = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../..'); +const root = await mkdtemp(path.join(tmpdir(), 'openbitfun-html-preview-')); +const userRoot = path.join(root, 'user'); +const productHome = path.join(root, 'home'); +const frontend = path.join(root, 'frontend'); +const frontendSource = path.resolve(process.env.OPENBITFUN_E2E_FRONTEND_DIR || path.join(repo, 'dist')); +const manifest = await readFile(path.join(frontendSource, 'frontend-revision.json'), 'utf8'); +await cp(frontendSource, frontend, { recursive: true }); +assert.equal(await readFile(path.join(frontendSource, 'frontend-revision.json'), 'utf8'), manifest, + 'The frontend changed during the snapshot. Finish the build and rerun.'); +await mkdir(userRoot, { recursive: true }); +await mkdir(productHome, { recursive: true }); +const binaryName = 'openbitfun-desktop' + (process.platform === 'win32' ? '.exe' : ''); +const binarySource = path.resolve(process.env.OPENBITFUN_E2E_DESKTOP_BINARY || path.join(repo, 'target/debug', binaryName)); +const binary = path.join(root, binaryName); +const before = await stat(binarySource); +await cp(binarySource, binary); +const after = await stat(binarySource); +assert.equal(before.mtimeMs, after.mtimeMs, 'The Desktop binary changed during the snapshot. Rerun after the build.'); +assert.equal(before.size, after.size); + +const probe = createServer(); +probe.listen(0, '127.0.0.1'); +await once(probe, 'listening'); +const port = probe.address().port; +await new Promise(resolve => probe.close(resolve)); +const endpoint = 'http://127.0.0.1:' + port; +const log = await open(path.join(root, 'desktop.log'), 'w'); +const app = spawn(binary, [], { + cwd: repo, windowsHide: true, stdio: ['ignore', log.fd, log.fd], + env: { ...process.env, + OPENBITFUN_USER_ROOT: userRoot, OPENBITFUN_E2E_USER_ROOT: userRoot, + OPENBITFUN_HOME: productHome, OPENBITFUN_E2E_HOME: productHome, + OPENBITFUN_E2E_STORAGE_GUARD: '1', OPENBITFUN_E2E_PACKAGED_FRONTEND: '1', + OPENBITFUN_E2E_FRONTEND_DIR: frontend, OPENBITFUN_E2E_LOG_DIR: path.join(root, 'logs'), + OPENBITFUN_WEBDRIVER_PORT: String(port), OPENBITFUN_WEBDRIVER_LABEL: 'main', + WEBVIEW2_USER_DATA_FOLDER: path.join(root, 'webview'), + }, +}); +console.log('Native HTML preview evidence: ' + root); +let exitError; +app.on('error', error => { exitError = error; }); +let session; +const report = { passed: false, platform: process.platform, binarySource, + binaryModified: before.mtime.toISOString(), frontendRevision: JSON.parse(manifest).revision, checks: [] }; +async function request(route, body, method = body === undefined ? 'GET' : 'POST') { + const response = await fetch(endpoint + route, { + method, signal: AbortSignal.timeout(15_000), + ...(body === undefined ? {} : { headers: { 'content-type': 'application/json' }, body: JSON.stringify(body) }), + }); + const result = await response.json(); + if (!response.ok || (result.value?.error && result.value?.ok === undefined)) throw new Error(JSON.stringify(result)); + return result.value; +} +async function until(read, description, timeout = 45_000) { + const started = Date.now(); + let last; + while (Date.now() - started < timeout) { + if (exitError) throw exitError; + if (app.exitCode !== null || app.signalCode !== null) throw new Error('Desktop exited before ' + description); + try { if (await read()) return; } catch (error) { last = error; } + await new Promise(resolve => setTimeout(resolve, 250)); + } + throw new Error('Timed out waiting for ' + description + ': ' + (last ?? 'not ready')); +} +const execute = (fn, ...args) => request('/session/' + session + '/execute/sync', { script: fn.toString(), args }); +const invokeResult = (command, args = {}) => execute(async (command, args) => { + try { return { ok: true, value: await window.__TAURI_INTERNALS__.invoke(command, args) }; } + catch (error) { return { ok: false, error: String(error) }; } +}, command, args); +const invoke = (command, args = {}) => invokeResult(command, args).then(result => { + assert.equal(result.ok, true, command + ': ' + result.error); + return result.value; +}); +const workspace = path.join(root, 'workspace'); +for (const [directory, color, marker] of [['site/dist', 'rgb(23, 81, 139)', 'first'], ['other/build', 'rgb(163, 45, 18)', 'second']]) { + const site = path.join(workspace, directory); + await mkdir(path.join(site, 'assets'), { recursive: true }); + await writeFile(path.join(site, 'index #1.html'), '
waiting
'); + await writeFile(path.join(site, 'assets/app.css'), '#result { color: ' + color + '; }'); + await writeFile(path.join(site, '../shared.css'), '#result { letter-spacing: 3px; }'); + await writeFile(path.join(site, 'assets/main.mjs'), 'const { marker } = await import("./chunk.mjs"); document.querySelector("#result").textContent = marker;'); + await writeFile(path.join(site, 'assets/chunk.mjs'), 'export const marker = ' + JSON.stringify(marker) + ';'); + await writeFile(path.join(site, 'assets/pixel.png'), Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jRZkAAAAASUVORK5CYII=', 'base64')); +} +const previews = []; +async function checkPage(label, marker, color) { + await request('/session/' + session + '/window', { handle: label }); + try { + await until(() => execute((marker, color) => { + const result = document.querySelector('#result'); + const image = document.querySelector('#image'); + return result?.textContent === marker && getComputedStyle(result).color === color + && getComputedStyle(result).letterSpacing === '3px' && image?.complete && image.naturalWidth === 1; + }, marker, color), 'HTML, CSS, module import and image for ' + marker); + const details = await execute(() => ({ url: location.href, text: document.querySelector('#result').textContent, + color: getComputedStyle(document.querySelector('#result')).color, + resources: performance.getEntriesByType('resource').map(entry => entry.name) })); + assert.ok(!details.url.includes('previewToken='), 'Bootstrap token must leave the document URL'); + assert.ok(details.url.endsWith('/index%20%231.html')); + report.checks.push(details); + } finally { + await request('/session/' + session + '/window', { handle: 'main' }); + } +} +try { + await until(async () => (await request('/status')).ready, 'embedded driver'); + session = (await request('/session', {})).sessionId; + await request('/session/' + session + '/timeouts', { script: 10000 }); + await until(() => execute(() => !!window.__TAURI_INTERNALS__), 'main frontend'); + const opened = await invoke('open_workspace', { request: { path: workspace, createLocal: true } }); + for (const [directory, marker, color] of [['site/dist', 'first', 'rgb(23, 81, 139)'], ['other/build', 'second', 'rgb(163, 45, 18)']]) { + const preview = await invoke('html_preview_create', { request: { workspaceId: opened.id, + filePath: path.join(workspace, directory, 'index #1.html') } }); + previews.push(preview); + const label = 'embedded-browser-panel-view-html-' + marker; + await invoke('browser_webview_create', { request: { label, url: preview.url, x: 350, y: 80, width: 480, height: 320 } }); + await checkPage(label, marker, color); + } + // The second preview must not replace the first preview's authorization cookie. + await request('/session/' + session + '/window', { handle: 'embedded-browser-panel-view-html-first' }); + await request('/session/' + session + '/refresh', {}); + await request('/session/' + session + '/window', { handle: 'main' }); + await checkPage('embedded-browser-panel-view-html-first', 'first', 'rgb(23, 81, 139)'); + for (const preview of previews) await invoke('html_preview_release', { request: { sessionId: preview.sessionId } }); + report.passed = true; +} catch (error) { + report.error = String(error); + throw error; +} finally { + await writeFile(path.join(root, 'result.json'), JSON.stringify(report, null, 2)); + if (session) await request('/session/' + session, undefined, 'DELETE').catch(() => {}); + if (app.exitCode === null && app.signalCode === null) { + app.kill(); + await Promise.race([once(app, 'exit'), new Promise(resolve => setTimeout(resolve, 5000))]); + if (app.exitCode === null && app.signalCode === null) app.kill('SIGKILL'); + } + await log.close(); + if (process.env.OPENBITFUN_E2E_KEEP_SNAPSHOT !== '1') { + await rm(binary, { force: true }); + await rm(frontend, { recursive: true, force: true }); + } + console.log(JSON.stringify({ ...report, artifactRoot: root })); +} From 6ed7b2e01e1b6e6d21bc2e781dc7d3c042120329 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 12:29:51 +0800 Subject: [PATCH 11/12] fix(cli): resolve legacy file requests using their operation path --- .../cli/src/peer_host/commands/filesystem.rs | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/apps/cli/src/peer_host/commands/filesystem.rs b/src/apps/cli/src/peer_host/commands/filesystem.rs index 6a0fec7c7d..08ce89fc05 100644 --- a/src/apps/cli/src/peer_host/commands/filesystem.rs +++ b/src/apps/cli/src/peer_host/commands/filesystem.rs @@ -15,13 +15,18 @@ pub(crate) async fn html_preview_read_resource(args: &Value) -> Result Result, String> { - directory_connection(&state.workspace_service, request).await +async fn file_connection( + state: &PeerHostState, + request: &Value, + path: &str, +) -> Result, String> { + directory_connection(&state.workspace_service, request, path).await } async fn directory_connection( workspaces: &WorkspaceService, request: &Value, + path: &str, ) -> Result, String> { if let Some(id) = optional_string(request, "workspaceId").or_else(|| optional_string(request, "workspace_id")) @@ -36,7 +41,7 @@ async fn directory_connection( } workspaces .upgrade_legacy_file_connection( - &get_string(request, "path")?, + path, optional_string(request, "workspacePath").as_deref(), optional_string(request, "remoteConnectionId").as_deref(), ) @@ -69,7 +74,7 @@ pub(crate) async fn get_directory_children( ) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let preferred = directory_connection(workspaces, request).await?; + let preferred = directory_connection(workspaces, request, &path).await?; let nodes = files .get_directory_contents_on_connection(&path, preferred.as_deref()) .await @@ -84,7 +89,7 @@ pub(crate) async fn get_directory_children_paginated( ) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let preferred = directory_connection(workspaces, request).await?; + let preferred = directory_connection(workspaces, request, &path).await?; let offset = request.get("offset").and_then(|v| v.as_u64()).unwrap_or(0) as usize; let limit = request.get("limit").and_then(|v| v.as_u64()).unwrap_or(100) as usize; @@ -117,7 +122,7 @@ pub(crate) async fn get_file_tree( ) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let preferred = directory_connection(workspaces, request).await?; + let preferred = directory_connection(workspaces, request, &path).await?; let nodes = files .build_file_tree_on_connection(&path, preferred.as_deref()) .await @@ -134,6 +139,7 @@ pub(crate) async fn get_file_tree( path.trim_end_matches('/') .rsplit('/') .next() + .filter(|name| !name.is_empty()) .unwrap_or(&path) } else { std::path::Path::new(&path) @@ -154,7 +160,7 @@ pub(crate) async fn check_path_exists( ) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let hint = file_connection(state, request).await?; + let hint = file_connection(state, request, &path).await?; openbitfun_core::service::filesystem::path_operations::exists( &state.filesystem_service, &path, @@ -167,7 +173,7 @@ pub(crate) async fn check_path_exists( pub(crate) async fn create_directory(state: &PeerHostState, args: &Value) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let hint = file_connection(state, request).await?; + let hint = file_connection(state, request, &path).await?; openbitfun_core::service::filesystem::path_operations::create_directory( &state.filesystem_service, &path, @@ -183,7 +189,7 @@ pub(crate) async fn read_file_content( ) -> Result { let request = request_value(args); let path = get_string(request, "filePath")?; - let hint = file_connection(state, request).await?; + let hint = file_connection(state, request, &path).await?; if let Some(encoding) = optional_string(request, "encoding") { if !encoding.eq_ignore_ascii_case("utf-8") && !encoding.eq_ignore_ascii_case("utf8") { return Err(format!("Unsupported text encoding: {encoding}")); @@ -205,7 +211,7 @@ pub(crate) async fn write_file_content( let request = request_value(args); let path = get_string(request, "filePath")?; let content = get_string(request, "content")?; - let hint = file_connection(state, request).await?; + let hint = file_connection(state, request, &path).await?; openbitfun_core::service::filesystem::path_operations::write_text_checked( &state.filesystem_service, &path, @@ -224,7 +230,7 @@ pub(crate) async fn rename_file(state: &PeerHostState, args: &Value) -> Result Result { let request = request_value(args); let path = get_string(request, "path")?; - let hint = file_connection(state, request).await?; + let hint = file_connection(state, request, &path).await?; let recursive = request .get("recursive") .and_then(Value::as_bool) @@ -264,7 +270,7 @@ pub(crate) async fn list_directory_files( ) -> Result { let request = request_value(args); let path = get_string(request, "path")?; - let hint = file_connection(state, request).await?; + let hint = file_connection(state, request, &path).await?; let extensions = request.get("extensions").and_then(Value::as_array); let nodes = state .filesystem_service @@ -363,6 +369,16 @@ mod tests { .find(|node| node["name"] == "nested") .unwrap(); assert_eq!(nested["children"][0]["name"], "child.js"); + // Legacy file operations name their path differently from directory reads. + // Their selected host-local provider must not require a second `path` field. + for field in ["filePath", "oldPath"] { + let legacy = json!({field: root.join("alpha.txt"), "remoteConnectionId": ""}); + let selected = + directory_connection(&workspaces, &legacy, legacy[field].as_str().unwrap()) + .await + .unwrap(); + assert!(selected.is_none()); + } let mut invalid = args; invalid["request"]["workspaceId"] = json!("missing-workspace"); assert!(get_directory_children(&files, &workspaces, &invalid) From 1dcc09437d250ad845357c8875e3521cb16f0d46 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Wed, 28 Oct 2026 12:31:50 +0800 Subject: [PATCH 12/12] test(remote): keep preview fixture paths POSIX on Windows --- .../src/remote_connect/html_preview/tests.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs b/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs index bff2106dff..be39ffb174 100644 --- a/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs +++ b/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs @@ -199,11 +199,21 @@ async fn nested_site_serves_absolute_relative_css_imports_and_binary_assets() { } #[tokio::test] -async fn remote_preview_never_reads_same_named_controller_files_or_falls_back_offline() { +async fn remote_preview_uses_its_provider_and_never_falls_back_offline() { let directory = tempfile::tempdir().unwrap(); - let root = directory.path().to_string_lossy().into_owned(); + std::fs::write( + directory.path().join("index.html"), + b"private-controller-content", + ) + .unwrap(); + // Unix can collide on the exact absolute path. Windows controller paths + // are a different namespace; the remote fixture must still remain POSIX. + let root = if cfg!(unix) { + directory.path().to_string_lossy().into_owned() + } else { + "/remote/preview".to_string() + }; let path = format!("{root}/index.html"); - std::fs::write(&path, b"private-controller-content").unwrap(); let fs = Arc::new(RemoteFs::default()); fs.file(&path, b"remote-page"); let source = HtmlPreviewSource::new(