From 58ead527a91ae43ef50b8a5423c61b2e61ed84a3 Mon Sep 17 00:00:00 2001 From: Duncan Date: Mon, 17 Aug 2026 13:27:52 -0400 Subject: [PATCH] fix(agents): bind session-config frames to the emitting runtime (P23-C1) The process-global session_config_cache was keyed by {pubkey, relay_url} with no owner, scope, or generation, and put_agent_session_config gated only on a same-pubkey record in the then-active store. A delayed session_config_captured frame from workspace A could survive an A->B drain, repopulate B's colliding key, and surface A's live session as B's. Lifecycle frames already carried the missing capability (startNonce + tracked-live-runtime check); session-config frames did not. Move the cache onto ManagedAgentPairRuntime (session_config), making an ownerless entry unrepresentable and destroyed atomically with the runtime on drain/removal/exit-prune. session_config_captured now carries startNonce; no-nonce old-harness frames are dropped with no fallback. Admission requires the frame's exact {pubkey, relay_url, start_nonce} to match a still-live tracked runtime whose scope_id equals the current exact scope, validated under the runtime-map lock immediately before the sole mutation; the store read is demoted to enrichment. get_agent_config_surface consumes the cache only through the same still-current capability. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-acp/src/lib.rs | 1 + crates/buzz-acp/src/pool.rs | 14 +- desktop/src-tauri/src/app_state.rs | 28 - .../src-tauri/src/commands/agent_config.rs | 158 ++++-- .../commands/agent_config_capability_tests.rs | 508 ++++++++++++++++++ .../src-tauri/src/commands/agent_models.rs | 10 +- .../src-tauri/src/commands/agent_settings.rs | 10 +- desktop/src-tauri/src/commands/agents.rs | 31 +- .../src-tauri/src/commands/personas/mod.rs | 6 +- .../src/managed_agents/runtime/stop.rs | 12 +- .../src/managed_agents/runtime_commands.rs | 24 +- .../src/managed_agents/runtime_types.rs | 19 +- 12 files changed, 671 insertions(+), 150 deletions(-) create mode 100644 desktop/src-tauri/src/commands/agent_config_capability_tests.rs diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 65c9dd620..2ad0af755 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1843,6 +1843,7 @@ async fn tokio_main() -> Result<()> { memory_enabled: config.memory_enabled, harness_name: crate::config::normalize_agent_command_identity(&config.agent_command), relay_url: config.relay_url.clone(), + start_nonce: runtime_start_nonce.clone(), }); if !config.memory_enabled { diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 8430307d9..e664b807d 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -564,6 +564,13 @@ pub struct PromptContext { /// the desktop keys per (agent, relay) pair, e.g. `session_config_captured`, /// mirroring the `managed_agent_runtime_lifecycle` frames. pub relay_url: String, + /// Unpredictable identity for this exact harness generation + /// (`BUZZ_MANAGED_AGENT_START_NONCE`). Rides in `session_config_captured` + /// alongside `relay_url` so the desktop can bind the frame to the exact + /// tracked runtime that emitted it, the same generation check the lifecycle + /// frames already carry. Empty when the harness was launched outside a + /// managed-agent runtime (no nonce in env) — such frames the desktop drops. + pub start_nonce: String, } impl AgentPool { @@ -1008,9 +1015,11 @@ async fn create_session_and_apply_model( "modes": resp.raw.get("modes").cloned().unwrap_or(serde_json::Value::Null), "models": resp.raw.get("models").cloned().unwrap_or(serde_json::Value::Null), "modelOverridden": agent.model_overridden && switch_succeeded, - // Pair identity for the desktop session-config cache, which is - // keyed by (agent, relay) like the lifecycle frames. + // Pair identity for the desktop session-config cache, which binds + // the frame to the exact tracked runtime by (agent, relay, nonce) + // like the lifecycle frames. A frame with no nonce is dropped. "relayUrl": ctx.relay_url, + "startNonce": ctx.start_nonce, }), ); @@ -6542,6 +6551,7 @@ mod tests { memory_enabled: false, harness_name: "goose".to_string(), relay_url: "ws://127.0.0.1:3000".to_string(), + start_nonce: "test-nonce".to_string(), } } diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 7e61fb688..5d1130581 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -13,7 +13,6 @@ use tokio::sync::Mutex as AsyncMutex; use crate::huddle::HuddleState; pub(crate) use crate::identity_storage::{IdentityStorage, RecoveryState, ResolvedIdentity}; -use crate::managed_agents::config_bridge::SessionConfigCache; use crate::managed_agents::scope::WorkspaceAgentScope; use crate::managed_agents::{ManagedAgentPairRuntime, ManagedAgentRuntimeKey}; @@ -108,10 +107,6 @@ pub struct AppState { /// Ordering: written once in `setup()` with `Ordering::Release`; read in /// `get_identity` with `Ordering::Acquire`. pub reset_failed: AtomicBool, - /// Cached ACP session config from running agents, keyed by canonical - /// `(agent pubkey, relay URL)` runtime identity. - /// Populated when the harness emits `session_config_captured` observer events. - pub session_config_cache: Mutex>, /// IOKit power assertion state — prevents idle sleep while agents run. pub prevent_sleep: Arc>, /// In-process mesh-llm node started by Buzz Desktop. @@ -222,7 +217,6 @@ pub fn build_app_state() -> AppState { managed_agents_store_lock: Mutex::new(()), channel_templates_store_lock: Mutex::new(()), managed_agent_processes: Mutex::new(HashMap::new()), - session_config_cache: Mutex::new(HashMap::new()), huddle_state: Mutex::new(HuddleState::default()), huddle_audio: Default::default(), app_handle: Mutex::new(None), @@ -253,28 +247,6 @@ impl AppState { self.huddle_state.lock().map_err(|e| e.to_string()) } - pub fn get_session_cache(&self, key: &ManagedAgentRuntimeKey) -> Option { - self.session_config_cache.lock().ok()?.get(key).cloned() - } - - pub fn put_session_cache(&self, key: ManagedAgentRuntimeKey, cache: SessionConfigCache) { - if let Ok(mut map) = self.session_config_cache.lock() { - map.insert(key, cache); - } - } - - pub fn clear_agent_session_cache(&self, key: &ManagedAgentRuntimeKey) { - if let Ok(mut map) = self.session_config_cache.lock() { - map.remove(key); - } - } - - pub fn clear_agent_session_caches(&self, pubkey: &str) { - if let Ok(mut map) = self.session_config_cache.lock() { - map.retain(|key, _| key.pubkey != pubkey); - } - } - /// Return the active identity keys if they are in a signable state. /// /// Returns `Err` when the identity is in a lost state (`identity_lost` diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 2dc0ba0d6..083ffdc18 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -251,42 +251,72 @@ pub async fn get_agent_config_surface( app: AppHandle, state: State<'_, AppState>, ) -> Result { - let record = { + get_agent_config_surface_for(pubkey, &app, &state) +} + +/// Runtime-generic core of [`get_agent_config_surface`]. +/// +/// Split out so the capability gate — read the session cache only through a +/// still-live, same-scope runtime — can be exercised under +/// `tauri::test::MockRuntime`. The command is a thin `AppHandle` wrapper. +pub(crate) fn get_agent_config_surface_for( + pubkey: String, + app: &tauri::AppHandle, + state: &AppState, +) -> Result { + // Capture the active scope up front so the session-config gate below + // compares against a single consistent scope, even if a workspace switch + // races this read. + let current_scope_id = state.capture_active_scope().map(|scope| scope.scope_id); + + let (record, session_cache) = { let _store_guard = state .managed_agents_store_lock .lock() .map_err(|e| e.to_string())?; - let mut records = load_managed_agents(&app)?; + let mut records = load_managed_agents(app)?; let mut runtimes = state .managed_agent_processes .lock() .map_err(|e| e.to_string())?; - let (sync_changed, exited_pubkeys) = - sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(&app)); + // Exited runtimes are pruned here; their `session_config` dies with the + // removed entry, so no separate cache clear is needed. + let (sync_changed, _exited) = + sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(app)); if sync_changed { - save_managed_agents(&app, &records)?; + save_managed_agents(app, &records)?; } - for pubkey in &exited_pubkeys { - state.clear_agent_session_caches(pubkey); - } - records + let record = records .into_iter() .find(|r| r.pubkey == pubkey) - .ok_or_else(|| format!("agent {pubkey} not found"))? + .ok_or_else(|| format!("agent {pubkey} not found"))?; + + // Consumption is capability-gated: the cache is read only through the + // same still-current runtime that could have written it. No live + // matching runtime (untracked, exited, or a different scope) ⇒ the + // pre-spawn surface, exactly as if no frame had arrived. A delayed + // frame from a drained workspace cannot surface here because the only + // place its cache could live — that runtime entry — is gone. + let runtime_key = ManagedAgentRuntimeKey::new( + pubkey.clone(), + &crate::relay::effective_agent_relay_url( + &record.relay_url, + &crate::relay::relay_ws_url_with_override(state), + ), + )?; + let session_cache = runtimes.get_mut(&runtime_key).and_then(|runtime| { + let live = matches!(runtime.child.try_wait(), Ok(None)); + (runtime.scope_id == current_scope_id && live) + .then(|| runtime.session_config.clone()) + .flatten() + }); + (record, session_cache) }; - let personas = load_personas(&app).unwrap_or_default(); + let personas = load_personas(app).unwrap_or_default(); let effective_cmd = crate::managed_agents::record_agent_command(&record, &personas); let runtime_meta = known_acp_runtime(&effective_cmd); - let runtime_key = ManagedAgentRuntimeKey::new( - pubkey.clone(), - &crate::relay::effective_agent_relay_url( - &record.relay_url, - &crate::relay::relay_ws_url_with_override(&state), - ), - )?; - let session_cache = state.get_session_cache(&runtime_key); - let global = crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); Ok(resolve_config_surface( record, @@ -297,46 +327,49 @@ pub async fn get_agent_config_surface( )) } -/// Store a `session_config_captured` observer event payload into the session cache. +/// Store a `session_config_captured` observer event payload onto the emitting +/// runtime's session cache. /// -/// Called by the TypeScript observer relay when it decrypts a `session_config_captured` -/// event from a running agent. The payload contains raw ACP session/new fields. +/// Called by the TypeScript observer relay when it decrypts a +/// `session_config_captured` event from a running agent. The payload contains +/// raw ACP session/new fields plus the pair identity (`relayUrl`, `startNonce`) +/// the harness attaches. +/// +/// This command is in the runtime-capability sub-class of `arrival_routed` +/// (§3.3a): its authority is the emitting PROCESS, not an issuing workspace, so +/// it cannot be `owned`; its purpose is a runtime-cache mutation, so it cannot +/// be `read_only`. The frame is admitted only when its exact +/// `{pubkey, relay_url, start_nonce}` resolves to a tracked runtime whose child +/// is still live and whose `scope_id` equals the current active scope. Any miss +/// — missing nonce, untracked pair, generation mismatch, exited process, scope +/// mismatch — discards the frame, mutating nothing. A managed-agent store read +/// never establishes ownership: the then-active store is precisely the authority +/// that rotates underneath a delayed frame. #[tauri::command] pub fn put_agent_session_config( pubkey: String, payload: serde_json::Value, - app: AppHandle, state: State<'_, AppState>, ) { - let record_relay_url = { - let _guard = match state.managed_agents_store_lock.lock() { - Ok(g) => g, - Err(_) => return, - }; - match load_managed_agents(&app) { - Ok(records) => match records.into_iter().find(|r| r.pubkey == pubkey) { - Some(record) => record.relay_url, - None => return, - }, - _ => return, - } + // No nonce ⇒ old harness. Drop, never fall back to a relay-only key: a + // fallback would recreate the ownerless write this contract removes. + let Some(start_nonce) = payload + .get("startNonce") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + else { + return; + }; + let Some(relay_url) = payload.get("relayUrl").and_then(|v| v.as_str()) else { + return; + }; + let Ok(runtime_key) = ManagedAgentRuntimeKey::new(pubkey, relay_url) else { + return; }; - // Pair identity: prefer the relay URL the harness attached to the payload - // (same pattern as lifecycle frames). Older harnesses don't attach one; - // fall back to the record's effective relay — with no attached URL the - // frame can only have arrived over the active workspace relay, which is - // exactly what effective_agent_relay_url resolves to absent a pin. - let relay_url = payload - .get("relayUrl") - .and_then(|v| v.as_str()) - .map(str::to_string) - .unwrap_or_else(|| { - crate::relay::effective_agent_relay_url( - &record_relay_url, - &crate::relay::relay_ws_url_with_override(&state), - ) - }); + // Capture the active scope BEFORE taking the runtime lock so a concurrent + // workspace switch cannot slip a stale scope past the check. + let current_scope_id = state.capture_active_scope().map(|scope| scope.scope_id); let config_options = parse_config_options(payload.get("configOptions")); let available_modes = parse_modes(&config_options, payload.get("modes")); @@ -356,10 +389,25 @@ pub fn put_agent_session_config( captured_at: crate::util::now_iso(), }; - let Ok(runtime_key) = ManagedAgentRuntimeKey::new(pubkey, &relay_url) else { + // Validate + mutate under the runtime-map lock so the resolved runtime + // cannot be drained between the check and the write. + let Ok(mut runtimes) = state.managed_agent_processes.lock() else { return; }; - state.put_session_cache(runtime_key, cache); + let Some(runtime) = runtimes.get_mut(&runtime_key) else { + return; + }; + if runtime.start_nonce != start_nonce { + return; + } + if runtime.scope_id != current_scope_id { + return; + } + match runtime.child.try_wait() { + Ok(None) => {} + _ => return, + } + runtime.session_config = Some(cache); } fn parse_config_options(raw: Option<&serde_json::Value>) -> Vec { @@ -506,3 +554,7 @@ fn parse_models(raw: Option<&serde_json::Value>) -> (Vec, Option< #[cfg(test)] #[path = "agent_config_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "agent_config_capability_tests.rs"] +mod capability_tests; diff --git a/desktop/src-tauri/src/commands/agent_config_capability_tests.rs b/desktop/src-tauri/src/commands/agent_config_capability_tests.rs new file mode 100644 index 000000000..8543b41b3 --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_config_capability_tests.rs @@ -0,0 +1,508 @@ +//! Command-level tests for the observer-frame runtime-capability seam (P23-C1). +//! +//! Included via `#[path = "agent_config_capability_tests.rs"] mod ...;` at the +//! bottom of `agent_config.rs`, so `use super::*` reaches `put_agent_session_config` +//! and `get_agent_config_surface_for`. +//! +//! These drive the real command entry points against a `tauri::test::MockRuntime` +//! app with a committed workspace scope and a seeded runtime, proving the +//! capability contract end to end: a `session_config_captured` frame mutates a +//! runtime's `session_config` ONLY when its `{pubkey, relay_url, start_nonce}` +//! resolves to a still-live runtime whose `scope_id` equals the current active +//! scope, and `get_agent_config_surface` reads that cache back only through the +//! same still-current runtime. Because the cache lives on the runtime entry, a +//! frame that misses the gate mutates nothing, and runtime removal destroys the +//! embedded cache with no separate clear step. + +use super::*; +use crate::managed_agents::{ + scope::{current_scope_generation, WorkspaceAgentScope, SCOPE_GENERATION_TEST_LOCK}, + ManagedAgentPairRuntime, ManagedAgentProcess, ManagedAgentRuntimeKey, +}; + +const TEST_RELAY: &str = "ws://localhost:3000"; + +/// Long-lived child so `try_wait()` reports the process as still running for +/// the duration of a test (avoids sync eviction / the dead-process reject). +fn spawn_live_child() -> std::process::Child { + #[cfg(not(windows))] + { + std::process::Command::new("sh") + .args(["-c", "while true; do sleep 1; done"]) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn long-lived test child (sh loop)") + } + #[cfg(windows)] + { + std::process::Command::new("ping") + .args(["-n", "100000", "127.0.0.1"]) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn long-lived test child (ping)") + } +} + +/// An immediately-exited child so `try_wait()` reports `Some(status)` — the +/// dead-process reject arm of the capability gate. +fn spawn_dead_child() -> std::process::Child { + #[cfg(not(windows))] + let program = "/usr/bin/true"; + #[cfg(windows)] + let program = "cmd"; + let mut cmd = std::process::Command::new(program); + #[cfg(windows)] + cmd.args(["/C", "exit", "0"]); + let mut child = cmd + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn short-lived test child"); + child.wait().expect("reap short-lived test child"); + child +} + +fn test_record(pubkey: &str) -> ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "{pubkey}", + "name": "capability-test", + "relay_url": "{TEST_RELAY}", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 300, + "system_prompt": "", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + }}"#, + )) + .unwrap() +} + +fn make_process(record: &ManagedAgentRecord, start_nonce: &str) -> ManagedAgentProcess { + ManagedAgentProcess { + child: spawn_live_child(), + log_path: std::path::PathBuf::new(), + spawn_config: crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot( + record, + &[], + &[], + TEST_RELAY, + &Default::default(), + ), + setup_mode: false, + adapter_availability: None, + start_nonce: start_nonce.to_string(), + #[cfg(windows)] + job: None, + } +} + +/// A `session_config_captured` observer payload carrying `startNonce` + +/// `relayUrl` (the pair identity the harness attaches) plus a model so the +/// surfaced config is assertable. `model_overridden` makes the ACP model the +/// live winner over the record's structured model, so its presence is a direct +/// signal the cache was consumed. +fn frame(start_nonce: &str, model_id: &str) -> serde_json::Value { + serde_json::json!({ + "startNonce": start_nonce, + "relayUrl": TEST_RELAY, + "modelOverridden": true, + "models": { + "currentModelId": model_id, + "availableModels": [{ "modelId": model_id, "name": model_id }], + }, + }) +} + +struct Harness { + _tmp: tempfile::TempDir, + app: tauri::App, + scope_id: String, +} + +impl Harness { + /// Build a mock app, write the record to the tmp store, and commit an + /// active scope pointing at it. Holds no runtime yet. + fn new(record: &ManagedAgentRecord) -> Self { + let tmp = tempfile::tempdir().unwrap(); + crate::managed_agents::save_managed_agents_at(tmp.path(), std::slice::from_ref(record)) + .unwrap(); + std::fs::write(tmp.path().join("personas.json"), b"[]").unwrap(); + std::fs::write(tmp.path().join("global-agent-config.json"), b"{}").unwrap(); + + let app = tauri::test::mock_builder() + .manage(crate::app_state::build_app_state()) + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("failed to build mock app"); + + // The surface's runtime-key resolution goes through the active + // workspace relay (`relay_ws_url_with_override`), matching production + // where the harness attaches that same relay to the frame. Pin it to + // TEST_RELAY so the read key equals the seeded runtime key. + { + use tauri::Manager; + *app.state::() + .relay_url_override + .lock() + .unwrap() = Some(TEST_RELAY.to_string()); + } + + let scope_id = "scope-a".to_string(); + Self::commit_scope(&app, &scope_id, tmp.path()); + Self { + _tmp: tmp, + app, + scope_id, + } + } + + fn commit_scope( + app: &tauri::App, + scope_id: &str, + definitions_dir: &std::path::Path, + ) { + use tauri::Manager; + let scope = WorkspaceAgentScope { + scope_id: scope_id.to_string(), + relay_url: TEST_RELAY.to_string(), + owner_pubkey: "aa".repeat(32), + definitions_dir: definitions_dir.to_path_buf(), + generation: current_scope_generation(), + }; + app.state::() + .commit_active_scope(scope); + } + + /// Re-commit the active scope under a new `scope_id` pointing at the same + /// store — the shape of an A→B same-relay/different-owner switch as far as + /// the capability gate sees it (the gate compares `scope_id`). + fn switch_scope_to(&self, scope_id: &str) { + Self::commit_scope(&self.app, scope_id, self._tmp.path()); + } + + fn state(&self) -> tauri::State<'_, crate::app_state::AppState> { + use tauri::Manager; + self.app.state::() + } + + /// Insert a live runtime for `record` keyed on the current relay, stamped + /// with `scope_id` and `start_nonce`. + fn seed_runtime(&self, record: &ManagedAgentRecord, scope_id: &str, start_nonce: &str) { + let key = ManagedAgentRuntimeKey::new(&record.pubkey, TEST_RELAY).unwrap(); + let process = make_process(record, start_nonce); + let state = self.state(); + let mut runtimes = state.managed_agent_processes.lock().unwrap(); + runtimes.insert( + key, + ManagedAgentPairRuntime::starting(process, Some(scope_id.to_string())), + ); + } + + fn surface(&self, pubkey: &str) -> RuntimeConfigSurface { + get_agent_config_surface_for(pubkey.to_string(), self.app.handle(), &self.state()) + .expect("surface must resolve") + } + + /// White-box: does the tracked runtime hold a cached session config? + fn runtime_has_cache(&self, pubkey: &str) -> bool { + let key = ManagedAgentRuntimeKey::new(pubkey, TEST_RELAY).unwrap(); + self.state() + .managed_agent_processes + .lock() + .unwrap() + .get(&key) + .map(|rt| rt.session_config.is_some()) + .unwrap_or(false) + } + + /// Kill and reap every seeded child so no OS process leaks past the test. + fn reap(&self) { + let state = self.state(); + let mut runtimes = state.managed_agent_processes.lock().unwrap(); + for (_, rt) in runtimes.iter_mut() { + let _ = rt.child.kill(); + let _ = rt.child.wait(); + } + runtimes.clear(); + } +} + +/// Guard: serialise against every other test that reads/writes the global scope +/// generation, and against the shared process map, since these tests commit +/// scopes and seed runtimes on the same static-free but shared `AppState` shape. +fn gen_guard() -> std::sync::MutexGuard<'static, ()> { + SCOPE_GENERATION_TEST_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()) +} + +/// Positive control (no switch): a frame whose `{pubkey, relay, start_nonce}` +/// matches a live same-scope runtime is cached, and the surface serves it. +#[test] +fn matching_frame_on_live_runtime_is_cached_and_served() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + h.seed_runtime(&record, &h.scope_id, "nonce-1"); + + assert!(h.surface(&pubkey).is_pre_spawn, "no frame yet ⇒ pre-spawn"); + + put_agent_session_config(pubkey.clone(), frame("nonce-1", "model-x"), h.state()); + + assert!( + h.runtime_has_cache(&pubkey), + "matching frame must be cached" + ); + let surface = h.surface(&pubkey); + assert!(!surface.is_pre_spawn, "cached frame ⇒ post-spawn surface"); + assert_eq!( + surface.normalized.model.and_then(|m| m.value).as_deref(), + Some("model-x"), + "surface must serve the cached model" + ); + h.reap(); +} + +/// A→B same-relay/different-owner switch: A's valid frame arrives after the +/// workspace has switched to B (same pubkey/relay, different scope). The frame +/// is rejected at the scope check — no mutation — and B's surface shows no A +/// value until B's own runtime, under B's nonce, emits its own frame. +#[test] +fn frame_after_scope_switch_is_rejected_until_new_runtime_emits() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + + // A is live under scope-a; the workspace then switches to B (scope-b), and + // A's runtime is drained (removed) as part of the transition. + h.seed_runtime(&record, &h.scope_id, "nonce-a"); + h.reap(); // drain A: its runtime entry (and any cache) is destroyed + h.switch_scope_to("scope-b"); + + // A's delayed frame arrives: no tracked runtime at all ⇒ rejected, nothing + // cached, and B's surface is pre-spawn with no A value. + put_agent_session_config(pubkey.clone(), frame("nonce-a", "model-a"), h.state()); + assert!( + !h.runtime_has_cache(&pubkey), + "no runtime exists ⇒ delayed A frame cannot cache" + ); + let surface_b = h.surface(&pubkey); + assert!( + surface_b.is_pre_spawn, + "B surface must be pre-spawn, no A value" + ); + + // B spawns its own runtime under scope-b with a fresh nonce. A stale A frame + // (wrong nonce) still rejects; B's own frame is served. + h.seed_runtime(&record, "scope-b", "nonce-b"); + put_agent_session_config(pubkey.clone(), frame("nonce-a", "model-a"), h.state()); + assert!( + !h.runtime_has_cache(&pubkey), + "A's nonce must not cache onto B's runtime" + ); + assert!( + h.surface(&pubkey).is_pre_spawn, + "still pre-spawn after stale frame" + ); + + put_agent_session_config(pubkey.clone(), frame("nonce-b", "model-b"), h.state()); + assert!(h.runtime_has_cache(&pubkey), "B's own frame must cache"); + assert_eq!( + h.surface(&pubkey) + .normalized + .model + .and_then(|m| m.value) + .as_deref(), + Some("model-b"), + "surface must reflect B's payload" + ); + h.reap(); +} + +/// A frame whose scope no longer matches the current active scope — the runtime +/// survived but the workspace rotated — is rejected with zero mutation. +#[test] +fn frame_with_stale_scope_is_rejected() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + // Runtime stamped scope-a, but the active scope is now scope-b. + h.seed_runtime(&record, "scope-a", "nonce-1"); + h.switch_scope_to("scope-b"); + + put_agent_session_config(pubkey.clone(), frame("nonce-1", "model-x"), h.state()); + assert!( + !h.runtime_has_cache(&pubkey), + "scope mismatch must reject the frame" + ); + // The read gate also refuses: runtime scope-a != active scope-b. + assert!(h.surface(&pubkey).is_pre_spawn); + h.reap(); +} + +/// N→N+1 same-scope respawn: a frame from the old process generation (stale +/// `start_nonce`) arriving after respawn is rejected; the new runtime's cache +/// stays empty until the new nonce's frame lands. +#[test] +fn stale_nonce_after_respawn_is_rejected() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + // Respawn: the live runtime now carries nonce-2 (the old process was nonce-1). + h.seed_runtime(&record, &h.scope_id, "nonce-2"); + + put_agent_session_config(pubkey.clone(), frame("nonce-1", "model-old"), h.state()); + assert!( + !h.runtime_has_cache(&pubkey), + "stale-generation frame must be rejected" + ); + assert!(h.surface(&pubkey).is_pre_spawn); + + put_agent_session_config(pubkey.clone(), frame("nonce-2", "model-new"), h.state()); + assert!( + h.runtime_has_cache(&pubkey), + "current-nonce frame must cache" + ); + h.reap(); +} + +/// Control: a frame whose pair is untracked (no runtime entry) mutates nothing. +#[test] +fn untracked_pair_frame_mutates_nothing() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + // No runtime seeded. + put_agent_session_config(pubkey.clone(), frame("nonce-1", "model-x"), h.state()); + assert!(!h.runtime_has_cache(&pubkey)); + assert!(h.surface(&pubkey).is_pre_spawn); +} + +/// Control: a frame for a runtime whose process has exited (`try_wait` Some) is +/// rejected — a dead generation cannot publish config. +#[test] +fn dead_process_frame_is_rejected() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + // Seed a runtime whose child has already exited. + { + let key = ManagedAgentRuntimeKey::new(&pubkey, TEST_RELAY).unwrap(); + let mut process = make_process(&record, "nonce-1"); + let _ = process.child.kill(); + let _ = process.child.wait(); + process.child = spawn_dead_child(); + let state = h.state(); + let mut runtimes = state.managed_agent_processes.lock().unwrap(); + runtimes.insert( + key, + ManagedAgentPairRuntime::starting(process, Some(h.scope_id.clone())), + ); + } + put_agent_session_config(pubkey.clone(), frame("nonce-1", "model-x"), h.state()); + assert!( + !h.runtime_has_cache(&pubkey), + "exited-process frame must be rejected" + ); + h.reap(); +} + +/// Control: a missing-nonce frame (old harness) is dropped — no relay-only +/// fallback that could recreate an ownerless write. +#[test] +fn missing_nonce_frame_is_dropped() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + h.seed_runtime(&record, &h.scope_id, "nonce-1"); + + let payload = serde_json::json!({ + "relayUrl": TEST_RELAY, + "models": { "currentModelId": "model-x", "availableModels": [] }, + }); + put_agent_session_config(pubkey.clone(), payload, h.state()); + assert!( + !h.runtime_has_cache(&pubkey), + "a frame with no startNonce must be dropped" + ); + h.reap(); +} + +/// Atomic cache destruction: removing the runtime entry destroys the embedded +/// cache — there is no separate clear step, and no map for a stale cache to +/// linger in. After removal the surface is pre-spawn again. +#[test] +fn runtime_removal_destroys_embedded_cache() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + h.seed_runtime(&record, &h.scope_id, "nonce-1"); + put_agent_session_config(pubkey.clone(), frame("nonce-1", "model-x"), h.state()); + assert!(h.runtime_has_cache(&pubkey), "precondition: frame cached"); + + // Remove the runtime entry (drain/removal/exit-prune all funnel here). + { + let key = ManagedAgentRuntimeKey::new(&pubkey, TEST_RELAY).unwrap(); + let state = h.state(); + let mut runtimes = state.managed_agent_processes.lock().unwrap(); + if let Some(mut rt) = runtimes.remove(&key) { + let _ = rt.child.kill(); + let _ = rt.child.wait(); + } + } + assert!( + !h.runtime_has_cache(&pubkey), + "removing the runtime destroys the embedded cache" + ); + assert!( + h.surface(&pubkey).is_pre_spawn, + "no runtime ⇒ pre-spawn surface" + ); +} + +/// §3.3a sibling binding: `put_managed_agent_runtime_lifecycle` shares this +/// runtime-capability sub-class. Its base checks already reject a stale-nonce +/// frame; this binds the declaration to the suite so the consistency check is +/// covered where the capability contract is tested. +#[test] +fn lifecycle_sibling_rejects_stale_nonce() { + let _guard = gen_guard(); + let pubkey = "aa".repeat(32); + let record = test_record(&pubkey); + let h = Harness::new(&record); + h.seed_runtime(&record, &h.scope_id, "nonce-2"); + + let payload = crate::managed_agents::ManagedAgentRuntimeLifecycleObserverPayload { + pubkey: pubkey.clone(), + relay_url: TEST_RELAY.to_string(), + start_nonce: "nonce-1".to_string(), + lifecycle: crate::managed_agents::ManagedAgentRuntimeLifecycle::Ready, + error: None, + }; + let result = crate::managed_agents::put_managed_agent_runtime_lifecycle_for( + pubkey.clone(), + payload, + h.app.handle(), + ); + assert!( + result.is_err(), + "stale-generation lifecycle frame must be rejected" + ); + h.reap(); +} diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 9ff48c42d..68fab64db 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -48,14 +48,11 @@ pub async fn get_agent_models( .managed_agent_processes .lock() .map_err(|e| e.to_string())?; - let (sync_changed, exited_pubkeys) = + let (sync_changed, _exited) = sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(&app)); if sync_changed { save_managed_agents(&app, &records)?; } - for pubkey in &exited_pubkeys { - state.clear_agent_session_caches(pubkey); - } let record = records .iter() @@ -747,11 +744,8 @@ pub async fn update_managed_agent( .managed_agent_processes .lock() .map_err(|e| e.to_string())?; - let (_, exited_pubkeys) = + let _ = sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(&app)); - for pubkey in &exited_pubkeys { - state.clear_agent_session_caches(pubkey); - } let record = find_managed_agent_mut(&mut records, &input.pubkey)?; let previous_record = record.clone(); diff --git a/desktop/src-tauri/src/commands/agent_settings.rs b/desktop/src-tauri/src/commands/agent_settings.rs index 2317930c1..8ff321643 100644 --- a/desktop/src-tauri/src/commands/agent_settings.rs +++ b/desktop/src-tauri/src/commands/agent_settings.rs @@ -36,14 +36,11 @@ pub async fn set_managed_agent_start_on_app_launch( .lock() .map_err(|error| error.to_string())?; - let (sync_changed, exited_pubkeys) = + let (sync_changed, _exited) = sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(&app)); if sync_changed { save_managed_agents(&app, &records)?; } - for pubkey in &exited_pubkeys { - state.clear_agent_session_caches(pubkey); - } { let record = find_managed_agent_mut(&mut records, &pubkey)?; @@ -87,14 +84,11 @@ pub async fn set_managed_agent_auto_restart( .lock() .map_err(|error| error.to_string())?; - let (sync_changed, exited_pubkeys) = + let (sync_changed, _exited) = sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(&app)); if sync_changed { save_managed_agents(&app, &records)?; } - for pubkey in &exited_pubkeys { - state.clear_agent_session_caches(pubkey); - } { let record = find_managed_agent_mut(&mut records, &pubkey)?; diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 0cabb67c6..8d17e205c 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -457,14 +457,11 @@ pub async fn list_managed_agents(app: AppHandle) -> Result Result<(), String> { .managed_agent_processes .lock() .map_err(|error| error.to_string())?; - let (sync_changed, exited_pubkeys) = sync_managed_agent_processes( + let (sync_changed, _exited) = sync_managed_agent_processes( &mut agents, &mut runtimes, ¤t_instance_id(&app), @@ -188,9 +188,6 @@ pub async fn delete_persona(id: String, app: AppHandle) -> Result<(), String> { if sync_changed { save_managed_agents(&app, &agents)?; } - for pk in &exited_pubkeys { - state.clear_agent_session_caches(pk); - } // runtimes drops here (process lock released before Phase 2). } @@ -261,7 +258,6 @@ pub async fn delete_persona(id: String, app: AppHandle) -> Result<(), String> { // Side effects — strictly after records leave disk. for pk in &cascade { - state.clear_agent_session_caches(pk); // Remove nsec from keyring after the record is gone. delete_agent_key(pk); super::agents::tombstone_managed_agent_pending(&app, &state, pk); diff --git a/desktop/src-tauri/src/managed_agents/runtime/stop.rs b/desktop/src-tauri/src/managed_agents/runtime/stop.rs index 5a8b455c1..0d9b05ec2 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/stop.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/stop.rs @@ -125,12 +125,9 @@ pub fn stop_managed_agent_workspace_pair( record: &mut ManagedAgentRecord, runtimes: &mut HashMap, ) -> Result<(), String> { - use tauri::Manager; - let state = app.state::(); match super::workspace_pair_key(app, record) { Some(pair_key) if runtimes.contains_key(&pair_key) => { stop_managed_agent_pair(app, record, runtimes, &pair_key)?; - state.clear_agent_session_cache(&pair_key); super::super::remove_agent_pid_file(app, &record.pubkey); let now = now_iso(); record.runtime_pid = None; @@ -139,15 +136,14 @@ pub fn stop_managed_agent_workspace_pair( record.last_error = None; record.last_error_code = None; } - Some(pair_key) => { - // No tracked pair here — a pubkey-wide cache clear would disturb - // live pairs in other communities, so stay pair-scoped. + Some(_pair_key) => { + // No tracked pair here — nothing to stop but the legacy scalar PID. + // The session config lives on the (absent) runtime entry, so there + // is nothing separate to clear. stop_legacy_scalar_pid(app, record)?; - state.clear_agent_session_cache(&pair_key); } None => { stop_legacy_scalar_pid(app, record)?; - state.clear_agent_session_caches(&record.pubkey); } } Ok(()) diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index ed3ae2503..202aabe5e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -106,10 +106,19 @@ pub fn put_managed_agent_runtime_lifecycle( outer_pubkey: String, payload: super::ManagedAgentRuntimeLifecycleObserverPayload, app: AppHandle, +) -> Result { + put_managed_agent_runtime_lifecycle_for(outer_pubkey, payload, &app) +} + +/// Runtime-generic core so the sibling capability check is testable under `MockRuntime` (§3.3a / §7). +pub(crate) fn put_managed_agent_runtime_lifecycle_for( + outer_pubkey: String, + payload: super::ManagedAgentRuntimeLifecycleObserverPayload, + app: &tauri::AppHandle, ) -> Result { let key = observer_lifecycle_key(&outer_pubkey, &payload)?; let state = app.state::(); - let records = load_managed_agents(&app)?; + let records = load_managed_agents(app)?; let record = records .iter() .find(|record| record.pubkey.eq_ignore_ascii_case(&key.pubkey)) @@ -124,18 +133,18 @@ pub fn put_managed_agent_runtime_lifecycle( if runtime.start_nonce != payload.start_nonce { return Err("lifecycle frame does not match the current harness generation".into()); } - if runtime + let exited = runtime .child .try_wait() .map_err(|e| e.to_string())? - .is_some() - { + .is_some(); + if exited { return Err("lifecycle frame arrived after process exit".into()); } runtime.lifecycle = payload.lifecycle; runtime.error = payload.error; - let status = status_for(&app, record, &key, Some(runtime), None); - emit_status(&app, &status); + let status = status_for(app, record, &key, Some(runtime), None); + emit_status(app, &status); Ok(status) } @@ -184,7 +193,6 @@ pub fn list_managed_agent_runtimes( for key in exited_keys { runtimes.remove(&key); super::remove_agent_runtime_receipt(&app, &key); - state.clear_agent_session_cache(&key); if let Some(record) = records .iter_mut() .find(|record| record.pubkey.eq_ignore_ascii_case(&key.pubkey)) @@ -432,7 +440,6 @@ pub fn stop_managed_agent_runtime( terminate_untracked_pair_runtime(&app, &key)?; } super::remove_agent_runtime_receipt(&app, &key); - state.clear_agent_session_cache(&key); record.runtime_pid = None; record.updated_at = crate::util::now_iso(); record.last_stopped_at = Some(record.updated_at.clone()); @@ -821,7 +828,6 @@ pub(crate) fn drain_scope_runtimes( execute_drain_journal(&journal, &mut runtimes, |key| { super::remove_agent_runtime_receipt(app, key); - state.clear_agent_session_cache(key); }) } diff --git a/desktop/src-tauri/src/managed_agents/runtime_types.rs b/desktop/src-tauri/src/managed_agents/runtime_types.rs index 132a004f3..b715d6cc0 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_types.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_types.rs @@ -1,6 +1,7 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest as _, Sha256}; +use super::config_bridge::SessionConfigCache; use super::ManagedAgentProcess; /// Canonical identity of one managed-agent harness on one relay. @@ -52,11 +53,20 @@ pub struct ManagedAgentPairRuntime { pub start_nonce: String, /// Scope ID of the workspace this runtime was spawned into. Used by drain /// filtering and `list_managed_agent_runtimes` to detect cross-scope - /// entries (the seam that option 2 background-runtime pinning would build - /// on). Under active-scope-only policy, all live entries should always - /// match the current scope; this field makes the invariant testable. - #[allow(dead_code)] // Set at spawn; read in tests; seam for future option-2 pinning. + /// entries, and by the runtime-capability commands (`put_agent_session_config` + /// / `get_agent_config_surface`) to require that a session-config frame or + /// read matches the CURRENT active scope — the seam that keeps a delayed + /// frame from a drained workspace from surfacing under a rotated identity. pub scope_id: Option, + /// ACP session config captured from this exact harness generation. Set by + /// `put_agent_session_config` only after the frame validates against this + /// tracked runtime (`{pubkey, relay_url, start_nonce}` + live + current + /// scope); read by `get_agent_config_surface` through the same still-current + /// runtime. Living here — rather than in a process-global map — makes an + /// ownerless cache entry unrepresentable: the cache is destroyed atomically + /// with the runtime entry on drain/removal/exit-pruning, so no separate + /// cache-cleanup step can drift from runtime teardown. + pub session_config: Option, } impl std::ops::Deref for ManagedAgentPairRuntime { @@ -82,6 +92,7 @@ impl ManagedAgentPairRuntime { error: None, start_nonce, scope_id, + session_config: None, } } }