diff --git a/docs/architecture/remote-workspace-transport.md b/docs/architecture/remote-workspace-transport.md index 1fa507c2a7..28ad3c833b 100644 --- a/docs/architecture/remote-workspace-transport.md +++ b/docs/architecture/remote-workspace-transport.md @@ -291,6 +291,65 @@ selection, or cache ownership. UI events retain workspace IDs through every adapter; session operations recover their workspace from the session's stored ID. Across devices, IDs are interpreted only by the selected owning host. +Request ownership also includes the device activation epoch. In the Web UI, +`invokePrepared` captures `SurfaceScope` before asynchronous parameter preparation +(including legacy capability negotiation), checks it before dispatch, and preserves +`SurfaceChangedError` through service error translation. `ApiClient` retains the +same activation through middleware, transport, retries and response handling. +Pending reads belong to one epoch through scoped keys or activation-owned caches; +returning to the same device does not revive an earlier activation's pending work. +Settled caches are keyed by the rendered device as well as the workspace ID, +because same-path local workspaces hash to the same ID on every device. +Multi-step preparation must +check the captured scope before starting another host request. Stream listeners +are detached on activation change and must never send cancellation to the newly +selected host for a search started elsewhere. Controller-local commands keep +their authority across activation in `invokePrepared` exactly as in `ApiClient`. +The Web UI lint configuration rejects `await` inside `api.invoke(...)` +arguments, because the activation would be captured after they resolve. + +`CoreSessionStorePort` owns session storage resolution. The temporary path adapter +converts a legacy selector to a catalog ID once, then uses the same ID resolver as +current requests. It must not reconstruct identity from local filesystem +existence, choose a worktree's parent when its execution record is missing, or +return an execution path when resolution fails. A registered but unavailable +folder can still own readable persisted history. + +After session admission, `SessionManager` retains the committed storage binding. +History restore, persistence, autosave, idle eviction and internal continuations +reuse that binding. The binding survives in-memory session eviction; a process +restart re-admits the session from its persisted ID and storage owner. Internal +queued turns retain the session's workspace ID, while external legacy submissions +still validate their locator at the compatibility boundary. All resolution uses +the persistence owner's `PathManager`. Readers never commit a binding: before +admission they resolve from the session's workspace configuration, and a pending +claim for a different location makes them fail instead of following an +uncommitted index entry. + +The supported SSH history layout remains host plus remote root for upgrade +compatibility, so two saved connections to one host and root share a workspace +record and session mirror. Activation never rejects such records: imported or +persisted records for each connection stay listed and activatable, and session +identity verification, not workspace activation, keeps their histories apart. + +Reopening an existing remote record with a different connection rebinds the +record only for an allowed reason: + +- the two connection IDs are equivalent (the legacy `ssh-user@host:port` form and + the current `ssh-user@host` form); +- the previous owner is no longer a saved SSH connection; +- the user confirmed the rebind, sent as `rebindConnection` on + `open_remote_workspace` by Desktop and the CLI peer host. + +Otherwise the open fails with the stable code +`remote_workspace_connection_conflict` as the whole error message, so remote +controllers can match it. The interactive Web UI asks the user and retries with +`rebindConnection`; startup restore defers with a localized notification instead +of rebinding silently. Older hosts ignore the field and keep their previous +behavior. Supporting multiple simultaneous endpoints for one host and root +requires a versioned storage-identity migration covering sessions and mirrors; +changing the directory hash alone is not a safe migration. + Persisted IDs are opaque. Catalog validation checks record/map-key agreement and reference integrity; it must not recompute IDs from paths or require a working SSH profile. Keep unavailable records in the catalog. Activation validates the diff --git a/src/apps/cli/src/peer_host/commands/workspace.rs b/src/apps/cli/src/peer_host/commands/workspace.rs index 07388a423c..d7ea5615e2 100644 --- a/src/apps/cli/src/peer_host/commands/workspace.rs +++ b/src/apps/cli/src/peer_host/commands/workspace.rs @@ -2,10 +2,13 @@ use std::path::PathBuf; +use openbitfun_core::service::workspace::{ + remote_workspace_connection_conflict_message, RemoteConnectionRebind, +}; use openbitfun_runtime_ports::SessionStoragePathRequest; use serde_json::{json, Value}; -use crate::peer_host::args::{get_string, request_value}; +use crate::peer_host::args::{get_string, optional_bool, optional_string, request_value}; use crate::peer_host::state::PeerHostState; use crate::peer_host::workspace_dto::{workspace_info_to_json, workspace_list_to_json}; @@ -113,7 +116,12 @@ pub(crate) async fn open_remote_workspace( let request = request_value(args); let path = get_string(request, "remotePath")?; let connection_id = get_string(request, "connectionId")?; - let host = crate::peer_host::args::optional_string(request, "sshHost"); + let host = optional_string(request, "sshHost"); + let rebind = if optional_bool(request, "rebindConnection").unwrap_or(false) { + RemoteConnectionRebind::UserConfirmed + } else { + RemoteConnectionRebind::Reject + }; let coordinator = openbitfun_core::agentic::coordination::get_global_coordinator() .ok_or("Conversation coordinator is unavailable")?; let info = coordinator @@ -122,9 +130,15 @@ pub(crate) async fn open_remote_workspace( &path, &connection_id, host.as_deref(), + rebind, ) .await - .map_err(|e| e.to_string())?; + .map_err(|e| { + let message = e.to_string(); + remote_workspace_connection_conflict_message(&message) + .map(str::to_string) + .unwrap_or(message) + })?; Ok(workspace_info_to_json(&info)) } diff --git a/src/apps/desktop/src/api/commands.rs b/src/apps/desktop/src/api/commands.rs index a84d48e97c..adc5788c65 100644 --- a/src/apps/desktop/src/api/commands.rs +++ b/src/apps/desktop/src/api/commands.rs @@ -389,6 +389,10 @@ pub struct OpenRemoteWorkspaceRequest { /// SSH config `host` (DNS or alias). When set, used for session mirror paths even if not connected. #[serde(default)] pub ssh_host: Option, + /// The user confirmed moving an existing record owned by another saved + /// connection to `connection_id`. + #[serde(default)] + pub rebind_connection: bool, } #[derive(Debug, Deserialize, Default)] @@ -1343,7 +1347,10 @@ pub async fn open_remote_workspace( ) -> Result { use openbitfun_core::service::remote_ssh::normalize_remote_workspace_path; use openbitfun_core::service::remote_ssh::workspace_state::remote_workspace_stable_id; - use openbitfun_core::service::workspace::WorkspaceCreateOptions; + use openbitfun_core::service::workspace::{ + remote_workspace_connection_conflict_message, RemoteConnectionRebind, + WorkspaceCreateOptions, + }; let ssh = state.get_ssh_manager_async().await?; let saved = ssh @@ -1431,6 +1438,11 @@ pub async fn open_remote_workspace( remote_connection_id: Some(request.connection_id.clone()), remote_ssh_host: Some(ssh_host.clone()), stable_workspace_id: Some(stable_workspace_id), + remote_connection_rebind: if request.rebind_connection { + RemoteConnectionRebind::UserConfirmed + } else { + RemoteConnectionRebind::Reject + }, }; match state @@ -1489,6 +1501,11 @@ pub async fn open_remote_workspace( Ok(WorkspaceInfoDto::from_workspace_info(&workspace_info)) } Err(e) => { + let message = e.to_string(); + if let Some(conflict) = remote_workspace_connection_conflict_message(&message) { + warn!("Remote workspace open needs a connection rebind decision: {conflict}"); + return Err(conflict.to_string()); + } error!("Failed to open remote workspace: {}", e); Err(format!("Failed to open remote workspace: {}", e)) } diff --git a/src/crates/assembly/core/AGENTS.md b/src/crates/assembly/core/AGENTS.md index cd8d389a63..203829187b 100644 --- a/src/crates/assembly/core/AGENTS.md +++ b/src/crates/assembly/core/AGENTS.md @@ -221,6 +221,22 @@ are not the default Core precheck. For documentation-only changes, run For assistant discovery, opened-state persistence, and reopening by workspace ID: `cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git --lib service::workspace::service::tests::assistant_`. +For local/SSH workspace identity collisions, SSH connection rebinding, committed +session storage, legacy path compatibility, and queue admission, use the matching +owner filter: + +```bash +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git,remote-workspace --lib service::workspace:: +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git,remote-workspace --lib agentic::session:: +cargo test --locked -p openbitfun-core --no-default-features --features agent-runtime,git,remote-workspace --lib agentic::coordination:: +``` + +The coordination filter includes the scheduler admission tests. These catalog and +storage fixtures do not require or validate a live SSH connection. CI also runs +them under `product-full`. `PathManager::with_user_root_for_tests` derives product +home from the user root's parent, so a fixture that relocates managed paths must +place its user root below the directory it means to exercise. + For disk-backed history paging and legacy sessions without a catalog: `cargo test --locked -p openbitfun-core --no-default-features --features remote-connect,git --lib history_page_`. Also run the `staged_revert_catalog_projection` and `load_relay_session_turns_` diff --git a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs index 116080384a..6cb62bca19 100644 --- a/src/crates/assembly/core/src/agentic/coordination/coordinator.rs +++ b/src/crates/assembly/core/src/agentic/coordination/coordinator.rs @@ -82,8 +82,8 @@ use crate::service::session::{ ToolItemIdentityExt, TurnStatus, }; use crate::service::workspace::{ - get_global_workspace_service, WorkspaceActivityMode, WorkspaceInfo, WorkspaceKind, - WorkspaceService, + get_global_workspace_service, RemoteConnectionRebind, WorkspaceActivityMode, WorkspaceInfo, + WorkspaceKind, WorkspaceService, }; use crate::service_agent_runtime::CoreServiceAgentRuntime; use crate::util::errors::{OpenBitFunError, OpenBitFunResult}; @@ -1885,31 +1885,9 @@ impl ConversationCoordinator { &self, session_id: &str, ) -> OpenBitFunResult { - if let Some(binding) = self - .session_manager - .resolve_session_workspace_binding(session_id) + self.session_manager + .require_session_storage_path(session_id) .await - { - return Ok(binding.session_storage_dir()); - } - - let session = self - .session_manager - .get_session(session_id) - .ok_or_else(|| { - OpenBitFunError::NotFound(format!("Session not found: {}", session_id)) - })?; - session - .config - .workspace_path - .as_deref() - .map(PathBuf::from) - .ok_or_else(|| { - OpenBitFunError::Validation(format!( - "workspace_path is required when restoring session: {}", - session_id - )) - }) } async fn is_chinese_locale() -> bool { @@ -2386,6 +2364,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet path: &str, connection_id: &str, ssh_host: Option<&str>, + remote_connection_rebind: RemoteConnectionRebind, ) -> OpenBitFunResult { let workspace = workspace_service .prepare_remote_workspace(path, connection_id, ssh_host) @@ -2399,7 +2378,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet .and_then(|host| host.as_str()), )?; workspace_service - .open_known_remote_workspace(&workspace) + .open_known_remote_workspace_with_rebind(&workspace, remote_connection_rebind) .await } @@ -4178,8 +4157,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet || (context_messages.len() == 1 && !session.dialog_turn_ids.is_empty())) && !session.dialog_turn_ids.is_empty() { - let restore_path = - Self::resolve_session_restore_path(&project_workspace_path, None, None).await?; + let restore_path = self.restore_path_for_existing_session(&session_id).await?; self.restore_session_from_storage_path(&restore_path, &session_id) .await?; session = self @@ -4816,6 +4794,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet } async fn resolve_session_restore_scope( + &self, workspace_path: &str, remote_connection_id: Option<&str>, remote_ssh_host: Option<&str>, @@ -4826,18 +4805,19 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet remote_ssh_host: remote_ssh_host.map(ToOwned::to_owned), }; - CoreSessionStorePort::default() + CoreSessionStorePort::with_path_manager(self.session_manager.path_manager()) .resolve_session_storage_path(request) .await .map_err(|error| OpenBitFunError::Session(error.to_string())) } async fn resolve_session_restore_path( + &self, workspace_path: &str, remote_connection_id: Option<&str>, remote_ssh_host: Option<&str>, ) -> OpenBitFunResult { - Self::resolve_session_restore_scope(workspace_path, remote_connection_id, remote_ssh_host) + self.resolve_session_restore_scope(workspace_path, remote_connection_id, remote_ssh_host) .await .map(|resolution| resolution.effective_storage_path) } @@ -6162,7 +6142,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet ); let requested_restore = match storage_workspace_path.as_deref() { Some(workspace_path) => Some( - Self::resolve_session_restore_scope( + self.resolve_session_restore_scope( workspace_path, remote_connection_id.as_deref(), remote_ssh_host.as_deref(), @@ -6408,32 +6388,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet "Starting session history restore: session_id={}", session_id ); - let restore_workspace_path = session - .config - .project_workspace_path - .as_deref() - .or(session.config.workspace_path.as_deref()) - .or(storage_workspace_path.as_deref()) - .ok_or_else(|| { - OpenBitFunError::Validation(format!( - "workspace_path is required when restoring session: {}", - session_id - )) - })?; - let restore_path = Self::resolve_session_restore_path( - restore_workspace_path, - session - .config - .remote_connection_id - .as_deref() - .or(remote_connection_id.as_deref()), - session - .config - .remote_ssh_host - .as_deref() - .or(remote_ssh_host.as_deref()), - ) - .await?; + let restore_path = self.restore_path_for_existing_session(&session_id).await?; match self .restore_session_from_storage_path(&restore_path, &session_id) .await @@ -8354,7 +8309,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet let session_storage_path = self .session_manager .resolve_storage_path_for_workspace_path(workspace_path) - .await; + .await?; let has_revert_state = self .session_manager .persistence_manager() @@ -15040,7 +14995,7 @@ impl openbitfun_runtime_ports::AgentThreadGoalManagementPort for ConversationCoo .await .map_err(runtime_port_error_preserving_message)? } else { - Self::resolve_session_restore_path( + self.resolve_session_restore_path( &request.workspace_path, request.remote_connection_id.as_deref(), request.remote_ssh_host.as_deref(), @@ -20300,13 +20255,14 @@ mod tests { ssh_host, ) .await; - let storage_path = ConversationCoordinator::resolve_session_restore_path( - logical_workspace_path, - Some(connection_id), - Some(ssh_host), - ) - .await - .expect("remote storage path should resolve"); + let storage_path = coordinator + .resolve_session_restore_path( + logical_workspace_path, + Some(connection_id), + Some(ssh_host), + ) + .await + .expect("remote storage path should resolve"); let goal = ThreadGoal { goal_id: format!("goal-{index}"), session_id: session_id.clone(), diff --git a/src/crates/assembly/core/src/agentic/coordination/scheduler.rs b/src/crates/assembly/core/src/agentic/coordination/scheduler.rs index 013119ddb3..074ac23dc9 100644 --- a/src/crates/assembly/core/src/agentic/coordination/scheduler.rs +++ b/src/crates/assembly/core/src/agentic/coordination/scheduler.rs @@ -900,7 +900,10 @@ impl DialogScheduler { turn_id: Some(resolved_turn_id.clone()), agent_type: delivery.agent_type, workspace_path: delivery.workspace_path, - workspace_id: None, + workspace_id: self + .session_manager + .get_session(&delivery.session_id) + .and_then(|session| session.config.workspace_id), remote_connection_id: delivery.remote_connection_id, remote_ssh_host: delivery.remote_ssh_host, policy: DialogSubmissionPolicy::new(DialogTriggerSource::AgentSession, queue_priority), @@ -1000,13 +1003,14 @@ impl DialogScheduler { user_message_metadata: Option, image_contexts: Option>, ) -> Result { - self.submit_with_prepended_messages( + self.submit_with_workspace_reference( session_id, user_input, original_user_input, turn_id, agent_type, workspace_path, + None, remote_connection_id, remote_ssh_host, policy, @@ -1034,6 +1038,47 @@ impl DialogScheduler { user_message_metadata: Option, prepended_messages: Vec, image_contexts: Option>, + ) -> Result { + let workspace_id = self + .session_manager + .get_session(&session_id) + .and_then(|session| session.config.workspace_id); + self.submit_with_workspace_reference( + session_id, + user_input, + original_user_input, + turn_id, + agent_type, + workspace_path, + workspace_id, + remote_connection_id, + remote_ssh_host, + policy, + reply_route, + user_message_metadata, + prepended_messages, + image_contexts, + ) + .await + } + + #[allow(clippy::too_many_arguments)] + async fn submit_with_workspace_reference( + &self, + session_id: String, + user_input: String, + original_user_input: Option, + turn_id: Option, + agent_type: String, + workspace_path: Option, + workspace_id: Option, + remote_connection_id: Option, + remote_ssh_host: Option, + policy: DialogSubmissionPolicy, + reply_route: Option, + user_message_metadata: Option, + prepended_messages: Vec, + image_contexts: Option>, ) -> Result { let resolved_turn_id = turn_id.unwrap_or_else(|| Uuid::new_v4().to_string()); let queued_turn = QueuedTurn { @@ -1043,7 +1088,7 @@ impl DialogScheduler { turn_id: Some(resolved_turn_id.clone()), agent_type, workspace_path, - workspace_id: None, + workspace_id, remote_connection_id, remote_ssh_host, policy, @@ -1092,7 +1137,7 @@ impl DialogScheduler { turn_id: Some(resolved_turn_id.clone()), agent_type, workspace_path: session.config.workspace_path.clone(), - workspace_id: None, + workspace_id: session.config.workspace_id.clone(), remote_connection_id: session.config.remote_connection_id.clone(), remote_ssh_host: session.config.remote_ssh_host.clone(), policy: DialogSubmissionPolicy::for_source(DialogTriggerSource::AgentSession), @@ -1170,19 +1215,28 @@ impl DialogScheduler { let session = match self.session_manager.get_session(session_id) { Some(session) => session, None => { - let workspace_path = workspace_path.ok_or_else(|| { - format!( - "workspace_path is required when restoring session: {}", - session_id - ) - })?; - let restore_path = Self::resolve_session_restore_path( - workspace_path, - remote_connection_id, - remote_ssh_host, - ) - .await - .map_err(|error| error.to_string())?; + let restore_path = match self + .session_manager + .require_session_storage_path(session_id) + .await + { + Ok(path) => path, + Err(OpenBitFunError::NotFound(_)) => { + let workspace_path = workspace_path.ok_or_else(|| { + format!( + "workspace_path is required when restoring session: {session_id}" + ) + })?; + self.resolve_session_restore_path( + workspace_path, + remote_connection_id, + remote_ssh_host, + ) + .await + .map_err(|error| error.to_string())? + } + Err(error) => return Err(error.to_string()), + }; self.coordinator .restore_session_from_storage_path(&restore_path, session_id) .await @@ -1198,6 +1252,7 @@ impl DialogScheduler { } async fn resolve_session_restore_path( + &self, workspace_path: &str, remote_connection_id: Option<&str>, remote_ssh_host: Option<&str>, @@ -1208,7 +1263,7 @@ impl DialogScheduler { remote_ssh_host: remote_ssh_host.map(ToOwned::to_owned), }; - CoreSessionStorePort::default() + CoreSessionStorePort::with_path_manager(self.session_manager.path_manager()) .resolve_session_storage_path(request) .await .map(|resolution| resolution.effective_storage_path) @@ -1289,19 +1344,15 @@ impl DialogScheduler { // the loaded session's authoritative local/remote storage binding. Some( self.session_manager - .effective_session_storage_path(&session_id) + .require_session_storage_path(&session_id) .await - .ok_or_else(|| { - SchedulerSubmitError::Message( - "Host session storage binding unavailable".into(), - ) - })?, + .map_err(SchedulerSubmitError::Core)?, ) } else if let Some(workspace_id) = requested_workspace_id.as_deref() { // ID-aware callers locate the session by its owning workspace; the // path on the request is only an execution-root projection. Some( - CoreSessionStorePort::default() + CoreSessionStorePort::with_path_manager(self.session_manager.path_manager()) .resolve_workspace_storage(workspace_id) .await .map(|resolution| resolution.effective_storage_path) @@ -1309,7 +1360,7 @@ impl DialogScheduler { ) } else if let Some(workspace_path) = queued_turn.workspace_path.as_deref() { Some( - Self::resolve_session_restore_path( + self.resolve_session_restore_path( workspace_path, queued_turn.remote_connection_id.as_deref(), queued_turn.remote_ssh_host.as_deref(), @@ -4254,6 +4305,62 @@ mod tests { ); } + #[tokio::test] + async fn internal_follow_up_keeps_workspace_id_when_a_remote_root_collides() { + let (scheduler, manager, _, root) = test_scheduler(); + let workspace = fixture_workspace_dir(root.path().join("same-name")); + let session = manager + .create_session( + "Owner".into(), + "Standard".into(), + SessionConfig { + workspace_path: Some(workspace.to_string_lossy().into_owned()), + ..Default::default() + }, + ) + .await + .unwrap(); + crate::service::workspace::legacy_compat::register_remote_fixture( + &workspace.to_string_lossy(), + "other-connection", + "localhost", + ) + .await; + manager + .update_session_state( + &session.session_id, + SessionState::Processing { + current_turn_id: "running".into(), + phase: ProcessingPhase::Thinking, + }, + ) + .await + .unwrap(); + let outcome = scheduler + .submit_with_prepended_messages( + session.session_id.clone(), + "follow-up".into(), + None, + Some("next-turn".into()), + "Standard".into(), + Some(workspace.to_string_lossy().into_owned()), + None, + None, + DialogSubmissionPolicy::for_source(DialogTriggerSource::AgentSession), + None, + None, + Vec::new(), + None, + ) + .await + .unwrap(); + assert!(matches!(outcome, DialogSubmitOutcome::Queued { .. })); + let queued = + remove_queued_turn_by_id(&scheduler.queues, &session.session_id, "next-turn").unwrap(); + assert_eq!(queued.workspace_id, session.config.workspace_id); + assert_eq!(queued.workspace_path, None); + } + #[tokio::test] async fn dialog_port_preserves_not_found_for_a_missing_session() { let (scheduler, _, _, root) = test_scheduler(); diff --git a/src/crates/assembly/core/src/agentic/session/session_manager.rs b/src/crates/assembly/core/src/agentic/session/session_manager.rs index 6a208b6311..579fc76641 100644 --- a/src/crates/assembly/core/src/agentic/session/session_manager.rs +++ b/src/crates/assembly/core/src/agentic/session/session_manager.rs @@ -1087,44 +1087,20 @@ impl SessionManager { persistence_manager: &PersistenceManager, config: &SessionConfig, ) -> Option { - if let Some(id) = config.workspace_id.as_deref() { - return CoreSessionStorePort::with_path_manager( - persistence_manager.path_manager().clone(), + CoreSessionStorePort::with_path_manager(persistence_manager.path_manager().clone()) + .resolve_storage_for_reference( + config.workspace_id.as_deref(), + config + .workspace_path + .as_deref() + .or(config.project_workspace_path.as_deref()) + .unwrap_or_default(), + config.remote_connection_id.clone(), + config.remote_ssh_host.clone(), ) - .resolve_workspace_storage(id) .await .ok() - .map(|resolution| resolution.effective_storage_path); - } - let workspace_path = config.workspace_path.as_ref()?; - let identity = - crate::service::remote_ssh::workspace_state::resolve_workspace_session_identity( - workspace_path, - config.remote_connection_id.as_deref(), - config.remote_ssh_host.as_deref(), - ) - .await?; - - let runtime_service = persistence_manager.runtime_service(); - Some(if !identity.is_remote() { - let project_workspace_path = config - .project_workspace_path - .as_deref() - .unwrap_or_else(|| identity.logical_workspace_path()); - runtime_service - .context_for_local_workspace(Path::new(project_workspace_path)) - .sessions_dir - } else if identity.hostname == "_unresolved" { - openbitfun_services_core::workspace_identity::unresolved_remote_session_storage_dir( - runtime_service.path_manager().remote_ssh_mirror_root_dir(), - identity.remote_connection_id.as_deref().unwrap_or_default(), - identity.logical_workspace_path(), - ) - } else { - runtime_service - .context_for_remote_workspace(&identity.hostname, identity.logical_workspace_path()) - .sessions_dir - }) + .map(|resolution| resolution.effective_storage_path) } async fn effective_storage_path_for_config(&self, config: &SessionConfig) -> Option { @@ -1135,37 +1111,25 @@ impl SessionManager { .await } - async fn effective_storage_path_for_workspace_path(&self, workspace_path: &Path) -> PathBuf { - if self - .persistence_manager - .is_resolved_sessions_dir(workspace_path) - { - return workspace_path.to_path_buf(); - } - let tmp_config = SessionConfig { - workspace_path: Some(workspace_path.to_string_lossy().to_string()), - ..Default::default() - }; - self.effective_storage_path_for_config(&tmp_config) - .await - .unwrap_or_else(|| workspace_path.to_path_buf()) - } - pub(crate) async fn resolve_storage_path_for_workspace_path( &self, workspace_path: &Path, - ) -> PathBuf { + ) -> OpenBitFunResult { let storage_path_started_at = Instant::now(); let session_storage_path = self - .effective_storage_path_for_workspace_path(workspace_path) - .await; + .resolve_storage_path_for_request(SessionStoragePathRequest { + workspace_path: workspace_path.to_path_buf(), + remote_connection_id: None, + remote_ssh_host: None, + }) + .await?; debug!( "Session storage path resolved from workspace: workspace_path={}, session_storage_path={}, duration_ms={}", workspace_path.display(), session_storage_path.display(), elapsed_ms_u64(storage_path_started_at) ); - session_storage_path + Ok(session_storage_path) } async fn resolve_storage_path_for_restore_workspace_path( @@ -1181,9 +1145,8 @@ impl SessionManager { workspace_path.display() ))); } - Ok(self - .resolve_storage_path_for_workspace_path(workspace_path) - .await) + self.resolve_storage_path_for_workspace_path(workspace_path) + .await } async fn resolve_storage_path_for_request( @@ -1215,11 +1178,58 @@ impl SessionManager { .and_then(|session| Self::session_workspace_from_config(&session.config)) } - /// Resolve the effective storage path for a session by ID. - /// For remote workspaces, maps the remote path to a local session storage path. + fn committed_session_storage_path( + index: &DashMap, + session_id: &str, + ) -> Option { + index + .get(session_id) + .filter(|binding| binding.committed) + .map(|binding| binding.path.clone()) + } + + /// Reuse the storage binding admitted when this session was created/restored. + /// A workspace path is an execution projection, never a replacement for this + /// binding. In particular, a later same-path SSH record or runtime-layout + /// change must not redirect history, persistence, or queued continuations. + /// + /// Without a committed binding the path is resolved from the session config + /// on every call. Only create/restore admission commits a binding; a read + /// must not pin whatever the current workspace catalog happens to resolve. + pub(crate) async fn require_session_storage_path( + &self, + session_id: &str, + ) -> OpenBitFunResult { + openbitfun_core_types::validate_session_id(session_id) + .map_err(OpenBitFunError::Validation)?; + if let Some(path) = + Self::committed_session_storage_path(&self.session_storage_path_index, session_id) + { + return Ok(path); + } + let config = self + .get_session(session_id) + .ok_or_else(|| OpenBitFunError::NotFound(format!("Session not found: {session_id}")))? + .config; + let resolution = CoreSessionStorePort::with_path_manager(self.path_manager()) + .resolve_storage_for_reference( + config.workspace_id.as_deref(), + config + .workspace_path + .as_deref() + .or(config.project_workspace_path.as_deref()) + .unwrap_or_default(), + config.remote_connection_id, + config.remote_ssh_host, + ) + .await + .map_err(|error| OpenBitFunError::Session(error.to_string()))?; + self.validate_session_storage_path_binding(session_id, &resolution.effective_storage_path)?; + Ok(resolution.effective_storage_path) + } + pub(crate) async fn effective_session_storage_path(&self, session_id: &str) -> Option { - let config = self.sessions.get(session_id)?.config.clone(); - self.effective_storage_path_for_config(&config).await + self.require_session_storage_path(session_id).await.ok() } pub(crate) fn path_manager(&self) -> Arc { @@ -1235,11 +1245,6 @@ impl SessionManager { let storage_path = self .effective_session_storage_path(parent_session_id) .await - .or_else(|| { - self.session_storage_path_index - .get(parent_session_id) - .map(|entry| entry.value().path.clone()) - }) .ok_or_else(|| { OpenBitFunError::NotFound(format!( "Session storage path not found: {parent_session_id}" @@ -1267,11 +1272,6 @@ impl SessionManager { let storage_path = self .effective_session_storage_path(session_id) .await - .or_else(|| { - self.session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()) - }) .ok_or_else(|| { OpenBitFunError::Validation(format!( "Session storage path is unavailable: {}", @@ -1310,14 +1310,7 @@ impl SessionManager { return None; } - let storage_path = self - .effective_session_storage_path(session_id) - .await - .or_else(|| { - self.session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()) - })?; + let storage_path = self.effective_session_storage_path(session_id).await?; Some(SessionStorageLayout::new(storage_path).request_traces_dir(session_id)) } @@ -1353,11 +1346,6 @@ impl SessionManager { let source_storage_path = self .effective_session_storage_path(source_session_id) .await - .or_else(|| { - self.session_storage_path_index - .get(source_session_id) - .map(|entry| entry.value().path.clone()) - }) .ok_or_else(|| { OpenBitFunError::NotFound(format!( "Current session storage path is unavailable: {}", @@ -1449,10 +1437,8 @@ impl SessionManager { } } - let indexed_storage_path = self - .session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()); + let indexed_storage_path = + Self::committed_session_storage_path(&self.session_storage_path_index, session_id); if let Some(session_storage_path) = indexed_storage_path { if let Some(binding) = self .resolve_persisted_session_workspace_binding( @@ -2124,11 +2110,6 @@ impl SessionManager { let storage_path = self .effective_session_storage_path(&event.session_id) .await - .or_else(|| { - self.session_storage_path_index - .get(&event.session_id) - .map(|entry| entry.value().path.clone()) - }) .ok_or_else(|| { OpenBitFunError::session(format!( "Session storage path unavailable while persisting evidence: {}", @@ -4239,10 +4220,8 @@ impl SessionManager { // If the session was evicted from memory (idle > 1h), try to restore it // using the storage path recorded when it was first created/restored. if !self.sessions.contains_key(session_id) && self.config.enable_persistence { - let session_storage_path = self - .session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()); + let session_storage_path = + Self::committed_session_storage_path(&self.session_storage_path_index, session_id); if let Some(session_storage_path) = session_storage_path { debug!( "Session evicted from memory, restoring for model update: session_id={}", @@ -4367,10 +4346,8 @@ impl SessionManager { // Match the model-selection path: an evicted session is restored before // the mutation permit is taken, because restore owns the same keyed lock. if !self.sessions.contains_key(session_id) && self.config.enable_persistence { - let session_storage_path = self - .session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()); + let session_storage_path = + Self::committed_session_storage_path(&self.session_storage_path_index, session_id); if let Some(session_storage_path) = session_storage_path { debug!( "Session evicted from memory, restoring for permission mode update: session_id={}", @@ -4530,10 +4507,8 @@ impl SessionManager { // not populate the storage-path index, so use the owning project path as // the stable fallback locator. if !self.sessions.contains_key(session_id) && self.config.enable_persistence { - let session_storage_path = self - .session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()); + let session_storage_path = + Self::committed_session_storage_path(&self.session_storage_path_index, session_id); let restore_result = if let Some(session_storage_path) = session_storage_path { self.restore_session_from_storage_path(&session_storage_path, session_id) .await @@ -4693,7 +4668,7 @@ impl SessionManager { ) -> OpenBitFunResult<()> { let session_storage_path = self .resolve_storage_path_for_workspace_path(workspace_path) - .await; + .await?; self.validate_session_storage_path_binding(session_id, &session_storage_path)?; let cleanup_workspace_path = self .resolve_session_cleanup_workspace_path( @@ -4714,29 +4689,7 @@ impl SessionManager { openbitfun_core_types::validate_session_id(session_id) .map_err(OpenBitFunError::Validation)?; let _mutation_guard = self.lock_session_mutation(session_id).await; - let session = self - .sessions - .get(session_id) - .map(|entry| entry.value().clone()); - let session_storage_path = if let Some(session) = session.as_ref() { - self.effective_storage_path_for_config(&session.config) - .await - .or_else(|| { - self.session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()) - }) - } else { - self.session_storage_path_index - .get(session_id) - .map(|entry| entry.value().path.clone()) - }; - let Some(session_storage_path) = session_storage_path else { - return Err(OpenBitFunError::NotFound(format!( - "Session storage path not found: {}", - session_id - ))); - }; + let session_storage_path = self.require_session_storage_path(session_id).await?; self.validate_session_storage_path_binding(session_id, &session_storage_path)?; let cleanup_workspace_path = self .resolve_session_cleanup_workspace_path( @@ -7054,15 +7007,7 @@ impl SessionManager { ))); } } - let workspace_path = self - .effective_storage_path_for_config(&session.config) - .await - .ok_or_else(|| { - OpenBitFunError::Validation(format!( - "Session workspace_path is missing: {}", - session_id - )) - })?; + let workspace_path = self.require_session_storage_path(session_id).await?; let turn_index = session.dialog_turn_ids.len(); let turn_id = new_turn_id(turn_id); @@ -7415,10 +7360,7 @@ impl SessionManager { } self.ensure_persisted_turn_append_allowed(session_id) .await?; - let storage = self - .effective_storage_path_for_config(&session.config) - .await - .ok_or_else(|| OpenBitFunError::Validation("Session storage is unavailable".into()))?; + let storage = self.require_session_storage_path(session_id).await?; let index = session.dialog_turn_ids.len(); let timestamp = SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -7512,15 +7454,7 @@ impl SessionManager { let session = self.get_session(session_id).ok_or_else(|| { OpenBitFunError::NotFound(format!("Session not found: {}", session_id)) })?; - let workspace_path = self - .effective_storage_path_for_config(&session.config) - .await - .ok_or_else(|| { - OpenBitFunError::Validation(format!( - "Session workspace_path is missing: {}", - session_id - )) - })?; + let workspace_path = self.require_session_storage_path(session_id).await?; let turn_id = new_turn_id(turn_id); let turn_index = session @@ -8595,14 +8529,7 @@ impl SessionManager { "Only the latest dialog turn can be interrupted: {turn_id}" )) })?; - let workspace_path = self - .effective_storage_path_for_config(&session.config) - .await - .ok_or_else(|| { - OpenBitFunError::Validation(format!( - "Session workspace_path is missing: {session_id}" - )) - })?; + let workspace_path = self.require_session_storage_path(session_id).await?; let mut turn = self .persistence_manager .load_dialog_turn(&workspace_path, session_id, turn_index) @@ -8708,14 +8635,7 @@ impl SessionManager { "Only the latest dialog turn can be recovered: {turn_id}" )) })?; - let workspace_path = self - .effective_storage_path_for_config(&session.config) - .await - .ok_or_else(|| { - OpenBitFunError::Validation(format!( - "Session workspace_path is missing: {session_id}" - )) - })?; + let workspace_path = self.require_session_storage_path(session_id).await?; let mut turn = self .persistence_manager .load_dialog_turn(&workspace_path, session_id, turn_index) @@ -9054,12 +8974,7 @@ impl SessionManager { let Some(turn_index) = session.dialog_turn_ids.len().checked_sub(1) else { return Ok(false); }; - let Some(workspace_path) = self - .effective_storage_path_for_config(&session.config) - .await - else { - return Ok(false); - }; + let workspace_path = self.require_session_storage_path(session_id).await?; let Some(turn) = self .persistence_manager .load_dialog_turn(&workspace_path, session_id, turn_index) @@ -9093,12 +9008,7 @@ impl SessionManager { let Some(turn_index) = turn_index else { return Ok(None); }; - let Some(workspace_path) = self - .effective_storage_path_for_config(&session.config) - .await - else { - return Ok(None); - }; + let workspace_path = self.require_session_storage_path(session_id).await?; let Some(mut turn) = self .persistence_manager .load_dialog_turn(&workspace_path, session_id, turn_index) @@ -9813,6 +9723,7 @@ impl SessionManager { let persistence = self.persistence_manager.clone(); let session_mutation_locks = self.session_mutation_locks.clone(); let interval = self.config.auto_save_interval; + let session_storage_path_index = self.session_storage_path_index.clone(); tokio::spawn(async move { let mut ticker = Self::auto_save_interval(interval); @@ -9826,13 +9737,20 @@ impl SessionManager { if !Self::auto_save_snapshot_is_current(&sessions, &snapshot) { continue; } - if let Some(workspace_path) = - Self::effective_storage_path_for_config_with_persistence( - persistence.as_ref(), - &snapshot.session.config, - ) - .await - { + let storage_path = match Self::committed_session_storage_path( + &session_storage_path_index, + &snapshot.session_id, + ) { + Some(path) => Some(path), + None => { + Self::effective_storage_path_for_config_with_persistence( + persistence.as_ref(), + &snapshot.session.config, + ) + .await + } + }; + if let Some(workspace_path) = storage_path { if !Self::auto_save_snapshot_is_current(&sessions, &snapshot) { continue; } @@ -9861,6 +9779,7 @@ impl SessionManager { let active_session_permits = self.active_session_permits.clone(); let timeout = self.config.session_idle_timeout; let persistence = self.persistence_manager.clone(); + let session_storage_path_index = self.session_storage_path_index.clone(); let enable_persistence = self.config.enable_persistence; let session_mutation_locks = self.session_mutation_locks.clone(); let session_write_locks = self.session_write_locks.clone(); @@ -9913,13 +9832,20 @@ impl SessionManager { &transient_session_ids, ) { - if let Some(workspace_path) = - Self::effective_storage_path_for_config_with_persistence( - persistence.as_ref(), - &session.config, - ) - .await - { + let storage_path = match Self::committed_session_storage_path( + &session_storage_path_index, + &session.session_id, + ) { + Some(path) => Some(path), + None => { + Self::effective_storage_path_for_config_with_persistence( + persistence.as_ref(), + &session.config, + ) + .await + } + }; + if let Some(workspace_path) = storage_path { if Self::cleanup_snapshot_for_candidate( &sessions, &candidate, @@ -13248,6 +13174,80 @@ mod tests { .expect("retry should not be blocked by partial persistence"); } + #[tokio::test] + async fn session_storage_reads_never_commit_or_follow_uncommitted_bindings() { + let workspace = TestWorkspace::new(); + let persistence_manager = Arc::new( + PersistenceManager::new(workspace.path_manager()).expect("persistence manager"), + ); + let sessions_dir = persistence_manager + .path_manager() + .project_sessions_dir(workspace.path()); + let manager = test_manager(persistence_manager); + let session = manager + .create_session( + "Storage binding".to_string(), + "Standard".to_string(), + SessionConfig { + workspace_path: Some(workspace.path().to_string_lossy().to_string()), + ..Default::default() + }, + ) + .await + .expect("session should create"); + let session_id = session.session_id.clone(); + manager + .sessions + .get_mut(&session_id) + .expect("loaded session") + .dialog_turn_ids + .push("turn-without-binding".to_string()); + + manager.session_storage_path_index.remove(&session_id); + let resolved = manager + .require_session_storage_path(&session_id) + .await + .expect("config resolution"); + assert_eq!( + SessionManager::normalize_session_storage_path(&resolved), + SessionManager::normalize_session_storage_path(&sessions_dir) + ); + assert!( + manager.storage_path_binding_for_test(&session_id).is_none(), + "a read must not pin the currently resolved storage" + ); + + let in_flight = workspace.path().join("in-flight-claim"); + assert!(manager + .claim_session_storage_path(&session_id, &in_flight, true) + .expect("pending claim")); + assert!(manager + .require_session_storage_path(&session_id) + .await + .is_err()); + assert!(manager + .effective_session_storage_path(&session_id) + .await + .is_none()); + assert!( + manager + .persistent_model_exchange_trace_dir(&session_id) + .await + .is_none(), + "an uncommitted claim must not become a storage fallback" + ); + // Dispatch admission must surface an unresolvable binding instead of + // reporting that no interrupted turn holds the queue. + assert!(manager + .latest_dialog_turn_holds_dispatch(&session_id) + .await + .is_err()); + assert!(manager + .abandon_interrupted_dialog_turn(&session_id, None) + .await + .is_err()); + } + #[tokio::test] async fn background_title_update_cannot_recreate_storage_during_deletion() { let workspace = TestWorkspace::new(); @@ -13310,7 +13310,7 @@ mod tests { let persistence_manager = Arc::new( PersistenceManager::new(workspace.path_manager()).expect("persistence manager"), ); - let manager = test_manager(persistence_manager); + let manager = test_manager(persistence_manager.clone()); let session = manager .create_session( "Original".to_string(), @@ -13323,23 +13323,12 @@ mod tests { .await .expect("session should create"); - { - // Simulate a session whose persistence location can no longer be - // resolved: neither its workspace record nor an IO projection. - let mut loaded = manager - .sessions - .get_mut(&session.session_id) - .expect("loaded session"); - loaded.config.workspace_id = None; - loaded.config.project_workspace_id = None; - loaded.config.workspace_path = None; - loaded.config.project_workspace_path = None; - } + persistence_manager.fail_next_session_metadata_write_for_test(&session.session_id); manager .update_session_title(&session.session_id, "Not persisted") .await - .expect_err("missing persistence path must reject the title update"); + .expect_err("failed metadata persistence must reject the title update"); let loaded = manager .get_session(&session.session_id) @@ -14956,6 +14945,249 @@ mod tests { ); } + #[cfg(feature = "remote-workspace")] + #[tokio::test] + async fn core_session_store_port_keeps_colliding_local_and_ssh_owners_separate() { + use crate::service::workspace::legacy_compat::register_remote_fixture; + use openbitfun_runtime_ports::{ + SessionStorageKind, SessionStoragePathRequest, SessionStorePort, + }; + + let workspace = TestWorkspace::new(); + let root = workspace.path().to_string_lossy().into_owned(); + let loopback = register_remote_fixture(&root, "connection-loopback", "localhost").await; + let remote = register_remote_fixture(&root, "connection-remote", "other-host").await; + let port = CoreSessionStorePort::with_path_manager_for_tests(workspace.path_manager()); + let local = port + .resolve_workspace_storage(workspace.workspace_id()) + .await + .unwrap(); + let loopback_storage = port.resolve_workspace_storage(&loopback.id).await.unwrap(); + let remote_storage = port.resolve_workspace_storage(&remote.id).await.unwrap(); + assert_eq!(local.storage_kind, SessionStorageKind::Local); + assert_eq!(loopback_storage.storage_kind, SessionStorageKind::Remote); + assert_ne!( + local.effective_storage_path, + loopback_storage.effective_storage_path + ); + assert_ne!( + loopback_storage.effective_storage_path, + remote_storage.effective_storage_path + ); + + let old_request = SessionStoragePathRequest { + workspace_path: workspace.path().to_path_buf(), + remote_connection_id: None, + remote_ssh_host: None, + }; + let error = port + .resolve_session_storage_path(old_request) + .await + .unwrap_err(); + assert!(error.message.contains("ambiguous"), "{error}"); + let selected = port + .resolve_session_storage_path(SessionStoragePathRequest { + workspace_path: workspace.path().to_path_buf(), + remote_connection_id: Some("connection-loopback".into()), + remote_ssh_host: Some("localhost".into()), + }) + .await + .unwrap(); + assert_eq!( + selected.effective_storage_path, + loopback_storage.effective_storage_path + ); + let unknown = port + .resolve_storage_for_reference(Some("unknown-id"), &root, None, None) + .await + .unwrap_err(); + assert!(unknown.message.contains("unknown-id")); + } + + #[tokio::test] + async fn core_session_store_port_legacy_worktree_uses_its_registered_project_storage() { + use openbitfun_runtime_ports::{SessionStoragePathRequest, SessionStorePort}; + let project = TestWorkspace::new(); + let execution = project.path().join("worktree"); + std::fs::create_dir_all(&execution).unwrap(); + let record = crate::service::workspace::legacy_compat::register_local_fixture( + &execution, + Some(project.path()), + ) + .await; + let port = CoreSessionStorePort::with_path_manager_for_tests(project.path_manager()); + let by_id = port.resolve_workspace_storage(&record.id).await.unwrap(); + let by_legacy = port + .resolve_session_storage_path(SessionStoragePathRequest { + workspace_path: execution.clone(), + remote_connection_id: None, + remote_ssh_host: None, + }) + .await + .unwrap(); + assert_eq!( + by_id.effective_storage_path, + project.path_manager().project_sessions_dir(project.path()) + ); + assert_eq!( + by_legacy.effective_storage_path, + by_id.effective_storage_path + ); + assert_eq!(by_legacy.requested_workspace_path, execution); + + std::fs::remove_dir_all(&execution).unwrap(); + let offline = port + .resolve_session_storage_path(SessionStoragePathRequest { + workspace_path: execution, + remote_connection_id: None, + remote_ssh_host: None, + }) + .await + .unwrap(); + assert_eq!(offline.effective_storage_path, by_id.effective_storage_path); + } + + #[cfg(feature = "remote-workspace")] + #[tokio::test] + async fn bound_session_writes_and_deletion_do_not_reinfer_a_colliding_legacy_path() { + let workspace = TestWorkspace::new(); + let persistence = Arc::new(PersistenceManager::new(workspace.path_manager()).unwrap()); + let manager = test_manager(persistence.clone()); + let session = manager + .create_session( + "Bound writes".into(), + "Standard".into(), + SessionConfig { + workspace_id: Some(workspace.workspace_id().into()), + ..Default::default() + }, + ) + .await + .unwrap(); + let storage = manager + .require_session_storage_path(&session.session_id) + .await + .unwrap(); + crate::service::workspace::legacy_compat::register_remote_fixture( + &workspace.path().to_string_lossy(), + "colliding-remote", + "localhost", + ) + .await; + manager + .sessions + .get_mut(&session.session_id) + .unwrap() + .config + .workspace_id = None; + + let turn = manager + .append_completed_local_command_turn( + &session.session_id, + "Bound local command".into(), + Some("bound-local-turn".into()), + None, + None, + ) + .await + .unwrap(); + assert_eq!( + persistence + .load_dialog_turn(&storage, &session.session_id, 0) + .await + .unwrap() + .unwrap() + .turn_id, + turn.turn_id + ); + manager + .delete_session_by_id(&session.session_id) + .await + .unwrap(); + assert!(persistence + .load_dialog_turn(&storage, &session.session_id, 0) + .await + .unwrap() + .is_none()); + } + + #[cfg(feature = "remote-workspace")] + #[tokio::test] + async fn session_storage_binding_survives_path_collision_eviction_and_restart() { + let workspace = TestWorkspace::new(); + let persistence = Arc::new(PersistenceManager::new(workspace.path_manager()).unwrap()); + let manager = test_manager(persistence.clone()); + let session = manager + .create_session( + "Bound history".into(), + "Standard".into(), + SessionConfig { + workspace_id: Some(workspace.workspace_id().into()), + ..Default::default() + }, + ) + .await + .unwrap(); + let storage = manager + .require_session_storage_path(&session.session_id) + .await + .unwrap(); + crate::service::workspace::legacy_compat::register_remote_fixture( + &workspace.path().to_string_lossy(), + "colliding-remote", + "localhost", + ) + .await; + // In-memory legacy projections cannot replace an admitted storage owner. + manager + .sessions + .get_mut(&session.session_id) + .unwrap() + .config + .workspace_id = None; + assert_eq!( + manager + .require_session_storage_path(&session.session_id) + .await + .unwrap(), + storage + ); + manager.evict_loaded_session_for_test(&session.session_id); + assert_eq!( + manager + .require_session_storage_path(&session.session_id) + .await + .unwrap(), + storage + ); + let restored = manager + .restore_session_from_storage_path(&storage, &session.session_id) + .await + .unwrap(); + assert_eq!( + restored.config.workspace_id.as_deref(), + Some(workspace.workspace_id()) + ); + manager.evict_loaded_session_for_test(&session.session_id); + + let restarted = test_manager(persistence); + let restored = restarted + .restore_session_from_storage_path(&storage, &session.session_id) + .await + .unwrap(); + assert_eq!( + restored.config.workspace_id.as_deref(), + Some(workspace.workspace_id()) + ); + assert_eq!( + restarted + .require_session_storage_path(&session.session_id) + .await + .unwrap(), + storage + ); + } + #[cfg(feature = "remote-workspace")] #[tokio::test] async fn core_session_store_port_resolves_unresolved_remote_storage_path() { diff --git a/src/crates/assembly/core/src/agentic/session/session_store_port.rs b/src/crates/assembly/core/src/agentic/session/session_store_port.rs index b6b7a3401b..502392257a 100644 --- a/src/crates/assembly/core/src/agentic/session/session_store_port.rs +++ b/src/crates/assembly/core/src/agentic/session/session_store_port.rs @@ -9,30 +9,6 @@ use openbitfun_runtime_ports::{ use crate::agentic::core::SessionConfig; use crate::infrastructure::{get_path_manager_arc, PathManager}; use crate::service::WorkspaceRuntimeService; -use openbitfun_services_core::workspace_identity::{ - unresolved_remote_session_storage_dir, WorkspaceSessionIdentity, -}; - -async fn resolve_workspace_session_identity( - workspace_path: &str, - remote_connection_id: Option<&str>, - remote_ssh_host: Option<&str>, -) -> Option { - let mut config = SessionConfig { - workspace_path: Some(workspace_path.to_owned()), - remote_connection_id: remote_connection_id.map(str::to_owned), - remote_ssh_host: remote_ssh_host.map(str::to_owned), - ..Default::default() - }; - crate::agentic::workspace::normalize_session_workspace(&mut config) - .await - .ok()?; - openbitfun_services_core::workspace_identity::workspace_session_identity( - workspace_path, - config.remote_connection_id.as_deref(), - config.remote_ssh_host.as_deref(), - ) -} #[derive(Debug, Clone, Default)] pub struct CoreSessionStorePort { @@ -119,26 +95,26 @@ impl CoreSessionStorePort { None } - fn is_confined_to_managed_root(root: &Path, path: &Path) -> bool { - if Self::has_parent_traversal(path) || !path.starts_with(root) { - return false; + fn canonical_storage_projection(path: &Path) -> Option { + if Self::has_parent_traversal(path) { + return None; } + let ancestor = Self::nearest_existing_ancestor(path)?; + Some( + dunce::canonicalize(ancestor) + .ok()? + .join(path.strip_prefix(ancestor).ok()?), + ) + } - if !root.exists() { - return true; + fn is_confined_to_managed_root(root: &Path, path: &Path) -> bool { + match ( + Self::canonical_storage_projection(root), + Self::canonical_storage_projection(path), + ) { + (Some(root), Some(path)) => path.starts_with(root), + _ => false, } - - let Ok(canonical_root) = dunce::canonicalize(root) else { - return false; - }; - let Some(existing_ancestor) = Self::nearest_existing_ancestor(path) else { - return false; - }; - let Ok(canonical_ancestor) = dunce::canonicalize(existing_ancestor) else { - return false; - }; - - canonical_ancestor == canonical_root || canonical_ancestor.starts_with(canonical_root) } fn looks_like_resolved_sessions_dir(path_manager: &PathManager, path: &Path) -> bool { @@ -151,9 +127,11 @@ impl CoreSessionStorePort { } let projects_root = path_manager.projects_root(); - path.parent() + let lexical_shape = path + .parent() .and_then(Path::parent) - .is_some_and(|candidate| candidate == projects_root) + .is_some_and(|candidate| candidate == projects_root); + lexical_shape || Self::resolved_sessions_dir_kind(path_manager, path).is_some() } fn project_runtime_sessions_kind( @@ -192,6 +170,13 @@ impl CoreSessionStorePort { return None; } + // Committed bindings may be canonical (for example /private/var on + // macOS) while PathManager retains an equivalent symlink spelling. + // Compare physical projections, including not-yet-created suffixes, + // so a sessions directory cannot be reinterpreted as a workspace root. + let canonical_path = Self::canonical_storage_projection(path)?; + let path = canonical_path.as_path(); + let remote_mirror_root = path_manager.remote_ssh_mirror_root_dir(); if Self::is_confined_to_managed_root(&remote_mirror_root, path) { return Some( @@ -206,7 +191,7 @@ impl CoreSessionStorePort { ); } - let projects_root = path_manager.projects_root(); + let projects_root = Self::canonical_storage_projection(&path_manager.projects_root())?; let has_local_shape = path .parent() .and_then(|runtime_root| runtime_root.parent()) @@ -326,62 +311,36 @@ impl SessionStorePort for CoreSessionStorePort { } let workspace_path = request.workspace_path.to_string_lossy().to_string(); - let identity = resolve_workspace_session_identity( - &workspace_path, - request.remote_connection_id.as_deref(), - request.remote_ssh_host.as_deref(), - ) - .await - .ok_or_else(|| { - PortError::new( - PortErrorKind::InvalidRequest, - format!( - "Session workspace_path does not resolve to a local workspace or a \ - registered remote workspace: {workspace_path}" - ), - ) - })?; - - let requested_workspace_path = request.workspace_path; - let runtime_service = WorkspaceRuntimeService::new(path_manager.clone()); - let (effective_storage_path, storage_kind, remote_ssh_host) = if !identity.is_remote() { - ( - runtime_service - .context_for_local_workspace(Path::new(identity.logical_workspace_path())) - .sessions_dir, - SessionStorageKind::Local, - None, - ) - } else if identity.hostname == "_unresolved" { - ( - unresolved_remote_session_storage_dir( - path_manager.remote_ssh_mirror_root_dir(), - identity.remote_connection_id.as_deref().unwrap_or_default(), - identity.logical_workspace_path(), - ), - SessionStorageKind::UnresolvedRemote, - None, - ) - } else { - ( - runtime_service - .context_for_remote_workspace( - &identity.hostname, - identity.logical_workspace_path(), - ) - .sessions_dir, - SessionStorageKind::Remote, - Some(identity.hostname.clone()), - ) + let mut config = SessionConfig { + workspace_path: Some(workspace_path.clone()), + remote_connection_id: request.remote_connection_id, + remote_ssh_host: request.remote_ssh_host, + ..Default::default() }; - - Ok(SessionStoragePathResolution::new( - requested_workspace_path, - effective_storage_path, - storage_kind, - identity.remote_connection_id, - remote_ssh_host, - )) + // Convert pre-ID input exactly once at this compatibility boundary. + // Storage then follows the same catalog record as current ID requests; + // do not re-infer local/SSH identity from filesystem existence. + crate::agentic::workspace::normalize_session_workspace(&mut config) + .await + .map_err(|error| { + PortError::new( + PortErrorKind::InvalidRequest, + format!( + "Session workspace_path does not resolve to a local workspace or a \ + registered remote workspace: {workspace_path}: {error}" + ), + ) + })?; + let mut resolution = self + .resolve_workspace_storage(config.workspace_id.as_deref().ok_or_else(|| { + PortError::new( + PortErrorKind::InvalidRequest, + "Session workspace ID is unavailable", + ) + })?) + .await?; + resolution.requested_workspace_path = request.workspace_path; + Ok(resolution) } } @@ -425,6 +384,46 @@ mod tests { let _ = std::fs::remove_dir_all(test_root); } + #[cfg(unix)] + #[tokio::test] + async fn resolved_sessions_path_accepts_canonical_user_root_alias_without_relocating_history() { + let root = tempfile::tempdir().unwrap(); + let physical = root.path().join("physical"); + let alias = root.path().join("alias"); + std::fs::create_dir_all(&physical).unwrap(); + std::os::unix::fs::symlink(&physical, &alias).unwrap(); + // Product home is derived from the user root's parent, so the user + // root must sit below the alias for managed paths to use it. + let user_root = alias.join("user"); + std::fs::create_dir_all(&user_root).unwrap(); + let path_manager = Arc::new(PathManager::with_user_root_for_tests(user_root)); + let port = CoreSessionStorePort::with_path_manager_for_tests(path_manager.clone()); + let projected = path_manager + .projects_root() + .join("project-key") + .join("sessions"); + std::fs::create_dir_all(&projected).unwrap(); + let canonical = dunce::canonicalize(&projected).unwrap(); + assert_ne!(canonical, projected); + for path in [projected, canonical.clone()] { + let resolution = port + .resolve_session_storage_path(SessionStoragePathRequest { + workspace_path: path.clone(), + remote_connection_id: None, + remote_ssh_host: None, + }) + .await + .unwrap(); + assert_eq!(resolution.effective_storage_path, path); + assert_eq!(resolution.storage_kind, SessionStorageKind::Local); + } + std::fs::remove_dir(&canonical).unwrap(); + assert_eq!( + CoreSessionStorePort::resolved_sessions_dir_kind(&path_manager, &canonical), + Some(SessionStorageKind::Local) + ); + } + #[cfg(unix)] #[tokio::test] async fn resolved_sessions_path_rejects_symlink_escape() { diff --git a/src/crates/assembly/core/src/service/workspace/manager.rs b/src/crates/assembly/core/src/service/workspace/manager.rs index 050222adf5..4e5e551d1d 100644 --- a/src/crates/assembly/core/src/service/workspace/manager.rs +++ b/src/crates/assembly/core/src/service/workspace/manager.rs @@ -8,12 +8,12 @@ pub use super::types::{ use super::worktree_topology::global_worktree_topology_service; use super::WorktreeTopologyFreshness; use crate::util::{errors::*, FrontMatterMarkdown}; -use log::warn; +use log::{info, warn}; pub use openbitfun_runtime_ports::RelatedPath; use openbitfun_services_core::workspace_identity::{ - canonicalize_local_workspace_root, local_workspace_stable_storage_id, - normalize_local_workspace_root_for_stable_id, normalize_remote_workspace_path, - remote_workspace_stable_id, LOCAL_WORKSPACE_SSH_HOST, + canonical_ssh_connection_id, canonicalize_local_workspace_root, + local_workspace_stable_storage_id, normalize_local_workspace_root_for_stable_id, + normalize_remote_workspace_path, remote_workspace_stable_id, LOCAL_WORKSPACE_SSH_HOST, }; use serde::{Deserialize, Serialize}; @@ -167,6 +167,23 @@ impl Default for ScanOptions { } } +/// Whether reopening an existing remote workspace record may move it to a +/// different SSH connection. +/// +/// A record's `connectionId` routes its file, terminal, and Agent execution. +/// Replacing it silently would send another profile's history to a different +/// endpoint, so only an equivalent id, a provably orphaned owner, or an +/// explicit user decision may rebind it. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum RemoteConnectionRebind { + #[default] + Reject, + /// The record's owning connection is no longer saved on this host. + PreviousOwnerMissing, + /// The user confirmed moving the record to the requested connection. + UserConfirmed, +} + /// Options for opening a workspace. #[derive(Debug, Clone)] pub struct WorkspaceOpenOptions { @@ -176,14 +193,16 @@ pub struct WorkspaceOpenOptions { pub workspace_kind: WorkspaceKind, pub assistant_id: Option, pub display_name: Option, - /// For [`WorkspaceKind::Remote`], must match persisted `metadata["connectionId"]` so two - /// servers opened at the same path (e.g. `/`) are separate workspace tabs. + /// For [`WorkspaceKind::Remote`], the SSH connection that owns execution routing. + /// Reopening a record owned by another connection is governed by + /// [`Self::remote_connection_rebind`]. pub remote_connection_id: Option, /// SSH `host` (connection config) for remote mirror paths and metadata. pub remote_ssh_host: Option, /// Deterministic workspace id for remote workspaces (see `remote_workspace_stable_id`). /// Local/assistant workspaces use a stable `local_*` id from `localhost` + canonical root path. pub stable_workspace_id: Option, + pub remote_connection_rebind: RemoteConnectionRebind, } impl Default for WorkspaceOpenOptions { @@ -198,10 +217,69 @@ impl Default for WorkspaceOpenOptions { remote_connection_id: None, remote_ssh_host: None, stable_workspace_id: None, + remote_connection_rebind: RemoteConnectionRebind::Reject, } } } +/// Error code returned when reopening a remote record would move it to another +/// SSH connection without an allowed rebind reason. +pub const REMOTE_WORKSPACE_CONNECTION_CONFLICT: &str = "remote_workspace_connection_conflict"; + +/// The stable `remote_workspace_connection_conflict: ...` message carried by an +/// open error, so hosts can return it without wrapping the code in prose. +pub fn remote_workspace_connection_conflict_message(error_text: &str) -> Option<&str> { + error_text + .find(REMOTE_WORKSPACE_CONNECTION_CONFLICT) + .map(|start| &error_text[start..]) +} + +/// Whether two persisted connection ids name the same saved SSH connection. +/// +/// Only one side is canonicalized at a time: a legacy id is the current id plus +/// `:port`, while a current id for a bare IPv6 host may itself end in `:digits`. +pub(crate) fn remote_connection_ids_equivalent(left: &str, right: &str) -> bool { + let (left, right) = (left.trim(), right.trim()); + left == right + || canonical_ssh_connection_id(left) == right + || canonical_ssh_connection_id(right) == left +} + +fn authorize_remote_connection_rebind( + existing: &WorkspaceInfo, + requested: Option<&str>, + rebind: RemoteConnectionRebind, +) -> OpenBitFunResult<()> { + let (Some(current), Some(requested)) = ( + existing.remote_ssh_connection_id(), + requested.map(str::trim).filter(|value| !value.is_empty()), + ) else { + return Ok(()); + }; + if current == requested { + return Ok(()); + } + let reason = if remote_connection_ids_equivalent(current, requested) { + "legacy_connection_id" + } else { + match rebind { + RemoteConnectionRebind::Reject => { + return Err(OpenBitFunError::service(format!( + "{REMOTE_WORKSPACE_CONNECTION_CONFLICT}: Workspace {} is bound to SSH connection {current}; reopening it with connection {requested} requires confirmation.", + existing.id + ))); + } + RemoteConnectionRebind::PreviousOwnerMissing => "previous_owner_missing", + RemoteConnectionRebind::UserConfirmed => "user_confirmed", + } + }; + info!( + "Rebinding remote workspace connection: workspace_id={}, from={}, to={}, reason={}", + existing.id, current, requested, reason + ); + Ok(()) +} + /// Runtime operations stay in Core; persisted records are shared with offline tools. #[async_trait::async_trait] pub trait WorkspaceInfoRuntimeExt: Sized { @@ -861,33 +939,8 @@ impl WorkspaceManager { } let existing_workspace_id = if is_remote { - let host = options - .remote_ssh_host - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()); - let path_norm = normalize_remote_workspace_path(&path.to_string_lossy()); - let stable = options - .stable_workspace_id - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(str::to_string) - .or_else(|| host.map(|host| remote_workspace_stable_id(host, &path_norm))); - - stable - .as_deref() - .and_then(|sid| self.workspaces.get(sid)) - .and_then(|w| { - if w.workspace_kind == WorkspaceKind::Remote - && normalize_remote_workspace_path(&w.root_path.to_string_lossy()) - == path_norm - { - Some(w.id.clone()) - } else { - None - } - }) + self.existing_remote_workspace(&path, &options) + .map(|workspace| workspace.id.clone()) } else { let canon_norm = match normalize_local_workspace_root_for_stable_id(&path) { Ok(n) => n, @@ -901,6 +954,15 @@ impl WorkspaceManager { }; if let Some(workspace_id) = existing_workspace_id { + if is_remote { + if let Some(existing) = self.workspaces.get(&workspace_id) { + authorize_remote_connection_rebind( + existing, + options.remote_connection_id.as_deref(), + options.remote_connection_rebind, + )?; + } + } if let Some(workspace) = self.workspaces.get_mut(&workspace_id) { workspace.workspace_kind = options.workspace_kind.clone(); workspace.assistant_id = if options.workspace_kind == WorkspaceKind::Assistant { @@ -1098,6 +1160,32 @@ impl WorkspaceManager { self.workspaces.get(workspace_id) } + /// The remote record that opening `path` with `options` would reuse. + pub(crate) fn existing_remote_workspace( + &self, + path: &Path, + options: &WorkspaceOpenOptions, + ) -> Option<&WorkspaceInfo> { + let host = options + .remote_ssh_host + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()); + let path_norm = normalize_remote_workspace_path(&path.to_string_lossy()); + let stable = options + .stable_workspace_id + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string) + .or_else(|| host.map(|host| remote_workspace_stable_id(host, &path_norm)))?; + self.workspaces.get(&stable).filter(|workspace| { + workspace.workspace_kind == WorkspaceKind::Remote + && normalize_remote_workspace_path(&workspace.root_path.to_string_lossy()) + == path_norm + }) + } + /// Gets all opened workspaces. pub fn get_opened_workspace_infos(&self) -> Vec<&WorkspaceInfo> { self.opened_workspace_ids @@ -1455,6 +1543,154 @@ pub struct WorkspaceManagerStatistics { mod tests { use super::{WorkspaceIdentity, WorkspaceIdentityRuntimeExt}; + #[tokio::test] + async fn remote_profiles_sharing_a_mirror_remain_independently_activatable() { + use super::{ + WorkspaceInfo, WorkspaceInfoRuntimeExt, WorkspaceKind, WorkspaceManager, + WorkspaceManagerConfig, WorkspaceOpenOptions, WorkspaceStatus, + }; + + let mut manager = WorkspaceManager::new(WorkspaceManagerConfig::default()); + let options = WorkspaceOpenOptions { + workspace_kind: WorkspaceKind::Remote, + remote_connection_id: Some("ssh-first".into()), + remote_ssh_host: Some("remote.example".into()), + ..Default::default() + }; + let active = manager + .open_workspace_with_options("/srv/active".into(), options.clone()) + .await + .unwrap(); + let mut shared = WorkspaceInfo::new_without_worktree( + "/srv/shared".into(), + WorkspaceOpenOptions { + auto_set_current: false, + ..options + }, + ) + .await + .unwrap(); + let first_id = shared.id.clone(); + manager.workspaces.insert(first_id.clone(), shared.clone()); + shared.id = "imported-second-profile".into(); + shared.metadata.insert( + "connectionId".into(), + serde_json::Value::String("ssh-second".into()), + ); + let second_id = shared.id.clone(); + manager.workspaces.insert(second_id.clone(), shared); + manager.opened_workspace_ids = vec![first_id.clone(), second_id.clone(), active.id.clone()]; + + // Session identity, not workspace activation, separates profiles that + // share a host + root mirror. + manager.close_workspace(&active.id).unwrap(); + assert_eq!( + manager.current_workspace_id.as_deref(), + Some(first_id.as_str()) + ); + manager.set_active_workspace(&second_id).unwrap(); + assert_eq!( + manager.current_workspace_id.as_deref(), + Some(second_id.as_str()) + ); + assert_eq!( + manager.get_workspace(&first_id).unwrap().status, + WorkspaceStatus::Inactive + ); + assert_eq!( + manager + .get_workspace(&second_id) + .unwrap() + .remote_ssh_connection_id(), + Some("ssh-second") + ); + } + + #[tokio::test] + async fn remote_workspace_reopen_rebinds_connection_only_with_an_allowed_reason() { + use super::{ + remote_workspace_connection_conflict_message, RemoteConnectionRebind, WorkspaceKind, + WorkspaceManager, WorkspaceManagerConfig, WorkspaceOpenOptions, + REMOTE_WORKSPACE_CONNECTION_CONFLICT, + }; + let mut manager = WorkspaceManager::new(WorkspaceManagerConfig::default()); + let options = WorkspaceOpenOptions { + workspace_kind: WorkspaceKind::Remote, + remote_connection_id: Some("ssh-root@remote.example:22".into()), + remote_ssh_host: Some("remote.example".into()), + ..Default::default() + }; + let original = manager + .open_workspace_with_options("/srv/shared".into(), options.clone()) + .await + .unwrap(); + let reopen = |connection_id: &str, rebind: RemoteConnectionRebind| WorkspaceOpenOptions { + remote_connection_id: Some(connection_id.into()), + remote_connection_rebind: rebind, + ..options.clone() + }; + + // A record written by an older build with a port-qualified id is the + // same saved connection. + let upgraded = manager + .open_workspace_with_options( + "/srv/shared".into(), + reopen("ssh-root@remote.example", RemoteConnectionRebind::Reject), + ) + .await + .unwrap(); + assert_eq!(upgraded.id, original.id); + assert_eq!( + upgraded.remote_ssh_connection_id(), + Some("ssh-root@remote.example") + ); + + let error = manager + .open_workspace_with_options( + "/srv/shared".into(), + reopen("ssh-deploy@remote.example", RemoteConnectionRebind::Reject), + ) + .await + .unwrap_err() + .to_string(); + assert!(error.contains(REMOTE_WORKSPACE_CONNECTION_CONFLICT)); + assert!(error.contains("bound to SSH connection ssh-root@remote.example;")); + let wrapped = format!("Failed to open remote workspace: {error}"); + assert!(remote_workspace_connection_conflict_message(&wrapped) + .is_some_and(|message| message.starts_with("remote_workspace_connection_conflict: "))); + assert_eq!( + remote_workspace_connection_conflict_message( + "Remote workspace path is not a directory" + ), + None + ); + assert_eq!( + manager + .get_workspace(&original.id) + .unwrap() + .remote_ssh_connection_id(), + Some("ssh-root@remote.example") + ); + + for (connection_id, rebind) in [ + ( + "ssh-deploy@remote.example", + RemoteConnectionRebind::UserConfirmed, + ), + ( + "ssh-ops@remote.example", + RemoteConnectionRebind::PreviousOwnerMissing, + ), + ] { + let rebound = manager + .open_workspace_with_options("/srv/shared".into(), reopen(connection_id, rebind)) + .await + .unwrap(); + assert_eq!(rebound.id, original.id); + assert_eq!(rebound.remote_ssh_connection_id(), Some(connection_id)); + } + } + #[test] fn workspace_identity_reads_optional_avatar_without_requiring_it() { let with_avatar = WorkspaceIdentity::from_markdown( diff --git a/src/crates/assembly/core/src/service/workspace/mod.rs b/src/crates/assembly/core/src/service/workspace/mod.rs index 6a5f4da669..e268184442 100644 --- a/src/crates/assembly/core/src/service/workspace/mod.rs +++ b/src/crates/assembly/core/src/service/workspace/mod.rs @@ -42,10 +42,11 @@ pub use factory::WorkspaceFactory; pub use identity_watch::WorkspaceIdentityWatchService; #[cfg(feature = "workspace-runtime")] pub use manager::{ - GitInfo, PrimaryAssistantKey, RelatedPath, ScanOptions, WorkspaceIdentity, WorkspaceInfo, - WorkspaceInfoRuntimeExt, WorkspaceKind, WorkspaceManager, WorkspaceManagerConfig, - WorkspaceManagerStatistics, WorkspaceOpenOptions, WorkspaceStatistics, WorkspaceStatus, - WorkspaceSummary, WorkspaceType, WorkspaceWorktreeInfo, + remote_workspace_connection_conflict_message, GitInfo, PrimaryAssistantKey, RelatedPath, + RemoteConnectionRebind, ScanOptions, WorkspaceIdentity, WorkspaceInfo, WorkspaceInfoRuntimeExt, + WorkspaceKind, WorkspaceManager, WorkspaceManagerConfig, WorkspaceManagerStatistics, + WorkspaceOpenOptions, WorkspaceStatistics, WorkspaceStatus, WorkspaceSummary, WorkspaceType, + WorkspaceWorktreeInfo, REMOTE_WORKSPACE_CONNECTION_CONFLICT, }; #[cfg(feature = "workspace-runtime")] pub use provider::{WorkspaceCleanupResult, WorkspaceProvider, WorkspaceSystemSummary}; diff --git a/src/crates/assembly/core/src/service/workspace/service.rs b/src/crates/assembly/core/src/service/workspace/service.rs index a1aa603fdf..e3be493f63 100644 --- a/src/crates/assembly/core/src/service/workspace/service.rs +++ b/src/crates/assembly/core/src/service/workspace/service.rs @@ -3,9 +3,10 @@ //! Provides comprehensive workspace management functionality. use super::manager::{ - PrimaryAssistantKey, RelatedPath, ScanOptions, WorkspaceIdentity, WorkspaceInfo, WorkspaceKind, - WorkspaceManager, WorkspaceManagerConfig, WorkspaceManagerStatistics, WorkspaceOpenOptions, - WorkspaceStatus, WorkspaceSummary, WorkspaceType, WorkspaceWorktreeInfo, + remote_connection_ids_equivalent, PrimaryAssistantKey, RelatedPath, RemoteConnectionRebind, + ScanOptions, WorkspaceIdentity, WorkspaceInfo, WorkspaceKind, WorkspaceManager, + WorkspaceManagerConfig, WorkspaceManagerStatistics, WorkspaceOpenOptions, WorkspaceStatus, + WorkspaceSummary, WorkspaceType, WorkspaceWorktreeInfo, }; use super::manager::{WorkspaceIdentityRuntimeExt, WorkspaceInfoRuntimeExt}; use super::persistence::{ @@ -44,6 +45,12 @@ use tokio::sync::RwLock; const MAX_WORKSPACE_NAME_CHARS: usize = 80; +fn remote_connection_owner_missing(owner: &str, saved_connection_ids: &[String]) -> bool { + !saved_connection_ids + .iter() + .any(|saved| remote_connection_ids_equivalent(saved, owner)) +} + /// Workspace service. pub struct WorkspaceService { manager: Arc>, @@ -71,6 +78,8 @@ pub struct WorkspaceCreateOptions { pub remote_ssh_host: Option, /// Deterministic id for [`WorkspaceKind::Remote`] (host + remote path hash). pub stable_workspace_id: Option, + /// See [`crate::service::workspace::manager::RemoteConnectionRebind`]. + pub remote_connection_rebind: RemoteConnectionRebind, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -93,6 +102,7 @@ impl Default for WorkspaceCreateOptions { remote_connection_id: None, remote_ssh_host: None, stable_workspace_id: None, + remote_connection_rebind: RemoteConnectionRebind::Reject, } } } @@ -407,6 +417,7 @@ impl WorkspaceService { "Remote workspace support is not compiled into this product profile", )); } + let options = self.resolve_remote_connection_rebind(&path, options).await; let worktree = if options.workspace_kind == WorkspaceKind::Remote { None } else { @@ -530,9 +541,87 @@ impl WorkspaceService { } } + /// Upgrades the rebind policy when the record's previous owner is gone. + /// + /// A connection that is no longer saved can never reopen its record, so + /// moving the record to the requested connection loses no routing. A saved + /// owner keeps the record until the user explicitly confirms the move. + async fn resolve_remote_connection_rebind( + &self, + path: &Path, + mut options: WorkspaceCreateOptions, + ) -> WorkspaceCreateOptions { + if options.remote_connection_rebind != RemoteConnectionRebind::Reject { + return options; + } + let Some(owner) = self + .conflicting_remote_connection_owner(path, &options) + .await + else { + return options; + }; + if let Some(saved) = Self::saved_ssh_connection_ids().await { + if remote_connection_owner_missing(&owner, &saved) { + options.remote_connection_rebind = RemoteConnectionRebind::PreviousOwnerMissing; + } + } + options + } + + /// Connection that owns the record `options` would reuse, when it is not + /// the requested connection. + async fn conflicting_remote_connection_owner( + &self, + path: &Path, + options: &WorkspaceCreateOptions, + ) -> Option { + if options.workspace_kind != WorkspaceKind::Remote { + return None; + } + let requested = options + .remote_connection_id + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty())?; + let manager = self.manager.read().await; + let owner = manager + .existing_remote_workspace(path, &Self::to_manager_open_options(options))? + .remote_ssh_connection_id()?; + (!remote_connection_ids_equivalent(owner, requested)).then(|| owner.to_string()) + } + + /// Saved SSH connection ids, or `None` when the host cannot prove which + /// connections exist. + async fn saved_ssh_connection_ids() -> Option> { + #[cfg(feature = "ssh-remote")] + { + let ssh = get_remote_workspace_manager()?.get_ssh_manager().await?; + Some( + ssh.get_saved_connections() + .await + .into_iter() + .map(|profile| profile.id) + .collect(), + ) + } + #[cfg(not(feature = "ssh-remote"))] + { + None + } + } + pub(crate) async fn open_known_remote_workspace( &self, known: &WorkspaceInfo, + ) -> OpenBitFunResult { + self.open_known_remote_workspace_with_rebind(known, RemoteConnectionRebind::Reject) + .await + } + + pub(crate) async fn open_known_remote_workspace_with_rebind( + &self, + known: &WorkspaceInfo, + remote_connection_rebind: RemoteConnectionRebind, ) -> OpenBitFunResult { let connection_id = known.remote_ssh_connection_id().ok_or_else(|| { OpenBitFunError::service(format!( @@ -560,6 +649,7 @@ impl WorkspaceService { remote_connection_id: Some(connection_id.to_string()), remote_ssh_host: Some(ssh_host.to_string()), stable_workspace_id: Some(known.id.clone()), + remote_connection_rebind, ..Default::default() }; @@ -1291,6 +1381,7 @@ impl WorkspaceService { .filter(|s| !s.is_empty()) .map(|s| s.to_string()), stable_workspace_id: None, + remote_connection_rebind: RemoteConnectionRebind::Reject, }, ) .await?; @@ -1955,6 +2046,7 @@ impl WorkspaceService { remote_connection_id: options.remote_connection_id.clone(), remote_ssh_host: options.remote_ssh_host.clone(), stable_workspace_id: options.stable_workspace_id.clone(), + remote_connection_rebind: options.remote_connection_rebind, } } @@ -2879,6 +2971,164 @@ mod tests { ); } + #[tokio::test] + async fn imported_remote_profiles_sharing_a_mirror_stay_activatable_and_round_trip() { + let env = TestEnvironment::new(); + let service = build_test_workspace_service(env.path_manager.clone()).await; + let first = WorkspaceInfo::new_without_worktree( + "/srv/shared".into(), + WorkspaceOpenOptions { + workspace_kind: WorkspaceKind::Remote, + remote_connection_id: Some("first-endpoint".into()), + remote_ssh_host: Some("same-host".into()), + ..Default::default() + }, + ) + .await + .unwrap(); + let mut second = first.clone(); + second.id = "opaque-imported-workspace".into(); + second + .metadata + .insert("connectionId".into(), serde_json::json!("second-endpoint")); + let old_payload = serde_json::json!({ + "workspaces": [first, second], + "current_workspace_id": null, + "recent_workspaces": [], + "export_timestamp": "2026-01-01T00:00:00Z", + "version": "1.0.0" + }); + let imported: WorkspaceExport = serde_json::from_value(old_payload).unwrap(); + service.import_workspaces(imported, true).await.unwrap(); + // Session identity verification separates the two profiles; workspace + // activation must keep both records usable. + for (id, connection_id) in [ + (&first.id, "first-endpoint"), + (&second.id, "second-endpoint"), + ] { + let opened = service.open_workspace_by_id(id).await.unwrap(); + assert_eq!(opened.remote_ssh_connection_id(), Some(connection_id)); + assert_eq!( + service.require_workspace(id).await.unwrap().id.as_str(), + id.as_str() + ); + } + let exported = service.export_workspaces().await.unwrap(); + let round_trip: WorkspaceExport = + serde_json::from_slice(&serde_json::to_vec(&exported).unwrap()).unwrap(); + for (id, connection_id) in [ + (&first.id, "first-endpoint"), + (&second.id, "second-endpoint"), + ] { + let workspace = round_trip + .workspaces + .iter() + .find(|workspace| &workspace.id == id) + .unwrap(); + assert_eq!(workspace.remote_ssh_connection_id(), Some(connection_id)); + } + } + + #[tokio::test] + async fn reopening_remote_record_detects_owner_conflicts_and_honors_confirmation() { + let env = TestEnvironment::new(); + let service = build_test_workspace_service(env.path_manager.clone()).await; + let options = WorkspaceCreateOptions { + workspace_kind: WorkspaceKind::Remote, + remote_connection_id: Some("ssh-root@remote.example".into()), + remote_ssh_host: Some("remote.example".into()), + ..Default::default() + }; + let original = service + .open_workspace_with_options("/srv/shared".into(), options.clone()) + .await + .unwrap(); + let reopen = |connection_id: &str| WorkspaceCreateOptions { + remote_connection_id: Some(connection_id.into()), + ..options.clone() + }; + let path = Path::new("/srv/shared"); + assert_eq!( + service + .conflicting_remote_connection_owner(path, &reopen("ssh-deploy@remote.example")) + .await + .as_deref(), + Some("ssh-root@remote.example") + ); + for equivalent in ["ssh-root@remote.example", "ssh-root@remote.example:22"] { + assert_eq!( + service + .conflicting_remote_connection_owner(path, &reopen(equivalent)) + .await, + None + ); + } + let other_root = WorkspaceCreateOptions { + remote_connection_id: Some("ssh-deploy@remote.example".into()), + ..options.clone() + }; + assert_eq!( + service + .conflicting_remote_connection_owner(Path::new("/srv/other"), &other_root) + .await, + None + ); + assert_eq!( + service + .require_workspace(&original.id) + .await + .unwrap() + .remote_ssh_connection_id(), + Some("ssh-root@remote.example") + ); + let rebound = service + .open_workspace_with_options( + "/srv/shared".into(), + WorkspaceCreateOptions { + remote_connection_id: Some("ssh-deploy@remote.example".into()), + remote_connection_rebind: RemoteConnectionRebind::UserConfirmed, + ..options + }, + ) + .await + .unwrap(); + assert_eq!(rebound.id, original.id); + assert_eq!( + rebound.remote_ssh_connection_id(), + Some("ssh-deploy@remote.example") + ); + } + + #[test] + fn remote_connection_owner_is_missing_only_when_no_equivalent_profile_is_saved() { + let saved = vec![ + "ssh-root@remote.example".to_string(), + "c3f1c0de-profile".to_string(), + ]; + assert!(!remote_connection_owner_missing( + "ssh-root@remote.example", + &saved + )); + assert!(!remote_connection_owner_missing( + "ssh-root@remote.example:22", + &saved + )); + assert!(!remote_connection_owner_missing("c3f1c0de-profile", &saved)); + assert!(remote_connection_owner_missing( + "ssh-deploy@remote.example", + &saved + )); + assert!(remote_connection_owner_missing("anything", &[])); + + // Current ids for bare IPv6 hosts end in `:digits` without a port. + let ipv6 = vec!["ssh-root@fe80::1".to_string()]; + assert!(!remote_connection_owner_missing( + "ssh-root@fe80::1:22", + &ipv6 + )); + assert!(remote_connection_owner_missing("ssh-root@fe80::2", &ipv6)); + } + #[tokio::test] async fn remote_workspace_rejects_noncanonical_supplied_id() { let error = WorkspaceInfo::new( diff --git a/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs b/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs index 4b6dc0fd18..13ccbaa156 100644 --- a/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs +++ b/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs @@ -11,6 +11,7 @@ use openbitfun_product_domains::legacy_migration::{ ConflictResolution, FindingSeverity, MigrationConflict, MigrationDiagnostic, MigrationDomainId, MigrationDomainResult, MigrationDomainState, ScanFinding, }; +use openbitfun_services_core::workspace_identity::canonical_ssh_connection_id as canonical_connection_id; use openbitfun_services_integrations::remote_persistence as owner; use owner::{ KnownHostRecord, RemoteWorkspaceRecord, SavedAuthTypeRecord, SavedConnectionRecord, @@ -779,17 +780,6 @@ fn record_unavailable_workspace_references(state: &SshState, outcome: &mut Remot } } -fn canonical_connection_id(id: &str) -> String { - if let Some(rest) = id.strip_prefix("ssh-") { - if let (Some(at), Some(colon)) = (rest.find('@'), rest.rfind(':')) { - if colon > at && rest[colon + 1..].parse::().is_ok() { - return format!("ssh-{}", &rest[..colon]); - } - } - } - id.to_string() -} - fn read_staged(context: &DomainContext<'_>) -> LegacyMigrationResult { read_state(&stage_domain_dir(context, DOMAIN_DIR), false) } diff --git a/src/crates/services/services-core/src/workspace_identity.rs b/src/crates/services/services-core/src/workspace_identity.rs index 931c615a41..9803dea71d 100644 --- a/src/crates/services/services-core/src/workspace_identity.rs +++ b/src/crates/services/services-core/src/workspace_identity.rs @@ -38,6 +38,22 @@ pub fn normalize_remote_workspace_path(path: &str) -> String { s.trim_end_matches('/').to_string() } +/// Canonical form of an SSH connection id. +/// +/// Older builds generated `ssh-user@host:port`; current profiles use +/// `ssh-user@host`. Both spellings name the same saved connection, so persisted +/// workspace records written by either build compare equal after this mapping. +pub fn canonical_ssh_connection_id(id: &str) -> String { + if let Some(rest) = id.strip_prefix("ssh-") { + if let (Some(at), Some(colon)) = (rest.find('@'), rest.rfind(':')) { + if colon > at && rest[colon + 1..].parse::().is_ok() { + return format!("ssh-{}", &rest[..colon]); + } + } + } + id.to_string() +} + /// Connection id as one safe local path component. pub fn sanitize_ssh_connection_id_for_local_dir(connection_id: &str) -> String { if connection_id == "." { @@ -374,3 +390,32 @@ pub fn build_project_runtime_slug(canonical: &str) -> String { let prefix = slug[..max_prefix_len].trim_end_matches('-'); format!("{}-{}", prefix, suffix) } + +#[cfg(test)] +mod tests { + use super::canonical_ssh_connection_id; + + #[test] + fn canonical_ssh_connection_id_strips_only_legacy_port_suffixes() { + assert_eq!( + canonical_ssh_connection_id("ssh-root@example.com:22"), + "ssh-root@example.com" + ); + assert_eq!( + canonical_ssh_connection_id("ssh-root@example.com"), + "ssh-root@example.com" + ); + assert_eq!( + canonical_ssh_connection_id("ssh-root@example.com:not-a-port"), + "ssh-root@example.com:not-a-port" + ); + assert_eq!( + canonical_ssh_connection_id("ssh-user:name@example.com"), + "ssh-user:name@example.com" + ); + assert_eq!( + canonical_ssh_connection_id("0c9f6c1e-profile:22"), + "0c9f6c1e-profile:22" + ); + } +} diff --git a/src/web-ui/eslint.config.mjs b/src/web-ui/eslint.config.mjs index bd1b38d900..d82095fd09 100644 --- a/src/web-ui/eslint.config.mjs +++ b/src/web-ui/eslint.config.mjs @@ -65,6 +65,15 @@ export default tseslint.config( '业务命令必须经 api.invoke(ApiClient) 统一适配层,不可动态 import invoke。' + '如需直连平台 invoke,放到 adapters/ 内并经 api 暴露。', }, + { + // api.invoke captures the device surface when it is called, after + // its arguments were awaited. An await inside the arguments can + // therefore send the previous device's IDs or paths to the next one. + selector: "CallExpression[callee.property.name='invoke'] > * AwaitExpression, CallExpression[callee.property.name='invoke'] > AwaitExpression", + message: + 'Do not await inside api.invoke(...) arguments: the device surface is captured after ' + + 'they resolve. Use invokePrepared(command, async scope => args) instead.', + }, ], }, }, diff --git a/src/web-ui/src/app/components/NavPanel/MainNav.tsx b/src/web-ui/src/app/components/NavPanel/MainNav.tsx index a74bb05f46..e5694d9740 100644 --- a/src/web-ui/src/app/components/NavPanel/MainNav.tsx +++ b/src/web-ui/src/app/components/NavPanel/MainNav.tsx @@ -183,7 +183,9 @@ const MainNav: React.FC = () => { const handleSelectRemoteWorkspace = useCallback(async (path: string) => { try { - await sshRemote.openWorkspace(path); + if (!(await sshRemote.openWorkspace(path))) { + return; + } sshRemote.setShowFileBrowser(false); setIsSSHConnectionDialogOpen(false); } catch (err) { 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 7efb4858ae..1974c7f3f8 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 @@ -4,6 +4,7 @@ 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 { ReviewPlatformPanel } from './ReviewPlatformPanel'; +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; 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 } })); @@ -94,6 +95,32 @@ describe('Gitee panel state and asynchronous request ordering', () => { }); 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. + const path = '/same-path-on-two-devices'; + const remount = async () => { + await act(async () => root.unmount()); + root = createRoot(host); + await act(async () => { root.render(); }); + }; + try { + const late = deferred(); + mocks.snapshot.mockImplementationOnce(() => late.promise); + await remount(); + 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 remount(); + expect(mocks.snapshot).toHaveBeenCalledTimes(3); + } finally { + activateSurface('local'); + } + }); + it('uses list statistics in the selected detail while its overview is still pending', async () => { mocks.snapshot.mockImplementation(snapshotFor(result => { result.pullRequests = result.pullRequests.map((pr, index) => ({ ...pr, 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 8326ea41fb..cb811693d5 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx @@ -66,6 +66,7 @@ import { withGitRepositoryTrustRecovery, } from '@/shared/services/gitTrustService'; import type { PullRequestContext } from '@/shared/types/context'; +import { getActiveSurfaceId, onSurfaceActivated, surfaceScopedKey } from '@/infrastructure/peer-device/deviceSurface'; import { currentPullRequestReviewStatusText, effectivePullRequestReviewFreshness, @@ -169,6 +170,14 @@ const detailPageCache = new Map(); const reviewLaunchesInFlight = new Set(); const EMPTY_REVIEW_THREADS: ReviewPlatformThread[] = []; +// A device switch starts from fresh provider reads; a late response from the +// departed device can only land under that device's own keys. +onSurfaceActivated(() => { + snapshotCache.clear(); + detailCache.clear(); + detailPageCache.clear(); +}); + function detailPageInfo(pagination: ReviewPlatformPagination, itemCount: number): PageInfo { const pageIndex = Math.max(0, (pagination.page || 1) - 1); const perPage = Math.max(1, pagination.perPage || itemCount || 1); @@ -190,22 +199,27 @@ function detailPageInfo(pagination: ReviewPlatformPagination, itemCount: number) }; } -// Caches are keyed by the owning workspace ID, never by path: two workspaces -// (for example a local checkout and a remote one) may share a root path. +// Caches are keyed by the rendered device and the owning workspace ID, never +// by path: two workspaces (for example a local checkout and a remote one) may +// share a root path, and same-path local workspaces share an ID across devices. +function surfaceCachePrefix(): string { + return `${JSON.stringify(getActiveSurfaceId())}::`; +} + function snapshotCacheKey(workspaceId: string, remoteId: string | null, page: number, perPage: number, mode: 'list' | 'context', state: ListStateFilter): string { - return `${workspaceId}::${remoteId ?? 'default'}::${page}::${perPage}::${mode}::${state}`; + return `${surfaceCachePrefix()}${workspaceId}::${remoteId ?? 'default'}::${page}::${perPage}::${mode}::${state}`; } function detailCacheKey(workspaceId: string, remoteId: string, pullRequestId: string): string { - return `${workspaceId}::${remoteId}::${pullRequestId}`; + return `${surfaceCachePrefix()}${workspaceId}::${remoteId}::${pullRequestId}`; } function detailPageCacheKey(workspaceId: string, remoteId: string, pullRequestId: string, section: ReviewPlatformDetailSection, page: number, perPage: number): string { - return `${workspaceId}::${remoteId}::${pullRequestId}::${section}::${page}::${perPage}`; + return `${detailCacheKey(workspaceId, remoteId, pullRequestId)}::${section}::${page}::${perPage}`; } function clearDetailPageCacheForPullRequest(workspaceId: string, remoteId: string, pullRequestId: string): void { - const prefix = `${workspaceId}::${remoteId}::${pullRequestId}::`; + const prefix = `${detailCacheKey(workspaceId, remoteId, pullRequestId)}::`; for (const key of detailPageCache.keys()) { if (key.startsWith(prefix)) { detailPageCache.delete(key); @@ -1601,11 +1615,11 @@ export const ReviewPlatformPanel: React.FC = ({ return samePullRequestIdentity(evidence?.pullRequest, freshIdentity) && pullRequestReviewFreshness(evidence, freshPullRequest) === 'current'; }); - sharedLaunchKey = pullRequestReviewLaunchKey({ + sharedLaunchKey = surfaceScopedKey(getActiveSurfaceId(), pullRequestReviewLaunchKey({ ...freshIdentity, baseRevision: freshPullRequest.baseRevision, headRevision: freshPullRequest.headRevision, - }); + })); const cacheKey = detailCacheKey(workspaceId, selectedRemote.id, selectedPr.id); setDetail((current) => current ? { ...current, ...reviewTarget.pullRequest } : current); setSnapshot((current) => ({ diff --git a/src/web-ui/src/app/scenes/git/GitScene.tsx b/src/web-ui/src/app/scenes/git/GitScene.tsx index 6b6a525a72..fa434251c3 100644 --- a/src/web-ui/src/app/scenes/git/GitScene.tsx +++ b/src/web-ui/src/app/scenes/git/GitScene.tsx @@ -14,6 +14,7 @@ import { useCurrentWorkspace } from '@/infrastructure/contexts/WorkspaceContext' import { LoadingState } from '@openbitfun/ui'; import { globalEventBus } from '@/infrastructure/event-bus'; import { requestGitRepositoryTrust } from '@/shared/services/gitTrustService'; +import { isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; import './GitScene.scss'; interface GitSceneProps { @@ -87,6 +88,8 @@ const GitScene: React.FC = ({ if (trusted) { await refresh({ force: true, layers: ['basic', 'status'], reason: 'manual' }); } + } catch (error) { + if (!isSurfaceChangedError(error)) throw error; } finally { setIsTrusting(false); } diff --git a/src/web-ui/src/features/ssh-remote/SSHRemoteContext.ts b/src/web-ui/src/features/ssh-remote/SSHRemoteContext.ts index 14dbe9d41f..6f9916fa70 100644 --- a/src/web-ui/src/features/ssh-remote/SSHRemoteContext.ts +++ b/src/web-ui/src/features/ssh-remote/SSHRemoteContext.ts @@ -22,7 +22,8 @@ export interface SSHContextValue { options?: { browseAfterConnect?: boolean } ) => Promise; disconnect: () => Promise; - openWorkspace: (path: string) => Promise; + /** Resolves `false` when the user keeps an existing connection binding. */ + openWorkspace: (path: string) => Promise; closeWorkspace: () => Promise; setShowConnectionDialog: (show: boolean) => void; setShowFileBrowser: (show: boolean) => void; diff --git a/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.test.tsx b/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.test.tsx index 2657a5a331..31522c7c8a 100644 --- a/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.test.tsx +++ b/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.test.tsx @@ -6,12 +6,22 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { WorkspaceKind, WorkspaceType } from '@/shared/types/global-state'; import { notificationService } from '@/shared/notification-system'; +import { + activateSurface, + getActiveSurfaceId, + getActiveSurfaceScope, + isSurfaceChangedError, +} from '@/infrastructure/peer-device/deviceSurface'; import { SSHRemoteProvider } from './SSHRemoteProvider'; -import { SSHContext, type ConnectionStatus } from './SSHRemoteContext'; +import { SSHContext, type ConnectionStatus, type SSHContextValue } from './SSHRemoteContext'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; +beforeEach(() => { + activateSurface('local'); +}); + const peerModeFlagMock = vi.hoisted(() => ({ active: false })); vi.mock('@/infrastructure/peer-device/peerModeFlag', () => ({ @@ -66,6 +76,19 @@ vi.mock('@/shared/notification-system', () => ({ }, })); +const confirmWarningMock = vi.hoisted(() => vi.fn()); + +vi.mock('@/infrastructure/confirm-dialog', () => ({ + confirmWarning: confirmWarningMock, +})); + +const translate = vi.hoisted(() => (key: string, params?: Record) => + params ? `${key} ${JSON.stringify(params)}` : key); + +vi.mock('@/infrastructure/i18n', () => ({ + useI18n: () => ({ t: translate }), +})); + vi.mock('@/shared/utils/logger', () => ({ createLogger: () => ({ debug: vi.fn(), @@ -643,3 +666,352 @@ describe('SSHRemoteProvider workspace connection state', () => { expect(notificationService.error).not.toHaveBeenCalled(); }); }); + +function connectionConflictError(owner: string, requested: string): Error { + return new Error( + `remote_workspace_connection_conflict: Workspace remote_shared is bound to SSH connection ${owner}; reopening it with connection ${requested} requires confirmation.` + ); +} + +describe('SSHRemoteProvider remote workspace connection conflicts', () => { + let container: HTMLDivElement; + let root: Root; + let context: SSHContextValue | null; + + function ContextProbe() { + context = React.useContext(SSHContext); + return null; + } + + const savedConnections = [ + { + id: 'ssh-root@example.com', name: 'root-profile', host: 'example.com', port: 22, + username: 'root', authType: { type: 'PrivateKey', keyPath: '/tmp/root_key' }, + }, + { + id: 'ssh-deploy@example.com', name: 'deploy-profile', host: 'example.com', port: 22, + username: 'deploy', authType: { type: 'PrivateKey', keyPath: '/tmp/deploy_key' }, + }, + ]; + + beforeEach(() => { + vi.clearAllMocks(); + peerModeFlagMock.active = false; + context = null; + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + workspaceManagerMock.getState.mockReturnValue({ + loading: false, + openedWorkspaces: new Map(), + activeWorkspaceId: null, + }); + workspaceManagerMock.addEventListener.mockReturnValue(() => undefined); + workspaceManagerMock.consumeStartupLegacyRemoteWorkspaceSnapshot.mockReturnValue({ + available: true, + workspace: null, + }); + sshApiMock.getWorkspaceInfo.mockResolvedValue(null); + sshApiMock.listSavedConnections.mockResolvedValue(savedConnections); + sshApiMock.isConnected.mockResolvedValue(true); + sshApiMock.openWorkspace.mockResolvedValue(undefined); + sshApiMock.removeWorkspace.mockResolvedValue(undefined); + workspaceManagerMock.removeRemoteWorkspace.mockResolvedValue(undefined); + }); + + afterEach(() => { + act(() => { + root.unmount(); + }); + container.remove(); + }); + + async function renderProvider(): Promise { + await act(async () => { + root.render( + + + + ); + }); + await act(async () => { + await Promise.resolve(); + }); + } + + async function connectDeployProfile(): Promise { + sshApiMock.connect.mockResolvedValue({ + success: true, + connectionId: 'ssh-deploy@example.com', + serverInfo: { homeDir: '/home/deploy' }, + }); + await act(async () => { + await context!.connect('ssh-deploy@example.com', { + id: 'ssh-deploy@example.com', + name: 'deploy-profile', + host: 'example.com', + port: 22, + username: 'deploy', + auth: { type: 'PrivateKey', keyPath: '/tmp/deploy_key' }, + } as never); + }); + sshApiMock.openWorkspace.mockClear(); + } + + it('keeps a background-restored record bound to its owner and reports it', async () => { + workspaceManagerMock.consumeStartupLegacyRemoteWorkspaceSnapshot.mockReturnValue({ + available: true, + workspace: { + connectionId: 'ssh-deploy@example.com', + connectionName: 'deploy-profile', + remotePath: '/srv/shared', + sshHost: 'example.com', + }, + }); + workspaceManagerMock.openRemoteWorkspace.mockRejectedValue( + connectionConflictError('ssh-root@example.com', 'ssh-deploy@example.com') + ); + + await renderProvider(); + + expect(workspaceManagerMock.openRemoteWorkspace).toHaveBeenCalledTimes(1); + expect(workspaceManagerMock.openRemoteWorkspace.mock.calls[0]).toHaveLength(1); + expect(confirmWarningMock).not.toHaveBeenCalled(); + expect(notificationService.warning).toHaveBeenCalledWith( + 'ssh.remote.connectionConflictRestoreDeferred {"path":"/srv/shared"}', + { duration: 8000 } + ); + expect(workspaceManagerMock.removeRemoteWorkspace).not.toHaveBeenCalled(); + expect(sshApiMock.removeWorkspace).not.toHaveBeenCalled(); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + }); + + it('does not activate a reconnected workspace whose record has another owner', async () => { + workspaceManagerMock.consumeStartupLegacyRemoteWorkspaceSnapshot.mockReturnValue({ + available: true, + workspace: { + connectionId: 'ssh-deploy@example.com', + connectionName: 'deploy-profile', + remotePath: '/srv/shared', + sshHost: 'example.com', + }, + }); + sshApiMock.isConnected.mockResolvedValue(false); + sshApiMock.connect.mockResolvedValue({ success: true, connectionId: 'ssh-deploy@example.com' }); + workspaceManagerMock.openRemoteWorkspace.mockRejectedValue( + connectionConflictError('ssh-root@example.com', 'ssh-deploy@example.com') + ); + + await renderProvider(); + + expect(sshApiMock.connect).toHaveBeenCalledTimes(1); + expect(notificationService.warning).toHaveBeenCalled(); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + expect(context!.remoteWorkspace).toBeNull(); + }); + + it.each([true, false])('checks ownership before activating a restored workspace (connected=%s)', async alreadyConnected => { + workspaceManagerMock.consumeStartupLegacyRemoteWorkspaceSnapshot.mockReturnValue({ + available: true, + workspace: { + connectionId: 'ssh-deploy@example.com', + connectionName: 'deploy-profile', + remotePath: '/srv/shared', + sshHost: 'example.com', + }, + }); + sshApiMock.isConnected.mockResolvedValue(alreadyConnected); + sshApiMock.connect.mockResolvedValue({ success: true, connectionId: 'ssh-deploy@example.com' }); + workspaceManagerMock.openRemoteWorkspace.mockResolvedValue({ id: 'remote_shared' }); + + await renderProvider(); + + expect(workspaceManagerMock.openRemoteWorkspace.mock.invocationCallOrder[0]) + .toBeLessThan(sshApiMock.openWorkspace.mock.invocationCallOrder[0]); + expect(context!.remoteWorkspace).toMatchObject({ workspaceId: 'remote_shared' }); + }); + + it.each(['connection-probe', 'reconnect', 'ownership-check'])('abandons background restore after a device switch during %s', async phase => { + workspaceManagerMock.consumeStartupLegacyRemoteWorkspaceSnapshot.mockReturnValue({ + available: true, + workspace: { + connectionId: 'ssh-deploy@example.com', + connectionName: 'deploy-profile', + remotePath: '/srv/shared', + sshHost: 'example.com', + }, + }); + let resume!: () => void; + const paused = new Promise(resolve => { resume = resolve; }); + sshApiMock.isConnected.mockImplementation(async () => { + if (phase === 'connection-probe') await paused; + return phase !== 'reconnect'; + }); + sshApiMock.connect.mockImplementation(async () => { + await paused; + return { success: true, connectionId: 'ssh-deploy@example.com' }; + }); + workspaceManagerMock.openRemoteWorkspace.mockImplementation(async () => { + await paused; + return { id: 'remote_shared' }; + }); + await renderProvider(); + + activateSurface('peer-b'); + await act(async () => { resume(); }); + + expect(workspaceManagerMock.openRemoteWorkspace).toHaveBeenCalledTimes(phase === 'ownership-check' ? 1 : 0); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + expect(notificationService.warning).not.toHaveBeenCalled(); + expect(context!.remoteWorkspace).toBeNull(); + }); + + it.each(['peer-b', 'local'])('abandons an old confirmation after switching to %s', async nextSurface => { + await renderProvider(); + await connectDeployProfile(); + const dispatchedSurfaces: string[] = []; + workspaceManagerMock.openRemoteWorkspace.mockImplementation(async (_workspace, options) => { + dispatchedSurfaces.push(getActiveSurfaceId()); + if (!options?.rebindConnection) { + throw connectionConflictError('ssh-root@example.com', 'ssh-deploy@example.com'); + } + return { id: 'remote_shared' }; + }); + let confirm!: (value: boolean) => void; + confirmWarningMock.mockImplementation(() => new Promise(resolve => { confirm = resolve; })); + let pending!: Promise; + await act(async () => { + pending = context!.openWorkspace('/srv/shared').catch(error => error); + }); + expect(confirmWarningMock).toHaveBeenCalledTimes(1); + + activateSurface('peer-b'); + if (nextSurface === 'local') activateSurface('local'); + let result: unknown; + await act(async () => { + confirm(true); + result = await pending; + }); + + expect(isSurfaceChangedError(result)).toBe(true); + expect(dispatchedSurfaces).toEqual(['local']); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + }); + + it('does not turn a cancelled connection lookup into a new-device confirmation', async () => { + await renderProvider(); + await connectDeployProfile(); + workspaceManagerMock.openRemoteWorkspace.mockRejectedValue( + connectionConflictError('ssh-root@example.com', 'ssh-deploy@example.com') + ); + const scope = getActiveSurfaceScope(); + let rejectLookup!: (reason: unknown) => void; + sshApiMock.listSavedConnections.mockImplementation(() => new Promise((_resolve, reject) => { + rejectLookup = reject; + })); + confirmWarningMock.mockResolvedValue(true); + let pending!: Promise; + await act(async () => { + pending = context!.openWorkspace('/srv/shared').catch(error => error); + }); + + activateSurface('peer-b'); + let result: unknown; + await act(async () => { + try { scope.assertCurrent(); } catch (error) { rejectLookup(error); } + result = await pending; + }); + + expect(isSurfaceChangedError(result)).toBe(true); + expect(confirmWarningMock).not.toHaveBeenCalled(); + expect(workspaceManagerMock.openRemoteWorkspace).toHaveBeenCalledTimes(1); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + }); + + it('does not activate an interactive workspace after its device has changed', async () => { + await renderProvider(); + await connectDeployProfile(); + let finishOpen!: (value: { id: string }) => void; + workspaceManagerMock.openRemoteWorkspace.mockImplementation(() => new Promise(resolve => { finishOpen = resolve; })); + let pending!: Promise; + await act(async () => { + pending = context!.openWorkspace('/srv/shared').catch(error => error); + }); + + activateSurface('peer-b'); + let result: unknown; + await act(async () => { + finishOpen({ id: 'remote_shared' }); + result = await pending; + }); + + expect(isSurfaceChangedError(result)).toBe(true); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + }); + + it('rebinds an interactively selected record only after the user confirms', async () => { + await renderProvider(); + await connectDeployProfile(); + workspaceManagerMock.openRemoteWorkspace + .mockRejectedValueOnce(connectionConflictError('ssh-root@example.com', 'ssh-deploy@example.com')) + .mockResolvedValueOnce({ id: 'remote_shared' }); + confirmWarningMock.mockResolvedValue(true); + + let opened: boolean | undefined; + await act(async () => { + opened = await context!.openWorkspace('/srv/shared'); + }); + + expect(opened).toBe(true); + expect(confirmWarningMock).toHaveBeenCalledWith( + 'ssh.remote.connectionConflictTitle', + 'ssh.remote.connectionConflictMessage {"path":"/srv/shared","owner":"root-profile","connection":"deploy-profile"}', + { confirmText: 'ssh.remote.connectionConflictConfirm' } + ); + expect(workspaceManagerMock.openRemoteWorkspace).toHaveBeenCalledTimes(2); + expect(workspaceManagerMock.openRemoteWorkspace.mock.calls[1][1]).toEqual({ rebindConnection: true }); + expect(sshApiMock.openWorkspace).toHaveBeenCalledWith('ssh-deploy@example.com', '/srv/shared'); + expect(context!.remoteWorkspace).toMatchObject({ + workspaceId: 'remote_shared', + connectionId: 'ssh-deploy@example.com', + remotePath: '/srv/shared', + }); + }); + + it('leaves no host-side state when the user keeps the existing binding', async () => { + await renderProvider(); + await connectDeployProfile(); + workspaceManagerMock.openRemoteWorkspace.mockRejectedValue( + connectionConflictError('ssh-root@example.com', 'ssh-deploy@example.com') + ); + confirmWarningMock.mockResolvedValue(false); + + let opened: boolean | undefined; + await act(async () => { + opened = await context!.openWorkspace('/srv/shared'); + }); + + expect(opened).toBe(false); + expect(workspaceManagerMock.openRemoteWorkspace).toHaveBeenCalledTimes(1); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + expect(context!.remoteWorkspace).toBeNull(); + expect(context!.showFileBrowser).toBe(true); + }); + + it('propagates other open failures without asking to rebind', async () => { + await renderProvider(); + await connectDeployProfile(); + workspaceManagerMock.openRemoteWorkspace.mockRejectedValue( + new Error('Remote workspace path is not a directory') + ); + + let failure: unknown; + await act(async () => { + failure = await context!.openWorkspace('/srv/missing').catch(error => error); + }); + + expect(failure).toBeInstanceOf(Error); + expect(confirmWarningMock).not.toHaveBeenCalled(); + expect(sshApiMock.openWorkspace).not.toHaveBeenCalled(); + }); +}); diff --git a/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.tsx b/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.tsx index ea69f68a24..15bba0d17c 100644 --- a/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.tsx +++ b/src/web-ui/src/features/ssh-remote/SSHRemoteProvider.tsx @@ -13,6 +13,19 @@ import { ACPClientAPI } from '@/infrastructure/api/service-api/ACPClientAPI'; import { normalizeRemoteWorkspacePath } from '@/shared/utils/pathUtils'; import { notificationService } from '@/shared/notification-system'; import { isPeerDeviceModeActive } from '@/infrastructure/peer-device/peerModeFlag'; +import { + getActiveSurfaceScope, + isSurfaceChangedError, + runInSurfaceScope, + type SurfaceScope, +} from '@/infrastructure/peer-device/deviceSurface'; +import { useI18n } from '@/infrastructure/i18n'; +import { confirmWarning } from '@/infrastructure/confirm-dialog'; +import { + isRemoteWorkspaceConnectionConflictError, + remoteWorkspaceConnectionConflictOwner, +} from '@/infrastructure/api/errors/TauriCommandError'; +import type { WorkspaceInfo } from '@/shared/types'; import { SSHContext, type ConnectionStatus, @@ -147,6 +160,11 @@ interface SSHRemoteProviderProps { } export const SSHRemoteProvider: React.FC = ({ children }) => { + const { t } = useI18n('common'); + // Restore callbacks feed the startup check effect; a language change must + // not re-run a remote reconnect probe. + const translateRef = useRef(t); + translateRef.current = t; const [status, setStatus] = useState('disconnected'); const [isConnected, setIsConnected] = useState(false); const [isConnecting, setIsConnecting] = useState(false); @@ -262,12 +280,19 @@ export const SSHRemoteProvider: React.FC = ({ children } const reportRemoteWorkspaceRestoreDeferred = useCallback(( workspace: RemoteWorkspace, - reason: 'missing-connection' | 'missing-password' + reason: 'missing-connection' | 'missing-password' | 'connection-conflict' ) => { if (isPeerDeviceModeActive()) { return; } const path = normalizeRemoteWorkspacePath(workspace.remotePath); + if (reason === 'connection-conflict') { + notificationService.warning( + translateRef.current('ssh.remote.connectionConflictRestoreDeferred', { path }), + { duration: 8000 } + ); + return; + } notificationService.warning( reason === 'missing-password' ? `Remote workspace was kept. Re-enter its SSH password to reconnect: ${path}` @@ -276,6 +301,33 @@ export const SSHRemoteProvider: React.FC = ({ children } ); }, []); + /** + * Background restore never moves a record to another connection: that is a + * user decision. The record stays bound to its owner and the entry is kept. + */ + const openRestoredRemoteWorkspaceRecord = useCallback(async ( + workspace: RemoteWorkspace, + scope: SurfaceScope, + ): Promise => { + try { + return await runInSurfaceScope(scope, 'restore SSH workspace record', () => + workspaceManager.openRemoteWorkspace(workspace)); + } catch (error) { + scope.assertCurrent('restore SSH workspace record'); + if (!isRemoteWorkspaceConnectionConflictError(error)) { + throw error; + } + log.warn('Deferring remote workspace restore because its record is bound to another connection', { + connectionId: workspace.connectionId, + remotePath: workspace.remotePath, + owner: remoteWorkspaceConnectionConflictOwner(error), + }); + setWorkspaceStatus(workspace.connectionId, 'error'); + reportRemoteWorkspaceRestoreDeferred(workspace, 'connection-conflict'); + return null; + } + }, [reportRemoteWorkspaceRestoreDeferred, setWorkspaceStatus]); + // Cleanup heartbeat on unmount useEffect(() => { const statusTimeouts = workspaceStatusTimeouts.current; @@ -294,10 +346,13 @@ export const SSHRemoteProvider: React.FC = ({ children } // Fast connection failures must keep retrying inside the budget; only then remove. const tryReconnectWithRetry = useCallback(async ( workspace: RemoteWorkspace, + scope: SurfaceScope, timeoutMs: number = REMOTE_WORKSPACE_RECONNECT_TIMEOUT_MS ): Promise => { + scope.assertCurrent('reconnect SSH workspace'); const connectionKey = workspace.connectionId.trim(); - let reconnect = reconnectByConnectionRef.current.get(connectionKey); + const flightKey = scope.key(scope.epoch, connectionKey); + let reconnect = reconnectByConnectionRef.current.get(flightKey); if (!reconnect) { log.info('tryReconnectWithRetry: starting connection restore', { @@ -306,6 +361,7 @@ export const SSHRemoteProvider: React.FC = ({ children } }); reconnect = (async () => { const savedConnections = await sshApi.listSavedConnections(); + scope.assertCurrent('read SSH reconnect profile'); const savedConn = savedConnections.find(c => c.id === connectionKey); if (!savedConn) { @@ -351,6 +407,7 @@ export const SSHRemoteProvider: React.FC = ({ children } const result = await reconnectUntilDeadline({ totalTimeoutMs: timeoutMs, attempt: async (attemptTimeoutMs, attempt) => { + if (!scope.isCurrent()) return false as const; if (isPeerDeviceModeActive()) { // Abort controller-side reconnects: connecting now would open an SSH // session on the peer with controller-local credentials. @@ -364,6 +421,7 @@ export const SSHRemoteProvider: React.FC = ({ children } const connectWithTimeout = async (): Promise<{ connectionId: string }> => { const connectionResult = await sshApi.connect(reconnectConfig); + scope.assertCurrent('reconnect SSH workspace'); if (!connectionResult.success || !connectionResult.connectionId) { throw new Error(connectionResult.error || 'Connection failed'); } @@ -380,6 +438,7 @@ export const SSHRemoteProvider: React.FC = ({ children } }); return await Promise.race([connectWithTimeout(), timeoutPromise]); } catch (err) { + if (!scope.isCurrent() || isSurfaceChangedError(err)) return false as const; log.warn(`Reconnect attempt ${attempt} failed`, { connectionId: connectionKey, error: err, @@ -402,10 +461,10 @@ export const SSHRemoteProvider: React.FC = ({ children } sshHost: reconnectConfig.host?.trim() || workspace.sshHost?.trim() || undefined, }; })(); - reconnectByConnectionRef.current.set(connectionKey, reconnect); + reconnectByConnectionRef.current.set(flightKey, reconnect); const clearReconnect = () => { - if (reconnectByConnectionRef.current.get(connectionKey) === reconnect) { - reconnectByConnectionRef.current.delete(connectionKey); + if (reconnectByConnectionRef.current.get(flightKey) === reconnect) { + reconnectByConnectionRef.current.delete(flightKey); } }; void reconnect.then(clearReconnect, clearReconnect); @@ -414,13 +473,13 @@ export const SSHRemoteProvider: React.FC = ({ children } } const result = await reconnect; + scope.assertCurrent('reconnect SSH workspace'); if (result === false) { return false; } - // A connection can own several opened workspace roots. Connect once, then - // register every caller's path against the shared live transport. - await sshApi.openWorkspace(result.connectionId, workspace.remotePath); + // Connecting the transport does not authorize moving a workspace record. + // The caller checks ownership before registering this root on the host. const reconnectedWorkspace: RemoteWorkspace = { connectionId: result.connectionId, connectionName: result.connectionName, @@ -487,6 +546,7 @@ export const SSHRemoteProvider: React.FC = ({ children } return; } checkRemoteWorkspaceInFlightRef.current = true; + const scope = getActiveSurfaceScope(); try { // ── Collect all remote workspaces to reconnect ────────────────────── const wmState0 = workspaceManager.getState(); @@ -509,6 +569,7 @@ export const SSHRemoteProvider: React.FC = ({ children } // Ignore } } + scope.assertCurrent('read SSH restore snapshot'); // Opened workspaces are keyed by workspace ID. Only the pre-ID legacy // snapshot still uses connection + path, and it is dropped when an @@ -548,6 +609,7 @@ export const SSHRemoteProvider: React.FC = ({ children } const reconnectList = Array.from(toReconnect.values()); const savedConnectionsList = await sshApi.listSavedConnections(); + scope.assertCurrent('read SSH restore profiles'); const skipPasswordAutoReconnect = new Set(); const missingSavedConnections = new Set(); @@ -564,6 +626,7 @@ export const SSHRemoteProvider: React.FC = ({ children } } catch { hasVault = false; } + scope.assertCurrent('read SSH stored password'); if (!hasVault) { skipPasswordAutoReconnect.add(ws.connectionId); } @@ -596,14 +659,19 @@ export const SSHRemoteProvider: React.FC = ({ children } }, openedRemote); const alreadyConnected = await sshApi.isConnected(workspace.connectionId).catch(() => false); + scope.assertCurrent('check SSH restore connection'); if (alreadyConnected) { log.info('Remote workspace already connected', { connectionId: workspace.connectionId }); - await sshApi.openWorkspace(workspace.connectionId, workspace.remotePath).catch(() => {}); + const record = openedRecord ?? await openRestoredRemoteWorkspaceRecord(workspace, scope); + if (!record) { + return { ok: false as const }; + } + await runInSurfaceScope(scope, 'activate restored SSH workspace', () => + sshApi.openWorkspace(workspace.connectionId, workspace.remotePath)); + scope.assertCurrent('publish restored SSH workspace'); setWorkspaceStatus(workspace.connectionId, 'connected'); refreshRemoteAcpCapabilities(workspace.connectionId); - - const record = openedRecord ?? await workspaceManager.openRemoteWorkspace(workspace); workspace.workspaceId = record.id; void flowChatStore.initializeFromDisk(record.id, 'ssh_remote_auto_restore_existing').catch(() => {}); @@ -634,14 +702,20 @@ export const SSHRemoteProvider: React.FC = ({ children } remotePath: workspace.remotePath, }); setWorkspaceStatuses(prev => ({ ...prev, [workspace.connectionId]: 'connecting' })); - const result = await tryReconnectWithRetry(workspace); + const result = await tryReconnectWithRetry(workspace, scope); + scope.assertCurrent('restore reconnected SSH workspace'); if (result !== false) { log.info('Reconnection successful', { newConnectionId: result.connectionId }); + const record = openedRecord ?? await openRestoredRemoteWorkspaceRecord(result.workspace, scope); + if (!record) { + return { ok: false as const }; + } + await runInSurfaceScope(scope, 'activate reconnected SSH workspace', () => + sshApi.openWorkspace(result.connectionId, result.workspace.remotePath)); + scope.assertCurrent('publish reconnected SSH workspace'); setWorkspaceStatus(result.workspace.connectionId, 'connected'); refreshRemoteAcpCapabilities(result.connectionId); - - const record = openedRecord ?? await workspaceManager.openRemoteWorkspace(result.workspace); result.workspace.workspaceId = record.id; void flowChatStore.initializeFromDisk(record.id, 'ssh_remote_auto_restore_reconnected').catch(() => {}); @@ -661,6 +735,7 @@ export const SSHRemoteProvider: React.FC = ({ children } return { ok: false as const }; }) ); + scope.assertCurrent('select restored SSH workspace'); const connectedEntries: ConnectedEntry[] = results .filter((r): r is { ok: true; connected: ConnectedEntry } => r.ok) @@ -674,11 +749,13 @@ export const SSHRemoteProvider: React.FC = ({ children } startHeartbeatRef.current(chosen.connectionId); } } catch (e) { + if (isSurfaceChangedError(e)) return; log.error('checkRemoteWorkspace failed', e); } finally { checkRemoteWorkspaceInFlightRef.current = false; } }, [ + openRestoredRemoteWorkspaceRecord, reportRemoteWorkspaceReconnectFailure, reportRemoteWorkspaceRestoreDeferred, setWorkspaceStatus, @@ -877,30 +954,87 @@ export const SSHRemoteProvider: React.FC = ({ children } } }, [connectionId, remoteWorkspace, setWorkspaceStatus]); - const openWorkspace = useCallback(async (pingPath: string) => { + /** + * Opens the record for an interactive selection. A record bound to another + * saved connection is only moved after the user confirms; `null` means the + * user kept the existing binding. + */ + const openSelectedRemoteWorkspaceRecord = useCallback(async ( + remoteWs: RemoteWorkspace, + scope: SurfaceScope, + ): Promise => { + try { + return await runInSurfaceScope(scope, 'open SSH workspace record', () => + workspaceManager.openRemoteWorkspace(remoteWs)); + } catch (error) { + scope.assertCurrent('open SSH workspace record'); + if (!isRemoteWorkspaceConnectionConflictError(error)) { + throw error; + } + const ownerId = remoteWorkspaceConnectionConflictOwner(error); + const savedConnections = await sshApi.listSavedConnections().catch(() => []); + scope.assertCurrent('read SSH workspace owner'); + const owner = savedConnections.find(connection => connection.id === ownerId)?.name || ownerId || ''; + const confirmed = await confirmWarning( + t('ssh.remote.connectionConflictTitle'), + t('ssh.remote.connectionConflictMessage', { + path: remoteWs.remotePath, + owner, + connection: remoteWs.connectionName, + }), + { confirmText: t('ssh.remote.connectionConflictConfirm') } + ); + scope.assertCurrent('confirm SSH workspace rebind'); + if (!confirmed) { + log.info('Kept remote workspace bound to its existing connection', { + connectionId: remoteWs.connectionId, + remotePath: remoteWs.remotePath, + owner: ownerId, + }); + return null; + } + return runInSurfaceScope(scope, 'rebind SSH workspace record', () => + workspaceManager.openRemoteWorkspace(remoteWs, { rebindConnection: true })); + } + }, [t]); + + const openWorkspace = useCallback(async (pingPath: string): Promise => { + const scope = getActiveSurfaceScope(); if (!connectionId) { throw new Error('Not connected'); } const connName = connectionConfig?.name || 'Remote'; const remotePath = normalizeRemoteWorkspacePath(pingPath); - await sshApi.openWorkspace(connectionId, remotePath); const remoteWs: RemoteWorkspace = { connectionId, connectionName: connName, remotePath, sshHost: connectionConfig?.host?.trim() || undefined, }; + const previousRemoteWorkspace = remoteWorkspaceRef.current; setRemoteWorkspace(remoteWs); setShowFileBrowser(false); setWorkspaceStatus(connectionId, 'connected'); - const record = await workspaceManager.openRemoteWorkspace(remoteWs); + // The record is opened before the active remote pointer moves, so keeping + // an existing binding leaves no host-side state behind. + const record = await openSelectedRemoteWorkspaceRecord(remoteWs, scope); + scope.assertCurrent('open SSH workspace'); + if (!record) { + setRemoteWorkspace(previousRemoteWorkspace); + setShowFileBrowser(true); + return false; + } + await runInSurfaceScope(scope, 'activate selected SSH workspace', () => + sshApi.openWorkspace(connectionId, remotePath)); + scope.assertCurrent('publish selected SSH workspace'); // The opened record is the identity; keep it on the provider state so // close/disconnect can name the exact workspace instead of its connection. setRemoteWorkspace(current => current && sameRemoteWorkspace(current, remoteWs) ? { ...current, workspaceId: record.id } : current ); - }, [connectionId, connectionConfig, setWorkspaceStatus]); + return true; + }, [connectionId, connectionConfig, openSelectedRemoteWorkspaceRecord, setWorkspaceStatus]); const closeWorkspace = useCallback(async () => { const currentRemoteWorkspace = remoteWorkspace; diff --git a/src/web-ui/src/flow_chat/services/FlowChatManager.test.ts b/src/web-ui/src/flow_chat/services/FlowChatManager.test.ts index 2c49ae98d4..1855c0227d 100644 --- a/src/web-ui/src/flow_chat/services/FlowChatManager.test.ts +++ b/src/web-ui/src/flow_chat/services/FlowChatManager.test.ts @@ -398,6 +398,23 @@ describe('FlowChatManager initialization', () => { expect(storeMocks.store.loadSessionMetadataPage).not.toHaveBeenCalled(); }); + it('does not initiate old-workspace history reads after a device switch during listener setup', async () => { + const listenerInitialization = createDeferred<() => void>(); + storeMocks.initializeEventListeners.mockReturnValue(listenerInitialization.promise); + storeMocks.store = { + registerPersistUnreadCompletionCallback: vi.fn(), + getSurfaceGeneration: vi.fn(() => 0), + loadSessionMetadataPage: vi.fn(), + }; + const manager = FlowChatManager.getInstance(); + const pending = manager.initialize(workspaceFixture('/same/repo', undefined, undefined), undefined); + await flushAsyncWork(); + activateSurface('other-device'); + listenerInitialization.resolve(vi.fn()); + await expect(pending).rejects.toSatisfy(isSurfaceChangedError); + expect(storeMocks.store.loadSessionMetadataPage).not.toHaveBeenCalled(); + }); + it('reuses concurrent initialization for the same workspace history restore', async () => { const metadataLoad = createDeferred<{ sessions: unknown[]; @@ -683,7 +700,7 @@ describe('FlowChatManager initialization', () => { // The same repository is routinely open at the same path on two devices, so a // request key without the surface handed device A's bootstrap the in-flight // initialization of device B — and A then read back B's session list. - it('does not deduplicate initialization for the same path across devices', async () => { + it.each(['device-b', 'local'])('does not reuse an earlier activation when initializing the same path on %s', async (destination) => { const metadataLoads = [ createDeferred>(), createDeferred>(), @@ -710,6 +727,7 @@ describe('FlowChatManager initialization', () => { expect(storeMocks.store.loadSessionMetadataPage).toHaveBeenCalledTimes(1); activateSurface('device-b'); + activateSurface(destination); const peerInitialize = manager.initialize(workspaceFixture('D:/workspace/OpenBitFun', undefined, undefined), undefined); await flushAsyncWork(); // A shared key would have handed this bootstrap the local device's request. diff --git a/src/web-ui/src/flow_chat/services/FlowChatManager.ts b/src/web-ui/src/flow_chat/services/FlowChatManager.ts index 42965d16a3..0429da544e 100644 --- a/src/web-ui/src/flow_chat/services/FlowChatManager.ts +++ b/src/web-ui/src/flow_chat/services/FlowChatManager.ts @@ -17,7 +17,6 @@ import { EventBatcher } from './EventBatcher'; import { createLogger } from '@/shared/utils/logger'; import { installSessionNavStatusService, sessionNavStatusService } from './sessionNavStatusService'; import { - getActiveSurfaceId, getActiveSurfaceScope, isSurfaceChangedError, onSurfaceActivated, @@ -187,7 +186,8 @@ export class FlowChatManager { return false; } - const requestKey = JSON.stringify([getActiveSurfaceId(), workspace.id, preferredMode ?? '']); + const scope = getActiveSurfaceScope(); + const requestKey = scope.key(scope.epoch, workspace.id, preferredMode ?? ''); const existingRequest = this.initializationRequests.get(requestKey); this.latestInitializationRequestKey = requestKey; if (existingRequest) { @@ -221,6 +221,8 @@ export class FlowChatManager { return false; } + scope.assertCurrent('initialize workspace listeners'); + const initialMetadataPage = await this.context.flowChatStore.loadSessionMetadataPage( workspaceId, 5, undefined, 'flow_chat_manager' diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts index e05bfaa664..7c5d3be18f 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts @@ -31,8 +31,9 @@ vi.mock('../session-drivers/registry', () => ({ })); const workspaceFixtures = vi.hoisted(() => new Map()); +const recentWorkspaceFixtures = vi.hoisted(() => [] as any[]); vi.mock('@/infrastructure/services/business/workspaceManager', () => ({ - workspaceManager: { getState: () => ({ openedWorkspaces: workspaceFixtures, recentWorkspaces: [] }) }, + workspaceManager: { getState: () => ({ openedWorkspaces: workspaceFixtures, recentWorkspaces: recentWorkspaceFixtures }) }, })); function fixtureWorkspaceId(rootPath: string, connectionId?: string, sshHost?: string) { const existing = [...workspaceFixtures.values()].find(record => record.rootPath === rootPath && record.connectionId === connectionId && record.sshHost === sshHost); @@ -1812,6 +1813,46 @@ describe('FlowChatStore historical session hydration state', () => { }); }); + it('lists pre-ID worktree sessions under their project without borrowing its execution identity', async () => { + const legacyWorktreeMetadata = (sessionId: string, projectPath: string) => ({ + sessionId, + title: 'Legacy worktree session', + agentType: 'Standard', + createdAt: 10, + lastActiveAt: 20, + workspacePath: `${projectPath}/tree`, + projectWorkspacePath: projectPath, + }); + + try { + const unknownProjectId = fixtureWorkspaceId('/legacy/unknown-repo', undefined, undefined); + apiMocks.listSessions.mockResolvedValueOnce([ + legacyWorktreeMetadata('legacy-worktree-unknown', '/legacy/unknown-repo'), + ]); + await flowChatStore.initializeFromDisk(unknownProjectId, undefined); + const unresolved = flowChatStore.getState().sessions.get('legacy-worktree-unknown'); + expect(unresolved?.projectWorkspaceId).toBe(unknownProjectId); + expect(unresolved?.workspaceId).toBeUndefined(); + expect(unresolved?.workspacePath).toBe('/legacy/unknown-repo/tree'); + + // A worktree the host knows about but has not opened still owns it. + const knownProjectId = fixtureWorkspaceId('/legacy/known-repo', undefined, undefined); + recentWorkspaceFixtures.push({ + id: 'legacy-worktree-record', rootPath: '/legacy/known-repo/tree', workspaceKind: 'normal', + }); + apiMocks.listSessions.mockResolvedValueOnce([ + legacyWorktreeMetadata('legacy-worktree-known', '/legacy/known-repo'), + ]); + await flowChatStore.initializeFromDisk(knownProjectId, undefined); + expect(flowChatStore.getState().sessions.get('legacy-worktree-known')).toMatchObject({ + workspaceId: 'legacy-worktree-record', + projectWorkspaceId: knownProjectId, + }); + } finally { + recentWorkspaceFixtures.length = 0; + } + }); + it('keeps persisted workspace identity separate from remote execution scope', async () => { apiMocks.listSessions.mockResolvedValueOnce([ { diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.ts index 6a757907b0..893a382bd2 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.ts @@ -130,6 +130,15 @@ import { const log = createLogger('FlowChatStore'); +/** + * Records that can own a pre-ID session projection. A linked worktree may be + * known to the host without being opened, so recent records are candidates too. + */ +function legacySessionWorkspaceCandidates(): WorkspaceInfo[] { + const state = workspaceManager.getState(); + return [...state.openedWorkspaces.values(), ...(state.recentWorkspaces ?? [])]; +} + function firstNonEmptyString(...values: unknown[]): string | undefined { for (const value of values) { if (typeof value === 'string' && value.trim()) { @@ -7365,14 +7374,15 @@ export class FlowChatStore { } } + const legacyRecords = legacySessionWorkspaceCandidates(); const workspaceId = metadata.workspaceId ?? resolveLegacySessionWorkspace({ workspacePath: metadata.workspacePath || workspacePath, projectWorkspacePath: metadata.projectWorkspacePath, remoteConnectionId, remoteSshHost, - }, [...workspaceManager.getState().openedWorkspaces.values()])?.id; + }, legacyRecords)?.id; const projectWorkspaceId = metadata.projectWorkspaceId ?? resolveLegacySessionWorkspace({ workspacePath: metadata.projectWorkspacePath || workspacePath, remoteConnectionId, remoteSshHost, - }, [...workspaceManager.getState().openedWorkspaces.values()])?.id; + }, legacyRecords)?.id; const relationship = deriveSessionRelationshipFromMetadata(metadata); const lastFinishedAt = deriveLastFinishedAtFromMetadata(metadata); const titleState = deriveSessionTitleStateFromMetadata(metadata); @@ -7837,14 +7847,15 @@ export class FlowChatStore { } } + const legacyRecords = legacySessionWorkspaceCandidates(); const workspaceId = metadata.workspaceId ?? resolveLegacySessionWorkspace({ workspacePath: metadata.workspacePath || workspacePath, projectWorkspacePath: metadata.projectWorkspacePath, remoteConnectionId, remoteSshHost, - }, [...workspaceManager.getState().openedWorkspaces.values()])?.id; + }, legacyRecords)?.id; const projectWorkspaceId = metadata.projectWorkspaceId ?? resolveLegacySessionWorkspace({ workspacePath: metadata.projectWorkspacePath || workspacePath, remoteConnectionId, remoteSshHost, - }, [...workspaceManager.getState().openedWorkspaces.values()])?.id; + }, legacyRecords)?.id; const relationship = deriveSessionRelationshipFromMetadata(metadata); const lastFinishedAt = deriveLastFinishedAtFromMetadata(metadata); const titleState = deriveSessionTitleStateFromMetadata(metadata); diff --git a/src/web-ui/src/infrastructure/api/errors/TauriCommandError.test.ts b/src/web-ui/src/infrastructure/api/errors/TauriCommandError.test.ts index fa9c9c4c54..1c6686e0f7 100644 --- a/src/web-ui/src/infrastructure/api/errors/TauriCommandError.test.ts +++ b/src/web-ui/src/infrastructure/api/errors/TauriCommandError.test.ts @@ -1,13 +1,22 @@ import { describe, expect, it } from 'vitest'; import { + createTauriCommandError, gitRepositoryUntrustedPath, isGitRepositoryNotFoundError, isGitRepositoryUntrustedError, isNotAvailableError, isOutcomeUnknownError, + isRemoteWorkspaceConnectionConflictError, isSessionInUseError, + remoteWorkspaceConnectionConflictOwner, TauriCommandError, } from './TauriCommandError'; +import { SurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; + +it('preserves device cancellation through command error translation', () => { + const cancellation = new SurfaceChangedError('peer', 4, 'list_sessions'); + expect(createTauriCommandError('list_sessions', cancellation)).toBe(cancellation); +}); describe('isGitRepositoryNotFoundError', () => { const legacyError = "Failed to get Git status: Repository not found: could not find repository at '/workspace'; class=Repository (6); code=NotFound (-3)"; @@ -159,3 +168,30 @@ describe('isGitRepositoryUntrustedError', () => { ).toBeUndefined(); }); }); + +describe('isRemoteWorkspaceConnectionConflictError', () => { + const conflict = + 'remote_workspace_connection_conflict: Workspace remote_1 is bound to SSH connection ssh-root@example.com; reopening it with connection ssh-deploy@example.com requires confirmation.'; + + it.each([ + conflict, + new TauriCommandError('Command failed', { + command: 'open_remote_workspace', + originalError: conflict, + }), + { message: 'Host command failed', details: { originalError: conflict } }, + { message: 'Internal error', data: conflict }, + ])('recognizes the stable code through Desktop and Peer wrappers: %j', (error) => { + expect(isRemoteWorkspaceConnectionConflictError(error)).toBe(true); + expect(remoteWorkspaceConnectionConflictOwner(error)).toBe('ssh-root@example.com'); + }); + + it.each([ + 'Failed to open remote workspace: Remote workspace path is not a directory', + 'Failed to open remote workspace: remote_workspace_connection_conflict: wrapped by an old host', + 'remote_workspace_storage_conflict: Workspace remote_1 is owned by SSH connection ssh-root@example.com', + ])('does not classify other failures: %s', (message) => { + expect(isRemoteWorkspaceConnectionConflictError(new Error(message))).toBe(false); + expect(remoteWorkspaceConnectionConflictOwner(new Error(message))).toBeUndefined(); + }); +}); diff --git a/src/web-ui/src/infrastructure/api/errors/TauriCommandError.ts b/src/web-ui/src/infrastructure/api/errors/TauriCommandError.ts index 8446deee28..980fe766fe 100644 --- a/src/web-ui/src/infrastructure/api/errors/TauriCommandError.ts +++ b/src/web-ui/src/infrastructure/api/errors/TauriCommandError.ts @@ -1,4 +1,4 @@ - +import { isSurfaceChangedError, type SurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; export interface TauriCommandErrorContext { command: string; @@ -79,7 +79,11 @@ export function createTauriCommandError( command: string, originalError: any, request?: any -): TauriCommandError { +): TauriCommandError | SurfaceChangedError { + // Device activation cancellation is control flow across every service API. + // Wrapping it would make callers report/retry stale work on the next host. + if (isSurfaceChangedError(originalError)) return originalError; + let message = 'Unknown error'; if (originalError?.message) { @@ -198,6 +202,22 @@ export function isGitUnavailableError(error: unknown): boolean { return hasStableErrorPrefix(error, 'git_unavailable:'); } +const REMOTE_WORKSPACE_CONNECTION_CONFLICT_PREFIX = 'remote_workspace_connection_conflict:'; + +/** + * Identifies a remote workspace record bound to another saved SSH connection. + * The host keeps the record until the user confirms moving it. + */ +export function isRemoteWorkspaceConnectionConflictError(error: unknown): boolean { + return hasStableErrorPrefix(error, REMOTE_WORKSPACE_CONNECTION_CONFLICT_PREFIX); +} + +/** Connection that currently owns the conflicting remote workspace record. */ +export function remoteWorkspaceConnectionConflictOwner(error: unknown): string | undefined { + const payload = stableErrorPayload(error, REMOTE_WORKSPACE_CONNECTION_CONFLICT_PREFIX); + return payload?.match(/bound to SSH connection (\S+);/)?.[1]; +} + /** Stable Review-platform failure kind, preserved through transport wrappers. */ export function reviewPlatformErrorCode(error: unknown): string | undefined { return stableErrorPayload(error, 'review_platform_error:')?.split(':', 1)[0].trim(); diff --git a/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.test.ts index 2492d139cf..b21253a4b4 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.test.ts @@ -30,6 +30,64 @@ describe('ACPClientAPI client list startup cache', () => { vi.stubGlobal('window', { dispatchEvent: vi.fn() }); }); + it.each(['clients', 'requirements'])('never reuses cached %s from another device', async (kind) => { + const ACPClientAPI = await importApi(); + const { activateSurface } = await import('@/infrastructure/peer-device/deviceSurface'); + const read = () => kind === 'clients' ? ACPClientAPI.getClients() + : ACPClientAPI.probeClientRequirements({ remoteConnectionId: 'same-connection-id' }); + invokeMock.mockResolvedValueOnce([{ id: 'first-device' }]).mockResolvedValueOnce([{ id: 'second-device' }]); + expect(await read()).toEqual([{ id: 'first-device' }]); + activateSurface('peer-b'); + expect(await read()).toEqual([{ id: 'second-device' }]); + expect(invokeMock).toHaveBeenCalledTimes(2); + }); + + it('keeps a new activation request when a previous request settles late', async () => { + const ACPClientAPI = await importApi(); + const { activateSurface, isSurfaceChangedError } = await import('@/infrastructure/peer-device/deviceSurface'); + const oldRequest = createDeferred<[]>(); + const currentRequest = createDeferred<[]>(); + invokeMock.mockReturnValueOnce(oldRequest.promise).mockReturnValueOnce(currentRequest.promise); + const old = ACPClientAPI.getClients(); + const rejection = expect(old).rejects.toSatisfy(isSurfaceChangedError); + activateSurface('peer-b'); + activateSurface('local'); + const current = ACPClientAPI.getClients(); + oldRequest.resolve([]); + await rejection; + const duplicate = ACPClientAPI.getClients(); + expect(invokeMock).toHaveBeenCalledTimes(2); + currentRequest.resolve([]); + await expect(Promise.all([current, duplicate])).resolves.toEqual([[], []]); + }); + + it('never saves an ACP config read from a previous device to the current device', async () => { + const ACPClientAPI = await importApi(); + const { activateSurface, isSurfaceChangedError } = await import('@/infrastructure/peer-device/deviceSurface'); + const deferred = createDeferred(); + invokeMock.mockReturnValueOnce(deferred.promise); + const pending = ACPClientAPI.updateClientSubagentConfig({ clientId: 'codex', enabled: true }); + const rejection = expect(pending).rejects.toSatisfy(isSurfaceChangedError); + activateSurface('peer-b'); + deferred.resolve(JSON.stringify({ acpClients: { codex: { command: 'codex' } } })); + await rejection; + expect(invokeMock).toHaveBeenCalledTimes(1); + expect(window.dispatchEvent).not.toHaveBeenCalled(); + }); + + it('does not publish an ACP change after its originating device is deactivated', async () => { + const ACPClientAPI = await importApi(); + const { activateSurface, isSurfaceChangedError } = await import('@/infrastructure/peer-device/deviceSurface'); + const deferred = createDeferred(); + invokeMock.mockReturnValueOnce(deferred.promise); + const pending = ACPClientAPI.stopClient({ clientId: 'codex' }); + const rejection = expect(pending).rejects.toSatisfy(isSurfaceChangedError); + activateSurface('peer-b'); + deferred.resolve(); + await rejection; + expect(window.dispatchEvent).not.toHaveBeenCalled(); + }); + it('deduplicates concurrent client list requests', async () => { const ACPClientAPI = await importApi(); const clients = [ diff --git a/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts index a47ee80a6f..e6a076357b 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ACPClientAPI.ts @@ -1,4 +1,5 @@ import { api } from './ApiClient'; +import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; import type { ImageContextData as ImageInputContextData } from './ImageContextTypes'; export type AcpClientPermissionMode = 'ask' | 'allow_once' | 'reject_once'; @@ -238,6 +239,8 @@ const requirementProbeCache = new Map(); const requirementProbeInFlight = new Map>(); export class ACPClientAPI { + private static cacheScope = getActiveSurfaceScope(); + private static clientListCache: { clients: AcpClientInfo[]; expiresAt: number; @@ -255,14 +258,27 @@ export class ACPClientAPI { requirementProbeInFlight.clear(); } + private static prepareCacheScope() { + const scope = getActiveSurfaceScope(); + if (ACPClientAPI.cacheScope.epoch !== scope.epoch) { + ACPClientAPI.invalidateClientListCache(); + ACPClientAPI.invalidateRequirementProbeCache(); + ACPClientAPI.cacheScope = scope; + } + return scope; + } + static async initializeClients(): Promise { + const scope = getActiveSurfaceScope(); await api.invoke('initialize_acp_clients'); + scope.assertCurrent('publish ACP client changes'); ACPClientAPI.invalidateClientListCache(); ACPClientAPI.invalidateRequirementProbeCache(); window.dispatchEvent(new Event('openbitfun:acp-clients-changed')); } static async getClients(): Promise { + const scope = ACPClientAPI.prepareCacheScope(); const now = Date.now(); if (ACPClientAPI.clientListCache && ACPClientAPI.clientListCache.expiresAt > now) { return ACPClientAPI.clientListCache.clients; @@ -273,6 +289,7 @@ export class ACPClientAPI { const inFlight = api.invoke('get_acp_clients') .then((clients) => { + scope.assertCurrent('cache ACP clients'); ACPClientAPI.clientListCache = { clients, expiresAt: Date.now() + CLIENT_LIST_CACHE_TTL_MS, @@ -280,7 +297,9 @@ export class ACPClientAPI { return clients; }) .finally(() => { - ACPClientAPI.clientListInFlight = null; + if (ACPClientAPI.clientListInFlight === inFlight) { + ACPClientAPI.clientListInFlight = null; + } }); ACPClientAPI.clientListInFlight = inFlight; @@ -290,6 +309,7 @@ export class ACPClientAPI { static async probeClientRequirements( options: { force?: boolean; remoteConnectionId?: string } = {} ): Promise { + const scope = ACPClientAPI.prepareCacheScope(); const cacheKey = options.remoteConnectionId || LOCAL_REQUIREMENT_CACHE_KEY; if (!options.force && requirementProbeCache.has(cacheKey)) { return requirementProbeCache.get(cacheKey) ?? []; @@ -304,12 +324,15 @@ export class ACPClientAPI { const inFlight = api.invoke('probe_acp_client_requirements', { request }) .then((probes) => { + scope.assertCurrent('cache ACP client requirements'); requirementProbeCache.set(cacheKey, probes); window.dispatchEvent(new Event('openbitfun:acp-requirements-changed')); return probes; }) .finally(() => { - requirementProbeInFlight.delete(cacheKey); + if (requirementProbeInFlight.get(cacheKey) === inFlight) { + requirementProbeInFlight.delete(cacheKey); + } }); requirementProbeInFlight.set(cacheKey, inFlight); @@ -317,21 +340,27 @@ export class ACPClientAPI { } static async predownloadClientAdapter(request: AcpClientIdRequest): Promise { + const scope = getActiveSurfaceScope(); await api.invoke('predownload_acp_client_adapter', { request }); + scope.assertCurrent('publish ACP client requirements'); ACPClientAPI.invalidateClientListCache(); ACPClientAPI.invalidateRequirementProbeCache(); window.dispatchEvent(new Event('openbitfun:acp-requirements-changed')); } static async installClientCli(request: AcpClientIdRequest): Promise { + const scope = getActiveSurfaceScope(); await api.invoke('install_acp_client_cli', { request }); + scope.assertCurrent('publish ACP client requirements'); ACPClientAPI.invalidateClientListCache(); ACPClientAPI.invalidateRequirementProbeCache(); window.dispatchEvent(new Event('openbitfun:acp-requirements-changed')); } static async stopClient(request: AcpClientIdRequest): Promise { + const scope = getActiveSurfaceScope(); await api.invoke('stop_acp_client', { request }); + scope.assertCurrent('publish ACP client changes'); ACPClientAPI.invalidateClientListCache(); window.dispatchEvent(new Event('openbitfun:acp-clients-changed')); } @@ -341,7 +370,9 @@ export class ACPClientAPI { } static async saveJsonConfig(jsonConfig: string): Promise { + const scope = getActiveSurfaceScope(); await api.invoke('save_acp_json_config', { jsonConfig }); + scope.assertCurrent('publish ACP client changes'); ACPClientAPI.invalidateClientListCache(); ACPClientAPI.invalidateRequirementProbeCache(); window.dispatchEvent(new Event('openbitfun:acp-clients-changed')); @@ -350,7 +381,10 @@ export class ACPClientAPI { static async updateClientSubagentConfig( request: UpdateAcpClientSubagentConfigRequest ): Promise { - const rawConfig = JSON.parse(await ACPClientAPI.loadJsonConfig()) as unknown; + const scope = getActiveSurfaceScope(); + const jsonConfig = await ACPClientAPI.loadJsonConfig(); + scope.assertCurrent('update ACP client configuration'); + const rawConfig = JSON.parse(jsonConfig) as unknown; if (!rawConfig || typeof rawConfig !== 'object' || Array.isArray(rawConfig)) { throw new Error('ACP client configuration is invalid'); } @@ -388,7 +422,9 @@ export class ACPClientAPI { static async createFlowSession( request: CreateAcpFlowSessionRequest ): Promise { + const scope = getActiveSurfaceScope(); const response = await api.invoke('create_acp_flow_session', { request }); + scope.assertCurrent('publish ACP client changes'); ACPClientAPI.invalidateClientListCache(); window.dispatchEvent(new Event('openbitfun:acp-clients-changed')); return response; diff --git a/src/web-ui/src/infrastructure/api/service-api/AgentAPI.ts b/src/web-ui/src/infrastructure/api/service-api/AgentAPI.ts index 125e91632e..9c764f3c9b 100644 --- a/src/web-ui/src/infrastructure/api/service-api/AgentAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/AgentAPI.ts @@ -1,4 +1,4 @@ -import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { invokePrepared } from './invokePrepared'; import { workspaceHistoryRequest, workspaceIdRequest } from './legacyWorkspaceCompatibility'; import { translateAgentIdentityFields } from '../../../../../shared/agent-harness/wire'; @@ -1050,9 +1050,9 @@ export class AgentAPI { workspaceId: string ): Promise { try { - await api.invoke('delete_session', { + await invokePrepared('delete_session', async () => ({ request: { sessionId, ...await workspaceIdRequest(workspaceId, 'workspacePath') } - }); + })); } catch (error) { throw createTauriCommandError('delete_session', error, { sessionId, workspaceId }); } @@ -1065,18 +1065,15 @@ export class AgentAPI { traceId?: string, includeInternal?: boolean, ): Promise { - const scope = getActiveSurfaceScope(); try { - const workspace = await workspaceIdRequest(workspaceId, 'workspacePath'); - scope.assertCurrent(); - return await api.invoke('restore_session', { + return await invokePrepared('restore_session', async () => ({ request: { sessionId, - ...workspace, + ...await workspaceIdRequest(workspaceId, 'workspacePath'), traceId, includeInternal, }, - }); + })); } catch (error) { throw createTauriCommandError('restore_session', error, { sessionId, workspaceId }); } @@ -1088,18 +1085,15 @@ export class AgentAPI { traceId?: string, includeInternal?: boolean, ): Promise { - const scope = getActiveSurfaceScope(); try { - const workspace = await workspaceIdRequest(workspaceId, 'workspacePath'); - scope.assertCurrent(); - return await api.invoke('restore_session_with_turns', { + return await invokePrepared('restore_session_with_turns', async () => ({ request: { sessionId, - ...workspace, + ...await workspaceIdRequest(workspaceId, 'workspacePath'), traceId, includeInternal, }, - }); + })); } catch (error) { throw createTauriCommandError('restore_session_with_turns', error, { sessionId, workspaceId }); } @@ -1116,19 +1110,16 @@ export class AgentAPI { includeInternal?: boolean, tailTurnCount?: number, ): Promise { - const scope = getActiveSurfaceScope(); try { - const workspace = await workspaceIdRequest(workspaceId, 'workspacePath'); - scope.assertCurrent(); - return await api.invoke('restore_session_view', { + return await invokePrepared('restore_session_view', async () => ({ request: { sessionId, - ...workspace, + ...await workspaceIdRequest(workspaceId, 'workspacePath'), traceId, includeInternal, ...(tailTurnCount !== undefined ? { tailTurnCount } : {}), }, - }); + })); } catch (error) { throw createTauriCommandError('restore_session_view', error, { sessionId, workspaceId }); } @@ -1173,9 +1164,9 @@ export class AgentAPI { ): Promise { try { const { workspaceId, ...mutation } = request; - const outcome = await api.invoke('rollback_session_to_turn', { + const outcome = await invokePrepared('rollback_session_to_turn', async () => ({ request: { ...mutation, ...await workspaceHistoryRequest(workspaceId) }, - }); + })); if (outcome.status === 'completed') { return { ...outcome, @@ -1232,12 +1223,11 @@ export class AgentAPI { workspaceId: string; includeInternal?: boolean; }): Promise { - const scope = getActiveSurfaceScope(); try { const { workspaceId, ...session } = request; - const workspace = await workspaceIdRequest(workspaceId, 'workspacePath'); - scope.assertCurrent(); - await api.invoke('ensure_coordinator_session', { request: { ...session, ...workspace } }); + await invokePrepared('ensure_coordinator_session', async () => ({ + request: { ...session, ...await workspaceIdRequest(workspaceId, 'workspacePath') }, + })); } catch (error) { throw createTauriCommandError('ensure_coordinator_session', error, request); } @@ -1326,11 +1316,10 @@ export class AgentAPI { async listSessions(workspaceId: string): Promise { - const scope = getActiveSurfaceScope(); try { - const request = await workspaceIdRequest(workspaceId, 'workspacePath'); - scope.assertCurrent(); - return await api.invoke('list_sessions', { request }); + return await invokePrepared('list_sessions', async () => ({ + request: await workspaceIdRequest(workspaceId, 'workspacePath'), + })); } catch (error) { throw createTauriCommandError('list_sessions', error, { workspaceId }); } @@ -1713,8 +1702,9 @@ export class AgentAPI { async getAvailableModes(request: { workspaceId?: string } = {}): Promise { try { if (request.workspaceId !== undefined && !request.workspaceId.trim()) throw new Error('Workspace identity is unresolved'); - const wire = request.workspaceId !== undefined ? await workspaceIdRequest(request.workspaceId, 'workspacePath') : {}; - return translateAgentIdentityFields(await api.invoke('get_available_modes', { request: wire }), 'canonical'); + return translateAgentIdentityFields(await invokePrepared('get_available_modes', async () => ({ + request: request.workspaceId !== undefined ? await workspaceIdRequest(request.workspaceId, 'workspacePath') : {}, + })), 'canonical'); } catch (error) { throw createTauriCommandError('get_available_modes', error); } diff --git a/src/web-ui/src/infrastructure/api/service-api/CanvasAPI.ts b/src/web-ui/src/infrastructure/api/service-api/CanvasAPI.ts index 5527d21084..28079501e8 100644 --- a/src/web-ui/src/infrastructure/api/service-api/CanvasAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/CanvasAPI.ts @@ -1,4 +1,4 @@ -import { api } from './ApiClient'; +import { invokePrepared } from './invokePrepared'; import { workspaceScopedRequest } from './legacyWorkspaceCompatibility'; export interface CanvasStateValue { @@ -90,23 +90,23 @@ async function canvasRequest(request: T) { class CanvasAPI { async loadArtifact(request: CanvasStateRequest): Promise { - return api.invoke('load_canvas_artifact', { request: await canvasRequest(request) }); + return invokePrepared('load_canvas_artifact', async () => ({ request: await canvasRequest(request) })); } async loadState(request: CanvasStateRequest): Promise { - return api.invoke('load_canvas_state', { request: await canvasRequest(request) }); + return invokePrepared('load_canvas_state', async () => ({ request: await canvasRequest(request) })); } async saveState(request: SaveCanvasStateRequest): Promise { - return api.invoke('save_canvas_state', { request: await canvasRequest(request) }); + return invokePrepared('save_canvas_state', async () => ({ request: await canvasRequest(request) })); } async reportRuntimeError(request: ReportCanvasRuntimeErrorRequest): Promise { - return api.invoke('report_canvas_runtime_error', { request: await canvasRequest(request) }); + return invokePrepared('report_canvas_runtime_error', async () => ({ request: await canvasRequest(request) })); } async reportRuntimeReady(request: ReportCanvasRuntimeReadyRequest): Promise { - return api.invoke('report_canvas_runtime_ready', { request: await canvasRequest(request) }); + return invokePrepared('report_canvas_runtime_ready', async () => ({ request: await canvasRequest(request) })); } } diff --git a/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts index 557931e455..f70da44a05 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ConfigAPI.ts @@ -1,3 +1,4 @@ +import { invokePrepared } from './invokePrepared'; import { workspaceScopedRequest } from './legacyWorkspaceCompatibility'; @@ -379,9 +380,9 @@ export class ConfigAPI { workspaceId, }: GetSkillConfigsParams = {}): Promise { try { - return await api.invoke( + return await invokePrepared( 'get_skill_configs', - await workspaceScopedRequest({ forceRefresh, workspaceId }), + async () => (await workspaceScopedRequest({ forceRefresh, workspaceId })), { timeout: SKILL_CONFIG_REQUEST_TIMEOUT_MS }, ); } catch (error) { @@ -396,9 +397,9 @@ export class ConfigAPI { workspaceId, }: GetModeSkillConfigsParams): Promise { try { - return await api.invoke( + return await invokePrepared( 'get_mode_skill_configs', - await workspaceScopedRequest({ modeId, forceRefresh, workspaceId }), + async () => (await workspaceScopedRequest({ modeId, forceRefresh, workspaceId })), { timeout: SKILL_CONFIG_REQUEST_TIMEOUT_MS }, ); } catch (error) { @@ -408,8 +409,8 @@ export class ConfigAPI { async getSkillScanReport({ forceRefresh, workspaceId }: GetSkillConfigsParams = {}): Promise { try { - const response = await api.invoke>( - 'get_skill_configs', await workspaceScopedRequest({ forceRefresh, workspaceId, includeDiagnostics: true }), + const response = await invokePrepared>( + 'get_skill_configs', async () => (await workspaceScopedRequest({ forceRefresh, workspaceId, includeDiagnostics: true })), { timeout: SKILL_CONFIG_REQUEST_TIMEOUT_MS }, ); return normalizeSkillScanReport(response); @@ -420,8 +421,8 @@ export class ConfigAPI { async getModeSkillScanReport({ modeId, forceRefresh, workspaceId }: GetModeSkillConfigsParams): Promise> { try { - const response = await api.invoke, 'diagnosticsAvailable'>>( - 'get_mode_skill_configs', await workspaceScopedRequest({ modeId, forceRefresh, workspaceId, includeDiagnostics: true }), + const response = await invokePrepared, 'diagnosticsAvailable'>>( + 'get_mode_skill_configs', async () => (await workspaceScopedRequest({ modeId, forceRefresh, workspaceId, includeDiagnostics: true })), { timeout: SKILL_CONFIG_REQUEST_TIMEOUT_MS }, ); return normalizeSkillScanReport(response); @@ -432,7 +433,7 @@ export class ConfigAPI { async getGlobalSkillSettings(workspaceId?: string): Promise { try { - return await api.invoke('get_global_skill_settings', workspaceId !== undefined ? { request: await workspaceScopedRequest({ workspaceId }) } : undefined); + return await invokePrepared('get_global_skill_settings', async () => (workspaceId !== undefined ? { request: await workspaceScopedRequest({ workspaceId }) } : undefined)); } catch (error) { throw createTauriCommandError('get_global_skill_settings', error); } @@ -444,9 +445,9 @@ export class ConfigAPI { disabled, }: SetGlobalSkillDisabledParams): Promise { try { - return await api.invoke('set_global_skill_disabled', { + return await invokePrepared('set_global_skill_disabled', async () => ({ request: await workspaceScopedRequest({ skillKey, disabled, workspaceId }), - }); + })); } catch (error) { throw createTauriCommandError('set_global_skill_disabled', error, { skillKey, disabled }); } @@ -460,7 +461,7 @@ export class ConfigAPI { workspaceId, }: SetModeSkillDisabledParams): Promise { try { - return await api.invoke('set_mode_skill_disabled', await workspaceScopedRequest({ modeId, skillKey, disabled, workspaceId })); + return await invokePrepared('set_mode_skill_disabled', async () => (await workspaceScopedRequest({ modeId, skillKey, disabled, workspaceId }))); } catch (error) { throw createTauriCommandError('set_mode_skill_disabled', error, { modeId, skillKey, disabled, workspaceId }); } @@ -472,9 +473,9 @@ export class ConfigAPI { workspaceId, }: ReplaceModeSkillSelectionParams): Promise { try { - return await api.invoke('replace_mode_skill_selection', { + return await invokePrepared('replace_mode_skill_selection', async () => ({ request: await workspaceScopedRequest({ modeId, enabledSkillKeys, workspaceId }), - }); + })); } catch (error) { throw createTauriCommandError('replace_mode_skill_selection', error, { modeId, @@ -489,9 +490,9 @@ export class ConfigAPI { workspaceId, }: ResetModeSkillSelectionParams): Promise { try { - return await api.invoke('reset_mode_skill_selection', { + return await invokePrepared('reset_mode_skill_selection', async () => ({ request: await workspaceScopedRequest({ modeId, workspaceId }), - }); + })); } catch (error) { throw createTauriCommandError('reset_mode_skill_selection', error, { modeId, @@ -503,7 +504,7 @@ export class ConfigAPI { async validateSkillPath(path: string, source?: { sourceKey: string; workspaceId?: string }): Promise { try { - return await api.invoke('validate_skill_path', await workspaceScopedRequest({ path, ...source })); + return await invokePrepared('validate_skill_path', async () => (await workspaceScopedRequest({ path, ...source }))); } catch (error) { throw createTauriCommandError('validate_skill_path', error, { path }); } @@ -519,7 +520,7 @@ export class ConfigAPI { workspaceId, }: AddSkillParams): Promise { try { - return await api.invoke('add_skill', await workspaceScopedRequest({ sourcePath, level, workspaceId, ...(sourceKey ? { sourceKey } : {}), ...(targetName ? { targetName } : {}), ...(expectedSourceFingerprint !== undefined ? { expectedSourceFingerprint } : {}) })); + return await invokePrepared('add_skill', async () => (await workspaceScopedRequest({ sourcePath, level, workspaceId, ...(sourceKey ? { sourceKey } : {}), ...(targetName ? { targetName } : {}), ...(expectedSourceFingerprint !== undefined ? { expectedSourceFingerprint } : {}) }))); } catch (error) { throw createTauriCommandError('add_skill', error, { sourcePath, level, workspaceId }); } @@ -532,7 +533,7 @@ export class ConfigAPI { workspaceId, }: DeleteSkillParams): Promise { try { - return await api.invoke('delete_skill', await workspaceScopedRequest({ skillKey, workspaceId, ...(expectedImportId ? { expectedImportId } : {}) })); + return await invokePrepared('delete_skill', async () => (await workspaceScopedRequest({ skillKey, workspaceId, ...(expectedImportId ? { expectedImportId } : {}) }))); } catch (error) { throw createTauriCommandError('delete_skill', error, { skillKey, workspaceId }); } @@ -581,9 +582,9 @@ export class ConfigAPI { workspaceId, }: DownloadSkillMarketParams): Promise { try { - return await api.invoke('download_skill_market', { + return await invokePrepared('download_skill_market', async () => ({ request: await workspaceScopedRequest({ package: packageId, level, workspaceId }) - }); + })); } catch (error) { throw createTauriCommandError('download_skill_market', error, { package: packageId, diff --git a/src/web-ui/src/infrastructure/api/service-api/CronAPI.ts b/src/web-ui/src/infrastructure/api/service-api/CronAPI.ts index 30ef65f993..d9556a27df 100644 --- a/src/web-ui/src/infrastructure/api/service-api/CronAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/CronAPI.ts @@ -1,4 +1,5 @@ -import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import type { SurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { invokePrepared } from './invokePrepared'; import { upgradeLegacyCronJobs, workspaceIdRequest } from './legacyWorkspaceCompatibility'; import { api } from './ApiClient'; import { createTauriCommandError } from '../errors/TauriCommandError'; @@ -128,8 +129,10 @@ export class CronAPI { try { // Tauri listener registration is async. Wait for listeners already issued // by FlowChat before allowing cron to emit startup events. - await api.waitForListenerRegistrations(); - await api.invoke('notify_cron_host_ready'); + await invokePrepared('notify_cron_host_ready', async () => { + await api.waitForListenerRegistrations(); + return undefined; + }); } catch (error) { throw createTauriCommandError('notify_cron_host_ready', error); } @@ -137,12 +140,13 @@ export class CronAPI { async listJobs(request: ListCronJobsRequest = {}): Promise { try { - const scope = getActiveSurfaceScope(); - const identity = request.workspaceId !== undefined ? await workspaceIdRequest(request.workspaceId, 'workspacePath') : {}; - scope.assertCurrent('list scheduled jobs'); - const { workspaceId: _workspaceId, ...filters } = request; - const jobs = await api.invoke('list_cron_jobs', { request: { ...filters, ...identity } }); - scope.assertCurrent('read scheduled jobs'); + const { workspaceId, ...filters } = request; + let scope!: SurfaceScope; + const jobs = await invokePrepared('list_cron_jobs', async (current) => { + scope = current; + const identity = workspaceId !== undefined ? await workspaceIdRequest(workspaceId, 'workspacePath') : {}; + return { request: { ...filters, ...identity } }; + }); return await upgradeLegacyCronJobs(jobs, () => scope.assertCurrent('upgrade scheduled job references')); } catch (error) { throw createTauriCommandError('list_cron_jobs', error, request); @@ -151,12 +155,15 @@ export class CronAPI { async createJob(request: CreateCronJobRequest): Promise { try { - const scope = getActiveSurfaceScope(); - const workspace = await workspaceIdRequest(request.target.workspace.workspaceId, 'workspacePath'); - scope.assertCurrent('create scheduled job'); - return await api.invoke('create_cron_job', { request: { - ...request, target: { ...request.target, workspace }, - } }); + return await invokePrepared('create_cron_job', async () => ({ + request: { + ...request, + target: { + ...request.target, + workspace: await workspaceIdRequest(request.target.workspace.workspaceId, 'workspacePath'), + }, + }, + })); } catch (error) { throw createTauriCommandError('create_cron_job', error, request); } @@ -164,17 +171,17 @@ export class CronAPI { async updateJob(jobId: string, changes: UpdateCronJobRequest): Promise { try { - const scope = getActiveSurfaceScope(); - const target = changes.target ? { ...changes.target, - workspace: await workspaceIdRequest(changes.target.workspace.workspaceId, 'workspacePath'), - } : undefined; - scope.assertCurrent('update scheduled job'); - return await api.invoke('update_cron_job', { - request: { - jobId, - ...changes, - ...(target ? { target } : {}), - }, + return await invokePrepared('update_cron_job', async () => { + const target = changes.target ? { ...changes.target, + workspace: await workspaceIdRequest(changes.target.workspace.workspaceId, 'workspacePath'), + } : undefined; + return { + request: { + jobId, + ...changes, + ...(target ? { target } : {}), + }, + }; }); } catch (error) { throw createTauriCommandError('update_cron_job', error, { jobId, ...changes }); diff --git a/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.test.ts new file mode 100644 index 0000000000..c8114757a9 --- /dev/null +++ b/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.test.ts @@ -0,0 +1,30 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { CustomAgentAPI } from './CustomAgentAPI'; +import { globalEventBus } from '@/infrastructure/event-bus'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; + +const invoke = vi.hoisted(() => vi.fn()); +vi.mock('./ApiClient', () => ({ api: { invoke } })); + +describe('CustomAgentAPI device ownership', () => { + beforeEach(() => { + activateSurface('local'); + invoke.mockReset(); + }); + + it('does not publish a completed mutation into the next device catalog', async () => { + invoke.mockImplementationOnce(() => { + queueMicrotask(() => queueMicrotask(() => activateSurface('other-device'))); + return Promise.resolve(); + }); + const changed = vi.fn(); + const unsubscribe = globalEventBus.on('mode:config:updated', changed); + try { + await expect(CustomAgentAPI.deleteCustomAgent('same-agent', 'same-workspace')) + .rejects.toSatisfy(isSurfaceChangedError); + expect(changed).not.toHaveBeenCalled(); + } finally { + unsubscribe(); + } + }); +}); diff --git a/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.ts b/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.ts index e7e85d79a1..69badd6330 100644 --- a/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/CustomAgentAPI.ts @@ -1,6 +1,7 @@ -import { api } from './ApiClient'; +import { invokePrepared } from './invokePrepared'; import { workspaceScopedRequest } from './legacyWorkspaceCompatibility'; import { globalEventBus } from '@/infrastructure/event-bus'; +import { getActiveSurfaceScope, type SurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; export type AgentSource = 'builtin' | 'project' | 'user' | 'external'; export type CustomAgentKind = 'mode' | 'subagent'; @@ -58,11 +59,12 @@ export interface UpdateCustomAgentPayload { workspaceId?: string; } -function emitCustomAgentCatalogUpdated(payload: { +function emitCustomAgentCatalogUpdated(scope: SurfaceScope, payload: { agentId?: string; kind?: CustomAgentKind; workspaceId?: string; }) { + scope.assertCurrent('publish custom agent catalog update'); globalEventBus.emit('custom-agent:updated', payload); globalEventBus.emit('mode:config:updated', { reason: 'custom-agent-catalog-updated', @@ -74,16 +76,17 @@ export const CustomAgentAPI = { async getCustomAgentDetail( payload: GetCustomAgentDetailPayload, ): Promise { - return api.invoke('get_custom_agent_detail', { + return invokePrepared('get_custom_agent_detail', async () => ({ request: await workspaceScopedRequest(payload), - }); + })); }, async createCustomAgent(payload: CreateCustomAgentPayload): Promise { - await api.invoke('create_custom_agent', { + const scope = getActiveSurfaceScope(); + await invokePrepared('create_custom_agent', async () => ({ request: await workspaceScopedRequest(payload), - }); - emitCustomAgentCatalogUpdated({ + })); + emitCustomAgentCatalogUpdated(scope, { agentId: payload.id, kind: payload.kind, workspaceId: payload.workspaceId, @@ -91,26 +94,29 @@ export const CustomAgentAPI = { }, async updateCustomAgent(payload: UpdateCustomAgentPayload): Promise { - await api.invoke('update_custom_agent', { + const scope = getActiveSurfaceScope(); + await invokePrepared('update_custom_agent', async () => ({ request: await workspaceScopedRequest(payload), - }); - emitCustomAgentCatalogUpdated({ + })); + emitCustomAgentCatalogUpdated(scope, { agentId: payload.agentId, workspaceId: payload.workspaceId, }); }, async deleteCustomAgent(agentId: string, workspaceId?: string): Promise { - await api.invoke('delete_custom_agent', { + const scope = getActiveSurfaceScope(); + await invokePrepared('delete_custom_agent', async () => ({ request: await workspaceScopedRequest({ agentId, workspaceId }), - }); - emitCustomAgentCatalogUpdated({ agentId, workspaceId }); + })); + emitCustomAgentCatalogUpdated(scope, { agentId, workspaceId }); }, async reloadCustomAgents(workspaceId?: string): Promise { - await api.invoke('reload_custom_agents', { + const scope = getActiveSurfaceScope(); + await invokePrepared('reload_custom_agents', async () => ({ request: await workspaceScopedRequest({ workspaceId }), - }); - emitCustomAgentCatalogUpdated({ workspaceId }); + })); + emitCustomAgentCatalogUpdated(scope, { workspaceId }); }, }; diff --git a/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.test.ts index 36c5a66d3e..c33df004ba 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.test.ts @@ -5,6 +5,7 @@ import { PeerProductCommandError } from '../adapters/peer-device-adapter'; import { ApiClient } from './ApiClient'; import { globalEventBus } from '@/infrastructure/event-bus'; import { MCP_CONFIG_CHANGED } from '@/infrastructure/mcp/configEvents'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; const invokeMock = vi.hoisted(() => vi.fn()); const adapterMocks = vi.hoisted(() => ({ @@ -62,6 +63,7 @@ vi.mock('./ApiClient', async importOriginal => { describe('ExternalSourcesAPI', () => { beforeEach(() => { + activateSurface('local'); vi.clearAllMocks(); invokeMock.mockReset(); invokeMock.mockResolvedValue(surface({})); @@ -75,6 +77,34 @@ describe('ExternalSourcesAPI', () => { integrationPolicy: { status: 'compatible', effective: { enabled: false, ecosystems: {} }, registeredEcosystems: [] } }, }); + it('does not refresh another device after a discovery preference write has returned', async () => { + invokeMock.mockImplementationOnce(() => { + // Complete the transport and prepared-command promises, then switch + // before the compound operation resumes to issue its follow-up read. + queueMicrotask(() => queueMicrotask(() => queueMicrotask(() => activateSurface('other-device')))); + return Promise.resolve({}); + }); + + await expect(externalSourcesAPI.setAutomaticDiscovery(undefined, false, 8)) + .rejects.toSatisfy(isSurfaceChangedError); + expect(invokeMock.mock.calls.map(([command]) => command)).toEqual([ + 'update_external_integration_policy_command', + ]); + }); + + it('does not negotiate a legacy fallback on a device selected after the first response', async () => { + invokeMock.mockImplementationOnce(() => { + queueMicrotask(() => queueMicrotask(() => queueMicrotask(() => queueMicrotask(() => activateSurface('other-device'))))); + return Promise.reject(new Error('Unknown command get_external_source_control_snapshot')); + }); + + await expect(externalSourcesAPI.getControlSnapshot()) + .rejects.toSatisfy(isSurfaceChangedError); + expect(invokeMock.mock.calls.map(([command]) => command)).toEqual([ + 'get_external_source_control_snapshot', + ]); + }); + it('negotiates independent catalog discovery while keeping runtime authorization off', async () => { invokeMock.mockResolvedValue(discovery()); const result = await externalSourcesAPI.getDiscoverySnapshot(' /project ', true); diff --git a/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.ts b/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.ts index 7739709af6..34b66a09a2 100644 --- a/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/ExternalSourcesAPI.ts @@ -1,6 +1,6 @@ import { workspaceIdRequest } from './legacyWorkspaceCompatibility'; -import { api } from './ApiClient'; -import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { invokePrepared } from './invokePrepared'; +import { getActiveSurfaceScope, isSurfaceChangedError, type SurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; import { notifyMcpConfigChanged } from '@/infrastructure/mcp/configEvents'; import { globalEventBus } from '@/infrastructure/event-bus'; @@ -1198,17 +1198,20 @@ export async function invokeExternalSourceCommand( args: Record, ): Promise { try { - const request = args.request as Record | undefined; - if (typeof request?.workspaceId === 'string') { - const { workspaceId, ...rest } = request; - const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); - // Old external-source DTOs reject extra SSH fields. Remote workspaces - // are unsupported by this local discovery surface on both versions. - const wire = 'workspaceId' in reference ? { workspaceId } : { workspacePath: reference.workspacePath }; - args = { ...args, request: { ...rest, ...wire } }; - } - return await api.invoke(command, args); + return await invokePrepared(command, async () => { + const request = args.request as Record | undefined; + if (typeof request?.workspaceId === 'string') { + const { workspaceId, ...rest } = request; + const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); + // Old external-source DTOs reject extra SSH fields. Remote workspaces + // are unsupported by this local discovery surface on both versions. + const wire = 'workspaceId' in reference ? { workspaceId } : { workspacePath: reference.workspacePath }; + return { ...args, request: { ...rest, ...wire } }; + } + return args; + }); } catch (error) { + if (isSurfaceChangedError(error)) throw error; const parsed = parseOperationError(error); let raw = typeof error === 'string' ? error @@ -1251,7 +1254,9 @@ async function invokeSnapshot( command: string, args: Record, ): Promise { + const scope = getActiveSurfaceScope(); await invokeExternalSourceCommand(command, args); + scope.assertCurrent('refresh external source catalog'); const request = args.request && typeof args.request === 'object' ? args.request as Record : {}; @@ -1297,9 +1302,11 @@ function legacySurfaceSnapshot(catalog: ExternalSourceCatalogSnapshot): External async function invokeCompatibleSurfaceSnapshot( args: Record, ): Promise { + const scope = getActiveSurfaceScope(); try { return await invokeSurfaceSnapshot('get_external_source_control_snapshot', args); } catch (error) { + scope.assertCurrent('negotiate external source catalog'); if (!(error instanceof ExternalSourceApiError) || error.code !== 'incompatible_version') { throw error; } @@ -1339,7 +1346,8 @@ function controlRequest( }; } -function emitExternalAgentCatalogUpdated(workspaceId?: string) { +function emitExternalAgentCatalogUpdated(scope: SurfaceScope, workspaceId?: string) { + scope.assertCurrent('publish external agent catalog update'); globalEventBus.emit('mode:config:updated', { reason: 'external-agent-catalog-updated', workspaceId: normalizeOptionalWorkspaceId(workspaceId), @@ -1394,6 +1402,7 @@ export const externalSourcesAPI = { }, async getDiscoverySnapshot(workspaceId?: string, forceRefresh = false): Promise { + const scope = getActiveSurfaceScope(); try { const value = await invokeExternalSourceCommand<{ schemaVersion: number; @@ -1432,6 +1441,7 @@ export const externalSourcesAPI = { }, }; } catch (error) { + scope.assertCurrent('negotiate external source discovery'); if (!(error instanceof ExternalSourceApiError) || error.code !== 'incompatible_version') throw error; // Old hosts remain viewable, but never receive the new mutation. return this.getSnapshot(workspaceId, forceRefresh); @@ -1439,6 +1449,7 @@ export const externalSourcesAPI = { }, async setAutomaticDiscovery(workspaceId: string | undefined, enabled: boolean, expectedPreferenceRevision: number) { + const scope = getActiveSurfaceScope(); const path = normalizeOptionalWorkspaceId(workspaceId); await invokeExternalSourceCommand('update_external_integration_policy_command', { request: { @@ -1447,6 +1458,7 @@ export const externalSourcesAPI = { change: { operation: 'set_automatic_discovery', enabled } }, }, }); + scope.assertCurrent('refresh external source discovery'); return this.getDiscoverySnapshot(workspaceId); }, @@ -1552,6 +1564,7 @@ export const externalSourcesAPI = { enabled: boolean, expectedPreferenceRevision: number, ) { + const scope = getActiveSurfaceScope(); const normalizedWorkspaceId = normalizeOptionalWorkspaceId(workspaceId); try { const surface = await invokeSurfaceSnapshot('apply_external_source_control_action_command', { @@ -1563,9 +1576,10 @@ export const externalSourcesAPI = { ), }, }); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return surface.catalog; } catch (error) { + scope.assertCurrent('negotiate external source mutation'); if (!(error instanceof ExternalSourceApiError) || error.code !== 'incompatible_version') { throw error; } @@ -1577,7 +1591,7 @@ export const externalSourcesAPI = { expectedPreferenceRevision, }, }); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return catalog; } }, @@ -1675,6 +1689,7 @@ export const externalSourcesAPI = { expectedPreferenceRevision: number, decisionKey: string, ) { + const scope = getActiveSurfaceScope(); const catalog = await invokeSnapshot('set_external_subagent_activation_command', { request: { workspaceId: normalizeOptionalWorkspaceId(workspaceId), @@ -1685,7 +1700,7 @@ export const externalSourcesAPI = { decisionKey, }, }); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return catalog; }, @@ -1696,6 +1711,7 @@ export const externalSourcesAPI = { expectedSubagentGeneration: number, expectedPreferenceRevision: number, ) { + const scope = getActiveSurfaceScope(); const catalog = await invokeSnapshot('set_external_subagents_enabled_command', { request: { workspaceId: normalizeOptionalWorkspaceId(workspaceId), @@ -1705,7 +1721,7 @@ export const externalSourcesAPI = { expectedPreferenceRevision, }, }); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return catalog; }, @@ -1716,6 +1732,7 @@ export const externalSourcesAPI = { expectedSubagentGeneration: number, expectedPreferenceRevision: number, ) { + const scope = getActiveSurfaceScope(); const catalog = await invokeSnapshot('set_external_subagent_model_binding_command', { request: { workspaceId: normalizeOptionalWorkspaceId(workspaceId), @@ -1725,7 +1742,7 @@ export const externalSourcesAPI = { expectedPreferenceRevision, }, }); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return catalog; }, @@ -1737,6 +1754,7 @@ export const externalSourcesAPI = { expectedSubagentGeneration: number, expectedPreferenceRevision: number, ) { + const scope = getActiveSurfaceScope(); const catalog = await invokeSnapshot('choose_external_subagent_conflict_command', { request: { workspaceId: normalizeOptionalWorkspaceId(workspaceId), @@ -1747,7 +1765,7 @@ export const externalSourcesAPI = { expectedPreferenceRevision, }, }); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return catalog; }, @@ -1813,11 +1831,12 @@ export const externalSourcesAPI = { workspaceId: string | undefined, mutation: ExternalIntegrationPolicyMutation, ) { + const scope = getActiveSurfaceScope(); const catalog = await invokeSnapshot( 'update_external_integration_policy_command', { request: { workspaceId: normalizeOptionalWorkspaceId(workspaceId), mutation } }, ); - emitExternalAgentCatalogUpdated(workspaceId); + emitExternalAgentCatalogUpdated(scope, workspaceId); return catalog; }, diff --git a/src/web-ui/src/infrastructure/api/service-api/GitAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/GitAPI.test.ts index da5d75107a..8cd844fd07 100644 --- a/src/web-ui/src/infrastructure/api/service-api/GitAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/GitAPI.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { GitAPI } from './GitAPI'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; const workspace = { workspaceId: 'workspace-1' }; @@ -25,10 +26,39 @@ describe('GitAPI repository probe cache', () => { let gitAPI: GitAPI; beforeEach(() => { + activateSurface('local'); gitAPI = new GitAPI(); invokeMock.mockReset(); }); + it('does not dispatch a repository probe after its device changes during preparation', async () => { + invokeMock.mockResolvedValue(true); + const pending = gitAPI.isGitRepository(workspace); + const rejection = expect(pending).rejects.toSatisfy(isSurfaceChangedError); + activateSurface('peer-b'); + await rejection; + expect(invokeMock).not.toHaveBeenCalled(); + }); + + it('starts a fresh probe after returning to the same device while an old probe is pending', async () => { + const deferred = createDeferred(); + invokeMock.mockReturnValueOnce(deferred.promise).mockResolvedValue(true); + const first = gitAPI.isGitRepository(workspace); + await vi.waitFor(() => expect(invokeMock).toHaveBeenCalledTimes(1)); + activateSurface('peer-b'); + activateSurface('local'); + const second = gitAPI.isGitRepository(workspace); + const results = Promise.allSettled([first, second]); + deferred.resolve(false); + const [old, current] = await results; + expect(old.status).toBe('rejected'); + if (old.status === 'rejected') expect(isSurfaceChangedError(old.reason)).toBe(true); + expect(current).toEqual({ status: 'fulfilled', value: true }); + expect(invokeMock).toHaveBeenCalledTimes(2); + await expect(gitAPI.isGitRepository(workspace)).resolves.toBe(true); + expect(invokeMock).toHaveBeenCalledTimes(2); + }); + it('deduplicates concurrent repository probes for the same path', async () => { const deferred = createDeferred(); invokeMock.mockReturnValueOnce(deferred.promise); diff --git a/src/web-ui/src/infrastructure/api/service-api/GitAPI.ts b/src/web-ui/src/infrastructure/api/service-api/GitAPI.ts index 5e35cc856e..e1b7ac47e1 100644 --- a/src/web-ui/src/infrastructure/api/service-api/GitAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/GitAPI.ts @@ -1,6 +1,6 @@ +import { invokePrepared } from './invokePrepared'; -import { api } from './ApiClient'; import { createTauriCommandError } from '../errors/TauriCommandError'; import { createLogger } from '@/shared/utils/logger'; import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; @@ -239,21 +239,25 @@ export class GitAPI { async isGitRepository(workspace: GitWorkspaceScope): Promise { // Host and workspace identity isolate probes even when repository paths match. + const surface = getActiveSurfaceScope(); const key = gitWorkspaceKey(workspace); + const inFlightKey = surface.key(surface.epoch, workspace.workspaceId); const now = Date.now(); const cached = this.repositoryProbeCache.get(key); if (cached && cached.expiresAt > now) { return cached.value; } - const inFlight = this.repositoryProbeInFlight.get(key); + const inFlight = this.repositoryProbeInFlight.get(inFlightKey); if (inFlight) { return inFlight; } - const probe = gitWorkspaceRequest(workspace) - .then(request => api.invoke('git_is_repository', { request })) + const probe = invokePrepared('git_is_repository', async () => ({ + request: await gitWorkspaceRequest(workspace), + })) .then((value) => { + surface.assertCurrent('cache repository probe'); this.repositoryProbeCache.set(key, { value, expiresAt: Date.now() + REPOSITORY_PROBE_CACHE_TTL_MS, @@ -264,19 +268,23 @@ export class GitAPI { throw createTauriCommandError('git_is_repository', error, { workspace }); }) .finally(() => { - this.repositoryProbeInFlight.delete(key); + if (this.repositoryProbeInFlight.get(inFlightKey) === probe) { + this.repositoryProbeInFlight.delete(inFlightKey); + } }); - this.repositoryProbeInFlight.set(key, probe); + this.repositoryProbeInFlight.set(inFlightKey, probe); return probe; } /** Reads whether Git trusts the repository's ownership. Never writes. */ async getRepositoryTrust(workspace: GitWorkspaceScope): Promise { + const surface = getActiveSurfaceScope(); try { - const report: GitTrustReport = await api.invoke('git_get_repository_trust', { + const report: GitTrustReport = await invokePrepared('git_get_repository_trust', async () => ({ request: { ...await gitWorkspaceRequest(workspace) }, - }); + })); + surface.assertCurrent('update repository trust cache'); // Trust can be granted outside this product — the user runs the manual // command in a terminal, or the repository's owner fixes it. Whoever // learns that first has to drop the `false` the probe cached while the @@ -306,10 +314,12 @@ export class GitAPI { * is introduced here. */ async trustRepository(workspace: GitWorkspaceScope): Promise { + const surface = getActiveSurfaceScope(); try { - const outcome = await api.invoke('git_trust_repository', { + const outcome = await invokePrepared('git_trust_repository', async () => ({ request: { ...await gitWorkspaceRequest(workspace) }, - }); + })); + surface.assertCurrent('update repository trust cache'); // The probe cache may hold the `false` this repository returned while it // was still refused; a granted decision must not wait it out. if (outcome.state === 'trusted') { @@ -324,9 +334,9 @@ export class GitAPI { async getRepository(workspace: GitWorkspaceScope): Promise { try { - return await api.invoke('git_get_repository', { + return await invokePrepared('git_get_repository', async () => ({ request: { ...await gitWorkspaceRequest(workspace) } - }); + })); } catch (error) { throw createTauriCommandError('git_get_repository', error, { workspace }); } @@ -335,9 +345,9 @@ export class GitAPI { async getRepositoryBasic(workspace: GitWorkspaceScope): Promise { try { - return await api.invoke('git_get_repository_basic', { + return await invokePrepared('git_get_repository_basic', async () => ({ request: { ...await gitWorkspaceRequest(workspace) } - }); + })); } catch (error) { throw createTauriCommandError('git_get_repository_basic', error, { workspace }); } @@ -345,9 +355,9 @@ export class GitAPI { async resolveRevision(workspace: GitWorkspaceScope, revision: string): Promise { try { - return await api.invoke('git_resolve_revision', { + return await invokePrepared('git_resolve_revision', async () => ({ request: { ...await gitWorkspaceRequest(workspace), revision }, - }); + })); } catch (error) { throw createTauriCommandError('git_resolve_revision', error, { workspace, @@ -362,9 +372,9 @@ export class GitAPI { if (globalThis.__OPENBITFUN_PERF_TRACE_ENABLED__ === true) { startupTrace.markPhase('git_status_request', { source: traceSource }); } - return await api.invoke('git_get_status', { + return await invokePrepared('git_get_status', async () => ({ request: { ...await gitWorkspaceRequest(workspace) } - }); + })); } catch (error) { throw createTauriCommandError('git_get_status', error, { workspace }); } @@ -373,9 +383,9 @@ export class GitAPI { async getBranches(workspace: GitWorkspaceScope, includeRemote: boolean = false): Promise { try { - return await api.invoke('git_get_branches', { + return await invokePrepared('git_get_branches', async () => ({ request: { ...await gitWorkspaceRequest(workspace), includeRemote } - }); + })); } catch (error) { throw createTauriCommandError('git_get_branches', error, { workspace, includeRemote }); } @@ -384,9 +394,9 @@ export class GitAPI { async getEnhancedBranches(workspace: GitWorkspaceScope, includeRemote: boolean = false): Promise { try { - return await api.invoke('git_get_enhanced_branches', { + return await invokePrepared('git_get_enhanced_branches', async () => ({ request: { ...await gitWorkspaceRequest(workspace), includeRemote } - }); + })); } catch (error) { throw createTauriCommandError('git_get_enhanced_branches', error, { workspace, includeRemote }); } @@ -395,9 +405,9 @@ export class GitAPI { async getCommits(workspace: GitWorkspaceScope, params: GitLogParams = {}): Promise { try { - return await api.invoke('git_get_commits', { + return await invokePrepared('git_get_commits', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params } - }); + })); } catch (error) { throw createTauriCommandError('git_get_commits', error, { workspace, params }); } @@ -406,9 +416,9 @@ export class GitAPI { async addFiles(workspace: GitWorkspaceScope, params: GitAddParams): Promise { try { - return await api.invoke('git_add_files', { + return await invokePrepared('git_add_files', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params } - }); + })); } catch (error) { throw createTauriCommandError('git_add_files', error, { workspace, params }); } @@ -417,9 +427,9 @@ export class GitAPI { async commit(workspace: GitWorkspaceScope, params: GitCommitParams): Promise { try { - return await api.invoke('git_commit', { + return await invokePrepared('git_commit', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params } - }); + })); } catch (error) { throw createTauriCommandError('git_commit', error, { workspace, params }); } @@ -436,9 +446,9 @@ export class GitAPI { set_upstream: params.setUpstream }; - return await api.invoke('git_push', { + return await invokePrepared('git_push', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params: backendParams } - }); + })); } catch (error) { throw createTauriCommandError('git_push', error, { workspace, params }); } @@ -447,9 +457,9 @@ export class GitAPI { async pull(workspace: GitWorkspaceScope, params: GitPullParams = {}): Promise { try { - return await api.invoke('git_pull', { + return await invokePrepared('git_pull', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params } - }); + })); } catch (error) { throw createTauriCommandError('git_pull', error, { workspace, params }); } @@ -458,9 +468,9 @@ export class GitAPI { async checkoutBranch(workspace: GitWorkspaceScope, branchName: string): Promise { try { - return await api.invoke('git_checkout_branch', { + return await invokePrepared('git_checkout_branch', async () => ({ request: { ...await gitWorkspaceRequest(workspace), branchName } - }); + })); } catch (error) { throw createTauriCommandError('git_checkout_branch', error, { workspace, branchName }); } @@ -471,9 +481,9 @@ export class GitAPI { try { const effectiveStartPoint = startPoint && startPoint.trim() ? startPoint : undefined; - return await api.invoke('git_create_branch', { + return await invokePrepared('git_create_branch', async () => ({ request: { ...await gitWorkspaceRequest(workspace), branchName, startPoint: effectiveStartPoint } - }); + })); } catch (error) { throw createTauriCommandError('git_create_branch', error, { workspace, branchName, startPoint }); } @@ -482,9 +492,9 @@ export class GitAPI { async deleteBranch(workspace: GitWorkspaceScope, branchName: string, force: boolean = false): Promise { try { - return await api.invoke('git_delete_branch', { + return await invokePrepared('git_delete_branch', async () => ({ request: { ...await gitWorkspaceRequest(workspace), branchName, force } - }); + })); } catch (error) { throw createTauriCommandError('git_delete_branch', error, { workspace, branchName, force }); } @@ -493,9 +503,9 @@ export class GitAPI { async resetToCommit(workspace: GitWorkspaceScope, commitHash: string, mode: 'soft' | 'mixed' | 'hard' = 'mixed'): Promise { try { - return await api.invoke('git_reset_to_commit', { + return await invokePrepared('git_reset_to_commit', async () => ({ request: { ...await gitWorkspaceRequest(workspace), commitHash, mode } - }); + })); } catch (error) { throw createTauriCommandError('git_reset_to_commit', error, { workspace, commitHash, mode }); } @@ -504,9 +514,9 @@ export class GitAPI { async getDiff(workspace: GitWorkspaceScope, params: GitDiffParams): Promise { try { - return await api.invoke('git_get_diff', { + return await invokePrepared('git_get_diff', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params } - }); + })); } catch (error) { throw createTauriCommandError('git_get_diff', error, { workspace, params }); } @@ -515,9 +525,9 @@ export class GitAPI { async getChangedFiles(workspace: GitWorkspaceScope, params: GitChangedFilesParams): Promise { try { - return await api.invoke('git_get_changed_files', { + return await invokePrepared('git_get_changed_files', async () => ({ request: { ...await gitWorkspaceRequest(workspace), params } - }); + })); } catch (error) { throw createTauriCommandError('git_get_changed_files', error, { workspace, params }); } @@ -526,9 +536,9 @@ export class GitAPI { async resetFiles(workspace: GitWorkspaceScope, files: string[], staged: boolean = false): Promise { try { - return await api.invoke('git_reset_files', { + return await invokePrepared('git_reset_files', async () => ({ request: { ...await gitWorkspaceRequest(workspace), files, staged } - }); + })); } catch (error) { throw createTauriCommandError('git_reset_files', error, { workspace, files, staged }); } @@ -537,9 +547,9 @@ export class GitAPI { async getFileContent(workspace: GitWorkspaceScope, filePath: string, commit?: string): Promise { try { - return await api.invoke('git_get_file_content', { + return await invokePrepared('git_get_file_content', async () => ({ request: { ...await gitWorkspaceRequest(workspace), filePath, commit } - }); + })); } catch (error) { throw createTauriCommandError('git_get_file_content', error, { workspace, filePath, commit }); } @@ -547,11 +557,11 @@ export class GitAPI { async getGraph(workspace: GitWorkspaceScope, maxCount?: number, branchName?: string): Promise { try { - const result = await api.invoke('git_get_graph', { + const result = await invokePrepared('git_get_graph', async () => ({ ...await gitWorkspaceRequest(workspace), maxCount: maxCount || null, branchName: branchName || null - }); + })); return result; } catch (error) { log.error('Failed to get git graph', { workspace, maxCount, branchName, error }); @@ -562,9 +572,9 @@ export class GitAPI { async cherryPick(workspace: GitWorkspaceScope, commitHash: string, noCommit: boolean = false): Promise { try { - return await api.invoke('git_cherry_pick', { + return await invokePrepared('git_cherry_pick', async () => ({ request: { ...await gitWorkspaceRequest(workspace), commitHash, noCommit } - }); + })); } catch (error) { throw createTauriCommandError('git_cherry_pick', error, { workspace, commitHash, noCommit }); } @@ -573,9 +583,9 @@ export class GitAPI { async cherryPickAbort(workspace: GitWorkspaceScope): Promise { try { - return await api.invoke('git_cherry_pick_abort', { + return await invokePrepared('git_cherry_pick_abort', async () => ({ request: { ...await gitWorkspaceRequest(workspace) } - }); + })); } catch (error) { throw createTauriCommandError('git_cherry_pick_abort', error, { workspace }); } @@ -584,9 +594,9 @@ export class GitAPI { async cherryPickContinue(workspace: GitWorkspaceScope): Promise { try { - return await api.invoke('git_cherry_pick_continue', { + return await invokePrepared('git_cherry_pick_continue', async () => ({ request: { ...await gitWorkspaceRequest(workspace) } - }); + })); } catch (error) { throw createTauriCommandError('git_cherry_pick_continue', error, { workspace }); } @@ -597,9 +607,9 @@ export class GitAPI { async listWorktrees(workspace: GitWorkspaceScope): Promise { try { - return await api.invoke('git_list_worktrees', { + return await invokePrepared('git_list_worktrees', async () => ({ request: { ...await gitWorkspaceRequest(workspace) } - }); + })); } catch (error) { throw createTauriCommandError('git_list_worktrees', error, { workspace }); } @@ -608,9 +618,9 @@ export class GitAPI { async addWorktree(workspace: GitWorkspaceScope, branch: string, createBranch: boolean = false): Promise { try { - return await api.invoke('git_add_worktree', { + return await invokePrepared('git_add_worktree', async () => ({ request: { ...await gitWorkspaceRequest(workspace), branch, createBranch } - }); + })); } catch (error) { throw createTauriCommandError('git_add_worktree', error, { workspace, branch, createBranch }); } @@ -619,9 +629,9 @@ export class GitAPI { async removeWorktree(workspace: GitWorkspaceScope, worktreePath: string, force: boolean = false): Promise { try { - return await api.invoke('git_remove_worktree', { + return await invokePrepared('git_remove_worktree', async () => ({ request: { ...await gitWorkspaceRequest(workspace), worktreePath, force } - }); + })); } catch (error) { throw createTauriCommandError('git_remove_worktree', error, { workspace, worktreePath, force }); } diff --git a/src/web-ui/src/infrastructure/api/service-api/GlobalAPI.ts b/src/web-ui/src/infrastructure/api/service-api/GlobalAPI.ts index 5c6752da0f..7c50b02f6f 100644 --- a/src/web-ui/src/infrastructure/api/service-api/GlobalAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/GlobalAPI.ts @@ -1,3 +1,4 @@ +import { invokePrepared } from './invokePrepared'; import { getActiveSurfaceScope, isLocalSurface } from '@/infrastructure/peer-device/deviceSurface'; @@ -88,6 +89,14 @@ export interface WorkspaceStartupStateSnapshot { legacyRemoteWorkspace?: RemoteWorkspaceSnapshot | null; } +export interface OpenRemoteWorkspaceOptions { + /** + * The user confirmed moving an existing record owned by another saved SSH + * connection to this connection. + */ + rebindConnection?: boolean; +} + export interface UpdateAppStatusRequest { status: AppStatus; } @@ -194,7 +203,7 @@ export class GlobalAPI { async openWorkspaceById(workspaceId: string): Promise { - return api.invoke('open_workspace', { request: await workspaceIdRequest(workspaceId, 'path') }); + return invokePrepared('open_workspace', async () => ({ request: await workspaceIdRequest(workspaceId, 'path') })); } async createLocalWorkspace(path: string): Promise { @@ -211,7 +220,8 @@ export class GlobalAPI { remotePath: string, connectionId: string, connectionName: string, - sshHost?: string + sshHost?: string, + options: OpenRemoteWorkspaceOptions = {}, ): Promise { try { const h = sshHost?.trim(); @@ -221,6 +231,7 @@ export class GlobalAPI { connectionId, connectionName, ...(h ? { sshHost: h } : {}), + ...(options.rebindConnection ? { rebindConnection: true } : {}), }, }); } catch (error) { @@ -229,6 +240,7 @@ export class GlobalAPI { connectionId, connectionName, sshHost, + rebindConnection: options.rebindConnection === true, }); } } @@ -394,9 +406,9 @@ export class GlobalAPI { async scanWorkspaceInfo(workspaceId: string): Promise { try { - return await api.invoke('scan_workspace_info', { + return await invokePrepared('scan_workspace_info', async () => ({ request: await workspaceIdRequest(workspaceId, 'workspacePath') - }); + })); } catch (error) { throw createTauriCommandError('scan_workspace_info', error, { workspaceId }); } diff --git a/src/web-ui/src/infrastructure/api/service-api/SessionAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/SessionAPI.test.ts index d52e58c909..c4cd196767 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SessionAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SessionAPI.test.ts @@ -1,20 +1,43 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { SessionAPI } from './SessionAPI'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; const invokeMock = vi.hoisted(() => vi.fn()); +const peerCapabilities = vi.hoisted(() => ({ workspaceIdReferencesV1: true })); vi.mock('./ApiClient', () => ({ api: { invoke: invokeMock, }, })); +vi.mock('@/infrastructure/peer-device/PeerConnectionManager', () => ({ + peerConnectionManager: { get: () => ({ getState: () => ({ capabilities: peerCapabilities }) }) }, +})); describe('SessionAPI paged metadata reads', () => { let sessionAPI: SessionAPI; beforeEach(() => { + activateSurface('local'); sessionAPI = new SessionAPI(); invokeMock.mockReset(); + peerCapabilities.workspaceIdReferencesV1 = true; + }); + + it('does not send a local workspace request to an equal ID on a newly selected peer', async () => { + const pending = sessionAPI.listSessionsPage({ workspaceId: 'same-workspace-id', limit: 5 }); + activateSurface('peer-with-the-same-workspace'); + + await expect(pending).rejects.toSatisfy(isSurfaceChangedError); + expect(invokeMock).not.toHaveBeenCalled(); + }); + + it('does not delete an equal session ID on a peer selected during request preparation', async () => { + const pending = sessionAPI.deleteSession('same-session-id', 'same-workspace-id'); + activateSurface('peer-with-the-same-workspace'); + + await expect(pending).rejects.toSatisfy(isSurfaceChangedError); + expect(invokeMock).not.toHaveBeenCalled(); }); it.each(['local-project', 'remote-loopback', 'remote-project'])('reads %s by ID without path or SSH hints', async workspaceId => { @@ -26,6 +49,23 @@ describe('SessionAPI paged metadata reads', () => { }); }); + it('preserves the selected SSH host when serializing for a peer without workspace ID support', async () => { + peerCapabilities.workspaceIdReferencesV1 = false; + activateSurface('legacy-peer'); + const records = [ + { id: 'local-id', rootPath: '/same/root', workspaceKind: 'normal' }, + { id: 'remote-id', rootPath: '/same/root', workspaceKind: 'remote', connectionId: 'ssh-id', sshHost: 'localhost' }, + ]; + invokeMock.mockImplementation(async (command: string) => + command === 'get_opened_workspaces' || command === 'get_recent_workspaces' ? records : []); + + await sessionAPI.listSessions('remote-id'); + + expect(invokeMock).toHaveBeenCalledWith('list_persisted_sessions', { + request: { workspace_path: '/same/root', remote_connection_id: 'ssh-id', remote_ssh_host: 'localhost' }, + }); + }); + it('loads the scoped hidden Session lineage without listing all internal Sessions', async () => { const snapshot = { rootSessionId: 'root', sessions: [] }; invokeMock.mockResolvedValueOnce(snapshot); diff --git a/src/web-ui/src/infrastructure/api/service-api/SessionAPI.ts b/src/web-ui/src/infrastructure/api/service-api/SessionAPI.ts index a176d27591..a5f83e3bf0 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SessionAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SessionAPI.ts @@ -1,3 +1,4 @@ +import { invokePrepared } from './invokePrepared'; import { sessionWorkspaceIdRequest } from './legacyWorkspaceCompatibility'; import { isTauriRuntime } from '@/infrastructure/runtime/environment'; import { getActiveSurfaceId, isLocalSurface } from '@/infrastructure/peer-device/deviceSurface'; @@ -349,13 +350,13 @@ export class SessionAPI { workspaceId: string ): Promise<{ sessionId: string; sessionName: string; agentType: string }> { try { - return await api.invoke('fork_session', { + return await invokePrepared('fork_session', async () => ({ request: { source_session_id: sourceSessionId, source_turn_id: sourceTurnId, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('fork_session', error, { sourceSessionId, @@ -366,23 +367,23 @@ export class SessionAPI { } async listSessions(workspaceId: string): Promise { - return api.invoke('list_persisted_sessions', { + return invokePrepared('list_persisted_sessions', async () => ({ request: await sessionWorkspaceIdRequest(workspaceId), - }); + })); } async listSessionsPage( request: SessionMetadataPageRequest ): Promise { try { - return await api.invoke('list_persisted_sessions_page', { + return await invokePrepared('list_persisted_sessions_page', async () => ({ request: { ...await sessionWorkspaceIdRequest(request.workspaceId), limit: request.limit, ...(request.cursor ? { cursor: request.cursor } : {}), ...(request.sessionIds ? { session_ids: request.sessionIds } : {}), } - }); + })); } catch (error) { throw createTauriCommandError('list_persisted_sessions_page', error, { workspaceId: request.workspaceId, @@ -396,13 +397,13 @@ export class SessionAPI { request: SessionLineageRequest ): Promise { try { - return await api.invoke('get_session_lineage', { + return await invokePrepared('get_session_lineage', async () => ({ request: { session_id: request.sessionId, ...await sessionWorkspaceIdRequest(request.workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('get_session_lineage', error, { sessionId: request.sessionId, @@ -417,19 +418,13 @@ export class SessionAPI { limit?: number ): Promise { try { - const request: Record = { - session_id: sessionId, - ...await sessionWorkspaceIdRequest(workspaceId), - - }; - - if (limit !== undefined) { - request.limit = limit; - } - - return await api.invoke('load_session_turns', { - request - }); + return await invokePrepared('load_session_turns', async () => ({ + request: { + session_id: sessionId, + ...await sessionWorkspaceIdRequest(workspaceId), + ...(limit !== undefined ? { limit } : {}), + }, + })); } catch (error) { throw createTauriCommandError('load_session_turns', error, { sessionId, workspaceId, limit }); } @@ -440,13 +435,13 @@ export class SessionAPI { workspaceId: string ): Promise { try { - await api.invoke('save_session_turn', { + await invokePrepared('save_session_turn', async () => ({ request: { turn_data: turnData, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('save_session_turn', error, { turnData, workspaceId }); } @@ -458,14 +453,14 @@ export class SessionAPI { fields: UiSessionMetadataField[] ): Promise { try { - await api.invoke('save_session_metadata', { + await invokePrepared('save_session_metadata', async () => ({ request: { metadata, fields, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('save_session_metadata', error, { metadata, workspaceId }); } @@ -476,13 +471,13 @@ export class SessionAPI { workspaceId: string ): Promise { try { - await api.invoke('delete_persisted_session', { + await invokePrepared('delete_persisted_session', async () => ({ request: { session_id: sessionId, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('delete_persisted_session', error, { sessionId, workspaceId }); } @@ -493,13 +488,13 @@ export class SessionAPI { workspaceId: string ): Promise { try { - await api.invoke('touch_session_activity', { + await invokePrepared('touch_session_activity', async () => ({ request: { session_id: sessionId, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('touch_session_activity', error, { sessionId, workspaceId }); } @@ -510,13 +505,13 @@ export class SessionAPI { workspaceId: string ): Promise { try { - return await api.invoke('load_persisted_session_metadata', { + return await invokePrepared('load_persisted_session_metadata', async () => ({ request: { session_id: sessionId, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('load_persisted_session_metadata', error, { sessionId, workspaceId }); } @@ -526,14 +521,14 @@ export class SessionAPI { request: SessionUsageReportRequest ): Promise { try { - return await api.invoke('get_session_usage_report', { + return await invokePrepared('get_session_usage_report', async () => ({ request: { session_id: request.sessionId, ...await sessionWorkspaceIdRequest(request.workspaceId), include_hidden_subagents: request.includeHiddenSubagents ?? true, } - }); + })); } catch (error) { throw createTauriCommandError('get_session_usage_report', error, { sessionId: request.sessionId, @@ -547,13 +542,13 @@ export class SessionAPI { workspaceId: string ): Promise { try { - await api.invoke('archive_session', { + await invokePrepared('archive_session', async () => ({ request: { session_id: sessionId, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('archive_session', error, { sessionId, workspaceId }); } @@ -564,13 +559,13 @@ export class SessionAPI { workspaceId: string ): Promise { try { - await api.invoke('unarchive_session', { + await invokePrepared('unarchive_session', async () => ({ request: { session_id: sessionId, ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('unarchive_session', error, { sessionId, workspaceId }); } @@ -580,12 +575,12 @@ export class SessionAPI { workspaceId: string ): Promise { try { - return await api.invoke('archive_all_sessions', { + return await invokePrepared('archive_all_sessions', async () => ({ request: { ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('archive_all_sessions', error, { workspaceId }); } @@ -595,12 +590,12 @@ export class SessionAPI { workspaceId: string ): Promise { try { - return await api.invoke('list_archived_sessions', { + return await invokePrepared('list_archived_sessions', async () => ({ request: { ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('list_archived_sessions', error, { workspaceId }); } @@ -610,12 +605,12 @@ export class SessionAPI { workspaceId: string ): Promise { try { - return await api.invoke('delete_all_archived_sessions', { + return await invokePrepared('delete_all_archived_sessions', async () => ({ request: { ...await sessionWorkspaceIdRequest(workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('delete_all_archived_sessions', error, { workspaceId }); } diff --git a/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.test.ts index 65c234cb42..c31082955c 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { SnapshotAPI } from './SnapshotAPI'; -import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; const invokeMock = vi.hoisted(() => vi.fn()); const peerCapabilities = vi.hoisted(() => ({ workspaceIdReferencesV1: true })); @@ -88,13 +88,14 @@ describe('SnapshotAPI workspace identity', () => { invokeMock.mockImplementationOnce(() => new Promise(resolve => { resolveFirst = resolve; })); invokeMock.mockResolvedValueOnce({ linesAdded: 2 }); const first = snapshotAPI.getOperationSummary('same-session', 'operation-1', 'same-id'); + const firstRejection = expect(first).rejects.toSatisfy(isSurfaceChangedError); await vi.waitFor(() => expect(invokeMock).toHaveBeenCalledTimes(1)); activateSurface('peer-b'); const second = snapshotAPI.getOperationSummary('same-session', 'operation-1', 'same-id'); await expect(second).resolves.toMatchObject({ linesAdded: 2 }); expect(invokeMock).toHaveBeenCalledTimes(2); resolveFirst({ linesAdded: 1 }); - await expect(first).resolves.toMatchObject({ linesAdded: 1 }); + await firstRejection; }); it('does not dispatch after the driving host changes during serialization', async () => { @@ -120,4 +121,21 @@ describe('SnapshotAPI workspace identity', () => { }); }); + it('stops a multi-turn snapshot read when the device changes during a turn read', async () => { + invokeMock.mockImplementation(async (command: string) => { + if (command === 'get_session_turns') return [0, 1]; + if (command === 'get_turn_files') { + activateSurface('other-device'); + return ['old-device.txt']; + } + return []; + }); + + await expect(snapshotAPI.getSessionTurnSnapshots('same-session', 'same-workspace')) + .rejects.toSatisfy(isSurfaceChangedError); + expect(invokeMock.mock.calls.map(([command]) => command)).toEqual([ + 'get_session_turns', 'get_turn_files', + ]); + }); + }); diff --git a/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.ts b/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.ts index 9e3a15a0e8..35715be61f 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SnapshotAPI.ts @@ -1,9 +1,10 @@ +import { invokePrepared } from './invokePrepared'; import { api } from './ApiClient'; import { createTauriCommandError } from '../errors/TauriCommandError'; import { createLogger } from '@/shared/utils/logger'; -import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { getActiveSurfaceScope, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; import { flowChatStore } from '@/flow_chat/store/FlowChatStore'; import { workspaceIdRequest } from './legacyWorkspaceCompatibility'; @@ -29,12 +30,8 @@ const requireSessionSnapshotScope = ( const snapshotScopeKey = (scope: SnapshotSessionScope): string => scope.workspaceId; -async function snapshotWorkspaceRequest(workspaceId: string) { - const surface = getActiveSurfaceScope(); - const request = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('resolve snapshot workspace'); - return request; -} +/** Only call inside an invokePrepared preparation, which owns the device scope. */ +const snapshotWorkspaceRequest = (workspaceId: string) => workspaceIdRequest(workspaceId, 'workspacePath'); @@ -179,9 +176,9 @@ export class SnapshotAPI { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); const key = `get_session_stats:${snapshotScopeKey(scope)}:${sessionId}`; - return await this.dedupeInFlight(key, async () => api.invoke('get_session_stats', { + return await this.dedupeInFlight(key, async () => invokePrepared('get_session_stats', async () => ({ request: { session_id: sessionId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - })); + }))); } catch (error) { throw createTauriCommandError('get_session_stats', error, { sessionId, workspaceId }); } @@ -192,9 +189,9 @@ export class SnapshotAPI { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); const key = `get_session_files:${snapshotScopeKey(scope)}:${sessionId}`; - return await this.dedupeInFlight(key, async () => api.invoke('get_session_files', { + return await this.dedupeInFlight(key, async () => invokePrepared('get_session_files', async () => ({ request: { session_id: sessionId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - })); + }))); } catch (error) { throw createTauriCommandError('get_session_files', error, { sessionId, workspaceId }); } @@ -209,9 +206,9 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - return await api.invoke('get_operation_diff', { + return await invokePrepared('get_operation_diff', async () => ({ request: { sessionId, filePath, operationId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('get_operation_diff', error, { sessionId, @@ -230,9 +227,9 @@ export class SnapshotAPI { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); const key = `get_session_file_diff_stats:${snapshotScopeKey(scope)}:${sessionId}:${filePath}`; - return await this.dedupeInFlight(key, async () => api.invoke('get_session_file_diff_stats', { + return await this.dedupeInFlight(key, async () => invokePrepared('get_session_file_diff_stats', async () => ({ request: { sessionId, filePath, ...await snapshotWorkspaceRequest(scope.workspaceId) }, - })); + }))); } catch (error) { throw createTauriCommandError('get_session_file_diff_stats', error, { sessionId, @@ -250,9 +247,9 @@ export class SnapshotAPI { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); const key = `get_operation_summary:${snapshotScopeKey(scope)}:${sessionId}:${operationId}`; - return await this.dedupeInFlight(key, async () => api.invoke('get_operation_summary', { + return await this.dedupeInFlight(key, async () => invokePrepared('get_operation_summary', async () => ({ request: { sessionId, operationId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - })); + }))); } catch (error) { throw createTauriCommandError('get_operation_summary', error, { sessionId, @@ -269,9 +266,9 @@ export class SnapshotAPI { ): Promise { try { const resolvedWorkspaceId = requireWorkspaceId(workspaceId); - return await api.invoke('get_baseline_snapshot_diff', { + return await invokePrepared('get_baseline_snapshot_diff', async () => ({ request: { filePath, ...await snapshotWorkspaceRequest(resolvedWorkspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('get_baseline_snapshot_diff', error, { filePath, workspaceId }); } @@ -283,9 +280,9 @@ export class SnapshotAPI { async acceptSessionModifications(sessionId: string, workspaceId?: string): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('accept_session', { + await invokePrepared('accept_session', async () => ({ request: { sessionId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('accept_session', error, { sessionId, workspaceId }); } @@ -295,9 +292,9 @@ export class SnapshotAPI { async rejectSessionModifications(sessionId: string, workspaceId?: string): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('rollback_session', { + await invokePrepared('rollback_session', async () => ({ request: { sessionId, deleteSession: true, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('rollback_session', error, { sessionId, workspaceId }); } @@ -311,9 +308,9 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('accept_file', { + await invokePrepared('accept_file', async () => ({ request: { sessionId, filePath, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('accept_file', error, { sessionId, filePath, workspaceId }); } @@ -327,9 +324,9 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('reject_file', { + await invokePrepared('reject_file', async () => ({ request: { sessionId, filePath, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('reject_file', error, { sessionId, filePath, workspaceId }); } @@ -365,9 +362,9 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('accept_operation', { + await invokePrepared('accept_operation', async () => ({ request: { sessionId, operationId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('accept_operation', error, { sessionId, operationId, workspaceId }); } @@ -381,9 +378,9 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('reject_operation', { + await invokePrepared('reject_operation', async () => ({ request: { sessionId, operationId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('reject_operation', error, { sessionId, operationId, workspaceId }); } @@ -393,9 +390,9 @@ export class SnapshotAPI { async rollbackSession(sessionId: string, workspaceId?: string): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('rollback_session', { + await invokePrepared('rollback_session', async () => ({ request: { sessionId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('rollback_session', error, { sessionId, workspaceId }); } @@ -417,9 +414,9 @@ export class SnapshotAPI { ): Promise { try { const resolvedWorkspaceId = requireWorkspaceId(workspaceId); - return await api.invoke('get_snapshot_system_stats', { + return await invokePrepared('get_snapshot_system_stats', async () => ({ request: { ...await snapshotWorkspaceRequest(resolvedWorkspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('get_snapshot_system_stats', error, { workspaceId }); } @@ -431,9 +428,9 @@ export class SnapshotAPI { ): Promise { try { const resolvedWorkspaceId = requireWorkspaceId(workspaceId); - return await api.invoke('get_snapshot_sessions', { + return await invokePrepared('get_snapshot_sessions', async () => ({ request: { ...await snapshotWorkspaceRequest(resolvedWorkspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('get_snapshot_sessions', error, { workspaceId }); } @@ -443,9 +440,9 @@ export class SnapshotAPI { async getSessionOperations(sessionId: string, workspaceId?: string): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - return await api.invoke('get_session_operations', { + return await invokePrepared('get_session_operations', async () => ({ request: { sessionId, ...await snapshotWorkspaceRequest(scope.workspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('get_session_operations', error, { sessionId, workspaceId }); } @@ -462,12 +459,12 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - await api.invoke('record_turn_snapshot', { + await invokePrepared('record_turn_snapshot', async () => ({ session_id: sessionId, turn_index: turnIndex, modified_files: modifiedFiles, ...await snapshotWorkspaceRequest(scope.workspaceId), - }); + })); } catch (error) { throw createTauriCommandError('record_turn_snapshot', error, { sessionId, @@ -486,13 +483,13 @@ export class SnapshotAPI { ): Promise { try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - return await api.invoke('rollback_session', { + return await invokePrepared('rollback_session', async () => ({ request: { session_id: sessionId, delete_session: deleteSession, ...await snapshotWorkspaceRequest(scope.workspaceId), } - }); + })); } catch (error) { throw createTauriCommandError('rollback_session', error, { sessionId, workspaceId }); } @@ -503,27 +500,30 @@ export class SnapshotAPI { sessionId: string, workspaceId?: string, ): Promise { + const surface = getActiveSurfaceScope(); try { const scope = requireSessionSnapshotScope(sessionId, workspaceId); - const turnIndices: number[] = await api.invoke('get_session_turns', { + const turnIndices: number[] = await invokePrepared('get_session_turns', async () => ({ request: { session_id: sessionId, ...await snapshotWorkspaceRequest(scope.workspaceId), } - }); + })); const turnSnapshots: TurnSnapshot[] = []; for (const turnIndex of turnIndices) { + surface.assertCurrent('read session turn snapshots'); try { - const files: string[] = await api.invoke('get_turn_files', { + const files: string[] = await invokePrepared('get_turn_files', async () => ({ request: { session_id: sessionId, turn_index: turnIndex, ...await snapshotWorkspaceRequest(scope.workspaceId), } - }); + })); + surface.assertCurrent('read session turn snapshots'); turnSnapshots.push({ sessionId, @@ -532,6 +532,8 @@ export class SnapshotAPI { timestamp: Date.now() / 1000, }); } catch (error) { + if (isSurfaceChangedError(error)) throw error; + surface.assertCurrent('read session turn snapshots'); log.warn('Failed to get turn files', { sessionId, turnIndex, error }); // Continue processing the remaining turns. turnSnapshots.push({ @@ -543,6 +545,7 @@ export class SnapshotAPI { } } + surface.assertCurrent('read session turn snapshots'); return turnSnapshots; } catch (error) { throw createTauriCommandError('get_session_turns', error, { sessionId, workspaceId }); @@ -556,9 +559,9 @@ export class SnapshotAPI { ): Promise { try { const resolvedWorkspaceId = requireWorkspaceId(workspaceId); - const result = await api.invoke('get_file_change_history', { + const result = await invokePrepared('get_file_change_history', async () => ({ request: { file_path: filePath, ...await snapshotWorkspaceRequest(resolvedWorkspaceId) } - }); + })); return result as FileChangeEntry[]; } catch (error) { throw createTauriCommandError('get_file_change_history', error, { filePath, workspaceId }); @@ -571,9 +574,9 @@ export class SnapshotAPI { ): Promise { try { const resolvedWorkspaceId = requireWorkspaceId(workspaceId); - return await api.invoke('get_all_modified_files', { + return await invokePrepared('get_all_modified_files', async () => ({ request: { ...await snapshotWorkspaceRequest(resolvedWorkspaceId) } - }); + })); } catch (error) { throw createTauriCommandError('get_all_modified_files', error, { workspaceId }); } diff --git a/src/web-ui/src/infrastructure/api/service-api/SubagentAPI.ts b/src/web-ui/src/infrastructure/api/service-api/SubagentAPI.ts index bc5a1b3e54..3e127426e2 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SubagentAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SubagentAPI.ts @@ -1,3 +1,4 @@ +import { invokePrepared } from './invokePrepared'; /** * Subagent API */ @@ -134,35 +135,35 @@ export interface UpdateSubagentPayload { export const SubagentAPI = { async listSubagents(options?: ListSubagentsOptions): Promise { - return api.invoke('list_subagents', { + return invokePrepared('list_subagents', async () => ({ request: await workspaceScopedRequest(options ?? {}), - }); + })); }, async listVisibleSubagents(options: ListVisibleSubagentsOptions): Promise { - return api.invoke('list_visible_subagents', { + return invokePrepared('list_visible_subagents', async () => ({ request: await workspaceScopedRequest(options), - }); + })); }, async listManageableSubagents(options: ListManageableSubagentsOptions): Promise { - return api.invoke('list_manageable_subagents', { + return invokePrepared('list_manageable_subagents', async () => ({ request: await workspaceScopedRequest(options), - }); + })); }, async reloadSubagents(options: ReloadSubagentsOptions = {}): Promise { - return api.invoke('reload_subagents', { + return invokePrepared('reload_subagents', async () => ({ request: await workspaceScopedRequest(options), - }); + })); }, async createSubagent(payload: CreateSubagentPayload): Promise { - return api.invoke('create_subagent', { + return invokePrepared('create_subagent', async () => ({ request: await workspaceScopedRequest(payload), - }); + })); }, @@ -174,18 +175,18 @@ export const SubagentAPI = { async updateSubagentConfig( payload: UpdateSubagentConfigPayload, ): Promise { - return api.invoke('update_subagent_config', { + return invokePrepared('update_subagent_config', async () => ({ request: await workspaceScopedRequest(payload), - }); + })); }, async getSubagentDetail(payload: GetSubagentDetailPayload): Promise { - const raw = await api.invoke('get_subagent_detail', { + const raw = await invokePrepared('get_subagent_detail', async () => ({ request: await workspaceScopedRequest({ subagentId: payload.subagentId, workspaceId: payload.workspaceId, }), - }); + })); return { ...raw, level: raw.level === 'project' ? 'project' : 'user', @@ -193,7 +194,7 @@ export const SubagentAPI = { }, async updateSubagent(payload: UpdateSubagentPayload): Promise { - return api.invoke('update_subagent', { + return invokePrepared('update_subagent', async () => ({ request: await workspaceScopedRequest({ subagentId: payload.subagentId, description: payload.description, @@ -203,12 +204,12 @@ export const SubagentAPI = { review: payload.review, workspaceId: payload.workspaceId, }), - }); + })); }, async deleteSubagent(subagentId: string, workspaceId?: string): Promise { - return api.invoke('delete_subagent', { + return invokePrepared('delete_subagent', async () => ({ request: await workspaceScopedRequest({ subagentId, workspaceId }), - }); + })); }, }; diff --git a/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.test.ts index 40af560842..957470cf42 100644 --- a/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { workspaceAPI } from './WorkspaceAPI'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; const invokeMock = vi.hoisted(() => vi.fn()); const listenMock = vi.hoisted(() => vi.fn(() => vi.fn())); @@ -19,6 +20,7 @@ vi.mock('./ApiClient', () => ({ describe('WorkspaceAPI', () => { beforeEach(() => { + activateSurface('local'); invokeMock.mockReset(); invokeMock.mockResolvedValue('file content'); listenMock.mockReset(); @@ -248,6 +250,27 @@ describe('WorkspaceAPI', () => { ))).toBe(false); }); + it('cancels streaming on device activation without dispatching or cancelling on the next host', async () => { + streamCapabilityMock.supported = true; + let finish!: () => void; + waitForListenerRegistrationsMock.mockReturnValueOnce(new Promise(resolve => { finish = resolve; })); + const signal = new AbortController(); + const onProgress = vi.fn(); + const pending = workspaceAPI.searchFilenamesOnlyStreamDetailed( + 'same-workspace', 'file', false, false, false, 'same-search', 30, true, { onProgress }, signal.signal, + ); + const rejection = expect(pending).rejects.toSatisfy(isSurfaceChangedError); + await vi.waitFor(() => expect(waitForListenerRegistrationsMock).toHaveBeenCalledOnce()); + activateSurface('peer-with-identical-workspace'); + signal.abort(); + finish(); + await rejection; + await Promise.resolve(); + expect(invokeMock).not.toHaveBeenCalled(); + for (const result of listenMock.mock.results) expect(result.value).toHaveBeenCalledOnce(); + expect(onProgress).not.toHaveBeenCalled(); + }); + it('resolves browser-dropped file paths through a structured host request', async () => { invokeMock.mockResolvedValueOnce(['C:\\drop\\report.pdf']); diff --git a/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.ts b/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.ts index 054a8bbf03..ce49aff896 100644 --- a/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/WorkspaceAPI.ts @@ -1,10 +1,11 @@ +import { invokePrepared } from './invokePrepared'; import { workspaceScopedRequest } from './legacyWorkspaceCompatibility'; import { api } from './ApiClient'; import { workspaceIdRequest, workspaceSearchRequest, workspaceWatchRequest } from './legacyWorkspaceCompatibility'; import { globalEventBus } from '@/infrastructure/event-bus'; -import { getActiveSurfaceId, getActiveSurfaceScope, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; +import { getActiveSurfaceId, getActiveSurfaceScope, SurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; import type { FileResourceRenamedEvent } from '@/shared/types/contentResource'; import { createTauriCommandError } from '../errors/TauriCommandError'; import type { @@ -223,20 +224,15 @@ export class WorkspaceAPI { async readWorkspaceFile(workspaceId: string, filePath: string, encoding?: string): Promise { - const surface = getActiveSurfaceScope(); - const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('read workspace file'); - const content = await api.invoke('read_file_content', { request: { ...reference, filePath, encoding } }); - surface.assertCurrent('read workspace file'); - return content; + return invokePrepared('read_file_content', async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), filePath, encoding }, + })); } async writeWorkspaceFile(workspaceId: string, filePath: string, content: string): Promise { - const surface = getActiveSurfaceScope(); - const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('write workspace file'); - await api.invoke('write_file_content', { request: { ...reference, filePath, content } }); - surface.assertCurrent('write workspace file'); + await invokePrepared('write_file_content', async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), filePath, content }, + })); } /** @@ -246,67 +242,52 @@ export class WorkspaceAPI { */ private async invokeWorkspaceFileCommand( command: string, - action: string, workspaceId: string, request: Record, ): Promise { - const surface = getActiveSurfaceScope(); - const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent(action); try { - const result = await api.invoke(command, { request: { ...reference, ...request } }); - surface.assertCurrent(action); - return result; + return await invokePrepared(command, async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), ...request }, + })); } catch (error) { - if (isSurfaceChangedError(error)) throw error; throw createTauriCommandError(command, error, { workspaceId, ...request }); } } async createWorkspaceFile(workspaceId: string, path: string): Promise { - await this.invokeWorkspaceFileCommand('create_file', 'create workspace file', workspaceId, { path }); + await this.invokeWorkspaceFileCommand('create_file', workspaceId, { path }); } async deleteWorkspaceFile(workspaceId: string, path: string): Promise { - await this.invokeWorkspaceFileCommand('delete_file', 'delete workspace file', workspaceId, { path }); + await this.invokeWorkspaceFileCommand('delete_file', workspaceId, { path }); } async createWorkspaceDirectory(workspaceId: string, path: string): Promise { - await this.invokeWorkspaceFileCommand( - 'create_directory', 'create workspace directory', workspaceId, { path }, - ); + await this.invokeWorkspaceFileCommand('create_directory', workspaceId, { path }); } async deleteWorkspaceDirectory(workspaceId: string, path: string, recursive: boolean = true): Promise { - await this.invokeWorkspaceFileCommand( - 'delete_directory', 'delete workspace directory', workspaceId, { path, recursive }, - ); + await this.invokeWorkspaceFileCommand('delete_directory', workspaceId, { path, recursive }); } async renameWorkspaceFile(workspaceId: string, oldPath: string, newPath: string): Promise { const surfaceId = getActiveSurfaceId(); - await this.invokeWorkspaceFileCommand( - 'rename_file', 'rename workspace file', workspaceId, { oldPath, newPath }, - ); + await this.invokeWorkspaceFileCommand('rename_file', workspaceId, { oldPath, newPath }); globalEventBus.emit('workspace:file-renamed', { surfaceId, workspaceId, oldPath, newPath }); } async compressWorkspacePath(workspaceId: string, path: string): Promise { - return this.invokeWorkspaceFileCommand('compress_path', 'compress workspace path', workspaceId, { path }); + return this.invokeWorkspaceFileCommand('compress_path', workspaceId, { path }); } async decompressWorkspacePath(workspaceId: string, path: string): Promise { - return this.invokeWorkspaceFileCommand( - 'decompress_path', 'decompress workspace path', workspaceId, { path }, - ); + return this.invokeWorkspaceFileCommand('decompress_path', workspaceId, { path }); } async getWorkspaceFileMetadata(workspaceId: string, path: string): Promise { - const surface = getActiveSurfaceScope(); - const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('read workspace file metadata'); - const raw = await api.invoke>('get_file_metadata', { request: { ...reference, path } }); - surface.assertCurrent('read workspace file metadata'); + const raw = await invokePrepared>('get_file_metadata', async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), path }, + })); return this.fileMetadataFromRaw(raw, path); } @@ -331,9 +312,10 @@ export class WorkspaceAPI { } async resetWorkspacePersonaFiles(workspaceId: string): Promise { - const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); try { - await api.invoke('reset_workspace_persona_files', { request: reference }); + await invokePrepared('reset_workspace_persona_files', async () => ({ + request: await workspaceIdRequest(workspaceId, 'workspacePath'), + })); } catch (error) { throw createTauriCommandError('reset_workspace_persona_files', error, { workspaceId }); } @@ -417,12 +399,9 @@ export class WorkspaceAPI { async getFileTree(workspaceId: string, path: string, maxDepth?: number): Promise { try { - const surface = getActiveSurfaceScope(); - const scope = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('access workspace files'); - return await api.invoke('get_file_tree', { - request: { ...scope, path, maxDepth } - }); + return await invokePrepared('get_file_tree', async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), path, maxDepth }, + })); } catch (error) { throw createTauriCommandError('get_file_tree', error, { path, maxDepth }); } @@ -454,12 +433,9 @@ export class WorkspaceAPI { limit: number = 100 ): Promise { try { - const surface = getActiveSurfaceScope(); - const scope = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('access workspace files'); - return await api.invoke('get_directory_children_paginated', { - request: { ...scope, path, offset, limit } - }); + return await invokePrepared('get_directory_children_paginated', async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), path, offset, limit }, + })); } catch (error) { throw createTauriCommandError('get_directory_children_paginated', error, { path, offset, limit }); } @@ -468,12 +444,9 @@ export class WorkspaceAPI { async explorerGetChildren(workspaceId: string, path: string): Promise { if (!workspaceId) throw new Error('A workspace ID is required to browse workspace files'); try { - const surface = getActiveSurfaceScope(); - const scope = await workspaceIdRequest(workspaceId, 'workspacePath'); - surface.assertCurrent('access workspace files'); - return await api.invoke('explorer_get_children', { - request: { ...scope, path } - }); + return await invokePrepared('explorer_get_children', async () => ({ + request: { ...await workspaceIdRequest(workspaceId, 'workspacePath'), path }, + })); } catch (error) { throw createTauriCommandError('explorer_get_children', error, { workspaceId, path }); } @@ -548,6 +521,7 @@ export class WorkspaceAPI { searchId: string, signal?: AbortSignal ): Promise { + const surface = getActiveSurfaceScope(); if (!signal) { return resultPromise; } @@ -564,7 +538,7 @@ export class WorkspaceAPI { const abortPromise = new Promise((_, reject) => { handleAbort = () => { - void this.cancelSearch(searchId); + if (surface.isCurrent()) void this.cancelSearch(searchId); reject(new DOMException(`${commandName} aborted`, 'AbortError')); }; signal.addEventListener('abort', handleAbort, { once: true }); @@ -600,6 +574,7 @@ export class WorkspaceAPI { callbacks: FileSearchStreamCallbacks = {}, signal?: AbortSignal ): Promise { + const surface = getActiveSurfaceScope(); if (!this.supportsSearchStreamEvents()) { throw new Error(`Search streaming is unavailable for ${searchKind} searches outside Tauri`); } @@ -611,6 +586,10 @@ export class WorkspaceAPI { return await new Promise((resolve, reject) => { let settled = false; + const isCurrentSearch = (event: { searchId: string; searchKind: FileSearchStreamKind }) => ( + !settled && surface.isCurrent() + && event.searchId === request.searchId && event.searchKind === searchKind + ); const cleanupCallbacks: Array<() => void> = []; const cleanup = () => { @@ -647,10 +626,18 @@ export class WorkspaceAPI { }; const handleAbort = () => { - void this.cancelSearch(request.searchId); + if (surface.isCurrent()) void this.cancelSearch(request.searchId); settleReject(new DOMException(`${commandName} aborted`, 'AbortError')); }; + const handleSurfaceChange = () => { + settleReject(new SurfaceChangedError(surface.surfaceId, surface.epoch, commandName)); + }; + surface.signal.addEventListener('abort', handleSurfaceChange, { once: true }); + cleanupCallbacks.push(() => { + surface.signal.removeEventListener('abort', handleSurfaceChange); + }); + if (signal) { signal.addEventListener('abort', handleAbort, { once: true }); cleanupCallbacks.push(() => { @@ -660,7 +647,7 @@ export class WorkspaceAPI { void (async () => { cleanupCallbacks.push(api.listen(FILE_SEARCH_PROGRESS_EVENT, (event) => { - if (event.searchId !== request.searchId || event.searchKind !== searchKind) { + if (!isCurrentSearch(event)) { return; } @@ -668,7 +655,7 @@ export class WorkspaceAPI { })); cleanupCallbacks.push(api.listen(FILE_SEARCH_COMPLETE_EVENT, (event) => { - if (event.searchId !== request.searchId || event.searchKind !== searchKind) { + if (!isCurrentSearch(event)) { return; } @@ -676,20 +663,22 @@ export class WorkspaceAPI { })); cleanupCallbacks.push(api.listen(FILE_SEARCH_ERROR_EVENT, (event) => { - if (event.searchId !== request.searchId || event.searchKind !== searchKind) { + if (!isCurrentSearch(event)) { return; } settleReject(new Error(event.error)); })); - await api.waitForListenerRegistrations(); - if (settled || signal?.aborted) { - return; - } - const wireRequest = await workspaceSearchRequest(request); - if (settled || signal?.aborted) return; - await api.invoke(commandName, { request: wireRequest }); + await invokePrepared(commandName, async (scope) => { + await api.waitForListenerRegistrations(); + scope.assertCurrent(commandName); + const wireRequest = await workspaceSearchRequest(request); + if (settled || signal?.aborted) { + throw new DOMException(`${commandName} aborted`, 'AbortError'); + } + return { request: wireRequest }; + }); })().catch((error) => { settleReject( createTauriCommandError(commandName, error, { @@ -718,7 +707,7 @@ export class WorkspaceAPI { const effectiveSearchId = searchId ?? this.createSearchId(searchContent ? 'legacy-content' : 'legacy-filenames'); try { - const resultPromise = api.invoke('search_files', { + const resultPromise = invokePrepared('search_files', async () => ({ request: await workspaceSearchRequest({ workspaceId, pattern, @@ -730,7 +719,7 @@ export class WorkspaceAPI { maxResults, includeDirectories, }) - }); + })); return await this.raceCancelable('search_files', resultPromise, effectiveSearchId, signal); } catch (error) { @@ -796,7 +785,7 @@ export class WorkspaceAPI { } try { - const resultPromise = api.invoke('search_filenames', { + const resultPromise = invokePrepared('search_filenames', async () => ({ request: await workspaceSearchRequest({ workspaceId, pattern, @@ -807,7 +796,7 @@ export class WorkspaceAPI { maxResults, includeDirectories, }) - }); + })); return await this.raceCancelable('search_filenames', resultPromise, effectiveSearchId, effectiveSignal); } catch (error) { @@ -930,7 +919,7 @@ export class WorkspaceAPI { typeof searchIdOrSignal === 'string' ? searchIdOrSignal : this.createSearchId('content'); try { - const resultPromise = api.invoke('search_file_contents', { + const resultPromise = invokePrepared('search_file_contents', async () => ({ request: await workspaceSearchRequest({ workspaceId, pattern, @@ -940,7 +929,7 @@ export class WorkspaceAPI { wholeWord, maxResults, }) - }); + })); return await this.raceCancelable('search_file_contents', resultPromise, effectiveSearchId, effectiveSignal); } catch (error) { @@ -1024,9 +1013,10 @@ export class WorkspaceAPI { async getSearchRepoStatus(workspaceId: string): Promise { if (!workspaceId) throw new Error('Workspace ID is required for search indexing'); - const request = await workspaceIdRequest(workspaceId, 'rootPath'); try { - const raw = await api.invoke('search_get_repo_status', { request }); + const raw = await invokePrepared('search_get_repo_status', async () => ({ + request: await workspaceIdRequest(workspaceId, 'rootPath'), + })); return mapWorkspaceSearchIndexStatus(raw); } catch (error) { throw createTauriCommandError('search_get_repo_status', error, { workspaceId }); @@ -1035,9 +1025,10 @@ export class WorkspaceAPI { async buildSearchIndex(workspaceId: string): Promise { if (!workspaceId) throw new Error('Workspace ID is required for search indexing'); - const request = await workspaceIdRequest(workspaceId, 'rootPath'); try { - const raw = await api.invoke('search_build_index', { request }); + const raw = await invokePrepared('search_build_index', async () => ({ + request: await workspaceIdRequest(workspaceId, 'rootPath'), + })); return mapWorkspaceSearchIndexTaskHandle(raw); } catch (error) { throw createTauriCommandError('search_build_index', error, { workspaceId }); @@ -1046,9 +1037,10 @@ export class WorkspaceAPI { async rebuildSearchIndex(workspaceId: string): Promise { if (!workspaceId) throw new Error('Workspace ID is required for search indexing'); - const request = await workspaceIdRequest(workspaceId, 'rootPath'); try { - const raw = await api.invoke('search_rebuild_index', { request }); + const raw = await invokePrepared('search_rebuild_index', async () => ({ + request: await workspaceIdRequest(workspaceId, 'rootPath'), + })); return mapWorkspaceSearchIndexTaskHandle(raw); } catch (error) { throw createTauriCommandError('search_rebuild_index', error, { workspaceId }); @@ -1073,9 +1065,9 @@ export class WorkspaceAPI { */ async exportLocalFileToPath(sourcePath: string, destinationPath: string, workspaceId?: string): Promise { try { - await api.invoke('export_local_file_to_path', { + await invokePrepared('export_local_file_to_path', async () => ({ request: await workspaceScopedRequest({ sourcePath, destinationPath, workspaceId, controllerLocal: workspaceId === undefined }), - }); + })); } catch (error) { throw createTauriCommandError('export_local_file_to_path', error, { sourcePath, @@ -1097,11 +1089,11 @@ export class WorkspaceAPI { async startFileWatch(workspaceId: string, path: string, recursive?: boolean): Promise { - await api.invoke('start_file_watch', await workspaceWatchRequest(workspaceId, path, recursive)); + await invokePrepared('start_file_watch', async () => (await workspaceWatchRequest(workspaceId, path, recursive))); } async stopFileWatch(workspaceId: string, path: string): Promise { - await api.invoke('stop_file_watch', await workspaceWatchRequest(workspaceId, path)); + await invokePrepared('stop_file_watch', async () => (await workspaceWatchRequest(workspaceId, path))); } async getWatchedPaths(): Promise { @@ -1159,11 +1151,11 @@ export class WorkspaceAPI { workspaceId?: string, ): Promise<{ successCount: number; directoryCount: number; failedFiles: Array<{ path: string; error: string }> }> { try { - return await api.invoke('paste_files', { + return await invokePrepared('paste_files', async () => ({ request: await workspaceScopedRequest({ sourcePaths, targetDirectory, isCut, workspaceId, controllerLocal: workspaceId === undefined, }) - }); + })); } catch (error) { throw createTauriCommandError('paste_files', error, { sourcePaths, targetDirectory, isCut }); } diff --git a/src/web-ui/src/infrastructure/api/service-api/invokePrepared.test.ts b/src/web-ui/src/infrastructure/api/service-api/invokePrepared.test.ts new file mode 100644 index 0000000000..bd75287ef2 --- /dev/null +++ b/src/web-ui/src/infrastructure/api/service-api/invokePrepared.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; +import { invokePrepared } from './invokePrepared'; + +const invoke = vi.hoisted(() => vi.fn()); +vi.mock('./ApiClient', () => ({ api: { invoke } })); + +describe('prepared command device ownership', () => { + beforeEach(() => { + activateSurface('local'); + invoke.mockReset(); + }); + + it.each(['peer-b', 'local'])('rejects preparation from an expired activation before dispatch to %s', async (destination) => { + let prepare!: (args: Record) => void; + const pending = invokePrepared('delete_session', () => new Promise(resolve => { prepare = resolve; })); + const rejection = expect(pending).rejects.toSatisfy(isSurfaceChangedError); + activateSurface('peer-b'); + activateSurface(destination); + prepare({ request: { workspaceId: 'identical-id', sessionId: 'identical-session' } }); + await rejection; + expect(invoke).not.toHaveBeenCalled(); + }); + + it('classifies a late preparation failure as cancellation of the departed device', async () => { + let fail!: (error: Error) => void; + const pending = invokePrepared('get_sessions', () => new Promise((_, reject) => { fail = reject; })); + const rejection = expect(pending).rejects.toSatisfy(isSurfaceChangedError); + activateSurface('peer'); + fail(new Error('old host unavailable')); + await rejection; + expect(invoke).not.toHaveBeenCalled(); + }); + + it('preserves a current-device preparation error without dispatch', async () => { + const failure = new Error('Workspace is ambiguous'); + await expect(invokePrepared('get_sessions', async () => { throw failure; })).rejects.toBe(failure); + expect(invoke).not.toHaveBeenCalled(); + }); + + it('keeps controller-local commands authoritative across a device switch', async () => { + let prepare!: (args: Record) => void; + invoke.mockResolvedValue({ ok: true }); + const pending = invokePrepared('account_github_info', () => new Promise(resolve => { prepare = resolve; })); + activateSurface('peer-b'); + prepare({ request: {} }); + await expect(pending).resolves.toEqual({ ok: true }); + expect(invoke).toHaveBeenCalledWith('account_github_info', { request: {} }); + }); + + it('passes prepared arguments and transport options unchanged on the current device', async () => { + const args = { request: { workspaceId: 'opaque-id' } }; + const config = { timeout: 2000 }; + invoke.mockResolvedValue({ sessions: [] }); + await expect(invokePrepared('get_sessions', async () => args, config)).resolves.toEqual({ sessions: [] }); + expect(invoke).toHaveBeenCalledWith('get_sessions', args, config); + }); +}); diff --git a/src/web-ui/src/infrastructure/api/service-api/invokePrepared.ts b/src/web-ui/src/infrastructure/api/service-api/invokePrepared.ts new file mode 100644 index 0000000000..042c059e11 --- /dev/null +++ b/src/web-ui/src/infrastructure/api/service-api/invokePrepared.ts @@ -0,0 +1,41 @@ +import { getActiveSurfaceScope, type SurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; +import { PEER_CONTROLLER_LOCAL_COMMANDS } from '../generated/remoteSurface'; +import { api } from './ApiClient'; +import type { ApiRequestConfig } from './types'; + +/** + * Own the device scope before asynchronous argument preparation starts. + * + * Workspace capability negotiation can yield even on the local fast path. + * Capturing the surface only in api.invoke would then adopt the next device + * and send the previous device's IDs or paths to it. Keep preparation and + * dispatch in one scope; ApiClient retains that scope through transport/retry. + * + * Controller-local commands keep their authority across surface activation, + * matching ApiClient, so they are never cancelled by a device switch. + */ +export async function invokePrepared( + command: string, + prepare: (scope: SurfaceScope) => Promise | undefined>, + config?: ApiRequestConfig, +): Promise { + const scope = getActiveSurfaceScope(); + const controllerLocal = PEER_CONTROLLER_LOCAL_COMMANDS.has(command); + const assertCurrent = () => { + if (!controllerLocal) scope.assertCurrent(command); + }; + try { + const args = await prepare(scope); + assertCurrent(); + const result = config === undefined + ? await api.invoke(command, args) + : await api.invoke(command, args, config); + assertCurrent(); + return result; + } catch (error) { + // A late preparation/transport failure belongs to the departed device too. + // Preserve the shared cancellation signal so callers never retry it here. + assertCurrent(); + throw error; + } +} diff --git a/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.test.ts b/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.test.ts index 230bafa587..830b4bf233 100644 --- a/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest'; import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; const invoke = vi.hoisted(() => vi.fn()); vi.mock('./ApiClient', () => ({ api: { invoke } })); -import { migrateLegacySkillReceipts, upgradeLegacyWorktreeReferences, upgradeLegacyEditorWorkspaceId } from './legacyWorkspaceCompatibility'; +import { migrateLegacySkillReceipts, upgradeLegacyWorktreeReferences, upgradeLegacyEditorWorkspaceId, resolveLegacySessionWorkspace } from './legacyWorkspaceCompatibility'; const parent = { id: 'parent-id', rootPath: '/repo', workspaceKind: 'normal' }; const execution = { @@ -10,6 +10,35 @@ const execution = { worktree: { isMain: false, mainRepoPath: '/repo', mainWorkspaceId: undefined as string | undefined }, }; +describe('legacy session workspace identity', () => { + const remote = { ...parent, id: 'remote-id', workspaceKind: 'remote', connectionId: 'ssh-1', sshHost: 'localhost' }; + const otherHost = { ...remote, id: 'other-host', connectionId: 'ssh-2', sshHost: 'server-2' }; + + it('keeps same-path local and SSH records ambiguous without identity hints', () => { + expect(resolveLegacySessionWorkspace({ workspacePath: '/repo' }, [parent, remote])).toBeUndefined(); + expect(resolveLegacySessionWorkspace({ workspacePath: '/repo', remoteSshHost: 'localhost' }, [parent, remote])).toBeUndefined(); + }); + + it('uses the SSH connection and host together, including loopback hosts', () => { + const records = [parent, remote, otherHost, { ...remote, id: 'stale-host', sshHost: 'stale' }]; + expect(resolveLegacySessionWorkspace({ workspacePath: '/repo', remoteConnectionId: 'ssh-1', remoteSshHost: 'localhost' }, records)).toBe(remote); + expect(resolveLegacySessionWorkspace({ workspacePath: '/repo', remoteSshHost: 'server-2' }, records)).toBe(otherHost); + }); + + it('does not substitute a worktree project for its missing execution workspace', () => { + const old = { workspacePath: '/repo/tree', projectWorkspacePath: '/repo' }; + expect(resolveLegacySessionWorkspace(old, [parent])).toBeUndefined(); + expect(resolveLegacySessionWorkspace(old, [parent, execution])).toBe(execution); + expect(resolveLegacySessionWorkspace({ projectWorkspacePath: '/repo' }, [parent])).toBe(parent); + }); + + it('deduplicates opened/recent records by ID and never falls back from an unknown ID', () => { + expect(resolveLegacySessionWorkspace({ workspacePath: '/repo' }, [parent, parent])).toBe(parent); + expect(resolveLegacySessionWorkspace({ workspaceId: 'missing', workspacePath: '/repo' }, [parent])).toBeUndefined(); + expect(resolveLegacySessionWorkspace({ workspaceId: remote.id, workspacePath: '/stale' }, [parent, remote])).toBe(remote); + }); +}); + describe('temporary legacy worktree catalog upgrade', () => { it('resolves a local parent despite a remote record with the same path', () => { const result = upgradeLegacyWorktreeReferences([ diff --git a/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.ts b/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.ts index 72b3b2084a..3d47681a6a 100644 --- a/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.ts +++ b/src/web-ui/src/infrastructure/api/service-api/legacyWorkspaceCompatibility.ts @@ -37,13 +37,17 @@ export function resolveLegacySessionWorkspace record.id === session.workspaceId); - const roots = [session.workspacePath, session.projectWorkspacePath].filter(Boolean); - const candidates = records.filter(record => { + // Execution and project roots are different roles. A stale execution root + // must not silently select the project (or a similarly named local folder). + const root = session.workspacePath || session.projectWorkspacePath; + if (!root) return undefined; + const uniqueRecords = [...new Map(records.map(record => [record.id, record])).values()]; + const candidates = uniqueRecords.filter(record => { const normalize = record.workspaceKind === 'remote' ? normalizeRemoteWorkspacePath : normalizePath; - const matchesPath = roots.some(root => root && normalize(root).replace(/\/$/, '') === normalize(record.rootPath).replace(/\/$/, '')); + const matchesPath = normalize(root).replace(/\/$/, '') === normalize(record.rootPath).replace(/\/$/, ''); return matchesPath && (!session.remoteConnectionId || (record.workspaceKind === 'remote' && record.connectionId === session.remoteConnectionId)) - && (!session.remoteSshHost || session.remoteSshHost === 'localhost' + && (!session.remoteSshHost || (session.remoteSshHost === 'localhost' && !session.remoteConnectionId) || (record.workspaceKind === 'remote' && record.sshHost === session.remoteSshHost)); }); return candidates.length === 1 ? candidates[0] : undefined; @@ -66,7 +70,8 @@ export async function workspaceIdRequest(workspaceId: string, legacyPathField: ' export async function sessionWorkspaceIdRequest(workspaceId: string) { const request = await workspaceIdRequest(workspaceId, 'workspacePath'); if ('workspaceId' in request) return { workspace_id: request.workspaceId }; - return { workspace_path: request.workspacePath, remote_connection_id: request.remoteConnectionId }; + return { workspace_path: request.workspacePath, remote_connection_id: request.remoteConnectionId, + remote_ssh_host: request.remoteSshHost }; } /** Upgrade-only migration of 1.0.0 controller-local terminal profile keys. diff --git a/src/web-ui/src/infrastructure/mcp/toolInfoCache.test.ts b/src/web-ui/src/infrastructure/mcp/toolInfoCache.test.ts new file mode 100644 index 0000000000..f1ca3e1222 --- /dev/null +++ b/src/web-ui/src/infrastructure/mcp/toolInfoCache.test.ts @@ -0,0 +1,39 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { activateSurface } from '@/infrastructure/peer-device/deviceSurface'; +import { getCachedToolInfo, resetToolInfoCache } from './toolInfoCache'; + +const getToolInfo = vi.hoisted(() => vi.fn()); +vi.mock('@/infrastructure/api/service-api/ToolAPI', () => ({ toolAPI: { getToolInfo } })); + +describe('tool info cache', () => { + beforeEach(() => { + activateSurface('local'); + resetToolInfoCache(); + getToolInfo.mockReset(); + }); + + it('reuses a description only on the device that reported it', async () => { + getToolInfo + .mockResolvedValueOnce({ name: 'mcp_search', description: 'local server' }) + .mockResolvedValueOnce({ name: 'mcp_search', description: 'peer server' }); + + await expect(getCachedToolInfo('mcp_search')).resolves.toMatchObject({ description: 'local server' }); + await expect(getCachedToolInfo('mcp_search')).resolves.toMatchObject({ description: 'local server' }); + activateSurface('peer-b'); + await expect(getCachedToolInfo('mcp_search')).resolves.toMatchObject({ description: 'peer server' }); + activateSurface('local'); + await expect(getCachedToolInfo('mcp_search')).resolves.toMatchObject({ description: 'local server' }); + expect(getToolInfo).toHaveBeenCalledTimes(2); + }); + + it('retries after a failure without dropping a newer request', async () => { + getToolInfo + .mockRejectedValueOnce(new Error('host unavailable')) + .mockResolvedValueOnce({ name: 'mcp_search', description: 'recovered' }); + + await expect(getCachedToolInfo('mcp_search')).rejects.toThrow('host unavailable'); + await expect(getCachedToolInfo('mcp_search')).resolves.toMatchObject({ description: 'recovered' }); + await expect(getCachedToolInfo('mcp_search')).resolves.toMatchObject({ description: 'recovered' }); + expect(getToolInfo).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/web-ui/src/infrastructure/mcp/toolInfoCache.ts b/src/web-ui/src/infrastructure/mcp/toolInfoCache.ts index f10a85f4cc..bc0ce4ffe2 100644 --- a/src/web-ui/src/infrastructure/mcp/toolInfoCache.ts +++ b/src/web-ui/src/infrastructure/mcp/toolInfoCache.ts @@ -1,19 +1,29 @@ import { toolAPI } from '@/infrastructure/api/service-api/ToolAPI'; +import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; import type { ToolInfo } from '@/shared/types/agent-api'; +/** Tool descriptions come from the rendered device's MCP servers. */ const toolInfoCache = new Map>(); export function getCachedToolInfo(toolName: string): Promise { - const cached = toolInfoCache.get(toolName); + const key = getActiveSurfaceScope().key(toolName); + const cached = toolInfoCache.get(key); if (cached) { return cached; } const request = toolAPI.getToolInfo(toolName).catch((error) => { - toolInfoCache.delete(toolName); + if (toolInfoCache.get(key) === request) { + toolInfoCache.delete(key); + } throw error; }); - toolInfoCache.set(toolName, request); + toolInfoCache.set(key, request); return request; } + +/** Test seam: forgets every cached tool description. */ +export function resetToolInfoCache(): void { + toolInfoCache.clear(); +} diff --git a/src/web-ui/src/infrastructure/peer-device/README.md b/src/web-ui/src/infrastructure/peer-device/README.md index ef91be22b8..dbf1856e00 100644 --- a/src/web-ui/src/infrastructure/peer-device/README.md +++ b/src/web-ui/src/infrastructure/peer-device/README.md @@ -102,9 +102,26 @@ listener alone recovers an interaction emitted before attachment. peer host (e.g. Mac). `initialize()` failure must **throw**, never return `false` (callers treat `false` as “no history → create session”). -5. **Create-session always passes the live workspace path** - (`flowChatSessionConfigForWorkspace`). Empty `{}` configs are unsafe after - peer switch. +5. **Create-session always passes the live workspace ID** + (`flowChatSessionConfigForWorkspace`). Paths are execution projections; the + temporary old-peer adapter alone converts an ID to legacy path/SSH fields. + Empty `{}` configs are unsafe after a peer switch. + + Service calls with asynchronous argument preparation use `invokePrepared` so + preparation and invocation share one `SurfaceScope`. Capturing the scope only + inside `api.invoke(command, { request: await ... })` is too late: argument + evaluation may resume on another device. Multi-step preparation checks that + scope before each subsequent host operation. Service error wrappers preserve + `SurfaceChangedError`, and pending-operation dedup includes the activation + epoch so A → B → A never reuses abandoned work from A's earlier activation. + ESLint rejects `await` inside `api.invoke(...)` arguments. Controller-local + commands (`PEER_CONTROLLER_LOCAL_COMMANDS`) skip the activation check in + `invokePrepared`, as they do in `ApiClient`. + + Settled caches and in-flight maps key by the rendered surface as well as the + workspace ID: two devices with the same local path produce the same + workspace ID. A user answer collected on one device (for example a Git trust + prompt) is re-checked against the current activation before it is applied. 6. **Config / mode HostInvokes are high priority** during peer hydrate (`get_config`, `get_configs`, `get_available_modes`, diff --git a/src/web-ui/src/infrastructure/services/business/workspaceManager.ts b/src/web-ui/src/infrastructure/services/business/workspaceManager.ts index d34cf85883..ff0d671f63 100644 --- a/src/web-ui/src/infrastructure/services/business/workspaceManager.ts +++ b/src/web-ui/src/infrastructure/services/business/workspaceManager.ts @@ -25,6 +25,8 @@ import { type SurfaceScope, } from '@/infrastructure/peer-device/deviceSurface'; import { routeSurfaceEvent } from '@/infrastructure/peer-device/deviceSurfaceRouting'; +import { isRemoteWorkspaceConnectionConflictError } from '@/infrastructure/api/errors/TauriCommandError'; +import type { OpenRemoteWorkspaceOptions } from '@/infrastructure/api/service-api/GlobalAPI'; const log = createLogger('WorkspaceManager'); @@ -1068,7 +1070,7 @@ class WorkspaceManager { connectionName: string; remotePath: string; sshHost?: string; - }): Promise { + }, options: OpenRemoteWorkspaceOptions = {}): Promise { const surface = this.captureSurface(); try { this.setLoading(true); @@ -1083,6 +1085,7 @@ class WorkspaceManager { remoteWorkspace.connectionId, remoteWorkspace.connectionName, remoteWorkspace.sshHost, + options, ); const [recentWorkspaces, openedWorkspaces] = await Promise.all([ @@ -1105,6 +1108,12 @@ class WorkspaceManager { if (!this.isSurfaceUnchanged(surface)) { throw error; } + if (isRemoteWorkspaceConnectionConflictError(error)) { + // The caller owns the decision; the record is unchanged on the host. + log.warn('Remote workspace is bound to another SSH connection', { remoteWorkspace }); + this.setLoading(false); + throw error; + } log.error('Failed to open remote workspace', { remoteWorkspace, error }); const errorMessage = error instanceof Error ? error.message : String(error); this.updateState({ loading: false, error: errorMessage }, { type: 'workspace:error', error: errorMessage }); diff --git a/src/web-ui/src/locales/en-US/common.json b/src/web-ui/src/locales/en-US/common.json index 0041f8b44b..700941ec23 100644 --- a/src/web-ui/src/locales/en-US/common.json +++ b/src/web-ui/src/locales/en-US/common.json @@ -1864,7 +1864,11 @@ "downloadDialogTitle": "Save file as", "transferFailed": "Transfer failed", "transferNeedsDesktop": "Upload and download require the desktop app.", - "transferring": "Transferring..." + "transferring": "Transferring...", + "connectionConflictTitle": "Workspace bound to another connection", + "connectionConflictMessage": "The remote workspace {{path}} is bound to SSH connection {{owner}}. Rebind it to {{connection}}? Its files, terminals, and sessions will then run through {{connection}}.", + "connectionConflictConfirm": "Rebind", + "connectionConflictRestoreDeferred": "Remote workspace was kept because it is bound to another SSH connection. Open it again from the remote connection dialog to choose a connection: {{path}}" }, "portForward": { "menuEntry": "Forward a Port", diff --git a/src/web-ui/src/locales/zh-CN/common.json b/src/web-ui/src/locales/zh-CN/common.json index 76860d7808..b1f3ff31d7 100644 --- a/src/web-ui/src/locales/zh-CN/common.json +++ b/src/web-ui/src/locales/zh-CN/common.json @@ -1864,7 +1864,11 @@ "downloadDialogTitle": "保存文件", "transferFailed": "传输失败", "transferNeedsDesktop": "上传与下载仅适用于桌面版应用。", - "transferring": "正在传输…" + "transferring": "正在传输…", + "connectionConflictTitle": "工作区已绑定其他连接", + "connectionConflictMessage": "远程工作区 {{path}} 已绑定到 SSH 连接 {{owner}}。是否改为绑定到 {{connection}}？之后它的文件、终端和会话都将通过 {{connection}} 执行。", + "connectionConflictConfirm": "改为绑定", + "connectionConflictRestoreDeferred": "远程工作区已保留，因为它绑定的是另一个 SSH 连接。请从远程连接对话框重新打开它并选择连接：{{path}}" }, "portForward": { "menuEntry": "端口映射", diff --git a/src/web-ui/src/locales/zh-TW/common.json b/src/web-ui/src/locales/zh-TW/common.json index 1e8af19a23..38d2a50cef 100644 --- a/src/web-ui/src/locales/zh-TW/common.json +++ b/src/web-ui/src/locales/zh-TW/common.json @@ -1864,7 +1864,11 @@ "downloadDialogTitle": "儲存檔案", "transferFailed": "傳輸失敗", "transferNeedsDesktop": "上傳與下載僅適用於桌面版應用。", - "transferring": "正在傳輸…" + "transferring": "正在傳輸…", + "connectionConflictTitle": "工作區已綁定其他連線", + "connectionConflictMessage": "遠端工作區 {{path}} 已綁定到 SSH 連線 {{owner}}。是否改為綁定到 {{connection}}？之後它的檔案、終端機和工作階段都將透過 {{connection}} 執行。", + "connectionConflictConfirm": "改為綁定", + "connectionConflictRestoreDeferred": "遠端工作區已保留，因為它綁定的是另一個 SSH 連線。請從遠端連線對話框重新開啟它並選擇連線：{{path}}" }, "portForward": { "menuEntry": "連接埠對應", diff --git a/src/web-ui/src/shared/services/gitTrustService.test.ts b/src/web-ui/src/shared/services/gitTrustService.test.ts index 34c2633f83..b7325218f0 100644 --- a/src/web-ui/src/shared/services/gitTrustService.test.ts +++ b/src/web-ui/src/shared/services/gitTrustService.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { TauriCommandError } from '@/infrastructure/api/errors/TauriCommandError'; +import { activateSurface, isSurfaceChangedError } from '@/infrastructure/peer-device/deviceSurface'; import { describeGitTrustFailure, requestGitRepositoryTrust, @@ -59,6 +60,7 @@ function grantedOutcome() { } beforeEach(() => { + activateSurface('local'); resetGitTrustDecisions(); confirmWarningMock.mockReset(); trustRepositoryMock.mockReset(); @@ -114,6 +116,29 @@ describe('requestGitRepositoryTrust', () => { expect(trustRepositoryMock).toHaveBeenCalledTimes(1); }); + it('never grants an answer from a departed device to the device rendered now', async () => { + let resolveConfirm!: (value: boolean) => void; + confirmWarningMock.mockReturnValueOnce( + new Promise((resolve) => { + resolveConfirm = resolve; + }), + ); + trustRepositoryMock.mockResolvedValue(grantedOutcome()); + const workspace = { workspaceId: 'same-path-id', repositoryPath: REPOSITORY_PATH }; + + const departed = requestGitRepositoryTrust(workspace); + activateSurface('peer-b'); + confirmWarningMock.mockResolvedValueOnce(false); + const current = requestGitRepositoryTrust(workspace); + resolveConfirm(true); + + await expect(departed).rejects.toSatisfy(isSurfaceChangedError); + await expect(current).resolves.toBe(false); + expect(confirmWarningMock).toHaveBeenCalledTimes(2); + expect(trustRepositoryMock).not.toHaveBeenCalled(); + expect(warningMock).not.toHaveBeenCalled(); + }); + it('does not ask again in the quiet period after a decline', async () => { confirmWarningMock.mockResolvedValue(false); diff --git a/src/web-ui/src/shared/services/gitTrustService.ts b/src/web-ui/src/shared/services/gitTrustService.ts index 4cedd448a3..6529489cbd 100644 --- a/src/web-ui/src/shared/services/gitTrustService.ts +++ b/src/web-ui/src/shared/services/gitTrustService.ts @@ -23,6 +23,11 @@ import { i18nService } from '@/infrastructure/i18n'; import { notificationService } from '@/shared/notification-system'; import { createLogger } from '@/shared/utils/logger'; import { GitWorkspaceScope, gitWorkspaceKey } from '@/infrastructure/api/service-api/GitAPI'; +import { + getActiveSurfaceScope, + isSurfaceChangedError, + type SurfaceScope, +} from '@/infrastructure/peer-device/deviceSurface'; const log = createLogger('GitTrustService'); @@ -72,6 +77,7 @@ async function readTrustReport(repositoryPath: GitWorkspaceScope): Promise { +async function promptAndTrust(repositoryPath: GitWorkspaceScope, scope: SurfaceScope): Promise { const confirmed = await confirmWarning( i18nService.t('panels/git:trust.title'), i18nService.t('panels/git:trust.message', { path: repositoryPath.repositoryPath ?? repositoryPath.workspaceId }), @@ -141,6 +147,9 @@ async function promptAndTrust(repositoryPath: GitWorkspaceScope): Promise { + const scope = getActiveSurfaceScope(); const key = promptKey(repositoryPath); - const pending = inFlightRequests.get(key); + const flightKey = scope.key(scope.epoch, key); + const pending = inFlightRequests.get(flightKey); if (pending) { return pending; } @@ -214,10 +226,12 @@ export function requestGitRepositoryTrust( promptQuietUntil.delete(key); } - const request = promptAndTrust(repositoryPath).finally(() => { - inFlightRequests.delete(key); + const request = promptAndTrust(repositoryPath, scope).finally(() => { + if (inFlightRequests.get(flightKey) === request) { + inFlightRequests.delete(flightKey); + } }); - inFlightRequests.set(key, request); + inFlightRequests.set(flightKey, request); return request; } diff --git a/src/web-ui/src/shared/types/global-state.ts b/src/web-ui/src/shared/types/global-state.ts index 73265d3961..de0a5f720a 100644 --- a/src/web-ui/src/shared/types/global-state.ts +++ b/src/web-ui/src/shared/types/global-state.ts @@ -6,6 +6,7 @@ import { workspaceAPI } from '@/infrastructure/api'; import type { ApplicationState as APIApplicationState, AppStatus as APIAppStatus, + OpenRemoteWorkspaceOptions, RemoteWorkspaceSnapshot as APIRemoteWorkspaceSnapshot, WorkspaceStartupStateSnapshot as APIWorkspaceStartupStateSnapshot, WorkspaceInfo as APIWorkspaceInfo, @@ -240,7 +241,8 @@ export interface GlobalStateAPI { remotePath: string, connectionId: string, connectionName: string, - sshHost?: string + sshHost?: string, + options?: OpenRemoteWorkspaceOptions ): Promise; createAssistantWorkspace(): Promise; getPrimaryAssistantWorkspace(): Promise; @@ -550,10 +552,17 @@ export function createGlobalStateAPI(): GlobalStateAPI { remotePath: string, connectionId: string, connectionName: string, - sshHost?: string + sshHost?: string, + options?: OpenRemoteWorkspaceOptions ): Promise { return mapWorkspaceInfo( - await globalAPI.openRemoteWorkspace(remotePath, connectionId, connectionName, sshHost) + await globalAPI.openRemoteWorkspace( + remotePath, + connectionId, + connectionName, + sshHost, + options, + ) ); }, diff --git a/src/web-ui/src/tools/editor/services/EditorDocument.tsx b/src/web-ui/src/tools/editor/services/EditorDocument.tsx index 44aa4d05ce..16498dc6d1 100644 --- a/src/web-ui/src/tools/editor/services/EditorDocument.tsx +++ b/src/web-ui/src/tools/editor/services/EditorDocument.tsx @@ -2,7 +2,7 @@ import { createContext, useContext } from 'react'; import { getActiveSurfaceId, getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface'; import type { ContentResourceScope } from '@/shared/types/contentResource'; import { workspaceAPI } from '@/infrastructure/api/service-api/WorkspaceAPI'; -import { api } from '@/infrastructure/api/service-api/ApiClient'; +import { invokePrepared } from '@/infrastructure/api/service-api/invokePrepared'; import { upgradeLegacyEditorWorkspaceId, workspaceIdRequest } from '@/infrastructure/api/service-api/legacyWorkspaceCompatibility'; import { monacoModelManager } from './MonacoModelManager'; import { resourcePathKey } from '@/shared/utils/resourcePath'; @@ -77,14 +77,13 @@ export class EditorDocument { }, }; readonly invoke = (command: string, args: { request: Record }): Promise => - this.run(async assertCurrent => { + this.run(() => invokePrepared(command, async scope => { const workspaceId = await this.workspaceId(); - assertCurrent(); + scope.assertCurrent(command); const reference = await workspaceIdRequest(workspaceId, 'workspacePath'); - assertCurrent(); const { workspacePath: _legacyRoot, remoteConnectionId: _legacyConnection, workspaceId: _callerId, ...request } = args.request; - return api.invoke(command, { ...args, request: { ...request, ...reference } }); - }); + return { ...args, request: { ...request, ...reference } }; + })); } const documents = new Map(); diff --git a/src/web-ui/src/tools/terminal/services/TerminalService.ts b/src/web-ui/src/tools/terminal/services/TerminalService.ts index f07296dd22..7fc7dd71f4 100644 --- a/src/web-ui/src/tools/terminal/services/TerminalService.ts +++ b/src/web-ui/src/tools/terminal/services/TerminalService.ts @@ -6,6 +6,7 @@ import { workspaceIdRequest } from '@/infrastructure/api/service-api/legacyWorks */ import { api } from '@/infrastructure/api/service-api/ApiClient'; +import { invokePrepared } from '@/infrastructure/api/service-api/invokePrepared'; import { createLogger } from '@/shared/utils/logger'; import { getActiveSurfaceScope, @@ -256,20 +257,17 @@ export class TerminalService { } async createSession(request: CreateSessionRequest): Promise { - const scope = getActiveSurfaceScope(); + const { surfaceId } = getActiveSurfaceScope(); try { - let payload = request; - if (request.workspaceId) { + const session = await invokePrepared('terminal_create', async () => { + if (!request.workspaceId) return { request }; const reference = await workspaceIdRequest(request.workspaceId, 'workspacePath'); - if (!('workspaceId' in reference)) { - payload = { ...request, workspaceId: undefined, connectionId: reference.remoteConnectionId ?? '' }; - } - } - scope.assertCurrent('resolve terminal workspace'); - const session = await api.invoke('terminal_create', { request: payload }); - scope.assertCurrent('terminal_create'); + return 'workspaceId' in reference + ? { request } + : { request: { ...request, workspaceId: undefined, connectionId: reference.remoteConnectionId ?? '' } }; + }); log.debug('Session created', { sessionId: session.id }); - return this.projectSession(scope.surfaceId, { + return this.projectSession(surfaceId, { ...session, workspaceId: session.workspaceId || request.workspaceId, initialCwd: session.initialCwd || request.workingDirectory || session.cwd, }); } catch (error) {