diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index a51809efa..24d6125d7 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -5,7 +5,7 @@ use tauri::AppHandle; use crate::{ managed_agents::{ managed_agents_base_dir, - persona_events::persona_from_event, + persona_events::{persona_from_event, PersonaEventContent}, retention::{get_retained_personas, has_retained_personas, open_retention_db}, PersonaRecord, }, @@ -733,10 +733,22 @@ fn retention_db_path(app: &AppHandle) -> Result { Ok(managed_agents_base_dir(app)?.join("retention.db")) } +/// Placeholder pubkey used by migration when signing keys are unavailable. +const UNKEYED_MIGRATION_PUBKEY: &str = "unkeyed"; + /// Load personas from the retention store, returning them as PersonaRecords. /// /// Returns `Ok(None)` if the retention DB doesn't exist or has no personas /// for the current user pubkey. +/// +/// Handles two row types: +/// - Rows with a valid signed `raw_event`: parsed via `persona_from_event`. +/// - Rows with an empty or placeholder `raw_event` (written when signing keys +/// were unavailable during migration): reconstructed from the `content` +/// column directly. +/// +/// Also picks up "unkeyed" migration rows that were written before the +/// identity key was resolved. #[allow(dead_code)] fn load_from_retention( app: &AppHandle, @@ -748,16 +760,29 @@ fn load_from_retention( } let conn = open_retention_db(&db_path)?; - if !has_retained_personas(&conn, pubkey)? { + + // Query rows for the real pubkey and any unkeyed migration rows. + let has_keyed = has_retained_personas(&conn, pubkey)?; + let has_unkeyed = has_retained_personas(&conn, UNKEYED_MIGRATION_PUBKEY)?; + if !has_keyed && !has_unkeyed { return Ok(None); } - let retained = get_retained_personas(&conn, pubkey)?; + let mut retained = get_retained_personas(&conn, pubkey)?; + if has_unkeyed { + let unkeyed = get_retained_personas(&conn, UNKEYED_MIGRATION_PUBKEY)?; + // Only include unkeyed rows whose d_tag isn't already covered by a + // properly-keyed row (keyed rows take precedence). + for row in unkeyed { + if !retained.iter().any(|r| r.d_tag == row.d_tag) { + retained.push(row); + } + } + } + let mut records = Vec::with_capacity(retained.len()); for row in &retained { - let event: nostr::Event = serde_json::from_str(&row.raw_event) - .map_err(|e| format!("failed to parse retained event: {e}"))?; - match persona_from_event(&event) { + match persona_from_retained_row(row) { Ok(record) => records.push(record), Err(e) => { eprintln!( @@ -775,6 +800,44 @@ fn load_from_retention( } } +/// Reconstruct a `PersonaRecord` from a retention row. +/// +/// Tries the signed `raw_event` path first; falls back to parsing `content` +/// directly when the raw event is empty or invalid (migration without keys). +fn persona_from_retained_row( + row: &crate::managed_agents::retention::RetainedEvent, +) -> Result { + // Try parsing raw_event as a valid nostr::Event first. + if !row.raw_event.is_empty() { + if let Ok(event) = serde_json::from_str::(&row.raw_event) { + return persona_from_event(&event); + } + } + + // Fallback: parse content column directly as PersonaEventContent. + let content: PersonaEventContent = serde_json::from_str(&row.content) + .map_err(|e| format!("failed to parse persona content: {e}"))?; + + let now = crate::util::now_iso(); + Ok(PersonaRecord { + id: row.d_tag.clone(), + display_name: content.display_name, + avatar_url: content.avatar_url, + system_prompt: content.system_prompt, + runtime: content.runtime, + model: content.model, + provider: content.provider, + name_pool: content.name_pool, + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: Some(row.d_tag.clone()), + env_vars: content.env_vars, + created_at: now.clone(), + updated_at: now, + }) +} + pub fn load_personas(app: &AppHandle) -> Result, String> { let path = personas_store_path(app)?; let now = now_iso(); diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index e513d4c58..a3f2f92a2 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -460,3 +460,120 @@ fn migrate_is_idempotent() { // 4. Run again on result of (3) — should be no-op. assert!(!migrate_retired_personas(&mut stored_pre_demotion, now)); } + +// --- persona_from_retained_row round-trip tests --- + +#[test] +fn retained_row_with_valid_signed_event_round_trips() { + use crate::managed_agents::persona_events::build_persona_event; + use crate::managed_agents::retention::RetainedEvent; + use nostr::JsonUtil; + use sprout_core::kind::KIND_PERSONA; + + let record = custom_persona("my-persona", "My Persona"); + let keys = nostr::Keys::generate(); + let builder = build_persona_event(&record).unwrap(); + let event = builder.sign_with_keys(&keys).unwrap(); + + let retained = RetainedEvent { + kind: KIND_PERSONA, + pubkey: keys.public_key().to_hex(), + d_tag: "my-persona".to_string(), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: false, + }; + + let restored = super::persona_from_retained_row(&retained).unwrap(); + assert_eq!(restored.display_name, "My Persona"); + assert_eq!(restored.system_prompt, "Custom prompt"); + assert_eq!( + restored.avatar_url, + Some("https://example.com/avatar.png".to_string()) + ); +} + +#[test] +fn retained_row_with_empty_raw_event_falls_back_to_content() { + use crate::managed_agents::retention::RetainedEvent; + use sprout_core::kind::KIND_PERSONA; + + let content_json = serde_json::json!({ + "display_name": "Unkeyed Persona", + "system_prompt": "You are helpful.", + "avatar_url": "https://example.com/pic.png", + "runtime": "goose", + "model": "claude-opus-4", + "provider": "anthropic", + "name_pool": ["Alpha"], + "env_vars": {"KEY": "val"}, + }); + + let retained = RetainedEvent { + kind: KIND_PERSONA, + pubkey: "unkeyed".to_string(), + d_tag: "test-slug".to_string(), + content: content_json.to_string(), + created_at: 1000, + raw_event: String::new(), + pending_sync: true, + }; + + let restored = super::persona_from_retained_row(&retained).unwrap(); + assert_eq!(restored.id, "test-slug"); + assert_eq!(restored.display_name, "Unkeyed Persona"); + assert_eq!(restored.system_prompt, "You are helpful."); + assert_eq!( + restored.avatar_url, + Some("https://example.com/pic.png".to_string()) + ); + assert_eq!(restored.runtime, Some("goose".to_string())); + assert_eq!(restored.model, Some("claude-opus-4".to_string())); + assert_eq!(restored.provider, Some("anthropic".to_string())); + assert_eq!(restored.name_pool, vec!["Alpha"]); + assert_eq!(restored.env_vars.get("KEY"), Some(&"val".to_string())); + assert_eq!( + restored.source_team_persona_slug, + Some("test-slug".to_string()) + ); + assert!(!restored.is_builtin); + assert!(restored.is_active); +} + +#[test] +fn retained_row_with_invalid_raw_event_falls_back_to_content() { + use crate::managed_agents::retention::RetainedEvent; + use sprout_core::kind::KIND_PERSONA; + + // Simulate the old broken synthetic event that can't parse as nostr::Event + let content_json = serde_json::json!({ + "display_name": "Broken Synthetic", + "system_prompt": "Hello", + }); + + let bad_raw = serde_json::json!({ + "id": "migration-placeholder-test", + "pubkey": "0000000000000000000000000000000000000000000000000000000000000000", + "kind": KIND_PERSONA, + "created_at": 1000, + "content": content_json.to_string(), + "tags": [["d", "test"]], + "sig": "0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + }); + + let retained = RetainedEvent { + kind: KIND_PERSONA, + pubkey: "0000000000000000000000000000000000000000000000000000000000000000".to_string(), + d_tag: "test".to_string(), + content: content_json.to_string(), + created_at: 1000, + raw_event: bad_raw.to_string(), + pending_sync: true, + }; + + let restored = super::persona_from_retained_row(&retained).unwrap(); + assert_eq!(restored.display_name, "Broken Synthetic"); + assert_eq!(restored.system_prompt, "Hello"); + assert_eq!(restored.id, "test"); +} diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index b2c69995c..919ebccbd 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -573,11 +573,16 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { /// retention store. /// /// Must run AFTER `migrate_packs_to_teams` (depends on field renames being -/// complete). Idempotent: checks a sentinel file before running. +/// complete). Idempotent: skips if retention.db already has rows. /// /// Strategy: write to local SQLite retention first (durable copy), mark as /// `pending_sync = 1` for later relay publish. Migration succeeds on local /// write, not relay acknowledgment. + +/// Placeholder pubkey used when migration runs without signing keys. +/// `load_from_retention` queries for this value in addition to the real pubkey. +const UNKEYED_MIGRATION_PUBKEY: &str = "unkeyed"; + pub fn migrate_personas_to_events(app: &tauri::AppHandle) { use crate::managed_agents::{ managed_agents_base_dir, @@ -592,13 +597,20 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { return; }; - // Check sentinel — skip if already migrated. - let sentinel_path = base_dir.join("migration_state.json"); - if sentinel_path.exists() { - if let Ok(content) = std::fs::read_to_string(&sentinel_path) { - if content.contains(r#""persona_events_migrated":true"#) - || content.contains(r#""persona_events_migrated": true"#) - { + // Idempotency: if retention.db already has any persona rows, migration + // already ran. This replaces the old sentinel file approach which caused + // write amplification on every launch. + let db_path = base_dir.join("retention.db"); + if db_path.exists() { + if let Ok(conn) = open_retention_db(&db_path) { + let has_rows: bool = conn + .query_row( + "SELECT EXISTS(SELECT 1 FROM persona_events LIMIT 1)", + [], + |row| row.get(0), + ) + .unwrap_or(false); + if has_rows { return; } } @@ -607,8 +619,6 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { // Read personas.json fresh at migration time. let personas_path = base_dir.join("personas.json"); if !personas_path.exists() { - // No personas to migrate — write sentinel and return. - let _ = std::fs::write(&sentinel_path, r#"{"persona_events_migrated":true}"#); return; } @@ -631,7 +641,6 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { }; if records.is_empty() { - let _ = std::fs::write(&sentinel_path, r#"{"persona_events_migrated":true}"#); return; } @@ -645,16 +654,19 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { } }; - // Get the user's pubkey for the event. We need keys to sign events for - // the raw_event field, but during migration we may not have the app state - // initialized yet. Use a deterministic placeholder approach: store the - // persona content without a real signature. The raw_event will be a - // minimal JSON structure that persona_from_event can parse. - // - // We use the keys from the environment or identity file if available. + // Resolve signing keys: env var first, then persisted identity.key file. + // Migration runs before resolve_persisted_identity(), but the file exists + // from prior launches. Only truly first-ever launches lack both sources — + // and those have no personas.json to migrate either. let keys = std::env::var("SPROUT_PRIVATE_KEY") .ok() - .and_then(|k| k.parse::().ok()); + .and_then(|k| k.parse::().ok()) + .or_else(|| { + let data_dir = app.path().app_data_dir().ok()?; + let key_path = data_dir.join("identity.key"); + let content = std::fs::read_to_string(&key_path).ok()?; + content.trim().parse::().ok() + }); let mut migrated = 0u32; let mut errors = 0u32; @@ -716,8 +728,10 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { } } None => { - // No keys available — store a synthetic event structure. - // This will be re-signed and published when keys become available. + // No keys available — store content directly without a signed + // event. load_from_retention handles these rows by parsing the + // content column as PersonaEventContent. Uses a placeholder + // pubkey so rows can be found and re-keyed later. let content_json = serde_json::json!({ "display_name": record.display_name, "avatar_url": record.avatar_url, @@ -730,24 +744,14 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { }); let now = chrono::Utc::now().timestamp(); - let raw_event = serde_json::json!({ - "id": format!("migration-placeholder-{}", d_tag), - "pubkey": "0000000000000000000000000000000000000000000000000000000000000000", - "kind": KIND_PERSONA, - "created_at": now, - "content": content_json.to_string(), - "tags": [["d", d_tag]], - "sig": "0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" - }); let retained = RetainedEvent { kind: KIND_PERSONA, - pubkey: "0000000000000000000000000000000000000000000000000000000000000000" - .to_string(), + pubkey: UNKEYED_MIGRATION_PUBKEY.to_string(), d_tag, content: content_json.to_string(), created_at: now, - raw_event: raw_event.to_string(), + raw_event: String::new(), pending_sync: true, }; @@ -764,10 +768,9 @@ pub fn migrate_personas_to_events(app: &tauri::AppHandle) { } } - // Write sentinel regardless of partial errors — individual failures are - // logged and the migration is best-effort. Re-running won't help if a - // specific persona can't be serialized. - let _ = std::fs::write(&sentinel_path, r#"{"persona_events_migrated":true}"#); + // Migration is best-effort. Individual failures are logged above. + // Idempotency is handled by the DB row check at function entry — + // no sentinel file needed. if migrated > 0 || errors > 0 { eprintln!(