From 59795b1a5d2ea707fb66f632580900e499809cc9 Mon Sep 17 00:00:00 2001 From: anilkishan <10408515+anilkishan@users.noreply.github.com> Date: Wed, 5 Aug 2026 00:42:39 -0700 Subject: [PATCH 1/6] fix(desktop): connect managed agents with the configured relay URL, not the canonical identity URL Managed agents were spawned with BUZZ_RELAY_URL set to the canonical (pubkey, relay_url) identity URL from normalize_relay_url, which folds loopback spellings to 127.0.0.1. Relay tenancy is host-derived and does NOT fold spellings, so an agent configured against ws://localhost:PORT connected to the ws://127.0.0.1:PORT community instead - discovering 0 channels and sitting idle (#2444, #3283, #3505, #4147). Pass the configured relay URL through to the child connection at all three spawn call sites, keeping the canonical key URL for runtime identity and for the spawn-config snapshot (the restart-drift check recomputes with key.relay_url, so the stamp must agree or agents restart-loop when the spellings differ). Co-authored-by: anilkishan <10408515+anilkishan@users.noreply.github.com> Signed-off-by: anilkishan <10408515+anilkishan@users.noreply.github.com> --- .../src-tauri/src/managed_agents/restore.rs | 2 +- .../src-tauri/src/managed_agents/runtime.rs | 19 ++++++++++++++----- .../src/managed_agents/runtime/tests.rs | 8 ++++++++ .../src/managed_agents/runtime_commands.rs | 2 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 25dadbeec60..02c7223458f 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -335,7 +335,7 @@ pub async fn restore_managed_agents_on_launch( spawn_agent_child( app, record, - &key.relay_url, + &relay_url, true, owner_hex_ref, ) diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index b1c342e9955..c02b54cc6ae 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -395,6 +395,10 @@ pub(crate) fn configure_runtime_cli( } } +fn connection_relay_url(configured_relay_url: &str) -> String { + configured_relay_url.trim().to_string() +} + /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -496,9 +500,10 @@ pub fn spawn_agent_child( .map(|p| p.display().to_string()) .unwrap_or_else(|| effective_command.clone()); - // The caller supplies the explicit canonical pair relay. This is the only - // relay this child may connect to, regardless of the record/workspace default. - let effective_relay_url = runtime_key.relay_url.clone(); + // The canonical relay is only the pair identity. Preserve the configured + // authority for the network connection because relay communities are + // host-derived (`localhost` and `127.0.0.1` can be distinct tenants). + let effective_relay_url = connection_relay_url(relay_url); // Augment PATH for DMG launches so child processes can find: // - bundled CLI via ~/.local/bin symlink @@ -852,7 +857,11 @@ pub fn spawn_agent_child( super::spawn_snapshot::SpawnConfigInputs { record, descriptor: &descriptor, - relay_url: &effective_relay_url, + // Snapshot the canonical key relay, not the connection URL: the + // restart-drift check recomputes the prospective snapshot with + // `key.relay_url`, and the two must agree even when the configured + // spelling differs from the canonical one. + relay_url: &runtime_key.relay_url, team_instructions: team_instructions.as_deref(), system_prompt: effective_prompt.as_deref(), model: effective_model.as_deref(), @@ -964,7 +973,7 @@ pub fn start_managed_agent_process( // Scalar PIDs are migration-only and never establish pair liveness. record.runtime_pid = None; - let mut process = spawn_agent_child(app, record, &key.relay_url, false, owner_hex)?; + let mut process = spawn_agent_child(app, record, &relay_url, false, owner_hex)?; let now = now_iso(); let receipt = super::ManagedAgentRuntimeReceipt { key: key.clone(), diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 762b0fe2a61..18e14c3ca71 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -1,5 +1,13 @@ use crate::managed_agents::known_acp_runtime; +#[test] +fn agent_connection_preserves_loopback_authority() { + assert_eq!( + super::connection_relay_url(" ws://localhost:3200/ "), + "ws://localhost:3200/" + ); +} + // ── desktop binary name tests ─────────────────────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index c0e55184b19..971f8a31815 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -283,7 +283,7 @@ fn start_pair( .lock() .ok() .map(|keys| keys.public_key().to_hex()); - let mut process = spawn_agent_child(&app, record, &key.relay_url, lazy, owner.as_deref())?; + let mut process = spawn_agent_child(&app, record, &relay_url, lazy, owner.as_deref())?; let now = crate::util::now_iso(); let receipt = ManagedAgentRuntimeReceipt { key: key.clone(), From 7c66c8e62ce51f002cbfa271ffdfd529ff3b8aa5 Mon Sep 17 00:00:00 2001 From: anilkishan <10408515+anilkishan@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:55:03 -0700 Subject: [PATCH 2/6] fix(desktop): probe and post-probe start use the requested relay URL Review follow-up on the same tenancy-vs-identity split: the relay-access probe built its HTTP base from the canonical key spelling (probing a different or unmapped community than the one being configured), and the post-probe start passed the canonical spelling as the child's connection URL. Both now use the requested URL; start_pair re-derives the same canonical key, so identity is unchanged. Co-authored-by: anilkishan <10408515+anilkishan@users.noreply.github.com> Signed-off-by: anilkishan <10408515+anilkishan@users.noreply.github.com> --- .../src-tauri/src/managed_agents/runtime_commands.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index 971f8a31815..59f4fb36ba8 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -403,7 +403,10 @@ async fn probe_agent_relay_access( let key = ManagedAgentRuntimeKey::new(record.pubkey.clone(), &requested_relay_url)?; let keys = nostr::Keys::parse(record.private_key_nsec.trim()) .map_err(|error| format!("invalid managed-agent key: {error}"))?; - let api_base = crate::relay::relay_http_base_url(&key.relay_url); + // Probe the community the user actually configured: relay tenancy is + // host-derived, so the canonical key spelling can resolve to a different + // (or unmapped) community than the requested URL. + let api_base = crate::relay::relay_http_base_url(&requested_relay_url); tokio::time::timeout( std::time::Duration::from_secs(10), crate::relay::query_relay_at_with_keys( @@ -504,7 +507,10 @@ pub async fn reconcile_managed_agent_runtimes( Ok((record, key, requested)) => { match start_pair( record.pubkey.clone(), - key.relay_url.clone(), + // Start with the requested spelling so the spawned + // child connects to the community that was probed. + // start_pair re-derives the same canonical key. + requested.clone(), true, Some(&record.updated_at), app.clone(), From 8f1ffc0c65077c5a956105c732726b7b1a0ed153 Mon Sep 17 00:00:00 2001 From: anilkishan <10408515+anilkishan@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:56:27 -0700 Subject: [PATCH 3/6] fix(desktop): persist the connection relay URL and use it on every restart path Config-change and post-install restarts rebuilt their dial targets from the canonical runtime key, so an initially healthy localhost agent restarted against 127.0.0.1 - a different (empty, fail-closed) community on a host-scoped relay - and went idle again. The spawn-path fix earlier in this branch corrected first starts; this closes the restart paths. - Stamp the exact dial URL (the trimmed value written to the child's BUZZ_RELAY_URL) onto ManagedAgentProcess at spawn; every receipt write persists it as `connectRelayUrl`. Old receipts remain valid when the field is absent; every live runtime carries the stamped URL. - Validate that a stored connection URL canonicalizes back to the receipt's own pair key; a foreign or unparseable URL fails validation. - Add managed_agent_restart_targets(): collects each live pair's stamped connection URL before the stop drops the pair from the runtimes map; both backend restart orchestrations (set_global_agent_config, install_acp_runtime) now dial those targets instead of key.relay_url. - status_for echoes the live pair's connection spelling through requestedRelayUrl, so status rows correlate across spellings. Interactive community actions already submit the configured community URL on current main. - Launch restore carries the URL inside the process struct from the Phase-B spawn to the Phase-C receipt write - stamped once, never recomputed between phases. - ManagedAgentProcess Debug masks the stored URL (relay URLs may carry query tokens), pinned by the owning-process Debug sentinel test. The canonical URL remains the runtime key and spawn-snapshot input, unchanged. Co-authored-by: Ahmet Karapinar Co-authored-by: Michael Kennedy Co-authored-by: Lewis Wang Co-authored-by: anilkishan <10408515+anilkishan@users.noreply.github.com> Signed-off-by: anilkishan <10408515+anilkishan@users.noreply.github.com> --- .../src-tauri/src/commands/agent_discovery.rs | 16 ++- .../src/commands/global_agent_config.rs | 16 ++- .../src/managed_agents/process_lifecycle.rs | 2 + .../src-tauri/src/managed_agents/restore.rs | 5 + .../src-tauri/src/managed_agents/runtime.rs | 10 +- .../src/managed_agents/runtime/process.rs | 9 ++ .../src/managed_agents/runtime/stop.rs | 17 +++ .../src/managed_agents/runtime/tests.rs | 124 ++++++++++++++++++ .../src/managed_agents/runtime_commands.rs | 8 +- .../src/managed_agents/runtime_types.rs | 14 +- .../src/managed_agents/spawn_snapshot/diff.rs | 2 +- .../spawn_snapshot/diff/tests.rs | 3 + desktop/src-tauri/src/managed_agents/types.rs | 29 +++- desktop/src/shared/api/types.ts | 4 +- 14 files changed, 238 insertions(+), 21 deletions(-) diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 9609db5f2df..17c3d492aa2 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -562,9 +562,12 @@ async fn restart_single_agent_after_install( if record.backend != BackendKind::Local { return Err(format!("agent {pubkey_owned} is no longer a local agent")); } - let runtime_keys = - crate::managed_agents::managed_agent_runtime_keys(&runtimes, &pubkey_owned); - if runtime_keys.is_empty() { + // Collect the restart dial targets (each pair's configured connection + // URL, not the canonical key spelling) BEFORE the stop below drops the + // pairs — and their URLs — from the runtimes map. + let restart_targets = + crate::managed_agents::managed_agent_restart_targets(&runtimes, &pubkey_owned); + if restart_targets.is_empty() { return Err(format!( "agent {pubkey_owned} no longer has a live pair runtime after sync" )); @@ -606,12 +609,12 @@ async fn restart_single_agent_after_install( stop_managed_agent_process(&app_for_stop, record_mut, &mut runtimes)?; save_managed_agents(&app_for_stop, &records)?; - Ok(runtime_keys) + Ok(restart_targets) }) .await; - let runtime_keys = match stop_result { - Ok(Ok(runtime_keys)) => runtime_keys, + let relay_urls = match stop_result { + Ok(Ok(restart_targets)) => restart_targets, Ok(Err(e)) => { eprintln!("buzz-desktop: install_acp_runtime: skipping restart of {pubkey}: {e}"); return InstallRestartOutcome::Skipped; @@ -624,7 +627,6 @@ async fn restart_single_agent_after_install( } }; - let relay_urls: Vec<_> = runtime_keys.into_iter().map(|key| key.relay_url).collect(); let state = app.state::(); match super::agents::start_local_agent_pairs_with_preflight(app, &state, pubkey, &relay_urls) .await diff --git a/desktop/src-tauri/src/commands/global_agent_config.rs b/desktop/src-tauri/src/commands/global_agent_config.rs index 91219bafb9c..73dce035439 100644 --- a/desktop/src-tauri/src/commands/global_agent_config.rs +++ b/desktop/src-tauri/src/commands/global_agent_config.rs @@ -292,9 +292,12 @@ async fn restart_local_agent_on_config_change( if record.backend != BackendKind::Local { return Err(format!("agent {pubkey_owned} is no longer a local agent")); } - let runtime_keys = - crate::managed_agents::managed_agent_runtime_keys(&runtimes, &pubkey_owned); - if runtime_keys.is_empty() { + // Collect the restart dial targets (each pair's configured connection + // URL, not the canonical key spelling) BEFORE the stop below drops the + // pairs — and their URLs — from the runtimes map. + let restart_targets = + crate::managed_agents::managed_agent_restart_targets(&runtimes, &pubkey_owned); + if restart_targets.is_empty() { return Err(format!( "agent {pubkey_owned} no longer has a live pair runtime after sync" )); @@ -327,12 +330,12 @@ async fn restart_local_agent_on_config_change( stop_managed_agent_process(&app_for_stop, record_mut, &mut runtimes)?; save_managed_agents(&app_for_stop, &records)?; - Ok(runtime_keys) + Ok(restart_targets) }) .await; - let runtime_keys = match stop_result { - Ok(Ok(runtime_keys)) => runtime_keys, + let relay_urls = match stop_result { + Ok(Ok(restart_targets)) => restart_targets, Ok(Err(e)) => { eprintln!("buzz-desktop: set_global_agent_config: skipping restart of {pubkey}: {e}"); return RestartOutcome::Skipped; @@ -345,7 +348,6 @@ async fn restart_local_agent_on_config_change( } }; - let relay_urls: Vec<_> = runtime_keys.into_iter().map(|key| key.relay_url).collect(); use tauri::Manager; let state = app.state::(); match super::agents::start_local_agent_pairs_with_preflight(app, &state, pubkey, &relay_urls) diff --git a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs index 479d6ec913e..c14160b59c3 100644 --- a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs +++ b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs @@ -133,6 +133,7 @@ pub fn taskkill_tree(pid: u32) -> Result<(), String> { pub fn finish_spawn( child: std::process::Child, log_path: std::path::PathBuf, + connect_relay_url: String, spawn_config: super::spawn_snapshot::SpawnConfigSnapshot, setup_mode: bool, adapter_availability: Option, @@ -149,6 +150,7 @@ pub fn finish_spawn( super::ManagedAgentProcess { child, log_path, + connect_relay_url, spawn_config, setup_mode, adapter_availability, diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 02c7223458f..9c3c38def3a 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -390,6 +390,11 @@ pub async fn restore_managed_agents_on_launch( pid: process.child.id(), desktop_instance_id: super::current_instance_id(app), started_at: now.clone(), + // Phase B stamped the dial URL onto the process at spawn; + // reading it back here (not recomputing from the record) + // keeps the receipt truthful even if the workspace relay + // changed between phases. + connect_relay_url: Some(process.connect_relay_url.clone()), }; if let Err(error) = super::write_agent_runtime_receipt(app, &receipt) { let _ = super::terminate_process(process.child.id()); diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index c02b54cc6ae..f73a7fbf8e9 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -27,7 +27,7 @@ pub(crate) use metadata::{ }; mod stop; -pub(crate) use stop::managed_agent_runtime_keys; +pub(crate) use stop::{managed_agent_restart_targets, managed_agent_runtime_keys}; pub use stop::{stop_managed_agent_process, stop_managed_agent_workspace_pair}; mod sweep; @@ -916,6 +916,7 @@ pub fn spawn_agent_child( return Ok(super::process_lifecycle::finish_spawn( child, log_path, + effective_relay_url, spawn_config, spawned_setup_mode, spawned_adapter_availability, @@ -926,6 +927,9 @@ pub fn spawn_agent_child( Ok(crate::managed_agents::ManagedAgentProcess { child, log_path, + // The trimmed value that actually went into the child's + // BUZZ_RELAY_URL above, not the raw `relay_url` parameter. + connect_relay_url: effective_relay_url, spawn_config, setup_mode: spawned_setup_mode, adapter_availability: spawned_adapter_availability, @@ -980,6 +984,10 @@ pub fn start_managed_agent_process( pid: process.child.id(), desktop_instance_id: current_instance_id(app), started_at: now.clone(), + // The URL the child actually dialed, not the local `relay_url` + // binding — same string by construction here, but reading it off the + // process keeps every receipt site identical to the spawn. + connect_relay_url: Some(process.connect_relay_url.clone()), }; if let Err(error) = super::write_agent_runtime_receipt(app, &receipt) { let _ = terminate_process(process.child.id()); diff --git a/desktop/src-tauri/src/managed_agents/runtime/process.rs b/desktop/src-tauri/src/managed_agents/runtime/process.rs index 37eb5659a4a..fc66d69e0e7 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/process.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/process.rs @@ -408,7 +408,16 @@ pub(crate) fn valid_agent_runtime_receipt_with( else { return false; }; + // A stored connection URL must belong to this pair: canonicalizing it has + // to reproduce the receipt's own key. Absent is fine (pre-field receipts); + // present-but-foreign or unparseable is a corrupt receipt, not a fallback. + let connect_url_matches_key = match receipt.connect_relay_url.as_deref() { + None => true, + Some(connect_url) => ManagedAgentRuntimeKey::new(receipt.key.pubkey.clone(), connect_url) + .is_ok_and(|from_connect| from_connect == receipt.key), + }; canonical == receipt.key + && connect_url_matches_key && path.file_name().and_then(|name| name.to_str()) == Some(&format!("{}.json", receipt.key.runtime_id())) && receipt.desktop_instance_id == instance_id diff --git a/desktop/src-tauri/src/managed_agents/runtime/stop.rs b/desktop/src-tauri/src/managed_agents/runtime/stop.rs index 08bca15febb..c8763ed228a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/stop.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/stop.rs @@ -19,6 +19,23 @@ pub(crate) fn managed_agent_runtime_keys( .collect() } +/// The relay URLs to dial when restarting `pubkey`'s live pairs: each pair's +/// stamped connection URL. Restart paths must use this instead of +/// `key.relay_url` — the canonical spelling folds loopback hosts, and on a +/// host-scoped multi-tenant relay that lands the child in the wrong (empty) +/// community. Collect before stopping: stopping removes the pair, and with it +/// the only in-memory copy of the configured spelling. +pub(crate) fn managed_agent_restart_targets( + runtimes: &HashMap, + pubkey: &str, +) -> Vec { + runtimes + .iter() + .filter(|(key, _)| key.pubkey.eq_ignore_ascii_case(pubkey)) + .map(|(_, runtime)| runtime.connect_relay_url.clone()) + .collect() +} + #[cfg(test)] pub(crate) fn managed_agent_runtime_relay_urls( runtimes: &HashMap, diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 18e14c3ca71..befc4106dee 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -900,6 +900,7 @@ fn receipt_fixture( pid: std::process::id(), desktop_instance_id: "test-instance".into(), started_at: "now".into(), + connect_relay_url: None, } } @@ -931,6 +932,122 @@ fn receipt_validation_rejects_wrong_pair_filename() { )); } +#[test] +fn receipt_without_connect_url_deserializes_and_validates() { + // Receipts persisted before `connectRelayUrl` existed must keep loading + // (field absent -> None) and keep validating. + let key = + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(); + let json = format!( + r#"{{"key":{{"pubkey":"{}","relayUrl":"{}"}},"pid":{},"desktopInstanceId":"test-instance","startedAt":"now"}}"#, + key.pubkey, + key.relay_url, + std::process::id(), + ); + let receipt: crate::managed_agents::ManagedAgentRuntimeReceipt = + serde_json::from_str(&json).expect("pre-connect-url receipt must deserialize"); + assert_eq!(receipt.connect_relay_url, None); + + let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); + assert!(super::valid_agent_runtime_receipt_with( + &path, + &receipt, + "test-instance", + |_| true, + |_, _| true, + )); +} + +#[test] +fn receipt_connect_url_matching_pair_validates() { + // The configured spelling (`localhost`) canonicalizes to the receipt's own + // key (`127.0.0.1`), so the receipt is valid — this is the normal shape + // written by every spawn on a loopback workspace. + let mut receipt = receipt_fixture( + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(), + ); + receipt.connect_relay_url = Some("ws://localhost:3100".into()); + let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); + assert!(super::valid_agent_runtime_receipt_with( + &path, + &receipt, + "test-instance", + |_| true, + |_, _| true, + )); +} + +#[test] +fn receipt_connect_url_foreign_pair_rejected() { + // A connection URL that canonicalizes to a DIFFERENT pair key is a corrupt + // or cross-wired receipt — it must fail validation, not fall back. + let mut receipt = receipt_fixture( + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(), + ); + receipt.connect_relay_url = Some("ws://localhost:4000".into()); + let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); + assert!(!super::valid_agent_runtime_receipt_with( + &path, + &receipt, + "test-instance", + |_| true, + |_, _| true, + )); +} + +#[test] +fn receipt_connect_url_roundtrips_through_persistence() { + // Present field serializes (camelCase) and deserializes unchanged, so a + // restart in a later session re-dials the exact configured spelling. + let mut receipt = receipt_fixture( + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(), + ); + receipt.connect_relay_url = Some("ws://localhost:3100".into()); + let json = serde_json::to_string(&receipt).expect("serialize receipt"); + assert!(json.contains("\"connectRelayUrl\":\"ws://localhost:3100\"")); + let restored: crate::managed_agents::ManagedAgentRuntimeReceipt = + serde_json::from_str(&json).expect("deserialize receipt"); + assert_eq!(restored, receipt); +} + +#[test] +fn restart_targets_preserve_configured_spelling_per_pair() { + // Restart targets come from each live pair's stamped connection URL — + // the configured loopback spelling survives (never the canonical fold), + // and only the requested agent's pairs are selected. + let agent_a = "aa".repeat(32); + let agent_b = "bb".repeat(32); + let mut runtimes = std::collections::HashMap::new(); + runtimes.insert( + crate::managed_agents::ManagedAgentRuntimeKey::new(agent_a.clone(), "ws://localhost:3100") + .unwrap(), + make_pair_runtime_with_connect_url("ws://localhost:3100"), + ); + runtimes.insert( + crate::managed_agents::ManagedAgentRuntimeKey::new(agent_a.clone(), "wss://other.example") + .unwrap(), + make_pair_runtime_with_connect_url("wss://other.example"), + ); + runtimes.insert( + crate::managed_agents::ManagedAgentRuntimeKey::new(agent_b, "ws://localhost:9999").unwrap(), + make_pair_runtime_with_connect_url("ws://localhost:9999"), + ); + + let mut targets = super::managed_agent_restart_targets(&runtimes, &agent_a); + targets.sort(); + assert_eq!( + targets, + vec![ + "ws://localhost:3100".to_string(), + "wss://other.example".to_string(), + ], + ); +} + #[test] fn replacement_removes_receipt_only_after_confirmed_exit() { use std::cell::{Cell, RefCell}; @@ -1244,6 +1361,12 @@ fn minimal_record(pubkey: &str) -> crate::managed_agents::ManagedAgentRecord { } fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRuntime { + make_pair_runtime_with_connect_url("wss://relay.example") +} + +fn make_pair_runtime_with_connect_url( + connect_relay_url: &str, +) -> crate::managed_agents::ManagedAgentPairRuntime { use std::process::{Command, Stdio}; // Spawn a real child so ManagedAgentProcess's Child field is satisfied. // `true` exits immediately with 0 — just a handle we need for type purposes. @@ -1265,6 +1388,7 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun let process = crate::managed_agents::ManagedAgentProcess { child, log_path: std::path::PathBuf::new(), + connect_relay_url: connect_relay_url.to_string(), spawn_config: crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot( &minimal_record(&"cc".repeat(32)), &[], diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index 59f4fb36ba8..888fb942c45 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -60,7 +60,12 @@ fn status_for_with( ManagedAgentRuntimeStatus { pubkey: key.pubkey.clone(), relay_url: key.relay_url.clone(), - requested_relay_url, + // Callers that know the exact descriptor URL (reconcile) pass it in; + // otherwise fall back to the live pair's stamped connection URL so + // frontend start/restart actions re-dial the configured spelling + // instead of the canonical form. + requested_relay_url: requested_relay_url + .or_else(|| runtime.map(|runtime| runtime.connect_relay_url.clone())), local_setup, lifecycle: runtime .map(|runtime| runtime.lifecycle.clone()) @@ -290,6 +295,7 @@ fn start_pair( pid: process.child.id(), desktop_instance_id: current_instance_id(&app), started_at: now.clone(), + connect_relay_url: Some(process.connect_relay_url.clone()), }; if let Err(error) = write_agent_runtime_receipt(&app, &receipt) { let _ = terminate_process(process.child.id()); diff --git a/desktop/src-tauri/src/managed_agents/runtime_types.rs b/desktop/src-tauri/src/managed_agents/runtime_types.rs index 4862cedbae3..d65a45dcf45 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_types.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_types.rs @@ -83,8 +83,10 @@ impl ManagedAgentPairRuntime { pub struct ManagedAgentRuntimeStatus { pub pubkey: String, pub relay_url: String, - /// Exact descriptor URL echoed only by reconcile result rows so callers can - /// correlate a canonical response without normalizing on the frontend. + /// The requested (non-canonical) URL when known: the exact submitted + /// descriptor on reconcile result rows, otherwise the live pair's actual + /// connection spelling. Lets callers correlate rows across spellings + /// without normalizing on the frontend. #[serde(skip_serializing_if = "Option::is_none")] pub requested_relay_url: Option, pub local_setup: bool, @@ -117,4 +119,12 @@ pub struct ManagedAgentRuntimeReceipt { pub pid: u32, pub desktop_instance_id: String, pub started_at: String, + /// The exact relay URL the child was dialed with, persisted alongside the + /// pair identity. Optional so receipts written before this field existed + /// still deserialize; absent marks a valid legacy receipt whose connection + /// spelling is unknown. When present it must canonicalize back to + /// `key.relay_url` (see `valid_agent_runtime_receipt_with`), so a receipt + /// can never smuggle a connection URL belonging to a different pair. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub connect_relay_url: Option, } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs index a61eb92e2e5..8c8749eb284 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs @@ -21,7 +21,7 @@ use crate::managed_agents::AcpAvailabilityStatus; /// the process was spawned with. const ADAPTER_AVAILABILITY_FIELD: &str = "adapter_availability"; -const MASK: &str = "••••"; +pub(crate) const MASK: &str = "••••"; /// One changed field. `field` is a dotted path built from serde field names, /// with dynamic map keys appended verbatim (`env.OPENAI_API_KEY`). The UI diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs index a7a8cab93e7..075451f5e7d 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs @@ -438,6 +438,9 @@ fn no_sentinel_reaches_the_owning_process_debug_output() { let process = crate::managed_agents::ManagedAgentProcess { child, log_path: std::path::PathBuf::new(), + // Token-bearing URL: the process Debug impl must mask this field, + // exactly like the snapshot masks its own relay_url. + connect_relay_url: RELAY_WITH_TOKEN.to_string(), spawn_config: seeded_with_sentinels(), setup_mode: false, adapter_availability: None, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index e5be105fed0..1924c027d60 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -458,10 +458,16 @@ pub struct RelayMeshConfig { pub model_ref: String, } -#[derive(Debug)] pub struct ManagedAgentProcess { pub child: Child, pub log_path: PathBuf, + /// The exact relay URL this child was told to dial (`BUZZ_RELAY_URL`). + /// May differ from the canonical `ManagedAgentRuntimeKey::relay_url` + /// spelling — the tenancy boundary is host-derived, so restarts must + /// reuse this URL, never the canonical form. Stamped from the same + /// string passed to the child env; receipts persist it alongside the + /// pair identity. + pub connect_relay_url: String, /// The effective spawn config this process was launched with (see /// `spawn_snapshot::SpawnConfigSnapshot`). Runtime-only — never persisted. /// The summary builder recomputes a prospective snapshot and reports @@ -490,6 +496,27 @@ pub struct ManagedAgentProcess { pub job: Option, } +/// Hand-written so `connect_relay_url` never renders verbatim: +/// `normalize_relay_url` rejects userinfo but deliberately preserves query +/// strings, so `wss://relay.example/ws?token=...` is a valid value. Same +/// masking policy as `SpawnConfigSnapshot`'s `relay_url` (its single +/// redaction authority), pinned by the owning-process Debug sentinel test. +impl std::fmt::Debug for ManagedAgentProcess { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut s = f.debug_struct("ManagedAgentProcess"); + s.field("child", &self.child) + .field("log_path", &self.log_path) + .field("connect_relay_url", &super::spawn_snapshot::diff::MASK) + .field("spawn_config", &self.spawn_config) + .field("setup_mode", &self.setup_mode) + .field("adapter_availability", &self.adapter_availability) + .field("start_nonce", &self.start_nonce); + #[cfg(windows)] + s.field("job", &self.job); + s.finish() + } +} + #[derive(Debug, Clone, Serialize)] pub struct ManagedAgentSummary { pub pubkey: String, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 24ef6257832..453351c8861 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -289,7 +289,9 @@ export type ManagedAgentRuntimeLifecycle = export type ManagedAgentRuntimeStatus = { pubkey: string; - /** Exact submitted descriptor, present only on startup reconcile results. */ + /** The requested (non-canonical) URL when known: the exact submitted + * descriptor on reconcile results, otherwise the live pair's actual + * connection spelling. */ requestedRelayUrl?: string; /** Canonical, backend-owned pair identity component. Do not normalize in TS. */ relayUrl: string; From 4f759f915414781d8bcafb857c457f3c31bf532a Mon Sep 17 00:00:00 2001 From: anilkishan <10408515+anilkishan@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:15:02 -0700 Subject: [PATCH 4/6] fix(desktop): fail cross-spelling pair reuse; split oversized files Review round 2 on this PR (P1 + desktop-check ratchet). Fix - connection-target conflict (P1): all three canonical-key reuse sites (start_pair, start_managed_agent_process, and launch restore's Phase-B already-live check) reused any live runtime matching the canonical pair key without comparing its stamped connection URL, so a start request for one loopback spelling could reuse a child connected to the other spelling's tenant and falsely report the requested tenant as started, stopping reconciliation retries. All three sites now call ensure_pair_connection_matches(); the command paths return an explicit connection-target-conflict error and restore records the conflict as a failed outcome in Phase C. The error deliberately omits both URLs (they may carry query tokens and the string lands in last_error and the UI). Covered by pair_reuse_with_matching_spelling_is_allowed, pair_reuse_across_spellings_is_a_connection_target_conflict, and restore_reuses_live_pair_only_for_matching_spelling. Restructure - file-size ratchet (no behavior change): - runtime/connect_url_tests.rs: new home for the connection-URL tests; shared fixtures (receipt, minimal record, pair-runtime placeholder) move to runtime/test_fixtures.rs. - ManagedAgentProcess Debug impl moves from types.rs to runtime/process.rs (cross-platform, beside the receipt validation); connection_relay_url() moves there too, next to its only coupling. - child_rust_log_filter() moves to agent_env.rs; persona_drift_state() moves to runtime/metadata.rs. - Comment tightening in agent_discovery.rs and shared/api/types.ts. All five flagged files are back at or under their ratchet limits; no limit was raised. Co-authored-by: anilkishan <10408515+anilkishan@users.noreply.github.com> Signed-off-by: anilkishan <10408515+anilkishan@users.noreply.github.com> --- .../src-tauri/src/commands/agent_discovery.rs | 4 +- .../src-tauri/src/managed_agents/agent_env.rs | 8 + .../src-tauri/src/managed_agents/restore.rs | 60 +++++- .../src-tauri/src/managed_agents/runtime.rs | 58 ++---- .../runtime/connect_url_tests.rs | 137 ++++++++++++ .../src/managed_agents/runtime/metadata.rs | 29 +++ .../src/managed_agents/runtime/process.rs | 51 +++++ .../managed_agents/runtime/test_fixtures.rs | 80 +++++++ .../src/managed_agents/runtime/tests.rs | 196 +----------------- .../src/managed_agents/runtime_commands.rs | 6 + desktop/src-tauri/src/managed_agents/types.rs | 29 +-- desktop/src/shared/api/types.ts | 4 +- 12 files changed, 382 insertions(+), 280 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 17c3d492aa2..6df68098e5a 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -562,9 +562,7 @@ async fn restart_single_agent_after_install( if record.backend != BackendKind::Local { return Err(format!("agent {pubkey_owned} is no longer a local agent")); } - // Collect the restart dial targets (each pair's configured connection - // URL, not the canonical key spelling) BEFORE the stop below drops the - // pairs — and their URLs — from the runtimes map. + // Dial targets (configured spellings) must be collected before the stop. let restart_targets = crate::managed_agents::managed_agent_restart_targets(&runtimes, &pubkey_owned); if restart_targets.is_empty() { diff --git a/desktop/src-tauri/src/managed_agents/agent_env.rs b/desktop/src-tauri/src/managed_agents/agent_env.rs index 59b300d9d17..39376e88b82 100644 --- a/desktop/src-tauri/src/managed_agents/agent_env.rs +++ b/desktop/src-tauri/src/managed_agents/agent_env.rs @@ -137,6 +137,14 @@ pub(crate) fn parse_agent_env_lines(raw: &str) -> Vec<(&str, &str)> { .collect() } +pub(super) fn child_rust_log_filter() -> String { + match std::env::var("RUST_LOG") { + Ok(existing) if existing.contains("buzz_acp") => existing, + Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"), + _ => "buzz_acp=info".to_string(), + } +} + #[cfg(test)] mod tests { use super::{ diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 9c3c38def3a..21964c147ff 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -308,21 +308,23 @@ pub async fn restore_managed_agents_on_launch( Ok(key) => { // F2: if a concurrent startup reconcile already // tracked a live child for this exact pair during - // the Phase A window, leave it alone. Mirrors the + // the Phase A window, leave it alone - but only + // when it dials the requested URL. Mirrors the // live-child guard in `start_pair`. - let already_live = app + let live_outcome = app .state::() .managed_agent_processes .lock() .ok() .and_then(|mut runtimes| { - runtimes.get_mut(&key).map(|runtime| { - runtime.child.try_wait().ok().flatten().is_none() - }) - }) - .unwrap_or(false); - if already_live { - SpawnOutcome::Skipped + let runtime = runtimes.get_mut(&key)?; + if runtime.child.try_wait().ok().flatten().is_some() { + return None; + } + Some(live_pair_outcome(runtime, &relay_url)) + }); + if let Some(outcome) = live_outcome { + outcome } else { match super::terminate_untracked_pair_runtime(app, &key) .and_then(|()| { @@ -494,3 +496,43 @@ fn persist_restore_error( record.last_error = Some(error); save_managed_agents(app, &records) } + +/// Phase-B decision for an already-tracked live pair: reuse (skip spawning) +/// only when the live child dials the requested URL; a cross-spelling child +/// is a connection-target conflict recorded as a failed outcome so Phase C +/// persists the sanitized error instead of silently keeping the wrong tenant. +fn live_pair_outcome( + runtime: &super::ManagedAgentPairRuntime, + requested_relay_url: &str, +) -> SpawnOutcome { + match super::ensure_pair_connection_matches(runtime, requested_relay_url) { + Ok(()) => SpawnOutcome::Skipped, + Err(error) => SpawnOutcome::Failed(error), + } +} + +#[cfg(test)] +mod tests { + use super::SpawnOutcome; + use crate::managed_agents::make_pair_runtime_with_connect_url; + + #[test] + fn restore_reuses_live_pair_only_for_matching_spelling() { + let matching = make_pair_runtime_with_connect_url("ws://localhost:3100"); + assert!(matches!( + super::live_pair_outcome(&matching, " ws://localhost:3100 "), + SpawnOutcome::Skipped + )); + + // localhost and 127.0.0.1 share a canonical key but are distinct + // tenants: restore must record the conflict, not keep the wrong one. + let foreign = make_pair_runtime_with_connect_url("ws://127.0.0.1:3100"); + match super::live_pair_outcome(&foreign, "ws://localhost:3100") { + SpawnOutcome::Failed(error) => { + assert!(error.contains("connection-target conflict")); + assert!(!error.contains("3100"), "error must not echo URLs"); + } + _ => panic!("cross-spelling live pair must fail, not be reused"), + } + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index f73a7fbf8e9..5956406c270 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -2,7 +2,9 @@ use std::collections::HashMap; use tauri::AppHandle; -use super::agent_env::{build_buzz_agent_provider_defaults, idle_pool_sleep_env}; +use super::agent_env::{ + build_buzz_agent_provider_defaults, child_rust_log_filter, idle_pool_sleep_env, +}; use crate::{ managed_agents::{ @@ -21,6 +23,7 @@ pub(crate) use path::{compose_path_entries, should_skip_claude_executable, shoul pub(crate) use super::access_policy::{build_respond_to_env_with_policy, RespondToEnv}; mod metadata; +use metadata::persona_drift_state; pub(crate) use metadata::{ apply_agent_display_env, resolve_session_title, runtime_metadata_env_vars, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, @@ -40,8 +43,9 @@ use process::{ terminate_runtime_receipt_with, valid_agent_runtime_receipt_with, }; pub(crate) use process::{ - current_instance_id, process_belongs_to_us, process_has_buzz_marker, process_is_running, - terminate_process, terminate_untracked_pair_runtime, valid_agent_runtime_receipt, + connection_relay_url, current_instance_id, ensure_pair_connection_matches, + process_belongs_to_us, process_has_buzz_marker, process_is_running, terminate_process, + terminate_untracked_pair_runtime, valid_agent_runtime_receipt, }; mod orphan_sweep; @@ -68,35 +72,6 @@ mod lifecycle; use lifecycle::kill_stale_tracked_processes_with; pub use lifecycle::{kill_stale_tracked_processes, sync_managed_agent_processes}; -/// Classify an agent's persona against the live catalog for the Agents-menu -/// drift indicator. Returns `(out_of_date, orphaned)`. -/// -/// Drift basis is the RECORD's `persona_source_version`, never the engram: -/// - persona_id set + persona present: out_of_date when the snapshot hash -/// differs from the persona's current content hash. -/// - persona_id set + persona gone: orphaned (no current hash to respawn into, -/// so never out_of_date — we must not tell the user to respawn into nothing). -/// - no persona_id: neither — a hand-built agent has no persona to drift from. -fn persona_drift_state( - record: &ManagedAgentRecord, - personas: &[crate::managed_agents::types::AgentDefinition], -) -> (bool, bool) { - let Some(persona_id) = record.persona_id.as_deref() else { - return (false, false); - }; - let Some(persona) = personas.iter().find(|p| p.id == persona_id) else { - return (false, true); - }; - let current = crate::managed_agents::persona_events::persona_content_hash( - &crate::managed_agents::persona_events::persona_event_content(persona), - ); - let out_of_date = record - .persona_source_version - .as_deref() - .is_some_and(|pinned| pinned != current); - (out_of_date, false) -} - /// Resolve the runtime-pair key this record maps to for the active /// workspace: always the active workspace relay (the legacy per-record relay /// pin is ignored — see `effective_agent_relay_url`). Returns `None` for @@ -395,10 +370,6 @@ pub(crate) fn configure_runtime_cli( } } -fn connection_relay_url(configured_relay_url: &str) -> String { - configured_relay_url.trim().to_string() -} - /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -927,8 +898,6 @@ pub fn spawn_agent_child( Ok(crate::managed_agents::ManagedAgentProcess { child, log_path, - // The trimmed value that actually went into the child's - // BUZZ_RELAY_URL above, not the raw `relay_url` parameter. connect_relay_url: effective_relay_url, spawn_config, setup_mode: spawned_setup_mode, @@ -937,14 +906,6 @@ pub fn spawn_agent_child( }) } -fn child_rust_log_filter() -> String { - match std::env::var("RUST_LOG") { - Ok(existing) if existing.contains("buzz_acp") => existing, - Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"), - _ => "buzz_acp=info".to_string(), - } -} - pub fn start_managed_agent_process( app: &AppHandle, record: &mut ManagedAgentRecord, @@ -967,6 +928,7 @@ pub fn start_managed_agent_process( .map_err(|error| format!("failed to inspect running process: {error}"))? .is_none() { + ensure_pair_connection_matches(runtime, &relay_url)?; return Ok(()); } @@ -1008,6 +970,10 @@ pub fn start_managed_agent_process( #[cfg(test)] mod test_fixtures; +#[cfg(test)] +pub(crate) use test_fixtures::make_pair_runtime_with_connect_url; +#[cfg(test)] +mod connect_url_tests; #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs new file mode 100644 index 00000000000..35e2cc53015 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs @@ -0,0 +1,137 @@ +//! Connection-URL persistence tests: receipt back-compat and validation, +//! restart-target selection, and the pair-reuse conflict guard. + +use super::test_fixtures::{make_pair_runtime_with_connect_url, receipt_fixture}; + +#[test] +fn receipt_without_connect_url_deserializes_and_validates() { + // Receipts persisted before `connectRelayUrl` existed must keep loading + // (field absent -> None) and keep validating. + let key = + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(); + let json = format!( + r#"{{"key":{{"pubkey":"{}","relayUrl":"{}"}},"pid":{},"desktopInstanceId":"test-instance","startedAt":"now"}}"#, + key.pubkey, + key.relay_url, + std::process::id(), + ); + let receipt: crate::managed_agents::ManagedAgentRuntimeReceipt = + serde_json::from_str(&json).expect("pre-connect-url receipt must deserialize"); + assert_eq!(receipt.connect_relay_url, None); + + let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); + assert!(super::valid_agent_runtime_receipt_with( + &path, + &receipt, + "test-instance", + |_| true, + |_, _| true, + )); +} + +#[test] +fn receipt_connect_url_matching_pair_validates() { + // The configured spelling (`localhost`) canonicalizes to the receipt's own + // key (`127.0.0.1`), so the receipt is valid — this is the normal shape + // written by every spawn on a loopback workspace. + let mut receipt = receipt_fixture( + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(), + ); + receipt.connect_relay_url = Some("ws://localhost:3100".into()); + let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); + assert!(super::valid_agent_runtime_receipt_with( + &path, + &receipt, + "test-instance", + |_| true, + |_, _| true, + )); +} + +#[test] +fn receipt_connect_url_foreign_pair_rejected() { + // A connection URL that canonicalizes to a DIFFERENT pair key is a corrupt + // or cross-wired receipt — it must fail validation, not fall back. + let mut receipt = receipt_fixture( + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(), + ); + receipt.connect_relay_url = Some("ws://localhost:4000".into()); + let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); + assert!(!super::valid_agent_runtime_receipt_with( + &path, + &receipt, + "test-instance", + |_| true, + |_, _| true, + )); +} + +#[test] +fn receipt_connect_url_roundtrips_through_persistence() { + // Present field serializes (camelCase) and deserializes unchanged, so a + // restart in a later session re-dials the exact configured spelling. + let mut receipt = receipt_fixture( + crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") + .unwrap(), + ); + receipt.connect_relay_url = Some("ws://localhost:3100".into()); + let json = serde_json::to_string(&receipt).expect("serialize receipt"); + assert!(json.contains("\"connectRelayUrl\":\"ws://localhost:3100\"")); + let restored: crate::managed_agents::ManagedAgentRuntimeReceipt = + serde_json::from_str(&json).expect("deserialize receipt"); + assert_eq!(restored, receipt); +} + +#[test] +fn restart_targets_preserve_configured_spelling_per_pair() { + // Restart targets come from each live pair's stamped connection URL — + // the configured loopback spelling survives (never the canonical fold), + // and only the requested agent's pairs are selected. + let agent_a = "aa".repeat(32); + let agent_b = "bb".repeat(32); + let mut runtimes = std::collections::HashMap::new(); + runtimes.insert( + crate::managed_agents::ManagedAgentRuntimeKey::new(agent_a.clone(), "ws://localhost:3100") + .unwrap(), + make_pair_runtime_with_connect_url("ws://localhost:3100"), + ); + runtimes.insert( + crate::managed_agents::ManagedAgentRuntimeKey::new(agent_a.clone(), "wss://other.example") + .unwrap(), + make_pair_runtime_with_connect_url("wss://other.example"), + ); + runtimes.insert( + crate::managed_agents::ManagedAgentRuntimeKey::new(agent_b, "ws://localhost:9999").unwrap(), + make_pair_runtime_with_connect_url("ws://localhost:9999"), + ); + + let mut targets = super::managed_agent_restart_targets(&runtimes, &agent_a); + targets.sort(); + assert_eq!( + targets, + vec![ + "ws://localhost:3100".to_string(), + "wss://other.example".to_string(), + ], + ); +} + +#[test] +fn pair_reuse_with_matching_spelling_is_allowed() { + let runtime = make_pair_runtime_with_connect_url("ws://localhost:3100"); + assert!(super::ensure_pair_connection_matches(&runtime, " ws://localhost:3100 ").is_ok()); +} + +#[test] +fn pair_reuse_across_spellings_is_a_connection_target_conflict() { + // localhost and 127.0.0.1 share a canonical key but are distinct tenants: + // reuse must fail loudly instead of reporting the requested tenant started. + let runtime = make_pair_runtime_with_connect_url("ws://127.0.0.1:3100"); + let err = super::ensure_pair_connection_matches(&runtime, "ws://localhost:3100").unwrap_err(); + assert!(err.contains("connection-target conflict")); + // No URL disclosure: the message must not echo either spelling. + assert!(!err.contains("3100")); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs index 5aef424ea61..41da7d9a940 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs @@ -74,6 +74,35 @@ pub(crate) fn resolve_session_title(display_name: Option<&str>, name: &str) -> O .find(|value| !value.is_empty()) } +/// Classify an agent's persona against the live catalog for the Agents-menu +/// drift indicator. Returns `(out_of_date, orphaned)`. +/// +/// Drift basis is the RECORD's `persona_source_version`, never the engram: +/// - persona_id set + persona present: out_of_date when the snapshot hash +/// differs from the persona's current content hash. +/// - persona_id set + persona gone: orphaned (no current hash to respawn into, +/// so never out_of_date — we must not tell the user to respawn into nothing). +/// - no persona_id: neither — a hand-built agent has no persona to drift from. +pub(super) fn persona_drift_state( + record: &crate::managed_agents::ManagedAgentRecord, + personas: &[crate::managed_agents::types::AgentDefinition], +) -> (bool, bool) { + let Some(persona_id) = record.persona_id.as_deref() else { + return (false, false); + }; + let Some(persona) = personas.iter().find(|p| p.id == persona_id) else { + return (false, true); + }; + let current = crate::managed_agents::persona_events::persona_content_hash( + &crate::managed_agents::persona_events::persona_event_content(persona), + ); + let out_of_date = record + .persona_source_version + .as_deref() + .is_some_and(|pinned| pinned != current); + (out_of_date, false) +} + #[cfg(test)] mod tests { use super::resolve_session_title; diff --git a/desktop/src-tauri/src/managed_agents/runtime/process.rs b/desktop/src-tauri/src/managed_agents/runtime/process.rs index fc66d69e0e7..5ead0dfa08e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/process.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/process.rs @@ -476,3 +476,54 @@ pub(crate) fn terminate_untracked_pair_runtime( super::super::remove_agent_runtime_receipt_path, ) } + +/// A live pair may only be reused for a start request that dials the same +/// URL it already holds. Canonical keys fold host spellings, so two distinct +/// tenants can share one key; silently reusing across spellings would report +/// the requested tenant as started while the child stays connected to the +/// old one, and reconciliation would stop retrying. +/// The error deliberately omits both URLs: they may carry query tokens, and +/// this string lands in `last_error` and the UI. +pub(crate) fn connection_relay_url(configured_relay_url: &str) -> String { + configured_relay_url.trim().to_string() +} + +pub(crate) fn ensure_pair_connection_matches( + runtime: &ManagedAgentPairRuntime, + requested_relay_url: &str, +) -> Result<(), String> { + if runtime.connect_relay_url == connection_relay_url(requested_relay_url) { + Ok(()) + } else { + Err( + "connection-target conflict: a live runtime for this agent already exists under the \ + same community identity but a different connection URL; stop the pair before \ + starting it with the requested URL" + .into(), + ) + } +} + +/// Hand-written so `connect_relay_url` never renders verbatim: +/// `normalize_relay_url` rejects userinfo but deliberately preserves query +/// strings, so `wss://relay.example/ws?token=...` is a valid value. Same +/// masking policy as `SpawnConfigSnapshot`'s `relay_url` (its single +/// redaction authority), pinned by the owning-process Debug sentinel test. +impl std::fmt::Debug for crate::managed_agents::ManagedAgentProcess { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut s = f.debug_struct("ManagedAgentProcess"); + s.field("child", &self.child) + .field("log_path", &self.log_path) + .field( + "connect_relay_url", + &crate::managed_agents::spawn_snapshot::diff::MASK, + ) + .field("spawn_config", &self.spawn_config) + .field("setup_mode", &self.setup_mode) + .field("adapter_availability", &self.adapter_availability) + .field("start_nonce", &self.start_nonce); + #[cfg(windows)] + s.field("job", &self.job); + s.finish() + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs index 9836d983ed3..1a5dc8a67b6 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs @@ -91,3 +91,83 @@ pub(super) fn fixture( relay_mesh: None, } } + +pub(crate) fn make_pair_runtime_with_connect_url( + connect_relay_url: &str, +) -> crate::managed_agents::ManagedAgentPairRuntime { + use std::process::{Command, Stdio}; + // Spawn a real child so ManagedAgentProcess's Child field is satisfied. + // `true` exits immediately with 0 — just a handle we need for type purposes. + // + // Absolute `/usr/bin/true` on unix (present on both macOS and Linux): + // parallel tests holding `lock_path_mutex` swap PATH to a tempdir, and a + // bare `true` lookup during that window fails with NotFound (observed + // flake). Windows keeps the PATH lookup — no test there swaps PATH. + #[cfg(unix)] + let program = "/usr/bin/true"; + #[cfg(windows)] + let program = "true"; + let child = Command::new(program) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn true for placeholder"); + let process = crate::managed_agents::ManagedAgentProcess { + child, + log_path: std::path::PathBuf::new(), + connect_relay_url: connect_relay_url.to_string(), + spawn_config: crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot( + &minimal_record(&"cc".repeat(32)), + &[], + &[], + "wss://relay.example", + &Default::default(), + ), + setup_mode: false, + adapter_availability: None, + start_nonce: "test-nonce".to_string(), + #[cfg(windows)] + job: None, + }; + crate::managed_agents::ManagedAgentPairRuntime::starting(process) +} + +pub(super) fn minimal_record(pubkey: &str) -> crate::managed_agents::ManagedAgentRecord { + serde_json::from_str(&format!( + r#"{{ + "pubkey": "{pubkey}", + "name": "test", + "private_key_nsec": "nsec1fake", + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "buzz-agent", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "model": null, + "provider": null, + "env_vars": {{}}, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }}"# + )) + .expect("minimal_record fixture") +} + +pub(super) fn receipt_fixture( + key: crate::managed_agents::ManagedAgentRuntimeKey, +) -> crate::managed_agents::ManagedAgentRuntimeReceipt { + crate::managed_agents::ManagedAgentRuntimeReceipt { + key, + pid: std::process::id(), + desktop_instance_id: "test-instance".into(), + started_at: "now".into(), + connect_relay_url: None, + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index befc4106dee..76afd4b331f 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -125,7 +125,9 @@ fn unknown_command_returns_none() { // ── build_respond_to_env tests ─────────────────────────────────────── -use super::test_fixtures::{expected_mode, expected_owner_only, fixture}; +use super::test_fixtures::{ + expected_mode, expected_owner_only, fixture, minimal_record, receipt_fixture, +}; use super::{build_respond_to_env, build_respond_to_env_with_policy}; use crate::managed_agents::types::{ManagedAgentRecord, RespondTo}; @@ -892,18 +894,6 @@ fn own_group_grandchild_detected_by_ancestor_walk() { // ── pair receipt validation tests ─────────────────────────────────────── -fn receipt_fixture( - key: crate::managed_agents::ManagedAgentRuntimeKey, -) -> crate::managed_agents::ManagedAgentRuntimeReceipt { - crate::managed_agents::ManagedAgentRuntimeReceipt { - key, - pid: std::process::id(), - desktop_instance_id: "test-instance".into(), - started_at: "now".into(), - connect_relay_url: None, - } -} - #[test] fn receipt_validation_rejects_noncanonical_identity() { let mut receipt = receipt_fixture( @@ -932,122 +922,6 @@ fn receipt_validation_rejects_wrong_pair_filename() { )); } -#[test] -fn receipt_without_connect_url_deserializes_and_validates() { - // Receipts persisted before `connectRelayUrl` existed must keep loading - // (field absent -> None) and keep validating. - let key = - crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") - .unwrap(); - let json = format!( - r#"{{"key":{{"pubkey":"{}","relayUrl":"{}"}},"pid":{},"desktopInstanceId":"test-instance","startedAt":"now"}}"#, - key.pubkey, - key.relay_url, - std::process::id(), - ); - let receipt: crate::managed_agents::ManagedAgentRuntimeReceipt = - serde_json::from_str(&json).expect("pre-connect-url receipt must deserialize"); - assert_eq!(receipt.connect_relay_url, None); - - let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); - assert!(super::valid_agent_runtime_receipt_with( - &path, - &receipt, - "test-instance", - |_| true, - |_, _| true, - )); -} - -#[test] -fn receipt_connect_url_matching_pair_validates() { - // The configured spelling (`localhost`) canonicalizes to the receipt's own - // key (`127.0.0.1`), so the receipt is valid — this is the normal shape - // written by every spawn on a loopback workspace. - let mut receipt = receipt_fixture( - crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") - .unwrap(), - ); - receipt.connect_relay_url = Some("ws://localhost:3100".into()); - let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); - assert!(super::valid_agent_runtime_receipt_with( - &path, - &receipt, - "test-instance", - |_| true, - |_, _| true, - )); -} - -#[test] -fn receipt_connect_url_foreign_pair_rejected() { - // A connection URL that canonicalizes to a DIFFERENT pair key is a corrupt - // or cross-wired receipt — it must fail validation, not fall back. - let mut receipt = receipt_fixture( - crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") - .unwrap(), - ); - receipt.connect_relay_url = Some("ws://localhost:4000".into()); - let path = std::path::PathBuf::from(format!("{}.json", receipt.key.runtime_id())); - assert!(!super::valid_agent_runtime_receipt_with( - &path, - &receipt, - "test-instance", - |_| true, - |_, _| true, - )); -} - -#[test] -fn receipt_connect_url_roundtrips_through_persistence() { - // Present field serializes (camelCase) and deserializes unchanged, so a - // restart in a later session re-dials the exact configured spelling. - let mut receipt = receipt_fixture( - crate::managed_agents::ManagedAgentRuntimeKey::new("aa".repeat(32), "ws://localhost:3100") - .unwrap(), - ); - receipt.connect_relay_url = Some("ws://localhost:3100".into()); - let json = serde_json::to_string(&receipt).expect("serialize receipt"); - assert!(json.contains("\"connectRelayUrl\":\"ws://localhost:3100\"")); - let restored: crate::managed_agents::ManagedAgentRuntimeReceipt = - serde_json::from_str(&json).expect("deserialize receipt"); - assert_eq!(restored, receipt); -} - -#[test] -fn restart_targets_preserve_configured_spelling_per_pair() { - // Restart targets come from each live pair's stamped connection URL — - // the configured loopback spelling survives (never the canonical fold), - // and only the requested agent's pairs are selected. - let agent_a = "aa".repeat(32); - let agent_b = "bb".repeat(32); - let mut runtimes = std::collections::HashMap::new(); - runtimes.insert( - crate::managed_agents::ManagedAgentRuntimeKey::new(agent_a.clone(), "ws://localhost:3100") - .unwrap(), - make_pair_runtime_with_connect_url("ws://localhost:3100"), - ); - runtimes.insert( - crate::managed_agents::ManagedAgentRuntimeKey::new(agent_a.clone(), "wss://other.example") - .unwrap(), - make_pair_runtime_with_connect_url("wss://other.example"), - ); - runtimes.insert( - crate::managed_agents::ManagedAgentRuntimeKey::new(agent_b, "ws://localhost:9999").unwrap(), - make_pair_runtime_with_connect_url("ws://localhost:9999"), - ); - - let mut targets = super::managed_agent_restart_targets(&runtimes, &agent_a); - targets.sort(); - assert_eq!( - targets, - vec![ - "ws://localhost:3100".to_string(), - "wss://other.example".to_string(), - ], - ); -} - #[test] fn replacement_removes_receipt_only_after_confirmed_exit() { use std::cell::{Cell, RefCell}; @@ -1333,33 +1207,6 @@ fn receipt_invalid_when_process_not_running() { // ── Test helpers ──────────────────────────────────────────────────────────── -fn minimal_record(pubkey: &str) -> crate::managed_agents::ManagedAgentRecord { - serde_json::from_str(&format!( - r#"{{ - "pubkey": "{pubkey}", - "name": "test", - "private_key_nsec": "nsec1fake", - "relay_url": "", - "acp_command": "buzz-acp", - "agent_command": "buzz-agent", - "agent_args": [], - "mcp_command": "", - "turn_timeout_seconds": 320, - "system_prompt": null, - "model": null, - "provider": null, - "env_vars": {{}}, - "created_at": "2026-01-01T00:00:00Z", - "updated_at": "2026-01-01T00:00:00Z", - "last_started_at": null, - "last_stopped_at": null, - "last_exit_code": null, - "last_error": null - }}"# - )) - .expect("minimal_record fixture") -} - fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRuntime { make_pair_runtime_with_connect_url("wss://relay.example") } @@ -1367,40 +1214,5 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun fn make_pair_runtime_with_connect_url( connect_relay_url: &str, ) -> crate::managed_agents::ManagedAgentPairRuntime { - use std::process::{Command, Stdio}; - // Spawn a real child so ManagedAgentProcess's Child field is satisfied. - // `true` exits immediately with 0 — just a handle we need for type purposes. - // - // Absolute `/usr/bin/true` on unix (present on both macOS and Linux): - // parallel tests holding `lock_path_mutex` swap PATH to a tempdir, and a - // bare `true` lookup during that window fails with NotFound (observed - // flake). Windows keeps the PATH lookup — no test there swaps PATH. - #[cfg(unix)] - let program = "/usr/bin/true"; - #[cfg(windows)] - let program = "true"; - let child = Command::new(program) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("spawn true for placeholder"); - let process = crate::managed_agents::ManagedAgentProcess { - child, - log_path: std::path::PathBuf::new(), - connect_relay_url: connect_relay_url.to_string(), - spawn_config: crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot( - &minimal_record(&"cc".repeat(32)), - &[], - &[], - "wss://relay.example", - &Default::default(), - ), - setup_mode: false, - adapter_availability: None, - start_nonce: "test-nonce".to_string(), - #[cfg(windows)] - job: None, - }; - crate::managed_agents::ManagedAgentPairRuntime::starting(process) + super::test_fixtures::make_pair_runtime_with_connect_url(connect_relay_url) } diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index 888fb942c45..d6aac5b1661 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -277,6 +277,12 @@ fn start_pair( .get_mut(&key) .is_some_and(|runtime| runtime.child.try_wait().ok().flatten().is_none()) { + // Reuse is only valid when the live pair dials the requested URL: + // the canonical key folds host spellings, and a cross-spelling reuse + // would falsely report the requested tenant as started. + if let Some(runtime) = runtimes.get(&key) { + super::ensure_pair_connection_matches(runtime, &relay_url)?; + } let status = status_for(&app, record, &key, runtimes.get(&key), None); return Ok(status); } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 1924c027d60..34f8e7eac78 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -461,12 +461,8 @@ pub struct RelayMeshConfig { pub struct ManagedAgentProcess { pub child: Child, pub log_path: PathBuf, - /// The exact relay URL this child was told to dial (`BUZZ_RELAY_URL`). - /// May differ from the canonical `ManagedAgentRuntimeKey::relay_url` - /// spelling — the tenancy boundary is host-derived, so restarts must - /// reuse this URL, never the canonical form. Stamped from the same - /// string passed to the child env; receipts persist it alongside the - /// pair identity. + /// The exact URL this child dials (`BUZZ_RELAY_URL`); may differ from the + /// canonical key spelling. Restarts must reuse it; receipts persist it. pub connect_relay_url: String, /// The effective spawn config this process was launched with (see /// `spawn_snapshot::SpawnConfigSnapshot`). Runtime-only — never persisted. @@ -496,27 +492,6 @@ pub struct ManagedAgentProcess { pub job: Option, } -/// Hand-written so `connect_relay_url` never renders verbatim: -/// `normalize_relay_url` rejects userinfo but deliberately preserves query -/// strings, so `wss://relay.example/ws?token=...` is a valid value. Same -/// masking policy as `SpawnConfigSnapshot`'s `relay_url` (its single -/// redaction authority), pinned by the owning-process Debug sentinel test. -impl std::fmt::Debug for ManagedAgentProcess { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut s = f.debug_struct("ManagedAgentProcess"); - s.field("child", &self.child) - .field("log_path", &self.log_path) - .field("connect_relay_url", &super::spawn_snapshot::diff::MASK) - .field("spawn_config", &self.spawn_config) - .field("setup_mode", &self.setup_mode) - .field("adapter_availability", &self.adapter_availability) - .field("start_nonce", &self.start_nonce); - #[cfg(windows)] - s.field("job", &self.job); - s.finish() - } -} - #[derive(Debug, Clone, Serialize)] pub struct ManagedAgentSummary { pub pubkey: String, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 453351c8861..efd8fc85c60 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -289,9 +289,7 @@ export type ManagedAgentRuntimeLifecycle = export type ManagedAgentRuntimeStatus = { pubkey: string; - /** The requested (non-canonical) URL when known: the exact submitted - * descriptor on reconcile results, otherwise the live pair's actual - * connection spelling. */ + /** Requested (non-canonical) URL: reconcile descriptor or live pair connection spelling. */ requestedRelayUrl?: string; /** Canonical, backend-owned pair identity component. Do not normalize in TS. */ relayUrl: string; From 2c765c507e41e9b03cfecd403cbe5492dc8cfcef Mon Sep 17 00:00:00 2001 From: anilkishan <10408515+anilkishan@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:00:57 -0700 Subject: [PATCH 5/6] fix(desktop): tenant-equivalent connection matching; requested URL wins in UI Review round 3 (two P1s from automated review). - ensure_pair_connection_matches compared trimmed strings byte-for-byte, so connection-equivalent spellings (host case, explicit default port, root slash, FQDN root dot) read as a connection-target conflict and could strand reconcile/start after harmless config formatting drift. Equivalence is now by comparable connection target, parsed with url::Url: lowercased scheme, parsed-host normalization (ASCII lowercase, single FQDN root dot stripped), a port kept only when it differs from the scheme's own default, a root-slash-folded path, and the query preserved verbatim as its own field. Tenancy-significant differences stay conflicts: localhost, 127.0.0.1, and [::1] are three distinct hosts, and ws vs wss, non-default ports (including the other scheme's default), paths, and query strings stay distinct. Unparsable URLs fail closed to exact comparison. - findManagedAgentRuntime accepted canonical identity matches even when a row carried the actual dial spelling, so with both loopback communities configured the wrong community's card could show the child as running there and its stop/restart could target it through the shared canonical key. requestedRelayUrl is now authoritative when present, matched by a TS mirror of the same connection-target rule; the canonical fallback remains only for legacy rows without it. - Regressions: Rust equivalence set (case, default port, root slash, FQDN dot, query with/without root slash, IPv6 textual forms) plus retained loopback/scheme/port/path conflicts and new wrong-scheme default-port, query-difference, and [::1]-vs-loopback conflicts; TS two-spelling test proving only the actually dialed community resolves the runtime while the other loopback card resolves nothing, plus spelling-equivalence and target-rule tests. Co-authored-by: anilkishan <10408515+anilkishan@users.noreply.github.com> Signed-off-by: anilkishan <10408515+anilkishan@users.noreply.github.com> --- .../runtime/connect_url_tests.rs | 45 ++++++++++ .../src/managed_agents/runtime/process.rs | 77 +++++++++++++++-- .../agents/managedAgentRuntimeStatus.test.mjs | 84 +++++++++++++++++++ .../agents/managedAgentRuntimeStatus.ts | 51 ++++++++--- 4 files changed, 239 insertions(+), 18 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs index 35e2cc53015..869770bcc0f 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs @@ -135,3 +135,48 @@ fn pair_reuse_across_spellings_is_a_connection_target_conflict() { // No URL disclosure: the message must not echo either spelling. assert!(!err.contains("3100")); } + +#[test] +fn pair_reuse_folds_connection_equivalent_spellings() { + // Host case, explicit default port, root slash, and the FQDN root dot + // are connection-equivalent per the tenancy authority - none of these + // may read as a conflict after harmless config formatting drift. + for (live, requested) in [ + ("ws://localhost:3000", "ws://LocalHost:3000"), + ("wss://relay.example", "wss://relay.example:443"), + ("ws://localhost:3000", "ws://localhost:3000/"), + ("wss://relay.example", "wss://relay.example."), + ("ws://relay.example:80/ws", "ws://Relay.Example:80/ws"), + ("ws://relay.example?token=x", "ws://relay.example/?token=x"), + ("ws://[::1]:3000", "ws://[0:0:0:0:0:0:0:1]:3000"), + ] { + let runtime = make_pair_runtime_with_connect_url(live); + assert!( + super::ensure_pair_connection_matches(&runtime, requested).is_ok(), + "equivalent spellings must not conflict: {live} vs {requested}", + ); + } +} + +#[test] +fn pair_reuse_keeps_tenancy_significant_differences_conflicting() { + // Scheme, non-default port, and path differences are real target + // differences - and the loopback split stays a conflict (see + // pair_reuse_across_spellings_is_a_connection_target_conflict). + for (live, requested) in [ + ("ws://relay.example:3000", "wss://relay.example:3000"), + ("ws://relay.example:3000", "ws://relay.example:3001"), + ("ws://relay.example:3000/a", "ws://relay.example:3000/b"), + ("ws://relay.example:443", "ws://relay.example"), + ("wss://relay.example:80", "wss://relay.example"), + ("ws://relay.example?token=x", "ws://relay.example?token=y"), + ("ws://[::1]:3000", "ws://localhost:3000"), + ("ws://[::1]:3000", "ws://127.0.0.1:3000"), + ] { + let runtime = make_pair_runtime_with_connect_url(live); + assert!( + super::ensure_pair_connection_matches(&runtime, requested).is_err(), + "distinct targets must conflict: {live} vs {requested}", + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/process.rs b/desktop/src-tauri/src/managed_agents/runtime/process.rs index 5ead0dfa08e..2922d006ac9 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/process.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/process.rs @@ -477,22 +477,83 @@ pub(crate) fn terminate_untracked_pair_runtime( ) } -/// A live pair may only be reused for a start request that dials the same -/// URL it already holds. Canonical keys fold host spellings, so two distinct -/// tenants can share one key; silently reusing across spellings would report -/// the requested tenant as started while the child stays connected to the -/// old one, and reconciliation would stop retrying. -/// The error deliberately omits both URLs: they may carry query tokens, and -/// this string lands in `last_error` and the UI. +/// The URL a child is dialed with: the configured spelling, trimmed. The +/// canonical form is identity-only; connection code preserves the authority. pub(crate) fn connection_relay_url(configured_relay_url: &str) -> String { configured_relay_url.trim().to_string() } +/// Comparable connection target, parsed with `url::Url`: lowercased scheme, +/// case-folded host with a single FQDN root dot stripped (mirroring the +/// tenancy authority `tenant::normalize_host`), a port only when it is not +/// the scheme's own default, a root-slash-folded path, and the query kept +/// verbatim. Folds spellings that reach the same tenant while preserving +/// tenancy-significant differences: `localhost`, `127.0.0.1`, and `[::1]` +/// stay three distinct hosts, and `ws` vs `wss`, non-default ports (including +/// the OTHER scheme's default), paths, and query strings stay distinct. +/// `None` for unparsable URLs - the caller falls back to exact comparison. +fn connection_target(raw: &str) -> Option { + let url = url::Url::parse(raw.trim()).ok()?; + let scheme = url.scheme().to_ascii_lowercase(); + let host = { + let host = url.host_str()?.to_ascii_lowercase(); + host.strip_suffix('.').map(str::to_string).unwrap_or(host) + }; + let default_port = match scheme.as_str() { + "ws" | "http" => Some(80), + "wss" | "https" => Some(443), + _ => None, + }; + let port = url + .port_or_known_default() + .filter(|port| Some(*port) != default_port); + let path = match url.path() { + "/" => String::new(), + path => path.to_string(), + }; + let query = url.query().map(str::to_string); + Some(ConnectionTarget { + scheme, + host, + port, + path, + query, + }) +} + +/// See [`connection_target`]. +#[derive(PartialEq)] +struct ConnectionTarget { + scheme: String, + host: String, + port: Option, + path: String, + query: Option, +} + +/// A live pair may only be reused for a start request that dials the same +/// connection target it already holds. Canonical keys fold host spellings, +/// so two distinct tenants can share one key; silently reusing across +/// spellings would report the requested tenant as started while the child +/// stays connected to the old one, and reconciliation would stop retrying. +/// Equivalence is by [`connection_target`], so harmless formatting drift +/// (host case, default port, root slash, FQDN dot) never reads as a +/// conflict. The error deliberately omits both URLs: they may carry query +/// tokens, and this string lands in `last_error` and the UI. pub(crate) fn ensure_pair_connection_matches( runtime: &ManagedAgentPairRuntime, requested_relay_url: &str, ) -> Result<(), String> { - if runtime.connect_relay_url == connection_relay_url(requested_relay_url) { + let matches = match ( + connection_target(&runtime.connect_relay_url), + connection_target(requested_relay_url), + ) { + (Some(live), Some(requested)) => live == requested, + // Fail closed on unparsable URLs: only byte-identical trimmed + // spellings count as the same target. + _ => runtime.connect_relay_url == connection_relay_url(requested_relay_url), + }; + if matches { Ok(()) } else { Err( diff --git a/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs b/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs index edd368eccd6..bc30541f78c 100644 --- a/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs +++ b/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs @@ -5,6 +5,7 @@ import { agentCommunityAvailability, agentCommunityStatusDetail, canonicalRelayUrl, + connectionTargetUrl, findManagedAgentRuntime, managedAgentRuntimeKey, } from "./managedAgentRuntimeStatus.ts"; @@ -111,3 +112,86 @@ test("matches a stored community URL against canonical backend rows", () => { undefined, ); }); + +test("requestedRelayUrl is authoritative: distinct loopback tenants never alias", () => { + // One agent, one canonical key, child actually dialed to localhost while + // both loopback communities are configured simultaneously. + const runtimes = [ + runtime({ + relayUrl: "ws://127.0.0.1:3000", + requestedRelayUrl: "ws://localhost:3000", + lifecycle: "ready", + }), + ]; + // The community that was actually dialed resolves the runtime... + assert.equal( + findManagedAgentRuntime(runtimes, "aa", "ws://localhost:3000")?.lifecycle, + "ready", + ); + // ...the OTHER loopback community must not: its card would otherwise show + // this agent as running there, and its Stop/Restart action could target + // the localhost child through the shared canonical key. + assert.equal( + findManagedAgentRuntime(runtimes, "aa", "ws://127.0.0.1:3000"), + undefined, + ); +}); + +test("requested-URL matching folds connection-equivalent spellings only", () => { + const runtimes = [ + runtime({ + relayUrl: "ws://127.0.0.1:80", + requestedRelayUrl: "ws://localhost:80", + }), + ]; + for (const spelling of [ + "ws://LocalHost:80", + "ws://localhost", + "ws://localhost/", + ]) { + assert.ok( + findManagedAgentRuntime(runtimes, "aa", spelling), + `equivalent spelling must match: ${spelling}`, + ); + } + assert.equal( + findManagedAgentRuntime(runtimes, "aa", "wss://localhost"), + undefined, + ); +}); + +test("connectionTargetUrl folds formatting but preserves tenant hosts", () => { + assert.equal(connectionTargetUrl("ws://LocalHost:80/"), "ws://localhost"); + assert.equal( + connectionTargetUrl("wss://relay.example."), + "wss://relay.example", + ); + assert.notEqual( + connectionTargetUrl("ws://localhost:3000"), + connectionTargetUrl("ws://127.0.0.1:3000"), + ); + assert.equal(connectionTargetUrl("https://relay.example"), null); + // Query rides along; the root slash folds with or without one. + assert.equal( + connectionTargetUrl("ws://relay.example?token=x"), + connectionTargetUrl("ws://relay.example/?token=x"), + ); + assert.notEqual( + connectionTargetUrl("ws://relay.example?token=x"), + connectionTargetUrl("ws://relay.example?token=y"), + ); + // The other scheme's default port is a real port, not foldable. + assert.notEqual( + connectionTargetUrl("ws://relay.example:443"), + connectionTargetUrl("ws://relay.example"), + ); + // Three loopback spellings, three distinct tenants. + assert.notEqual( + connectionTargetUrl("ws://[::1]:3000"), + connectionTargetUrl("ws://localhost:3000"), + ); + assert.notEqual( + connectionTargetUrl("ws://[::1]:3000"), + connectionTargetUrl("ws://127.0.0.1:3000"), + ); +}); diff --git a/desktop/src/features/agents/managedAgentRuntimeStatus.ts b/desktop/src/features/agents/managedAgentRuntimeStatus.ts index c3a952f7d5d..0fec069dc59 100644 --- a/desktop/src/features/agents/managedAgentRuntimeStatus.ts +++ b/desktop/src/features/agents/managedAgentRuntimeStatus.ts @@ -92,22 +92,53 @@ export function canonicalRelayUrl(raw: string): string | null { ); } +/** + * Comparable connection target mirroring the backend's tenancy authority + * (buzz-core's `tenant::normalize_host`): lowercase host, strip an explicit + * default port and the FQDN root dot, fold the root-path slash - WITHOUT + * folding loopback spellings, which are distinct tenants on a host-scoped + * relay. Returns null when the URL cannot be parsed as ws/wss. + */ +export function connectionTargetUrl(raw: string): string | null { + let url: URL; + try { + url = new URL(raw.trim()); + } catch { + return null; + } + if (url.protocol !== "ws:" && url.protocol !== "wss:") return null; + const host = url.hostname.toLowerCase().replace(/\.$/, ""); + const defaultPort = url.protocol === "ws:" ? "80" : "443"; + const port = url.port && url.port !== defaultPort ? `:${url.port}` : ""; + const path = url.pathname === "/" ? "" : url.pathname; + return `${url.protocol}//${host}${port}${path}${url.search}`; +} + export function findManagedAgentRuntime( runtimes: readonly ManagedAgentRuntimeStatus[], pubkey: string, relayUrl: string, ): ManagedAgentRuntimeStatus | undefined { const normalizedPubkey = pubkey.toLowerCase(); - // Backend rows carry the canonical pair URL; the caller passes the - // community's stored URL, which may differ in spelling (localhost vs - // 127.0.0.1, default port, trailing slash). Compare canonically, keeping - // the exact-string checks as a fallback for unparsable stored URLs. + const requestedTarget = connectionTargetUrl(relayUrl); const canonical = canonicalRelayUrl(relayUrl); - return runtimes.find( - (runtime) => - runtime.pubkey.toLowerCase() === normalizedPubkey && - (runtime.relayUrl === relayUrl || + return runtimes.find((runtime) => { + if (runtime.pubkey.toLowerCase() !== normalizedPubkey) return false; + // A row carrying the actual dial spelling is authoritative: canonical + // matching would alias distinct loopback tenants that share one runtime + // key, letting the wrong community's card claim - and stop - this child. + if (runtime.requestedRelayUrl != null) { + return ( runtime.requestedRelayUrl === relayUrl || - (canonical !== null && runtime.relayUrl === canonical)), - ); + (requestedTarget !== null && + connectionTargetUrl(runtime.requestedRelayUrl) === requestedTarget) + ); + } + // Legacy rows without the dial spelling keep the canonical fallback + // (exact-string check first, for unparsable stored URLs). + return ( + runtime.relayUrl === relayUrl || + (canonical !== null && runtime.relayUrl === canonical) + ); + }); } From 796c78a83d59e6569681b11b60a9174763f0d801 Mon Sep 17 00:00:00 2001 From: anilkishan <10408515+anilkishan@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:37:58 -0700 Subject: [PATCH 6/6] fix(desktop): fail closed on invalid connection targets connection_target and its TS mirror connectionTargetUrl silently dropped userinfo and fragments while folding, so invalid relay spellings like wss://alice@relay.example and wss://bob@relay.example compared as the same target instead of taking the documented exact-string fallback. The Rust helper also accepted non-ws(s) schemes the TS mirror rejects. Reject credential-bearing, fragment-bearing, and non-ws(s) URLs in both helpers (all rejected by normalize_relay_url), routing them to the exact-comparison fallback, and add regressions on both sides. Reported-by: Miguel Amador (review on #5993) Signed-off-by: anilkishan <10408515+anilkishan@users.noreply.github.com> --- .../runtime/connect_url_tests.rs | 36 +++++++++++++++++++ .../src/managed_agents/runtime/process.rs | 22 ++++++++---- .../agents/managedAgentRuntimeStatus.test.mjs | 34 ++++++++++++++++++ .../agents/managedAgentRuntimeStatus.ts | 9 ++++- 4 files changed, 93 insertions(+), 8 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs index 869770bcc0f..333b2377aaf 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/connect_url_tests.rs @@ -180,3 +180,39 @@ fn pair_reuse_keeps_tenancy_significant_differences_conflicting() { ); } } + +#[test] +fn pair_reuse_fails_closed_on_invalid_connection_targets() { + // Userinfo, fragments, and non-ws(s) schemes are rejected by + // `normalize_relay_url` and never fold — target comparison would + // otherwise alias `alice@` with `bob@` (or `#a` with `#b`). They fall + // back to exact comparison: only the byte-identical spelling reuses. + for spelling in [ + "ws://alice@relay.example:3000", + "wss://relay.example#a", + "http://relay.example:3000", + ] { + let runtime = make_pair_runtime_with_connect_url(spelling); + assert!( + super::ensure_pair_connection_matches(&runtime, spelling).is_ok(), + "byte-identical invalid spelling must reuse via exact fallback: {spelling}", + ); + } + for (live, requested) in [ + ( + "ws://alice@relay.example:3000", + "ws://bob@relay.example:3000", + ), + ("ws://alice@relay.example:3000", "ws://relay.example:3000"), + ("wss://relay.example#a", "wss://relay.example#b"), + ("wss://relay.example#a", "wss://relay.example"), + // Non-relay schemes never fold, even across pure formatting drift. + ("http://relay.example:3000", "http://Relay.Example:3000"), + ] { + let runtime = make_pair_runtime_with_connect_url(live); + assert!( + super::ensure_pair_connection_matches(&runtime, requested).is_err(), + "invalid targets must not alias: {live} vs {requested}", + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/process.rs b/desktop/src-tauri/src/managed_agents/runtime/process.rs index 2922d006ac9..ce40beb9a58 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/process.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/process.rs @@ -491,22 +491,30 @@ pub(crate) fn connection_relay_url(configured_relay_url: &str) -> String { /// tenancy-significant differences: `localhost`, `127.0.0.1`, and `[::1]` /// stay three distinct hosts, and `ws` vs `wss`, non-default ports (including /// the OTHER scheme's default), paths, and query strings stay distinct. -/// `None` for unparsable URLs - the caller falls back to exact comparison. +/// `None` for unparsable URLs - and for parseable ones that are not valid +/// relay targets: a non-ws(s) scheme, userinfo, or a fragment (all rejected +/// by `normalize_relay_url`, and matching the TS mirror's scheme gate). +/// Folding those away would alias distinct spellings like +/// `wss://alice@relay.example` and `wss://bob@relay.example`; returning +/// `None` fails closed onto the caller's exact comparison instead. fn connection_target(raw: &str) -> Option { let url = url::Url::parse(raw.trim()).ok()?; let scheme = url.scheme().to_ascii_lowercase(); + let default_port = match scheme.as_str() { + "ws" => 80, + "wss" => 443, + _ => return None, + }; + if !url.username().is_empty() || url.password().is_some() || url.fragment().is_some() { + return None; + } let host = { let host = url.host_str()?.to_ascii_lowercase(); host.strip_suffix('.').map(str::to_string).unwrap_or(host) }; - let default_port = match scheme.as_str() { - "ws" | "http" => Some(80), - "wss" | "https" => Some(443), - _ => None, - }; let port = url .port_or_known_default() - .filter(|port| Some(*port) != default_port); + .filter(|port| *port != default_port); let path = match url.path() { "/" => String::new(), path => path.to_string(), diff --git a/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs b/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs index bc30541f78c..c34babe45ee 100644 --- a/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs +++ b/desktop/src/features/agents/managedAgentRuntimeStatus.test.mjs @@ -195,3 +195,37 @@ test("connectionTargetUrl folds formatting but preserves tenant hosts", () => { connectionTargetUrl("ws://127.0.0.1:3000"), ); }); + +test("connectionTargetUrl fails closed on userinfo and fragments", () => { + // normalize_relay_url rejects these; folding them away here would alias + // wss://alice@relay with wss://bob@relay. Null routes the caller to the + // exact-string fallback instead. + assert.equal(connectionTargetUrl("ws://alice@relay.example"), null); + assert.equal(connectionTargetUrl("ws://:secret@relay.example"), null); + assert.equal(connectionTargetUrl("wss://relay.example#a"), null); +}); + +test("userinfo spellings never alias through requested-URL matching", () => { + const runtimes = [ + runtime({ + relayUrl: "ws://relay.example:3000", + requestedRelayUrl: "ws://alice@relay.example:3000", + lifecycle: "ready", + }), + ]; + // Exact fallback still resolves the row that was actually dialed... + assert.equal( + findManagedAgentRuntime(runtimes, "aa", "ws://alice@relay.example:3000") + ?.lifecycle, + "ready", + ); + // ...but a different credential spelling of the same authority must not. + assert.equal( + findManagedAgentRuntime(runtimes, "aa", "ws://bob@relay.example:3000"), + undefined, + ); + assert.equal( + findManagedAgentRuntime(runtimes, "aa", "ws://relay.example:3000"), + undefined, + ); +}); diff --git a/desktop/src/features/agents/managedAgentRuntimeStatus.ts b/desktop/src/features/agents/managedAgentRuntimeStatus.ts index 0fec069dc59..afcb88b4cd6 100644 --- a/desktop/src/features/agents/managedAgentRuntimeStatus.ts +++ b/desktop/src/features/agents/managedAgentRuntimeStatus.ts @@ -97,7 +97,11 @@ export function canonicalRelayUrl(raw: string): string | null { * (buzz-core's `tenant::normalize_host`): lowercase host, strip an explicit * default port and the FQDN root dot, fold the root-path slash - WITHOUT * folding loopback spellings, which are distinct tenants on a host-scoped - * relay. Returns null when the URL cannot be parsed as ws/wss. + * relay. Returns null when the URL cannot be parsed as ws/wss — or carries + * userinfo or a fragment, which `normalize_relay_url` rejects: folding those + * away would alias distinct spellings like `wss://alice@relay.example` and + * `wss://bob@relay.example`, so they fail closed onto the caller's + * exact-string fallback instead (mirrors the Rust `connection_target`). */ export function connectionTargetUrl(raw: string): string | null { let url: URL; @@ -107,6 +111,9 @@ export function connectionTargetUrl(raw: string): string | null { return null; } if (url.protocol !== "ws:" && url.protocol !== "wss:") return null; + if (url.username !== "" || url.password !== "" || url.hash !== "") { + return null; + } const host = url.hostname.toLowerCase().replace(/\.$/, ""); const defaultPort = url.protocol === "ws:" ? "80" : "443"; const port = url.port && url.port !== defaultPort ? `:${url.port}` : "";