diff --git a/crates/buzz-core/src/private_managed_agent.rs b/crates/buzz-core/src/private_managed_agent.rs index 180dd6fa0c3..11fd681508c 100644 --- a/crates/buzz-core/src/private_managed_agent.rs +++ b/crates/buzz-core/src/private_managed_agent.rs @@ -1,8 +1,8 @@ //! NIP-PMA private managed-agent wire codec. //! -//! This module defines and validates the inert wire format only. Relays must -//! not accept [`KIND_PRIVATE_MANAGED_AGENT`](crate::kind::KIND_PRIVATE_MANAGED_AGENT) -//! until the dedicated privacy and aggregate-CAS transactions are deployed. +//! This module defines and validates the owner-authored encrypted wire format. +//! Relays treat it as global owner data; Desktop performs all decryption and +//! device-specific runtime validation. use std::collections::{BTreeMap, HashSet}; use std::fmt; @@ -18,7 +18,7 @@ use serde_json::Value; use sha2::{Digest, Sha256}; use thiserror::Error; -use crate::kind::{KIND_MANAGED_AGENT, KIND_PERSONA, KIND_PRIVATE_MANAGED_AGENT}; +use crate::kind::KIND_PRIVATE_MANAGED_AGENT; /// Wire-format discriminator for decrypted private managed-agent payloads. pub const FORMAT: &str = "buzz-private-managed-agent"; @@ -63,65 +63,6 @@ pub enum Error { Sign, } -/// Authoritative lifecycle state repeated in the outer tags and ciphertext. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "lowercase")] -pub enum State { - /// Runnable aggregate. - Active, - /// Anti-resurrection tombstone. - Deleted, -} - -impl State { - fn as_str(self) -> &'static str { - match self { - Self::Active => "active", - Self::Deleted => "deleted", - } - } -} - -/// Versioned signed-event recovery material for a bound public projection. -/// -/// Retaining the complete signed event makes reconstruction unambiguous: its -/// signature, ID, author, kind, coordinate, and exact content bytes can all be -/// checked without trusting replaceable-event history. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct ProjectionRecoveryV1 { - /// Recovery schema version. Version 1 stores one complete signed event. - pub version: u32, - /// Exact signed public projection event. - pub signed_event: Event, -} - -/// Complete definition projection binding and recovery material. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct DefinitionBinding { - /// CAS-managed definition revision pinned by this aggregate. - pub revision: u64, - /// Exact signed kind:30175 event ID. - pub event_id: String, - /// Lowercase SHA-256 of the exact projection content bytes. - pub content_sha256: String, - /// Versioned signed event sufficient to reproduce the projection. - pub recovery: ProjectionRecoveryV1, -} - -/// Complete kind:30177 projection binding and recovery material. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct InstanceBinding { - /// Exact signed kind:30177 event ID. - pub event_id: String, - /// Lowercase SHA-256 of the exact projection content bytes. - pub content_sha256: String, - /// Versioned signed event sufficient to reproduce the projection. - pub recovery: ProjectionRecoveryV1, -} - /// Secret agent identity material. It never appears in public projections. #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] @@ -144,14 +85,46 @@ impl fmt::Debug for PrivateIdentity { } /// Portable private runnable configuration. +/// +/// Forward-compatible: unknown JSON members authored by a newer Desktop are +/// preserved verbatim in [`PrivateConfig::extra`] rather than rejected, so an +/// older writer round-tripping this config cannot silently drop them. Known +/// members are still strictly typed; unknown members can never override a +/// known field (serde routes a matching key to the typed field first). #[derive(Clone, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] pub struct PrivateConfig { - /// Explicit kind:30175 coordinate, when definition-backed. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub definition_coordinate: Option, /// Intended relay endpoint; validated again on each device before use. pub relay_url: String, + /// Unique agent handle (`ManagedAgentRecord.name`). Required for fresh-device + /// reconstruction. Non-empty. + pub name: String, + /// Stable definition/persona slug (`ManagedAgentRecord.persona_id`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub persona_id: Option, + /// Preferred ACP runtime id, e.g. `"goose"`/`"claude"`. `None` = inherit. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub runtime: Option, + /// Desired LLM model id. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub model: Option, + /// LLM inference provider. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider: Option, + /// System prompt. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub system_prompt: Option, + /// Turn parallelism. `None` = the Desktop default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parallelism: Option, + /// Inbound author gate mode as the NIP-AP wire string + /// (`"owner-only"`/`"allowlist"`/`"anyone"`). Wire string, not the Desktop + /// `RespondTo` enum, so unknown future modes round-trip verbatim. `None` = + /// the Desktop default. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub respond_to: Option, + /// Allowlist used when `respond_to == "allowlist"`; normalized lowercase hex. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub respond_to_allowlist: Vec, /// Explicit harness override; never launched without local validation. #[serde(default, skip_serializing_if = "Option::is_none")] pub agent_command_override: Option, @@ -181,6 +154,14 @@ pub struct PrivateConfig { /// Versioned provider/definition relay-mesh marker. #[serde(default, skip_serializing_if = "Option::is_none")] pub relay_mesh: Option, + /// Unknown JSON members preserved verbatim for forward compatibility. + /// + /// A newer Desktop may author config keys this version does not model; they + /// round-trip here untouched so an older writer never drops them. Never + /// contains a key that collides with a known field above (serde binds known + /// keys first). Core semantics must never depend on this map. + #[serde(flatten)] + pub extra: serde_json::Map, } impl fmt::Debug for PrivateConfig { @@ -192,23 +173,13 @@ impl fmt::Debug for PrivateConfig { } } -/// Fields present only when [`Payload::state`] is [`State::Active`]. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct ActivePayload { - /// Exact definition projection binding. - pub definition: DefinitionBinding, - /// Exact public instance projection binding. - pub instance_projection: InstanceBinding, - /// Secret identity material. - pub identity: PrivateIdentity, - /// Private portable/device-validated configuration. - pub config: PrivateConfig, -} - /// Decrypted private managed-agent payload. +/// +/// Forward-compatible at the top level: unknown JSON members authored by a +/// newer Desktop round-trip verbatim in [`Payload::extra`] (see [`PrivateConfig`] +/// for the same guarantee on config). Known members remain strictly typed and +/// validated; an unknown member can never override a known field. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] pub struct Payload { /// Always [`FORMAT`]. pub format: String, @@ -218,24 +189,27 @@ pub struct Payload { pub agent_pubkey: String, /// Owner pubkey and signed event author. pub owner_pubkey: String, - /// Monotonic CAS generation. + /// Advisory monotonic generation (validated shape, never CAS-enforced). pub generation: u64, - /// Exact predecessor event ID; absent only for generation one. + /// Advisory predecessor event ID; absent exactly at generation one. #[serde(default, skip_serializing_if = "Option::is_none")] pub previous_event_id: Option, - /// Lifecycle state, repeated in the outer `state` tag. - pub state: State, /// RFC3339 bookkeeping timestamp; never used for conflict resolution. pub updated_at: String, - /// Required for active records and forbidden for tombstones. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub active: Option, - /// Required for tombstones and forbidden for active records. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub deleted_at: Option, + /// Secret identity material. + pub identity: PrivateIdentity, + /// Private portable/device-validated configuration. + pub config: PrivateConfig, /// Forward-compatible namespaced data. Core semantics must never depend on it. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub extensions: BTreeMap, + /// Unknown top-level JSON members preserved verbatim for forward + /// compatibility. A newer Desktop may author payload keys this version does + /// not model; they round-trip here untouched so an older writer never drops + /// them. Never contains a key that collides with a known field above (serde + /// binds known keys first). Core semantics must never depend on this map. + #[serde(flatten)] + pub extra: serde_json::Map, } /// Validated public metadata from a private managed-agent event. @@ -245,12 +219,10 @@ pub struct Envelope { pub agent_pubkey: PublicKey, /// Owner pubkey from the signed event author. pub owner_pubkey: PublicKey, - /// CAS generation from `g`. + /// Advisory generation from `g` (validated shape, never CAS-enforced). pub generation: u64, - /// CAS predecessor from `prev`. + /// Advisory predecessor from `prev`. pub previous_event_id: Option, - /// Lifecycle state from `state`. - pub state: State, } /// Compute the lowercase SHA-256 binding for exact projection content bytes. @@ -280,7 +252,6 @@ pub fn validate_envelope(event: &Event, expected_owner: &PublicKey) -> Result Result &mut d, "g" => &mut g, "prev" => &mut prev, - "state" => &mut state, name => return Err(Error::InvalidEnvelope(format!("unexpected tag: {name}"))), }; if slot.replace(parts[1].clone()).is_some() { @@ -322,18 +292,11 @@ pub fn validate_envelope(event: &Event, expected_owner: &PublicKey) -> Result State::Active, - Some("deleted") => State::Deleted, - Some(_) => return Err(Error::InvalidEnvelope("invalid state tag".into())), - None => return Err(Error::InvalidEnvelope("missing state tag".into())), - }; Ok(Envelope { agent_pubkey, owner_pubkey, generation, previous_event_id, - state, }) } @@ -362,7 +325,6 @@ pub fn build_event(owner_keys: &Keys, payload: &Payload, created_at: u64) -> Res let mut tags = vec![ parse_tag(["d", payload.agent_pubkey.as_str()])?, parse_tag(["g", payload.generation.to_string().as_str()])?, - parse_tag(["state", payload.state.as_str()])?, ]; if let Some(previous) = payload.previous_event_id.as_deref() { tags.push(parse_tag(["prev", previous])?); @@ -396,7 +358,6 @@ pub fn validate_and_decrypt( if payload.agent_pubkey != envelope.agent_pubkey.to_hex() || payload.owner_pubkey != envelope.owner_pubkey.to_hex() || payload.generation != envelope.generation - || payload.state != envelope.state || payload.previous_event_id.as_deref() != envelope .previous_event_id @@ -420,7 +381,7 @@ pub fn validate_payload(payload: &Payload) -> Result<(), Error> { } let agent = parse_canonical_pubkey("agent_pubkey", &payload.agent_pubkey) .map_err(|e| Error::InvalidPayload(e.to_string()))?; - parse_canonical_pubkey("owner_pubkey", &payload.owner_pubkey) + let owner = parse_canonical_pubkey("owner_pubkey", &payload.owner_pubkey) .map_err(|e| Error::InvalidPayload(e.to_string()))?; validate_generation_and_prev(payload.generation, payload.previous_event_id.as_deref())?; parse_rfc3339("updated_at", &payload.updated_at)?; @@ -432,77 +393,36 @@ pub fn validate_payload(payload: &Payload) -> Result<(), Error> { } validate_value_size("extension", value)?; } - match payload.state { - State::Active => { - if payload.deleted_at.is_some() { - return Err(Error::InvalidPayload( - "active payload must not contain deleted_at".into(), - )); - } - let active = payload.active.as_ref().ok_or_else(|| { - Error::InvalidPayload("active payload missing active body".into()) - })?; - validate_active(active, &agent, &payload.owner_pubkey)?; - } - State::Deleted => { - if payload.active.is_some() { - return Err(Error::InvalidPayload( - "deleted payload must not contain active body".into(), - )); - } - parse_rfc3339( - "deleted_at", - payload.deleted_at.as_deref().ok_or_else(|| { - Error::InvalidPayload("deleted payload missing deleted_at".into()) - })?, - )?; - } - } + validate_identity_and_config(&payload.identity, &payload.config, &agent, &owner)?; Ok(()) } -fn validate_active( - active: &ActivePayload, +/// Validate the secret identity and portable config of a payload. +/// +/// The nsec must derive the payload's `agent_pubkey` (the `d` coordinate), and +/// the config's bounds must hold. This is the nsec→coordinate binding gate. +fn validate_identity_and_config( + identity: &PrivateIdentity, + config: &PrivateConfig, agent: &PublicKey, - owner_pubkey: &str, + owner: &PublicKey, ) -> Result<(), Error> { - if active.definition.revision == 0 || active.definition.revision > MAX_SAFE_GENERATION { - return Err(Error::InvalidPayload("invalid definition revision".into())); - } - let definition_d = - parse_definition_coordinate(active.config.definition_coordinate.as_deref(), owner_pubkey)?; - validate_binding( - "definition", - KIND_PERSONA, - owner_pubkey, - Some(&definition_d), - &active.definition.event_id, - &active.definition.content_sha256, - &active.definition.recovery, - )?; - validate_binding( - "instance_projection", - KIND_MANAGED_AGENT, - owner_pubkey, - Some(&agent.to_hex()), - &active.instance_projection.event_id, - &active.instance_projection.content_sha256, - &active.instance_projection.recovery, - )?; - let agent_keys = Keys::parse(active.identity.private_key_nsec.trim()) + let agent_keys = Keys::parse(identity.private_key_nsec.trim()) .map_err(|_| Error::InvalidPayload("invalid agent nsec".into()))?; if agent_keys.public_key() != *agent { return Err(Error::InvalidPayload( "agent nsec does not derive agent_pubkey".into(), )); } - if let Some(auth_tag) = &active.identity.auth_tag { - validate_auth_tag(auth_tag, owner_pubkey, agent)?; + if let Some(auth_tag) = &identity.auth_tag { + validate_auth_tag(auth_tag, &owner.to_hex(), agent)?; } - let config = &active.config; if config.relay_url.is_empty() || config.relay_url.len() > 4096 { return Err(Error::InvalidPayload("invalid relay_url length".into())); } + if config.name.is_empty() || config.name.len() > 4096 { + return Err(Error::InvalidPayload("invalid name length".into())); + } if config.agent_args.len() > MAX_AGENT_ARGS || config .agent_args @@ -566,82 +486,6 @@ fn validate_auth_tag(auth_tag: &str, expected_owner: &str, agent: &PublicKey) -> .map_err(|_| Error::InvalidPayload("invalid auth_tag signature".into())) } -fn parse_definition_coordinate( - coordinate: Option<&str>, - owner_pubkey: &str, -) -> Result { - let coordinate = coordinate.ok_or_else(|| { - Error::InvalidPayload("active payload missing definition_coordinate".into()) - })?; - let mut parts = coordinate.splitn(3, ':'); - let kind = parts.next(); - let owner = parts.next(); - let d = parts.next(); - if kind != Some("30175") || owner != Some(owner_pubkey) || d.is_none_or(str::is_empty) { - return Err(Error::InvalidPayload( - "definition_coordinate must be 30175::".into(), - )); - } - Ok(d.unwrap().to_owned()) -} - -fn validate_binding( - label: &str, - expected_kind: u32, - owner_pubkey: &str, - expected_d: Option<&str>, - event_id: &str, - hash: &str, - recovery: &ProjectionRecoveryV1, -) -> Result<(), Error> { - parse_event_id(label, event_id).map_err(|e| Error::InvalidPayload(e.to_string()))?; - parse_lower_hex_32(&format!("{label}.content_sha256"), hash) - .map_err(|e| Error::InvalidPayload(e.to_string()))?; - if recovery.version != 1 { - return Err(Error::InvalidPayload(format!( - "unsupported {label} recovery version" - ))); - } - let event = &recovery.signed_event; - if !event.verify_id() || !event.verify_signature() { - return Err(Error::InvalidPayload(format!( - "invalid {label} recovery event" - ))); - } - if event.id.to_hex() != event_id - || event.kind.as_u16() as u32 != expected_kind - || event.pubkey.to_hex() != owner_pubkey - || content_sha256(event.content.as_bytes()) != hash - { - return Err(Error::InvalidPayload(format!( - "{label} recovery does not match binding" - ))); - } - let d_tags: Vec<_> = event - .tags - .iter() - .filter_map(|tag| { - let parts = tag.as_slice(); - (parts.first().map(String::as_str) == Some("d")).then_some(parts) - }) - .collect(); - if d_tags.len() != 1 || d_tags[0].len() != 2 || d_tags[0][1].is_empty() { - return Err(Error::InvalidPayload(format!( - "{label} recovery must have exactly one non-empty d tag" - ))); - } - if expected_d.is_some_and(|expected| d_tags[0][1] != expected) { - return Err(Error::InvalidPayload(format!( - "{label} recovery has wrong coordinate" - ))); - } - validate_value_size( - label, - &serde_json::to_value(recovery) - .map_err(|_| Error::InvalidPayload(format!("invalid {label}")))?, - ) -} - fn validate_generation_and_prev(generation: u64, previous: Option<&str>) -> Result<(), Error> { if generation == 0 || generation > MAX_SAFE_GENERATION { return Err(Error::InvalidPayload( @@ -814,21 +658,9 @@ mod tests { .to_string() } + /// Minimal valid payload: the nsec derives `agent_pubkey`, generation 1, + /// required config fields present, no unknown members. fn payload(owner: &Keys, agent: &Keys) -> Payload { - let definition_event = EventBuilder::new(Kind::Custom(KIND_PERSONA as u16), "definition") - .tags(vec![Tag::parse(["d", "test-agent"]).unwrap()]) - .custom_created_at(nostr::Timestamp::from(1_785_780_000)) - .sign_with_keys(owner) - .unwrap(); - let instance_event = EventBuilder::new(Kind::Custom(KIND_MANAGED_AGENT as u16), "instance") - .tags(vec![Tag::parse([ - "d", - agent.public_key().to_hex().as_str(), - ]) - .unwrap()]) - .custom_created_at(nostr::Timestamp::from(1_785_780_000)) - .sign_with_keys(owner) - .unwrap(); Payload { format: FORMAT.into(), version: VERSION, @@ -836,50 +668,36 @@ mod tests { owner_pubkey: owner.public_key().to_hex(), generation: 1, previous_event_id: None, - state: State::Active, updated_at: "2026-08-03T18:00:00Z".into(), - active: Some(ActivePayload { - definition: DefinitionBinding { - revision: 1, - event_id: definition_event.id.to_hex(), - content_sha256: content_sha256(definition_event.content.as_bytes()), - recovery: ProjectionRecoveryV1 { - version: 1, - signed_event: definition_event, - }, - }, - instance_projection: InstanceBinding { - event_id: instance_event.id.to_hex(), - content_sha256: content_sha256(instance_event.content.as_bytes()), - recovery: ProjectionRecoveryV1 { - version: 1, - signed_event: instance_event, - }, - }, - identity: PrivateIdentity { - private_key_nsec: agent.secret_key().to_bech32().unwrap(), - auth_tag: None, - }, - config: PrivateConfig { - definition_coordinate: Some(format!( - "30175:{}:test-agent", - owner.public_key().to_hex() - )), - relay_url: "wss://relay.example".into(), - agent_command_override: None, - agent_args: vec![], - idle_timeout_seconds: Some(300), - max_turn_duration_seconds: None, - env_vars: BTreeMap::from([("SECRET".into(), "not-public".into())]), - backend: serde_json::json!({"type": "local"}), - backend_agent_id: None, - team_id: None, - persona_name_in_team: None, - relay_mesh: None, - }, - }), - deleted_at: None, + identity: PrivateIdentity { + private_key_nsec: agent.secret_key().to_bech32().unwrap(), + auth_tag: None, + }, + config: PrivateConfig { + relay_url: "wss://relay.example".into(), + name: "aphid".into(), + persona_id: Some("aphid-def".into()), + runtime: Some("goose".into()), + model: None, + provider: None, + system_prompt: Some("be terse".into()), + parallelism: Some(2), + respond_to: Some("owner-only".into()), + respond_to_allowlist: vec![], + agent_command_override: None, + agent_args: vec![], + idle_timeout_seconds: Some(300), + max_turn_duration_seconds: None, + env_vars: BTreeMap::from([("SECRET".into(), "not-public".into())]), + backend: serde_json::json!({"type": "local"}), + backend_agent_id: None, + team_id: None, + persona_name_in_team: None, + relay_mesh: None, + extra: serde_json::Map::new(), + }, extensions: BTreeMap::new(), + extra: serde_json::Map::new(), } } @@ -894,7 +712,7 @@ mod tests { assert_eq!(envelope.agent_pubkey, agent.public_key()); assert_eq!(envelope.owner_pubkey, owner.public_key()); assert_eq!(envelope.generation, 1); - assert_eq!(envelope.state, State::Active); + assert_eq!(envelope.previous_event_id, None); } #[test] @@ -902,16 +720,9 @@ mod tests { let owner = Keys::generate(); let agent = Keys::generate(); let mut candidate = payload(&owner, &agent); - let private_key_nsec = candidate - .active - .as_ref() - .unwrap() - .identity - .private_key_nsec - .clone(); - let active = candidate.active.as_mut().unwrap(); - active.identity.auth_tag = Some("secret-auth-tag".into()); - active.config.backend = serde_json::json!({"token": "secret-backend-token"}); + let private_key_nsec = candidate.identity.private_key_nsec.clone(); + candidate.identity.auth_tag = Some("secret-auth-tag".into()); + candidate.config.backend = serde_json::json!({"token": "secret-backend-token"}); let debug = format!("{candidate:?}"); assert!(debug.contains("")); @@ -921,6 +732,7 @@ mod tests { assert!(!debug.contains("secret-backend-token")); } + // (1) privacy / wrong-owner + tamper fail closed. #[test] fn wrong_owner_and_tampering_fail_closed() { let owner = Keys::generate(); @@ -940,192 +752,216 @@ mod tests { )); } + // (2) nsec -> pubkey binding: the identity nsec must derive agent_pubkey (d). #[test] - fn duplicate_and_unknown_json_fields_are_rejected() { - let duplicate = br#"{"format":"a","format":"b"}"#; + fn active_identity_must_derive_coordinate() { + let owner = Keys::generate(); + let mut candidate = payload(&owner, &Keys::generate()); + candidate.identity.private_key_nsec = Keys::generate().secret_key().to_bech32().unwrap(); assert!(matches!( - parse_strict_json(duplicate), - Err(Error::InvalidPayload(message)) if message.contains("duplicate key") + validate_payload(&candidate), + Err(Error::InvalidPayload(message)) if message.contains("does not derive") )); + } + #[test] + fn valid_owner_attestation_passes_and_binds_agent() { let owner = Keys::generate(); let agent = Keys::generate(); - let mut value = serde_json::to_value(payload(&owner, &agent)).unwrap(); - value - .as_object_mut() - .unwrap() - .insert("surprise".into(), Value::Bool(true)); - let err = serde_json::from_value::(value).unwrap_err(); - assert!(err.to_string().contains("unknown field")); + let mut candidate = payload(&owner, &agent); + // The owner signs an unconditional attestation over the AGENT key. + candidate.identity.auth_tag = Some(auth_tag(&owner, &agent)); + assert!(validate_payload(&candidate).is_ok()); + + // Round-trips end-to-end with the attestation intact. + let event = build_event(&owner, &candidate, 1_785_780_000).unwrap(); + let (_envelope, decoded) = validate_and_decrypt(&event, &owner).unwrap(); + assert_eq!(decoded, candidate); } #[test] - fn auth_tag_must_be_unconditional_and_bound_to_owner_and_agent() { + fn auth_tag_from_wrong_attestor_is_rejected() { let owner = Keys::generate(); let agent = Keys::generate(); let mut candidate = payload(&owner, &agent); - candidate.active.as_mut().unwrap().identity.auth_tag = Some(auth_tag(&owner, &agent)); - validate_payload(&candidate).unwrap(); - - candidate.active.as_mut().unwrap().identity.auth_tag = - Some(auth_tag(&Keys::generate(), &agent)); - assert!(validate_payload(&candidate).is_err()); - - candidate.active.as_mut().unwrap().identity.auth_tag = - Some(auth_tag(&owner, &Keys::generate())); - assert!(validate_payload(&candidate).is_err()); - - let mut self_attested = payload(&owner, &owner); - self_attested.active.as_mut().unwrap().identity.auth_tag = Some(auth_tag(&owner, &owner)); + // A stranger (not the owner) signs the attestation: parts[1] is not the + // owner pubkey, so the attestation is rejected. + let stranger = Keys::generate(); + candidate.identity.auth_tag = Some(auth_tag(&stranger, &agent)); assert!(matches!( - validate_payload(&self_attested), - Err(Error::InvalidPayload(message)) if message.contains("distinct agent key") + validate_payload(&candidate), + Err(Error::InvalidPayload(message)) if message.contains("auth_tag") )); - - let valid = auth_tag(&owner, &agent); - let mut parts: Vec = serde_json::from_str(&valid).unwrap(); - parts[2] = "kind=9".into(); - candidate.active.as_mut().unwrap().identity.auth_tag = - Some(serde_json::to_string(&parts).unwrap()); - assert!(validate_payload(&candidate).is_err()); } #[test] - fn active_identity_must_derive_coordinate() { + fn auth_tag_signature_must_verify() { let owner = Keys::generate(); - let mut candidate = payload(&owner, &Keys::generate()); - candidate.active.as_mut().unwrap().identity.private_key_nsec = - Keys::generate().secret_key().to_bech32().unwrap(); + let agent = Keys::generate(); + let mut candidate = payload(&owner, &agent); + // Correct owner in parts[1], but the signature is over a DIFFERENT agent + // key, so schnorr verification against this agent's preimage fails. + let other_agent = Keys::generate(); + let preimage = format!("nostr:agent-auth:{}:", other_agent.public_key().to_hex()); + let digest = Sha256::digest(preimage.as_bytes()); + let signature = owner.sign_schnorr(&Message::from_digest(digest.into())); + candidate.identity.auth_tag = Some( + serde_json::json!([ + "auth", + owner.public_key().to_hex(), + "", + signature.to_string() + ]) + .to_string(), + ); assert!(matches!( validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("does not derive") + Err(Error::InvalidPayload(message)) if message.contains("signature") )); } + // (3) unknown-field round-trip: unknown top-level + config members survive + // verbatim through serialize/deserialize and land in the `extra` maps. #[test] - fn tombstone_requires_successor_shape() { + fn unknown_members_round_trip_verbatim() { let owner = Keys::generate(); let agent = Keys::generate(); - let mut deleted = payload(&owner, &agent); - deleted.generation = 2; - deleted.previous_event_id = Some("33".repeat(32)); - deleted.state = State::Deleted; - deleted.active = None; - deleted.deleted_at = Some("2026-08-03T18:01:00Z".into()); - validate_payload(&deleted).unwrap(); + let mut candidate = payload(&owner, &agent); + candidate + .extra + .insert("future_top".into(), serde_json::json!({"nested": [1, 2]})); + candidate + .config + .extra + .insert("future_cfg".into(), Value::String("keep-me".into())); - deleted.previous_event_id = None; - assert!(validate_payload(&deleted).is_err()); + let event = build_event(&owner, &candidate, 1_785_780_000).unwrap(); + let (_envelope, decoded) = validate_and_decrypt(&event, &owner).unwrap(); + assert_eq!(decoded, candidate); + assert_eq!( + decoded.extra.get("future_top"), + Some(&serde_json::json!({"nested": [1, 2]})) + ); + assert_eq!( + decoded.config.extra.get("future_cfg"), + Some(&Value::String("keep-me".into())) + ); } + // (3b) an unknown member can never override a known field: serde binds the + // typed field first, so a colliding key is impossible to smuggle into `extra`. #[test] - fn outer_tag_grammar_rejects_duplicates_and_noncanonical_generation() { + fn unknown_member_cannot_override_known_field() { let owner = Keys::generate(); let agent = Keys::generate(); - let body = payload(&owner, &agent); - let ciphertext = nip44::encrypt( - owner.secret_key(), - &owner.public_key(), - serde_json::to_string(&body).unwrap(), - Version::V2, - ) - .unwrap(); - let event = EventBuilder::new(Kind::Custom(KIND_PRIVATE_MANAGED_AGENT as u16), ciphertext) - .tags(vec![ - Tag::parse(["d", agent.public_key().to_hex().as_str()]).unwrap(), - Tag::parse(["g", "01"]).unwrap(), - Tag::parse(["state", "active"]).unwrap(), - ]) - .sign_with_keys(&owner) - .unwrap(); - assert!(matches!( - validate_envelope(&event, &owner.public_key()), - Err(Error::InvalidEnvelope(message)) if message.contains("canonical decimal") - )); + let mut value = serde_json::to_value(payload(&owner, &agent)).unwrap(); + // Inject a duplicate-looking known key into the config object; serde + // routes it to the typed `name`, NOT to `extra`. + value["config"]["name"] = Value::String("renamed".into()); + let decoded: Payload = serde_json::from_value(value).unwrap(); + assert_eq!(decoded.config.name, "renamed"); + assert!(!decoded.config.extra.contains_key("name")); } + // (4) required / null semantics: a missing required known field is rejected; + // duplicate JSON keys are rejected by the strict parser. #[test] - fn projection_recovery_must_match_binding_and_coordinate() { + fn required_fields_and_duplicate_keys() { let owner = Keys::generate(); let agent = Keys::generate(); - let mut candidate = payload(&owner, &agent); - let active = candidate.active.as_mut().unwrap(); - active.instance_projection.content_sha256 = content_sha256(b"wrong"); - assert!(matches!( - validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("does not match binding") - )); - let mut candidate = payload(&owner, &agent); - candidate - .active - .as_mut() - .unwrap() - .config - .definition_coordinate = - Some(format!("30175:{}:wrong-slug", owner.public_key().to_hex())); + // Missing required `config.name`. + let mut value = serde_json::to_value(payload(&owner, &agent)).unwrap(); + value["config"].as_object_mut().unwrap().remove("name"); + assert!(serde_json::from_value::(value).is_err()); + + // Duplicate top-level key rejected pre-deserialization. + let duplicate = br#"{"format":"a","format":"b"}"#; assert!(matches!( - validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("wrong coordinate") + parse_strict_json(duplicate), + Err(Error::InvalidPayload(message)) if message.contains("duplicate key") )); - let mut candidate = payload(&owner, &agent); - candidate - .active - .as_mut() - .unwrap() - .definition - .recovery - .version = 2; + + // Empty required `name` fails semantic validation. + let mut empty_name = payload(&owner, &agent); + empty_name.config.name = String::new(); assert!(matches!( - validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("unsupported definition recovery version") + validate_payload(&empty_name), + Err(Error::InvalidPayload(message)) if message.contains("invalid name length") )); + } - let mut candidate = payload(&owner, &agent); - candidate - .active - .as_mut() - .unwrap() - .definition - .recovery - .signed_event - .content - .push('!'); + // gen/prev are a3 advisory metadata: shape is validated (gen1 XOR prev, + // canonical decimal, outer/inner equality) but ordering is never enforced. + #[test] + fn generation_prev_shape_is_validated_metadata() { + let owner = Keys::generate(); + let agent = Keys::generate(); + + // Higher generation with a well-formed prev round-trips fine — no head + // consult, no staleness rejection. + let mut successor = payload(&owner, &agent); + successor.generation = 7; + successor.previous_event_id = Some("33".repeat(32)); + let event = build_event(&owner, &successor, 1_785_780_001).unwrap(); + let (envelope, decoded) = validate_and_decrypt(&event, &owner).unwrap(); + assert_eq!(envelope.generation, 7); + assert_eq!(decoded.generation, 7); + + // prev present at generation 1 violates the shape rule. + let mut bad = payload(&owner, &agent); + bad.previous_event_id = Some("33".repeat(32)); assert!(matches!( - validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("invalid definition recovery event") + validate_payload(&bad), + Err(Error::InvalidPayload(message)) if message.contains("absent exactly at generation 1") )); + } - let mut candidate = payload(&owner, &agent); - let wrong_kind = EventBuilder::new(Kind::Custom(KIND_MANAGED_AGENT as u16), "definition") - .tags(vec![Tag::parse(["d", "test-agent"]).unwrap()]) - .sign_with_keys(&owner) - .unwrap(); - let definition = &mut candidate.active.as_mut().unwrap().definition; - definition.event_id = wrong_kind.id.to_hex(); - definition.content_sha256 = content_sha256(wrong_kind.content.as_bytes()); - definition.recovery.signed_event = wrong_kind; + #[test] + fn outer_tag_grammar_rejects_noncanonical_generation_and_stray_tags() { + let owner = Keys::generate(); + let agent = Keys::generate(); + let body = payload(&owner, &agent); + let ciphertext = nip44::encrypt( + owner.secret_key(), + &owner.public_key(), + serde_json::to_string(&body).unwrap(), + Version::V2, + ) + .unwrap(); + // Non-canonical generation "01". + let event = EventBuilder::new( + Kind::Custom(KIND_PRIVATE_MANAGED_AGENT as u16), + ciphertext.clone(), + ) + .tags(vec![ + Tag::parse(["d", agent.public_key().to_hex().as_str()]).unwrap(), + Tag::parse(["g", "01"]).unwrap(), + ]) + .sign_with_keys(&owner) + .unwrap(); assert!(matches!( - validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("does not match binding") + validate_envelope(&event, &owner.public_key()), + Err(Error::InvalidEnvelope(message)) if message.contains("canonical decimal") )); - let mut candidate = payload(&owner, &agent); - let missing_d = EventBuilder::new(Kind::Custom(KIND_PERSONA as u16), "definition") + // A stray lifecycle `state` tag is now unexpected (lifecycle removed). + let stray = EventBuilder::new(Kind::Custom(KIND_PRIVATE_MANAGED_AGENT as u16), ciphertext) + .tags(vec![ + Tag::parse(["d", agent.public_key().to_hex().as_str()]).unwrap(), + Tag::parse(["g", "1"]).unwrap(), + Tag::parse(["state", "active"]).unwrap(), + ]) .sign_with_keys(&owner) .unwrap(); - let definition = &mut candidate.active.as_mut().unwrap().definition; - definition.event_id = missing_d.id.to_hex(); - definition.content_sha256 = content_sha256(missing_d.content.as_bytes()); - definition.recovery.signed_event = missing_d; assert!(matches!( - validate_payload(&candidate), - Err(Error::InvalidPayload(message)) if message.contains("exactly one non-empty d tag") + validate_envelope(&stray, &owner.public_key()), + Err(Error::InvalidEnvelope(message)) if message.contains("unexpected tag") )); } #[test] - fn projection_hash_fixture_is_stable() { + fn hash_fixture_is_stable() { assert_eq!( content_sha256(b"buzz-private-managed-agent-v1"), "c3ca1603249c95343fc1766ba58d075d6bdf0e57b375bef38738729b2022cc80" diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 5f5019cd20c..5e6eb9144cc 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -38,22 +38,21 @@ pub struct AppState { pub managed_agent_restore_pending: AtomicBool, /// Disabled by agent-managed profiles so agent profile updates survive start/restore. pub managed_agent_profile_reconcile_enabled: AtomicBool, - /// Shared shutdown signal checked by launch-time agent restoration. + /// Shared shutdown signal for launch-time agent restoration. pub shutdown_started: AtomicBool, /// Serializes every managed-runtime transition that changes the protected /// PID set: spawn/register, adoption, stop, shutdown, and sweep snapshots. /// Never perform network I/O while holding this lock. pub managed_agent_runtime_transition: Mutex<()>, pub managed_agents_store_lock: Mutex<()>, + pub(crate) private_managed_agent_overlay: + Mutex, pub channel_templates_store_lock: Mutex<()>, pub managed_agent_processes: Mutex>, pub provider_deploy_locks: Mutex>>>, pub huddle_state: Mutex, pub huddle_audio: crate::huddle::tts_settings::HuddleAudioSettingsState, - /// Tauri app handle — stored after setup so huddle commands can emit - /// `huddle-state-changed` events without needing the handle threaded - /// through every call site. - /// + /// Tauri handle for emitting huddle events. /// Set once during `setup()` in `lib.rs`; never cleared. pub app_handle: Mutex>, /// Port of the localhost media streaming proxy (set during setup). @@ -211,6 +210,7 @@ pub fn build_app_state() -> AppState { managed_agent_runtime_transition: Mutex::new(()), identity_mutation: Mutex::new(()), managed_agents_store_lock: Mutex::new(()), + private_managed_agent_overlay: Mutex::new(Default::default()), channel_templates_store_lock: Mutex::new(()), managed_agent_processes: Mutex::new(HashMap::new()), provider_deploy_locks: Mutex::new(HashMap::new()), diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index cb809b6c04a..33ca7c7cdf0 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -702,87 +702,9 @@ mod update; pub use update::update_managed_agent; pub(super) use update::{flush_managed_agent_policy, managed_agent_access_policy_changed}; -// ── 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 00000000000..b437136b3f8 --- /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/agent_models_update.rs b/desktop/src-tauri/src/commands/agent_models_update.rs index bb045b81a24..ac21121da31 100644 --- a/desktop/src-tauri/src/commands/agent_models_update.rs +++ b/desktop/src-tauri/src/commands/agent_models_update.rs @@ -81,6 +81,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; diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 33b6ae44620..53c944ae7e0 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -156,6 +156,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) { @@ -202,125 +215,6 @@ pub(super) async fn start_local_agent_pairs_with_preflight( summarize_from_disk(app, record, &runtimes) } -pub(super) async fn start_local_agent_with_preflight( - app: &AppHandle, - state: &AppState, - pubkey: &str, - allow_fresh_create_start: bool, - expected_relay_url: Option<&str>, - expected_signer_pubkey: Option<&str>, -) -> Result { - let record_snapshot = { - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let records = load_managed_agents(app)?; - records - .iter() - .find(|record| record.pubkey == pubkey) - .cloned() - .ok_or_else(|| format!("agent {pubkey} not found"))? - }; - - if record_snapshot.backend != BackendKind::Local { - return Err(format!("agent {pubkey} is not a local agent")); - } - - // Preflight against the same resolution spawn uses — `resolve_effective_config` - // (definition → global fallback). A linked instance's own `provider`/`model`/ - // `relay_mesh` bytes never contribute: this reads the CURRENT definition - // directly, so a definition edit that flips `provider` to/from relay-mesh - // between saves is reflected here without needing a prospective re-snapshot; - // for a global-inherited blank definition, it also folds in the global - // default, which record-byte sniffing could never see. - let personas = load_personas(app).unwrap_or_default(); - let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); - let mesh_model_id = - crate::managed_agents::effective_config::resolve_effective_relay_mesh_model_id( - &record_snapshot, - &personas, - &global, - ); - ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), allow_fresh_create_start).await?; - - // The mesh preflight above is the suspension window Projects callbacks - // capture their scope against: a community switch during that await - // would otherwise spawn this pair keyed to the *new* workspace relay. - // Read the workspace relay ONCE, assert the caller's captured scope - // against that exact read, and hand the same bound value to the spawn - // below — the check is tied to its use, so a switch landing after this - // point can no longer retarget the spawn (it only changes state this - // call no longer consults). - let workspace_relay_url = crate::relay::bind_expected_relay_scope( - expected_relay_url, - crate::relay::relay_ws_url_with_override(state), - )?; - // Bind the active owner after the same final await as the relay. A - // same-relay identity replacement during mesh preflight must not release - // the stale preflight owner to spawn. - let workspace_owner = - crate::relay::bind_expected_signer(expected_signer_pubkey, workspace_owner_hex(state)?)?; - - let _store_guard = state - .managed_agents_store_lock - .lock() - .map_err(|e| e.to_string())?; - let mut records = load_managed_agents(app)?; - let mut runtimes = state - .managed_agent_processes - .lock() - .map_err(|e| e.to_string())?; - let record = find_managed_agent_mut(&mut records, pubkey)?; - if record.backend != BackendKind::Local { - return Err(format!("agent {pubkey} is no longer a local agent")); - } - // Re-snapshot the persona onto the record at every spawn so the agent always - // starts with the current persona config (system_prompt, model, provider, - // runtime). This clears the "out of date" drift badge without requiring a - // delete+recreate. See `apply_persona_snapshot` for the precedence and - // env-override self-heal rules. - // Load personas once: used for snapshot application below and summary build - // at the end — avoids a second disk read for the same file in the same call. - let personas = load_personas(app).unwrap_or_default(); - if let Some(persona_id) = record.persona_id.clone() { - match personas.iter().find(|p| p.id == persona_id) { - Some(persona) => { - crate::managed_agents::persona_events::apply_persona_snapshot(record, persona); - record.updated_at = crate::util::now_iso(); - } - None => { - return Err( - crate::managed_agents::effective_config::ORPHANED_INSTANCE_ERROR.to_string(), - ); - } - } - } - start_managed_agent_process( - app, - record, - &mut runtimes, - Some(workspace_owner.as_str()), - &workspace_relay_url, - )?; - save_managed_agents(app, &records)?; - if let Some(saved_record) = records.iter().find(|r| r.pubkey == pubkey) { - retain_managed_agent_pending(app, state, saved_record); - } - let record = records - .iter() - .find(|record| record.pubkey == pubkey) - .ok_or_else(|| format!("agent {pubkey} not found"))?; - build_managed_agent_summary( - app, - record, - &runtimes, - &personas, - &load_teams(app).unwrap_or_default(), - &crate::managed_agents::load_global_agent_config(app).unwrap_or_default(), - ) -} - pub(crate) use provider_deploy::deploy_to_provider; // Async so the blocking body (disk reads of agent/persona records, per-agent @@ -352,6 +246,11 @@ pub async fn list_managed_agents(app: AppHandle) -> Result Err(format!( "agent {pubkey} has unsupported backend kind: {backend:?}" @@ -1044,6 +959,10 @@ pub async fn stop_managed_agent( use tauri::Manager; tokio::task::spawn_blocking(move || { let state = app.state::(); + let _transition_guard = state + .managed_agent_runtime_transition + .lock() + .map_err(|error| error.to_string())?; let _store_guard = state .managed_agents_store_lock .lock() @@ -1063,25 +982,33 @@ pub async fn stop_managed_agent( state.clear_agent_session_caches(pubkey); } - { - let record = find_managed_agent_mut(&mut records, &pubkey)?; + let resolved_record = { + let disk_record = find_managed_agent_mut(&mut records, &pubkey)?; + let mut resolved = + crate::managed_agents::private_config_overlay::resolved_local_record( + &state, + disk_record, + )?; // Remote agents are stopped via !shutdown @mention from the frontend, - // not via this backend command. Reject the call. - if record.backend != BackendKind::Local { + // not via this backend command. Reject using the relay-resolved backend. + if resolved.backend != BackendKind::Local { return Err( "remote agents are stopped via !shutdown message, not this command".to_string(), ); } // Pair-scoped: stops only the active workspace's pair; delete and // the config-restart flows still drain every pair. - stop_managed_agent_workspace_pair(&app, record, &mut runtimes)?; - } + stop_managed_agent_workspace_pair(&app, &mut resolved, &mut runtimes)?; + crate::managed_agents::private_config_overlay::copy_lifecycle_state( + disk_record, + &resolved, + ); + resolved + }; save_managed_agents(&app, &records)?; - let record = records - .iter() - .find(|record| record.pubkey == pubkey) - .ok_or_else(|| format!("agent {pubkey} not found"))?; - summarize_from_disk(&app, record, &runtimes) + // Summarize the relay-resolved record so the response reflects the + // config this device follows, not raw disk. + summarize_from_disk(&app, &resolved_record, &runtimes) }) .await .map_err(|e| format!("spawn_blocking failed: {e}"))? @@ -1098,6 +1025,10 @@ pub async fn delete_managed_agent( use tauri::Manager; tokio::task::spawn_blocking(move || { let state = app.state::(); + let _transition_guard = state + .managed_agent_runtime_transition + .lock() + .map_err(|error| error.to_string())?; { let _store_guard = state .managed_agents_store_lock @@ -1126,7 +1057,19 @@ pub async fn delete_managed_agent( // invariant — a buggy or compromised IPC caller cannot silently orphan a live // remote deployment. The frontend sends force_remote_delete: true only after // the user confirms the orphan warning. - if let Some(record) = records.iter().find(|r| r.pubkey == pubkey) { + let resolved_record = + if let Some(record) = records.iter().find(|record| record.pubkey == pubkey) { + Some( + state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())? + .resolve_local_record(record), + ) + } else { + None + }; + if let Some(record) = resolved_record.as_ref() { if record.backend != BackendKind::Local && record.backend_agent_id.is_some() && !force_remote_delete.unwrap_or(false) @@ -1138,12 +1081,11 @@ pub async fn delete_managed_agent( } } - let persona_id = records - .iter() - .find(|record| record.pubkey == pubkey) + let persona_id = resolved_record + .as_ref() .and_then(|record| record.persona_id.clone()); - if let Some(record) = records.iter_mut().find(|record| record.pubkey == pubkey) { - stop_managed_agent_process(&app, record, &mut runtimes)?; + if let Some(mut record) = resolved_record { + stop_managed_agent_process(&app, &mut record, &mut runtimes)?; } state.clear_agent_session_caches(&pubkey); let initial_len = records.len(); @@ -1152,6 +1094,12 @@ pub async fn delete_managed_agent( return Err(format!("agent {pubkey} not found")); } save_managed_agents(&app, &records)?; + state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())? + .remove(&pubkey); + // Remove the agent's nsec from the keyring after the record is gone. crate::managed_agents::delete_agent_key(&pubkey); // Tombstone after confirmed removal (inside lock; every published agent tombstones). tombstone_managed_agent_pending(&app, &state, &pubkey); @@ -1172,6 +1120,10 @@ pub async fn delete_managed_agent( // 2. Harness sees it, exits gracefully, sets presence to "offline" // 3. Desktop's existing presence polling sees "offline" — UI updates automatically // No backend Tauri command needed. Presence IS the status. +#[path = "agents_lifecycle.rs"] +mod lifecycle; +use lifecycle::start_local_agent_with_preflight; + #[path = "agents_deploy.rs"] mod deploy; pub(super) mod provider_access; diff --git a/desktop/src-tauri/src/commands/agents/provider_access.rs b/desktop/src-tauri/src/commands/agents/provider_access.rs index 34c06d25919..f1ccc7ce95b 100644 --- a/desktop/src-tauri/src/commands/agents/provider_access.rs +++ b/desktop/src-tauri/src/commands/agents/provider_access.rs @@ -68,7 +68,19 @@ pub(crate) async fn reconcile_on_workspace_apply( .managed_agents_store_lock .lock() .map_err(|error| error.to_string())?; - collect_targets_with(load_managed_agents(app)?, owner_only_access, |record| { + // Resolve each disk row through the relay-primary overlay before + // selecting targets: the redeploy below is a final-use boundary, so + // both the selection predicate (backend, backend_agent_id, pending + // flag) and the payload handed to the provider must read the + // authoritative config, not raw disk bytes a newer relay head has + // superseded. + let records = load_managed_agents(app)? + .iter() + .map(|record| { + crate::managed_agents::private_config_overlay::resolved_local_record(state, record) + }) + .collect::, _>>()?; + collect_targets_with(records, owner_only_access, |record| { super::build_deploy_payload(app, state, record) }) }; @@ -231,4 +243,89 @@ mod tests { }) .is_empty()); } + + // ── Workspace reconcile: relay-overlay resolve before target selection ── + // + // The production wiring (`reconcile_on_workspace_apply` resolving every + // disk row through `resolved_local_record` before `collect_targets_with`) + // needs a live `AppHandle`, so its presence is pinned by + // `write_site_resolve_guard` in `private_config_overlay.rs`. This test + // proves the fold itself: both the selection predicate and the redeploy + // inputs read the resolved records, not raw disk rows. + + /// Carl round-9 P1 regression (stale-disk A / overlay B at workspace + /// provider-access reconciliation): a relay head that migrated the + /// backend must drive BOTH selection and the payload — the raw disk row's + /// provider must neither be redeployed (head says local) nor keep stale + /// policy inputs (head says a different provider). + #[test] + fn reconcile_selects_and_deploys_relay_resolved_records_not_raw_disk() { + use crate::managed_agents::private_config_overlay::{ + test_relay_payload, PrivateConfigOverlay, + }; + + // Disk row A: provider on disk, but the relay head migrated it to + // local — reconciliation must not select it at all. + let migrated_pubkey = "aa".repeat(32); + let mut migrated_disk = record( + BackendKind::Provider { + id: "stale-provider".into(), + config: serde_json::json!({}), + }, + Some("existing"), + ); + migrated_disk.pubkey = migrated_pubkey.clone(); + + // Disk row B: provider on disk AND on the relay head, but the head + // carries newer policy inputs — the redeploy payload must read them. + let repolicied_pubkey = "bb".repeat(32); + let mut repolicied_disk = record( + BackendKind::Provider { + id: "stale-provider".into(), + config: serde_json::json!({"region": "stale"}), + }, + Some("existing"), + ); + repolicied_disk.pubkey = repolicied_pubkey.clone(); + repolicied_disk.system_prompt = Some("stale disk prompt".into()); + + let mut overlay = PrivateConfigOverlay::default(); + overlay + .insert(test_relay_payload(&migrated_pubkey)) + .unwrap(); // backend: local + let mut repolicied_head = test_relay_payload(&repolicied_pubkey); + repolicied_head.config.backend = serde_json::json!({"type":"provider","id":"relay-provider","config":{"region":"relay"}}); + repolicied_head.config.backend_agent_id = Some("existing".into()); + overlay.insert(repolicied_head).unwrap(); + + let resolved: Vec<_> = [&migrated_disk, &repolicied_disk] + .into_iter() + .map(|record| overlay.resolve_local_record(record)) + .collect(); + let targets = collect_targets_with(resolved, true, |record| { + Ok(serde_json::json!({"system_prompt": record.system_prompt})) + }); + + assert_eq!( + targets.len(), + 1, + "the head-migrated-to-local row must not be selected" + ); + assert_eq!(targets[0].pubkey, repolicied_pubkey); + assert_eq!(targets[0].provider_id, "relay-provider"); + assert_eq!(targets[0].config["region"], "relay"); + assert_eq!( + targets[0].agent_json.as_ref().unwrap()["system_prompt"], + "relay prompt", + "the redeploy payload must be built from the resolved record" + ); + + // NEGATIVE CONTROL: raw disk rows select the migrated agent and keep + // the stale provider — proving the resolve, not the fixtures. + let raw = collect_targets_with(vec![migrated_disk, repolicied_disk], true, |_| { + Ok(serde_json::Value::Null) + }); + assert_eq!(raw.len(), 2); + assert!(raw.iter().all(|t| t.provider_id == "stale-provider")); + } } diff --git a/desktop/src-tauri/src/commands/agents/provider_deploy.rs b/desktop/src-tauri/src/commands/agents/provider_deploy.rs index bb56a67eaa4..3e9f0d19d5b 100644 --- a/desktop/src-tauri/src/commands/agents/provider_deploy.rs +++ b/desktop/src-tauri/src/commands/agents/provider_deploy.rs @@ -56,27 +56,34 @@ pub(crate) async fn deploy_to_provider( }; let _deploy_guard = deploy_lock.lock().await; // The payload may have waited behind another deployment. Rebuild it from - // the current record so the final provider invocation always carries the - // newest saved policy rather than the stale snapshot captured by its caller. + // the current record — resolved through the relay-primary overlay, since + // this is the final-use boundary the provider actually executes — so the + // invocation always carries the newest authoritative policy rather than + // the stale snapshot captured by its caller, and never raw disk bytes a + // newer relay head has superseded (stale prompt/model/env/credentials/ + // access, or a raw backend that no longer matches the resolved one). let (provider_id, config, cached_binary_path, agent_json) = { let _store_guard = state .managed_agents_store_lock .lock() .map_err(|error| error.to_string())?; let records = load_managed_agents(app)?; - let record = records + let disk_record = records .iter() .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found"))?; - let (provider_id, config) = match &record.backend { - BackendKind::Provider { id, config } => (id.clone(), config.clone()), - BackendKind::Local => return Err(format!("agent {pubkey} is not provider-backed")), - }; + let record = crate::managed_agents::private_config_overlay::resolved_local_record( + state, + disk_record, + )?; + let (provider_id, config) = resolved_provider_backend(&record)?; ( provider_id, config, + // Device-local field: the overlay never patches it, so the + // resolved record carries the disk value unchanged. record.provider_binary_path.clone(), - build_deploy_payload(app, state, record)?, + build_deploy_payload(app, state, &record)?, ) }; // The rebuild above re-read the live workspace relay and owner identity. @@ -121,6 +128,20 @@ pub(crate) async fn deploy_to_provider( result } +/// Extract the provider backend from the record REBUILT after the deploy +/// lock — the exact value invoked. Pure over the resolved record so the +/// post-lock final-use boundary is testable without a live `AppHandle`: a +/// relay head that migrated the agent back to the local backend must refuse +/// the deploy by name instead of deploying leftover raw-disk provider bytes. +fn resolved_provider_backend( + record: &crate::managed_agents::ManagedAgentRecord, +) -> Result<(String, serde_json::Value), String> { + match &record.backend { + BackendKind::Provider { id, config } => Ok((id.clone(), config.clone())), + BackendKind::Local => Err(format!("agent {} is not provider-backed", record.pubkey)), + } +} + /// Assert a caller-captured tenant scope against the payload that will /// actually be invoked. The relay lives at the payload's top-level /// `relay_url`; the deploying identity lives at `launch.owner_pubkey` — both @@ -329,4 +350,74 @@ mod tests { assert!(record.provider_policy_pending); assert_eq!(record.last_error.as_deref(), Some("provider unavailable")); } + + // ── Post-lock rebuild: relay-overlay resolve at the final-use boundary ── + // + // The production wiring (`deploy_to_provider` resolving the reloaded disk + // row through `resolved_local_record` after taking the deploy lock) needs + // a live `AppHandle`, so its presence is pinned by + // `write_site_resolve_guard` in `private_config_overlay.rs`. These tests + // prove the fold itself at the same overlay + backend-extraction seam the + // post-lock rebuild composes. + + use crate::managed_agents::private_config_overlay::{test_relay_payload, PrivateConfigOverlay}; + + /// A stale disk row as the post-lock rebuild reloads it. + fn stale_disk_provider_record(pubkey: &str) -> crate::managed_agents::ManagedAgentRecord { + let mut record = record(); + record.pubkey = pubkey.into(); + record.name = "stale disk name".into(); + record.system_prompt = Some("stale disk prompt".into()); + record.backend = BackendKind::Provider { + id: "stale-provider".into(), + config: serde_json::json!({"region": "stale"}), + }; + record + } + + /// Carl round-9 P1 regression (stale-disk A / overlay B at the post-lock + /// provider rebuild): the payload actually invoked must carry the relay + /// head's backend and config, not the raw disk bytes the rebuild reloads. + #[test] + fn post_lock_rebuild_deploys_relay_config_not_stale_disk() { + let pubkey = "aa".repeat(32); + let mut payload = test_relay_payload(&pubkey); + payload.config.backend = serde_json::json!({"type":"provider","id":"relay-provider","config":{"region":"relay"}}); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(payload).unwrap(); + + let disk = stale_disk_provider_record(&pubkey); + let resolved = overlay.resolve_local_record(&disk); + let (provider_id, config) = resolved_provider_backend(&resolved).unwrap(); + + assert_eq!(provider_id, "relay-provider"); + assert_eq!(config["region"], "relay"); + // Relay-owned payload inputs follow the head too. + assert_eq!(resolved.name, "relay name"); + assert_eq!(resolved.system_prompt.as_deref(), Some("relay prompt")); + + // NEGATIVE CONTROL: an empty overlay leaves the raw disk backend — + // the assertions above prove the patch, not the fixture. + let (stale_id, stale_config) = + resolved_provider_backend(&PrivateConfigOverlay::default().resolve_local_record(&disk)) + .unwrap(); + assert_eq!(stale_id, "stale-provider"); + assert_eq!(stale_config["region"], "stale"); + } + + /// A relay head that migrated the agent back to the LOCAL backend must + /// refuse the deploy by name — the raw disk row still says "provider", + /// and deploying it would execute configuration this device displays as + /// retired. + #[test] + fn relay_head_migrated_to_local_refuses_post_lock_deploy() { + let pubkey = "bb".repeat(32); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(test_relay_payload(&pubkey)).unwrap(); // backend: local + + let disk = stale_disk_provider_record(&pubkey); + let resolved = overlay.resolve_local_record(&disk); + let error = resolved_provider_backend(&resolved).unwrap_err(); + assert!(error.contains("not provider-backed"), "{error}"); + } } diff --git a/desktop/src-tauri/src/commands/agents_lifecycle.rs b/desktop/src-tauri/src/commands/agents_lifecycle.rs new file mode 100644 index 00000000000..5f103a1c7a4 --- /dev/null +++ b/desktop/src-tauri/src/commands/agents_lifecycle.rs @@ -0,0 +1,136 @@ +use super::*; + +pub(super) async fn start_local_agent_with_preflight( + app: &AppHandle, + state: &AppState, + pubkey: &str, + allow_fresh_create_start: bool, + expected_relay_url: Option<&str>, + expected_signer_pubkey: Option<&str>, +) -> Result { + let record_snapshot = { + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let records = load_managed_agents(app)?; + let record = records + .iter() + .find(|record| record.pubkey == pubkey) + .ok_or_else(|| format!("agent {pubkey} not found"))?; + crate::managed_agents::private_config_overlay::resolved_local_record(state, record)? + }; + + if record_snapshot.backend != BackendKind::Local { + return Err(format!("agent {pubkey} is not a local agent")); + } + + // Preflight against the same resolution spawn uses — `resolve_effective_config` + // (definition → global fallback). A linked instance's own `provider`/`model`/ + // `relay_mesh` bytes never contribute: this reads the CURRENT definition + // directly, so a definition edit that flips `provider` to/from relay-mesh + // between saves is reflected here without needing a prospective re-snapshot; + // for a global-inherited blank definition, it also folds in the global + // default, which record-byte sniffing could never see. + let personas = load_personas(app).unwrap_or_default(); + let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + let mesh_model_id = + crate::managed_agents::effective_config::resolve_effective_relay_mesh_model_id( + &record_snapshot, + &personas, + &global, + ); + ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), allow_fresh_create_start).await?; + + // The mesh preflight above is the suspension window Projects callbacks + // capture their scope against: a community switch during that await + // would otherwise spawn this pair keyed to the *new* workspace relay. + // Read the workspace relay ONCE, assert the caller's captured scope + // against that exact read, and hand the same bound value to the spawn + // below — the check is tied to its use, so a switch landing after this + // point can no longer retarget the spawn (it only changes state this + // call no longer consults). + let workspace_relay_url = crate::relay::bind_expected_relay_scope( + expected_relay_url, + crate::relay::relay_ws_url_with_override(state), + )?; + // Bind the active owner after the same final await as the relay. A + // same-relay identity replacement during mesh preflight must not release + // the stale preflight owner to spawn. + let workspace_owner = + crate::relay::bind_expected_signer(expected_signer_pubkey, workspace_owner_hex(state)?)?; + + let _transition_guard = state + .managed_agent_runtime_transition + .lock() + .map_err(|e| e.to_string())?; + if state + .shutdown_started + .load(std::sync::atomic::Ordering::Acquire) + { + return Err("desktop shutdown has started".into()); + } + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let mut records = load_managed_agents(app)?; + let mut runtimes = state + .managed_agent_processes + .lock() + .map_err(|e| e.to_string())?; + let disk_record = find_managed_agent_mut(&mut records, pubkey)?; + let mut resolved_record = + crate::managed_agents::private_config_overlay::resolved_local_record(state, disk_record)?; + if resolved_record.backend != BackendKind::Local { + return Err(format!("agent {pubkey} is no longer a local agent")); + } + // Re-snapshot the persona onto the resolved spawn record at every start so + // local persona state retains its established precedence without writing + // relay-owned configuration into the device-local migration record. + // Load personas once: used for snapshot application below and summary build + // at the end — avoids a second disk read for the same file in the same call. + let personas = load_personas(app).unwrap_or_default(); + if let Some(persona_id) = resolved_record.persona_id.clone() { + match personas.iter().find(|p| p.id == persona_id) { + Some(persona) => { + crate::managed_agents::persona_events::apply_persona_snapshot( + &mut resolved_record, + persona, + ); + resolved_record.updated_at = crate::util::now_iso(); + } + None => { + return Err( + crate::managed_agents::effective_config::ORPHANED_INSTANCE_ERROR.to_string(), + ); + } + } + } + start_managed_agent_process( + app, + &mut resolved_record, + &mut runtimes, + Some(workspace_owner.as_str()), + &workspace_relay_url, + )?; + // Persist operational lifecycle metadata only. Relay-owned configuration + // remains an in-memory overlay and is never copied over device-local fields. + crate::managed_agents::private_config_overlay::copy_lifecycle_state( + disk_record, + &resolved_record, + ); + save_managed_agents(app, &records)?; + // Retain the relay-resolved configuration. The projection equality guard + // makes a runtime-only start a no-op, while avoiding resurrection of stale + // disk config when this device is following a newer relay snapshot. + retain_managed_agent_pending(app, state, &resolved_record); + build_managed_agent_summary( + app, + &resolved_record, + &runtimes, + &personas, + &load_teams(app).unwrap_or_default(), + &crate::managed_agents::load_global_agent_config(app).unwrap_or_default(), + ) +} diff --git a/desktop/src-tauri/src/commands/agents_pending.rs b/desktop/src-tauri/src/commands/agents_pending.rs index 8b9564942c6..b78fbd31387 100644 --- a/desktop/src-tauri/src/commands/agents_pending.rs +++ b/desktop/src-tauri/src/commands/agents_pending.rs @@ -22,6 +22,16 @@ use crate::{app_state::AppState, managed_agents::ManagedAgentRecord}; /// only runtime fields produces an identical row and never re-enqueues a /// publish. Best-effort: a failure here is logged and swallowed so a retention /// hiccup never blocks the disk-authoritative write. +/// +/// Also writes the just-retained kind:30179 head back through to the in-memory +/// overlay. This is the ONLY path by which the overlay learns config this +/// device authored — inbound `insert_patch` never fires for our own event +/// (the relay echo dedupes to `Skipped`) and boot hydration runs once per +/// launch — so without it a second edit in the same session resolves the +/// stale patch onto the fresher disk record and publishes a silent revert of +/// the first edit. Overlay lock is taken UNDER `managed_agents_store_lock`, +/// which every caller already holds: the established order, same as +/// `resolved_local_record`. pub(crate) fn retain_managed_agent_pending( app: &AppHandle, state: &AppState, @@ -35,7 +45,12 @@ pub(crate) fn retain_managed_agent_pending( // Shared engine with the boot-time reconcile: projection content diff // (no republish for runtime-only churn) + monotonic created_at bump // past the retained head (NIP-AP step 3). - retain_agent_record(&conn, &scope.owner_keys, record).map(|_| ()) + retain_agent_record(&conn, &scope.owner_keys, record)?; + state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())? + .absorb_retained_head(&conn, &scope.owner_keys, &record.pubkey) })(); if let Err(e) = result { eprintln!("buzz-desktop: agent-retain: {e}"); @@ -52,50 +67,16 @@ pub(crate) fn retain_managed_agent_pending( /// is retained at its own `(5, owner, agent_pubkey)` coordinate with /// `pending_sync = 1`. The `d_tag` is the agent's pubkey. Best-effort: a /// failure is logged and swallowed so a retention hiccup never blocks the -/// disk-authoritative delete. +/// disk-authoritative delete. Delegates to +/// `managed_agents::agent_events::tombstone_managed_agent_pending`, which +/// removes BOTH the public (30177) and private (30179) retained heads and +/// enqueues both tombstones in one transaction. pub(crate) fn tombstone_managed_agent_pending( app: &AppHandle, state: &AppState, agent_pubkey: &str, ) { - use crate::managed_agents::{ - agent_events::build_agent_delete, - retention::{ - delete_retained_event, open_retention_db, retain_event, tombstone_retention_d_tag, - RetainedEvent, - }, - }; - use buzz_core_pkg::kind::KIND_MANAGED_AGENT; - use nostr::JsonUtil; - - const KIND_DELETE: u32 = 5; - - let result = (|| -> Result<(), String> { - let scope = crate::managed_agents::retention::active_retention_scope(app, state)?; - let owner_pubkey = scope.owner_keys.public_key().to_hex(); - let event = build_agent_delete(agent_pubkey, &owner_pubkey)? - .sign_with_keys(&scope.owner_keys) - .map_err(|e| format!("failed to sign managed-agent tombstone: {e}"))?; - let conn = open_retention_db(&scope.db_path)?; - delete_retained_event(&conn, KIND_MANAGED_AGENT, &owner_pubkey, agent_pubkey)?; - retain_event( - &conn, - &RetainedEvent { - kind: KIND_DELETE, - pubkey: owner_pubkey, - // Key by the target coordinate so cross-kind d-tag tombstones - // occupy distinct rows (F2c). - d_tag: tombstone_retention_d_tag(KIND_MANAGED_AGENT, agent_pubkey), - content: event.content.to_string(), - created_at: event.created_at.as_secs() as i64, - raw_event: event.as_json(), - pending_sync: true, - }, - ) - })(); - if let Err(e) = result { - eprintln!("buzz-desktop: agent-tombstone: {e}"); - } + crate::managed_agents::agent_events::tombstone_managed_agent_pending(app, state, agent_pubkey); } /// Build an owner-authenticated NIP-IA `kind:9035` archive request for a deleted agent. diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index ec2357b85e7..8a033a255c0 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -415,6 +415,14 @@ pub(crate) fn commit_imported_identity( let storage = persist(&keys)?; + // Serialize the identity scope transition with inbound private-config + // handling, which holds this lock from owner resolution through overlay + // insertion. Otherwise an old-owner patch could land after this clear. + let _managed_agents_store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + // Update in-memory keys BEFORE clearing recovery flags. The Release // stores below pair with Acquire loads in get_identity: a reader // observing false is guaranteed to see the updated keys. @@ -424,6 +432,11 @@ pub(crate) fn commit_imported_identity( *active_keys = keys; state.set_identity_storage(storage); } + state + .private_managed_agent_overlay + .lock() + .map_err(|e| e.to_string())? + .clear(); // Clear both recovery flags — an import is valid in either lost or // keyring-locked state and resolves both. In the locked case the diff --git a/desktop/src-tauri/src/commands/personas/card.rs b/desktop/src-tauri/src/commands/personas/card.rs index 14c7c196b2b..bf06eb1f071 100644 --- a/desktop/src-tauri/src/commands/personas/card.rs +++ b/desktop/src-tauri/src/commands/personas/card.rs @@ -29,8 +29,8 @@ use tauri::{AppHandle, State}; use super::super::export_util::save_bytes_with_dialog; use super::snapshot::{ - memory_entries_from_listing, parse_memory_level, resolve_from_lists, - validate_snapshot_encode_size, + load_effective_managed_agents, memory_entries_from_listing, parse_memory_level, + resolve_from_lists, validate_snapshot_encode_size, }; use crate::{ app_state::AppState, @@ -43,8 +43,8 @@ use crate::{ agent_snapshot_envelope::{ decrypt_envelope, encode_locked_snapshot_png, parse_chunk_payload, ChunkPayload, }, - load_agent_definitions, load_global_agent_config, load_managed_agents, load_personas, - save_global_agent_config, validate_global_config, + load_agent_definitions, load_global_agent_config, load_personas, save_global_agent_config, + validate_global_config, }, }; @@ -493,7 +493,7 @@ pub fn card_mint_key_status( .lock() .map_err(|e| e.to_string())?; - let instances = load_managed_agents(&app)?; + let instances = load_effective_managed_agents(&app, &state)?; let definitions = load_agent_definitions(&app)?; let (record, _) = resolve_from_lists(&id, &instances, &definitions)?; @@ -549,7 +549,7 @@ pub async fn mint_agent_card( .lock() .map_err(|e| e.to_string())?; - let instances = load_managed_agents(&app)?; + let instances = load_effective_managed_agents(&app, &state)?; let definitions = load_agent_definitions(&app)?; let (record, is_definition) = resolve_from_lists(&id, &instances, &definitions).map(|(r, d)| (r.clone(), d))?; diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index 5214dd5a27e..7b13473463c 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -155,7 +155,9 @@ fn reconcile_inbound_persona_event_blocking( save_managed_agents, save_teams, team_events::team_content_from_event, }; - use buzz_core_pkg::kind::{KIND_DELETION, KIND_MANAGED_AGENT, KIND_PERSONA, KIND_TEAM}; + use buzz_core_pkg::kind::{ + KIND_DELETION, KIND_MANAGED_AGENT, KIND_PERSONA, KIND_PRIVATE_MANAGED_AGENT, KIND_TEAM, + }; use nostr::JsonUtil; let state = app.state::(); @@ -175,7 +177,15 @@ fn reconcile_inbound_persona_event_blocking( return Ok(None); } - if !matches!(kind, KIND_PERSONA | KIND_TEAM | KIND_MANAGED_AGENT) { + if !matches!( + kind, + KIND_PERSONA | KIND_TEAM | KIND_MANAGED_AGENT | KIND_PRIVATE_MANAGED_AGENT + ) { + return Ok(None); + } + + if kind == KIND_PRIVATE_MANAGED_AGENT { + reconcile_inbound_private_managed_agent(&event, &arrival_relay_url, &app, &state)?; return Ok(None); } @@ -342,6 +352,80 @@ fn reconcile_inbound_persona_event_blocking( Ok(runtime_refresh) } +fn apply_inbound_private_managed_agent_event( + event: &nostr::Event, + owner_keys: &nostr::Keys, + conn: &rusqlite::Connection, + overlay: &mut crate::managed_agents::private_config_overlay::PrivateConfigOverlay, +) -> Result { + use crate::managed_agents::{ + private_config_overlay::PrivateConfigPatch, + retention::{retain_inbound_event, InboundOutcome, RetainedEvent}, + }; + use buzz_core_pkg::{kind::KIND_PRIVATE_MANAGED_AGENT, private_managed_agent}; + use nostr::JsonUtil; + + // The codec verifies signature/owner, decrypts, validates the nsec binding, + // and rejects malformed portable config before any local state changes. + let (_, payload) = private_managed_agent::validate_and_decrypt(event, owner_keys) + .map_err(|error| format!("invalid private managed-agent event: {error}"))?; + let d_tag = payload.agent_pubkey.clone(); + // Constructing the Desktop patch validates backend-specific fields without + // mutating the live overlay or retained head. + let patch = PrivateConfigPatch::from_payload(payload)?; + let outcome = retain_inbound_event( + conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: event.pubkey.to_hex(), + d_tag, + content: event.content.clone(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: false, + }, + )?; + if outcome == InboundOutcome::Applied { + overlay.insert_patch(patch); + } + Ok(outcome) +} + +fn reconcile_inbound_private_managed_agent( + event: &nostr::Event, + arrival_relay_url: &str, + app: &AppHandle, + state: &AppState, +) -> Result<(), String> { + use crate::managed_agents::retention::{open_retention_db, InboundOutcome}; + + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + let Some(scope) = + crate::managed_agents::retention::arrival_retention_scope(app, state, arrival_relay_url)? + else { + return Ok(()); + }; + + let mut overlay = state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())?; + let conn = open_retention_db(&scope.db_path)?; + let outcome = + apply_inbound_private_managed_agent_event(event, &scope.owner_keys, &conn, &mut overlay)?; + if outcome == InboundOutcome::Skipped { + return Ok(()); + } + + drop(overlay); + try_regenerate_nest(app); + let _ = app.emit("agents-data-changed", ()); + Ok(()) +} + fn validate_inbound_persona_definition(persona: &AgentDefinition) -> Result<(), String> { crate::managed_agents::validate_agent_definition_text( &persona.display_name, @@ -403,12 +487,118 @@ fn parse_deletion_coordinate(event: &nostr::Event) -> Option<(u32, String)> { }) } +/// Resolve an inbound kind:5 tombstone against the scope owner and the +/// retention store: authorize it, retain it, and return the +/// `(target_kind, target_d_tag)` the caller must delete locally — or `None` +/// when the tombstone must no-op. +/// +/// Authorization is TWO bindings, both enforced in Rust because the frontend +/// owner filter reads attacker-controlled fields and callable IPC bypasses it +/// anyway: `parse_deletion_coordinate` proves the signer owns the coordinate +/// it NAMES, and the check here proves that signer is the active workspace +/// owner. The local stores match by d-tag alone, so without the second +/// binding a validly signed foreign-owner tombstone naming its OWN coordinate +/// with a colliding d-tag would delete this owner's record. +/// +/// The covered retained upsert head is deleted in the SAME transaction as the +/// kind:5 retain, mirroring the local delete path +/// (`tombstone_managed_agent_pending`). Without this the retained kind:30179 +/// head survives the tombstone and the next boot's `hydrate_from_retention` +/// rematerializes the deleted secret-bearing config into the overlay +/// (head → tombstone → restart resurrection). +fn resolve_inbound_tombstone( + event: &nostr::Event, + scope_owner: &nostr::PublicKey, + conn: &rusqlite::Connection, +) -> Result, String> { + use crate::managed_agents::retention::{ + delete_retained_event, get_retained_event, retain_inbound_event, tombstone_retention_d_tag, + InboundOutcome, RetainedEvent, + }; + use buzz_core_pkg::kind::{ + KIND_DELETION, KIND_MANAGED_AGENT, KIND_PERSONA, KIND_PRIVATE_MANAGED_AGENT, KIND_TEAM, + }; + use nostr::JsonUtil; + + if event.pubkey != *scope_owner { + return Ok(None); // foreign-owner tombstone: not authorized in this scope + } + let Some((target_kind, target_d_tag)) = parse_deletion_coordinate(event) else { + return Ok(None); // no routable coordinate — nothing to delete + }; + if !matches!( + target_kind, + KIND_PERSONA | KIND_TEAM | KIND_MANAGED_AGENT | KIND_PRIVATE_MANAGED_AGENT + ) { + return Ok(None); // deletion for a kind we don't track locally + } + + // Resolve against the retained tombstone row (keyed by the target + // coordinate, F2c) so a re-received tombstone or one older than a pending + // local edit is a no-op. Retain + head delete commit atomically so a crash + // between them cannot leave a consumed tombstone with a live head. + let transaction = conn + .unchecked_transaction() + .map_err(|error| format!("failed to begin inbound tombstone transaction: {error}"))?; + let outcome = retain_inbound_event( + &transaction, + &RetainedEvent { + kind: KIND_DELETION, + pubkey: event.pubkey.to_hex(), + d_tag: tombstone_retention_d_tag(target_kind, &target_d_tag), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: false, + }, + )?; + if outcome == InboundOutcome::Skipped { + return Ok(None); + } + // The two agent kinds are one record locally (the match arm below removes + // the disk record AND the overlay entry for either), so both retained + // heads are covered — the local delete path tombstones both coordinates, + // but the sibling kind:5 may be lost or unsent, and a surviving 30179 head + // would rematerialize the agent at the next boot hydration. + let covered_kinds: &[u32] = + if matches!(target_kind, KIND_MANAGED_AGENT | KIND_PRIVATE_MANAGED_AGENT) { + &[KIND_MANAGED_AGENT, KIND_PRIVATE_MANAGED_AGENT] + } else { + std::slice::from_ref(&target_kind) + }; + // NIP-09: a deletion covers events up to its own `created_at`. A retained + // head NEWER than the tombstone means the record was recreated after the + // deletion (or the tombstone is a late replay) — keep the record and its + // heads. The kind:5 row retained above still dedupes future replays. + let owner_hex = event.pubkey.to_hex(); + let tombstone_created_at = event.created_at.as_secs() as i64; + for covered_kind in covered_kinds { + if let Some(head) = + get_retained_event(&transaction, *covered_kind, &owner_hex, &target_d_tag)? + { + if head.created_at > tombstone_created_at { + transaction.commit().map_err(|error| { + format!("failed to commit inbound tombstone transaction: {error}") + })?; + return Ok(None); + } + } + } + for covered_kind in covered_kinds { + delete_retained_event(&transaction, *covered_kind, &owner_hex, &target_d_tag)?; + } + transaction + .commit() + .map_err(|error| format!("failed to commit inbound tombstone transaction: {error}"))?; + Ok(Some((target_kind, target_d_tag))) +} + /// Apply an inbound kind:5 NIP-09 deletion: remove the local record at the /// tombstone's target coordinate, scoped per-kind. Mirrors the upsert spine — /// arrival-scoped retention resolution under the store lock, then a per-kind /// store mutation — but removes rather than patches. Unknown/malformed /// coordinates no-op, as does a tombstone whose arrival community is no longer -/// active. +/// active or whose signer is not the scope owner. fn reconcile_inbound_tombstone( event: &nostr::Event, arrival_relay_url: &str, @@ -416,54 +606,32 @@ fn reconcile_inbound_tombstone( state: &AppState, ) -> Result<(), String> { use crate::managed_agents::{ - load_managed_agents, load_teams, - retention::{ - open_retention_db, retain_inbound_event, tombstone_retention_d_tag, InboundOutcome, - RetainedEvent, - }, - save_managed_agents, save_teams, + load_managed_agents, load_teams, retention::open_retention_db, save_managed_agents, + save_teams, }; - use buzz_core_pkg::kind::{KIND_DELETION, KIND_MANAGED_AGENT, KIND_PERSONA, KIND_TEAM}; - use nostr::JsonUtil; - - let Some((target_kind, target_d_tag)) = parse_deletion_coordinate(event) else { - return Ok(()); // no routable coordinate — nothing to delete + use buzz_core_pkg::kind::{ + KIND_MANAGED_AGENT, KIND_PERSONA, KIND_PRIVATE_MANAGED_AGENT, KIND_TEAM, }; - if !matches!(target_kind, KIND_PERSONA | KIND_TEAM | KIND_MANAGED_AGENT) { - return Ok(()); // deletion for a kind we don't track locally - } let _store_guard = state .managed_agents_store_lock .lock() .map_err(|error| error.to_string())?; - // Resolve against the retained tombstone row (keyed by the target - // coordinate, F2c) so a re-received tombstone or one older than a pending - // local edit is a no-op. Scoped to the arrival community, so a workspace - // switch since arrival drops the tombstone instead of retaining it — and - // deleting a record — in the wrong community's store. + // Scoped to the arrival community, so a workspace switch since arrival + // drops the tombstone instead of retaining it — and deleting a record — + // in the wrong community's store. let Some(scope) = crate::managed_agents::retention::arrival_retention_scope(app, state, arrival_relay_url)? else { return Ok(()); }; let conn = open_retention_db(&scope.db_path)?; - let outcome = retain_inbound_event( - &conn, - &RetainedEvent { - kind: KIND_DELETION, - pubkey: event.pubkey.to_hex(), - d_tag: tombstone_retention_d_tag(target_kind, &target_d_tag), - content: event.content.to_string(), - created_at: event.created_at.as_secs() as i64, - raw_event: event.as_json(), - pending_sync: false, - }, - )?; - if outcome == InboundOutcome::Skipped { + let Some((target_kind, target_d_tag)) = + resolve_inbound_tombstone(event, &scope.owner_keys.public_key(), &conn)? + else { return Ok(()); - } + }; // Remove the local record using the SAME per-kind match rule the apply fns // use: persona by `persona_d_tag`, team by `id`, managed-agent by `pubkey`. @@ -478,7 +646,12 @@ fn reconcile_inbound_tombstone( teams.retain(|record| record.id != target_d_tag); save_teams(app, &teams)?; } - KIND_MANAGED_AGENT => { + KIND_MANAGED_AGENT | KIND_PRIVATE_MANAGED_AGENT => { + state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())? + .remove(&target_d_tag); let mut agents = load_managed_agents(app)?; agents.retain(|record| record.pubkey != target_d_tag); save_managed_agents(app, &agents)?; diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_security_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_security_tests.rs new file mode 100644 index 00000000000..c5ada4fa799 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_security_tests.rs @@ -0,0 +1,351 @@ +//! Security-focused inbound regressions: tombstone (kind:5) authorization +//! and retained-head coverage, the inbound signature gate, and +//! restart-rehydration of the private-config overlay. +//! +//! Extracted from `inbound_tests.rs` to keep it under the file-size cap; +//! `#[path]`-included from there. Helpers (`AGENT_PUBKEY`, +//! `private_agent_payload`, `local_agent`) live in the parent module. + +#[allow(unused_imports)] +use super::super::*; +use super::*; + +// ── Tombstone (kind:5) consume ──────────────────────────────────────────── + +fn deletion_event(coord: &str) -> nostr::Event { + deletion_event_with_keys(coord, &nostr::Keys::generate()) +} + +fn deletion_event_with_keys(coord: &str, keys: &nostr::Keys) -> nostr::Event { + use nostr::{EventBuilder, JsonUtil, Kind, Tag}; + let event = EventBuilder::new(Kind::Custom(5), "") + .tags(vec![Tag::parse(["a", coord]).unwrap()]) + .sign_with_keys(keys) + .unwrap(); + nostr::Event::from_json(event.as_json()).unwrap() +} + +/// A deletion event whose coordinate owner IS its signer — the only shape +/// `parse_deletion_coordinate` accepts since the owner check landed. +fn owned_deletion_event(kind: u32, d_tag: &str) -> nostr::Event { + let keys = nostr::Keys::generate(); + let owner = keys.public_key().to_hex(); + deletion_event_with_keys(&format!("{kind}:{owner}:{d_tag}"), &keys) +} + +/// Like [`deletion_event_with_keys`] but with a controlled `created_at`, for +/// tests that race a tombstone against a retained head's timestamp. +fn owned_deletion_event_at( + keys: &nostr::Keys, + kind: u32, + d_tag: &str, + created_at: u64, +) -> nostr::Event { + use nostr::{EventBuilder, JsonUtil, Kind, Tag, Timestamp}; + let owner = keys.public_key().to_hex(); + let event = EventBuilder::new(Kind::Custom(5), "") + .tags(vec![ + Tag::parse(["a", &format!("{kind}:{owner}:{d_tag}")]).unwrap() + ]) + .custom_created_at(Timestamp::from(created_at)) + .sign_with_keys(keys) + .unwrap(); + nostr::Event::from_json(event.as_json()).unwrap() +} + +// ── resolve_inbound_tombstone: authorization + retained-head coverage ────── +// +// The seam is deliberately a pure function over (event, scope_owner, conn) so +// the security decision is testable WITHOUT a live AppHandle — the production +// caller (`reconcile_inbound_tombstone`) is a `#[tauri::command]` body that +// only adds scope resolution and the per-kind store mutation around it. These +// are the direct-IPC regressions: the frontend owner filter never runs here. + +/// Issue-4 regression: a VALIDLY SIGNED tombstone whose signer owns the +/// coordinate it names — so it clears `parse_deletion_coordinate` — but who is +/// not the workspace owner must no-op. Local stores match by d-tag alone, so +/// without the scope-owner binding this would delete the victim's record. +#[test] +fn resolve_inbound_tombstone_rejects_foreign_owner() { + use crate::managed_agents::retention::open_retention_db; + use std::path::Path; + + let conn = open_retention_db(Path::new(":memory:")).unwrap(); + let scope_owner_keys = nostr::Keys::generate(); + let attacker_keys = nostr::Keys::generate(); + + // The attacker names their OWN coordinate (passes the NIP-09 signer == + // coordinate-owner check) with a d-tag colliding with the victim's agent. + let forged = owned_deletion_event_at(&attacker_keys, 30177, AGENT_PUBKEY, 100); + let resolved = + resolve_inbound_tombstone(&forged, &scope_owner_keys.public_key(), &conn).unwrap(); + assert_eq!(resolved, None, "foreign-owner tombstone must not authorize"); + + // Nothing was retained: the attacker cannot even park a kind:5 row in the + // victim's scoped store. + assert!( + !crate::managed_agents::retention::has_retained_personas( + &conn, + &attacker_keys.public_key().to_hex() + ) + .unwrap(), + "a rejected tombstone must leave no retained row" + ); + + // POSITIVE CONTROL: the same event shape signed by the scope owner + // resolves — proving the rejection above is the owner binding, not a + // broken fixture. + let owned = owned_deletion_event_at(&scope_owner_keys, 30177, AGENT_PUBKEY, 100); + assert_eq!( + resolve_inbound_tombstone(&owned, &scope_owner_keys.public_key(), &conn).unwrap(), + Some((30177, AGENT_PUBKEY.to_string())), + "control: the scope owner's tombstone authorizes" + ); +} + +/// Issue-2 regression: head → tombstone → restart. The tombstone must remove +/// the retained kind:30179 head so a fresh boot's `hydrate_from_retention` +/// cannot rematerialize the deleted secret-bearing agent. +#[test] +fn tombstoned_private_head_does_not_survive_restart_hydration() { + 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 agent_hex = agent_keys.public_key().to_hex(); + + // Session 1: a private head lands and is retained durably. + { + let conn = open_retention_db(&db_path).unwrap(); + let payload = private_agent_payload(&owner_keys, &agent_keys, "doomed", 4); + let event = private_managed_agent::build_event(&owner_keys, &payload, 20).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + apply_inbound_private_managed_agent_event(&event, &owner_keys, &conn, &mut overlay) + .unwrap(); + + // The delete arrives: a kind:5 covering the PUBLIC coordinate (either + // agent kind must cover both heads — the sibling tombstone may be + // lost). + let tombstone = owned_deletion_event_at(&owner_keys, 30177, &agent_hex, 30); + let resolved = + resolve_inbound_tombstone(&tombstone, &owner_keys.public_key(), &conn).unwrap(); + assert_eq!(resolved, Some((30177, agent_hex.clone()))); + } + + // Session 2 (restart, fresh overlay): hydration must NOT resurrect it. + let conn = open_retention_db(&db_path).unwrap(); + let overlay = hydrate_from_retention(&conn, &owner_keys).unwrap(); + assert!( + overlay.resolved_records(&[]).is_empty(), + "a tombstoned 30179 head must not rehydrate after restart" + ); + + // Replay: the same tombstone re-received dedupes to a no-op instead of + // re-deleting (retention row already at its created_at). + let replay = owned_deletion_event_at(&owner_keys, 30177, &agent_hex, 30); + assert_eq!( + resolve_inbound_tombstone(&replay, &owner_keys.public_key(), &conn).unwrap(), + None, + "a replayed tombstone must dedupe" + ); +} + +/// NIP-09 bound: a deletion covers events up to its own `created_at`. A head +/// NEWER than the tombstone is a recreation (or the tombstone is a late +/// replay) — the record and its heads must survive. +#[test] +fn tombstone_older_than_the_retained_head_preserves_it() { + use crate::managed_agents::{ + private_config_overlay::{hydrate_from_retention, PrivateConfigOverlay}, + retention::open_retention_db, + }; + use buzz_core_pkg::private_managed_agent; + use std::path::Path; + + let conn = open_retention_db(Path::new(":memory:")).unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let agent_hex = agent_keys.public_key().to_hex(); + + // Head at t=50; tombstone authored earlier at t=30 arrives late. + let payload = private_agent_payload(&owner_keys, &agent_keys, "survivor", 4); + let event = private_managed_agent::build_event(&owner_keys, &payload, 50).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + apply_inbound_private_managed_agent_event(&event, &owner_keys, &conn, &mut overlay).unwrap(); + + let stale_tombstone = owned_deletion_event_at(&owner_keys, 30179, &agent_hex, 30); + assert_eq!( + resolve_inbound_tombstone(&stale_tombstone, &owner_keys.public_key(), &conn).unwrap(), + None, + "a tombstone older than the head must not delete it" + ); + let overlay = hydrate_from_retention(&conn, &owner_keys).unwrap(); + assert_eq!( + overlay.resolved_records(&[]).len(), + 1, + "the newer head survives the stale tombstone" + ); +} + +#[test] +fn parse_deletion_coordinate_extracts_kind_and_d_tag() { + // Persona / team / agent coordinates all route by their leading kind. + let p = owned_deletion_event(30175, "my-persona"); + assert_eq!( + parse_deletion_coordinate(&p), + Some((30175, "my-persona".to_string())) + ); + let a = owned_deletion_event(30177, "agentpubkeyhex"); + assert_eq!( + parse_deletion_coordinate(&a), + Some((30177, "agentpubkeyhex".to_string())) + ); +} + +#[test] +fn parse_deletion_coordinate_rejects_foreign_owner() { + // A validly signed kind:5 naming ANOTHER owner's coordinate must no-op: + // NIP-09 scopes deletion to the record's own author. + let foreign_owner = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"; + let forged = deletion_event(&format!("30175:{foreign_owner}:my-persona")); + assert_eq!(parse_deletion_coordinate(&forged), None); +} + +#[test] +fn parse_deletion_coordinate_handles_colon_in_d_tag_and_rejects_malformed() { + // A d-tag containing ':' keeps its remainder intact (splitn(3)). + let weird = owned_deletion_event(30176, "a:b:c"); + assert_eq!( + parse_deletion_coordinate(&weird), + Some((30176, "a:b:c".to_string())) + ); + // Missing d-tag segment / non-numeric kind → None (no-op). + assert_eq!( + parse_deletion_coordinate(&deletion_event("30175:owner")), + None + ); + assert_eq!( + parse_deletion_coordinate(&deletion_event("notakind:owner:d")), + None + ); +} + +#[test] +fn tombstone_removal_predicates_match_apply_fn_keys() { + // The deletion path removes by the SAME per-kind key the apply fns use. + // Persona: by persona_d_tag (slug/id). + let mut personas = vec![local_in_app()]; + let target = persona_d_tag(&personas[0]); + personas.retain(|r| persona_d_tag(r) != target); + assert!(personas.is_empty(), "persona removed by its d-tag"); + + // Team: by id. + let mut teams = vec![local_team()]; + teams.retain(|r| r.id != TEAM_ID); + assert!(teams.is_empty(), "team removed by id"); + + // Managed-agent: by pubkey. A non-matching d-tag is a no-op. + let mut agents = vec![local_agent()]; + agents.retain(|r| r.pubkey != "someoneelse"); + assert_eq!(agents.len(), 1, "non-matching agent tombstone no-ops"); + agents.retain(|r| r.pubkey != AGENT_PUBKEY); + assert!(agents.is_empty(), "agent removed by pubkey"); +} + +// ── Inbound signature gate ────────────────────────────────────────────────── + +#[test] +fn inbound_gate_rejects_tampered_event() { + use nostr::JsonUtil; + // A validly signed event whose content was altered post-signing: the + // pubkey is real, the sig no longer covers the bytes. Must die at the + // gate before any store logic runs. + let keys = nostr::Keys::generate(); + let event = nostr::EventBuilder::new(nostr::Kind::Custom(30175), "{}") + .tags(vec![nostr::Tag::parse(["d", "victim-slug"]).unwrap()]) + .sign_with_keys(&keys) + .unwrap(); + let tampered = event.as_json().replace( + "\"content\":\"{}\"", + "\"content\":\"{\\\"system_prompt\\\":\\\"pwned\\\"}\"", + ); + assert_ne!( + tampered, + event.as_json(), + "string replace must have taken effect — if this fails the test is testing an un-tampered event" + ); + + let err = parse_verified_inbound_event(&tampered).unwrap_err(); + assert!( + err.contains("signature"), + "tampered event must fail the signature gate: {err}" + ); +} + +#[test] +fn inbound_gate_accepts_validly_signed_event() { + use nostr::JsonUtil; + let keys = nostr::Keys::generate(); + let event = nostr::EventBuilder::new(nostr::Kind::Custom(30175), "{}") + .tags(vec![nostr::Tag::parse(["d", "slug"]).unwrap()]) + .sign_with_keys(&keys) + .unwrap(); + 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/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index fbfede35886..e2679fa69b5 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -2,6 +2,7 @@ //! Extracted from the parent module to keep it under the file-size cap. use super::*; +use nostr::{JsonUtil, ToBech32}; use std::collections::BTreeMap; const UUID: &str = "11111111-2222-3333-4444-555555555555"; // sadscan:disable sq.pii.cc.visa -- fixed test UUID @@ -155,6 +156,185 @@ fn no_local_match_inserts_inbound_reusing_d_tag_as_id() { const AGENT_PUBKEY: &str = "agentpubkeyhex0000000000000000000000000000000000000000000000000000"; +fn private_agent_payload( + owner_keys: &nostr::Keys, + agent_keys: &nostr::Keys, + name: &str, + parallelism: u32, +) -> buzz_core_pkg::private_managed_agent::Payload { + use buzz_core_pkg::private_managed_agent::{ + Payload, PrivateConfig, PrivateIdentity, FORMAT, VERSION, + }; + + Payload { + format: FORMAT.into(), + version: VERSION, + agent_pubkey: agent_keys.public_key().to_hex(), + owner_pubkey: owner_keys.public_key().to_hex(), + generation: 1, + previous_event_id: None, + updated_at: "2026-08-06T00:00:00Z".into(), + identity: PrivateIdentity { + private_key_nsec: agent_keys.secret_key().to_bech32().unwrap(), + auth_tag: None, + }, + config: PrivateConfig { + relay_url: "wss://relay.example".into(), + name: name.into(), + persona_id: None, + runtime: Some("goose".into()), + model: None, + provider: None, + system_prompt: Some("relay prompt".into()), + parallelism: Some(parallelism), + respond_to: None, + respond_to_allowlist: vec![], + agent_command_override: None, + agent_args: vec![], + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + env_vars: BTreeMap::new(), + backend: serde_json::json!({"type":"local"}), + backend_agent_id: None, + team_id: None, + persona_name_in_team: None, + relay_mesh: None, + extra: serde_json::Map::new(), + }, + extensions: BTreeMap::new(), + extra: serde_json::Map::new(), + } +} + +#[test] +fn private_agent_inbound_rejects_before_retain_and_stale_event_preserves_overlay() { + use crate::managed_agents::{ + private_config_overlay::PrivateConfigOverlay, + retention::{get_retained_event, open_retention_db, InboundOutcome}, + }; + use buzz_core_pkg::{kind::KIND_PRIVATE_MANAGED_AGENT, private_managed_agent}; + use tempfile::TempDir; + + let dir = TempDir::new().unwrap(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let mut overlay = PrivateConfigOverlay::default(); + + let valid = private_agent_payload(&owner_keys, &agent_keys, "new", 4); + let newer_event = private_managed_agent::build_event(&owner_keys, &valid, 20).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, "new"); + + let mut malformed = private_agent_payload(&owner_keys, &agent_keys, "malformed", 4); + malformed.generation = 2; + malformed.previous_event_id = Some(newer_event.id.to_hex()); + malformed.config.backend = serde_json::json!({"type":"provider"}); + let malformed_event = private_managed_agent::build_event(&owner_keys, &malformed, 30).unwrap(); + assert!(apply_inbound_private_managed_agent_event( + &malformed_event, + &owner_keys, + &conn, + &mut overlay, + ) + .is_err()); + assert_eq!(overlay.resolved_records(&[])[0].name, "new"); + let retained = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + assert_eq!(retained.raw_event, newer_event.as_json()); + + let stale = private_agent_payload(&owner_keys, &agent_keys, "stale", 2); + let stale_event = private_managed_agent::build_event(&owner_keys, &stale, 10).unwrap(); + assert_eq!( + apply_inbound_private_managed_agent_event(&stale_event, &owner_keys, &conn, &mut overlay,) + .unwrap(), + InboundOutcome::Skipped + ); + 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 { @@ -721,135 +901,10 @@ fn inbound_team_propagates_persist_teams_error() { assert_eq!(err, "disk full"); } -// ── Tombstone (kind:5) consume ──────────────────────────────────────────── - -fn deletion_event(coord: &str) -> nostr::Event { - deletion_event_with_keys(coord, &nostr::Keys::generate()) -} - -fn deletion_event_with_keys(coord: &str, keys: &nostr::Keys) -> nostr::Event { - use nostr::{EventBuilder, JsonUtil, Kind, Tag}; - let event = EventBuilder::new(Kind::Custom(5), "") - .tags(vec![Tag::parse(["a", coord]).unwrap()]) - .sign_with_keys(keys) - .unwrap(); - nostr::Event::from_json(event.as_json()).unwrap() -} - -/// A deletion event whose coordinate owner IS its signer — the only shape -/// `parse_deletion_coordinate` accepts since the owner check landed. -fn owned_deletion_event(kind: u32, d_tag: &str) -> nostr::Event { - let keys = nostr::Keys::generate(); - let owner = keys.public_key().to_hex(); - deletion_event_with_keys(&format!("{kind}:{owner}:{d_tag}"), &keys) -} - -#[test] -fn parse_deletion_coordinate_extracts_kind_and_d_tag() { - // Persona / team / agent coordinates all route by their leading kind. - let p = owned_deletion_event(30175, "my-persona"); - assert_eq!( - parse_deletion_coordinate(&p), - Some((30175, "my-persona".to_string())) - ); - let a = owned_deletion_event(30177, "agentpubkeyhex"); - assert_eq!( - parse_deletion_coordinate(&a), - Some((30177, "agentpubkeyhex".to_string())) - ); -} - -#[test] -fn parse_deletion_coordinate_rejects_foreign_owner() { - // A validly signed kind:5 naming ANOTHER owner's coordinate must no-op: - // NIP-09 scopes deletion to the record's own author. - let foreign_owner = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"; - let forged = deletion_event(&format!("30175:{foreign_owner}:my-persona")); - assert_eq!(parse_deletion_coordinate(&forged), None); -} - -#[test] -fn parse_deletion_coordinate_handles_colon_in_d_tag_and_rejects_malformed() { - // A d-tag containing ':' keeps its remainder intact (splitn(3)). - let weird = owned_deletion_event(30176, "a:b:c"); - assert_eq!( - parse_deletion_coordinate(&weird), - Some((30176, "a:b:c".to_string())) - ); - // Missing d-tag segment / non-numeric kind → None (no-op). - assert_eq!( - parse_deletion_coordinate(&deletion_event("30175:owner")), - None - ); - assert_eq!( - parse_deletion_coordinate(&deletion_event("notakind:owner:d")), - None - ); -} - -#[test] -fn tombstone_removal_predicates_match_apply_fn_keys() { - // The deletion path removes by the SAME per-kind key the apply fns use. - // Persona: by persona_d_tag (slug/id). - let mut personas = vec![local_in_app()]; - let target = persona_d_tag(&personas[0]); - personas.retain(|r| persona_d_tag(r) != target); - assert!(personas.is_empty(), "persona removed by its d-tag"); - - // Team: by id. - let mut teams = vec![local_team()]; - teams.retain(|r| r.id != TEAM_ID); - assert!(teams.is_empty(), "team removed by id"); - - // Managed-agent: by pubkey. A non-matching d-tag is a no-op. - let mut agents = vec![local_agent()]; - agents.retain(|r| r.pubkey != "someoneelse"); - assert_eq!(agents.len(), 1, "non-matching agent tombstone no-ops"); - agents.retain(|r| r.pubkey != AGENT_PUBKEY); - assert!(agents.is_empty(), "agent removed by pubkey"); -} - -// ── Inbound signature gate ────────────────────────────────────────────────── - -#[test] -fn inbound_gate_rejects_tampered_event() { - use nostr::JsonUtil; - // A validly signed event whose content was altered post-signing: the - // pubkey is real, the sig no longer covers the bytes. Must die at the - // gate before any store logic runs. - let keys = nostr::Keys::generate(); - let event = nostr::EventBuilder::new(nostr::Kind::Custom(30175), "{}") - .tags(vec![nostr::Tag::parse(["d", "victim-slug"]).unwrap()]) - .sign_with_keys(&keys) - .unwrap(); - let tampered = event.as_json().replace( - "\"content\":\"{}\"", - "\"content\":\"{\\\"system_prompt\\\":\\\"pwned\\\"}\"", - ); - assert_ne!( - tampered, - event.as_json(), - "string replace must have taken effect — if this fails the test is testing an un-tampered event" - ); - - let err = parse_verified_inbound_event(&tampered).unwrap_err(); - assert!( - err.contains("signature"), - "tampered event must fail the signature gate: {err}" - ); -} - -#[test] -fn inbound_gate_accepts_validly_signed_event() { - use nostr::JsonUtil; - let keys = nostr::Keys::generate(); - let event = nostr::EventBuilder::new(nostr::Kind::Custom(30175), "{}") - .tags(vec![nostr::Tag::parse(["d", "slug"]).unwrap()]) - .sign_with_keys(&keys) - .unwrap(); - let parsed = parse_verified_inbound_event(&event.as_json()).unwrap(); - assert_eq!(parsed.pubkey, keys.public_key()); -} +// Tombstone authorization, signature-gate, and restart-rehydration +// regressions live in a sibling file to stay under the file-size cap. +#[path = "inbound_security_tests.rs"] +mod security_tests; #[test] fn inbound_persona_rejects_invisible_definition_text() { diff --git a/desktop/src-tauri/src/commands/personas/snapshot.rs b/desktop/src-tauri/src/commands/personas/snapshot.rs index e7bd1597e63..84fb706aaa9 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot.rs @@ -56,6 +56,29 @@ pub(crate) fn resolve_from_lists<'a>( Err(format!("agent {id:?} not found")) } +/// Load the managed-agent instances with the kind:30179 private-config +/// overlay folded on — patching disk records and synthesizing relay-only +/// entries, the same resolution the agent list uses. +/// +/// Every export/card resolver must read instances through this helper, never +/// raw `load_managed_agents`: raw disk exports stale values on a follower +/// device and fails with "agent not found" for a relay-only agent that has +/// never been started on this device. +/// +/// Lock order: callers already hold `managed_agents_store_lock` (outer); +/// this takes only the overlay lock (inner) and releases it before returning. +pub(crate) fn load_effective_managed_agents( + app: &AppHandle, + state: &State<'_, AppState>, +) -> Result, String> { + let instances = load_managed_agents(app)?; + Ok(state + .private_managed_agent_overlay + .lock() + .map_err(|e| e.to_string())? + .resolved_records(&instances)) +} + /// Validate that `memory_source_pubkey` is an appropriate source for a /// memory-bearing snapshot export. /// @@ -245,7 +268,10 @@ pub(crate) async fn materialize_snapshot_bytes( .lock() .map_err(|e| e.to_string())?; - let instances = load_managed_agents(&app)?; + // Fold the kind:30179 overlay onto the disk records (and synthesize + // relay-only entries) so export sees the EFFECTIVE config — see + // `load_effective_managed_agents`. + let instances = load_effective_managed_agents(&app, &state)?; let definitions = load_agent_definitions(&app)?; let (def_record, is_definition) = resolve_from_lists(&id, &instances, &definitions) .map(|(r, is_def)| (r.clone(), is_def))?; @@ -445,6 +471,9 @@ pub async fn encode_agent_snapshot_for_send( }) } +#[cfg(test)] +#[path = "snapshot/tests_export_resolver_guard.rs"] +mod export_resolver_guard; #[cfg(test)] mod fidelity_tests; #[cfg(test)] diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index fedb0e60585..49e9dfc8a8b 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -163,6 +163,12 @@ fn definition_slug_resolves_to_definition_and_linked_instance_is_valid_memory_so // ── Resolver edge cases ─────────────────────────────────────────────────── +// ── Overlay-fold resolution (kind:30179) ────────────────────────────────── +// Sibling file: keeps this file under the 1000-line gate. +#[cfg(not(target_os = "windows"))] +#[path = "tests_overlay_fold.rs"] +mod overlay_fold; + #[test] fn resolve_by_pubkey_finds_keyed_instance() { let inst = make_instance("pubkey-xyz", "my-agent"); diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests_export_resolver_guard.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests_export_resolver_guard.rs new file mode 100644 index 00000000000..65d0308b0c5 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests_export_resolver_guard.rs @@ -0,0 +1,117 @@ +//! Source guard: every export resolver folds the private-config overlay. +//! +//! Kept in a sibling file so `snapshot/tests.rs` stays under the 1000-line +//! gate; `#[path]`-included from `snapshot.rs` as a child module. +//! +//! The behavioural tests in `tests.rs`/`tests_overlay_fold.rs` model the fold +//! by calling `resolved_records` / `resolve_from_lists` directly. They CANNOT +//! reach the production call sites: all three live inside `#[tauri::command]` +//! bodies needing a live `AppHandle` + `State`. Measured before this +//! guard existed — deleting any one of the three production folds left the +//! full workspace suite green (2283 passed, 0 failed). +//! +//! Same weakness, same remedy, as `write_site_resolve_guard` in +//! `private_config_overlay.rs`: assert the call exists at each site. +//! +//! The fold lives in one helper, `load_effective_managed_agents`, so the +//! guarded invariant has three legs: +//! 1. every resolver site calls the helper (exact count per file); +//! 2. no resolver file reads raw `load_managed_agents(` outside the helper; +//! 3. the helper itself still performs the fold. + +/// Call-shaped needle for the helper. A resolver that reads raw disk instead +/// fails with "agent not found" for a relay-only agent that has never started +/// on this device, and bakes stale disk values into the exported manifest on +/// a follower device. +const HELPER_CALL: &str = "load_effective_managed_agents(&app, &state)?"; + +/// The fold inside the helper. +const FOLD_CALL: &str = ".resolved_records(&instances)"; + +/// Raw-disk read. Exact counts per file: the only permitted occurrence is the +/// one inside the helper in `snapshot.rs`. +const RAW_LOAD_CALL: &str = "load_managed_agents("; + +/// `(file, source, expected_helper_calls, allowed_raw_loads)` — every export +/// resolver that turns an id into a record destined for a snapshot manifest +/// or a card. +fn sites() -> Vec<(&'static str, &'static str, usize, usize)> { + vec![ + // `materialize_snapshot_bytes` (JSON + PNG + send-to-channel): 1 + // helper call. 1 raw load allowed: the helper's own body. + ( + "commands/personas/snapshot.rs", + include_str!("../snapshot.rs"), + 1, + 1, + ), + // Card precheck (`card_mint_key_status`) + mint resolver + // (`mint_agent_card`): 2 helper calls, 0 raw loads. Exact, not + // `>= 1`: a lower bound would not notice one site losing its call + // while the other gained a second. + ( + "commands/personas/card.rs", + include_str!("../card.rs"), + 2, + 0, + ), + ] +} + +#[test] +fn every_export_resolver_folds_the_private_config_overlay() { + for (file, source, expected, allowed_raw) in sites() { + let found = source.matches(HELPER_CALL).count(); + assert_eq!( + found, expected, + "{file}: expected {expected} `{HELPER_CALL}` call(s), found {found}. \ + An export resolver that reads raw disk fails with \"agent not found\" \ + for a relay-only agent that has never started on this device, and \ + bakes stale disk values into the exported manifest on a follower." + ); + let raw = source.matches(RAW_LOAD_CALL).count(); + assert_eq!( + raw, allowed_raw, + "{file}: expected {allowed_raw} raw `{RAW_LOAD_CALL}` call(s), found \ + {raw}. A resolver bypassing `load_effective_managed_agents` reads \ + raw disk and skips the kind:30179 overlay fold." + ); + } +} + +/// The helper must actually fold — the resolver-site counts above would pass +/// vacuously if `load_effective_managed_agents` degraded to a plain load. +#[test] +fn the_helper_itself_performs_the_fold() { + let source = include_str!("../snapshot.rs"); + let found = source.matches(FOLD_CALL).count(); + assert_eq!( + found, 1, + "snapshot.rs: expected exactly 1 `{FOLD_CALL}` call (inside \ + `load_effective_managed_agents`), found {found}" + ); +} + +/// The guards above are substring counts, so prove they can FAIL: a source +/// with the calls removed must not satisfy them. Without this, a typo in a +/// needle would make every row pass vacuously. +#[test] +fn guard_detects_missing_calls() { + for (file, source, expected, _) in sites() { + let stripped = source.replace(HELPER_CALL, "REMOVED(&app, &state)?"); + assert_eq!( + stripped.matches(HELPER_CALL).count(), + 0, + "{file}: helper needle never matched, so its {expected} expected \ + call(s) passed vacuously" + ); + } + let source = include_str!("../snapshot.rs"); + let stripped = source.replace(FOLD_CALL, ".REMOVED(&instances)"); + assert_eq!( + stripped.matches(FOLD_CALL).count(), + 0, + "snapshot.rs: fold needle never matched, so the helper check passed \ + vacuously" + ); +} diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests_overlay_fold.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests_overlay_fold.rs new file mode 100644 index 00000000000..97a4c65b930 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests_overlay_fold.rs @@ -0,0 +1,114 @@ +//! Overlay-fold resolution (kind:30179) tests for export. +//! +//! Kept in a sibling file so `snapshot/tests.rs` stays under the +//! 1000-line gate; `#[path]`-included from there as a child module, +//! so `super::*` still resolves to the shared test helpers. +//! +//! Export resolves against `resolved_records(load_managed_agents(..))`, not +//! raw disk. These tests pin the two behaviors that fold buys export: +//! a relay-only agent resolves before its first START on this device, and a +//! follower exports the effective relay config instead of stale disk values. + +use super::*; +use crate::managed_agents::private_config_overlay::PrivateConfigOverlay; +use buzz_core_pkg::private_managed_agent::{ + Payload, PrivateConfig, PrivateIdentity, FORMAT, VERSION, +}; +use serde_json::{json, Map}; + +fn relay_payload(pubkey: &str, name: &str, prompt: &str) -> Payload { + Payload { + format: FORMAT.into(), + version: VERSION, + agent_pubkey: pubkey.into(), + owner_pubkey: "11".repeat(32), + generation: 1, + previous_event_id: None, + updated_at: "2026-08-07T00:00:00Z".into(), + identity: PrivateIdentity { + private_key_nsec: "nsec-test".into(), + auth_tag: None, + }, + config: PrivateConfig { + relay_url: "wss://relay.example".into(), + name: name.into(), + persona_id: None, + runtime: Some("goose".into()), + model: Some("relay-model".into()), + provider: None, + system_prompt: Some(prompt.into()), + parallelism: None, + respond_to: Some("allowlist".into()), + respond_to_allowlist: vec!["ab".repeat(32)], + agent_command_override: None, + agent_args: vec![], + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + env_vars: BTreeMap::new(), + backend: json!({"type":"local"}), + backend_agent_id: None, + team_id: None, + persona_name_in_team: None, + relay_mesh: None, + extra: Map::new(), + }, + extensions: BTreeMap::new(), + extra: Map::new(), + } +} + +/// Gap B: a relay-only agent (30179 head, no disk record — never started +/// on this device) must resolve for export once the disk list is folded +/// through the overlay. Raw disk alone returns "agent not found". +#[test] +fn relay_only_agent_resolves_for_export_after_overlay_fold() { + let mut overlay = PrivateConfigOverlay::default(); + overlay + .insert(relay_payload("relay-only-pk", "Relay Only", "relay prompt")) + .unwrap(); + let disk: Vec = vec![]; + + // Raw disk: not found (the pre-fix behavior). + assert!(resolve_from_lists("relay-only-pk", &disk, &[]).is_err()); + + // Folded: resolves, with the relay config as the effective record. + let folded = overlay.resolved_records(&disk); + let (record, is_def) = resolve_from_lists("relay-only-pk", &folded, &[]).unwrap(); + assert!(!is_def); + assert_eq!(record.name, "Relay Only"); + assert_eq!(record.system_prompt.as_deref(), Some("relay prompt")); +} + +/// Gap A: a follower device with a stale disk record must export the +/// effective 30179 values, not the disk snapshot — and the exported +/// manifest must advertise the enforced respond_to via the instance +/// fallback (the overlay patch only populates instance fields). +#[test] +fn follower_exports_effective_relay_values_not_stale_disk() { + use crate::managed_agents::agent_snapshot::build_snapshot; + + let mut overlay = PrivateConfigOverlay::default(); + overlay + .insert(relay_payload("follower-pk", "Fresh Name", "fresh prompt")) + .unwrap(); + let mut stale = make_instance("follower-pk", "some-persona"); + stale.name = "Stale Name".into(); + stale.system_prompt = Some("stale prompt".into()); + + let folded = overlay.resolved_records(std::slice::from_ref(&stale)); + let (record, _) = resolve_from_lists("follower-pk", &folded, &[]).unwrap(); + assert_eq!(record.name, "Fresh Name"); + assert_eq!(record.system_prompt.as_deref(), Some("fresh prompt")); + assert_eq!(record.model.as_deref(), Some("relay-model")); + + let snapshot = build_snapshot(record, MemoryLevel::None, vec![], None); + assert_eq!( + snapshot.definition.system_prompt.as_deref(), + Some("fresh prompt") + ); + assert_eq!(snapshot.definition.respond_to.as_deref(), Some("allowlist")); + assert_eq!( + snapshot.definition.respond_to_allowlist, + vec!["ab".repeat(32)] + ); +} diff --git a/desktop/src-tauri/src/commands/personas/update.rs b/desktop/src-tauri/src/commands/personas/update.rs index b3830e62b52..a4938149daf 100644 --- a/desktop/src-tauri/src/commands/personas/update.rs +++ b/desktop/src-tauri/src/commands/personas/update.rs @@ -212,7 +212,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 556127373bf..0db67b3d4f4 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 @@ -160,3 +160,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/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index 77d519b94ba..86f6b2aca96 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -211,6 +211,18 @@ pub async fn apply_workspace( assert_current_apply_generation(&state.workspace_apply_generation, apply_generation)?; // ── Apply all state changes (nothing below can fail) ────────────────── + // Serialize the scope transition with inbound private-config handling. + // Inbound holds this lock from scope resolution through overlay insert, + // so a patch decrypted for the old workspace cannot land after this clear. + let _managed_agents_store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + state + .private_managed_agent_overlay + .lock() + .map_err(|e| e.to_string())? + .clear(); { let mut override_guard = state.relay_url_override.lock().map_err(|e| e.to_string())?; *override_guard = Some(relay_url); @@ -223,6 +235,7 @@ pub async fn apply_workspace( let mut keys_guard = state.keys.lock().map_err(|e| e.to_string())?; *keys_guard = keys; } + drop(_managed_agents_store_guard); // Keep the backend-side reconcile guard aligned with the frontend // experiment before launch-time restore can spawn any agents. Missing diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index 93990f2b24e..5de8560dbd0 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -28,9 +28,48 @@ pub fn run_event_sync( 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); Ok(()) } +/// 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}"), + } +} + /// Run the scoped event reconcile to completion on the blocking pool. /// /// Callers that must not let downstream work observe a not-yet-retained disk diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index f0a4fabfed8..1b44c581fbf 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -151,8 +151,91 @@ pub fn managed_agent_content_from_event( /// event-id deletion path, leaving the parameterized-replaceable coordinate /// live. The coordinate delete removes the agent for every client and across /// reboots. +pub(crate) fn tombstone_managed_agent_pending( + app: &tauri::AppHandle, + state: &crate::app_state::AppState, + agent_pubkey: &str, +) { + use crate::managed_agents::retention::{ + delete_retained_event, open_retention_db, retain_event, tombstone_retention_d_tag, + RetainedEvent, + }; + use buzz_core_pkg::kind::{KIND_MANAGED_AGENT, KIND_PRIVATE_MANAGED_AGENT}; + use nostr::JsonUtil; + + const KIND_DELETE: u32 = 5; + let result = (|| -> Result<(), String> { + let scope = crate::managed_agents::retention::active_retention_scope(app, state)?; + let owner_pubkey = scope.owner_keys.public_key().to_hex(); + let public_delete = build_agent_delete(agent_pubkey, &owner_pubkey)? + .sign_with_keys(&scope.owner_keys) + .map_err(|e| format!("failed to sign managed-agent tombstone: {e}"))?; + let private_delete = build_private_agent_delete(agent_pubkey, &owner_pubkey)? + .sign_with_keys(&scope.owner_keys) + .map_err(|e| format!("failed to sign private managed-agent tombstone: {e}"))?; + let conn = open_retention_db(&scope.db_path)?; + let transaction = conn + .unchecked_transaction() + .map_err(|error| format!("failed to begin agent deletion transaction: {error}"))?; + delete_retained_event( + &transaction, + KIND_MANAGED_AGENT, + &owner_pubkey, + agent_pubkey, + )?; + delete_retained_event( + &transaction, + KIND_PRIVATE_MANAGED_AGENT, + &owner_pubkey, + agent_pubkey, + )?; + for (target_kind, event) in [ + (KIND_MANAGED_AGENT, public_delete), + (KIND_PRIVATE_MANAGED_AGENT, private_delete), + ] { + retain_event( + &transaction, + &RetainedEvent { + kind: KIND_DELETE, + pubkey: owner_pubkey.clone(), + d_tag: tombstone_retention_d_tag(target_kind, agent_pubkey), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: true, + }, + )?; + } + transaction + .commit() + .map_err(|error| format!("failed to commit agent deletion transaction: {error}")) + })(); + if let Err(e) = result { + eprintln!("buzz-desktop: agent-tombstone: {e}"); + } +} + pub fn build_agent_delete(d_tag: &str, owner_pubkey_hex: &str) -> Result { - let coord = format!("{KIND_MANAGED_AGENT}:{owner_pubkey_hex}:{d_tag}"); + build_agent_delete_for_kind(KIND_MANAGED_AGENT, d_tag, owner_pubkey_hex) +} + +pub fn build_private_agent_delete( + d_tag: &str, + owner_pubkey_hex: &str, +) -> Result { + build_agent_delete_for_kind( + buzz_core_pkg::kind::KIND_PRIVATE_MANAGED_AGENT, + d_tag, + owner_pubkey_hex, + ) +} + +fn build_agent_delete_for_kind( + target_kind: u32, + d_tag: &str, + owner_pubkey_hex: &str, +) -> Result { + let coord = format!("{target_kind}:{owner_pubkey_hex}:{d_tag}"); let tag = Tag::parse(["a", coord.as_str()]).map_err(|e| format!("invalid a-tag: {e}"))?; Ok(EventBuilder::new(Kind::Custom(5), "").tags(vec![tag])) } diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs index 4b734ce1591..b234d9aa27f 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs @@ -201,6 +201,23 @@ pub fn build_snapshot( // Use definition-level fields (respond_to, allowlist, parallelism) for // portability — instance-level equivalents are spawn-time snapshots and // would be stale. + // + // respond_to falls back to the instance fields when the definition-level + // mode is unset — same rationale as the parallelism fallback below. A + // record materialized from a kind:30179 relay head carries its effective + // respond_to/allowlist ONLY on the instance fields (the overlay patch + // never populates the definition ones), so without this fallback a + // relay-hosted agent would export `respond_to: None` while actually + // enforcing an allowlist. Mode and list travel together: a definition + // mode uses the definition list, the instance fallback uses the instance + // list — never a mix. + let (respond_to, respond_to_allowlist) = match record.definition_respond_to.clone() { + Some(mode) => (Some(mode), record.definition_respond_to_allowlist.clone()), + None => ( + Some(record.respond_to.as_str().to_string()), + record.respond_to_allowlist.clone(), + ), + }; let definition = AgentSnapshotDefinition { name: record .display_name @@ -212,8 +229,8 @@ pub fn build_snapshot( model: record.model.clone(), provider: record.provider.clone(), parallelism: record.definition_parallelism.or(Some(record.parallelism)), - respond_to: record.definition_respond_to.clone(), - respond_to_allowlist: record.definition_respond_to_allowlist.clone(), + respond_to, + respond_to_allowlist, name_pool: record.name_pool.clone(), idle_timeout_seconds: record.idle_timeout_seconds, max_turn_duration_seconds: record.max_turn_duration_seconds, diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs index 9f234749bc9..5f6c3caaa84 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -595,6 +595,55 @@ fn definition_fields_present_in_snapshot() { assert!(!snapshot.definition.respond_to_allowlist.is_empty()); } +/// A definition-level mode must carry the DEFINITION list, never the +/// instance list — mode and list travel together (`minimal_record` has +/// distinct values on each level, so a mix-up is detectable). +#[test] +fn definition_respond_to_keeps_definition_list_not_instance_list() { + let record = minimal_record(); + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], None); + assert_eq!(snapshot.definition.respond_to.as_deref(), Some("allowlist")); + assert_eq!(snapshot.definition.respond_to_allowlist, vec!["abc123def"]); +} + +/// A record materialized from a kind:30179 relay head carries its effective +/// respond_to/allowlist only on the INSTANCE fields. Export must fall back +/// to them (like parallelism already does) instead of advertising +/// `respond_to: None` while the agent enforces an allowlist. +#[test] +fn respond_to_falls_back_to_instance_fields_when_definition_unset() { + let mut record = minimal_record(); + record.definition_respond_to = None; + record.definition_respond_to_allowlist = vec![]; + record.respond_to = RespondTo::Allowlist; + record.respond_to_allowlist = vec!["ef".repeat(32)]; + + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], None); + assert_eq!(snapshot.definition.respond_to.as_deref(), Some("allowlist")); + assert_eq!( + snapshot.definition.respond_to_allowlist, + vec!["ef".repeat(32)] + ); +} + +/// The instance fallback exports the enforced default explicitly rather +/// than `None` — an owner-only agent advertises owner-only. +#[test] +fn respond_to_fallback_exports_explicit_owner_only_default() { + let mut record = minimal_record(); + record.definition_respond_to = None; + record.definition_respond_to_allowlist = vec![]; + record.respond_to = RespondTo::default(); + record.respond_to_allowlist = vec![]; + + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], None); + assert_eq!( + snapshot.definition.respond_to.as_deref(), + Some("owner-only") + ); + assert!(snapshot.definition.respond_to_allowlist.is_empty()); +} + #[test] fn profile_fields_present_in_snapshot() { let record = minimal_record(); diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 272c03348b9..9e820a0f092 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -24,6 +24,7 @@ pub(crate) mod parallelism; mod persona_avatars; pub(crate) mod persona_events; mod personas; +pub(crate) mod private_config_overlay; #[cfg(windows)] mod process_lifecycle; pub(crate) mod readiness; diff --git a/desktop/src-tauri/src/managed_agents/private_config_overlay.rs b/desktop/src-tauri/src/managed_agents/private_config_overlay.rs new file mode 100644 index 00000000000..0faf381ae3b --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/private_config_overlay.rs @@ -0,0 +1,694 @@ +use std::collections::{BTreeMap, HashMap}; + +use buzz_core_pkg::private_managed_agent::Payload; + +use super::{ + validate_respond_to_allowlist, validate_user_env_keys, BackendKind, ManagedAgentRecord, + RelayMeshConfig, RespondTo, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, + DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, +}; + +#[derive(Clone)] +pub(crate) struct PrivateConfigPatch { + pubkey: String, + name: String, + private_key_nsec: String, + auth_tag: Option, + relay_url: String, + persona_id: Option, + runtime: Option, + model: Option, + provider: Option, + system_prompt: Option, + parallelism: u32, + respond_to: RespondTo, + respond_to_allowlist: Vec, + agent_command_override: Option, + agent_args: Vec, + idle_timeout_seconds: Option, + max_turn_duration_seconds: Option, + env_vars: BTreeMap, + backend: BackendKind, + backend_agent_id: Option, + team_id: Option, + persona_name_in_team: Option, + relay_mesh: Option, + updated_at: String, +} + +impl PrivateConfigPatch { + /// Convert a payload that has already passed the codec's + /// `validate_and_decrypt` gate. Callers must not feed unvalidated wire data. + pub(crate) fn from_payload(payload: Payload) -> Result { + let config = payload.config; + let backend = serde_json::from_value(config.backend) + .map_err(|error| format!("invalid private managed-agent backend: {error}"))?; + let relay_mesh = config + .relay_mesh + .map(serde_json::from_value) + .transpose() + .map_err(|error| format!("invalid private managed-agent relay_mesh: {error}"))?; + let respond_to = config + .respond_to + .as_deref() + .map(RespondTo::parse_wire) + .transpose()? + .unwrap_or_default(); + let respond_to_allowlist = validate_respond_to_allowlist(&config.respond_to_allowlist)?; + if respond_to == RespondTo::Allowlist && respond_to_allowlist.is_empty() { + return Err("private managed-agent allowlist mode requires at least one pubkey".into()); + } + validate_user_env_keys(&config.env_vars)?; + let parallelism = config.parallelism.unwrap_or(DEFAULT_AGENT_PARALLELISM); + if !(1..=32).contains(¶llelism) { + return Err("private managed-agent parallelism must be between 1 and 32".into()); + } + + Ok(Self { + pubkey: payload.agent_pubkey, + name: config.name, + private_key_nsec: payload.identity.private_key_nsec, + auth_tag: payload.identity.auth_tag, + relay_url: config.relay_url, + persona_id: config.persona_id, + runtime: config.runtime, + model: config.model, + provider: config.provider, + system_prompt: config.system_prompt, + parallelism, + respond_to, + respond_to_allowlist, + agent_command_override: config.agent_command_override, + agent_args: config.agent_args, + idle_timeout_seconds: config.idle_timeout_seconds, + max_turn_duration_seconds: config.max_turn_duration_seconds, + env_vars: config.env_vars, + backend, + backend_agent_id: config.backend_agent_id, + team_id: config.team_id, + persona_name_in_team: config.persona_name_in_team, + relay_mesh, + updated_at: payload.updated_at, + }) + } + + fn apply(&self, record: &mut ManagedAgentRecord) { + record.pubkey.clone_from(&self.pubkey); + record.name.clone_from(&self.name); + record.private_key_nsec.clone_from(&self.private_key_nsec); + record.auth_tag.clone_from(&self.auth_tag); + record.relay_url.clone_from(&self.relay_url); + record.persona_id.clone_from(&self.persona_id); + record.runtime.clone_from(&self.runtime); + record.model.clone_from(&self.model); + record.provider.clone_from(&self.provider); + record.system_prompt.clone_from(&self.system_prompt); + record.parallelism = self.parallelism; + record.respond_to = self.respond_to; + record + .respond_to_allowlist + .clone_from(&self.respond_to_allowlist); + record + .agent_command_override + .clone_from(&self.agent_command_override); + record.agent_args.clone_from(&self.agent_args); + record.idle_timeout_seconds = self.idle_timeout_seconds; + record.max_turn_duration_seconds = self.max_turn_duration_seconds; + record.env_vars.clone_from(&self.env_vars); + record.backend.clone_from(&self.backend); + record.backend_agent_id.clone_from(&self.backend_agent_id); + record.team_id.clone_from(&self.team_id); + record + .persona_name_in_team + .clone_from(&self.persona_name_in_team); + record.relay_mesh.clone_from(&self.relay_mesh); + record.updated_at.clone_from(&self.updated_at); + } + + fn fresh_record(&self) -> ManagedAgentRecord { + let mut record = ManagedAgentRecord { + pubkey: String::new(), + name: String::new(), + persona_id: None, + team_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: DEFAULT_ACP_COMMAND.into(), + agent_command: String::new(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: DEFAULT_AGENT_PARALLELISM, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars: BTreeMap::new(), + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: BackendKind::Local, + backend_agent_id: None, + provider_policy_pending: false, + provider_binary_path: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: self.updated_at.clone(), + updated_at: self.updated_at.clone(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: RespondTo::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: None, + name_pool: vec![], + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: vec![], + definition_parallelism: None, + relay_mesh: None, + effort_level: None, + }; + self.apply(&mut record); + record + } +} + +#[derive(Default)] +pub(crate) struct PrivateConfigOverlay(HashMap); + +impl PrivateConfigOverlay { + #[cfg(test)] + pub(crate) fn insert(&mut self, payload: Payload) -> Result<(), String> { + let patch = PrivateConfigPatch::from_payload(payload)?; + self.0.insert(patch.pubkey.clone(), patch); + Ok(()) + } + + pub(crate) fn insert_patch(&mut self, patch: PrivateConfigPatch) { + 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(); + } + + pub(crate) fn remove(&mut self, pubkey: &str) { + self.0.remove(pubkey); + } + + /// Write-through for a SELF-AUTHORED retain: adopt the kind:30179 head this + /// device just wrote as the config it is following. + /// + /// Both existing fill paths are inbound-only — `insert_patch` on an + /// `Applied` inbound event (`personas/inbound.rs`) and boot hydration + /// below — and neither fires for an event this device authored: the + /// relay's echo of our own event dedupes to `Skipped` against the row we + /// already retained. So without this the overlay stays pinned at the last + /// *received* generation, and the NEXT edit resolves that stale patch on + /// top of the fresher disk record, silently reverting the previous edit + /// and publishing the reversion as a valid successor. + /// + /// Absorbing unconditionally (not only when the retain reported a change) + /// is safe and strictly convergent: the overlay is never ahead of + /// retention — every insert either comes from a row written in the same + /// step or is read back out of retention. A missing/undecodable head + /// leaves the current entry alone rather than clearing it. + pub(crate) fn absorb_retained_head( + &mut self, + conn: &rusqlite::Connection, + owner_keys: &nostr::Keys, + agent_pubkey: &str, + ) -> Result<(), String> { + let row = crate::managed_agents::retention::get_retained_event( + conn, + buzz_core_pkg::kind::KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + agent_pubkey, + )?; + if let Some(patch) = row + .as_ref() + .and_then(|row| patch_from_retained_row(&row.raw_event, owner_keys)) + { + self.insert_patch(patch); + } + Ok(()) + } + + pub(crate) fn resolve_local_record(&self, record: &ManagedAgentRecord) -> ManagedAgentRecord { + let mut resolved = record.clone(); + if let Some(patch) = self.0.get(&record.pubkey) { + patch.apply(&mut resolved); + } + resolved + } + + pub(crate) fn materialize_relay_only_record( + &self, + pubkey: &str, + local: &[ManagedAgentRecord], + ) -> Option { + if local.iter().any(|record| record.pubkey == pubkey) { + return None; + } + let mut record = self.0.get(pubkey)?.fresh_record(); + // Persona definitions are device-local. A fresh device can still run the + // complete relay snapshot, but must not bind it to an absent local persona. + record.persona_id = None; + Some(record) + } + + pub(crate) fn resolved_records(&self, local: &[ManagedAgentRecord]) -> Vec { + let mut resolved = local.to_vec(); + for record in &mut resolved { + if let Some(patch) = self.0.get(&record.pubkey) { + patch.apply(record); + } + } + let mut relay_only: Vec<_> = self + .0 + .values() + .filter(|patch| !local.iter().any(|record| record.pubkey == patch.pubkey)) + .map(PrivateConfigPatch::fresh_record) + .collect(); + relay_only.sort_by(|left, right| left.pubkey.cmp(&right.pubkey)); + resolved.extend(relay_only); + resolved + } +} + +pub(crate) fn resolved_local_record( + state: &crate::app_state::AppState, + record: &ManagedAgentRecord, +) -> Result { + state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string()) + .map(|overlay| overlay.resolve_local_record(record)) +} + +pub(crate) fn copy_lifecycle_state( + destination: &mut ManagedAgentRecord, + source: &ManagedAgentRecord, +) { + destination.runtime_pid = source.runtime_pid; + destination + .last_started_at + .clone_from(&source.last_started_at); + destination + .last_stopped_at + .clone_from(&source.last_stopped_at); + destination.last_exit_code = source.last_exit_code; + destination.last_error.clone_from(&source.last_error); + destination.last_error_code = source.last_error_code; +} + +pub(crate) fn materialize_relay_only_agent( + app: &tauri::AppHandle, + state: &crate::app_state::AppState, + pubkey: &str, +) -> Result<(), String> { + let _transition = state + .managed_agent_runtime_transition + .lock() + .map_err(|error| error.to_string())?; + if state + .shutdown_started + .load(std::sync::atomic::Ordering::Acquire) + { + return Err("desktop shutdown has started".into()); + } + let _store = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + let mut records = super::load_managed_agents(app)?; + let relay_only = state + .private_managed_agent_overlay + .lock() + .map_err(|error| error.to_string())? + .materialize_relay_only_record(pubkey, &records); + if let Some(record) = relay_only { + if record.backend != BackendKind::Local { + return Err("relay-only provider agents cannot be started on this device".into()); + } + records.push(record); + super::save_managed_agents(app, &records)?; + } + Ok(()) +} + +/// Minimal valid decrypted kind:30179 payload for cross-module overlay tests +/// (the pair-spawn, provider-deploy, and provider-access final-use-boundary +/// regressions). Callers mutate `config` fields for scenario-specific shapes. +#[cfg(test)] +pub(crate) fn test_relay_payload(pubkey: &str) -> Payload { + use buzz_core_pkg::private_managed_agent::{PrivateConfig, PrivateIdentity, FORMAT, VERSION}; + Payload { + format: FORMAT.into(), + version: VERSION, + agent_pubkey: pubkey.into(), + owner_pubkey: "11".repeat(32), + generation: 2, + previous_event_id: None, + updated_at: "2026-08-20T00:00:00Z".into(), + identity: PrivateIdentity { + private_key_nsec: "nsec-relay".into(), + auth_tag: None, + }, + config: PrivateConfig { + relay_url: "wss://relay.example".into(), + name: "relay name".into(), + persona_id: None, + runtime: Some("goose".into()), + model: Some("relay-model".into()), + provider: None, + system_prompt: Some("relay prompt".into()), + parallelism: Some(4), + respond_to: None, + respond_to_allowlist: vec![], + agent_command_override: None, + agent_args: vec![], + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + env_vars: BTreeMap::new(), + backend: serde_json::json!({"type":"local"}), + backend_agent_id: None, + team_id: None, + persona_name_in_team: None, + relay_mesh: None, + extra: serde_json::Map::new(), + }, + extensions: BTreeMap::new(), + extra: serde_json::Map::new(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use buzz_core_pkg::private_managed_agent::{ + Payload, PrivateConfig, PrivateIdentity, FORMAT, VERSION, + }; + use serde_json::{json, Map}; + + fn payload(pubkey: &str, name: &str) -> Payload { + Payload { + format: FORMAT.into(), + version: VERSION, + agent_pubkey: pubkey.into(), + owner_pubkey: "11".repeat(32), + generation: 1, + previous_event_id: None, + updated_at: "2026-08-06T00:00:00Z".into(), + identity: PrivateIdentity { + private_key_nsec: "nsec-test".into(), + auth_tag: None, + }, + config: PrivateConfig { + relay_url: "wss://relay.example".into(), + name: name.into(), + persona_id: None, + runtime: Some("goose".into()), + model: Some("m".into()), + provider: None, + system_prompt: Some("relay prompt".into()), + parallelism: None, + respond_to: None, + respond_to_allowlist: vec![], + agent_command_override: None, + agent_args: vec![], + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + env_vars: BTreeMap::new(), + backend: json!({"type":"local"}), + backend_agent_id: None, + team_id: None, + persona_name_in_team: None, + relay_mesh: None, + extra: Map::new(), + }, + extensions: BTreeMap::new(), + extra: Map::new(), + } + } + + #[test] + fn resolves_overlay_and_relay_only_without_mutating_local() { + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(payload("aa", "relay local")).unwrap(); + overlay.insert(payload("bb", "relay only")).unwrap(); + let mut local = overlay.0["aa"].fresh_record(); + local.name = "disk".into(); + local.system_prompt = Some("disk prompt".into()); + let original = local.clone(); + + let resolved = overlay.resolved_records(std::slice::from_ref(&local)); + assert_eq!(resolved.len(), 2); + assert_eq!(resolved[0].name, "relay local"); + assert_eq!(resolved[0].system_prompt.as_deref(), Some("relay prompt")); + assert_eq!(resolved[1].pubkey, "bb"); + assert!(!resolved[1].start_on_app_launch); + assert_eq!(local, original); + } + + #[test] + fn materializes_only_relay_only_record_and_preserves_disk_overlay() { + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(payload("aa", "relay local")).unwrap(); + overlay.insert(payload("bb", "relay only")).unwrap(); + let mut local = overlay.0["aa"].fresh_record(); + local.name = "disk".into(); + local.private_key_nsec = "device-local-key".into(); + + let resolved = overlay.resolve_local_record(&local); + assert_eq!(resolved.name, "relay local"); + assert_eq!(resolved.private_key_nsec, "nsec-test"); + assert_eq!(local.name, "disk"); + assert_eq!(local.private_key_nsec, "device-local-key"); + assert!(overlay + .materialize_relay_only_record("aa", std::slice::from_ref(&local)) + .is_none()); + + let relay_only = overlay + .materialize_relay_only_record("bb", &[local]) + .unwrap(); + assert_eq!(relay_only.name, "relay only"); + assert_eq!(relay_only.private_key_nsec, "nsec-test"); + assert_eq!(relay_only.backend, BackendKind::Local); + assert!(relay_only.persona_id.is_none()); + } + + #[test] + fn rejected_patch_preserves_cached_value_and_clear_drops_scope() { + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(payload("aa", "valid")).unwrap(); + let mut invalid = payload("aa", "invalid"); + invalid.config.backend = json!({"type":"provider"}); + assert!(overlay.insert(invalid).is_err()); + assert_eq!(overlay.resolved_records(&[])[0].name, "valid"); + overlay.clear(); + assert!(overlay.resolved_records(&[]).is_empty()); + } + + #[test] + fn unresolved_harness_has_named_refusal() { + let mut patch = PrivateConfigPatch::from_payload(payload("aa", "agent")).unwrap(); + patch.runtime = Some("missing-custom-harness".into()); + let record = patch.fresh_record(); + let error = crate::managed_agents::try_record_agent_command(&record, &[]).unwrap_err(); + assert_eq!( + crate::managed_agents::dangling_harness_id(&error), + Some("missing-custom-harness") + ); + } +} + +/// Decode one retained kind:30179 row into a patch. Shared by boot hydration +/// and the self-authored write-through so both learn config through exactly +/// one decode path. Best-effort: a row that fails to parse, decrypt, or +/// validate yields `None` rather than an error, matching the inbound path's +/// per-record reject. +fn patch_from_retained_row( + raw_event: &str, + owner_keys: &nostr::Keys, +) -> Option { + use buzz_core_pkg::private_managed_agent; + use nostr::JsonUtil; + + let event = nostr::Event::from_json(raw_event).ok()?; + let (_, payload) = private_managed_agent::validate_and_decrypt(&event, owner_keys).ok()?; + PrivateConfigPatch::from_payload(payload).ok() +} + +/// 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; + + 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 { + if let Some(patch) = patch_from_retained_row(&row.raw_event, owner_keys) { + 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_update.rs", + include_str!("../commands/agent_models_update.rs"), + 1, + ), + // 4 = the 3 sites Carl already resolved correctly (start/stop/ + // delete) plus the pair-start snapshot re-apply. 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, + ), + // 2 = the preflight snapshot resolve plus the locked spawn-record + // resolve in `start_local_agent_with_preflight` (extracted from + // agents.rs by the upstream file-size split). + ( + "commands/agents_lifecycle.rs", + include_str!("../commands/agents_lifecycle.rs"), + 2, + ), + ( + "commands/personas/update.rs", + include_str!("../commands/personas/update.rs"), + 1, + ), + // The launch-restore fold: every Phase-A spawn candidate is + // resolved through the overlay before Phase B spawns it. Without + // this, a follower device hydrates relay config B and then + // auto-starts stale disk config A (obsolete prompt/model/ACL/ + // identity) — the boot-time variant of the stale-republish bug. + ("managed_agents/restore.rs", include_str!("restore.rs"), 1), + // 2 = the pair-start spawn record (`start_pair`) plus the + // multi-community reconcile fan-out candidates — the final-use + // boundaries Pair Start/Restart and boot reconcile execute. + // Without these, a follower device showing relay config B + // pair-starts stale disk config A. + ( + "managed_agents/runtime_commands.rs", + include_str!("runtime_commands.rs"), + 2, + ), + // The post-deploy-lock payload rebuild: the exact bytes the + // provider invocation executes after waiting behind another + // deployment. + ( + "commands/agents/provider_deploy.rs", + include_str!("../commands/agents/provider_deploy.rs"), + 1, + ), + // Workspace provider-access reconciliation: both the target + // selection predicate and the redeploy payload read resolved + // records, not raw disk rows. + ( + "commands/agents/provider_access.rs", + include_str!("../commands/agents/provider_access.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 90f05c5750d..3f1fb11f611 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile.rs @@ -26,8 +26,12 @@ use super::{ retention::{get_retained_event, open_retention_db, retain_event, RetainedEvent}, ManagedAgentRecord, }; -use buzz_core_pkg::kind::KIND_MANAGED_AGENT; +use buzz_core_pkg::{ + kind::{KIND_MANAGED_AGENT, KIND_PRIVATE_MANAGED_AGENT}, + private_managed_agent::{self, Payload, PrivateConfig, PrivateIdentity}, +}; use nostr::JsonUtil; +use std::collections::BTreeMap; /// Reconcile `managed-agents.json` into kind:30177 events in the retention /// store. Boot-time entry point, called from `event_sync::run_event_sync` @@ -56,24 +60,52 @@ 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)] pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Result { - reconcile_agents_in_dir_at(base_dir, keys, &base_dir.join("retention.db")) + // No key store: unit tests must never touch the live OS keyring (a macOS + // Keychain ACL prompt blocks a headless test binary forever). Tests that + // exercise key hydration inject a fake via + // [`reconcile_agents_in_dir_with`]. + reconcile_agents_in_dir_with( + base_dir, + keys, + &base_dir.join("retention.db"), + None::<&crate::secret_store::SecretStore>, + ) } +/// Production entry: hydrates keys from the real agent secret store (`None` +/// on builds without a keyring backend, where keys stay inline in the JSON). fn reconcile_agents_in_dir_at( base_dir: &Path, keys: &nostr::Keys, db_path: &Path, +) -> Result { + reconcile_agents_in_dir_with( + base_dir, + keys, + db_path, + super::storage::agent_secret_store(), + ) +} + +/// Testable core, generic over the [`super::storage::KeyStore`] seam so unit +/// tests never reach the live OS keyring. +fn reconcile_agents_in_dir_with( + base_dir: &Path, + keys: &nostr::Keys, + db_path: &Path, + store: Option<&impl super::storage::KeyStore>, ) -> Result { let store_path = base_dir.join("managed-agents.json"); if !store_path.exists() { @@ -83,7 +115,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}") })?; @@ -92,6 +124,14 @@ 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. + if let Some(store) = store { + super::storage::hydrate_keys_with(store, &mut records); + } + let conn = open_retention_db(db_path).map_err(|e| format!("failed to open retention db: {e}"))?; @@ -104,7 +144,7 @@ fn reconcile_agents_in_dir_at( continue; } - if retain_agent_record(&conn, keys, record)? { + if retain_agent_record_at_boot(&conn, keys, record)? { reconciled += 1; } } @@ -112,6 +152,51 @@ fn reconcile_agents_in_dir_at( Ok(reconciled) } +/// Boot-only variant of [`retain_agent_record`]: reconciles the kind:30177 +/// identity record exactly as the interactive paths do, but publishes the +/// kind:30179 private config **only when no retained head exists**. +/// +/// Boot reads `managed-agents.json` raw — there is no overlay to resolve +/// against, because `hydrate_private_config_overlay` runs after this leg and +/// depends on the very rows written here. On a device that FOLLOWS another +/// device's config, disk is stale by construction (inbound 30179 updates the +/// overlay and retention, never the JSON), so rebuilding the 30179 projection +/// from disk republishes every stale field over a newer head as an audit-clean +/// gen+1 successor, and `monotonic_created_at` makes it win LWW. That fires at +/// launch, unprompted, and re-arms on every new head the follower receives. +/// +/// Restricting boot to the head-absent case keeps the requirement boot exists +/// to serve — an agent whose nsec lives in the keyring must get its FIRST +/// 30179 published — while leaving an existing head to the interactive edit +/// paths, which resolve the overlay before retaining and so author from +/// relay-fresh state. Every 30177 (no-secrets projection) behaves exactly as +/// before: the upgrade republish waves run on that kind, not this one. +fn retain_agent_record_at_boot( + conn: &rusqlite::Connection, + keys: &nostr::Keys, + record: &ManagedAgentRecord, +) -> Result { + let transaction = conn + .unchecked_transaction() + .map_err(|error| format!("failed to begin agent retention transaction: {error}"))?; + let public_changed = retain_public_agent_record(&transaction, keys, record)?; + let private_head = get_retained_event( + &transaction, + KIND_PRIVATE_MANAGED_AGENT, + &keys.public_key().to_hex(), + &record.pubkey, + )?; + let private_changed = if private_head.is_some() { + false + } else { + retain_private_agent_record(&transaction, keys, record)? + }; + transaction + .commit() + .map_err(|error| format!("failed to commit agent retention transaction: {error}"))?; + Ok(public_changed || private_changed) +} + /// Retain `record`'s kind:30177 identity record, marking it `pending_sync` /// for the flush loop, when its projection differs from the retained head. /// Returns `Ok(true)` when a row was (re)written and `Ok(false)` when the @@ -126,6 +211,22 @@ pub(crate) fn retain_agent_record( conn: &rusqlite::Connection, keys: &nostr::Keys, record: &ManagedAgentRecord, +) -> Result { + let transaction = conn + .unchecked_transaction() + .map_err(|error| format!("failed to begin agent retention transaction: {error}"))?; + let public_changed = retain_public_agent_record(&transaction, keys, record)?; + let private_changed = retain_private_agent_record(&transaction, keys, record)?; + transaction + .commit() + .map_err(|error| format!("failed to commit agent retention transaction: {error}"))?; + Ok(public_changed || private_changed) +} + +fn retain_public_agent_record( + conn: &rusqlite::Connection, + keys: &nostr::Keys, + record: &ManagedAgentRecord, ) -> Result { let owner_pubkey = keys.public_key().to_hex(); let existing = get_retained_event(conn, KIND_MANAGED_AGENT, &owner_pubkey, &record.pubkey)?; @@ -165,5 +266,154 @@ pub(crate) fn retain_agent_record( Ok(true) } +fn retain_private_agent_record( + conn: &rusqlite::Connection, + keys: &nostr::Keys, + record: &ManagedAgentRecord, +) -> Result { + if record.private_key_nsec.is_empty() { + return Ok(false); + } + + let owner_pubkey = keys.public_key().to_hex(); + let existing = get_retained_event( + conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_pubkey, + &record.pubkey, + )?; + let previous_event = existing + .as_ref() + .and_then(|row| nostr::Event::from_json(&row.raw_event).ok()); + let generation = previous_event + .as_ref() + .and_then(event_generation) + .unwrap_or(0) + .checked_add(1) + .ok_or_else(|| format!("private config generation overflow for '{}'", record.name))?; + let previous_event_id = previous_event.as_ref().map(|event| event.id.to_hex()); + let created_at = monotonic_created_at(existing.as_ref().map(|row| row.created_at)); + let mut payload = + private_payload_from_record(record, &owner_pubkey, generation, previous_event_id)?; + + // Preserve fields authored by a newer client. This writer owns the known + // typed fields only; flatten/extension data must survive an older Desktop + // editing one known value. + let existing_payload = previous_event.as_ref().and_then(|existing_event| { + private_managed_agent::validate_and_decrypt(existing_event, keys) + .ok() + .map(|(_, payload)| payload) + }); + if let Some(existing_payload) = &existing_payload { + payload.extensions.clone_from(&existing_payload.extensions); + payload.extra.clone_from(&existing_payload.extra); + payload + .config + .extra + .clone_from(&existing_payload.config.extra); + } + + // NIP-44 encryption is randomized, so compare the validated plaintext + // payload rather than ciphertext. Metadata derived from the retained head + // changes only after a meaningful config mutation. + if existing_payload + .as_ref() + .is_some_and(|existing| private_payload_body_eq(existing, &payload)) + { + return Ok(false); + } + + let event = private_managed_agent::build_event(keys, &payload, created_at.as_secs()) + .map_err(|e| format!("failed to build private config for '{}': {e}", record.name))?; + + retain_event( + conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_pubkey, + d_tag: record.pubkey.clone(), + content: event.content.clone(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: true, + }, + ) + .map_err(|e| format!("failed to retain private config for '{}': {e}", record.name))?; + Ok(true) +} + +fn event_generation(event: &nostr::Event) -> Option { + event.tags.iter().find_map(|tag| { + let values = tag.as_slice(); + (values.first().map(String::as_str) == Some("g")) + .then(|| values.get(1)?.parse().ok()) + .flatten() + }) +} + +fn private_payload_from_record( + record: &ManagedAgentRecord, + owner_pubkey: &str, + generation: u64, + previous_event_id: Option, +) -> Result { + let backend = serde_json::to_value(&record.backend) + .map_err(|e| format!("failed to serialize backend for '{}': {e}", record.name))?; + let relay_mesh = record + .relay_mesh + .as_ref() + .map(serde_json::to_value) + .transpose() + .map_err(|e| format!("failed to serialize relay mesh for '{}': {e}", record.name))?; + + Ok(Payload { + format: private_managed_agent::FORMAT.to_string(), + version: private_managed_agent::VERSION, + agent_pubkey: record.pubkey.clone(), + owner_pubkey: owner_pubkey.to_string(), + generation, + previous_event_id, + updated_at: record.updated_at.clone(), + identity: PrivateIdentity { + private_key_nsec: record.private_key_nsec.clone(), + auth_tag: record.auth_tag.clone(), + }, + config: PrivateConfig { + relay_url: record.relay_url.clone(), + name: record.name.clone(), + persona_id: record.persona_id.clone(), + runtime: record.runtime.clone(), + model: record.model.clone(), + provider: record.provider.clone(), + system_prompt: record.system_prompt.clone(), + parallelism: Some(record.parallelism), + respond_to: Some(record.respond_to.as_str().to_string()), + respond_to_allowlist: record.respond_to_allowlist.clone(), + agent_command_override: record.agent_command_override.clone(), + agent_args: record.agent_args.clone(), + idle_timeout_seconds: record.idle_timeout_seconds, + max_turn_duration_seconds: record.max_turn_duration_seconds, + env_vars: record.env_vars.clone(), + backend, + backend_agent_id: record.backend_agent_id.clone(), + team_id: record.team_id.clone(), + persona_name_in_team: record.persona_name_in_team.clone(), + relay_mesh, + extra: serde_json::Map::new(), + }, + extensions: BTreeMap::new(), + extra: serde_json::Map::new(), + }) +} + +fn private_payload_body_eq(left: &Payload, right: &Payload) -> bool { + left.agent_pubkey == right.agent_pubkey + && left.owner_pubkey == right.owner_pubkey + && left.identity == right.identity + && left.config == right.config + && left.extensions == right.extensions + && left.extra == right.extra +} + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/reconcile/tests.rs b/desktop/src-tauri/src/managed_agents/reconcile/tests.rs index c9269dbf002..43d0845b2fc 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile/tests.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::managed_agents::retention::{get_pending_sync, get_retained_event, mark_synced}; +use nostr::ToBech32; use std::collections::BTreeMap; use tempfile::TempDir; @@ -34,6 +35,159 @@ fn write_store(dir: &TempDir, records: &[ManagedAgentRecord]) { .unwrap(); } +#[test] +fn private_config_conversion_encrypts_secrets_and_is_idempotent() { + 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 mut record = sample_record(&pubkey, "private-agent"); + record.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + record.env_vars = BTreeMap::from([("API_TOKEN".to_string(), "very-secret".to_string())]); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + assert!(retain_agent_record(&conn, &owner_keys, &record).unwrap()); + let row = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + assert!(!row.raw_event.contains("very-secret")); + assert!(!row.raw_event.contains("nsec1")); + + let event = nostr::Event::from_json(&row.raw_event).unwrap(); + let (_, payload) = private_managed_agent::validate_and_decrypt(&event, &owner_keys).unwrap(); + assert_eq!(payload.config.name, "private-agent"); + assert_eq!(payload.config.env_vars["API_TOKEN"], "very-secret"); + assert_eq!(payload.generation, 1); + assert_eq!(payload.previous_event_id, None); + + mark_synced( + &conn, + row.kind, + &row.pubkey, + &row.d_tag, + row.created_at, + &row.content, + ) + .unwrap(); + assert!(!retain_agent_record(&conn, &owner_keys, &record).unwrap()); + assert!(get_pending_sync(&conn) + .unwrap() + .iter() + .all(|pending| pending.kind != KIND_PRIVATE_MANAGED_AGENT)); +} + +#[test] +fn private_config_preserves_unknown_fields_without_generation_churn() { + 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 mut record = sample_record(&pubkey, "private-agent"); + record.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + let mut newer_payload = + private_payload_from_record(&record, &owner_keys.public_key().to_hex(), 1, None).unwrap(); + newer_payload.extensions.insert( + "future.example:feature".into(), + serde_json::json!({"enabled": true}), + ); + newer_payload + .extra + .insert("future_top_level".into(), serde_json::json!([1, 2, 3])); + newer_payload + .config + .extra + .insert("future_config".into(), serde_json::json!({"mode": "new"})); + let first_event = private_managed_agent::build_event(&owner_keys, &newer_payload, 1).unwrap(); + retain_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_keys.public_key().to_hex(), + d_tag: pubkey.clone(), + content: first_event.content.clone(), + created_at: first_event.created_at.as_secs() as i64, + raw_event: first_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + + record.system_prompt = Some("edited by an older client".into()); + assert!(retain_agent_record(&conn, &owner_keys, &record).unwrap()); + let row = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + let rebuilt_event = nostr::Event::from_json(&row.raw_event).unwrap(); + let (_, rebuilt) = + private_managed_agent::validate_and_decrypt(&rebuilt_event, &owner_keys).unwrap(); + assert_eq!(rebuilt.generation, 2); + assert_eq!(rebuilt.previous_event_id, Some(first_event.id.to_hex())); + assert_eq!(rebuilt.extensions, newer_payload.extensions); + assert_eq!(rebuilt.extra, newer_payload.extra); + assert_eq!(rebuilt.config.extra, newer_payload.config.extra); + + assert!(!retain_agent_record(&conn, &owner_keys, &record).unwrap()); + let unchanged = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + assert_eq!(unchanged.raw_event, row.raw_event); +} + +#[test] +fn private_config_change_advances_generation_and_links_previous_event() { + 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 mut record = sample_record(&pubkey, "private-agent"); + record.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + + retain_agent_record(&conn, &owner_keys, &record).unwrap(); + let first = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + let first_event = nostr::Event::from_json(&first.raw_event).unwrap(); + + record.env_vars.insert("TOKEN".into(), "rotated".into()); + retain_agent_record(&conn, &owner_keys, &record).unwrap(); + let second = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + let second_event = nostr::Event::from_json(&second.raw_event).unwrap(); + let (_, payload) = + private_managed_agent::validate_and_decrypt(&second_event, &owner_keys).unwrap(); + assert_eq!(payload.generation, 2); + assert_eq!(payload.previous_event_id, Some(first_event.id.to_hex())); +} + #[test] fn missing_store_is_noop() { let dir = TempDir::new().unwrap(); @@ -41,6 +195,69 @@ fn missing_store_is_noop() { assert_eq!(reconcile_agents_in_dir(dir.path(), &keys).unwrap(), 0); } +/// Boot reconcile on a default `system-keyring` build reads the agent nsec +/// from the keyring, not the JSON. Hydration through the [`KeyStore`] seam +/// must fill it in so the first 30179 gets published; without it, the +/// empty-nsec skip fires and only the 30177 lands. The `None`-store control +/// pins the other side: no store, no 30179 — which is also why the plain +/// [`reconcile_agents_in_dir`] test helper can never hit the live OS keyring +/// (a macOS Keychain ACL prompt blocks a headless test binary forever). +#[test] +fn keyring_resident_nsec_is_hydrated_for_private_config() { + use crate::managed_agents::storage::{agent_keyring_name, tests::FakeKeyStore}; + use nostr::ToBech32; + + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let nsec = agent_keys.secret_key().to_bech32().unwrap(); + // JSON carries an empty nsec — keyring-resident, as on a default build. + let record = sample_record(&pubkey, "keyring-agent"); + assert!(record.private_key_nsec.is_empty()); + + // Control: no key store → empty-nsec skip → no 30179 head. + let dir = TempDir::new().unwrap(); + write_store(&dir, std::slice::from_ref(&record)); + let db_path = dir.path().join("retention.db"); + reconcile_agents_in_dir_with( + dir.path(), + &owner_keys, + &db_path, + None::<&crate::secret_store::SecretStore>, + ) + .unwrap(); + let conn = open_retention_db(&db_path).unwrap(); + assert!(get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .is_none()); + drop(conn); + + // With the key in the (fake) keyring, hydration fills the nsec and the + // first 30179 is retained — and decrypts back to that exact key. + let dir = TempDir::new().unwrap(); + write_store(&dir, &[record]); + let db_path = dir.path().join("retention.db"); + let store = FakeKeyStore::reachable().with_key(&agent_keyring_name(&pubkey), &nsec); + reconcile_agents_in_dir_with(dir.path(), &owner_keys, &db_path, Some(&store)).unwrap(); + let conn = open_retention_db(&db_path).unwrap(); + let row = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .expect("hydrated nsec must publish the first 30179"); + let event = nostr::Event::from_json(&row.raw_event).unwrap(); + let (_, payload) = private_managed_agent::validate_and_decrypt(&event, &owner_keys).unwrap(); + assert_eq!(payload.identity.private_key_nsec, nsec); +} + #[test] fn fresh_record_is_retained_pending() { let dir = TempDir::new().unwrap(); @@ -400,3 +617,6 @@ fn retain_agent_record_is_noop_when_unchanged() { "no pending_sync churn for an unchanged record" ); } + +mod self_authored_overlay_tests; +mod stale_republish_tests; diff --git a/desktop/src-tauri/src/managed_agents/reconcile/tests/self_authored_overlay_tests.rs b/desktop/src-tauri/src/managed_agents/reconcile/tests/self_authored_overlay_tests.rs new file mode 100644 index 00000000000..a139e98e9d2 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests/self_authored_overlay_tests.rs @@ -0,0 +1,302 @@ +//! Regression coverage for the SELF-AUTHORED overlay write-through (defect 5): +//! the overlay only ever learned config from events this device *received*, so +//! a second edit in the same session resolved a stale patch onto the fresher +//! disk record and published a silent revert of the first edit. +//! +//! Split out of `stale_republish_tests.rs` to stay inside the desktop +//! file-size ratchet. Shares the parent module's fixtures (`sample_record`, +//! `private_payload_from_record`, `retain_agent_record`) via `use super::*`. + +use super::*; +use crate::managed_agents::private_config_overlay::{hydrate_from_retention, PrivateConfigOverlay}; + +/// The production body of `retain_managed_agent_pending` +/// (`commands/agents.rs:42`) minus its `AppHandle`/`AppState` plumbing: retain, +/// then write the just-retained head through to the overlay. The command is a +/// `#[tauri::command]` descendant needing a live `AppHandle`, so this models +/// the ordering; `retain_managed_agent_pending_writes_through_to_the_overlay` +/// below pins that production actually calls it. +fn retain_and_absorb( + conn: &rusqlite::Connection, + overlay: &mut PrivateConfigOverlay, + owner_keys: &nostr::Keys, + record: &ManagedAgentRecord, +) { + retain_agent_record(conn, owner_keys, record).unwrap(); + overlay + .absorb_retained_head(conn, owner_keys, &record.pubkey) + .unwrap(); +} + +fn published_head( + conn: &rusqlite::Connection, + owner_keys: &nostr::Keys, + pubkey: &str, +) -> buzz_core_pkg::private_managed_agent::Payload { + let row = get_retained_event( + conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + pubkey, + ) + .unwrap() + .unwrap(); + let (_, payload) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + owner_keys, + ) + .unwrap(); + payload +} + +/// SAMI RED-FIRST (defect 5, the live gate red Max captured): two edits in ONE +/// session on ONE device. The first edit publishes parallelism 19; the second +/// edit — a rename, touching no other field — must not take parallelism back +/// to the received head's 17. +/// +/// Discriminator is the PARALLELISM, not the name: Max retracted the Device-B +/// attribution (his broker had a single global native connection and no device +/// routing), so the name-reversion leg is unattributed. The +/// gen3-returns-19 → gen4-publishes-17 sequence is receipt-backed on a single +/// backend regardless of which app served it. +#[test] +fn sami_second_edit_in_one_session_preserves_the_first_edit() { + 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(); + + // Gen 2: the head this device RECEIVED, parallelism 17. Seeded through the + // real inbound path so the overlay is hydrated exactly as boot hydrates it. + let mut received = sample_record(&pubkey, "Fizz Relay"); + received.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + received.parallelism = 17; + let received_created_at = nostr::Timestamp::now().as_secs() as i64; + let received_payload = + private_payload_from_record(&received, &owner_hex, 2, Some("aa".repeat(32))).unwrap(); + let received_event = private_managed_agent::build_event( + &owner_keys, + &received_payload, + received_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: received_event.content.clone(), + created_at: received_created_at, + raw_event: received_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + let mut overlay = hydrate_from_retention(&conn, &owner_keys).unwrap(); + assert_eq!(overlay.len(), 1, "fixture: the overlay follows gen 2"); + + // Disk agrees with the head at the start of the session. + let mut disk = received.clone(); + + // EDIT 1 — parallelism 17 -> 19. `update_managed_agent`'s shape: resolve + // the overlay onto the disk record, apply the user's patch, SAVE to disk, + // then retain. + let mut edited = overlay.resolve_local_record(&disk); + edited.parallelism = 19; + disk = edited.clone(); + retain_and_absorb(&conn, &mut overlay, &owner_keys, &disk); + + let after_edit = published_head(&conn, &owner_keys, &pubkey); + assert_eq!(after_edit.generation, 3); + assert_eq!( + after_edit.config.parallelism, + Some(19), + "edit 1 published the user's value" + ); + + // EDIT 2 — a rename in the SAME session, touching only `name`. Same shape. + let mut renamed = overlay.resolve_local_record(&disk); + renamed.name = "Fizz Relay Rename".into(); + disk = renamed.clone(); + retain_and_absorb(&conn, &mut overlay, &owner_keys, &disk); + + let after_rename = published_head(&conn, &owner_keys, &pubkey); + assert_eq!(after_rename.generation, 4); + // The intended write lands... + assert_eq!(after_rename.config.name, "Fizz Relay Rename"); + // ...and the FIRST edit survives it. Without the write-through this is + // Some(17): the overlay is still pinned at the received gen-2 patch, so + // resolving it onto disk reverts parallelism, and the revert publishes as + // an audit-clean gen-4 successor that wins LWW. + assert_eq!( + after_rename.config.parallelism, + Some(19), + "the rename reverted the previous edit's parallelism from the stale overlay" + ); + // The revert is durable, not just in-flight: it was written back to disk. + assert_eq!(disk.parallelism, 19, "the reverted value also reached disk"); + + // NEGATIVE CONTROL: the same sequence with the write-through omitted must + // FAIL to preserve the edit, or the assertion above is vacuous — it would + // pass on a fixture where the overlay never had a competing value at all. + 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: received_event.content.clone(), + created_at: received_created_at, + raw_event: received_event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + let control_overlay = hydrate_from_retention(&control_conn, &owner_keys).unwrap(); + let mut control_disk = control_overlay.resolve_local_record(&received); + control_disk.parallelism = 19; + retain_agent_record(&control_conn, &owner_keys, &control_disk).unwrap(); + let mut control_renamed = control_overlay.resolve_local_record(&control_disk); + control_renamed.name = "Fizz Relay Rename".into(); + retain_agent_record(&control_conn, &owner_keys, &control_renamed).unwrap(); + let control = published_head(&control_conn, &owner_keys, &pubkey); + assert_eq!( + control.config.parallelism, + Some(17), + "control: WITHOUT the write-through the same sequence reverts to the \ + received head's 17 — this is the defect the test above pins as fixed" + ); +} + +/// The write-through must never CLEAR what the overlay is following. A retain +/// that produced no private row (empty nsec — the keyring-only case) or a +/// coordinate with no head at all must leave the existing entry alone. +#[test] +fn absorb_retained_head_leaves_the_overlay_alone_when_there_is_no_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(); + + let mut record = sample_record(&pubkey, "followed"); + record.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + record.parallelism = 16; + let payload = private_payload_from_record(&record, &owner_hex, 1, None).unwrap(); + let event = private_managed_agent::build_event( + &owner_keys, + &payload, + nostr::Timestamp::now().as_secs(), + ) + .unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex.clone(), + d_tag: pubkey.clone(), + content: event.content.clone(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + let mut overlay = hydrate_from_retention(&conn, &owner_keys).unwrap(); + + // A DIFFERENT db with no rows at all: absorbing must not drop the entry. + let empty = open_retention_db(&dir.path().join("empty.db")).unwrap(); + overlay + .absorb_retained_head(&empty, &owner_keys, &pubkey) + .unwrap(); + assert_eq!( + overlay.len(), + 1, + "a missing head must not clear the overlay" + ); + let mut disk = sample_record(&pubkey, "disk-name"); + disk.parallelism = 1; + assert_eq!( + overlay.resolve_local_record(&disk).parallelism, + 16, + "the followed head is still applied after a no-op absorb" + ); + + // POSITIVE CONTROL: the same call against the db that DOES hold a newer + // head updates the overlay, so the no-op above is the absent row and not a + // broken instrument. + let mut newer = record.clone(); + newer.parallelism = 24; + retain_agent_record(&conn, &owner_keys, &newer).unwrap(); + overlay + .absorb_retained_head(&conn, &owner_keys, &pubkey) + .unwrap(); + assert_eq!( + overlay.resolve_local_record(&disk).parallelism, + 24, + "control: absorbing a present head DOES update the overlay" + ); +} + +/// Source guard for the seam. Both behavioural tests above model +/// `retain_managed_agent_pending`'s body — every caller is inside a +/// `#[tauri::command]` needing a live `AppHandle`, so deleting the production +/// write-through leaves them green. Same weakness, same remedy, as +/// `write_site_resolve_guard` in `private_config_overlay.rs`: assert the call +/// exists in the one helper that all five writers funnel through. +#[cfg(test)] +mod retain_managed_agent_pending_writes_through_to_the_overlay { + const AGENTS_PENDING_RS: &str = include_str!("../../../commands/agents_pending.rs"); + + /// Exactly one — the single seam. A second copy would mean a per-caller + /// write-through crept back in, which is the shape this fix replaced. + #[test] + fn agents_pending_rs_absorbs_the_retained_head_exactly_once() { + assert_eq!( + AGENTS_PENDING_RS.matches("absorb_retained_head(").count(), + 1, + "commands/agents_pending.rs must write the just-retained 30179 head back to \ + the overlay exactly once, in `retain_managed_agent_pending`. Without \ + it the overlay stays pinned at the last RECEIVED generation and the \ + next edit republishes a revert of the previous one (see \ + `sami_second_edit_in_one_session_preserves_the_first_edit`)." + ); + } + + /// Positional, not semantic: the guard above is a substring count and + /// cannot see ordering. Absorbing BEFORE the retain would read the + /// previous head and reintroduce the defect one generation later, so pin + /// the source order too — the behavioural test pins the runtime ordering. + #[test] + fn the_absorb_follows_the_retain() { + let retain = AGENTS_PENDING_RS + .find("retain_agent_record(&conn") + .expect("positive control: the retain call must be present"); + let absorb = AGENTS_PENDING_RS + .find("absorb_retained_head(") + .expect("positive control: the absorb call must be present"); + assert!( + retain < absorb, + "the overlay must absorb the head AFTER the retain writes it" + ); + } + + /// Negative control: prove the searched strings are load-bearing. Without + /// this a typo in either literal makes both guards vacuous. + #[test] + fn the_guard_can_fail() { + let stripped = AGENTS_PENDING_RS.replace("absorb_retained_head(", "REMOVED("); + assert_eq!(stripped.matches("absorb_retained_head(").count(), 0); + assert_ne!( + AGENTS_PENDING_RS + .matches("retain_agent_record(&conn") + .count(), + 0 + ); + } +} 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 00000000000..6a895c84b93 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs @@ -0,0 +1,925 @@ +//! 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" + ); +} + +// ── Review item 5: boot reconcile as a stale-republish site ───────────────── +// +// `reconcile_agents_in_dir_at` reads `managed-agents.json` raw and cannot +// resolve the overlay: `hydrate_private_config_overlay` runs AFTER this leg +// (`event_sync.rs:19-20`) and reads the rows this leg writes. On a following +// device, disk is stale by construction, so rebuilding the 30179 from disk is +// the item-2 clobber with no user action at all. + +/// Builds the follower fixture: a stale disk store plus a NEWER inbound 30179 +/// head (`pending_sync = 0`, far-future `created_at`). Returns the head event. +fn seed_follower_with_newer_head( + dir: &TempDir, + owner_keys: &nostr::Keys, + disk: &ManagedAgentRecord, + fresh: &ManagedAgentRecord, + generation: u64, + created_at: i64, +) -> nostr::Event { + let owner_hex = owner_keys.public_key().to_hex(); + let payload = + private_payload_from_record(fresh, &owner_hex, generation, Some("aa".repeat(32))).unwrap(); + let event = + private_managed_agent::build_event(owner_keys, &payload, created_at as u64).unwrap(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex, + d_tag: disk.pubkey.clone(), + content: event.content.clone(), + created_at, + raw_event: event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + event +} + +fn retained_private_row(dir: &TempDir, owner_keys: &nostr::Keys, pubkey: &str) -> RetainedEvent { + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + pubkey, + ) + .unwrap() + .unwrap() +} + +/// Item 5 FIX: boot reconcile must leave an existing 30179 head alone. +/// +/// Red-first against `retain_agent_record` at boot: the probe measured +/// `name="stale-disk-name"`, `parallelism=Some(1)`, gen 5→6, `prev` = the +/// clobbered head, `created_at` = head+1 (so it wins LWW), `pending_sync=true` +/// — every stale disk field published over device A's newer config at launch, +/// with no user action. +#[test] +fn boot_reconcile_leaves_existing_private_head_intact() { + 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 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; + disk.env_vars = BTreeMap::from([("STALE_KEY".to_string(), "stale".to_string())]); + write_store(&dir, &[disk.clone()]); + + let mut fresh = disk.clone(); + fresh.name = "FRESH relay name".into(); + fresh.system_prompt = Some("FRESH relay prompt".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_event = + seed_follower_with_newer_head(&dir, &owner_keys, &disk, &fresh, 5, head_created_at); + + // BOOT. No user action. + reconcile_agents_in_dir(dir.path(), &owner_keys).unwrap(); + + let row = retained_private_row(&dir, &owner_keys, &pubkey); + assert_eq!( + row.raw_event, + head_event.as_json(), + "boot reconcile must not rebuild the 30179 from stale disk over an \ + existing head — byte-identical, so no gen bump and no re-encryption" + ); + // Not merely equal-by-content: nothing was queued for publish either. + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + assert!( + get_pending_sync(&conn) + .unwrap() + .iter() + .all(|event| event.kind != KIND_PRIVATE_MANAGED_AGENT), + "no stale 30179 enqueued for relay publish" + ); +} + +/// The requirement item 1 exists to serve, preserved: an agent with NO retained +/// 30179 head still publishes its first one at boot. Without this arm the fix +/// above is satisfied by never publishing a 30179 at boot at all — which is the +/// item-1 bug restored. +#[test] +fn boot_reconcile_still_publishes_first_private_config() { + 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 mut record = sample_record(&pubkey, "untouched-agent"); + record.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + record.parallelism = 7; + write_store(&dir, &[record]); + + assert_eq!(reconcile_agents_in_dir(dir.path(), &owner_keys).unwrap(), 1); + + let row = retained_private_row(&dir, &owner_keys, &pubkey); + let (_, payload) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + assert_eq!(payload.generation, 1); + assert_eq!(payload.previous_event_id, None); + assert_eq!(payload.config.parallelism, Some(7)); + assert!(row.pending_sync, "first 30179 is queued for publish"); +} + +/// The 30177 leg must be untouched by the 30179 gate: an edited record whose +/// PUBLIC projection changed still republishes at boot even though a private +/// head exists. This is what keeps the upgrade republish waves +/// (`slimming_republish_wave_is_one_time`) working, and it fails if the gate is +/// written at the wrong level (skipping the whole record instead of the 30179). +#[test] +fn boot_reconcile_still_republishes_public_projection_with_private_head_present() { + 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 mut disk = sample_record(&pubkey, "public-name-v2"); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + write_store(&dir, &[disk.clone()]); + + let mut fresh = disk.clone(); + fresh.parallelism = 16; + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + seed_follower_with_newer_head(&dir, &owner_keys, &disk, &fresh, 5, head_created_at); + + assert_eq!( + reconcile_agents_in_dir(dir.path(), &owner_keys).unwrap(), + 1, + "the 30177 identity projection still reconciles at boot" + ); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + let public_row = get_retained_event( + &conn, + KIND_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + assert!(public_row.content.contains("public-name-v2")); + assert!(public_row.pending_sync); +} + +/// WRONG-FIX PROBE (permanent): the tempting alternative is to resolve the +/// overlay inside boot reconcile (swapping the hydrate/reconcile order in +/// `run_event_sync`). It is wrong for the same reason the centralized resolve +/// was wrong at the edit site — but with a worse blast radius, because boot +/// touches EVERY agent rather than the one being edited. +/// +/// A local edit made while the relay was unreachable lives on disk AND in a +/// `pending_sync` 30179 that never flushed. Resolving disk through an overlay +/// hydrated from the last-known head would rebuild the payload from that older +/// head and discard the edit — at launch, silently, for every agent. +#[test] +fn probe_resolving_overlay_at_boot_would_discard_unflushed_local_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(); + + // The last head this device saw, which is what a boot-time overlay would + // hydrate from. + let mut head = sample_record(&pubkey, "head-name"); + head.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + head.parallelism = 16; + let head_payload = private_payload_from_record(&head, &owner_hex, 3, None).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert_patch(PrivateConfigPatch::from_payload(head_payload).unwrap()); + + // The user's offline edit, on disk and not yet flushed to the relay. + let mut disk = head.clone(); + disk.parallelism = 2; + + assert_eq!( + overlay.resolve_local_record(&disk).parallelism, + 16, + "resolving at boot DISCARDS the unflushed local edit (2 -> 16); this is \ + why the fix is a head-presence gate, not a resolve" + ); + // Positive control: with no patch the edit survives, so the discard above + // is the overlay winning rather than a broken fixture. + assert_eq!( + PrivateConfigOverlay::default() + .resolve_local_record(&disk) + .parallelism, + 2, + "control: without an overlay patch the offline edit survives" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index a225f492d33..7be885ea715 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -214,12 +214,31 @@ pub async fn restore_managed_agents_on_launch( record.updated_at = util::now_iso(); changed = true; } - // Re-collect to_start from the updated records so Phase B spawns the refreshed config. - agents_to_start = records + // Re-collect from the updated records, then resolve each candidate + // through the hydrated relay overlay so Phase B spawns relay-primary + // config instead of the raw disk record. `apply_workspace` awaits + // `run_event_sync_blocking` (which ends with + // `hydrate_private_config_overlay`) before spawning restore, so the + // overlay is populated by the time this runs — without this resolve, a + // follower device would hydrate config B and then launch stale disk + // config A (obsolete prompt/model/ACL/identity). Mirrors the resolve + // in `start_local_agent_with_preflight`: overlay patch first, then + // persona snapshot re-applied on top so the linked definition keeps + // its established precedence over both disk and relay bytes. + let mut resolved_to_start = Vec::with_capacity(agents_to_start.len()); + for record in records .iter() .filter(|r| agents_to_start.iter().any(|s| s.pubkey == r.pubkey)) - .cloned() - .collect(); + { + let resolved = crate::managed_agents::private_config_overlay::resolved_local_record( + &state, record, + )?; + if let Some(spawn_record) = finalize_restore_candidate(resolved, &personas_for_snapshot) + { + resolved_to_start.push(spawn_record); + } + } + agents_to_start = resolved_to_start; if changed { save_managed_agents(app, &records)?; @@ -427,13 +446,16 @@ pub async fn restore_managed_agents_on_launch( // Collect profile reconciliation data for successfully spawned agents before // releasing the lock. This mirrors the fire-and-forget pattern in // start_managed_agent — ensuring boot-restored agents get the same profile - // self-healing as UI-started agents. + // self-healing as UI-started agents. Read from `agents_to_start` (the + // overlay-resolved spawn records), not the raw disk `records`, so the + // reconciled profile matches the config the spawn actually used — the + // interactive path builds its reconcile data from the resolved record too. let reconcile_personas = super::load_personas(app).unwrap_or_default(); let reconcile_items: Vec<(String, crate::commands::ProfileReconcileData)> = successfully_spawned .iter() .filter_map(|(pubkey, spawn_relay)| { - let record = records.iter().find(|r| r.pubkey == *pubkey)?; + let record = agents_to_start.iter().find(|r| r.pubkey == *pubkey)?; // Resolve the effective harness for the avatar-fallback // derivation (the snapshot may be empty/stale for an inherited // harness). Mirrors the UI start path. @@ -487,6 +509,37 @@ fn profile_reconcile_completed(outcome: crate::commands::ProfileReconcileOutcome outcome == crate::commands::ProfileReconcileOutcome::Reconciled } +/// Final shaping of one Phase-A spawn candidate AFTER the relay-overlay +/// resolve, mirroring `start_local_agent_with_preflight` exactly: +/// +/// - a resolved backend that is no longer local cannot be spawned by this +/// device — the candidate is dropped (`None`); +/// - the linked persona snapshot is re-applied ON TOP of the overlay patch, +/// so the definition-authoritative quad keeps its established precedence +/// over both disk and relay bytes; +/// - an orphaned instance (persona_id with no live persona) is passed through +/// untouched: `spawn_agent_child` refuses it via +/// `resolve_effective_config`'s `OrphanedInstance` arm and Phase C persists +/// the refusal to `last_error` — restore must not silently drop it. +/// +/// Pure over (record, personas) so the restore fold is testable without an +/// `AppHandle`. +fn finalize_restore_candidate( + mut resolved: super::ManagedAgentRecord, + personas: &[super::AgentDefinition], +) -> Option { + if resolved.backend != BackendKind::Local { + return None; + } + if let Some(persona_id) = resolved.persona_id.clone() { + if let Some(persona) = personas.iter().find(|p| p.id == persona_id) { + super::persona_events::apply_persona_snapshot(&mut resolved, persona); + resolved.updated_at = util::now_iso(); + } + } + Some(resolved) +} + pub(crate) fn spawn_pending_profile_reconciliations(app: &tauri::AppHandle, workspace_relay: &str) { let state = app.state::(); if !state @@ -550,6 +603,203 @@ mod profile_reconcile_tests { } } +// ── Restore fold: relay-overlay resolve before spawn ──────────────────────── +// +// The production wiring (Phase A resolving every candidate through +// `resolved_local_record` before Phase B spawns) needs a live `AppHandle`, so +// its presence is pinned by `write_site_resolve_guard` in +// `private_config_overlay.rs`. These tests prove the fold itself: what a +// resolved candidate looks like, at the same overlay + finalize seam the +// production path composes. +#[cfg(test)] +mod restore_fold_tests { + use super::finalize_restore_candidate; + use crate::managed_agents::private_config_overlay::PrivateConfigOverlay; + use crate::managed_agents::{AgentDefinition, BackendKind, ManagedAgentRecord}; + use buzz_core_pkg::private_managed_agent::{ + Payload, PrivateConfig, PrivateIdentity, FORMAT, VERSION, + }; + use std::collections::BTreeMap; + + fn relay_payload(pubkey: &str) -> Payload { + Payload { + format: FORMAT.into(), + version: VERSION, + agent_pubkey: pubkey.into(), + owner_pubkey: "11".repeat(32), + generation: 2, + previous_event_id: None, + updated_at: "2026-08-20T00:00:00Z".into(), + identity: PrivateIdentity { + private_key_nsec: "nsec-relay".into(), + auth_tag: None, + }, + config: PrivateConfig { + relay_url: "wss://relay.example".into(), + name: "relay name".into(), + persona_id: None, + runtime: Some("goose".into()), + model: Some("relay-model".into()), + provider: None, + system_prompt: Some("relay prompt".into()), + parallelism: Some(4), + respond_to: None, + respond_to_allowlist: vec![], + agent_command_override: None, + agent_args: vec![], + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + env_vars: BTreeMap::new(), + backend: serde_json::json!({"type":"local"}), + backend_agent_id: None, + team_id: None, + persona_name_in_team: None, + relay_mesh: None, + extra: serde_json::Map::new(), + }, + extensions: BTreeMap::new(), + extra: serde_json::Map::new(), + } + } + + /// A stale disk record flagged for auto-start, as Phase A collects it. + fn stale_disk_record(pubkey: &str) -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": pubkey, + "name": "stale disk name", + "private_key_nsec": "nsec-stale-disk", + "relay_url": "wss://old.example", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": "stale disk prompt", + "model": "stale-model", + "provider": null, + "env_vars": {}, + "start_on_app_launch": true, + "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("stale_disk_record fixture") + } + + /// Issue-1 regression (stale-disk / newer-retained-head / start-on-launch): + /// a hydrated overlay head must win over the disk record for everything + /// relay-owned, while disk-only lifecycle state (`start_on_app_launch`) + /// survives untouched — the pair that previously spawned "config A shown + /// as config B". + #[test] + fn restore_candidate_spawns_relay_config_not_stale_disk() { + let pubkey = "aa".repeat(32); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(relay_payload(&pubkey)).unwrap(); + + let disk = stale_disk_record(&pubkey); + let resolved = overlay.resolve_local_record(&disk); + let spawn = finalize_restore_candidate(resolved, &[]) + .expect("local candidate must survive the fold"); + + assert_eq!(spawn.name, "relay name"); + assert_eq!(spawn.system_prompt.as_deref(), Some("relay prompt")); + assert_eq!(spawn.model.as_deref(), Some("relay-model")); + assert_eq!(spawn.private_key_nsec, "nsec-relay"); + assert_eq!(spawn.relay_url, "wss://relay.example"); + assert_eq!(spawn.parallelism, 4); + assert!( + spawn.start_on_app_launch, + "device-local lifecycle flag must survive the overlay resolve" + ); + + // NEGATIVE CONTROL: an empty overlay leaves the disk record as-is — + // the assertions above are proving the patch, not the fixture. + let untouched = PrivateConfigOverlay::default().resolve_local_record(&disk); + assert_eq!(untouched.name, "stale disk name"); + assert_eq!(untouched.private_key_nsec, "nsec-stale-disk"); + } + + /// The linked persona keeps its definition-authoritative precedence over + /// BOTH disk and relay bytes — mirroring the interactive start path, which + /// re-applies the snapshot after the overlay resolve. + #[test] + fn persona_snapshot_reapplies_on_top_of_overlay_patch() { + let pubkey = "bb".repeat(32); + let mut payload = relay_payload(&pubkey); + payload.config.persona_id = Some("def-1".into()); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(payload).unwrap(); + + let persona = AgentDefinition { + id: "def-1".into(), + display_name: "Definition".into(), + avatar_url: None, + system_prompt: "definition prompt".into(), + runtime: Some("goose".into()), + model: Some("definition-model".into()), + provider: None, + name_pool: vec![], + 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![], + parallelism: None, + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + }; + + let resolved = overlay.resolve_local_record(&stale_disk_record(&pubkey)); + let spawn = finalize_restore_candidate(resolved, std::slice::from_ref(&persona)).unwrap(); + assert_eq!(spawn.system_prompt.as_deref(), Some("definition prompt")); + assert_eq!(spawn.model.as_deref(), Some("definition-model")); + // Relay still owns what the definition does not. + assert_eq!(spawn.name, "relay name"); + assert_eq!(spawn.private_key_nsec, "nsec-relay"); + } + + /// A candidate whose RESOLVED backend is no longer local must not spawn on + /// this device — same refusal as `start_local_agent_with_preflight`. + #[test] + fn non_local_resolved_backend_is_dropped() { + let pubkey = "cc".repeat(32); + let mut record = stale_disk_record(&pubkey); + record.backend = BackendKind::Provider { + id: "cloud".into(), + config: serde_json::json!({}), + }; + assert!( + finalize_restore_candidate(record, &[]).is_none(), + "a non-local resolved backend must be refused at the fold" + ); + } + + /// An orphaned instance (persona_id with no live persona) passes through + /// unchanged: `spawn_agent_child` owns the refusal so Phase C persists it + /// to `last_error` — the fold must not silently drop the record. + #[test] + fn orphaned_instance_passes_through_for_spawn_refusal() { + let pubkey = "dd".repeat(32); + let mut record = stale_disk_record(&pubkey); + record.persona_id = Some("gone".into()); + let spawn = finalize_restore_candidate(record, &[]).unwrap(); + assert_eq!(spawn.persona_id.as_deref(), Some("gone")); + assert_eq!( + spawn.system_prompt.as_deref(), + Some("stale disk prompt"), + "no persona to re-apply: record passes through untouched" + ); + } +} + #[cfg(feature = "mesh-llm")] fn persist_restore_error( app: &tauri::AppHandle, diff --git a/desktop/src-tauri/src/managed_agents/retention.rs b/desktop/src-tauri/src/managed_agents/retention.rs index e6231bbe42b..3ea60898447 100644 --- a/desktop/src-tauri/src/managed_agents/retention.rs +++ b/desktop/src-tauri/src/managed_agents/retention.rs @@ -255,11 +255,17 @@ pub enum InboundOutcome { /// - No local row, or inbound strictly newer (`created_at >`): apply the /// inbound event, clearing `pending_sync`. Inbound wins; a stale local edit /// the relay already superseded stops republishing instead of looping. -/// - Equal `created_at`: skip. Nostr time is seconds-granularity, so a pending -/// local edit and an inbound event can share a timestamp; applying here would -/// clear `pending_sync` and drop the local publish. Skipping leaves the -/// pending row intact so the flush republishes and the relay resolves -/// last-writer-wins. (A re-received echo at equal time is also a no-op.) +/// - Equal `created_at`: NIP-01 addressable-event tiebreak — the event with +/// the lexicographically LOWEST id wins, exactly the head the relay itself +/// retains. Nostr time is seconds-granularity, so two devices can retain +/// distinct successors in the same second; without a shared deterministic +/// winner each side skips the other's head on every replay and the devices +/// diverge permanently. A pending local edit that WINS the tie stays +/// pending and republishes; one that LOSES is superseded — the relay would +/// refuse it as the head anyway, so clearing its `pending_sync` is what +/// converges both devices onto the relay's answer. (A re-received echo has +/// an equal id and stays a no-op; if either id is unavailable the inbound +/// event is skipped, preserving any pending local publish.) /// - Inbound older: skip — nothing to change. /// /// Decide whether an inbound event is newer than the retained coordinate without @@ -274,12 +280,39 @@ pub fn inbound_event_outcome( Ok(match existing { None => InboundOutcome::Applied, Some(row) if event.created_at > row.created_at => InboundOutcome::Applied, - // Equal or older: skip. Equal time may collide with a pending local - // edit, so we never clear its `pending_sync`; older is stale. + Some(row) + if event.created_at == row.created_at + && equal_second_inbound_wins(&event.raw_event, &row.raw_event) => + { + InboundOutcome::Applied + } + // Older, or an equal-second loser/echo: skip. A pending local edit + // that won (or an undecidable tie) keeps its `pending_sync`. Some(_) => InboundOutcome::Skipped, }) } +/// NIP-01 addressable-event tiebreak at equal `created_at`: the event with the +/// lexicographically lowest id is the head the relay retains. Returns `true` +/// only when BOTH ids are present and the inbound id is strictly lower — +/// an undecidable or equal comparison must not clobber the retained row (or a +/// pending local publish riding on it). +fn equal_second_inbound_wins(inbound_raw: &str, retained_raw: &str) -> bool { + match (raw_event_id(inbound_raw), raw_event_id(retained_raw)) { + (Some(inbound_id), Some(retained_id)) => inbound_id < retained_id, + _ => false, + } +} + +/// Extract the `id` field from a raw event JSON string, if present. +fn raw_event_id(raw_event: &str) -> Option { + serde_json::from_str::(raw_event) + .ok()? + .get("id")? + .as_str() + .map(str::to_owned) +} + pub fn retain_inbound_event( conn: &Connection, event: &RetainedEvent, @@ -443,6 +476,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, @@ -472,505 +542,4 @@ pub fn get_retained_event( } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn retention_scope_is_stable_and_separates_relay_and_owner() { - let base = Path::new("/tmp/buzz-retention-test"); - let owner_a = "a".repeat(64); - let owner_b = "b".repeat(64); - let community_a = scoped_retention_db_path(base, "wss://a.example/", &owner_a); - assert_eq!( - community_a, - scoped_retention_db_path(base, "wss://a.example", &owner_a) - ); - assert_ne!( - community_a, - scoped_retention_db_path(base, "wss://b.example", &owner_a) - ); - assert_ne!( - community_a, - scoped_retention_db_path(base, "wss://a.example", &owner_b) - ); - } - - #[test] - fn test_arrival_relay_matching_agrees_with_database_identity() { - let base = Path::new("/tmp/buzz-retention-test"); - let keys = nostr::Keys::generate(); - let owner = keys.public_key().to_hex(); - let scope = |relay: &str| RetentionScope { - db_path: scoped_retention_db_path(base, relay, &owner), - relay_url: relay.to_string(), - owner_keys: keys.clone(), - }; - let community_a = scoped_retention_db_path(base, "wss://a.example", &owner); - - // "Same relay" and "same database" must never disagree: every URL the - // match accepts has to hash to the scope's own db path, and every URL it - // rejects has to hash somewhere else. - for equivalent in ["wss://a.example", "wss://a.example/", " wss://a.example "] { - assert_eq!( - scope_for_arrival(scope("wss://a.example"), equivalent).map(|scope| scope.db_path), - Some(community_a.clone()), - "{equivalent}" - ); - assert_eq!( - scoped_retention_db_path(base, equivalent, &owner), - community_a, - "{equivalent}" - ); - } - - assert!( - scope_for_arrival(scope("wss://b.example"), "wss://a.example").is_none(), - "an event from community A must not be filed while community B is active" - ); - assert_ne!( - scoped_retention_db_path(base, "wss://b.example", &owner), - community_a - ); - } - - #[test] - fn concurrent_open_waits_for_initialization_lock() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("retention.db"); - let first = open_retention_db(&path).unwrap(); - first.execute_batch("BEGIN EXCLUSIVE").unwrap(); - - let second_path = path.clone(); - let second = std::thread::spawn(move || open_retention_db(&second_path)); - std::thread::sleep(std::time::Duration::from_millis(100)); - first.execute_batch("COMMIT").unwrap(); - - assert!(second.join().unwrap().is_ok()); - } - - fn test_db() -> Connection { - open_retention_db(Path::new(":memory:")).unwrap() - } - - fn sample_event() -> RetainedEvent { - RetainedEvent { - kind: 30175, - pubkey: "abc123".to_string(), - d_tag: "test-persona".to_string(), - content: r#"{"display_name":"Test"}"#.to_string(), - created_at: 1000, - raw_event: r#"{"id":"..."}"#.to_string(), - pending_sync: true, - } - } - - #[test] - fn inbound_preflight_does_not_consume_event_before_commit() { - let conn = test_db(); - let mut inbound = sample_event(); - inbound.pending_sync = false; - - assert_eq!( - inbound_event_outcome(&conn, &inbound).unwrap(), - InboundOutcome::Applied - ); - assert!( - get_retained_event(&conn, inbound.kind, &inbound.pubkey, &inbound.d_tag) - .unwrap() - .is_none() - ); - // A failed store/runtime apply can replay the same head because the - // preflight did not advance retention. - assert_eq!( - inbound_event_outcome(&conn, &inbound).unwrap(), - InboundOutcome::Applied - ); - assert_eq!( - retain_inbound_event(&conn, &inbound).unwrap(), - InboundOutcome::Applied - ); - assert_eq!( - inbound_event_outcome(&conn, &inbound).unwrap(), - InboundOutcome::Skipped - ); - } - - #[test] - fn retain_and_retrieve() { - let conn = test_db(); - let event = sample_event(); - retain_event(&conn, &event).unwrap(); - - let results = get_retained_personas(&conn, "abc123").unwrap(); - assert_eq!(results.len(), 1); - assert_eq!(results[0].d_tag, "test-persona"); - assert_eq!(results[0].created_at, 1000); - assert!(results[0].pending_sync); - } - - #[test] - fn tombstone_retention_keys_are_distinct_across_kinds() { - // A persona slug, team id, and agent pubkey that all happen to equal - // "shared" must occupy DISTINCT kind:5 rows so one tombstone's pending - // publish never clobbers another's (F2c). - let conn = test_db(); - for target_kind in [30175u32, 30176, 30177] { - retain_event( - &conn, - &RetainedEvent { - kind: 5, - pubkey: "owner".to_string(), - d_tag: tombstone_retention_d_tag(target_kind, "shared"), - content: String::new(), - created_at: 1000, - raw_event: format!("{{\"k\":{target_kind}}}"), - pending_sync: true, - }, - ) - .unwrap(); - } - // Three distinct rows survive — no PK collision clobbered any of them. - for target_kind in [30175u32, 30176, 30177] { - let row = get_retained_event( - &conn, - 5, - "owner", - &tombstone_retention_d_tag(target_kind, "shared"), - ) - .unwrap(); - assert!( - row.is_some(), - "tombstone for kind {target_kind} was clobbered" - ); - } - } - - #[test] - fn upsert_replaces_newer() { - let conn = test_db(); - let mut event = sample_event(); - retain_event(&conn, &event).unwrap(); - - event.content = r#"{"display_name":"Updated"}"#.to_string(); - event.created_at = 2000; - retain_event(&conn, &event).unwrap(); - - let results = get_retained_personas(&conn, "abc123").unwrap(); - assert_eq!(results.len(), 1); - assert_eq!(results[0].created_at, 2000); - assert!(results[0].content.contains("Updated")); - } - - #[test] - fn upsert_ignores_older() { - let conn = test_db(); - let mut event = sample_event(); - event.created_at = 2000; - retain_event(&conn, &event).unwrap(); - - event.content = r#"{"display_name":"Old"}"#.to_string(); - event.created_at = 1000; - retain_event(&conn, &event).unwrap(); - - let results = get_retained_personas(&conn, "abc123").unwrap(); - assert_eq!(results.len(), 1); - assert_eq!(results[0].created_at, 2000); - assert!(!results[0].content.contains("Old")); - } - - #[test] - fn pending_sync_query() { - let conn = test_db(); - let mut event = sample_event(); - event.pending_sync = true; - retain_event(&conn, &event).unwrap(); - - let mut event2 = sample_event(); - event2.d_tag = "other".to_string(); - event2.pending_sync = false; - retain_event(&conn, &event2).unwrap(); - - let pending = get_pending_sync(&conn).unwrap(); - assert_eq!(pending.len(), 1); - assert_eq!(pending[0].d_tag, "test-persona"); - } - - #[test] - fn test_mark_synced_matching_row_clears_flag() { - let conn = test_db(); - let event = sample_event(); - retain_event(&conn, &event).unwrap(); - - mark_synced(&conn, 30175, "abc123", "test-persona", 1000, &event.content).unwrap(); - - let pending = get_pending_sync(&conn).unwrap(); - assert!(pending.is_empty()); - - let results = get_retained_personas(&conn, "abc123").unwrap(); - assert_eq!(results.len(), 1); - assert!(!results[0].pending_sync); - } - - #[test] - fn test_mark_synced_stale_version_leaves_flag_set() { - let conn = test_db(); - let published = sample_event(); - retain_event(&conn, &published).unwrap(); - - // A newer edit lands at the same coordinate before the flush loop - // clears the version it published. - let mut newer = sample_event(); - newer.content = r#"{"display_name":"Edited"}"#.to_string(); - newer.created_at = 2000; - retain_event(&conn, &newer).unwrap(); - - // Clearing against the OLD version must not touch the newer pending row. - mark_synced( - &conn, - 30175, - "abc123", - "test-persona", - 1000, - &published.content, - ) - .unwrap(); - - let pending = get_pending_sync(&conn).unwrap(); - assert_eq!(pending.len(), 1); - assert_eq!(pending[0].created_at, 2000); - } - - #[test] - fn test_delete_retained_event_removes_row() { - let conn = test_db(); - retain_event(&conn, &sample_event()).unwrap(); - - delete_retained_event(&conn, 30175, "abc123", "test-persona").unwrap(); - - assert!(get_retained_event(&conn, 30175, "abc123", "test-persona") - .unwrap() - .is_none()); - } - - #[test] - fn test_delete_retained_event_missing_row_is_noop() { - let conn = test_db(); - delete_retained_event(&conn, 30175, "abc123", "nonexistent").unwrap(); - } - - #[test] - fn has_retained_personas_works() { - let conn = test_db(); - assert!(!has_retained_personas(&conn, "abc123").unwrap()); - - let event = sample_event(); - retain_event(&conn, &event).unwrap(); - - assert!(has_retained_personas(&conn, "abc123").unwrap()); - assert!(!has_retained_personas(&conn, "other").unwrap()); - } - - #[test] - fn get_retained_event_by_coordinate() { - let conn = test_db(); - let event = sample_event(); - retain_event(&conn, &event).unwrap(); - - let found = get_retained_event(&conn, 30175, "abc123", "test-persona").unwrap(); - assert!(found.is_some()); - assert_eq!(found.unwrap().d_tag, "test-persona"); - - let not_found = get_retained_event(&conn, 30175, "abc123", "nonexistent").unwrap(); - assert!(not_found.is_none()); - } - - #[test] - fn idempotent_retain_same_timestamp() { - let conn = test_db(); - let event = sample_event(); - retain_event(&conn, &event).unwrap(); - retain_event(&conn, &event).unwrap(); - - let results = get_retained_personas(&conn, "abc123").unwrap(); - assert_eq!(results.len(), 1); - } - - #[test] - fn inbound_no_local_row_applies() { - let conn = test_db(); - let mut event = sample_event(); - event.pending_sync = false; - - assert_eq!( - retain_inbound_event(&conn, &event).unwrap(), - InboundOutcome::Applied - ); - - let row = get_retained_event(&conn, 30175, "abc123", "test-persona") - .unwrap() - .unwrap(); - assert_eq!(row.created_at, 1000); - assert!(!row.pending_sync); - } - - #[test] - fn inbound_equal_second_skips_and_preserves_pending() { - let conn = test_db(); - // Pending local edit at t=1000. - let local = sample_event(); - retain_event(&conn, &local).unwrap(); - - // Inbound at the SAME second with different content. - let inbound = RetainedEvent { - content: r#"{"display_name":"Remote"}"#.to_string(), - pending_sync: false, - ..sample_event() - }; - assert_eq!( - retain_inbound_event(&conn, &inbound).unwrap(), - InboundOutcome::Skipped - ); - - // Local pending row is untouched: flag preserved, content unchanged so - // the flush republishes and the relay resolves last-writer-wins. - let row = get_retained_event(&conn, 30175, "abc123", "test-persona") - .unwrap() - .unwrap(); - assert!(row.pending_sync); - assert!(row.content.contains("Test")); - } - - #[test] - fn inbound_strictly_newer_applies_and_clears_pending() { - let conn = test_db(); - // Pending local edit at t=1000. - let local = sample_event(); - retain_event(&conn, &local).unwrap(); - - // Inbound strictly newer with different content. - let inbound = RetainedEvent { - content: r#"{"display_name":"Remote"}"#.to_string(), - created_at: 2000, - pending_sync: false, - ..sample_event() - }; - assert_eq!( - retain_inbound_event(&conn, &inbound).unwrap(), - InboundOutcome::Applied - ); - - // Inbound wins: content replaced and pending cleared, so the stale - // local edit stops republishing instead of looping. - let row = get_retained_event(&conn, 30175, "abc123", "test-persona") - .unwrap() - .unwrap(); - assert_eq!(row.created_at, 2000); - assert!(!row.pending_sync); - assert!(row.content.contains("Remote")); - } - - #[test] - fn inbound_older_skips() { - let conn = test_db(); - let mut local = sample_event(); - local.created_at = 2000; - retain_event(&conn, &local).unwrap(); - - let inbound = RetainedEvent { - content: r#"{"display_name":"Stale"}"#.to_string(), - created_at: 1000, - pending_sync: false, - ..sample_event() - }; - assert_eq!( - retain_inbound_event(&conn, &inbound).unwrap(), - InboundOutcome::Skipped - ); - - let row = get_retained_event(&conn, 30175, "abc123", "test-persona") - .unwrap() - .unwrap(); - assert_eq!(row.created_at, 2000); - assert!(!row.content.contains("Stale")); - } - - #[test] - fn pending_sync_publishes_tombstones_before_replacements() { - // B5 resurrection race: a kind:5 retained in session N and the same - // coordinate's replacement 30175 retained on the next boot can sit - // pending together. The relay's a-tag deletion ignores timestamps, - // so the tombstone MUST publish first or it wipes the replacement. - let conn = test_db(); - let replacement = RetainedEvent { - kind: 30175, - created_at: 2000, - pending_sync: true, - ..sample_event() - }; - retain_event(&conn, &replacement).unwrap(); - let tombstone = RetainedEvent { - kind: 5, - d_tag: tombstone_retention_d_tag(30175, "test-persona"), - content: String::new(), - created_at: 1000, - pending_sync: true, - ..sample_event() - }; - retain_event(&conn, &tombstone).unwrap(); - - let pending = get_pending_sync(&conn).unwrap(); - assert_eq!(pending.len(), 2); - assert_eq!(pending[0].kind, 5, "tombstone first"); - assert_eq!(pending[1].kind, 30175, "replacement second"); - } - - #[test] - fn deferral_predicate_is_kind_and_pubkey_qualified() { - // Mid-sweep barrier semantics: a failed tombstone defers ONLY the - // replacement at its exact coordinate — same target kind, same pubkey. - use std::collections::HashSet; - - let failed: HashSet<(String, String)> = HashSet::from([( - "abc123".to_string(), - tombstone_retention_d_tag(30175, "test-persona"), - )]); - - // The covered replacement defers. - assert!(deferred_behind_failed_tombstone( - 30175, - "abc123", - "test-persona", - &failed - )); - // Kind-qualified: a coinciding slug under a DIFFERENT kind is a - // distinct coordinate (the cross-kind collision the retention d-tag - // encoding exists to prevent) — never deferred. - assert!(!deferred_behind_failed_tombstone( - 30177, - "abc123", - "test-persona", - &failed - )); - // Never crosses pubkeys. - assert!(!deferred_behind_failed_tombstone( - 30175, - "other-key", - "test-persona", - &failed - )); - // Never defers kind:5 rows, even at a "matching" retention key. - assert!(!deferred_behind_failed_tombstone( - 5, - "abc123", - "test-persona", - &failed - )); - // Unrelated d-tags publish normally. - assert!(!deferred_behind_failed_tombstone( - 30175, - "abc123", - "other-persona", - &failed - )); - } -} +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/retention/tests.rs b/desktop/src-tauri/src/managed_agents/retention/tests.rs new file mode 100644 index 00000000000..fc625e06fff --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/retention/tests.rs @@ -0,0 +1,614 @@ +//! Unit tests for `managed_agents/retention.rs`. +//! +//! Kept in a sibling file so `retention.rs` stays under the 1000-line +//! file-size ratchet; `#[path]`-included from its `tests` module. + +use super::*; + +#[test] +fn retention_scope_is_stable_and_separates_relay_and_owner() { + let base = Path::new("/tmp/buzz-retention-test"); + let owner_a = "a".repeat(64); + let owner_b = "b".repeat(64); + let community_a = scoped_retention_db_path(base, "wss://a.example/", &owner_a); + assert_eq!( + community_a, + scoped_retention_db_path(base, "wss://a.example", &owner_a) + ); + assert_ne!( + community_a, + scoped_retention_db_path(base, "wss://b.example", &owner_a) + ); + assert_ne!( + community_a, + scoped_retention_db_path(base, "wss://a.example", &owner_b) + ); +} + +#[test] +fn test_arrival_relay_matching_agrees_with_database_identity() { + let base = Path::new("/tmp/buzz-retention-test"); + let keys = nostr::Keys::generate(); + let owner = keys.public_key().to_hex(); + let scope = |relay: &str| RetentionScope { + db_path: scoped_retention_db_path(base, relay, &owner), + relay_url: relay.to_string(), + owner_keys: keys.clone(), + }; + let community_a = scoped_retention_db_path(base, "wss://a.example", &owner); + + // "Same relay" and "same database" must never disagree: every URL the + // match accepts has to hash to the scope's own db path, and every URL it + // rejects has to hash somewhere else. + for equivalent in ["wss://a.example", "wss://a.example/", " wss://a.example "] { + assert_eq!( + scope_for_arrival(scope("wss://a.example"), equivalent).map(|scope| scope.db_path), + Some(community_a.clone()), + "{equivalent}" + ); + assert_eq!( + scoped_retention_db_path(base, equivalent, &owner), + community_a, + "{equivalent}" + ); + } + + assert!( + scope_for_arrival(scope("wss://b.example"), "wss://a.example").is_none(), + "an event from community A must not be filed while community B is active" + ); + assert_ne!( + scoped_retention_db_path(base, "wss://b.example", &owner), + community_a + ); +} + +#[test] +fn concurrent_open_waits_for_initialization_lock() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("retention.db"); + let first = open_retention_db(&path).unwrap(); + first.execute_batch("BEGIN EXCLUSIVE").unwrap(); + + let second_path = path.clone(); + let second = std::thread::spawn(move || open_retention_db(&second_path)); + std::thread::sleep(std::time::Duration::from_millis(100)); + first.execute_batch("COMMIT").unwrap(); + + assert!(second.join().unwrap().is_ok()); +} + +fn test_db() -> Connection { + open_retention_db(Path::new(":memory:")).unwrap() +} + +fn sample_event() -> RetainedEvent { + RetainedEvent { + kind: 30175, + pubkey: "abc123".to_string(), + d_tag: "test-persona".to_string(), + content: r#"{"display_name":"Test"}"#.to_string(), + created_at: 1000, + raw_event: r#"{"id":"..."}"#.to_string(), + pending_sync: true, + } +} + +#[test] +fn inbound_preflight_does_not_consume_event_before_commit() { + let conn = test_db(); + let mut inbound = sample_event(); + inbound.pending_sync = false; + + assert_eq!( + inbound_event_outcome(&conn, &inbound).unwrap(), + InboundOutcome::Applied + ); + assert!( + get_retained_event(&conn, inbound.kind, &inbound.pubkey, &inbound.d_tag) + .unwrap() + .is_none() + ); + // A failed store/runtime apply can replay the same head because the + // preflight did not advance retention. + assert_eq!( + inbound_event_outcome(&conn, &inbound).unwrap(), + InboundOutcome::Applied + ); + assert_eq!( + retain_inbound_event(&conn, &inbound).unwrap(), + InboundOutcome::Applied + ); + assert_eq!( + inbound_event_outcome(&conn, &inbound).unwrap(), + InboundOutcome::Skipped + ); +} + +#[test] +fn retain_and_retrieve() { + let conn = test_db(); + let event = sample_event(); + retain_event(&conn, &event).unwrap(); + + let results = get_retained_personas(&conn, "abc123").unwrap(); + assert_eq!(results.len(), 1); + assert_eq!(results[0].d_tag, "test-persona"); + assert_eq!(results[0].created_at, 1000); + assert!(results[0].pending_sync); +} + +#[test] +fn tombstone_retention_keys_are_distinct_across_kinds() { + // A persona slug, team id, and agent pubkey that all happen to equal + // "shared" must occupy DISTINCT kind:5 rows so one tombstone's pending + // publish never clobbers another's (F2c). + let conn = test_db(); + for target_kind in [30175u32, 30176, 30177] { + retain_event( + &conn, + &RetainedEvent { + kind: 5, + pubkey: "owner".to_string(), + d_tag: tombstone_retention_d_tag(target_kind, "shared"), + content: String::new(), + created_at: 1000, + raw_event: format!("{{\"k\":{target_kind}}}"), + pending_sync: true, + }, + ) + .unwrap(); + } + // Three distinct rows survive — no PK collision clobbered any of them. + for target_kind in [30175u32, 30176, 30177] { + let row = get_retained_event( + &conn, + 5, + "owner", + &tombstone_retention_d_tag(target_kind, "shared"), + ) + .unwrap(); + assert!( + row.is_some(), + "tombstone for kind {target_kind} was clobbered" + ); + } +} + +#[test] +fn upsert_replaces_newer() { + let conn = test_db(); + let mut event = sample_event(); + retain_event(&conn, &event).unwrap(); + + event.content = r#"{"display_name":"Updated"}"#.to_string(); + event.created_at = 2000; + retain_event(&conn, &event).unwrap(); + + let results = get_retained_personas(&conn, "abc123").unwrap(); + assert_eq!(results.len(), 1); + assert_eq!(results[0].created_at, 2000); + assert!(results[0].content.contains("Updated")); +} + +#[test] +fn upsert_ignores_older() { + let conn = test_db(); + let mut event = sample_event(); + event.created_at = 2000; + retain_event(&conn, &event).unwrap(); + + event.content = r#"{"display_name":"Old"}"#.to_string(); + event.created_at = 1000; + retain_event(&conn, &event).unwrap(); + + let results = get_retained_personas(&conn, "abc123").unwrap(); + assert_eq!(results.len(), 1); + assert_eq!(results[0].created_at, 2000); + assert!(!results[0].content.contains("Old")); +} + +#[test] +fn pending_sync_query() { + let conn = test_db(); + let mut event = sample_event(); + event.pending_sync = true; + retain_event(&conn, &event).unwrap(); + + let mut event2 = sample_event(); + event2.d_tag = "other".to_string(); + event2.pending_sync = false; + retain_event(&conn, &event2).unwrap(); + + let pending = get_pending_sync(&conn).unwrap(); + assert_eq!(pending.len(), 1); + assert_eq!(pending[0].d_tag, "test-persona"); +} + +#[test] +fn test_mark_synced_matching_row_clears_flag() { + let conn = test_db(); + let event = sample_event(); + retain_event(&conn, &event).unwrap(); + + mark_synced(&conn, 30175, "abc123", "test-persona", 1000, &event.content).unwrap(); + + let pending = get_pending_sync(&conn).unwrap(); + assert!(pending.is_empty()); + + let results = get_retained_personas(&conn, "abc123").unwrap(); + assert_eq!(results.len(), 1); + assert!(!results[0].pending_sync); +} + +#[test] +fn test_mark_synced_stale_version_leaves_flag_set() { + let conn = test_db(); + let published = sample_event(); + retain_event(&conn, &published).unwrap(); + + // A newer edit lands at the same coordinate before the flush loop + // clears the version it published. + let mut newer = sample_event(); + newer.content = r#"{"display_name":"Edited"}"#.to_string(); + newer.created_at = 2000; + retain_event(&conn, &newer).unwrap(); + + // Clearing against the OLD version must not touch the newer pending row. + mark_synced( + &conn, + 30175, + "abc123", + "test-persona", + 1000, + &published.content, + ) + .unwrap(); + + let pending = get_pending_sync(&conn).unwrap(); + assert_eq!(pending.len(), 1); + assert_eq!(pending[0].created_at, 2000); +} + +#[test] +fn test_delete_retained_event_removes_row() { + let conn = test_db(); + retain_event(&conn, &sample_event()).unwrap(); + + delete_retained_event(&conn, 30175, "abc123", "test-persona").unwrap(); + + assert!(get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .is_none()); +} + +#[test] +fn test_delete_retained_event_missing_row_is_noop() { + let conn = test_db(); + delete_retained_event(&conn, 30175, "abc123", "nonexistent").unwrap(); +} + +#[test] +fn has_retained_personas_works() { + let conn = test_db(); + assert!(!has_retained_personas(&conn, "abc123").unwrap()); + + let event = sample_event(); + retain_event(&conn, &event).unwrap(); + + assert!(has_retained_personas(&conn, "abc123").unwrap()); + assert!(!has_retained_personas(&conn, "other").unwrap()); +} + +#[test] +fn get_retained_event_by_coordinate() { + let conn = test_db(); + let event = sample_event(); + retain_event(&conn, &event).unwrap(); + + let found = get_retained_event(&conn, 30175, "abc123", "test-persona").unwrap(); + assert!(found.is_some()); + assert_eq!(found.unwrap().d_tag, "test-persona"); + + let not_found = get_retained_event(&conn, 30175, "abc123", "nonexistent").unwrap(); + assert!(not_found.is_none()); +} + +#[test] +fn idempotent_retain_same_timestamp() { + let conn = test_db(); + let event = sample_event(); + retain_event(&conn, &event).unwrap(); + retain_event(&conn, &event).unwrap(); + + let results = get_retained_personas(&conn, "abc123").unwrap(); + assert_eq!(results.len(), 1); +} + +#[test] +fn inbound_no_local_row_applies() { + let conn = test_db(); + let mut event = sample_event(); + event.pending_sync = false; + + assert_eq!( + retain_inbound_event(&conn, &event).unwrap(), + InboundOutcome::Applied + ); + + let row = get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert_eq!(row.created_at, 1000); + assert!(!row.pending_sync); +} + +#[test] +fn inbound_equal_second_skips_and_preserves_pending() { + let conn = test_db(); + // Pending local edit at t=1000. Same raw-event id as the inbound below + // (an echo / undecidable tie), so the tiebreak cannot decide a winner. + let local = sample_event(); + retain_event(&conn, &local).unwrap(); + + // Inbound at the SAME second with different content. + let inbound = RetainedEvent { + content: r#"{"display_name":"Remote"}"#.to_string(), + pending_sync: false, + ..sample_event() + }; + assert_eq!( + retain_inbound_event(&conn, &inbound).unwrap(), + InboundOutcome::Skipped + ); + + // Local pending row is untouched: flag preserved, content unchanged so + // the flush republishes and the relay resolves last-writer-wins. + let row = get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert!(row.pending_sync); + assert!(row.content.contains("Test")); +} + +/// Issue-3 regression: two devices retain DISTINCT successors in the same +/// second, then each receives the other's. Without a deterministic +/// equal-second winner both sides skip forever and diverge permanently. +/// The NIP-01 tiebreak (lowest event id wins) makes opposite delivery +/// orders converge on the SAME head — the one the relay itself retains. +#[test] +fn inbound_equal_second_opposite_delivery_orders_converge() { + let event_low = RetainedEvent { + content: r#"{"display_name":"Low"}"#.to_string(), + raw_event: r#"{"id":"0aaa"}"#.to_string(), + pending_sync: false, + ..sample_event() + }; + let event_high = RetainedEvent { + content: r#"{"display_name":"High"}"#.to_string(), + raw_event: r#"{"id":"0bbb"}"#.to_string(), + pending_sync: false, + ..sample_event() + }; + + // Device A: low first, then high. High loses the tie — skipped. + let device_a = test_db(); + assert_eq!( + retain_inbound_event(&device_a, &event_low).unwrap(), + InboundOutcome::Applied + ); + assert_eq!( + retain_inbound_event(&device_a, &event_high).unwrap(), + InboundOutcome::Skipped + ); + + // Device B: high first, then low. Low wins the tie — applied. + let device_b = test_db(); + assert_eq!( + retain_inbound_event(&device_b, &event_high).unwrap(), + InboundOutcome::Applied + ); + assert_eq!( + retain_inbound_event(&device_b, &event_low).unwrap(), + InboundOutcome::Applied + ); + + // Both devices converge on the lexically-lowest id. + for conn in [&device_a, &device_b] { + let row = get_retained_event(conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert!( + row.content.contains("Low"), + "both delivery orders must converge on the lowest event id" + ); + } +} + +/// A pending local edit that WINS the equal-second tie keeps its +/// `pending_sync` (the flush republishes it); one that LOSES is superseded +/// by the relay's head and stops republishing a refused event. +#[test] +fn inbound_equal_second_pending_local_winner_and_loser() { + // Local pending edit with the LOWER id: inbound loses, pending stays. + let conn = test_db(); + let local_low = RetainedEvent { + raw_event: r#"{"id":"0aaa"}"#.to_string(), + ..sample_event() + }; + retain_event(&conn, &local_low).unwrap(); + let inbound_high = RetainedEvent { + content: r#"{"display_name":"Remote"}"#.to_string(), + raw_event: r#"{"id":"0bbb"}"#.to_string(), + pending_sync: false, + ..sample_event() + }; + assert_eq!( + retain_inbound_event(&conn, &inbound_high).unwrap(), + InboundOutcome::Skipped + ); + let row = get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert!(row.pending_sync, "the winning local edit keeps its publish"); + + // Local pending edit with the HIGHER id: inbound wins, pending clears. + let conn = test_db(); + let local_high = RetainedEvent { + raw_event: r#"{"id":"0bbb"}"#.to_string(), + ..sample_event() + }; + retain_event(&conn, &local_high).unwrap(); + let inbound_low = RetainedEvent { + content: r#"{"display_name":"Remote"}"#.to_string(), + raw_event: r#"{"id":"0aaa"}"#.to_string(), + pending_sync: false, + ..sample_event() + }; + assert_eq!( + retain_inbound_event(&conn, &inbound_low).unwrap(), + InboundOutcome::Applied + ); + let row = get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert!( + !row.pending_sync, + "the losing local edit stops republishing a head the relay refused" + ); + assert!(row.content.contains("Remote")); +} + +#[test] +fn inbound_strictly_newer_applies_and_clears_pending() { + let conn = test_db(); + // Pending local edit at t=1000. + let local = sample_event(); + retain_event(&conn, &local).unwrap(); + + // Inbound strictly newer with different content. + let inbound = RetainedEvent { + content: r#"{"display_name":"Remote"}"#.to_string(), + created_at: 2000, + pending_sync: false, + ..sample_event() + }; + assert_eq!( + retain_inbound_event(&conn, &inbound).unwrap(), + InboundOutcome::Applied + ); + + // Inbound wins: content replaced and pending cleared, so the stale + // local edit stops republishing instead of looping. + let row = get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert_eq!(row.created_at, 2000); + assert!(!row.pending_sync); + assert!(row.content.contains("Remote")); +} + +#[test] +fn inbound_older_skips() { + let conn = test_db(); + let mut local = sample_event(); + local.created_at = 2000; + retain_event(&conn, &local).unwrap(); + + let inbound = RetainedEvent { + content: r#"{"display_name":"Stale"}"#.to_string(), + created_at: 1000, + pending_sync: false, + ..sample_event() + }; + assert_eq!( + retain_inbound_event(&conn, &inbound).unwrap(), + InboundOutcome::Skipped + ); + + let row = get_retained_event(&conn, 30175, "abc123", "test-persona") + .unwrap() + .unwrap(); + assert_eq!(row.created_at, 2000); + assert!(!row.content.contains("Stale")); +} + +#[test] +fn pending_sync_publishes_tombstones_before_replacements() { + // B5 resurrection race: a kind:5 retained in session N and the same + // coordinate's replacement 30175 retained on the next boot can sit + // pending together. The relay's a-tag deletion ignores timestamps, + // so the tombstone MUST publish first or it wipes the replacement. + let conn = test_db(); + let replacement = RetainedEvent { + kind: 30175, + created_at: 2000, + pending_sync: true, + ..sample_event() + }; + retain_event(&conn, &replacement).unwrap(); + let tombstone = RetainedEvent { + kind: 5, + d_tag: tombstone_retention_d_tag(30175, "test-persona"), + content: String::new(), + created_at: 1000, + pending_sync: true, + ..sample_event() + }; + retain_event(&conn, &tombstone).unwrap(); + + let pending = get_pending_sync(&conn).unwrap(); + assert_eq!(pending.len(), 2); + assert_eq!(pending[0].kind, 5, "tombstone first"); + assert_eq!(pending[1].kind, 30175, "replacement second"); +} + +#[test] +fn deferral_predicate_is_kind_and_pubkey_qualified() { + // Mid-sweep barrier semantics: a failed tombstone defers ONLY the + // replacement at its exact coordinate — same target kind, same pubkey. + use std::collections::HashSet; + + let failed: HashSet<(String, String)> = HashSet::from([( + "abc123".to_string(), + tombstone_retention_d_tag(30175, "test-persona"), + )]); + + // The covered replacement defers. + assert!(deferred_behind_failed_tombstone( + 30175, + "abc123", + "test-persona", + &failed + )); + // Kind-qualified: a coinciding slug under a DIFFERENT kind is a + // distinct coordinate (the cross-kind collision the retention d-tag + // encoding exists to prevent) — never deferred. + assert!(!deferred_behind_failed_tombstone( + 30177, + "abc123", + "test-persona", + &failed + )); + // Never crosses pubkeys. + assert!(!deferred_behind_failed_tombstone( + 30175, + "other-key", + "test-persona", + &failed + )); + // Never defers kind:5 rows, even at a "matching" retention key. + assert!(!deferred_behind_failed_tombstone( + 5, + "abc123", + "test-persona", + &failed + )); + // Unrelated d-tags publish normally. + assert!(!deferred_behind_failed_tombstone( + 30175, + "abc123", + "other-persona", + &failed + )); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index c0e55184b19..8d610a8cb39 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -236,6 +236,35 @@ pub fn start_managed_agent_runtime( start_managed_agent_runtime_pair_lazy(pubkey, relay_url, app) } +/// Fold the relay-resolved record into the exact bytes a pair spawn executes. +/// +/// Pure over `(resolved, personas)` so the final-use boundary is testable +/// without a live `AppHandle` (same seam strategy as +/// `finalize_restore_candidate` in `restore.rs`): +/// - a RESOLVED non-local backend is refused by name — pair runtimes are a +/// local-process concept, and a relay head that migrated the agent to a +/// provider backend must not spawn a local child from leftover disk bytes; +/// - the linked persona snapshot is re-applied LAST so the definition quad +/// (prompt/model/provider/runtime) keeps its established precedence over +/// both disk and relay bytes; +/// - an orphaned instance (persona_id with no live persona) passes through: +/// `spawn_agent_child` owns that refusal via `resolve_effective_config`. +fn resolve_pair_spawn_record( + mut resolved: super::ManagedAgentRecord, + personas: &[super::AgentDefinition], +) -> Result { + if resolved.backend != BackendKind::Local { + return Err("managed runtime pairs require a local agent".into()); + } + if let Some(persona_id) = resolved.persona_id.clone() { + if let Some(persona) = personas.iter().find(|persona| persona.id == persona_id) { + super::persona_events::apply_persona_snapshot(&mut resolved, persona); + resolved.updated_at = crate::util::now_iso(); + } + } + Ok(resolved) +} + fn start_pair( pubkey: String, relay_url: String, @@ -244,6 +273,13 @@ fn start_pair( app: AppHandle, ) -> Result { let state = app.state::(); + // A relay-only record (hydrated overlay head with no disk row) needs a + // durable device-local lifecycle anchor before the disk lookup below — + // exactly like the interactive start path. Without this, pair + // Start/Restart for a relay-only card fails "agent not found" instead of + // materializing it; a relay-only PROVIDER card is refused here by name. + // Takes and releases its own locks, so it must run before ours. + super::private_config_overlay::materialize_relay_only_agent(&app, &state, &pubkey)?; let _transition = state .managed_agent_runtime_transition .lock() @@ -257,12 +293,19 @@ fn start_pair( .map_err(|e| e.to_string())?; let mut records = load_managed_agents(&app)?; let record = find_managed_agent_mut(&mut records, &pubkey)?; - if record.backend != BackendKind::Local { - return Err("managed runtime pairs require a local agent".into()); - } if expected_updated_at.is_some_and(|expected| record.updated_at != expected) { return Err("managed agent changed while runtime reconciliation was in flight".into()); } + // Final-use boundary: the spawn below executes the relay-primary resolve + // of the disk row (persona snapshot re-applied last), never raw disk + // bytes — otherwise a follower device showing relay config B would + // execute stale disk config A. Lifecycle mutations further down still + // land on the DISK `record`; relay-owned configuration is never written + // back to the device-local store. + let spawn_record = resolve_pair_spawn_record( + super::private_config_overlay::resolved_local_record(&state, record)?, + &load_personas(&app).unwrap_or_default(), + )?; let key = ManagedAgentRuntimeKey::new(pubkey, &relay_url)?; let mut runtimes = state .managed_agent_processes @@ -272,7 +315,7 @@ fn start_pair( .get_mut(&key) .is_some_and(|runtime| runtime.child.try_wait().ok().flatten().is_none()) { - let status = status_for(&app, record, &key, runtimes.get(&key), None); + let status = status_for(&app, &spawn_record, &key, runtimes.get(&key), None); return Ok(status); } runtimes.remove(&key); @@ -283,7 +326,8 @@ 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, &spawn_record, &key.relay_url, lazy, owner.as_deref())?; let now = crate::util::now_iso(); let receipt = ManagedAgentRuntimeReceipt { key: key.clone(), @@ -302,7 +346,7 @@ fn start_pair( record.last_stopped_at = None; record.last_error = None; runtimes.insert(key.clone(), ManagedAgentPairRuntime::starting(process)); - let status = status_for(&app, record, &key, runtimes.get(&key), None); + let status = status_for(&app, &spawn_record, &key, runtimes.get(&key), None); drop(runtimes); save_managed_agents(&app, &records)?; emit_status(&app, &status); @@ -382,6 +426,13 @@ pub fn restart_managed_agent_runtime( relay_url: String, app: AppHandle, ) -> Result { + // Materialize a relay-only record before the stop half, which fails + // "agent not found" on a pubkey with no disk row. `start_pair` would + // materialize anyway, but only after stop has already failed the restart. + { + let state = app.state::(); + super::private_config_overlay::materialize_relay_only_agent(&app, &state, &pubkey)?; + } stop_managed_agent_runtime(pubkey.clone(), relay_url.clone(), app.clone())?; start_pair(pubkey, relay_url, true, None, app) } @@ -464,26 +515,50 @@ pub async fn reconcile_managed_agent_runtimes( use futures_util::{stream, StreamExt}; let records = load_managed_agents(&app)?; + // Fan-out candidates resolve through the relay-primary overlay BEFORE the + // probe: the probe authenticates as the agent (nsec + auth tag), and a + // follower device may hold a rotated identity only in the overlay. + // `start_on_app_launch` is device-local (never patched), so the + // auto-start choice itself still reads disk; the backend gate reads the + // RESOLVED record so a relay head that migrated an agent off the local + // backend is skipped instead of spawned from leftover disk bytes. The + // raw disk `updated_at` is carried alongside as the in-flight guard + // `start_pair` re-checks against the disk row it reloads. + let candidates = { + let state = app.state::(); + let mut candidates = Vec::new(); + for record in records.iter().filter(|record| record.start_on_app_launch) { + let resolved = super::private_config_overlay::resolved_local_record(&state, record)?; + if resolved.backend != BackendKind::Local { + continue; + } + candidates.push((resolved, record.updated_at.clone())); + } + candidates + }; let mut jobs = Vec::new(); for community in communities { - for record in records - .iter() - .filter(|record| record.start_on_app_launch && record.backend == BackendKind::Local) + for (record, disk_updated_at) in &candidates // The legacy per-record relay pin is deliberately ignored here — see // `effective_agent_relay_url`. Every local auto-start agent fans out // to every configured community. { - jobs.push((record.clone(), community.relay_url.clone())); + jobs.push(( + record.clone(), + disk_updated_at.clone(), + community.relay_url.clone(), + )); } } let probes: Vec<_> = stream::iter(jobs) - .map(|(record, requested)| { + .map(|(record, disk_updated_at, requested)| { let state = app.state::(); async move { let fallback_record = record.clone(); let fallback_requested = requested.clone(); probe_agent_relay_access(&state, record, requested) .await + .map(|(record, key, requested)| (record, key, requested, disk_updated_at)) .map_err(|error| (fallback_record, fallback_requested, error)) } }) @@ -501,12 +576,12 @@ pub async fn reconcile_managed_agent_runtimes( let mut rows = Vec::new(); for probe in probes { match probe { - Ok((record, key, requested)) => { + Ok((record, key, requested, disk_updated_at)) => { match start_pair( record.pubkey.clone(), key.relay_url.clone(), true, - Some(&record.updated_at), + Some(&disk_updated_at), app.clone(), ) { Ok(mut status) => { @@ -714,3 +789,168 @@ mod tests { assert!(observer_lifecycle_key(&ready_with_error.pubkey, &ready_with_error).is_err()); } } + +// ── Pair-spawn fold: relay-overlay resolve at the final-use boundary ───────── +// +// The production wiring (`start_pair` resolving the disk row through +// `resolved_local_record` before `spawn_agent_child`) needs a live +// `AppHandle`, so its presence is pinned by `write_site_resolve_guard` in +// `private_config_overlay.rs`. These tests prove the fold itself at the same +// overlay + finalize seam the production path composes — the pair-start +// variant of `restore_fold_tests`. +#[cfg(test)] +mod pair_spawn_resolve_tests { + use super::resolve_pair_spawn_record; + use crate::managed_agents::private_config_overlay::{test_relay_payload, PrivateConfigOverlay}; + use crate::managed_agents::{AgentDefinition, BackendKind, ManagedAgentRecord}; + use std::collections::BTreeMap; + + /// A stale disk row as `start_pair` reloads it under its store lock. + fn stale_disk_record(pubkey: &str) -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": pubkey, + "name": "stale disk name", + "private_key_nsec": "nsec-stale-disk", + "relay_url": "wss://old.example", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": "stale disk prompt", + "model": "stale-model", + "provider": null, + "env_vars": {}, + "start_on_app_launch": true, + "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("stale_disk_record fixture") + } + + /// Carl round-9 P1 regression (stale-disk A / overlay B at pair start): + /// the record handed to `spawn_agent_child` must carry the relay head for + /// everything relay-owned, while device-local lifecycle state + /// (`start_on_app_launch`) survives — previously a follower device + /// showing relay config B pair-started stale disk config A. + #[test] + fn pair_start_spawns_relay_config_not_stale_disk() { + let pubkey = "aa".repeat(32); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(test_relay_payload(&pubkey)).unwrap(); + + let disk = stale_disk_record(&pubkey); + let spawn = resolve_pair_spawn_record(overlay.resolve_local_record(&disk), &[]) + .expect("local resolved record must survive the fold"); + + assert_eq!(spawn.name, "relay name"); + assert_eq!(spawn.system_prompt.as_deref(), Some("relay prompt")); + assert_eq!(spawn.model.as_deref(), Some("relay-model")); + assert_eq!(spawn.private_key_nsec, "nsec-relay"); + assert_eq!(spawn.parallelism, 4); + assert!( + spawn.start_on_app_launch, + "device-local lifecycle flag must survive the overlay resolve" + ); + + // NEGATIVE CONTROL: an empty overlay leaves the disk record as-is — + // the assertions above prove the patch, not the fixture. + let untouched = resolve_pair_spawn_record( + PrivateConfigOverlay::default().resolve_local_record(&disk), + &[], + ) + .unwrap(); + assert_eq!(untouched.name, "stale disk name"); + assert_eq!(untouched.private_key_nsec, "nsec-stale-disk"); + } + + /// The linked persona keeps its definition-authoritative precedence over + /// BOTH disk and relay bytes — the snapshot is re-applied after the + /// overlay patch, mirroring the interactive start and restore paths. + #[test] + fn persona_snapshot_reapplies_on_top_of_overlay_patch() { + let pubkey = "bb".repeat(32); + let mut payload = test_relay_payload(&pubkey); + payload.config.persona_id = Some("def-1".into()); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(payload).unwrap(); + + let persona = AgentDefinition { + id: "def-1".into(), + display_name: "Definition".into(), + avatar_url: None, + system_prompt: "definition prompt".into(), + runtime: Some("goose".into()), + model: Some("definition-model".into()), + provider: None, + name_pool: vec![], + 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![], + parallelism: None, + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + }; + + let resolved = overlay.resolve_local_record(&stale_disk_record(&pubkey)); + let spawn = resolve_pair_spawn_record(resolved, std::slice::from_ref(&persona)).unwrap(); + assert_eq!(spawn.system_prompt.as_deref(), Some("definition prompt")); + assert_eq!(spawn.model.as_deref(), Some("definition-model")); + // Relay still owns what the definition does not. + assert_eq!(spawn.name, "relay name"); + assert_eq!(spawn.private_key_nsec, "nsec-relay"); + } + + /// A relay head that migrated the agent off the local backend must be + /// refused by name — never spawned as a local child from leftover disk + /// bytes. + #[test] + fn resolved_non_local_backend_is_refused() { + let pubkey = "cc".repeat(32); + let mut record = stale_disk_record(&pubkey); + record.backend = BackendKind::Provider { + id: "cloud".into(), + config: serde_json::json!({}), + }; + let error = resolve_pair_spawn_record(record, &[]).unwrap_err(); + assert_eq!(error, "managed runtime pairs require a local agent"); + } + + /// Carl round-9 P1, relay-only arm: a hydrated overlay head with no disk + /// row materializes into a spawnable record on the pair route (previously + /// "agent not found"); a relay-only PROVIDER head is refused by name at + /// the same fold. + #[test] + fn relay_only_record_materializes_for_pair_start() { + let pubkey = "dd".repeat(32); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(test_relay_payload(&pubkey)).unwrap(); + + let materialized = overlay + .materialize_relay_only_record(&pubkey, &[]) + .expect("relay-only head must materialize"); + let spawn = resolve_pair_spawn_record(materialized, &[]).unwrap(); + assert_eq!(spawn.name, "relay name"); + assert_eq!(spawn.private_key_nsec, "nsec-relay"); + assert_eq!(spawn.backend, BackendKind::Local); + + let mut provider = test_relay_payload(&"ee".repeat(32)); + provider.config.backend = serde_json::json!({"type":"provider","id":"cloud","config":{}}); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert(provider).unwrap(); + let materialized = overlay + .materialize_relay_only_record(&"ee".repeat(32), &[]) + .unwrap(); + assert!(resolve_pair_spawn_record(materialized, &[]).is_err()); + } +} diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 652bb9b9ea8..5ad4552bd0c 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -15,7 +15,7 @@ use crate::secret_store::{KeyringProbe, SecretStore}; /// Keyring key name for an agent's nsec, namespaced from the human identity /// key (`"identity"`) which shares the service. -fn agent_keyring_name(pubkey: &str) -> String { +pub(super) fn agent_keyring_name(pubkey: &str) -> String { format!("agent:{pubkey}") } @@ -24,7 +24,7 @@ fn agent_keyring_name(pubkey: &str) -> String { /// `SecretStore::shared` so identity and agent callers share one instance — /// and therefore one in-memory cache and one mutex — preventing last-writer-wins /// races on concurrent blob writes. -fn agent_secret_store() -> Option<&'static SecretStore> { +pub(super) fn agent_secret_store() -> Option<&'static SecretStore> { if cfg!(feature = "system-keyring") { Some(SecretStore::shared(keyring_service())) } else { @@ -131,7 +131,7 @@ fn newest_agent_log_in_dir(dir: &Path, pubkey: &str) -> Option { /// The keyring operations the migration chokepoint needs. Abstracted so the /// migrate-and-strip decision logic ([`migrate_inline_key`]) can be unit-tested /// against a fake without touching the live OS keyring. -trait KeyStore { +pub(super) trait KeyStore { fn probe(&self, name: &str) -> KeyringProbe; /// Read a key. `Ok(None)` is "no such entry" (absent); `Err` is a backend /// failure (keyring unreachable) — the caller MUST NOT collapse the two. @@ -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; }; @@ -317,7 +317,7 @@ fn hydrate_keys(records: &mut [ManagedAgentRecord]) { /// to spawn an agent whose key could not be read (see the empty-key bail in /// `spawn_agent_child`). Empty here never means "fine" — it means "no usable /// key this boot." -fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) { +pub(super) fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) { for record in records.iter_mut() { // A key-less definition (no pubkey yet — unified agent model) has no // keyring entry by construction; keys are minted on first start. @@ -906,4 +906,4 @@ pub fn meaningful_agent_error_from_log(path: &Path) -> Option { #[cfg(test)] #[path = "storage_tests.rs"] -mod tests; +pub(super) mod tests; diff --git a/desktop/src-tauri/src/managed_agents/storage_tests.rs b/desktop/src-tauri/src/managed_agents/storage_tests.rs index 9943c6b3ac3..b755b896861 100644 --- a/desktop/src-tauri/src/managed_agents/storage_tests.rs +++ b/desktop/src-tauri/src/managed_agents/storage_tests.rs @@ -19,7 +19,11 @@ use super::{ /// In-memory [`KeyStore`] for testing the migrate decision without the OS /// keyring. `reachable=false` simulates a backend outage; `fail_verify` /// simulates a write whose read-back does not confirm. -struct FakeKeyStore { +/// +/// `pub(in crate::managed_agents)` so sibling test modules (e.g. +/// `reconcile::tests`) can inject it through the same seam instead of +/// touching the live keyring. +pub(in crate::managed_agents) struct FakeKeyStore { reachable: bool, fail_verify: bool, stored: RefCell>, @@ -28,7 +32,7 @@ struct FakeKeyStore { } impl FakeKeyStore { - fn reachable() -> Self { + pub(in crate::managed_agents) fn reachable() -> Self { Self { reachable: true, fail_verify: false, @@ -56,7 +60,7 @@ impl FakeKeyStore { } } /// Seed a key as already present in the keyring. - fn with_key(self, name: &str, value: &str) -> Self { + pub(in crate::managed_agents) fn with_key(self, name: &str, value: &str) -> Self { self.stored .borrow_mut() .insert(name.to_string(), value.to_string()); diff --git a/desktop/src/features/agents/lib/usePersonaSync.test.mjs b/desktop/src/features/agents/lib/usePersonaSync.test.mjs index cfc0d901c3a..2a58c452299 100644 --- a/desktop/src/features/agents/lib/usePersonaSync.test.mjs +++ b/desktop/src/features/agents/lib/usePersonaSync.test.mjs @@ -6,6 +6,7 @@ import { KIND_DELETION, KIND_MANAGED_AGENT, KIND_PERSONA, + KIND_PRIVATE_MANAGED_AGENT, KIND_TEAM, } from "@/shared/constants/kinds"; import { @@ -17,6 +18,7 @@ const EXPECTED_KINDS = [ KIND_PERSONA, KIND_TEAM, KIND_MANAGED_AGENT, + KIND_PRIVATE_MANAGED_AGENT, KIND_DELETION, ]; diff --git a/desktop/src/features/agents/lib/usePersonaSync.ts b/desktop/src/features/agents/lib/usePersonaSync.ts index 57d33089a9b..b7f0d463609 100644 --- a/desktop/src/features/agents/lib/usePersonaSync.ts +++ b/desktop/src/features/agents/lib/usePersonaSync.ts @@ -7,6 +7,7 @@ import { KIND_DELETION, KIND_MANAGED_AGENT, KIND_PERSONA, + KIND_PRIVATE_MANAGED_AGENT, KIND_TEAM, } from "@/shared/constants/kinds"; @@ -17,6 +18,7 @@ const PERSONA_SYNC_KINDS = [ KIND_PERSONA, KIND_TEAM, KIND_MANAGED_AGENT, + KIND_PRIVATE_MANAGED_AGENT, KIND_DELETION, ]; diff --git a/desktop/src/shared/constants/kinds.ts b/desktop/src/shared/constants/kinds.ts index 4f8b7afe2bd..e4ab859ad37 100644 --- a/desktop/src/shared/constants/kinds.ts +++ b/desktop/src/shared/constants/kinds.ts @@ -57,6 +57,8 @@ export const KIND_COMMUNITY_THEME = 30078; export const KIND_PERSONA = 30175; export const KIND_TEAM = 30176; export const KIND_MANAGED_AGENT = 30177; +// Owner-authored, owner-readable encrypted runnable configuration. +export const KIND_PRIVATE_MANAGED_AGENT = 30179; export const KIND_USER_STATUS = 30315; export const KIND_AGENT_OBSERVER_FRAME = 24200; export const KIND_AGENT_TURN_METRIC = 44200;