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/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/filesystem.rs b/src/apps/cli/src/peer_host/commands/filesystem.rs index 7422cef735..08ce89fc05 100644 --- a/src/apps/cli/src/peer_host/commands/filesystem.rs +++ b/src/apps/cli/src/peer_host/commands/filesystem.rs @@ -1,16 +1,37 @@ //! 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; -async fn file_connection(state: &PeerHostState, request: &Value) -> Result, String> { +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, + 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")) { - let record = state - .workspace_service + let record = workspaces .require_workspace(&id) .await .map_err(|e| e.to_string())?; @@ -18,10 +39,9 @@ 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, &path).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 +83,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, &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; - 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,13 +115,52 @@ 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, &path).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() + .filter(|name| !name.is_empty()) + .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, ) -> 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, @@ -114,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, @@ -130,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}")); @@ -152,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, @@ -171,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) @@ -211,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 @@ -257,3 +316,74 @@ 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"); + // 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) + .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/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/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/adapters/ai-adapters/src/client/healthcheck.rs b/src/crates/adapters/ai-adapters/src/client/healthcheck.rs index 0578e29fd4..aca52f532f 100644 --- a/src/crates/adapters/ai-adapters/src/client/healthcheck.rs +++ b/src/crates/adapters/ai-adapters/src/client/healthcheck.rs @@ -589,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")), 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/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()) 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 8e3a1f7bcf..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:e0ac67cc604f9730", + "digest": "fnv1a64:99d567cb0f1aa0ad", "retiredCommandPrefixes": [ { "prefix": "lsp_", @@ -39,8 +39,10 @@ "product_control_presentation_v1", "cancel_tool", "tool_catalog", + "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" @@ -76,8 +78,10 @@ "product_control_presentation_v1", "cancel_tool", "tool_catalog", + "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" @@ -99,6 +103,7 @@ "tool_catalog", "chat_mcp_catalog_v1", "workspace_id_references_v1", + "workspace_html_preview_v1", "user_question_response", "user_question_interaction_v1" ] @@ -2238,8 +2243,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host does not expose the remote-workspace UI surface" + "kind": "handled" } }, { @@ -2250,8 +2254,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host does not expose the remote-workspace UI surface" + "kind": "handled" } }, { @@ -2262,8 +2265,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host does not expose the remote-workspace UI surface" + "kind": "handled" } }, { @@ -2800,8 +2802,7 @@ "kind": "proxied" }, "cliPeer": { - "kind": "unsupported", - "reason": "the CLI peer host has no handler for this command" + "kind": "handled" } }, { @@ -3973,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 e2e83c1e78..1c5c432e9f 100644 --- a/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs +++ b/src/crates/contracts/product-domains/src/remote_surface/capabilities.rs @@ -81,10 +81,14 @@ 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. 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. @@ -126,8 +130,10 @@ impl PeerHostCapability { Self::ProductControlPresentationV1, Self::CancelTool, Self::ToolCatalog, + Self::ReviewPlatformV1, Self::ChatMcpCatalogV1, Self::WorkspaceIdReferencesV1, + Self::WorkspaceHtmlPreviewV1, Self::WholeSessionCopyV1, Self::UserQuestionResponse, Self::UserQuestionInteractionV1, @@ -166,8 +172,10 @@ 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::WorkspaceHtmlPreviewV1 => "workspace_html_preview_v1", Self::WholeSessionCopyV1 => "whole_session_copy_v1", Self::UserQuestionResponse => "user_question_response", Self::UserQuestionInteractionV1 => "user_question_interaction_v1", @@ -201,6 +209,7 @@ const CLI_CAPABILITIES: &[PeerHostCapability] = &[ PeerHostCapability::ToolCatalog, PeerHostCapability::ChatMcpCatalogV1, PeerHostCapability::WorkspaceIdReferencesV1, + PeerHostCapability::WorkspaceHtmlPreviewV1, PeerHostCapability::UserQuestionResponse, PeerHostCapability::UserQuestionInteractionV1, ]; @@ -294,6 +303,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/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..be39ffb174 --- /dev/null +++ b/src/crates/services/services-integrations/src/remote_connect/html_preview/tests.rs @@ -0,0 +1,357 @@ +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_uses_its_provider_and_never_falls_back_offline() { + let directory = tempfile::tempdir().unwrap(); + 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"); + 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/AGENTS.md b/src/web-ui/AGENTS.md index 08861f7bef..5b4dedaeb7 100644 --- a/src/web-ui/AGENTS.md +++ b/src/web-ui/AGENTS.md @@ -257,3 +257,14 @@ 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 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, 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. 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..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,20 +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(), 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(), 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, 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: {} })); @@ -86,14 +99,149 @@ 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); + 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'); }); + + 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.messages.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('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')); + 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); }); - afterEach(async () => { await act(async () => root.unmount()); host.remove(); }); 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 +255,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..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 @@ -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, isLocalSurface, onSurfaceActivated, surfaceScopedKey, type DeviceSurfaceId, type SurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { useReviewPlatformSupport } from '@/infrastructure/peer-device/useReviewPlatformSupport'; import { currentPullRequestReviewStatusText, effectivePullRequestReviewFreshness, @@ -253,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. */ @@ -262,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; @@ -280,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 { @@ -712,14 +717,48 @@ 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.messages.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({ + surface, workspacePath, workspaceId, initialRemoteId, initialPullRequestId, initialPullRequestUrl, detailOnly = false, -}) => { +}: ReviewPlatformPanelProps & { surface: SurfaceScope }) { const { t } = useI18n('panels/git'); const { t: tReview } = useI18n('flow-chat'); const authFormId = useId(); @@ -733,6 +772,17 @@ export const ReviewPlatformPanel: React.FC = ({ 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); @@ -849,6 +899,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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; @@ -868,7 +919,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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; @@ -902,7 +953,9 @@ export const ReviewPlatformPanel: React.FC = ({ 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, @@ -911,15 +964,16 @@ export const ReviewPlatformPanel: React.FC = ({ 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); @@ -930,17 +984,18 @@ export const ReviewPlatformPanel: React.FC = ({ 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 || ''; @@ -972,7 +1027,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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); } @@ -980,18 +1035,18 @@ export const ReviewPlatformPanel: React.FC = ({ 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') { @@ -1014,6 +1069,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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); @@ -1049,7 +1105,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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 }); @@ -1059,15 +1115,15 @@ export const ReviewPlatformPanel: React.FC = ({ 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'; @@ -1133,7 +1189,7 @@ export const ReviewPlatformPanel: React.FC = ({ if (nextRemoteId && selectedRemoteId !== nextRemoteId) { setSelectedRemoteId(nextRemoteId); - rememberRemote(workspaceId, nextRemoteId); + rememberRemote(surface.surfaceId, workspaceId, nextRemoteId); } if (selectedPrId !== targetPullRequestId) { @@ -1144,6 +1200,7 @@ export const ReviewPlatformPanel: React.FC = ({ detailOnly, initialPullRequestId, initialPullRequestTarget, + surface.surfaceId, initialPullRequestUrl, initialRemoteId, selectedPrId, @@ -1364,10 +1421,10 @@ export const ReviewPlatformPanel: React.FC = ({ 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'); @@ -1460,6 +1517,7 @@ export const ReviewPlatformPanel: React.FC = ({ }, []); const loadCiLog = useCallback(async (item: ReviewPlatformCiItem): Promise => { + if (!isPanelCurrent()) return null; const cached = ciLogById[item.id]; if (cached) return cached; if (!canLoadCiLog(selectedRemote, item)) { @@ -1493,20 +1551,22 @@ export const ReviewPlatformPanel: React.FC = ({ 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)) { @@ -1534,12 +1594,14 @@ export const ReviewPlatformPanel: React.FC = ({ 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', @@ -1557,7 +1619,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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; @@ -1577,6 +1639,7 @@ export const ReviewPlatformPanel: React.FC = ({ }, [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, @@ -1596,6 +1659,7 @@ export const ReviewPlatformPanel: React.FC = ({ selectedRemote.id, selectedPr.id, ); + if (!isPanelCurrent()) return; const freshPullRequest = reviewTarget.pullRequest; const freshIdentity = { platform: selectedRemote.platform, @@ -1648,6 +1712,7 @@ export const ReviewPlatformPanel: React.FC = ({ repository, reviewTarget, }); + if (!isPanelCurrent()) return; if (prepared.mode === 'strict' && prepared.requiresConsent) { const confirmed = await confirmDeepReviewLaunch(prepared.runManifest, { sessionConcurrencyGuard: deriveDeepReviewSessionConcurrencyGuard( @@ -1655,7 +1720,7 @@ export const ReviewPlatformPanel: React.FC = ({ parentSession.sessionId, ), }); - if (!confirmed) return; + if (!isPanelCurrent() || !confirmed) return; } if (runningReviewExists() || reviewLaunchesInFlight.has(sharedLaunchKey)) { return; @@ -1669,12 +1734,14 @@ export const ReviewPlatformPanel: React.FC = ({ 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, @@ -1688,9 +1755,10 @@ export const ReviewPlatformPanel: React.FC = ({ reviewLaunchesInFlight.delete(sharedLaunchKey); } reviewLaunchInFlight.current = false; - setReviewLaunching(false); + if (isPanelCurrent()) setReviewLaunching(false); } }, [ + isPanelCurrent, confirmDeepReviewLaunch, tReview, latestCurrentReview?.lifecycle, @@ -1757,11 +1825,12 @@ export const ReviewPlatformPanel: React.FC = ({ }, [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(''); @@ -1770,6 +1839,7 @@ export const ReviewPlatformPanel: React.FC = ({ }, [setAuthError]); const handleSaveAuthToken = useCallback(async () => { + if (!isPanelCurrent()) return; if (!selectedRemote || selectedRemote.platform === 'unknown' || selectedRemote.platform === 'github') return; const token = authToken.trim(); if (!token) { @@ -1785,50 +1855,59 @@ export const ReviewPlatformPanel: React.FC = ({ 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); @@ -1837,15 +1916,17 @@ export const ReviewPlatformPanel: React.FC = ({ 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; @@ -2079,7 +2160,7 @@ export const ReviewPlatformPanel: React.FC = ({ 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'); @@ -2193,6 +2274,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' ? (
= ( 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..44e62ce5e4 --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatHttpLinkClick.ts @@ -0,0 +1,36 @@ +import { useCallback } from 'react'; +import { systemAPI } from '@/infrastructure/api/service-api/SystemAPI'; +import { useReviewPlatformSupport } from '@/infrastructure/peer-device/useReviewPlatformSupport'; +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 canViewPullRequests = useReviewPlatformSupport(); + + 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/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..9449922514 --- /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.messages.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..afc1ff77cd 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.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/infrastructure/api/generated/remoteSurface.ts b/src/web-ui/src/infrastructure/api/generated/remoteSurface.ts index 25878df83d..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:e0ac67cc604f9730" as const; +export const REMOTE_SURFACE_REGISTRY_DIGEST = "fnv1a64:99d567cb0f1aa0ad" as const; /** * Registered Tauri commands the Peer Device controller keeps on the controller @@ -200,8 +200,10 @@ 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", + "workspace_html_preview_v1", "whole_session_copy_v1", "user_question_response", "user_question_interaction_v1", @@ -244,8 +246,10 @@ 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", + "workspace_html_preview_v1", "whole_session_copy_v1", "user_question_response", "user_question_interaction_v1", @@ -267,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/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..be028e26fa 100644 --- a/src/web-ui/src/infrastructure/peer-device/README.md +++ b/src/web-ui/src/infrastructure/peer-device/README.md @@ -259,3 +259,31 @@ 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. + 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. + +### 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/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, 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..03ca18aea8 100644 --- a/src/web-ui/src/locales/en-US/common.json +++ b/src/web-ui/src/locales/en-US/common.json @@ -2067,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 40edda8334..b4f4beda9d 100644 --- a/src/web-ui/src/locales/zh-CN/common.json +++ b/src/web-ui/src/locales/zh-CN/common.json @@ -2067,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 d5ccd2eaf9..a3d2c5b05f 100644 --- a/src/web-ui/src/locales/zh-TW/common.json +++ b/src/web-ui/src/locales/zh-TW/common.json @@ -2067,6 +2067,7 @@ "targetIsPullRequest": "此項目是提取請求，請以提取請求開啟，而非以問題開啟。" }, "messages": { + "hostUnsupported": "目前裝置不支援內建 PR 檢視，請使用系統瀏覽器開啟 PR 連結。", "noWorkspace": "沒有可用的作用中工作區。", "unsupportedLink": "此連結不是支援的提取請求連結。", "detailsFailed": "載入提取請求詳細資料失敗。", 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 })); +}