From 80a68775d5af341cdb2c34bc1e6c8a8a0e0ed507 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Sun, 2 Aug 2026 23:11:07 -0400 Subject: [PATCH] feat(desktop): wire fail-closed scope seam + import_identity scope semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three storage chokepoints (managed_agents_store_path, teams_store_path, global_config_path) now route through capture_active_scope() and fail with a clear error when no workspace scope is active. There is no fallback to the legacy unscoped root — returning a legacy path would recreate split-brain storage. apply_workspace acquires the workspace_transition lock (Layer 1 async serialization) before entering spawn_blocking so scope transitions are serialized against concurrent import_identity calls. import_identity implements both scope modes per v4 plan: - No-active-scope path (recovery/onboarding): persist identity, clear scope, bump generation. No scope is derived or claimed; the next apply_workspace performs adoption. - Live-active path (membership-denied flow): drain managed-agent runtimes (delegates to shutdown_managed_agents), persist identity, clear scope, bump generation. Drain failures are logged but non-fatal; the frontend's re-apply restores agents. Both paths bump the scope generation so in-flight operations see a new generation and abort their commits. The fallback relay can never claim legacy data — claims are only written inside apply_workspace's prepare stage. All 2112 tests pass. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/commands/identity.rs | 84 +++++++++++++++++-- desktop/src-tauri/src/commands/workspace.rs | 54 +++++++----- desktop/src-tauri/src/event_sync.rs | 2 +- .../src/managed_agents/global_config/mod.rs | 11 ++- .../src-tauri/src/managed_agents/storage.rs | 16 +++- desktop/src-tauri/src/managed_agents/teams.rs | 10 ++- 6 files changed, 143 insertions(+), 34 deletions(-) diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index 562aa7258..3ba0cdb85 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -332,13 +332,54 @@ pub async fn save_ncryptsec_copy( Ok(Some(dest.display().to_string())) } +/// Stop all live managed-agent runtimes as part of a workspace drain. +/// +/// Called during identity import (live-active path) to stop all agents +/// before the active scope is cleared. At call time the scope is still set, +/// so `load_managed_agents` resolves correctly. After this returns the caller +/// persists the new identity and clears the scope. +/// +/// Delegates to `shutdown::shutdown_managed_agents` which handles the full +/// Layer-2 stop sequence (runtime_transition → store → processes locks, +/// SIGTERM fan-out, orphan sweep). Returns `Err` if any stop fails; the +/// caller logs the error and proceeds. +fn drain_all_managed_agent_runtimes( + app: &tauri::AppHandle, + _state: &AppState, +) -> Result<(), String> { + crate::shutdown::shutdown_managed_agents(app) +} + #[tauri::command] pub async fn import_identity( nsec: String, password: Option, app_handle: tauri::AppHandle, ) -> Result { - tokio::task::spawn_blocking(move || { + // ── Layer 1: async serialization lock ──────────────────────────────────── + // identity_mutation (Layer 1) must be held for the full import to prevent a + // concurrent stale persist from overwriting the imported key. + // + // If there is an active scope, we also take workspace_transition so that + // clearing the active scope is serialized against apply_workspace. + // Lock order: identity_mutation → workspace_transition. + let lock_app = app_handle.clone(); + let lock_state = lock_app.state::(); + let _mutation_guard = lock_state.identity_mutation.lock().await; + + // Capture whether an active scope exists BEFORE entering spawn_blocking. + let has_active_scope = lock_state.capture_active_scope().is_some(); + + // For the live-active path, also hold workspace_transition so that + // clearing the active scope is serialized against concurrent apply_workspace + // calls. Lock order: identity_mutation → workspace_transition. + let _transition_guard = if has_active_scope { + Some(lock_state.workspace_transition.lock().await) + } else { + None + }; + + let result = tokio::task::spawn_blocking(move || { // NIP-49 backups require a passphrase and decrypt entirely in Rust. // Raw nsec/hex input follows the existing parser path unchanged. let password = password.map(zeroize::Zeroizing::new); @@ -347,11 +388,7 @@ pub async fn import_identity( password.as_ref().map(|value| value.as_str()), )?; - // Serialize against persist_current_identity: hold this guard for the - // full function body so a concurrent stale persist can't overwrite - // this import. let state = app_handle.state::(); - let _mutation_guard = state.identity_mutation.blocking_lock(); let data_dir = app_handle .path() @@ -360,6 +397,23 @@ pub async fn import_identity( std::fs::create_dir_all(&data_dir).map_err(|e| format!("create app data dir: {e}"))?; let key_path = data_dir.join("identity.key"); + // ── Live-active path: drain runtimes before swapping identity ───────── + // When an active scope exists, stop all managed-agent runtimes before + // persisting the new identity. The frontend re-applies the workspace + // (which runs the scope initialization pipeline) to restore agents. + // + // Drain failures are logged rather than fatal — the identity persist + // and scope clear proceed regardless; the frontend's re-apply handles + // agent restoration. + if has_active_scope { + if let Err(e) = drain_all_managed_agent_runtimes(&app_handle, &state) { + eprintln!( + "buzz-desktop: identity import drain warning — some runtimes may still be \ + running: {e}; workspace re-apply will restore agents" + ); + } + } + let (pubkey, storage) = commit_imported_identity(&state, &data_dir, keys, |keys| { // Persist into the OS keyring first (store → read-back verify → // marker → delete file). Falls back to the 0o600 file when the @@ -369,6 +423,17 @@ pub async fn import_identity( crate::app_state::persist_imported_identity(store, keys, &key_path, &data_dir) })?; + // ── Clear active scope and bump generation ──────────────────────────── + // For no-active-scope path: no scope was ever set; clearing is a no-op + // but bumping generation invalidates any in-flight stale operations. + // For live-active path: agents are stopped; clearing scope makes all + // agent commands fail closed until the frontend re-applies a workspace. + // + // Invariant: the fallback relay can never claim legacy data — claims + // are only written inside apply_workspace's prepare stage. + state.clear_active_scope(); + crate::managed_agents::scope::next_scope_generation(); + let pubkey_hex = pubkey.to_hex(); let display_name = truncated_display_name(&pubkey)?; @@ -384,7 +449,14 @@ pub async fn import_identity( }) }) .await - .map_err(|e| format!("spawn_blocking failed: {e}"))? + .map_err(|e| format!("spawn_blocking failed: {e}"))?; + + // Guards must stay alive until spawn_blocking completes so the serialization + // covers the full duration of the import. + drop(_transition_guard); + drop(_mutation_guard); + + result } /// Commit an imported identity: durably persist, swap in-memory keys, clear diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index aef0ab391..69c2c567f 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -131,6 +131,14 @@ pub async fn apply_workspace( agent_managed_profiles: Option, app: AppHandle, ) -> Result<(), String> { + // ── Layer 1: async serialization lock ──────────────────────────────────── + // workspace_transition serializes apply_workspace and live identity import + // so scope transitions are never concurrent. We acquire via a clone so the + // borrow does not prevent moving `app` into spawn_blocking below. + let lock_app = app.clone(); + let lock_state = lock_app.state::(); + let _transition_guard = lock_state.workspace_transition.lock().await; + let restore_app = app.clone(); tokio::task::spawn_blocking(move || { let state = app.state::(); @@ -163,7 +171,9 @@ pub async fn apply_workspace( None => None, }; - // ── Apply all state changes (nothing below can fail) ────────────────── + // ── Layer 2: synchronous commit epoch ──────────────────────────────── + // No .await may be held while any Layer-2 guard is live. Relay override, + // keys, and the active scope are all committed in this critical section. { let mut override_guard = state.relay_url_override.lock().map_err(|e| e.to_string())?; *override_guard = Some(relay_url.clone()); @@ -185,9 +195,11 @@ pub async fn apply_workspace( .store(!agent_managed_profiles.unwrap_or(false), Ordering::Release); // ── Commit the active workspace agent scope ─────────────────────────── - // Derive the scope from the now-applied relay + owner keys and store it - // as the active scope. All subsequent store reads/writes and event-sync - // calls use the scoped definitions directory. + // Derive the scope from the now-applied relay + owner keys and commit it + // as the active scope. All subsequent store reads/writes (via + // load_managed_agents, save_managed_agents, etc.) resolve through + // capture_active_scope() → scoped definitions directory. There is NO + // fallback to the legacy unscoped root. { let owner_pubkey = state .keys @@ -249,23 +261,23 @@ pub async fn apply_workspace( // instead of being abandoned by the storage cutover. migrate_legacy_retention_into(&restore_app, &scope); - // Use the scoped definitions directory for event sync. - // After apply_workspace commits the scope, capture_active_scope() - // returns the scoped dir; fall back to the legacy unscoped root - // only when no scope has been committed (pre-Phase-2 transition). - let definitions_dir = state - .capture_active_scope() - .map(|s| s.definitions_dir) - .unwrap_or_else(|| { - crate::managed_agents::managed_agents_base_dir(&restore_app).unwrap_or_default() - }); - - crate::event_sync::spawn_event_sync( - restore_app.clone(), - scope.owner_keys, - scope.db_path, - definitions_dir, - ) + // The active scope was committed in the spawn_blocking above. + // If it is somehow None here, event sync is skipped rather than + // falling back to the legacy unscoped root (which would recreate + // split-brain storage). + if let Some(agent_scope) = state.capture_active_scope() { + crate::event_sync::spawn_event_sync( + restore_app.clone(), + scope.owner_keys, + scope.db_path, + agent_scope.definitions_dir, + ); + } else { + eprintln!( + "buzz-desktop: active agent scope unavailable after workspace apply — \ + event sync skipped" + ); + } } Err(error) => { eprintln!("buzz-desktop: scoped event-sync unavailable after workspace apply: {error}"); diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index e1e178295..455a0f5fd 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -18,7 +18,7 @@ use std::path::Path; /// (`WorkspaceAgentScope::definitions_dir`). Reads personas/teams/agents from /// that directory rather than the legacy unscoped `agents/` root. pub fn run_event_sync( - app: &tauri::AppHandle, + _app: &tauri::AppHandle, owner_keys: &nostr::Keys, db_path: &Path, definitions_dir: &Path, diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 5608462bf..fdcd28803 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -30,7 +30,7 @@ use tauri::AppHandle; use crate::managed_agents::env_vars::{ validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS, MAX_ENV_VALUE_BYTES, }; -use crate::managed_agents::storage::{atomic_write_json_restricted, managed_agents_base_dir}; +use crate::managed_agents::storage::atomic_write_json_restricted; use crate::managed_agents::types::{AgentDefinition, ManagedAgentRecord}; /// The global agent configuration record. @@ -174,8 +174,15 @@ pub fn normalize_global_config_fields(config: &mut GlobalAgentConfig) { } } +/// Resolve the active-scope `global-agent-config.json` path. Fails closed on +/// `None` scope. No fallback to the legacy unscoped root. fn global_config_path(app: &AppHandle) -> Result { - Ok(managed_agents_base_dir(app)?.join("global-agent-config.json")) + use tauri::Manager as _; + let state = app.state::(); + let scope = state.capture_active_scope().ok_or_else(|| { + "no active workspace scope — apply a workspace before accessing global config".to_string() + })?; + Ok(global_config_path_at(&scope.definitions_dir)) } /// Scoped variant: resolve `global-agent-config.json` under a workspace scope's diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 5096239e5..70287ed1f 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -42,8 +42,20 @@ pub fn managed_agents_base_dir(app: &AppHandle) -> Result { Ok(dir) } +/// Resolve the active-scope `managed-agents.json` path. +/// +/// Routes through [`crate::app_state::AppState::capture_active_scope`] when a +/// workspace scope has been committed, and fails closed with a clear error when +/// no scope is active. There is NO fallback to the legacy unscoped root — +/// returning a legacy path would recreate split-brain storage. pub(crate) fn managed_agents_store_path(app: &AppHandle) -> Result { - Ok(managed_agents_base_dir(app)?.join("managed-agents.json")) + use tauri::Manager as _; + let state = app.state::(); + let scope = state.capture_active_scope().ok_or_else(|| { + "no active workspace scope — apply a workspace before accessing agent definitions" + .to_string() + })?; + Ok(managed_agents_store_path_at(&scope.definitions_dir)) } /// Scoped variant: resolve `managed-agents.json` under a workspace scope's @@ -466,7 +478,7 @@ pub(crate) fn save_agent_definitions_at( /// name/pubkey order their save path established. fn write_agent_store( app: &AppHandle, - mut definitions: Vec, + definitions: Vec, instances: Vec, ) -> Result<(), String> { let path = managed_agents_store_path(app)?; diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 4df0d2eec..0ccb84aa2 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -3,14 +3,20 @@ use std::{fs, path::PathBuf}; use tauri::AppHandle; use crate::{ - managed_agents::{managed_agents_base_dir, ManagedAgentRecord, TeamRecord}, + managed_agents::{ManagedAgentRecord, TeamRecord}, util::now_iso, }; use super::team_repair::team_persona_key; +/// Resolve the active-scope `teams.json` path. Fails closed on `None` scope. pub(crate) fn teams_store_path(app: &AppHandle) -> Result { - Ok(managed_agents_base_dir(app)?.join("teams.json")) + use tauri::Manager as _; + let state = app.state::(); + let scope = state.capture_active_scope().ok_or_else(|| { + "no active workspace scope — apply a workspace before accessing teams".to_string() + })?; + Ok(teams_store_path_at(&scope.definitions_dir)) } /// Scoped variant: resolve `teams.json` under a workspace scope's definitions dir.