diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 470458237..cd5709ffb 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -754,6 +754,17 @@ pub async fn update_managed_agent( } let record = find_managed_agent_mut(&mut records, &input.pubkey)?; + // Item 2: fold the relay-config overlay onto the disk record BEFORE + // applying the user's patch, so the edit is authored on top of the + // config this device is actually following. Without this, retaining + // the raw disk record republishes every OTHER field from stale disk + // and LWW makes that the new relay head. Ordering is load-bearing: + // resolving AFTER the patch would discard the user's edit instead. + if let Ok(resolved) = + crate::managed_agents::private_config_overlay::resolved_local_record(&state, record) + { + *record = resolved; + } let previous_record = record.clone(); let mut name_changed = false; @@ -937,87 +948,9 @@ pub async fn update_managed_agent( }) } -// ── Model normalization ─────────────────────────────────────────────────────── - -/// Normalize raw `buzz-acp models --json` output into a typed DTO for the frontend. -/// -/// Merges models from both ACP paths (stable configOptions + unstable SessionModelState), -/// deduplicates by ID (stable takes precedence), and returns a unified list. -pub(super) fn normalize_agent_models( - raw: &serde_json::Value, - persisted_model: Option, -) -> AgentModelsResponse { - let agent_name = raw["agent"]["name"] - .as_str() - .unwrap_or("unknown") - .to_string(); - let agent_version = raw["agent"]["version"] - .as_str() - .unwrap_or("unknown") - .to_string(); - - let mut models: Vec = Vec::new(); - let mut seen_ids: HashSet = HashSet::new(); - - // 1. Stable configOptions (preferred). Only entries with category "model" - // are model options — the CLI pre-filters, but we're defensive here. - if let Some(config_options) = raw["stable"]["configOptions"].as_array() { - for opt in config_options { - if opt.get("category").and_then(|c| c.as_str()) != Some("model") { - continue; - } - if let Some(options) = opt.get("options").and_then(|v| v.as_array()) { - for o in options { - if let Some(value) = o.get("value").and_then(|v| v.as_str()) { - if seen_ids.insert(value.to_string()) { - models.push(AgentModelInfo { - id: value.to_string(), - name: o - .get("displayName") - .and_then(|v| v.as_str()) - .map(str::to_string), - description: None, - }); - } - } - } - } - } - } - - // 2. Unstable availableModels (fallback — skip duplicates from stable). - let mut agent_default_model: Option = None; - if let Some(unstable) = raw.get("unstable") { - agent_default_model = unstable["currentModelId"].as_str().map(str::to_string); - if let Some(available) = unstable["availableModels"].as_array() { - for m in available { - if let Some(id) = m.get("modelId").and_then(|v| v.as_str()) { - if seen_ids.insert(id.to_string()) { - models.push(AgentModelInfo { - id: id.to_string(), - name: m.get("name").and_then(|v| v.as_str()).map(str::to_string), - description: m - .get("description") - .and_then(|v| v.as_str()) - .map(str::to_string), - }); - } - } - } - } - } - - let supports_switching = !models.is_empty(); - - AgentModelsResponse { - agent_name, - agent_version, - models, - agent_default_model, - selected_model: persisted_model, - supports_switching, - } -} +#[path = "agent_models_normalize.rs"] +mod normalize; +pub(super) use normalize::normalize_agent_models; #[cfg(test)] #[path = "agent_models_tests.rs"] diff --git a/desktop/src-tauri/src/commands/agent_models_normalize.rs b/desktop/src-tauri/src/commands/agent_models_normalize.rs new file mode 100644 index 000000000..b437136b3 --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_normalize.rs @@ -0,0 +1,89 @@ +//! Normalization of raw `buzz-acp models --json` output into the frontend DTO. +//! +//! Split out of `agent_models.rs` to keep that file inside the desktop +//! file-size ratchet; it is a pure transform with no shared state, so the +//! seam is the same one the discovery/provider helpers already use. + +use std::collections::HashSet; + +use crate::managed_agents::{AgentModelInfo, AgentModelsResponse}; + +/// Normalize raw `buzz-acp models --json` output into a typed DTO for the frontend. +/// +/// Merges models from both ACP paths (stable configOptions + unstable SessionModelState), +/// deduplicates by ID (stable takes precedence), and returns a unified list. +pub(crate) fn normalize_agent_models( + raw: &serde_json::Value, + persisted_model: Option, +) -> AgentModelsResponse { + let agent_name = raw["agent"]["name"] + .as_str() + .unwrap_or("unknown") + .to_string(); + let agent_version = raw["agent"]["version"] + .as_str() + .unwrap_or("unknown") + .to_string(); + + let mut models: Vec = Vec::new(); + let mut seen_ids: HashSet = HashSet::new(); + + // 1. Stable configOptions (preferred). Only entries with category "model" + // are model options — the CLI pre-filters, but we're defensive here. + if let Some(config_options) = raw["stable"]["configOptions"].as_array() { + for opt in config_options { + if opt.get("category").and_then(|c| c.as_str()) != Some("model") { + continue; + } + if let Some(options) = opt.get("options").and_then(|v| v.as_array()) { + for o in options { + if let Some(value) = o.get("value").and_then(|v| v.as_str()) { + if seen_ids.insert(value.to_string()) { + models.push(AgentModelInfo { + id: value.to_string(), + name: o + .get("displayName") + .and_then(|v| v.as_str()) + .map(str::to_string), + description: None, + }); + } + } + } + } + } + } + + // 2. Unstable availableModels (fallback — skip duplicates from stable). + let mut agent_default_model: Option = None; + if let Some(unstable) = raw.get("unstable") { + agent_default_model = unstable["currentModelId"].as_str().map(str::to_string); + if let Some(available) = unstable["availableModels"].as_array() { + for m in available { + if let Some(id) = m.get("modelId").and_then(|v| v.as_str()) { + if seen_ids.insert(id.to_string()) { + models.push(AgentModelInfo { + id: id.to_string(), + name: m.get("name").and_then(|v| v.as_str()).map(str::to_string), + description: m + .get("description") + .and_then(|v| v.as_str()) + .map(str::to_string), + }); + } + } + } + } + } + + let supports_switching = !models.is_empty(); + + AgentModelsResponse { + agent_name, + agent_version, + models, + agent_default_model, + selected_model: persisted_model, + supports_switching, + } +} diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 88b1b8047..4a4857321 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -264,6 +264,19 @@ pub(super) async fn start_local_agent_pairs_with_preflight( .map_err(|e| e.to_string())?; let mut records = load_managed_agents(app)?; let record = find_managed_agent_mut(&mut records, pubkey)?; + // Item 2: fold the relay-config overlay on BEFORE the persona snapshot + // re-apply. Without this, retaining the saved record below republishes + // every non-quad field (parallelism, env overrides, name) from stale + // disk over a newer relay head, and LWW makes that the new head. + // Ordering is load-bearing in the other direction here: resolving + // AFTER `apply_persona_snapshot` would let the overlay clobber the + // definition quad (system_prompt/model/provider/runtime), so the + // snapshot must land last to stay definition-authoritative. + if let Ok(resolved) = + crate::managed_agents::private_config_overlay::resolved_local_record(state, record) + { + *record = resolved; + } let personas = load_personas(app).unwrap_or_default(); if let Some(persona_id) = record.persona_id.clone() { if let Some(persona) = personas.iter().find(|persona| persona.id == persona_id) { diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index 4e8c4ac2c..59ed14ab3 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -264,6 +264,77 @@ fn private_agent_inbound_rejects_before_retain_and_stale_event_preserves_overlay assert_eq!(overlay.resolved_records(&[])[0].name, "new"); } +/// SAMI PROBE: the retention DB survives a restart but the overlay does not. +/// On the next launch the backfill re-delivers the SAME event, which resolves +/// to `Skipped` against the retained row — so `insert_patch` never runs and the +/// overlay stays empty for the whole session. +#[test] +fn sami_probe_overlay_does_not_rehydrate_after_restart() { + use crate::managed_agents::{ + private_config_overlay::PrivateConfigOverlay, + retention::{open_retention_db, InboundOutcome}, + }; + use buzz_core_pkg::private_managed_agent; + use tempfile::TempDir; + + let dir = TempDir::new().unwrap(); + let db_path = dir.path().join("retention.db"); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + + let payload = private_agent_payload(&owner_keys, &agent_keys, "relay name", 4); + let event = private_managed_agent::build_event(&owner_keys, &payload, 20).unwrap(); + + // ── Session 1: event arrives, overlay hydrates. ── + { + let conn = open_retention_db(&db_path).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + assert_eq!( + apply_inbound_private_managed_agent_event(&event, &owner_keys, &conn, &mut overlay) + .unwrap(), + InboundOutcome::Applied + ); + assert_eq!( + overlay.resolved_records(&[]).len(), + 1, + "control: overlay hydrates on first arrival" + ); + } + + // ── Session 2: same DB file, fresh in-memory overlay (app restart). ── + let conn = open_retention_db(&db_path).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + let outcome = + apply_inbound_private_managed_agent_event(&event, &owner_keys, &conn, &mut overlay) + .unwrap(); + assert_eq!( + outcome, + InboundOutcome::Skipped, + "re-delivered event is deduped against the retained row" + ); + assert!( + overlay.resolved_records(&[]).is_empty(), + "DEFECT: overlay is empty after restart — relay config silently unavailable" + ); + + // ── Positive control: the probe CAN observe hydration in session 2. ── + // A strictly-newer event is the only thing that repopulates the overlay. + let mut newer = private_agent_payload(&owner_keys, &agent_keys, "newer name", 4); + newer.generation = 2; + newer.previous_event_id = Some(event.id.to_hex()); + let newer_event = private_managed_agent::build_event(&owner_keys, &newer, 30).unwrap(); + assert_eq!( + apply_inbound_private_managed_agent_event(&newer_event, &owner_keys, &conn, &mut overlay) + .unwrap(), + InboundOutcome::Applied + ); + assert_eq!( + overlay.resolved_records(&[])[0].name, + "newer name", + "positive control: this harness observes hydration when it happens" + ); +} + /// A local managed agent carrying every device-local secret that an inbound /// event must NEVER be able to overwrite. fn local_agent() -> ManagedAgentRecord { @@ -782,3 +853,52 @@ fn inbound_gate_accepts_validly_signed_event() { let parsed = parse_verified_inbound_event(&event.as_json()).unwrap(); assert_eq!(parsed.pubkey, keys.public_key()); } + +/// Item-0 FIX verification: after a "restart" (same retention db, fresh +/// overlay), `hydrate_from_retention` repopulates the overlay from the durable +/// rows — so the resolve sites see relay config instead of stale disk. +#[test] +fn sami_fix_overlay_rehydrates_from_retention_after_restart() { + use crate::managed_agents::{ + private_config_overlay::{hydrate_from_retention, PrivateConfigOverlay}, + retention::open_retention_db, + }; + use buzz_core_pkg::private_managed_agent; + use tempfile::TempDir; + + let dir = TempDir::new().unwrap(); + let db_path = dir.path().join("retention.db"); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + + let payload = private_agent_payload(&owner_keys, &agent_keys, "relay name", 4); + let event = private_managed_agent::build_event(&owner_keys, &payload, 20).unwrap(); + + // Session 1: the event lands and is retained durably. + { + let conn = open_retention_db(&db_path).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + apply_inbound_private_managed_agent_event(&event, &owner_keys, &conn, &mut overlay) + .unwrap(); + } + + // Session 2 (restart): hydrate straight from the retained rows — no + // inbound event required. + let conn = open_retention_db(&db_path).unwrap(); + let overlay = hydrate_from_retention(&conn, &owner_keys).unwrap(); + let resolved = overlay.resolved_records(&[]); + assert_eq!(resolved.len(), 1, "FIX: overlay rehydrates from retention"); + assert_eq!(resolved[0].name, "relay name"); + assert_eq!(resolved[0].parallelism, 4); + + // NEGATIVE CONTROL: a different owner's keys must hydrate NOTHING — proves + // the query is scoped by owner pubkey and not just returning every row. + let stranger = nostr::Keys::generate(); + assert!( + hydrate_from_retention(&conn, &stranger) + .unwrap() + .resolved_records(&[]) + .is_empty(), + "control: hydration is owner-scoped" + ); +} diff --git a/desktop/src-tauri/src/commands/personas/update.rs b/desktop/src-tauri/src/commands/personas/update.rs index ed2472d54..b7cbea782 100644 --- a/desktop/src-tauri/src/commands/personas/update.rs +++ b/desktop/src-tauri/src/commands/personas/update.rs @@ -211,7 +211,26 @@ pub(super) async fn update_persona_with( // Avatar-only edits are excluded — the avatar is not in the // projection, so retaining would be a guaranteed no-op. for record in records.iter().filter(|r| renamed.contains(&r.pubkey)) { - crate::commands::agents::retain_managed_agent_pending(&app, &state, record); + // Item 2: `private_payload_from_record` serializes EVERY + // config field, so retaining the raw disk record here + // republishes system_prompt/parallelism/env_vars from + // stale disk over a newer relay head. Fold the overlay + // on first, then re-apply the rename — the overlay's + // `apply` clobbers `name`, and resolving before the + // `name != old_display_name` gate above would instead + // make the rename skip records whose relay name already + // diverged. Disk stays untouched: it is the fallback, + // the relay is primary. + let mut resolved = + crate::managed_agents::private_config_overlay::resolved_local_record( + &state, record, + ) + .unwrap_or_else(|_| record.clone()); + resolved.name.clone_from(&record.name); + resolved.display_name.clone_from(&record.display_name); + crate::commands::agents::retain_managed_agent_pending( + &app, &state, &resolved, + ); } } diff --git a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs index c60215ae4..6a989a89d 100644 --- a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs @@ -158,3 +158,82 @@ fn test_rename_renames_all_matching_instances_in_one_pass() { assert_eq!(records[1].name, "Duncan Idaho"); assert_eq!(records[2].name, "Birch", "pool-named instance untouched"); } + +/// SAMI PROBE (fidelity pin for `sami_probe_rename_republishes_nonname_fields_from_stale_disk` +/// in `managed_agents/reconcile/tests.rs`): that probe hand-mutates `name` and +/// `display_name` to stand in for this helper. If the helper ever touched a +/// third field, the probe's fixture would silently stop modelling production. +/// Assert the mutation surface is EXACTLY those two fields, by diffing a +/// serialized before/after. +#[test] +fn rename_helper_mutates_only_name_and_display_name() { + let mut records = vec![agent("persona-1", "Paul", Some("Paul"))]; + records[0].system_prompt = Some("disk prompt".into()); + records[0].parallelism = 7; + let before = serde_json::to_value(&records[0]).unwrap(); + + propagate_persona_name_rename(&mut records, "persona-1", "Paul", "Paul Atreides"); + + let after = serde_json::to_value(&records[0]).unwrap(); + let changed: Vec = before + .as_object() + .unwrap() + .keys() + .chain(after.as_object().unwrap().keys()) + .filter(|key| before.get(*key) != after.get(*key)) + .cloned() + .collect::>() + .into_iter() + .collect(); + + assert_eq!( + changed, + vec!["display_name".to_string(), "name".to_string()], + "rename must mutate exactly name + display_name; a wider surface \ + invalidates the stale-disk republish probe's fixture" + ); +} + +/// SAMI PROBE (hazard in the PROPOSED fix, not in the current code): the fix +/// for the stale-disk republish is "resolve the overlay before the write". At +/// this site the write is gated on `record.name == old_display_name`, and the +/// overlay REPLACES `record.name` with the relay's name. So resolving before +/// the gate can change which records the rename reaches. +/// +/// Models a following device whose relay head carries a name that no longer +/// equals the persona's old display_name (device A already renamed, or the +/// instance is pool-named on the relay). Resolve-first makes the rename SKIP +/// the record entirely — the intended write is lost, which is the same +/// silent-data-loss class as the centralized-resolve probe. +#[test] +fn sami_probe_resolve_before_rename_can_skip_the_intended_rename() { + // Disk name matches the old persona display_name, so production renames it. + let mut disk_only = vec![agent("persona-1", "Paul", Some("Paul"))]; + let renamed = + propagate_persona_name_rename(&mut disk_only, "persona-1", "Paul", "Paul Atreides"); + assert_eq!( + renamed.len(), + 1, + "control: against the DISK name the rename fires" + ); + assert_eq!(disk_only[0].name, "Paul Atreides"); + + // Same record after the overlay resolves a relay head whose name differs + // (device A already applied the rename). `apply()` clobbers `record.name`. + let mut overlay_resolved = vec![agent("persona-1", "Paul", Some("Paul"))]; + overlay_resolved[0].name = "Paul Atreides".to_string(); // what the overlay wrote + + let renamed_after_resolve = + propagate_persona_name_rename(&mut overlay_resolved, "persona-1", "Paul", "Paul Atreides"); + + assert!( + renamed_after_resolve.is_empty(), + "resolve-before-rename makes the gate miss: the record is NOT reported \ + as renamed, so update.rs never retains it and never syncs its relay \ + profile" + ); + // Benign here (the names already agree), but the gate is now driven by + // relay state rather than disk state — so the fix must resolve for the + // PAYLOAD without moving the `name != old_display_name` decision onto the + // resolved name. +} diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index ee8e0d8b1..682977940 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -17,6 +17,45 @@ pub fn run_event_sync(app: &tauri::AppHandle, owner_keys: &nostr::Keys, db_path: migrate_personas_to_events(app, owner_keys, db_path); migrate_teams_to_events(app, owner_keys, db_path); crate::managed_agents::reconcile::reconcile_agents_to_events(app, owner_keys, db_path); + hydrate_private_config_overlay(app, owner_keys, db_path); +} + +/// Rebuild the relay-config overlay from the retained kind:30179 rows. +/// +/// Runs on the same boot seam as the disk→event reconcile but in the other +/// direction (retention→memory). Without it the overlay is empty on every +/// second-and-later launch, because the backfill's re-delivered events dedupe +/// against their own retained rows and never reach `insert_patch`. Best-effort: +/// a failure leaves the overlay empty, which is exactly today's behavior. +fn hydrate_private_config_overlay( + app: &tauri::AppHandle, + owner_keys: &nostr::Keys, + db_path: &Path, +) { + use tauri::Manager; + + let result = (|| -> Result { + let conn = crate::managed_agents::retention::open_retention_db(db_path)?; + let hydrated = crate::managed_agents::private_config_overlay::hydrate_from_retention( + &conn, owner_keys, + )?; + let count = hydrated.len(); + let state = app.state::(); + *state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())? = hydrated; + Ok(count) + })(); + match result { + Ok(0) => {} + Ok(count) => { + eprintln!( + "buzz-desktop: private-config-overlay: hydrated {count} agents from retention" + ) + } + Err(error) => eprintln!("buzz-desktop: private-config-overlay: {error}"), + } } /// Spawn the best-effort event reconcile off the synchronous Tauri setup path. diff --git a/desktop/src-tauri/src/managed_agents/private_config_overlay.rs b/desktop/src-tauri/src/managed_agents/private_config_overlay.rs index eb99810d2..68f5fa8c8 100644 --- a/desktop/src-tauri/src/managed_agents/private_config_overlay.rs +++ b/desktop/src-tauri/src/managed_agents/private_config_overlay.rs @@ -201,6 +201,10 @@ impl PrivateConfigOverlay { self.0.insert(patch.pubkey.clone(), patch); } + pub(crate) fn len(&self) -> usize { + self.0.len() + } + pub(crate) fn clear(&mut self) { self.0.clear(); } @@ -432,3 +436,125 @@ mod tests { ); } } + +/// Rebuild the in-memory overlay from the retained kind:30179 rows. +/// +/// The inbound path only calls `insert_patch` when `retain_inbound_event` +/// returns `Applied`, i.e. when the event is STRICTLY newer than the retained +/// row. After a restart the backfill re-delivers the same events, retention +/// dedupes them to `Skipped`, and the overlay would stay empty for the whole +/// session — every resolve site silently falling back to stale disk config. +/// Hydrating from the durable rows at boot makes relay-primary config survive +/// a restart. +/// +/// Best-effort per row: a row that fails to parse, decrypt, or validate is +/// skipped rather than failing the boot, matching the inbound path's +/// per-record reject. +pub(crate) fn hydrate_from_retention( + conn: &rusqlite::Connection, + owner_keys: &nostr::Keys, +) -> Result { + use buzz_core_pkg::{kind::KIND_PRIVATE_MANAGED_AGENT, private_managed_agent}; + use nostr::JsonUtil; + + let rows = crate::managed_agents::retention::get_retained_events_of_kind( + conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + )?; + + let mut overlay = PrivateConfigOverlay::default(); + for row in rows { + let Ok(event) = nostr::Event::from_json(&row.raw_event) else { + continue; + }; + let Ok((_, payload)) = private_managed_agent::validate_and_decrypt(&event, owner_keys) + else { + continue; + }; + if let Ok(patch) = PrivateConfigPatch::from_payload(payload) { + overlay.insert_patch(patch); + } + } + Ok(overlay) +} + +/// Guards that each known stale-disk-republish write site actually calls the +/// overlay resolve. The behavioural tests for these sites (in +/// `reconcile/tests.rs` and `personas/update/name_propagation_tests.rs`) can +/// only *model* the ordering: every site is inside a `#[tauri::command]` that +/// needs a live `AppHandle`, so they call `retain_agent_record` directly and +/// stay green even when the production call is deleted. Measured, not assumed: +/// removing the resolve from `agent_models.rs` left the full lib suite at +/// 2261 passed / 0 failed. This module is the only thing that fails when a +/// site loses its resolve — or when a NEW site is added without one. +/// +/// A source assertion is a weak instrument (it cannot see ordering, only +/// presence), so it is deliberately paired with the behavioural ordering tests +/// rather than replacing them. It exists because the alternative here is no +/// coverage at all. +#[cfg(test)] +mod write_site_resolve_guard { + /// `(file, source, expected_resolve_calls)` — every write site that + /// retains a managed-agent record derived from disk. + fn sites() -> Vec<(&'static str, &'static str, usize)> { + vec![ + ( + "commands/agent_models.rs", + include_str!("../commands/agent_models.rs"), + 1, + ), + // 4 = the 3 sites Carl already resolved correctly (start/stop/ + // delete, ~:999/:1069/:1149) plus the pair-start snapshot re-apply + // fixed here (~:276). The count is deliberately exact rather than + // `>= 1`: a lower bound would not notice a site losing its resolve + // while another gained one. + ( + "commands/agents.rs", + include_str!("../commands/agents.rs"), + 4, + ), + ( + "commands/personas/update.rs", + include_str!("../commands/personas/update.rs"), + 1, + ), + ] + } + + #[test] + fn every_stale_republish_write_site_resolves_the_overlay() { + for (file, source, expected) in sites() { + let found = source.matches("resolved_local_record(").count(); + assert_eq!( + found, expected, + "{file}: expected {expected} `resolved_local_record(` call(s), found {found}. \ + A write site that retains a disk-derived record without resolving the \ + relay overlay republishes stale config over a newer relay head as a \ + validly-chained successor event (see \ + `sami_probe_2b_stale_disk_republish_over_newer_relay_head`)." + ); + } + } + + /// The guard above is a substring count, so prove it can FAIL: a source + /// with the call removed must not satisfy it. Without this, a typo in the + /// searched string would make every row vacuously pass. + #[test] + fn guard_detects_a_missing_resolve_call() { + for (file, source, _) in sites() { + let stripped = source.replace("resolved_local_record(", "REMOVED("); + assert_eq!( + stripped.matches("resolved_local_record(").count(), + 0, + "{file}: negative control — the guard's search string must actually \ + match the production call, or the guard is vacuous" + ); + assert_ne!( + source.matches("resolved_local_record(").count(), + 0, + "{file}: positive control — the search string must be present at HEAD" + ); + } + } +} diff --git a/desktop/src-tauri/src/managed_agents/reconcile.rs b/desktop/src-tauri/src/managed_agents/reconcile.rs index 7d110c056..8a47aff6a 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile.rs @@ -60,13 +60,14 @@ pub(crate) fn reconcile_agents_to_events( /// Core reconcile logic, decoupled from the Tauri `AppHandle` for testing. /// -/// Reads `managed-agents.json` raw — no keyring hydration: the published +/// Reads `managed-agents.json` and hydrates keys from the keyring: the 30177 /// projection ([`super::agent_events::agent_event_content`]) is the opt-IN -/// no-secrets allowlist, so keys are never needed here. For each record it -/// compares the freshly built event's content against the retained row at -/// `(30177, owner, agent_pubkey)` and re-retains (marking `pending_sync = 1`) -/// only when the row is absent or its content differs — an unchanged agent -/// never churns `pending_sync`. +/// no-secrets allowlist and needs no keys, but the 30179 private-config +/// projection carries the agent nsec, which is keyring-resident on a default +/// build. For each record it compares the freshly built event's content against +/// the retained row at `(30177, owner, agent_pubkey)` and re-retains (marking +/// `pending_sync = 1`) only when the row is absent or its content differs — an +/// unchanged agent never churns `pending_sync`. /// /// Returns the number of agents (re)written to the retention store. #[cfg(test)] @@ -87,7 +88,7 @@ fn reconcile_agents_in_dir_at( let content = std::fs::read_to_string(&store_path) .map_err(|e| format!("failed to read managed-agents.json: {e}"))?; - let records: Vec = serde_json::from_str(&content).map_err(|e| { + let mut records: Vec = serde_json::from_str(&content).map_err(|e| { super::storage::backup_invalid_store(&store_path); format!("failed to parse managed-agents.json (preserved as .invalid): {e}") })?; @@ -96,6 +97,12 @@ fn reconcile_agents_in_dir_at( return Ok(0); } + // The 30179 private-config projection carries the agent nsec, which on a + // default `system-keyring` build lives in the keyring and NOT in the JSON. + // Without this, `retain_private_agent_record`'s empty-nsec skip fires for + // every untouched agent and boot reconcile publishes zero 30179s. + super::storage::hydrate_keys(&mut records); + let conn = open_retention_db(db_path).map_err(|e| format!("failed to open retention db: {e}"))?; diff --git a/desktop/src-tauri/src/managed_agents/reconcile/tests.rs b/desktop/src-tauri/src/managed_agents/reconcile/tests.rs index 69a03af3e..891afd53b 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile/tests.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests.rs @@ -554,3 +554,5 @@ fn retain_agent_record_is_noop_when_unchanged() { "no pending_sync churn for an unchanged record" ); } + +mod stale_republish_tests; diff --git a/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs b/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs new file mode 100644 index 000000000..a0e8499bd --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs @@ -0,0 +1,704 @@ +//! Regression coverage for the stale-disk republish class (review item 2) and +//! the overlay-resolution ordering at each write site. +//! +//! Split out of `tests.rs` to stay inside the desktop file-size ratchet. These +//! share the parent module's fixtures (`sample_record`, `write_store`) via +//! `use super::*`, so they stay one `cargo test` away from the engine they pin. + +use super::*; + +/// SAMI PROBE (2b): device B holds a NEWER relay head in retention (inbound, +/// pending_sync=0). A local edit then rebuilds the payload from the STALE disk +/// record and retains it. Does the projection-equality guard or LWW stop the +/// stale fields from becoming the new relay head? +#[test] +fn sami_probe_2b_stale_disk_republish_over_newer_relay_head() { + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + // Device B's disk record: stale on two fields. + let mut disk = sample_record(&pubkey, "stale-disk-name"); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + disk.system_prompt = Some("STALE disk prompt".into()); + disk.parallelism = 1; + + // Inbound relay head from device A: fresher config, gen 5, far-future + // created_at so LWW clearly favors it. + let mut fresh = disk.clone(); + fresh.name = "FRESH relay name".into(); + fresh.system_prompt = Some("FRESH relay prompt".into()); + fresh.parallelism = 16; + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + let head_payload = + private_payload_from_record(&fresh, &owner_hex, 5, Some("aa".repeat(32))).unwrap(); + let head_event = + private_managed_agent::build_event(&owner_keys, &head_payload, head_created_at as u64) + .unwrap(); + // Exactly what the inbound path writes: pending_sync = 0. + crate::managed_agents::retention::retain_inbound_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + + // A local edit on device B: update_managed_agent's tail — retain the DISK + // record. (`update_managed_agent` passes the just-saved disk record.) + let changed = retain_agent_record(&conn, &owner_keys, &disk).unwrap(); + + let row = get_retained_event(&conn, KIND_PRIVATE_MANAGED_AGENT, &owner_hex, &pubkey) + .unwrap() + .unwrap(); + let (_, republished) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + + // DEFECT: every stale disk field became the new relay head. + assert!(changed, "retain reported a change"); + assert_eq!(republished.config.name, "stale-disk-name"); + assert_eq!( + republished.config.system_prompt, + Some("STALE disk prompt".into()) + ); + assert_eq!(republished.config.parallelism, Some(1)); + // ...and it WINS LWW: monotonic_created_at bumped past the fresher head. + assert!( + row.created_at > head_created_at, + "stale republish must outrank the fresher head for the defect to matter" + ); + // ...and it is a VALIDLY CHAINED successor (gen 5 -> 6, prev = head id), + // so no peer can distinguish it from a legitimate edit. + assert_eq!(republished.generation, 6); + assert_eq!( + republished.previous_event_id, + Some(head_event.id.to_hex()), + "stale event chains cleanly off the head it clobbers" + ); + // ...and it is queued for publish, not merely local. + assert!( + get_pending_sync(&conn) + .unwrap() + .iter() + .any(|event| event.kind == KIND_PRIVATE_MANAGED_AGENT), + "the stale 30179 is enqueued for relay publish" + ); + + // POSITIVE CONTROL: the guard this probe claims is bypassed DOES fire when + // the input matches the head — proving the probe observes a real bypass and + // not a guard that never no-ops. Run against a PRISTINE head (a second db), + // because the stale write above already replaced the head in `conn`. Compare + // the private ROW, not `retain_agent_record`'s bool: that bool is + // `public_changed || private_changed`, so the fresh db's absent 30177 row + // would mask the private no-op. + let control_conn = open_retention_db(&dir.path().join("control.db")).unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &control_conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + retain_agent_record(&control_conn, &owner_keys, &fresh).unwrap(); + let control_row = get_retained_event( + &control_conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_hex, + &pubkey, + ) + .unwrap() + .unwrap(); + assert_eq!( + control_row.raw_event, + head_event.as_json(), + "control: retaining the RESOLVED (relay-fresh) record against the same \ + head leaves the head untouched — so the defect above is the stale \ + input, not a guard that never fires" + ); +} + +/// SAMI PROBE (item 2 cost): would the CHEAP fix — resolve the overlay once +/// inside `retain_managed_agent_pending` instead of at each call site — be +/// correct? Simulates that fix at the EDIT site: user edits one field on disk, +/// helper resolves the overlay on top, then retains. +#[test] +fn sami_probe_resolve_in_helper_would_discard_user_edits() { + use crate::managed_agents::private_config_overlay::{PrivateConfigOverlay, PrivateConfigPatch}; + + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + + // Relay head the overlay is following: parallelism 16, relay prompt. + let mut relay = sample_record(&pubkey, "relay-name"); + relay.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + relay.system_prompt = Some("relay prompt".into()); + relay.parallelism = 16; + let head_payload = private_payload_from_record(&relay, &owner_hex, 1, None).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert_patch(PrivateConfigPatch::from_payload(head_payload).unwrap()); + + // The user edits ONE field locally: parallelism 16 -> 2. This is the + // just-saved disk record `update_managed_agent` passes to the helper. + let mut edited = relay.clone(); + edited.parallelism = 2; + + // The "cheap fix": helper resolves the overlay onto the record it was + // handed, then retains that. + let resolved = overlay.resolve_local_record(&edited); + + assert_eq!( + resolved.parallelism, 16, + "the cheap one-place fix SILENTLY DISCARDS the user's edit: \ + parallelism went back to the relay's 16, not the edited 2" + ); + // Positive control: with no patch for this agent the edit survives, so the + // discard above is the overlay winning, not a broken fixture. + let empty = PrivateConfigOverlay::default(); + assert_eq!( + empty.resolve_local_record(&edited).parallelism, + 2, + "control: without an overlay patch the edit survives" + ); +} + +/// SAMI PROBE (settles Eva's contested third site, `personas/update.rs:214`): +/// on a device following a NEWER relay head, does a persona RENAME republish +/// the non-name config fields from stale disk? +/// +/// Eva traced that `private_payload_from_record` serializes every config field +/// from the disk record, so a name-only mutation should still clobber +/// system_prompt / parallelism / env_vars. I traced it as scoped-out ("folding +/// the overlay would fight the intended write"). Measured here rather than +/// argued: the rename mutates ONLY `name`/`display_name` (pinned faithful by +/// `rename_helper_mutates_only_name_and_display_name` in +/// `commands/personas/update/name_propagation_tests.rs`), then the retain fires +/// exactly as `update.rs:214` fires it. +#[test] +fn sami_probe_rename_republishes_nonname_fields_from_stale_disk() { + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + // Device B's disk record, stale on the NON-name fields. Its `name` still + // equals the old persona display_name, which is what makes the rename + // propagate to it at all. + let mut disk = sample_record(&pubkey, "Paul"); + disk.display_name = Some("Paul".into()); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + disk.system_prompt = Some("STALE disk prompt".into()); + disk.parallelism = 1; + disk.env_vars + .insert("STALE_KEY".into(), "stale-value".into()); + + // Device A's newer relay head: same agent, fresher non-name config. + let mut fresh = disk.clone(); + fresh.system_prompt = Some("FRESH relay prompt".into()); + fresh.parallelism = 16; + fresh.env_vars.clear(); + fresh + .env_vars + .insert("FRESH_KEY".into(), "fresh-value".into()); + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + let head_payload = + private_payload_from_record(&fresh, &owner_hex, 5, Some("aa".repeat(32))).unwrap(); + let head_event = + private_managed_agent::build_event(&owner_keys, &head_payload, head_created_at as u64) + .unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + + // The rename: `propagate_persona_name_rename` mutates name + display_name + // and NOTHING else, then `update.rs:214` retains that disk record. + let mut renamed = disk.clone(); + renamed.name = "Paul Atreides".into(); + renamed.display_name = Some("Paul Atreides".into()); + retain_agent_record(&conn, &owner_keys, &renamed).unwrap(); + + let row = get_retained_event(&conn, KIND_PRIVATE_MANAGED_AGENT, &owner_hex, &pubkey) + .unwrap() + .unwrap(); + let (_, republished) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + + // The intended write DID land. + assert_eq!(republished.config.name, "Paul Atreides"); + // EVA IS RIGHT: the non-name fields came back from STALE DISK, not the head. + assert_eq!( + republished.config.system_prompt, + Some("STALE disk prompt".into()), + "rename republished the stale disk prompt over the fresher relay head" + ); + assert_eq!(republished.config.parallelism, Some(1)); + assert_eq!( + republished + .config + .env_vars + .get("STALE_KEY") + .map(String::as_str), + Some("stale-value"), + "stale env var resurrected" + ); + assert!( + !republished.config.env_vars.contains_key("FRESH_KEY"), + "the head's env var was DROPPED, so this is a replace not a merge" + ); + // ...and it outranks the fresher head, chained cleanly: same clobber class + // as `agent_models.rs:867`. + assert!(row.created_at > head_created_at); + assert_eq!(republished.generation, 6); + assert_eq!(republished.previous_event_id, Some(head_event.id.to_hex())); + + // POSITIVE CONTROL: a rename applied to the RESOLVED (relay-fresh) record + // preserves every non-name field, so the defect above is the stale input, + // not something inherent to renaming. Pristine head in a second db, and + // compare the private ROW bytes (retain_agent_record's bool is + // public_changed || private_changed, so a fresh db's absent 30177 row makes + // it true regardless of the private outcome). + let control_conn = open_retention_db(&dir.path().join("control.db")).unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &control_conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + let mut resolved_then_renamed = fresh.clone(); + resolved_then_renamed.name = "Paul Atreides".into(); + resolved_then_renamed.display_name = Some("Paul Atreides".into()); + retain_agent_record(&control_conn, &owner_keys, &resolved_then_renamed).unwrap(); + let control_row = get_retained_event( + &control_conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_hex, + &pubkey, + ) + .unwrap() + .unwrap(); + let (_, control) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&control_row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + assert_eq!( + control.config.name, "Paul Atreides", + "control: rename landed" + ); + assert_eq!( + control.config.system_prompt, + Some("FRESH relay prompt".into()), + "control: resolve-then-rename preserves the head's prompt" + ); + assert_eq!(control.config.parallelism, Some(16)); + assert_eq!( + control.config.env_vars.get("FRESH_KEY").map(String::as_str), + Some("fresh-value"), + "control: resolve-then-rename preserves the head's env vars" + ); +} + +/// SAMI FIX VERIFICATION for the rename site (`personas/update.rs:214`): +/// the shipped shape is resolve-overlay → re-apply name/display_name → retain. +/// Assert that on the SAME fixture that produces the clobber above, this +/// ordering republishes the head's non-name fields while still landing the +/// rename. Mirrors the production expression exactly (`resolved.name` and +/// `resolved.display_name` re-copied from the renamed disk record). +#[test] +fn sami_fix_rename_over_resolved_record_preserves_relay_fields() { + use crate::managed_agents::private_config_overlay::{PrivateConfigOverlay, PrivateConfigPatch}; + + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + let mut disk = sample_record(&pubkey, "Paul"); + disk.display_name = Some("Paul".into()); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + disk.system_prompt = Some("STALE disk prompt".into()); + disk.parallelism = 1; + disk.env_vars + .insert("STALE_KEY".into(), "stale-value".into()); + + let mut fresh = disk.clone(); + fresh.system_prompt = Some("FRESH relay prompt".into()); + fresh.parallelism = 16; + fresh.env_vars.clear(); + fresh + .env_vars + .insert("FRESH_KEY".into(), "fresh-value".into()); + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + let head_payload = + private_payload_from_record(&fresh, &owner_hex, 5, Some("aa".repeat(32))).unwrap(); + let head_event = + private_managed_agent::build_event(&owner_keys, &head_payload, head_created_at as u64) + .unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + + // The overlay this device is following (what boot hydration installs). + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert_patch(PrivateConfigPatch::from_payload(head_payload).unwrap()); + + // Production's renamed disk record... + let mut renamed = disk.clone(); + renamed.name = "Paul Atreides".into(); + renamed.display_name = Some("Paul Atreides".into()); + // ...then the FIX: resolve, re-apply the rename, retain. + let mut resolved = overlay.resolve_local_record(&renamed); + resolved.name.clone_from(&renamed.name); + resolved.display_name.clone_from(&renamed.display_name); + retain_agent_record(&conn, &owner_keys, &resolved).unwrap(); + + let row = get_retained_event(&conn, KIND_PRIVATE_MANAGED_AGENT, &owner_hex, &pubkey) + .unwrap() + .unwrap(); + let (_, published) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + + // The rename still lands (the fix must not eat the intended write). + assert_eq!(published.config.name, "Paul Atreides"); + // ...and every non-name field is now the RELAY head's, not stale disk. + assert_eq!( + published.config.system_prompt, + Some("FRESH relay prompt".into()), + "fix: the head's prompt survives the rename" + ); + assert_eq!(published.config.parallelism, Some(16)); + assert_eq!( + published + .config + .env_vars + .get("FRESH_KEY") + .map(String::as_str), + Some("fresh-value"), + "fix: the head's env var survives" + ); + assert!( + !published.config.env_vars.contains_key("STALE_KEY"), + "fix: the stale disk env var is NOT resurrected" + ); + + // NEGATIVE CONTROL: with no overlay patch (e.g. pre-hydration, or an agent + // the relay has never described) the same code path must fall through to + // the disk record unchanged — the fix must not blank config on a device + // that legitimately has no relay head to follow. + let empty = PrivateConfigOverlay::default(); + let mut fallback = empty.resolve_local_record(&renamed); + fallback.name.clone_from(&renamed.name); + fallback.display_name.clone_from(&renamed.display_name); + assert_eq!( + fallback.system_prompt, + Some("STALE disk prompt".into()), + "control: with no patch the disk value is preserved, not cleared" + ); + assert_eq!(fallback.parallelism, 1); + assert_eq!( + fallback.name, "Paul Atreides", + "control: rename still lands" + ); +} + +/// EVA PROBE (item 2, third-party audit of `agents.rs:276`): the +/// persona-snapshot re-apply in `start_local_agent_pairs_with_preflight` +/// loads the DISK record, calls `apply_persona_snapshot` (which overwrites +/// only the definition quad: system_prompt/model/provider/runtime), saves, +/// and retains. On a device following a NEWER relay head, do the NON-quad +/// fields (parallelism, env overrides, name...) republish from stale disk? +#[test] +fn eva_probe_pair_start_snapshot_reapply_republishes_stale_nonquad_fields() { + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + // Device B's disk record: stale on non-quad fields. + let mut disk = sample_record(&pubkey, "stale-disk-name"); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + disk.parallelism = 1; + disk.env_vars = BTreeMap::from([("STALE_KEY".to_string(), "stale".to_string())]); + disk.persona_id = Some("test-persona".to_string()); + + // Fresher relay head from device A: gen 5, future created_at. + let mut fresh = disk.clone(); + fresh.name = "FRESH relay name".into(); + fresh.parallelism = 16; + fresh.env_vars = BTreeMap::from([("FRESH_KEY".to_string(), "fresh".to_string())]); + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + let head_payload = + private_payload_from_record(&fresh, &owner_hex, 5, Some("aa".repeat(32))).unwrap(); + let head_event = + private_managed_agent::build_event(&owner_keys, &head_payload, head_created_at as u64) + .unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &crate::managed_agents::retention::RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + + // The site's exact sequence (agents.rs:268-277): persona snapshot applied + // to the DISK record, then retain. The snapshot only touches the quad. + let persona = crate::managed_agents::AgentDefinition { + id: "test-persona".to_string(), + display_name: "Test Persona".to_string(), + avatar_url: None, + system_prompt: "Persona prompt.".to_string(), + runtime: Some("goose".to_string()), + model: Some("claude-opus-4".to_string()), + provider: Some("anthropic".to_string()), + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + parallelism: None, + created_at: "2025-01-01T00:00:00Z".to_string(), + updated_at: "2025-01-01T00:00:00Z".to_string(), + }; + let mut site_record = disk.clone(); + crate::managed_agents::persona_events::apply_persona_snapshot(&mut site_record, &persona); + let changed = retain_agent_record(&conn, &owner_keys, &site_record).unwrap(); + + let row = get_retained_event(&conn, KIND_PRIVATE_MANAGED_AGENT, &owner_hex, &pubkey) + .unwrap() + .unwrap(); + let (_, republished) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + + // DEFECT (if these pass): non-quad stale fields became the new head. + assert!(changed, "retain reported a change"); + assert_eq!(republished.config.name, "stale-disk-name"); + assert_eq!(republished.config.parallelism, Some(1)); + assert!( + republished.config.env_vars.contains_key("STALE_KEY"), + "stale env override resurrected" + ); + assert!( + !republished.config.env_vars.contains_key("FRESH_KEY"), + "head's env dropped — replace, not merge" + ); + assert!( + row.created_at > head_created_at, + "stale write outranks head" + ); + assert_eq!(republished.generation, 6, "validly chained gen bump"); + assert_eq!( + republished.previous_event_id, + Some(head_event.id.to_hex()), + "chains cleanly off the head it clobbers" + ); +} + +/// SAMI FIX VERIFICATION for `agents.rs:276` (red-first probe above is Eva's). +/// The shipped shape is resolve-overlay → `apply_persona_snapshot` → retain. +/// Both halves of that ordering are asserted, because each direction has its +/// own failure mode: +/// * resolve BEFORE the snapshot → non-quad fields come from the relay head +/// (fixes the stale republish), and +/// * snapshot AFTER the resolve → the definition quad stays +/// definition-authoritative rather than being clobbered by the overlay. +/// +/// A test asserting only the first half would pass with the calls in the wrong +/// order, since the overlay also carries system_prompt/model/provider/runtime. +#[test] +fn sami_fix_pair_start_resolve_then_snapshot_keeps_quad_definition_authoritative() { + use crate::managed_agents::private_config_overlay::{PrivateConfigOverlay, PrivateConfigPatch}; + + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + let mut disk = sample_record(&pubkey, "stale-disk-name"); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + disk.parallelism = 1; + disk.env_vars = BTreeMap::from([("STALE_KEY".to_string(), "stale".to_string())]); + disk.persona_id = Some("test-persona".to_string()); + + // Relay head: fresher non-quad fields AND a quad the persona disagrees + // with, so the two halves of the ordering are separable. + let mut fresh = disk.clone(); + fresh.name = "FRESH relay name".into(); + fresh.parallelism = 16; + fresh.env_vars = BTreeMap::from([("FRESH_KEY".to_string(), "fresh".to_string())]); + fresh.system_prompt = Some("RELAY prompt (must lose to the persona)".into()); + fresh.model = Some("relay-model".into()); + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + let head_payload = + private_payload_from_record(&fresh, &owner_hex, 5, Some("aa".repeat(32))).unwrap(); + let head_event = + private_managed_agent::build_event(&owner_keys, &head_payload, head_created_at as u64) + .unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &crate::managed_agents::retention::RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: head_event.content.clone(), + created_at: head_created_at, + raw_event: head_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert_patch(PrivateConfigPatch::from_payload(head_payload).unwrap()); + + let persona = crate::managed_agents::AgentDefinition { + id: "test-persona".to_string(), + display_name: "Test Persona".to_string(), + avatar_url: None, + system_prompt: "PERSONA prompt.".to_string(), + runtime: Some("goose".to_string()), + model: Some("persona-model".to_string()), + provider: Some("anthropic".to_string()), + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + parallelism: None, + created_at: "2025-01-01T00:00:00Z".to_string(), + updated_at: "2025-01-01T00:00:00Z".to_string(), + }; + + // The FIXED site sequence: resolve, then snapshot, then retain. + let mut site_record = overlay.resolve_local_record(&disk); + crate::managed_agents::persona_events::apply_persona_snapshot(&mut site_record, &persona); + retain_agent_record(&conn, &owner_keys, &site_record).unwrap(); + + let row = get_retained_event(&conn, KIND_PRIVATE_MANAGED_AGENT, &owner_hex, &pubkey) + .unwrap() + .unwrap(); + let (_, published) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + + // HALF 1 — resolve-before: non-quad fields are the RELAY head's, not stale disk. + assert_eq!(published.config.name, "FRESH relay name"); + assert_eq!(published.config.parallelism, Some(16)); + assert!( + published.config.env_vars.contains_key("FRESH_KEY"), + "head's env override survives" + ); + assert!( + !published.config.env_vars.contains_key("STALE_KEY"), + "stale disk env override is NOT resurrected" + ); + + // HALF 2 — snapshot-after: the definition quad is the PERSONA's, not the + // overlay's. This is the assertion that fails if the two calls are swapped. + assert_eq!( + published.config.system_prompt, + Some("PERSONA prompt.".into()), + "definition quad stays definition-authoritative after the resolve" + ); + assert_eq!(published.config.model, Some("persona-model".into())); + + // NEGATIVE CONTROL: with no overlay patch the site must fall through to the + // disk record — the fix must not blank config on a device that has no relay + // head to follow. + let empty = PrivateConfigOverlay::default(); + let mut fallback = empty.resolve_local_record(&disk); + crate::managed_agents::persona_events::apply_persona_snapshot(&mut fallback, &persona); + assert_eq!( + fallback.parallelism, 1, + "control: with no patch the disk value is preserved, not cleared" + ); + assert!( + fallback.env_vars.contains_key("STALE_KEY"), + "control: disk env override preserved when there is no relay head" + ); + assert_eq!( + fallback.system_prompt, + Some("PERSONA prompt.".into()), + "control: quad still definition-authoritative" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/retention.rs b/desktop/src-tauri/src/managed_agents/retention.rs index 7e97fa1f5..50cfaf8a0 100644 --- a/desktop/src-tauri/src/managed_agents/retention.rs +++ b/desktop/src-tauri/src/managed_agents/retention.rs @@ -432,6 +432,43 @@ pub fn has_retained_personas(conn: &Connection, pubkey: &str) -> Result Result, String> { + let mut stmt = conn + .prepare( + "SELECT kind, pubkey, d_tag, content, created_at, raw_event, pending_sync + FROM persona_events + WHERE kind = ?1 AND pubkey = ?2 + ORDER BY d_tag", + ) + .map_err(|e| format!("failed to prepare retained-kind query: {e}"))?; + + let rows = stmt + .query_map(params![kind, pubkey], |row| { + Ok(RetainedEvent { + kind: row.get(0)?, + pubkey: row.get(1)?, + d_tag: row.get(2)?, + content: row.get(3)?, + created_at: row.get(4)?, + raw_event: row.get(5)?, + pending_sync: row.get::<_, i32>(6)? != 0, + }) + }) + .map_err(|e| format!("failed to query retained events by kind: {e}"))?; + + rows.collect::, _>>() + .map_err(|e| format!("failed to read retained event row: {e}")) +} + /// Look up a single retained event by its coordinate. pub fn get_retained_event( conn: &Connection, diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 652bb9b9e..c789c59c5 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -302,7 +302,7 @@ pub(crate) fn backup_invalid_store(path: &Path) { /// writes clean JSON and plaintext stops lingering on disk; if still /// unreachable, leave it inline. This makes the strip deterministic on the /// next reachable boot rather than waiting for a non-deterministic save. -fn hydrate_keys(records: &mut [ManagedAgentRecord]) { +pub(crate) fn hydrate_keys(records: &mut [ManagedAgentRecord]) { let Some(store) = agent_secret_store() else { return; };